Quoting Taniya Das (2019-11-15 00:11:28) > Hi Stephen, > > Thanks for the review. > > On 11/6/2019 6:00 AM, Stephen Boyd wrote: > > Quoting Taniya Das (2019-10-31 05:21:10) > >> diff --git a/drivers/clk/qcom/gpucc-sc7180.c b/drivers/clk/qcom/gpucc-sc7180.c > >> new file mode 100644 > >> index 0000000..0d893e6 > >> --- /dev/null > >> +++ b/drivers/clk/qcom/gpucc-sc7180.c > >> @@ -0,0 +1,274 @@ > >> +// SPDX-License-Identifier: GPL-2.0-only > >> +/* > >> + * Copyright (c) 2019, The Linux Foundation. All rights reserved. > >> + */ > >> + > >> +#include <linux/clk-provider.h> > >> +#include <linux/err.h> > >> +#include <linux/kernel.h> > >> +#include <linux/module.h> > >> +#include <linux/of.h> > >> +#include <linux/of_device.h> > > > > Are these of includes used? > > > > yes, would clean up these headers. > Maybe they're used. I'm not sure. > >> + regmap = qcom_cc_map(pdev, &gpu_cc_sc7180_desc); > >> + if (IS_ERR(regmap)) > >> + return PTR_ERR(regmap); > >> + > >> + /* 360MHz Configuration */ > >> + gpu_cc_pll_config.l = 0x12; > >> + gpu_cc_pll_config.alpha = 0xC000; > >> + gpu_cc_pll_config.config_ctl_val = 0x20485699; > >> + gpu_cc_pll_config.config_ctl_hi_val = 0x00002067; > >> + gpu_cc_pll_config.user_ctl_val = 0x00000001; > >> + gpu_cc_pll_config.user_ctl_hi_val = 0x00004805; > >> + gpu_cc_pll_config.test_ctl_hi_val = 0x40000000; > > > > Is there a reason this is built on the stack? Save space or something? > > > > I have done as we had discussed during the dispcc review for SDM845 > https://patchwork.kernel.org/patch/10446073/ > >>> > >> +static const struct alpha_pll_config disp_cc_pll0_config = { > >> + .l = 0x2c, > >> + .alpha = 0xcaaa, > >> +}; > > > > Any reason this can't be put on the stack in the probe function? > > > I would move it. > >>> > > In case you think I should move it outside I can do that too. No I was just wondering what prompted it.
