[v2] clkdev: Update clkdev id usage to allow for longer names

Message ID 20240223163516.796606-1-michael.j.ruhl@intel.com
State New
Headers
Series [v2] clkdev: Update clkdev id usage to allow for longer names |

Commit Message

Michael J. Ruhl Feb. 23, 2024, 4:35 p.m. UTC
  clkdev DEV ID information is limited to an array of 20 bytes
(MAX_DEV_ID).  It is possible that the ID could be longer than
that.  If so, the lookup will fail because the "real ID" will
not match the copied value.

For instance, generating a device name for the I2C Designware
module using the PCI ID can result in a name of:

i2c_designware.39424

clkdev_create() will store:

i2c_designware.3942

The stored name is one off and will not match correctly during probe.

Increase the size of the ID to allow for a longer name.

v2: Removed CON_ID update and added example to commit

Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
---
 drivers/clk/clkdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Andy Shevchenko Feb. 23, 2024, 5:42 p.m. UTC | #1
On Fri, Feb 23, 2024 at 11:35:16AM -0500, Michael J. Ruhl wrote:
> clkdev DEV ID information is limited to an array of 20 bytes
> (MAX_DEV_ID).  It is possible that the ID could be longer than
> that.  If so, the lookup will fail because the "real ID" will
> not match the copied value.
> 
> For instance, generating a device name for the I2C Designware
> module using the PCI ID can result in a name of:
> 
> i2c_designware.39424
> 
> clkdev_create() will store:
> 
> i2c_designware.3942
> 
> The stored name is one off and will not match correctly during probe.
> 
> Increase the size of the ID to allow for a longer name.

> v2: Removed CON_ID update and added example to commit

Usually we put changelog out of the commit messages...

> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---

..somewhere here.

Also just noticed that you forgot CCF maintainers to be included.
I'm suggesting to use my script [1] which harvests more or less
adequate list of people and mailing lists to Cc email to.

[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
  
Michael J. Ruhl Feb. 23, 2024, 6:22 p.m. UTC | #2
>-----Original Message-----
>From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>Sent: Friday, February 23, 2024 12:43 PM
>To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: linux@armlinux.org.uk; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH v2] clkdev: Update clkdev id usage to allow for longer
>names
>
>On Fri, Feb 23, 2024 at 11:35:16AM -0500, Michael J. Ruhl wrote:
>> clkdev DEV ID information is limited to an array of 20 bytes
>> (MAX_DEV_ID).  It is possible that the ID could be longer than
>> that.  If so, the lookup will fail because the "real ID" will
>> not match the copied value.
>>
>> For instance, generating a device name for the I2C Designware
>> module using the PCI ID can result in a name of:
>>
>> i2c_designware.39424
>>
>> clkdev_create() will store:
>>
>> i2c_designware.3942
>>
>> The stored name is one off and will not match correctly during probe.
>>
>> Increase the size of the ID to allow for a longer name.
>
>> v2: Removed CON_ID update and added example to commit
>
>Usually we put changelog out of the commit messages...

Yeah, usually put it in the cover letter.  I will remove. I see your script automatically
does a cover page...will use that format int the future.

>> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
>> ---
>
>...somewhere here.

Ok.    Will keep for future reference.

>Also just noticed that you forgot CCF maintainers to be included.
>I'm suggesting to use my script [1] which harvests more or less
>adequate list of people and mailing lists to Cc email to.
>
>[1]: https://github.com/andy-shev/home-bin-
>tools/blob/master/ge2maintainer.sh

Using your script I got:

To: "Michael J. Ruhl" <michael.j.ruhl@intel.com>,
        linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Cc: Russell King <linux@armlinux.org.uk>

My list (using get_maintainers.pl) is:

linux@armlinux.org.uk
linux-arm-kernel@lists.infradead.org
linux-kernel@vger.kernel.org

They appear to be the same....

I don't have the plain text part on Russel's email (linxu@armlinux.org.uk).. Is that what is missing?

Not sure what CCF is?

M

>
>--
>With Best Regards,
>Andy Shevchenko
>
  
Andy Shevchenko Feb. 23, 2024, 6:32 p.m. UTC | #3
On Fri, Feb 23, 2024 at 06:22:13PM +0000, Ruhl, Michael J wrote:
> >From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >Sent: Friday, February 23, 2024 12:43 PM
> >On Fri, Feb 23, 2024 at 11:35:16AM -0500, Michael J. Ruhl wrote:

..

> I will remove.

Not remove, but move to the comments/changelog (after '---' line)

> I see your script automatically does a cover page...will use that format int
> the future.

Only if there are more than a single patch.

..

