Re: clk mess on omap4460 with mpu clock

[Andreas Kemnade <andreas@kemnade.info>]

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

> On Tue, Sep 03, 2024 at 07:22:03PM +0200, Andreas Kemnade wrote:
> > Am Tue, 3 Sep 2024 14:36:05 +0200
> > schrieb Geert Uytterhoeven <geert@linux-m68k.org>:
> >   
> > > Hi Andreas,
> > > 
> > > On Tue, Sep 3, 2024 at 2:34 PM Andreas Kemnade
> > > <andreas@kemnade.info> wrote:  
> > > > Am Mon, 2 Sep 2024 15:53:07 +0200
> > > > schrieb Geert Uytterhoeven <geert@linux-m68k.org>:    
> > > > > On Mon, Jun 3, 2024 at 11:41 PM Andreas Kemnade
> > > > > <andreas@kemnade.info> wrote:    
> > > > > > just stumbled across this on 6.10-rc1:
> > > > > >
> > > > > > [    1.475830] ocp:target-module@48210000:mpu:fck: device
> > > > > > ID is greater than 24 [    1.483154] ti-sysc
> > > > > > ocp:target-module@48210000: could not add child clock fck:
> > > > > > -12    
> > > > >
> > > > > And on boneblack:
> > > > >
> > > > > 48000000.interconnect:segment@200000:target-module@0:mpu@0:fck:
> > > > > device ID is greater than 24
> > > > > target-module@4b000000:target-module@140000:pmu@0:fck: device
> > > > > ID is greater than 24
> > > > >    
> > > > > > Maybe
> > > > > >         /*
> > > > > >          * Use clkdev_add() instead of clkdev_alloc() to
> > > > > > avoid the MAX_DEV_ID
> > > > > >          * limit for clk_get(). If cl ever needs to be
> > > > > > freed, it should be done
> > > > > >          * with clkdev_drop().
> > > > > >          */
> > > > > > in ti-sysc.c does not work anymore?
> > > > > >
> > > > > > The offending clock definition is in omap4.dtsi
> > > > > >
> > > > > > clocks = <&mpuss_clkctrl OMAP4_MPU_CLKCTRL 0>;
> > > > > >
> > > > > > Did not bisect that yet.    
> > > > >
> > > > > Commit 8d532528ff6a6b1b ("clkdev: report over-sized strings
> > > > > when creating clkdev entries") in v6.10-rc1, with follow-up
> > > > > commit 616501eccb58615f ("clkdev: don't fail clkdev_alloc() if
> > > > > over-sized") in v6.10-rc4.
> > > > >
> > > > > I have no idea if these clkdev registrations are still
> > > > > necessary/used.   
> > > > well, it might explain some mystery behavior in the past. Lets
> > > > see where it comes from. As the comment says, there should be a
> > > > workaround against that limitation. So the problem should not be
> > > > there in the first place. I have some strange problems with
> > > > clk_disable_unused.
> > > >
> > > > I first thought it is a id < 24 issue and not a
> > > > strlen(something) < 24.    
> > > 
> > > Me too :-(
> > >   
> > Ok, setting
> > MAX_DEV_ID to 64 in clkdev.c lets the warnings disappear. ti-sys.c
> > has at one place precautions for overlong dev_ids, but on another
> > place it happily calls clkdev_create() running into this issue. 
> > 
> > The follow-up commit then again at least does not cause a failure
> > for dev registration. I am still unsure what the consequences are.
> > Between 6.10 and 6.11 something interesting happened which makes
> > the bt200 reliably boot with near-mainline u-boot+spl even without
> > clk_ignore_unused. So no frankenstein-boot (vendor X-Loader + new
> > U-boot) anymore.  
> 
> The bottom line: if you are getting warnings that the strings exceed
> the existing sizes, then _any_ lookups using clkdev will have been
> failing. Nothing has changed with that. The only thing that changed
> recently was to print a warning for this case, and initially to fail
> the attempt to register with clkdev. However, that broke stuff, so it
> was made not to fail, but still report the problem.
> 
yes, understood. 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.
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

Regards,
Andreas