Re: [PATCH v4 12/19] clk: starfive: Add StarFive JH7110 always-on clock driver

[Hal Feng]

On Sun, 26 Feb 2023 18:34:52 +0100, Emil Renner Berthing wrote:
> On Tue, 21 Feb 2023 at 03:47, Hal Feng <hal.feng@xxxxxxxxxxxxxxxx> wrote:
>> From: Emil Renner Berthing <kernel@xxxxxxxx>
>>
>> Add driver for the StarFive JH7110 always-on clock controller
>> and register an auxiliary device for always-on reset controller
>> which is named as "reset-aon".
>>
>> Reported-by: kernel test robot <lkp@xxxxxxxxx>
>> Signed-off-by: Emil Renner Berthing <kernel@xxxxxxxx>
>> Co-developed-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx>
>> ---
>>  drivers/clk/starfive/Kconfig                  |  11 ++
>>  drivers/clk/starfive/Makefile                 |   1 +
>>  .../clk/starfive/clk-starfive-jh7110-aon.c    | 156 ++++++++++++++++++
>>  3 files changed, 168 insertions(+)
>>  create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-aon.c
>>
>> diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig
>> index 4640d0665d1c..2aa664f2cdee 100644
>> --- a/drivers/clk/starfive/Kconfig
>> +++ b/drivers/clk/starfive/Kconfig
>> @@ -31,3 +31,14 @@ config CLK_STARFIVE_JH7110_SYS
>>         help
>>           Say yes here to support the system clock controller on the
>>           StarFive JH7110 SoC.
>> +
>> +config CLK_STARFIVE_JH7110_AON
>> +       tristate "StarFive JH7110 always-on clock support"
>> +       depends on CLK_STARFIVE_JH7110_SYS
>> +       select AUXILIARY_BUS
>> +       select CLK_STARFIVE_JH71X0
>> +       select RESET_STARFIVE_JH7110
>> +       default CLK_STARFIVE_JH7110_SYS
> 
> As far as I can tell the JH7110 boots fine without this driver and it
> already depends on the _SYS driver above, so please do
> 
> default m if SOC_STARFIVE

OK. Will fix it.

> 
> And consider helping Conor by changing all the SOC_STARFIVE instances
> to ARCH_STARFIVE for the next version.

OK, I see. Will use the ARCH_ symbol instead.