> >[1]: https://github.com/andy-shev/home-bin-
> >tools/blob/master/ge2maintainer.sh
> 
> Using your script I got:
> 
> To: "Michael J. Ruhl" <michael.j.ruhl@intel.com>,
>         linux-arm-kernel@lists.infradead.org,
>         linux-kernel@vger.kernel.org
> Cc: Russell King <linux@armlinux.org.uk>
> 
> My list (using get_maintainers.pl) is:
> 
> linux@armlinux.org.uk
> linux-arm-kernel@lists.infradead.org
> linux-kernel@vger.kernel.org
> 
> They appear to be the same....

Ah, the Russel's email looked like a mailing list, that what confused me.

> I don't have the plain text part on Russel's email (linxu@armlinux.org.uk)... Is that what is missing?

Yes :-)
But my script also uses a heuristics (which is not visible here) to add active
developers of the code in question based on the git history.

> Not sure what CCF is?

Common Clock Framework (This is how it's advertised in MAINTAINERS)
  
Russell King (Oracle) Feb. 23, 2024, 7:49 p.m. UTC | #4
On Fri, Feb 23, 2024 at 08:32:27PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 23, 2024 at 06:22:13PM +0000, Ruhl, Michael J wrote:
> > >From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > >Sent: Friday, February 23, 2024 12:43 PM
> > >On Fri, Feb 23, 2024 at 11:35:16AM -0500, Michael J. Ruhl wrote:
> 
> ...
> 
> > I will remove.
> 
> Not remove, but move to the comments/changelog (after '---' line)
> 
> > I see your script automatically does a cover page...will use that format int
> > the future.
> 
> Only if there are more than a single patch.
> 
> ...
> 
> > >[1]: https://github.com/andy-shev/home-bin-
> > >tools/blob/master/ge2maintainer.sh
> > 
> > Using your script I got:
> > 
> > To: "Michael J. Ruhl" <michael.j.ruhl@intel.com>,
> >         linux-arm-kernel@lists.infradead.org,
> >         linux-kernel@vger.kernel.org
> > Cc: Russell King <linux@armlinux.org.uk>
> > 
> > My list (using get_maintainers.pl) is:
> > 
> > linux@armlinux.org.uk
> > linux-arm-kernel@lists.infradead.org
> > linux-kernel@vger.kernel.org
> > 
> > They appear to be the same....
> 
> Ah, the Russel's email looked like a mailing list, that what confused me.

Joe, I think you know that I'll pick up on your mis-spelling of my
name... and I take that as an implicit right to call you something
other than your proper name. :D

Secondly, because the Cc contained my name, I fail to see how you can
confuse that with a mailing list. Maybe your script that you mentioned
strips the names from the email addresses, thereby adding to your
confusion - and maybe that isn't such a good idea after all? I'm not
the only one who uses linux@... There are six people in total listed in
MAINTAINERS who have a linux@... email address there.

> > I don't have the plain text part on Russel's email (linxu@armlinux.org.uk)... Is that what is missing?
> 
> Yes :-)
> But my script also uses a heuristics (which is not visible here) to add active
> developers of the code in question based on the git history.

The developers in question for this part of the code is me and not the
CCF. Therefore, what has been done by the patch author is reasonable
and no special scripts are necessary.

While my main git server is offline, I'm happy for the CCF folk
to pick this up, so:

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Michael, please resubmit with my r-b line above, and include the CCF
folk in that posting:

Michael Turquette <mturquette@baylibre.com>
Stephen Boyd <sboyd@kernel.org>
linux-clk@vger.kernel.org

Thanks!
  
Michael J. Ruhl Feb. 23, 2024, 8:12 p.m. UTC | #5
>-----Original Message-----
>From: Russell King <linux@armlinux.org.uk>
>Sent: Friday, February 23, 2024 2:50 PM
>To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Ruhl, Michael J
><michael.j.ruhl@intel.com>
>Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH v2] clkdev: Update clkdev id usage to allow for longer
>names
>
>On Fri, Feb 23, 2024 at 08:32:27PM +0200, Andy Shevchenko wrote:
>> On Fri, Feb 23, 2024 at 06:22:13PM +0000, Ruhl, Michael J wrote:
>> > >From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> > >Sent: Friday, February 23, 2024 12:43 PM
>> > >On Fri, Feb 23, 2024 at 11:35:16AM -0500, Michael J. Ruhl wrote:
>>
>> ...
>>
>> > I will remove.
>>
>> Not remove, but move to the comments/changelog (after '---' line)
>>
>> > I see your script automatically does a cover page...will use that format int
>> > the future.
>>
>> Only if there are more than a single patch.
>>
>> ...
>>
>> > >[1]: https://github.com/andy-shev/home-bin-
>> > >tools/blob/master/ge2maintainer.sh
>> >
>> > Using your script I got:
>> >
>> > To: "Michael J. Ruhl" <michael.j.ruhl@intel.com>,
>> >         linux-arm-kernel@lists.infradead.org,
>> >         linux-kernel@vger.kernel.org
>> > Cc: Russell King <linux@armlinux.org.uk>
>> >
>> > My list (using get_maintainers.pl) is:
>> >
>> > linux@armlinux.org.uk
>> > linux-arm-kernel@lists.infradead.org
>> > linux-kernel@vger.kernel.org
>> >
>> > They appear to be the same....
>>
>> Ah, the Russel's email looked like a mailing list, that what confused me.
>
>Joe, I think you know that I'll pick up on your mis-spelling of my
>name... and I take that as an implicit right to call you something
>other than your proper name. :D
>
>Secondly, because the Cc contained my name, I fail to see how you can
>confuse that with a mailing list. Maybe your script that you mentioned
>strips the names from the email addresses, thereby adding to your
>confusion - and maybe that isn't such a good idea after all? I'm not
>the only one who uses linux@... There are six people in total listed in
>MAINTAINERS who have a linux@... email address there.

