clk mess on omap4460 with mpu clock

clk mess on omap4460 with mpu clock

[Andreas Kemnade]

Hi,

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

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.

Regards,
Andreas

Re: clk mess on omap4460 with mpu clock

[Geert Uytterhoeven]

Hi Andreas,

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.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: clk mess on omap4460 with mpu clock

[Andreas Kemnade]

Am Mon, 2 Sep 2024 15:53:07 +0200
schrieb Geert Uytterhoeven <geert@linux-m68k.org>:

> Hi Andreas,
> 
> 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.

Regards,
Andreas

Re: clk mess on omap4460 with mpu clock

[Geert Uytterhoeven]

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 :-(

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: clk mess on omap4460 with mpu clock

[H. Nikolaus Schaller]

Hi,

> Am 03.09.2024 um 14:36 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

I just have my PandaES up and running today (with LetuxOS) and there I can't see
such a report with v6.11-rc6 and everything seems to be fine (except some certainly
unrelated  44000000.l3-noc:L3 Standard Error: MASTER MPU TARGET DSS (Read Link): At Address: 0x00006070 : Data Access in User mode during Functional access).

So can it be a spurious error in 6.10-rc1 and is already fixed (maybe as a side-effect)?
Or do we have a happy fix in our distribution kernel? Or does it depend on CONFIG?

I can roll back and try to build&run letux-6.10-rc1 as well.

BR,
Nikolaus

Re: [Letux-kernel] clk mess on omap4460 with mpu clock

[H. Nikolaus Schaller]

> Am 03.09.2024 um 15:09 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> I can roll back and try to build&run letux-6.10-rc1 as well.

Ok, I can confirm this issue in 6.10-rc1:

[    0.000000] Linux version 6.10.0-rc1-letux+ (hns@iMac.local) (arm-linux-gnueabihf-gcc (GCC) 6.3.0, GNU ld (GNU Binutils) 2.27) #18983 SMP PREEMPT Tue Sep  3 15:18:59 CEST 2024
[    0.000000] CPU: ARMv7 Processor [412fc09a] revision 10 (ARMv7), cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] OF: fdt: Machine model: TI OMAP4 PandaBoard-ES
...
[    2.315277] omap_i2c 48350000.i2c: bus 3 rev0.11 at 400 kHz
[    2.325073] ocp:target-module@48210000:mpu:fck: device ID is greater than 24
[    2.332580] ti-sysc ocp:target-module@48210000: could not add child clock fck: -12
[    2.356781] omap_wdt: OMAP Watchdog Timer Rev 0x00: initial timeout 60 sec

Boot process is stuck with 
SELinux:  Could not open policy file <= /etc/selinux/targeted/policy/policy.33:  No such file or directory

Both effects are gone with 6.11-rc6.

If I find time I can try some versions in between.

BR,
Nikolaus

Re: [Letux-kernel] clk mess on omap4460 with mpu clock

[H. Nikolaus Schaller]

Hi all,

> Am 03.09.2024 um 16:00 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> 
> 
>> Am 03.09.2024 um 15:09 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>> 
>> I can roll back and try to build&run letux-6.10-rc1 as well.
> 
> Ok, I can confirm this issue in 6.10-rc1:
> 
> [    0.000000] Linux version 6.10.0-rc1-letux+ (hns@iMac.local) (arm-linux-gnueabihf-gcc (GCC) 6.3.0, GNU ld (GNU Binutils) 2.27) #18983 SMP PREEMPT Tue Sep  3 15:18:59 CEST 2024
> [    0.000000] CPU: ARMv7 Processor [412fc09a] revision 10 (ARMv7), cr=10c5387d
> [    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
> [    0.000000] OF: fdt: Machine model: TI OMAP4 PandaBoard-ES
> ...
> [    2.315277] omap_i2c 48350000.i2c: bus 3 rev0.11 at 400 kHz
> [    2.325073] ocp:target-module@48210000:mpu:fck: device ID is greater than 24
> [    2.332580] ti-sysc ocp:target-module@48210000: could not add child clock fck: -12
> [    2.356781] omap_wdt: OMAP Watchdog Timer Rev 0x00: initial timeout 60 sec
> 
> Boot process is stuck with 
> SELinux:  Could not open policy file <= /etc/selinux/targeted/policy/policy.33:  No such file or directory
> 
> Both effects are gone with 6.11-rc6.

I had made a mistake in updating the kernels and drawn a false conclusion.
The new result is:

all kernels I have tested between v6.10-rc1 and v6.11-rc6 report these messages.

But after 6.10-rc1 they appear to be harmless (I did not miss any functions so far)
and it is not clear if my 6.10-rc1 being stuck is related to this at all.

E.g. from 6.11-rc6:

root@letux:~# dmesg | fgrep 'device ID is greater than'
[    2.340148] ocp:target-module@48210000:mpu:fck: device ID is greater than 24
[    2.414978] ocp:target-module@54000000:pmu:fck: device ID is greater than 24
[    2.491973] 5a05a400.target-module:iva:fck: device ID is greater than 24
root@letux:~# 

BR,
Nikolaus

Re: clk mess on omap4460 with mpu clock

[Andreas Kemnade]

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.

Regards,
Andreas

Re: clk mess on omap4460 with mpu clock

[Russell King (Oracle)]

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.

Essentially, if you see the warning, the registration with clkdev is
both pointless and useless.

-- 
*** please note that I probably will only be occasionally responsive
*** for an unknown period of time due to recent eye surgery making
*** reading quite difficult.

RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: clk mess on omap4460 with mpu clock

[Andreas Kemnade]

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

Re: clk mess on omap4460 with mpu clock

[Russell King (Oracle)]

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.

An alternative approach may be this (untested, not even compile tested):

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);
+		va_end(ap_copy);
+	}
+
+	cla = kzalloc(size, GFP_KERNEL);
 	if (!cla)
 		return NULL;
 
