Messages in this thread | | | Subject | Re: [PATCH 07/10] clk: qcom: Add graphics clock controller driver for SM8150 | From | Jonathan Marek <> | Date | Mon, 29 Jun 2020 13:27:21 -0400 |
| |
On 5/27/20 4:44 AM, Stephen Boyd wrote: > Quoting Jonathan Marek (2020-05-24 14:06:08) >> diff --git a/drivers/clk/qcom/gpucc-sm8150.c b/drivers/clk/qcom/gpucc-sm8150.c >> new file mode 100644 >> index 000000000000..6e1fff0cde75 >> --- /dev/null >> +++ b/drivers/clk/qcom/gpucc-sm8150.c >> @@ -0,0 +1,429 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved. >> + */ >> + >> +#include <linux/clk-provider.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> + >> +#include <dt-bindings/clock/qcom,gpucc-sm8150.h> > [..] >> + >> +static struct clk_rcg2 gpu_cc_gmu_clk_src = { >> + .cmd_rcgr = 0x1120, >> + .mnd_width = 0, >> + .hid_width = 5, >> + .parent_map = gpu_cc_parent_map_0, >> + .freq_tbl = ftbl_gpu_cc_gmu_clk_src, >> + .clkr.hw.init = &(struct clk_init_data){ >> + .name = "gpu_cc_gmu_clk_src", >> + .parent_names = gpu_cc_parent_names_0, >> + .num_parents = 6, >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_rcg2_ops, >> + }, >> +}; >> + >> +static struct clk_branch gpu_cc_ahb_clk = { >> + .halt_reg = 0x1078, >> + .halt_check = BRANCH_HALT_DELAY, >> + .clkr = { >> + .enable_reg = 0x1078, >> + .enable_mask = BIT(0), >> + .hw.init = &(struct clk_init_data){ >> + .name = "gpu_cc_ahb_clk", >> + .flags = CLK_IS_CRITICAL, > > Why is this CLK_IS_CRITICAL? Why not just enable the clk manually with > a register write in probe and then remove this clk from the system? We > can save some memory that way. >
I copied this from downstream, so I don't know why. It works without it, so I removed the CLK_IS_CRITICAL in my v2 (from both sm8150 and sm8250).
>> + .ops = &clk_branch2_ops, >> + }, >> + }, >> +}; >> + > [...] >> + >> +static struct gdsc gpu_cx_gdsc = { >> + .gdscr = 0x106c, >> + .gds_hw_ctrl = 0x1540, >> + .pd = { >> + .name = "gpu_cx_gdsc", >> + }, >> + .pwrsts = PWRSTS_OFF_ON, >> + .flags = VOTABLE, >> +}; >> + >> +/* see comment in gpucc-sdm845 about this */ >> +static int gx_gdsc_enable(struct generic_pm_domain *domain) >> +{ >> + /* Do nothing but give genpd the impression that we were successful */ >> + return 0; >> +} > > Maybe we should export a helper from gdsc.c for this with the comment > and it named something obvious? gx_gdsc_do_nothing_enable()? >
Made this change in my v2.
>> + >> +static struct gdsc gpu_gx_gdsc = { >> + .gdscr = 0x100c, >> + .clamp_io_ctrl = 0x1508, >> + .pd = { >> + .name = "gpu_gx_gdsc", >> + .power_on = gx_gdsc_enable, >> + }, >> + .pwrsts = PWRSTS_OFF_ON, >> + .flags = CLAMP_IO | AON_RESET | POLL_CFG_GDSCR, >> +}; >> +
| |