> 
>> +       help
>> +         Say yes here to support the always-on clock controller on the
>> +         StarFive JH7110 SoC.
>> diff --git a/drivers/clk/starfive/Makefile b/drivers/clk/starfive/Makefile
>> index 5ca4e887fb9c..f3df7d957b1e 100644
>> --- a/drivers/clk/starfive/Makefile
>> +++ b/drivers/clk/starfive/Makefile
>> @@ -5,3 +5,4 @@ obj-$(CONFIG_CLK_STARFIVE_JH7100)       += clk-starfive-jh7100.o
>>  obj-$(CONFIG_CLK_STARFIVE_JH7100_AUDIO)        += clk-starfive-jh7100-audio.o
>>
>>  obj-$(CONFIG_CLK_STARFIVE_JH7110_SYS)  += clk-starfive-jh7110-sys.o
>> +obj-$(CONFIG_CLK_STARFIVE_JH7110_AON)  += clk-starfive-jh7110-aon.o
>> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-aon.c b/drivers/clk/starfive/clk-starfive-jh7110-aon.c
>> new file mode 100644
>> index 000000000000..da808dc93048
>> --- /dev/null
>> +++ b/drivers/clk/starfive/clk-starfive-jh7110-aon.c
>> @@ -0,0 +1,156 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * StarFive JH7110 Always-On Clock Driver
>> + *
>> + * Copyright (C) 2022 Emil Renner Berthing <kernel@xxxxxxxx>
>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/io.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include <dt-bindings/clock/starfive,jh7110-crg.h>
>> +
>> +#include "clk-starfive-jh71x0.h"
>> +
>> +/* external clocks */
>> +#define JH7110_AONCLK_OSC              (JH7110_AONCLK_END + 0)
>> +#define JH7110_AONCLK_RTC_OSC          (JH7110_AONCLK_END + 1)
>> +#define JH7110_AONCLK_GMAC0_RMII_REFIN (JH7110_AONCLK_END + 2)
>> +#define JH7110_AONCLK_GMAC0_RGMII_RXIN (JH7110_AONCLK_END + 3)
>> +#define JH7110_AONCLK_STG_AXIAHB       (JH7110_AONCLK_END + 4)
>> +#define JH7110_AONCLK_APB_BUS          (JH7110_AONCLK_END + 5)
>> +#define JH7110_AONCLK_GMAC0_GTXCLK     (JH7110_AONCLK_END + 6)
>> +
>> +static const struct jh71x0_clk_data jh7110_aonclk_data[] = {
>> +       /* source */
>> +       JH71X0__DIV(JH7110_AONCLK_OSC_DIV4, "osc_div4", 4, JH7110_AONCLK_OSC),
>> +       JH71X0__MUX(JH7110_AONCLK_APB_FUNC, "apb_func", 2,
>> +                   JH7110_AONCLK_OSC_DIV4,
>> +                   JH7110_AONCLK_OSC),
>> +       /* gmac0 */
>> +       JH71X0_GATE(JH7110_AONCLK_GMAC0_AHB, "gmac0_ahb", 0, JH7110_AONCLK_STG_AXIAHB),
>> +       JH71X0_GATE(JH7110_AONCLK_GMAC0_AXI, "gmac0_axi", 0, JH7110_AONCLK_STG_AXIAHB),
>> +       JH71X0__DIV(JH7110_AONCLK_GMAC0_RMII_RTX, "gmac0_rmii_rtx", 30,
>> +                   JH7110_AONCLK_GMAC0_RMII_REFIN),
>> +       JH71X0_GMUX(JH7110_AONCLK_GMAC0_TX, "gmac0_tx", 0, 2,
>> +                   JH7110_AONCLK_GMAC0_GTXCLK,
>> +                   JH7110_AONCLK_GMAC0_RMII_RTX),
>> +       JH71X0__INV(JH7110_AONCLK_GMAC0_TX_INV, "gmac0_tx_inv", JH7110_AONCLK_GMAC0_TX),
>> +       JH71X0__MUX(JH7110_AONCLK_GMAC0_RX, "gmac0_rx", 2,
>> +                   JH7110_AONCLK_GMAC0_RGMII_RXIN,
>> +                   JH7110_AONCLK_GMAC0_RMII_RTX),
>> +       JH71X0__INV(JH7110_AONCLK_GMAC0_RX_INV, "gmac0_rx_inv", JH7110_AONCLK_GMAC0_RX),
>> +       /* otpc */
>> +       JH71X0_GATE(JH7110_AONCLK_OTPC_APB, "otpc_apb", CLK_IGNORE_UNUSED, JH7110_AONCLK_APB_BUS),
>> +       /* rtc */
>> +       JH71X0_GATE(JH7110_AONCLK_RTC_APB, "rtc_apb", CLK_IGNORE_UNUSED, JH7110_AONCLK_APB_BUS),
>> +       JH71X0__DIV(JH7110_AONCLK_RTC_INTERNAL, "rtc_internal", 1022, JH7110_AONCLK_OSC),
>> +       JH71X0__MUX(JH7110_AONCLK_RTC_32K, "rtc_32k", 2,
>> +                   JH7110_AONCLK_RTC_OSC,
>> +                   JH7110_AONCLK_RTC_INTERNAL),
>> +       JH71X0_GATE(JH7110_AONCLK_RTC_CAL, "rtc_cal", 0, JH7110_AONCLK_OSC),
>> +};
> 
> This list also contains instances of the CLK_IGNORE_UNUSED flag. Again
> please go through them and figure out which clocks are critical and
> which are fine to turn off when not used.

I had synchronized these clock flags with JH7110 SDK before and I will
recheck these flags. Thanks.

Best regards,
Hal