| Message ID | 20230221024645.127922-13-hal.feng@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Basic clock, reset & device tree support for StarFive JH7110 RISC-V SoC | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
On Tue, 21 Feb 2023 at 03:47, Hal Feng <hal.feng@starfivetech.com> wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > 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@intel.com> > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > Co-developed-by: Hal Feng <hal.feng@starfivetech.com> > Signed-off-by: Hal Feng <hal.feng@starfivetech.com> > --- > 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 And consider helping Conor by changing all the SOC_STARFIVE instances to ARCH_STARFIVE for the next version. > + 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@esmil.dk> > + * 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. > + > +static struct clk_hw *jh7110_aonclk_get(struct of_phandle_args *clkspec, void *data) > +{ > + struct jh71x0_clk_priv *priv = data; > + unsigned int idx = clkspec->args[0]; > + > + if (idx < JH7110_AONCLK_END) > + return &priv->reg[idx].hw; > + > + return ERR_PTR(-EINVAL); > +} > + > +static int jh7110_aoncrg_probe(struct platform_device *pdev) > +{ > + struct jh71x0_clk_priv *priv; > + unsigned int idx; > + int ret; > + > + priv = devm_kzalloc(&pdev->dev, > + struct_size(priv, reg, JH7110_AONCLK_END), > + GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + spin_lock_init(&priv->rmw_lock); > + priv->dev = &pdev->dev; > + priv->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(priv->base)) > + return PTR_ERR(priv->base); > + > + dev_set_drvdata(priv->dev, (void *)(&priv->base)); > + > + for (idx = 0; idx < JH7110_AONCLK_END; idx++) { > + u32 max = jh7110_aonclk_data[idx].max; > + struct clk_parent_data parents[4] = {}; > + struct clk_init_data init = { > + .name = jh7110_aonclk_data[idx].name, > + .ops = starfive_jh71x0_clk_ops(max), > + .parent_data = parents, > + .num_parents = > + ((max & JH71X0_CLK_MUX_MASK) >> JH71X0_CLK_MUX_SHIFT) + 1, > + .flags = jh7110_aonclk_data[idx].flags, > + }; > + struct jh71x0_clk *clk = &priv->reg[idx]; > + unsigned int i; > + > + for (i = 0; i < init.num_parents; i++) { > + unsigned int pidx = jh7110_aonclk_data[idx].parents[i]; > + > + if (pidx < JH7110_AONCLK_END) > + parents[i].hw = &priv->reg[pidx].hw; > + else if (pidx == JH7110_AONCLK_OSC) > + parents[i].fw_name = "osc"; > + else if (pidx == JH7110_AONCLK_RTC_OSC) > + parents[i].fw_name = "rtc_osc"; > + else if (pidx == JH7110_AONCLK_GMAC0_RMII_REFIN) > + parents[i].fw_name = "gmac0_rmii_refin"; > + else if (pidx == JH7110_AONCLK_GMAC0_RGMII_RXIN) > + parents[i].fw_name = "gmac0_rgmii_rxin"; > + else if (pidx == JH7110_AONCLK_STG_AXIAHB) > + parents[i].fw_name = "stg_axiahb"; > + else if (pidx == JH7110_AONCLK_APB_BUS) > + parents[i].fw_name = "apb_bus"; > + else if (pidx == JH7110_AONCLK_GMAC0_GTXCLK) > + parents[i].fw_name = "gmac0_gtxclk"; > + } > + > + clk->hw.init = &init; > + clk->idx = idx; > + clk->max_div = max & JH71X0_CLK_DIV_MASK; > + > + ret = devm_clk_hw_register(&pdev->dev, &clk->hw); > + if (ret) > + return ret; > + } > + > + ret = devm_of_clk_add_hw_provider(&pdev->dev, jh7110_aonclk_get, priv); > + if (ret) > + return ret; > + > + return jh7110_reset_controller_register(priv, "reset-aon", 1); > +} > + > +static const struct of_device_id jh7110_aoncrg_match[] = { > + { .compatible = "starfive,jh7110-aoncrg" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, jh7110_aoncrg_match); > + > +static struct platform_driver jh7110_aoncrg_driver = { > + .probe = jh7110_aoncrg_probe, > + .driver = { > + .name = "clk-starfive-jh7110-aon", > + .of_match_table = jh7110_aoncrg_match, > + }, > +}; > +module_platform_driver(jh7110_aoncrg_driver); > + > +MODULE_AUTHOR("Emil Renner Berthing"); > +MODULE_DESCRIPTION("StarFive JH7110 always-on clock driver"); > +MODULE_LICENSE("GPL"); > -- > 2.38.1 >
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@starfivetech.com> wrote: >> From: Emil Renner Berthing <kernel@esmil.dk> >> >> 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@intel.com> >> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >> Co-developed-by: Hal Feng <hal.feng@starfivetech.com> >> Signed-off-by: Hal Feng <hal.feng@starfivetech.com> >> --- >> 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@esmil.dk> >> + * 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
On Tue, 28 Feb 2023 10:42:35 +0800, Hal Feng wrote: > 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@starfivetech.com> wrote: >>> From: Emil Renner Berthing <kernel@esmil.dk> >>> >>> 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@intel.com> >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >>> Co-developed-by: Hal Feng <hal.feng@starfivetech.com> >>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com> >>> --- >>> 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. Hi, Emil, The AON clock driver provides clocks for gmac0 which is used frequently. So I think it would be more convenient if we set "default y" here. Best regards, Hal
On Thu, 9 Mar 2023 at 10:44, Hal Feng <hal.feng@starfivetech.com> wrote: > On Tue, 28 Feb 2023 10:42:35 +0800, Hal Feng wrote: > > 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@starfivetech.com> wrote: > >>> From: Emil Renner Berthing <kernel@esmil.dk> > >>> > >>> 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@intel.com> > >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > >>> Co-developed-by: Hal Feng <hal.feng@starfivetech.com> > >>> Signed-off-by: Hal Feng <hal.feng@starfivetech.com> > >>> --- > >>> 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. > > Hi, Emil, > > The AON clock driver provides clocks for gmac0 which is used frequently. > So I think it would be more convenient if we set "default y" here. You're right that if we default y for the ethernet driver then the aon clock/reset should also default y. Personally I don't think we should default y for every ethernet driver that might be used on some supported risc-v platform, but I see now that arch/riscv/config/defconfig already contains CONFIG_MACB=y, CONFIG_E1000E=y, CONFIG_R8169=y and CONFIG_MICROSEMI_PHY=y, so maybe I'm wrong or just too late. > Best regards, > Hal
On Thu, Mar 09, 2023 at 03:06:13PM +0100, Emil Renner Berthing wrote: > On Thu, 9 Mar 2023 at 10:44, Hal Feng <hal.feng@starfivetech.com> wrote: > > The AON clock driver provides clocks for gmac0 which is used frequently. > > So I think it would be more convenient if we set "default y" here. > You're right that if we default y for the ethernet driver then the aon > clock/reset should also default y. Personally I don't think we should > default y for every ethernet driver that might be used on some > supported risc-v platform, but I see now that > arch/riscv/config/defconfig already contains CONFIG_MACB=y, > CONFIG_E1000E=y, CONFIG_R8169=y and CONFIG_MICROSEMI_PHY=y, so maybe > I'm wrong or just too late. The defconfig really needs a good bit of cleanup (one of the many things that I am telling myself I will do as part of kconfig.socs cleanup). w.r.t defconfig Palmer said it pretty well earlier on IRC: "defconfig should be useful for kernel devs, which means it should boot on the common dev boards". IMO, that means enough to boot an initramfs and poke the thing to see that it is alive, so: ethernet & serial, and the clocks/resets/pinctrl stuff required to get those going can all be set to y in defconfig. In the driver Kconfig entries, to me, it's more or less the same. I guess, answer the question "Will your customer's board get to the point where it can load a module ithout building this into the kernel?". If the answer to that question is yes, then don't make it default y. That's my €0.02!
On Thu, 9 Mar 2023 at 19:11, Conor Dooley <conor@kernel.org> wrote: > > On Thu, Mar 09, 2023 at 03:06:13PM +0100, Emil Renner Berthing wrote: > > On Thu, 9 Mar 2023 at 10:44, Hal Feng <hal.feng@starfivetech.com> wrote: > > > > The AON clock driver provides clocks for gmac0 which is used frequently. > > > So I think it would be more convenient if we set "default y" here. > > > You're right that if we default y for the ethernet driver then the aon > > clock/reset should also default y. Personally I don't think we should > > default y for every ethernet driver that might be used on some > > supported risc-v platform, but I see now that > > arch/riscv/config/defconfig already contains CONFIG_MACB=y, > > CONFIG_E1000E=y, CONFIG_R8169=y and CONFIG_MICROSEMI_PHY=y, so maybe > > I'm wrong or just too late. > > The defconfig really needs a good bit of cleanup (one of the many things > that I am telling myself I will do as part of kconfig.socs cleanup). > > w.r.t defconfig Palmer said it pretty well earlier on IRC: "defconfig > should be useful for kernel devs, which means it should boot on the > common dev boards". > > IMO, that means enough to boot an initramfs and poke the thing to see > that it is alive, so: ethernet & serial, and the clocks/resets/pinctrl > stuff required to get those going can all be set to y in defconfig. > > In the driver Kconfig entries, to me, it's more or less the same. > I guess, answer the question "Will your customer's board get to the > point where it can load a module ithout building this into the kernel?". > If the answer to that question is yes, then don't make it default y. > > That's my €0.02! Cool. Defaulting to m in the Kconfig for anything that can be loaded later is exactly what I was trying to say, except I mixed in the defconfig for no good reason. That means both the aon clocks and dwmac-starfive should default to m in Kconfig. The JH7110 (VisionFive 2) boots just fine like that and brings up aon clocks and ethernet after loading the modules. /Emil
On Thu, 09 Mar 2023 10:19:06 PST (-0800), emil.renner.berthing@canonical.com wrote: > On Thu, 9 Mar 2023 at 19:11, Conor Dooley <conor@kernel.org> wrote: >> >> On Thu, Mar 09, 2023 at 03:06:13PM +0100, Emil Renner Berthing wrote: >> > On Thu, 9 Mar 2023 at 10:44, Hal Feng <hal.feng@starfivetech.com> wrote: >> >> > > The AON clock driver provides clocks for gmac0 which is used frequently. >> > > So I think it would be more convenient if we set "default y" here. >> >> > You're right that if we default y for the ethernet driver then the aon >> > clock/reset should also default y. Personally I don't think we should >> > default y for every ethernet driver that might be used on some >> > supported risc-v platform, but I see now that >> > arch/riscv/config/defconfig already contains CONFIG_MACB=y, >> > CONFIG_E1000E=y, CONFIG_R8169=y and CONFIG_MICROSEMI_PHY=y, so maybe >> > I'm wrong or just too late. >> >> The defconfig really needs a good bit of cleanup (one of the many things >> that I am telling myself I will do as part of kconfig.socs cleanup). >> >> w.r.t defconfig Palmer said it pretty well earlier on IRC: "defconfig >> should be useful for kernel devs, which means it should boot on the >> common dev boards". >> >> IMO, that means enough to boot an initramfs and poke the thing to see >> that it is alive, so: ethernet & serial, and the clocks/resets/pinctrl >> stuff required to get those going can all be set to y in defconfig. >> >> In the driver Kconfig entries, to me, it's more or less the same. >> I guess, answer the question "Will your customer's board get to the >> point where it can load a module ithout building this into the kernel?". >> If the answer to that question is yes, then don't make it default y. >> >> That's my €0.02! > > Cool. Defaulting to m in the Kconfig for anything that can be loaded > later is exactly what I was trying to say, except I mixed in the > defconfig for no good reason. That means both the aon clocks and > dwmac-starfive should default to m in Kconfig. The JH7110 (VisionFive > 2) boots just fine like that and brings up aon clocks and ethernet > after loading the modules. That seems pretty reasonable to me. It's not like defconfig or Kconfig defaults or whatever are the only things we're going to test, but it's way easier for folks trying poke around with these dev boards if they boot defconfig.
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 + 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@esmil.dk> + * 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), +}; + +static struct clk_hw *jh7110_aonclk_get(struct of_phandle_args *clkspec, void *data) +{ + struct jh71x0_clk_priv *priv = data; + unsigned int idx = clkspec->args[0]; + + if (idx < JH7110_AONCLK_END) + return &priv->reg[idx].hw; + + return ERR_PTR(-EINVAL); +} + +static int jh7110_aoncrg_probe(struct platform_device *pdev) +{ + struct jh71x0_clk_priv *priv; + unsigned int idx; + int ret; + + priv = devm_kzalloc(&pdev->dev, + struct_size(priv, reg, JH7110_AONCLK_END), + GFP_KERNEL); + if (!priv) + return -ENOMEM; + + spin_lock_init(&priv->rmw_lock); + priv->dev = &pdev->dev; + priv->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(priv->base)) + return PTR_ERR(priv->base); + + dev_set_drvdata(priv->dev, (void *)(&priv->base)); + + for (idx = 0; idx < JH7110_AONCLK_END; idx++) { + u32 max = jh7110_aonclk_data[idx].max; + struct clk_parent_data parents[4] = {}; + struct clk_init_data init = { + .name = jh7110_aonclk_data[idx].name, + .ops = starfive_jh71x0_clk_ops(max), + .parent_data = parents, + .num_parents = + ((max & JH71X0_CLK_MUX_MASK) >> JH71X0_CLK_MUX_SHIFT) + 1, + .flags = jh7110_aonclk_data[idx].flags, + }; + struct jh71x0_clk *clk = &priv->reg[idx]; + unsigned int i; + + for (i = 0; i < init.num_parents; i++) { + unsigned int pidx = jh7110_aonclk_data[idx].parents[i]; + + if (pidx < JH7110_AONCLK_END) + parents[i].hw = &priv->reg[pidx].hw; + else if (pidx == JH7110_AONCLK_OSC) + parents[i].fw_name = "osc"; + else if (pidx == JH7110_AONCLK_RTC_OSC) + parents[i].fw_name = "rtc_osc"; + else if (pidx == JH7110_AONCLK_GMAC0_RMII_REFIN) + parents[i].fw_name = "gmac0_rmii_refin"; + else if (pidx == JH7110_AONCLK_GMAC0_RGMII_RXIN) + parents[i].fw_name = "gmac0_rgmii_rxin"; + else if (pidx == JH7110_AONCLK_STG_AXIAHB) + parents[i].fw_name = "stg_axiahb"; + else if (pidx == JH7110_AONCLK_APB_BUS) + parents[i].fw_name = "apb_bus"; + else if (pidx == JH7110_AONCLK_GMAC0_GTXCLK) + parents[i].fw_name = "gmac0_gtxclk"; + } + + clk->hw.init = &init; + clk->idx = idx; + clk->max_div = max & JH71X0_CLK_DIV_MASK; + + ret = devm_clk_hw_register(&pdev->dev, &clk->hw); + if (ret) + return ret; + } + + ret = devm_of_clk_add_hw_provider(&pdev->dev, jh7110_aonclk_get, priv); + if (ret) + return ret; + + return jh7110_reset_controller_register(priv, "reset-aon", 1); +} + +static const struct of_device_id jh7110_aoncrg_match[] = { + { .compatible = "starfive,jh7110-aoncrg" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, jh7110_aoncrg_match); + +static struct platform_driver jh7110_aoncrg_driver = { + .probe = jh7110_aoncrg_probe, + .driver = { + .name = "clk-starfive-jh7110-aon", + .of_match_table = jh7110_aoncrg_match, + }, +}; +module_platform_driver(jh7110_aoncrg_driver); + +MODULE_AUTHOR("Emil Renner Berthing"); +MODULE_DESCRIPTION("StarFive JH7110 always-on clock driver"); +MODULE_LICENSE("GPL");