-	va_copy(ap_copy, ap);
-
 	cla->cl.clk_hw = hw;
+	strp = cla->strings;
 	if (con_id) {
-		res = strscpy(cla->con_id, con_id, sizeof(cla->con_id));
-		if (res < 0) {
-			max_size = sizeof(cla->con_id);
-			failure = "connection";
-			goto fail;
-		}
-		cla->cl.con_id = cla->con_id;
+		strcpy(strp, con_id);
+		cla->cl.con_id = strp;
+		strp += 1 + strlen(con_id);
 	}
 
 	if (dev_fmt) {
-		res = vsnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap);
-		if (res >= sizeof(cla->dev_id)) {
-			max_size = sizeof(cla->dev_id);
-			failure = "device";
-			goto fail;
-		}
-		cla->cl.dev_id = cla->dev_id;
+		vsprintf(strp, dev_fmt, ap);
+		cla->cl.dev_id = strp;
 	}
 
-	va_end(ap_copy);
-
-	return &cla->cl;
-
-fail:
-	if (dev_fmt)
-		vaf.fmt = dev_fmt;
-	else
-		vaf.fmt = "null-device";
-	vaf.va = &ap_copy;
-	pr_err("%pV:%s: %s ID is greater than %zu\n",
-	       &vaf, con_id, failure, max_size);
-	va_end(ap_copy);
-
-	/*
-	 * Don't fail in this case, but as the entry won't ever match just
-	 * fill it with something that also won't match.
-	 */
-	strscpy(cla->con_id, "bad", sizeof(cla->con_id));
-	strscpy(cla->dev_id, "bad", sizeof(cla->dev_id));
-
 	return &cla->cl;
 }
 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: clk mess on omap4460 with mpu clock

[Andreas Kemnade]

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

Re: clk mess on omap4460 with mpu clock

[Russell King (Oracle)]

On Mon, Sep 23, 2024 at 02:04:47PM +0200, Andreas Kemnade wrote:
> 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

I'll also mention that if one is using DT, then the clkdev tables should
not be relevant - the lookups of clocks should be done by parsing the
clocks property in the description of the device.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: clk mess on omap4460 with mpu clock

[Andreas Kemnade]

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

> On Mon, Sep 23, 2024 at 02:04:47PM +0200, Andreas Kemnade wrote:
> > 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
> >  
> 
> I'll also mention that if one is using DT, then the clkdev tables
> should not be relevant - the lookups of clocks should be done by
> parsing the clocks property in the description of the device.
> 
I played around to ensure I know really what I am doing and what I am
messing around with:
we have in i2c-omap.c:
      
fclk = clk_get(omap->dev, "fck");
just for getting the module clock rate to configure i2c accordingly
That only works if
https://elixir.bootlin.com/linux/v6.11/source/drivers/bus/ti-sysc.c#L2380

is working. Here some clk_get(omap->dev->parent, ...) would also just
work without said line in ti-sysc.c because then dt lookup kicks in.

In the omap3 case, there are several devices using the older hwmod
mechanism and are not converted to ti-sysc yet. Then the clock gets
registered here:
https://elixir.bootlin.com/linux/v6.11/source/arch/arm/mach-omap2/omap_device.c#L116

In the case, a clk_get(omap->dev->parent, ...) in e.g. i2c-omap.c would
fail. No dt lookup can kick in.

The background of this is to ensure to keep soc-specific pm out of the
drivers and do it in a common place.

Regards,
Andreas