😊

Andy's script picked up your name for the CC... I stripped out all but the email text
for my posting... so this is my fault....

>
>> > I don't have the plain text part on Russel's email (linxu@armlinux.org.uk)...
>Is that what is missing?
>>
>> Yes :-)
>> But my script also uses a heuristics (which is not visible here) to add active
>> developers of the code in question based on the git history.
>
>The developers in question for this part of the code is me and not the
>CCF. Therefore, what has been done by the patch author is reasonable
>and no special scripts are necessary.
>
>While my main git server is offline, I'm happy for the CCF folk
>to pick this up, so:
>
>Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>
>Michael, please resubmit with my r-b line above, and include the CCF
>folk in that posting:

I will get the appropriate names listed and repost.

Thank you!

M

>Michael Turquette <mturquette@baylibre.com>
>Stephen Boyd <sboyd@kernel.org>
>linux-clk@vger.kernel.org
>
>Thanks!
>
>--
>RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
>FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
  
Andy Shevchenko Feb. 23, 2024, 8:14 p.m. UTC | #6
On Fri, Feb 23, 2024 at 07:49:33PM +0000, Russell King (Oracle) wrote:
> On Fri, Feb 23, 2024 at 08:32:27PM +0200, Andy Shevchenko wrote:
> > On Fri, Feb 23, 2024 at 06:22:13PM +0000, Ruhl, Michael J wrote:
> > > >From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > >Sent: Friday, February 23, 2024 12:43 PM
> > > >On Fri, Feb 23, 2024 at 11:35:16AM -0500, Michael J. Ruhl wrote:

..

> > > >[1]: https://github.com/andy-shev/home-bin-
> > > >tools/blob/master/ge2maintainer.sh
> > > 
> > > Using your script I got:
> > > 
> > > To: "Michael J. Ruhl" <michael.j.ruhl@intel.com>,
> > >         linux-arm-kernel@lists.infradead.org,
> > >         linux-kernel@vger.kernel.org
> > > Cc: Russell King <linux@armlinux.org.uk>
> > > 
> > > My list (using get_maintainers.pl) is:
> > > 
> > > linux@armlinux.org.uk
> > > linux-arm-kernel@lists.infradead.org
> > > linux-kernel@vger.kernel.org
> > > 
> > > They appear to be the same....
> > 
> > Ah, the Russel's email looked like a mailing list, that what confused me.
> 
> Joe, I think you know that I'll pick up on your mis-spelling of my
> name... and I take that as an implicit right to call you something
> other than your proper name. :D

Since, Javier, you told me that, I now remember some rumors... :D

> Secondly, because the Cc contained my name, I fail to see how you can
> confuse that with a mailing list. Maybe your script that you mentioned
> strips the names from the email addresses, thereby adding to your
> confusion - and maybe that isn't such a good idea after all?

It's other way around. My script uses full names.

> I'm not the only one who uses linux@... There are six people in total listed
> in MAINTAINERS who have a linux@... email address there.

Yes, but you are the only one which pops up WRT this file.

> > > I don't have the plain text part on Russel's email
> > > (linxu@armlinux.org.uk)... Is that what is missing?
> > 
> > Yes :-)
> > But my script also uses a heuristics (which is not visible here) to add active
> > developers of the code in question based on the git history.
> 
> The developers in question for this part of the code is me and not the
> CCF.

Yes, get_maintainer.pl seems to return that. It's me who naively considered you
as CCF maintainer.

> Therefore, what has been done by the patch author is reasonable
> and no special scripts are necessary.

The scripts makes life easier and robust against changes.
  

Patch

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index ee37d0be6877..9cd80522ca2d 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -144,7 +144,7 @@  void clkdev_add_table(struct clk_lookup *cl, size_t num)
 	mutex_unlock(&clocks_mutex);
 }
 
-#define MAX_DEV_ID	20
+#define MAX_DEV_ID	24
 #define MAX_CON_ID	16
 
 struct clk_lookup_alloc {