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

Emil Renner Berthing
2023-03-09T18:19:37Z

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