Re: [PATCH v6 3/6] clk: spacemit: Add clock support for SpacemiT K1 SoC

[Yixun Lan <dlan@gentoo.org>]

Hi Inochi,

On 09:20 Thu 10 Apr     , Inochi Amaoto wrote:
> On Wed, Apr 09, 2025 at 08:10:53PM -0500, Alex Elder wrote:
> > On 4/9/25 7:57 PM, Inochi Amaoto wrote:
> > > > > > > diff --git a/drivers/clk/spacemit/Kconfig b/drivers/clk/spacemit/Kconfig
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..4c4df845b3cb
> > > > > > > --- /dev/null
> > > > > > > +++ b/drivers/clk/spacemit/Kconfig
> > > > > > > @@ -0,0 +1,18 @@
> > > > > > > +# SPDX-License-Identifier: GPL-2.0-only
> > > > > > > +
> > > > > > > +config SPACEMIT_CCU
> > > > > > > +	tristate "Clock support for SpacemiT SoCs"
> > > > > > I don't know the answer to this, but...  Should this be a Boolean
> > > > > > rather than tristate?  Can a SpacemiT K1 SoC function without the
> > > > > > clock driver built in to the kernel?
> > > > > > 
> > > > > I agree to make it a Boolean, we've already made pinctrl driver Boolean
> > > > > and pinctrl depend on clk, besides, the SoC is unlikely functional
> > > > > without clock built in as it's such critical..
> > > > > 
> > > > I disagree. The kernel is only for spacemit only, and the pinctrl
> > > Sorry for a mistake, this first "only" should be "not".
> > 
> > This is a general problem.  You can't make a bootable
> > SpacemiT kernel unless you define this as built-in (at
> > least, that's what Yixun is saying). 
> 
> Why not putting the module in the initramfs? I have tested
> this in quite a lot of boards (Allwinner, rockchip, sophgo,
> starfive and etc.), all of them work well.
> 
it works, but not optimal, why delay clk initialzation at modules load stage?
IMO, it brings more overhead for using initramfs..

but there is always tradeoff and bikeshedding..

> > But we'd really rather *only* build it in to the kernel
> > for SpacemiT builds. You clearly want to minimize what
> > must be built in, but what if this is indeed required?
> > What goes in defconfig?
> > 
> 
> As defconfig is more like for a minimum example system. It
> is OK to put a y in the defconfig. But for a custom system,
> you do give a choice for the builder to remove your module
> in non spacemit system.

I get your meaning here to remove/disable at run time stage, while
we do provide compile time option, if don't want spacemit system
just disable CONFIG_ARCH_SPACEMIT I mentioned, clk/pinctrl will be gone

anyway, I'm open for this, make it tristate do provide more choices,
and it's probably better leave users to decide..

Ok, I'm fine with leave clk as tristate with proper default deconfig,
but if people want to pursue more to make more driver(pinctrl) modulized
feel free to test and send patches, I just won't put efforts myself.

thanks

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55