Messages in this thread | | | Date | Fri, 23 Jun 2023 22:06:49 +0530 | Subject | Re: [PATCH V4 2/4] clk: qcom: camcc-sm8550: Add camera clock controller driver for SM8550 | From | Jagadeesh Kona <> |
| |
On 6/14/2023 5:44 PM, Dmitry Baryshkov wrote: > On Wed, 14 Jun 2023 at 14:55, Jagadeesh Kona <quic_jkona@quicinc.com> wrote: >> >> >> >> On 6/9/2023 9:52 PM, Dmitry Baryshkov wrote: >>> On Fri, 9 Jun 2023 at 14:52, Jagadeesh Kona <quic_jkona@quicinc.com> wrote: >>>> >>>> Add support for the camera clock controller for camera clients to be >>>> able to request for camcc clocks on SM8550 platform. >>>> >>>> Co-developed-by: Taniya Das <quic_tdas@quicinc.com> >>>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com> >>>> --- >>>> Changes since V3: >>>> - No changes >>>> Changes since V2: >>>> - No changes >>>> Changes since V1: >>>> - Sorted the PLL names in proper order >>>> - Updated all PLL configurations to lower case hex >>>> - Reused evo ops instead of adding new ops for ole pll >>>> - Moved few clocks to separate patch to fix patch too long error >>>> >>>> drivers/clk/qcom/Kconfig | 7 + >>>> drivers/clk/qcom/Makefile | 1 + >>>> drivers/clk/qcom/camcc-sm8550.c | 3405 +++++++++++++++++++++++++++++++ >>>> 3 files changed, 3413 insertions(+) >>>> create mode 100644 drivers/clk/qcom/camcc-sm8550.c >>>> >>>> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig >>>> index 9cd1f05d436b..85efed78dc9a 100644 >>>> --- a/drivers/clk/qcom/Kconfig >>>> +++ b/drivers/clk/qcom/Kconfig >>>> @@ -756,6 +756,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 ARM64 || COMPILE_TEST >>>> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile >>>> index 75d035150118..97c8cefc2fd0 100644 >>>> --- a/drivers/clk/qcom/Makefile >>>> +++ b/drivers/clk/qcom/Makefile >>>> @@ -101,6 +101,7 @@ obj-$(CONFIG_SDX_GCC_75) += gcc-sdx75.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..85f0c1e09b2b >>>> --- /dev/null >>>> +++ b/drivers/clk/qcom/camcc-sm8550.c >>>> @@ -0,0 +1,3405 @@ >>>> +// 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_IFACE, >>>> + DT_BI_TCXO, >>>> +}; >>>> + >>>> +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_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_CAM_CC_PLL10_OUT_EVEN, >>>> + P_CAM_CC_PLL11_OUT_EVEN, >>>> + P_CAM_CC_PLL12_OUT_EVEN, >>>> +}; >>>> + >>>> +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 includes RINGOSC_CAL_L_VAL, CAL_L_VAL, L_VAL fields */ >>>> + .l = 0x4444003e, >>> >>> I'd still insist on not touching the config.l field semantics. >>> >> >> We feel it is better to update config->l field and reuse existing code >> than adding separate function for lucid ole pll configure. > > As you probably got it, I'm not convinced that it is a better > approach. You are feeding additional data in a single configuration > field and passing constant data as variadic one. >
Will avoid this approach and will add separate lucid ole pll configure function 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, >>>> +}; >>>> + >>> >>> [skipped the rest, LGTM] >>> >>>> + >>>> +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); >>> >>> As it was pointed out, this driver is built as a module by default. >>> Please perform the tesing and cleanup before sending the driver and >>> use module_platform_driver. >>> >> >> We want clock drivers to be probed early in the bootup to avoid any >> probe deferrals of consumer drivers. If there is any scenario where >> clock drivers are built statically into kernel, then subsys_initcall() >> will ensure clock drivers are probed earlier. When built as module, >> subsys_initcall() will fallback to module_init() which is same as >> module_platform_driver(). > > Consumer driver probe deferrals are nowadays significantly prevented > by using devlink rather than depending on the initialisation level. > And I think both GKI and defconfig build camcc as modules. >
Will use module_platform_driver() in next series.
Thanks, Jagadeesh
>> >> Thanks, >> Jagadeesh >> >>>> + >>>> +static void __exit cam_cc_sm8550_exit(void) >>>> +{ >>>> + platform_driver_unregister(&cam_cc_sm8550_driver); >>>> +} >>>> +module_exit(cam_cc_sm8550_exit); >>>> + >>>> +MODULE_DESCRIPTION("QTI CAMCC SM8550 Driver"); >>>> +MODULE_LICENSE("GPL"); >>>> -- >>>> 2.40.1 >>>> >>> >>> > > >
| |