On Fri 03 May 2024 at 14:16, Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote: > On Thu 25 Apr 2024 at 14:28, Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx> wrote: > >> Hi Rob, >> Thanks for your review. >> >> On 2024/4/25 04:01, Rob Herring wrote: >>> [ EXTERNAL EMAIL ] >>> On Wed, Apr 24, 2024 at 01:09:28PM +0800, Xianwei Zhao wrote: >>>> Add the C3 peripherals clock controller driver in the C3 SoC family. >>>> >>>> Co-developed-by: Chuan Liu <chuan.liu@xxxxxxxxxxx> >>>> Signed-off-by: Chuan Liu <chuan.liu@xxxxxxxxxxx> >>>> Signed-off-by: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx> >>>> --- >>>> drivers/clk/meson/Kconfig | 15 + >>>> drivers/clk/meson/Makefile | 1 + >>>> drivers/clk/meson/c3-peripherals.c | 2366 ++++++++++++++++++++++++++++ >>>> 3 files changed, 2382 insertions(+) >>>> create mode 100644 drivers/clk/meson/c3-peripherals.c >>>> >>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig >>>> index 9f975a980581..0b85d584910e 100644 >>>> --- a/drivers/clk/meson/Kconfig >>>> +++ b/drivers/clk/meson/Kconfig >>>> @@ -142,6 +142,21 @@ config COMMON_CLK_C3_PLL >>>> AKA C3. Say Y if you want the board to work, because PLLs are the parent >>>> of most peripherals. >>>> >>>> +config COMMON_CLK_C3_PERIPHERALS >>>> + tristate "Amlogic C3 peripherals clock controller" >>>> + depends on ARM64 >>>> + depends on ARM_SCMI_PROTOCOL >>> I may have missed it, but I don't see the dependency on SCMI in this >>> driver. >>> >> >> Some clock sources for peripherals controller from SCMI module. >> In previous version, Jerome suggest us the clock that relevant registers >> can only be accessed securely is implemented through SCMI. >> > > There is no dependency but you are guranteed to have have problem if > SCMI is missing because it provides several inputs to this controller. > > IMO, this controller should 'imply ARM_SCMI_PROTOCOL' > > You have already posted a v8 without it. > I expect a fix up change on top of v8 if there is no other comment on > this v8 version. Part of v9 otherwise. > >>>> + depends on COMMON_CLK_SCMI >>>> + depends on COMMON_CLK_C3_PLL Same here - imply, not depends >>>> + default y >>>> + select COMMON_CLK_MESON_REGMAP >>>> + select COMMON_CLK_MESON_DUALDIV >>>> + select COMMON_CLK_MESON_CLKC_UTILS >>>> + help >>>> + Support for the Peripherals clock controller on Amlogic C302X and >>>> + C308L devices, AKA C3. Say Y if you want the peripherals clock to >>>> + work. >>>> + >>>> config COMMON_CLK_G12A >>>> tristate "G12 and SM1 SoC clock controllers support" >>>> depends on ARM64 >>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile >>>> index 4420af628b31..20ad9482c892 100644 >>>> --- a/drivers/clk/meson/Makefile >>>> +++ b/drivers/clk/meson/Makefile >>>> @@ -20,6 +20,7 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o >>>> obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o >>>> obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o >>>> obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o >>>> +obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o >>>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o >>>> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o >>>> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o >>>> diff --git a/drivers/clk/meson/c3-peripherals.c b/drivers/clk/meson/c3-peripherals.c >>>> new file mode 100644 >>>> index 000000000000..0f834ced0ee9 >>>> --- /dev/null >>>> +++ b/drivers/clk/meson/c3-peripherals.c >>>> @@ -0,0 +1,2366 @@ >>>> +// SPDX-License-Identifier: GPL-2.0-only >>>> +/* >>>> + * Amlogic C3 Peripherals Clock Controller Driver >>>> + * >>>> + * Copyright (c) 2023 Amlogic, inc. >>>> + * Author: Chuan Liu <chuan.liu@xxxxxxxxxxx> >>>> + */ >>>> + >>>> +#include <linux/clk-provider.h> >>>> +#include <linux/of_device.h> >>> I don't think you need this header. >>> >> >> Yes, I will fix it with c3-pll.c. >> >>>> +#include <linux/platform_device.h> >>>> +#include "clk-regmap.h" >>>> +#include "clk-dualdiv.h" >>>> +#include "meson-clkc-utils.h" >>>> +#include <dt-bindings/clock/amlogic,c3-peripherals-clkc.h> -- Jerome
