Re: clk mess on omap4460 with mpu clock

[Andreas Kemnade <andreas@kemnade.info>]

Am Tue, 24 Sep 2024 17:52:00 +0100
schrieb "Russell King (Oracle)" <linux@armlinux.org.uk>:

> On Mon, Sep 23, 2024 at 02:04:47PM +0200, Andreas Kemnade wrote:
> > The main question what bothers me is whether we have
> > some real problems behind it. The warning message is just an
> > indicator of something odd which was already odd before the message
> > was introduced.  
> 
> Indeed.
> 
> > I have seen something working with some u-boot and some other not,
> > so things might not get properly initialized... 
> > 
> > So the way forward is to check whether that registration is really
> > needed at:
> > https://elixir.bootlin.com/linux/v6.11/source/drivers/bus/ti-sysc.c#L2380
> > If yes, then
> > a) increade the size of the name in the clk subsystem or
> > b) workaround like
> > https://elixir.bootlin.com/linux/v6.11/source/drivers/bus/ti-sysc.c#L353
> >  
> 
> Or we make the arrays larger - at the moment, the struct is a nice
> round 64 bytes in 32-bit systems - 6 pointers (24 bytes) plus 24 plus
> 16 = 64. For 64-bit systems, this is 88 bytes.
> 
of course that is a nice size, but since the devid string is not
directly visible, so chances are high, that an innocent looking
commit might mess it up.

> An alternative approach may be this (untested, not even compile
> tested):
> 
so this looks like my favourite approach.

> diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
> index 2f83fb97c6fb..222f0ccf9fc0 100644
> --- a/drivers/clk/clkdev.c
> +++ b/drivers/clk/clkdev.c
> @@ -149,8 +149,7 @@ void clkdev_add_table(struct clk_lookup *cl,
> size_t num) 
>  struct clk_lookup_alloc {
>  	struct clk_lookup cl;
> -	char	dev_id[MAX_DEV_ID];
> -	char	con_id[MAX_CON_ID];
> +	char	strings[0];
>  };
>  
>  static struct clk_lookup * __ref
> @@ -158,60 +157,36 @@ vclkdev_alloc(struct clk_hw *hw, const char
> *con_id, const char *dev_fmt, va_list ap)
>  {
>  	struct clk_lookup_alloc *cla;
> -	struct va_format vaf;
> -	const char *failure;
>  	va_list ap_copy;
> -	size_t max_size;
> -	ssize_t res;
> +	size_t size;
> +	char *strp;
>  
> -	cla = kzalloc(sizeof(*cla), GFP_KERNEL);
> +	size = sizeof(*cla);
> +	if (con_id)
> +		size += 1 + strlen(con_id);
> +	if (dev_fmt) {
> +		va_copy(ap_copy, ap);
> +		size += 1 + vsprintf(NULL, dev_fmt, ap_copy);
                
size += 1 + vsnprintf(NULL, 0, dev_fmt, ap_copy);

works for me.
> +		va_end(ap_copy);
> +	}
> +
or setting size to some big enough value here...

without that, even earlycon is silent.

Regards,
Andreas