Re: [PATCH v4 3/4] clk: sophgo: Add SG2042 clock generator driver

From: Inochi Amaoto
Date: Tue Dec 05 2023 - 01:34:47 EST


>On 2023/12/5 9:13, Inochi Amaoto wrote:
>>> From: Chen Wang <unicorn_wang@xxxxxxxxxxx>
>>>
>>> Add a driver for the SOPHGO SG2042 clock generator.
>>>
>>> Signed-off-by: Chen Wang <unicorn_wang@xxxxxxxxxxx>
>>> ---
>>> MAINTAINERS | 7 +
>>> drivers/clk/Kconfig | 1 +
>>> drivers/clk/Makefile | 1 +
>>> drivers/clk/sophgo/Kconfig | 8 +
>>> drivers/clk/sophgo/Makefile | 2 +
>>> drivers/clk/sophgo/clk-sophgo-sg2042.c | 1371 ++++++++++++++++++++++++
>>> drivers/clk/sophgo/clk-sophgo-sg2042.h | 226 ++++
>>> 7 files changed, 1616 insertions(+)
>>> create mode 100644 drivers/clk/sophgo/Kconfig
>>> create mode 100644 drivers/clk/sophgo/Makefile
>>> create mode 100644 drivers/clk/sophgo/clk-sophgo-sg2042.c
>>> create mode 100644 drivers/clk/sophgo/clk-sophgo-sg2042.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 97f51d5ec1cf..c9c75468f2cb 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -20269,6 +20269,13 @@ S: Maintained
>>> F: arch/riscv/boot/dts/sophgo/
>>> F: Documentation/devicetree/bindings/riscv/sophgo.yaml
>>>
>>> +SOPHGO CLOCK DRIVER
>>> +M: Chen Wang <unicorn_wang@xxxxxxxxxxx>
>>> +S: Maintained
>>> +F: Documentation/devicetree/bindings/clock/sophgo/
>>> +F: drivers/clk/sophgo/
>>> +F: include/dt-bindings/clock/sophgo,sg2042-clkgen.h
>>> +
>>> SOUND
>>> M: Jaroslav Kysela <perex@xxxxxxxx>
>>> M: Takashi Iwai <tiwai@xxxxxxxx>
>>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
>>> index c30d0d396f7a..514343934fda 100644
>>> --- a/drivers/clk/Kconfig
>>> +++ b/drivers/clk/Kconfig
>>> @@ -499,6 +499,7 @@ source "drivers/clk/rockchip/Kconfig"
>>> source "drivers/clk/samsung/Kconfig"
>>> source "drivers/clk/sifive/Kconfig"
>>> source "drivers/clk/socfpga/Kconfig"
>>> +source "drivers/clk/sophgo/Kconfig"
>>> source "drivers/clk/sprd/Kconfig"
>>> source "drivers/clk/starfive/Kconfig"
>>> source "drivers/clk/sunxi/Kconfig"
>>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>>> index ed71f2e0ee36..aa5d2cf0b6a6 100644
>>> --- a/drivers/clk/Makefile
>>> +++ b/drivers/clk/Makefile
>>> @@ -119,6 +119,7 @@ obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/
>>> obj-$(CONFIG_COMMON_CLK_SAMSUNG) += samsung/
>>> obj-$(CONFIG_CLK_SIFIVE) += sifive/
>>> obj-y += socfpga/
>>> +obj-$(CONFIG_ARCH_SOPHGO) += sophgo/
>>> obj-$(CONFIG_PLAT_SPEAR) += spear/
>>> obj-y += sprd/
>>> obj-$(CONFIG_ARCH_STI) += st/
>>> diff --git a/drivers/clk/sophgo/Kconfig b/drivers/clk/sophgo/Kconfig
>>> new file mode 100644
>>> index 000000000000..b0fbe4499870
>>> --- /dev/null
>>> +++ b/drivers/clk/sophgo/Kconfig
>>> @@ -0,0 +1,8 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +
>>> +config CLK_SOPHGO_SG2042
>>> + bool "Sophgo SG2042 clock support"
>>> + depends on ARCH_SOPHGO || COMPILE_TEST
>>> + default ARCH_SOPHGO
>>> + help
>>> + Say yes here to support the clock controller on the Sophgo SG2042 SoC.
>> Please add RISCV to its depends. Not all the sophgo SoCs are in the RISC-V
>> platform, Some are arm chips. It is not good to build this driver
>> in that arch.
>ARCH_SOPHGO is only defined in RISC-V arch, so I think RISCV should not be needed as depends.

It needs for the SG200X. which has a arm core. Maybe we should left it now
and add this in the future.

>>
>> Also, the condition ARCH_SOPHGO is duplicate in the Makefile. Please
>> remove one of them.
>
>Don't undestand your question "the condition ARCH_SOPHGO is duplicate in the Makefile ", I think you are talking about Kconifg, what Makefile are you talking about?
>

I mean your change in drivers/clk/Makefile.

>>> diff --git a/drivers/clk/sophgo/Makefile b/drivers/clk/sophgo/Makefile
>>> new file mode 100644
>>> index 000000000000..13834cce260c
>>> --- /dev/null
>>> +++ b/drivers/clk/sophgo/Makefile
>>> @@ -0,0 +1,2 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +obj-$(CONFIG_CLK_SOPHGO_SG2042) += clk-sophgo-sg2042.o
>>> diff --git a/drivers/clk/sophgo/clk-sophgo-sg2042.c b/drivers/clk/sophgo/clk-sophgo-sg2042.c
>>> new file mode 100644
>>> index 000000000000..421ebcc7192b
>>> ......
>>> + }
>>> +
>>> +#if defined(DEBUG)
>>> + pr_info("registered [%d : %s]\n", gate->id, gate->name);
>>> +#endif
>> Drop, no need for this.
>
>OK, this log should not be needed, I will remove it in next version.
>
>