| Message ID | 20230519155602.6642-1-quic_jkona@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add camera clock controller support for SM8550 | expand |
On 19.05.2023 17:56, Jagadeesh Kona wrote: > Add support for the camera clock controller for camera clients to be > able to request for camcc clocks on SM8550 platform. > > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> > Signed-off-by: Taniya Das <quic_tdas@quicinc.com> > --- > drivers/clk/qcom/Kconfig | 7 + > drivers/clk/qcom/Makefile | 1 + > drivers/clk/qcom/camcc-sm8550.c | 3572 +++++++++++++++++++++++++++++++ > 3 files changed, 3580 insertions(+) > create mode 100644 drivers/clk/qcom/camcc-sm8550.c > > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > index 6bb9b4aff047..c8eccd428736 100644 > --- a/drivers/clk/qcom/Kconfig > +++ b/drivers/clk/qcom/Kconfig > @@ -700,6 +700,13 @@ config SM_CAMCC_8450 > Support for the camera clock controller on SM8450 devices. > Say Y if you want to support camera devices and camera functionality. > > +config SM_CAMCC_8550 > + tristate "SM8550 Camera Clock Controller" > + select SM_GCC_8550 > + help > + Support for the camera clock controller on SM8550 devices. > + Say Y if you want to support camera devices and camera functionality. > + > config SM_DISPCC_6115 > tristate "SM6115 Display Clock Controller" > depends on SM_GCC_6115 > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile > index f0b95fc217aa..2b1fcd5c920a 100644 > --- a/drivers/clk/qcom/Makefile > +++ b/drivers/clk/qcom/Makefile > @@ -100,6 +100,7 @@ obj-$(CONFIG_SDX_GCC_65) += gcc-sdx65.o > obj-$(CONFIG_SM_CAMCC_6350) += camcc-sm6350.o > obj-$(CONFIG_SM_CAMCC_8250) += camcc-sm8250.o > obj-$(CONFIG_SM_CAMCC_8450) += camcc-sm8450.o > +obj-$(CONFIG_SM_CAMCC_8550) += camcc-sm8550.o > obj-$(CONFIG_SM_DISPCC_6115) += dispcc-sm6115.o > obj-$(CONFIG_SM_DISPCC_6125) += dispcc-sm6125.o > obj-$(CONFIG_SM_DISPCC_6350) += dispcc-sm6350.o > diff --git a/drivers/clk/qcom/camcc-sm8550.c b/drivers/clk/qcom/camcc-sm8550.c > new file mode 100644 > index 000000000000..2c3d2436da0f > --- /dev/null > +++ b/drivers/clk/qcom/camcc-sm8550.c > @@ -0,0 +1,3572 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/clk-provider.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/regmap.h> > + > +#include <dt-bindings/clock/qcom,sm8550-camcc.h> > + > +#include "clk-alpha-pll.h" > +#include "clk-branch.h" > +#include "clk-rcg.h" > +#include "clk-regmap.h" > +#include "common.h" > +#include "gdsc.h" > +#include "reset.h" > + > +enum { > + DT_BI_TCXO, > + DT_BI_TCXO_AO, > + DT_SLEEP_CLK, > +}; > + > +enum { > + P_BI_TCXO, > + P_CAM_CC_PLL0_OUT_EVEN, > + P_CAM_CC_PLL0_OUT_MAIN, > + P_CAM_CC_PLL0_OUT_ODD, > + P_CAM_CC_PLL10_OUT_EVEN, > + P_CAM_CC_PLL11_OUT_EVEN, > + P_CAM_CC_PLL12_OUT_EVEN, While it makes sense for a computer, I think it would make more sense to keep the >=10 entries after 9 > + P_CAM_CC_PLL1_OUT_EVEN, > + P_CAM_CC_PLL2_OUT_EVEN, > + P_CAM_CC_PLL2_OUT_MAIN, > + P_CAM_CC_PLL3_OUT_EVEN, > + P_CAM_CC_PLL4_OUT_EVEN, > + P_CAM_CC_PLL5_OUT_EVEN, > + P_CAM_CC_PLL6_OUT_EVEN, > + P_CAM_CC_PLL7_OUT_EVEN, > + P_CAM_CC_PLL8_OUT_EVEN, > + P_CAM_CC_PLL9_OUT_EVEN, > + P_CAM_CC_PLL9_OUT_ODD, > + P_SLEEP_CLK, > +}; > + > +static const struct pll_vco lucid_ole_vco[] = { > + { 249600000, 2300000000, 0 }, > +}; > + > +static const struct pll_vco rivian_ole_vco[] = { > + { 777000000, 1285000000, 0 }, > +}; > + > +static const struct alpha_pll_config cam_cc_pll0_config = { > + .l = 0x4400003E, Lowercase hex, everywhere, please. > + .alpha = 0x8000, > + .config_ctl_val = 0x20485699, > + .config_ctl_hi_val = 0x00182261, > + .config_ctl_hi1_val = 0x82AA299C, > + .test_ctl_val = 0x00000000, > + .test_ctl_hi_val = 0x00000003, > + .test_ctl_hi1_val = 0x00009000, > + .test_ctl_hi2_val = 0x00000034, > + .user_ctl_val = 0x00008400, > + .user_ctl_hi_val = 0x00000005, > +}; > + [...] > + > + clk_lucid_ole_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll10, regmap, &cam_cc_pll10_config); Same as the first comment Konrad > + clk_lucid_ole_pll_configure(&cam_cc_pll11, regmap, &cam_cc_pll11_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll12, regmap, &cam_cc_pll12_config); > + clk_rivian_ole_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll5, regmap, &cam_cc_pll5_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll6, regmap, &cam_cc_pll6_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll7, regmap, &cam_cc_pll7_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll8, regmap, &cam_cc_pll8_config); > + clk_lucid_ole_pll_configure(&cam_cc_pll9, regmap, &cam_cc_pll9_config); > + > + /* > + * Keep clocks always enabled: > + * cam_cc_gdsc_clk > + * cam_cc_sleep_clk > + */ > + regmap_update_bits(regmap, 0x1419c, BIT(0), BIT(0)); > + regmap_update_bits(regmap, 0x142cc, BIT(0), BIT(0)); > + > + ret = qcom_cc_really_probe(pdev, &cam_cc_sm8550_desc, regmap); > + > + pm_runtime_put(&pdev->dev); > + > + return ret; > +} > + > +static struct platform_driver cam_cc_sm8550_driver = { > + .probe = cam_cc_sm8550_probe, > + .driver = { > + .name = "cam_cc-sm8550", > + .of_match_table = cam_cc_sm8550_match_table, > + }, > +}; > + > +static int __init cam_cc_sm8550_init(void) > +{ > + return platform_driver_register(&cam_cc_sm8550_driver); > +} > +subsys_initcall(cam_cc_sm8550_init); > + > +static void __exit cam_cc_sm8550_exit(void) > +{ > + platform_driver_unregister(&cam_cc_sm8550_driver); > +} > +module_exit(cam_cc_sm8550_exit); > + > +MODULE_DESCRIPTION("QTI CAM_CC SM8550 Driver"); > +MODULE_LICENSE("GPL");
On 19/05/2023 18:55, Jagadeesh Kona wrote: > Add bindings, driver and devicetree node for camera clock controller > on SM8550. > > Depends on [1] for lucid ole pll ops definition > [1] https://patchwork.kernel.org/project/linux-clk/list/?series=746186&state=%2A&archive=both > > Jagadeesh Kona (4): > clk: qcom: clk-alpha-pll: Add support for rivian ole pll ops > dt-bindings: clock: qcom: Add SM8550 camera clock controller > clk: qcom: camcc-sm8550: Add camera clock controller driver for SM8550 This patch didn't make it to the list. Please consider splitting it somehow. > arm64: dts: qcom: sm8550: Add camera clock controller > > .../bindings/clock/qcom,sm8550-camcc.yaml | 82 + > arch/arm64/boot/dts/qcom/sm8550.dtsi | 15 + > drivers/clk/qcom/Kconfig | 7 + > drivers/clk/qcom/Makefile | 1 + > drivers/clk/qcom/camcc-sm8550.c | 3572 +++++++++++++++++ > drivers/clk/qcom/clk-alpha-pll.h | 4 + > include/dt-bindings/clock/qcom,sm8550-camcc.h | 187 + > 7 files changed, 3868 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/qcom,sm8550-camcc.yaml > create mode 100644 drivers/clk/qcom/camcc-sm8550.c > create mode 100644 include/dt-bindings/clock/qcom,sm8550-camcc.h >
On 19/05/2023 16:56, Jagadeesh Kona wrote: > +MODULE_DESCRIPTION("QTI CAM_CC SM8550 Driver"); This pattern looks odd and inconsistent in our clock drivers. We mostly have CAMCC or VIDEOCC but occasionally have CAM_CC and VIDEO_CC. I'd drop that underscore to, in addition to Konrad's comment on Ucase hex. Other than that, LGTM. Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Hi Konrad, Thanks for your review! On 5/19/2023 10:22 PM, Konrad Dybcio wrote: > > > On 19.05.2023 17:56, Jagadeesh Kona wrote: >> Add support for the camera clock controller for camera clients to be >> able to request for camcc clocks on SM8550 platform. >> >> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >> --- >> drivers/clk/qcom/Kconfig | 7 + >> drivers/clk/qcom/Makefile | 1 + >> drivers/clk/qcom/camcc-sm8550.c | 3572 +++++++++++++++++++++++++++++++ >> 3 files changed, 3580 insertions(+) >> create mode 100644 drivers/clk/qcom/camcc-sm8550.c >> >> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig >> index 6bb9b4aff047..c8eccd428736 100644 >> --- a/drivers/clk/qcom/Kconfig >> +++ b/drivers/clk/qcom/Kconfig >> @@ -700,6 +700,13 @@ config SM_CAMCC_8450 >> Support for the camera clock controller on SM8450 devices. >> Say Y if you want to support camera devices and camera functionality. >> >> +config SM_CAMCC_8550 >> + tristate "SM8550 Camera Clock Controller" >> + select SM_GCC_8550 >> + help >> + Support for the camera clock controller on SM8550 devices. >> + Say Y if you want to support camera devices and camera functionality. >> + >> config SM_DISPCC_6115 >> tristate "SM6115 Display Clock Controller" >> depends on SM_GCC_6115 >> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile >> index f0b95fc217aa..2b1fcd5c920a 100644 >> --- a/drivers/clk/qcom/Makefile >> +++ b/drivers/clk/qcom/Makefile >> @@ -100,6 +100,7 @@ obj-$(CONFIG_SDX_GCC_65) += gcc-sdx65.o >> obj-$(CONFIG_SM_CAMCC_6350) += camcc-sm6350.o >> obj-$(CONFIG_SM_CAMCC_8250) += camcc-sm8250.o >> obj-$(CONFIG_SM_CAMCC_8450) += camcc-sm8450.o >> +obj-$(CONFIG_SM_CAMCC_8550) += camcc-sm8550.o >> obj-$(CONFIG_SM_DISPCC_6115) += dispcc-sm6115.o >> obj-$(CONFIG_SM_DISPCC_6125) += dispcc-sm6125.o >> obj-$(CONFIG_SM_DISPCC_6350) += dispcc-sm6350.o >> diff --git a/drivers/clk/qcom/camcc-sm8550.c b/drivers/clk/qcom/camcc-sm8550.c >> new file mode 100644 >> index 000000000000..2c3d2436da0f >> --- /dev/null >> +++ b/drivers/clk/qcom/camcc-sm8550.c >> @@ -0,0 +1,3572 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved. >> + */ >> + >> +#include <linux/clk-provider.h> >> +#include <linux/module.h> >> +#include <linux/of_device.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/regmap.h> >> + >> +#include <dt-bindings/clock/qcom,sm8550-camcc.h> >> + >> +#include "clk-alpha-pll.h" >> +#include "clk-branch.h" >> +#include "clk-rcg.h" >> +#include "clk-regmap.h" >> +#include "common.h" >> +#include "gdsc.h" >> +#include "reset.h" >> + >> +enum { >> + DT_BI_TCXO, >> + DT_BI_TCXO_AO, >> + DT_SLEEP_CLK, >> +}; >> + >> +enum { >> + P_BI_TCXO, >> + P_CAM_CC_PLL0_OUT_EVEN, >> + P_CAM_CC_PLL0_OUT_MAIN, >> + P_CAM_CC_PLL0_OUT_ODD, >> + P_CAM_CC_PLL10_OUT_EVEN, >> + P_CAM_CC_PLL11_OUT_EVEN, >> + P_CAM_CC_PLL12_OUT_EVEN, > While it makes sense for a computer, I think it would make more > sense to keep the >=10 entries after 9 > Yes, will take care of this in next series >> + P_CAM_CC_PLL1_OUT_EVEN, >> + P_CAM_CC_PLL2_OUT_EVEN, >> + P_CAM_CC_PLL2_OUT_MAIN, >> + P_CAM_CC_PLL3_OUT_EVEN, >> + P_CAM_CC_PLL4_OUT_EVEN, >> + P_CAM_CC_PLL5_OUT_EVEN, >> + P_CAM_CC_PLL6_OUT_EVEN, >> + P_CAM_CC_PLL7_OUT_EVEN, >> + P_CAM_CC_PLL8_OUT_EVEN, >> + P_CAM_CC_PLL9_OUT_EVEN, >> + P_CAM_CC_PLL9_OUT_ODD, >> + P_SLEEP_CLK, >> +}; >> + >> +static const struct pll_vco lucid_ole_vco[] = { >> + { 249600000, 2300000000, 0 }, >> +}; >> + >> +static const struct pll_vco rivian_ole_vco[] = { >> + { 777000000, 1285000000, 0 }, >> +}; >> + >> +static const struct alpha_pll_config cam_cc_pll0_config = { >> + .l = 0x4400003E, > Lowercase hex, everywhere, please. > Will take care of this in next series >> + .alpha = 0x8000, >> + .config_ctl_val = 0x20485699, >> + .config_ctl_hi_val = 0x00182261, >> + .config_ctl_hi1_val = 0x82AA299C, >> + .test_ctl_val = 0x00000000, >> + .test_ctl_hi_val = 0x00000003, >> + .test_ctl_hi1_val = 0x00009000, >> + .test_ctl_hi2_val = 0x00000034, >> + .user_ctl_val = 0x00008400, >> + .user_ctl_hi_val = 0x00000005, >> +}; >> + > [...] > >> + >> + clk_lucid_ole_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll1, regmap, &cam_cc_pll1_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll10, regmap, &cam_cc_pll10_config); > Same as the first comment > Will take care in next series > Konrad >> + clk_lucid_ole_pll_configure(&cam_cc_pll11, regmap, &cam_cc_pll11_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll12, regmap, &cam_cc_pll12_config); >> + clk_rivian_ole_pll_configure(&cam_cc_pll2, regmap, &cam_cc_pll2_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll3, regmap, &cam_cc_pll3_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll4, regmap, &cam_cc_pll4_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll5, regmap, &cam_cc_pll5_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll6, regmap, &cam_cc_pll6_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll7, regmap, &cam_cc_pll7_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll8, regmap, &cam_cc_pll8_config); >> + clk_lucid_ole_pll_configure(&cam_cc_pll9, regmap, &cam_cc_pll9_config); >> + [skipped] >> +MODULE_DESCRIPTION("QTI CAM_CC SM8550 Driver"); >> +MODULE_LICENSE("GPL"); Thanks & Regards, Jagadeesh
Hi Bryan, Thanks for your review! On 5/20/2023 1:56 AM, Bryan O'Donoghue wrote: > On 19/05/2023 16:56, Jagadeesh Kona wrote: >> +MODULE_DESCRIPTION("QTI CAM_CC SM8550 Driver"); > > This pattern looks odd and inconsistent in our clock drivers. We mostly > have CAMCC or VIDEOCC but occasionally have CAM_CC and VIDEO_CC. > > I'd drop that underscore to, in addition to Konrad's comment on Ucase hex. > > Other than that, LGTM. > Will take care of this in next series. > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Thanks & Regards, Jagadeesh
Hi Dmitry, Thanks for your review! On 5/20/2023 12:59 AM, Dmitry Baryshkov wrote: > On 19/05/2023 18:55, Jagadeesh Kona wrote: >> Add bindings, driver and devicetree node for camera clock controller >> on SM8550. >> >> Depends on [1] for lucid ole pll ops definition >> [1] >> https://patchwork.kernel.org/project/linux-clk/list/?series=746186&state=%2A&archive=both >> >> Jagadeesh Kona (4): >> clk: qcom: clk-alpha-pll: Add support for rivian ole pll ops >> dt-bindings: clock: qcom: Add SM8550 camera clock controller >> clk: qcom: camcc-sm8550: Add camera clock controller driver for SM8550 > > This patch didn't make it to the list. Please consider splitting it > somehow. > Yes, will split it in next series. >> arm64: dts: qcom: sm8550: Add camera clock controller >> >> .../bindings/clock/qcom,sm8550-camcc.yaml | 82 + >> arch/arm64/boot/dts/qcom/sm8550.dtsi | 15 + >> drivers/clk/qcom/Kconfig | 7 + >> drivers/clk/qcom/Makefile | 1 + >> drivers/clk/qcom/camcc-sm8550.c | 3572 +++++++++++++++++ >> drivers/clk/qcom/clk-alpha-pll.h | 4 + >> include/dt-bindings/clock/qcom,sm8550-camcc.h | 187 + >> 7 files changed, 3868 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/clock/qcom,sm8550-camcc.yaml >> create mode 100644 drivers/clk/qcom/camcc-sm8550.c >> create mode 100644 include/dt-bindings/clock/qcom,sm8550-camcc.h >> > Thanks & Regards, Jagadeesh