Message ID | 20201203070241.2648874-1-vkoul@kernel.org (mailing list archive) |
---|---|
Headers | show |
Series | Add clock drivers for SM8350 | expand |
On Thu 03 Dec 01:02 CST 2020, Vinod Koul wrote: > diff --git a/drivers/clk/qcom/gcc-sm8350.c b/drivers/clk/qcom/gcc-sm8350.c [..] > +static int gcc_sm8350_probe(struct platform_device *pdev) > +{ > + struct regmap *regmap; > + int ret; > + > + regmap = qcom_cc_map(pdev, &gcc_sm8350_desc); > + if (IS_ERR(regmap)) { > + dev_err(&pdev->dev, "Failed to map gcc registers\n"); > + return PTR_ERR(regmap); > + } > + > + ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, ARRAY_SIZE(gcc_dfs_clocks)); > + if (ret) > + return ret; > + > + /* FORCE_MEM_CORE_ON for ufs phy ice core clocks */ > + regmap_update_bits(regmap, gcc_ufs_phy_ice_core_clk.halt_reg, BIT(14), BIT(14)); > + > + /* > + * Enable clocks required by the i2c-connected pm8008 regulators. Don't > + * register them with the clock framework so that client requests are > + * short-circuited before grabbing the enable/prepare locks. This > + * prevents deadlocks between the clk/regulator frameworks. > + * > + * gcc_qupv3_wrap_1_m_ahb_clk > + * gcc_qupv3_wrap_1_s_ahb_clk > + * gcc_qupv3_wrap1_s5_clk > + */ Isn't this a workaround inherited from the downstream control of regulators from within the clock core? Does this still apply upstream? Regards, Bjorn > + regmap_update_bits(regmap, 0x52008, BIT(20), BIT(20)); > + regmap_update_bits(regmap, 0x52008, BIT(21), BIT(21)); > + regmap_update_bits(regmap, 0x52008, BIT(27), BIT(27)); > + > + return qcom_cc_really_probe(pdev, &gcc_sm8350_desc, regmap); > +} > + > +static struct platform_driver gcc_sm8350_driver = { > + .probe = gcc_sm8350_probe, > + .driver = { > + .name = "sm8350-gcc", > + .of_match_table = gcc_sm8350_match_table, > + }, > +}; > + > +static int __init gcc_sm8350_init(void) > +{ > + return platform_driver_register(&gcc_sm8350_driver); > +} > +subsys_initcall(gcc_sm8350_init); > + > +static void __exit gcc_sm8350_exit(void) > +{ > + platform_driver_unregister(&gcc_sm8350_driver); > +} > +module_exit(gcc_sm8350_exit); > + > +MODULE_DESCRIPTION("QTI GCC SM8350 Driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.26.2 >
Hi Bjorn, On 03-12-20, 18:06, Bjorn Andersson wrote: > On Thu 03 Dec 01:02 CST 2020, Vinod Koul wrote: > > diff --git a/drivers/clk/qcom/gcc-sm8350.c b/drivers/clk/qcom/gcc-sm8350.c > [..] > > +static int gcc_sm8350_probe(struct platform_device *pdev) > > +{ > > + struct regmap *regmap; > > + int ret; > > + > > + regmap = qcom_cc_map(pdev, &gcc_sm8350_desc); > > + if (IS_ERR(regmap)) { > > + dev_err(&pdev->dev, "Failed to map gcc registers\n"); > > + return PTR_ERR(regmap); > > + } > > + > > + ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, ARRAY_SIZE(gcc_dfs_clocks)); > > + if (ret) > > + return ret; > > + > > + /* FORCE_MEM_CORE_ON for ufs phy ice core clocks */ > > + regmap_update_bits(regmap, gcc_ufs_phy_ice_core_clk.halt_reg, BIT(14), BIT(14)); > > + > > + /* > > + * Enable clocks required by the i2c-connected pm8008 regulators. Don't > > + * register them with the clock framework so that client requests are > > + * short-circuited before grabbing the enable/prepare locks. This > > + * prevents deadlocks between the clk/regulator frameworks. > > + * > > + * gcc_qupv3_wrap_1_m_ahb_clk > > + * gcc_qupv3_wrap_1_s_ahb_clk > > + * gcc_qupv3_wrap1_s5_clk > > + */ > > Isn't this a workaround inherited from the downstream control of > regulators from within the clock core? Does this still apply upstream? Let me check on this bit... Thanks
Hi Vinod, On 12/4/2020 10:05 AM, Vinod Koul wrote: > Hi Bjorn, > > On 03-12-20, 18:06, Bjorn Andersson wrote: >> On Thu 03 Dec 01:02 CST 2020, Vinod Koul wrote: >>> diff --git a/drivers/clk/qcom/gcc-sm8350.c b/drivers/clk/qcom/gcc-sm8350.c >> [..] >>> +static int gcc_sm8350_probe(struct platform_device *pdev) >>> +{ >>> + struct regmap *regmap; >>> + int ret; >>> + >>> + regmap = qcom_cc_map(pdev, &gcc_sm8350_desc); >>> + if (IS_ERR(regmap)) { >>> + dev_err(&pdev->dev, "Failed to map gcc registers\n"); >>> + return PTR_ERR(regmap); >>> + } >>> + >>> + ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, ARRAY_SIZE(gcc_dfs_clocks)); >>> + if (ret) >>> + return ret; >>> + >>> + /* FORCE_MEM_CORE_ON for ufs phy ice core clocks */ >>> + regmap_update_bits(regmap, gcc_ufs_phy_ice_core_clk.halt_reg, BIT(14), BIT(14)); >>> + >>> + /* >>> + * Enable clocks required by the i2c-connected pm8008 regulators. Don't >>> + * register them with the clock framework so that client requests are >>> + * short-circuited before grabbing the enable/prepare locks. This >>> + * prevents deadlocks between the clk/regulator frameworks. >>> + * >>> + * gcc_qupv3_wrap_1_m_ahb_clk >>> + * gcc_qupv3_wrap_1_s_ahb_clk >>> + * gcc_qupv3_wrap1_s5_clk >>> + */ >> >> Isn't this a workaround inherited from the downstream control of >> regulators from within the clock core? Does this still apply upstream? > > Let me check on this bit... > > Thanks > No it should not apply.
Hi Taniya, On 04-12-20, 14:20, Taniya Das wrote: > On 12/4/2020 10:05 AM, Vinod Koul wrote: > > On 03-12-20, 18:06, Bjorn Andersson wrote: > > > On Thu 03 Dec 01:02 CST 2020, Vinod Koul wrote: > > > > diff --git a/drivers/clk/qcom/gcc-sm8350.c b/drivers/clk/qcom/gcc-sm8350.c > > > [..] > > > > +static int gcc_sm8350_probe(struct platform_device *pdev) > > > > +{ > > > > + struct regmap *regmap; > > > > + int ret; > > > > + > > > > + regmap = qcom_cc_map(pdev, &gcc_sm8350_desc); > > > > + if (IS_ERR(regmap)) { > > > > + dev_err(&pdev->dev, "Failed to map gcc registers\n"); > > > > + return PTR_ERR(regmap); > > > > + } > > > > + > > > > + ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, ARRAY_SIZE(gcc_dfs_clocks)); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + /* FORCE_MEM_CORE_ON for ufs phy ice core clocks */ > > > > + regmap_update_bits(regmap, gcc_ufs_phy_ice_core_clk.halt_reg, BIT(14), BIT(14)); > > > > + > > > > + /* > > > > + * Enable clocks required by the i2c-connected pm8008 regulators. Don't > > > > + * register them with the clock framework so that client requests are > > > > + * short-circuited before grabbing the enable/prepare locks. This > > > > + * prevents deadlocks between the clk/regulator frameworks. > > > > + * > > > > + * gcc_qupv3_wrap_1_m_ahb_clk > > > > + * gcc_qupv3_wrap_1_s_ahb_clk > > > > + * gcc_qupv3_wrap1_s5_clk > > > > + */ > > > > > > Isn't this a workaround inherited from the downstream control of > > > regulators from within the clock core? Does this still apply upstream? > > > > Let me check on this bit... > > > > Thanks > > > > No it should not apply. Thanks for confirmation, removed now. Will send v2