* [PATCH] hwmon: (jc42) Fix name to have no illegal characters @ 2020-04-17 9:28 Sascha Hauer 2020-04-17 9:55 ` Jean Delvare 2020-04-18 17:13 ` Guenter Roeck 0 siblings, 2 replies; 7+ messages in thread From: Sascha Hauer @ 2020-04-17 9:28 UTC (permalink / raw) To: linux-hwmon; +Cc: Guenter Roeck, Jean Delvare, Sascha Hauer The jc42 driver passes I2C client's name as hwmon device name. In case of device tree probed devices this ends up being part of the compatible string, "jc-42.4-temp". This name contains hyphens and the hwmon core doesn't like this: jc42 2-0018: hwmon: 'jc-42.4-temp' is not a valid name attribute, please fix This changes the name to "jc42" which doesn't have any illegal characters. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- drivers/hwmon/jc42.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c index f2d81b0558e5..e3f1ebee7130 100644 --- a/drivers/hwmon/jc42.c +++ b/drivers/hwmon/jc42.c @@ -506,7 +506,7 @@ static int jc42_probe(struct i2c_client *client, const struct i2c_device_id *id) } data->config = config; - hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, + hwmon_dev = devm_hwmon_device_register_with_info(dev, "jc42", data, &jc42_chip_info, NULL); return PTR_ERR_OR_ZERO(hwmon_dev); -- 2.26.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] hwmon: (jc42) Fix name to have no illegal characters 2020-04-17 9:28 [PATCH] hwmon: (jc42) Fix name to have no illegal characters Sascha Hauer @ 2020-04-17 9:55 ` Jean Delvare 2020-04-17 10:32 ` Sascha Hauer 2020-04-18 3:14 ` Guenter Roeck 2020-04-18 17:13 ` Guenter Roeck 1 sibling, 2 replies; 7+ messages in thread From: Jean Delvare @ 2020-04-17 9:55 UTC (permalink / raw) To: Sascha Hauer; +Cc: linux-hwmon, Guenter Roeck Hi Sascha, On Fri, 17 Apr 2020 11:28:53 +0200, Sascha Hauer wrote: > The jc42 driver passes I2C client's name as hwmon device name. In case > of device tree probed devices this ends up being part of the compatible > string, "jc-42.4-temp". This name contains hyphens and the hwmon core > doesn't like this: > > jc42 2-0018: hwmon: 'jc-42.4-temp' is not a valid name attribute, please fix > > This changes the name to "jc42" which doesn't have any illegal > characters. I don't think "jc-42.4-temp" is a valid i2c client name either. I believe this should be fixed at the of->i2c level, rather than the i2c->hwmon level. Not sure how other drivers are dealing with that, it seems that in most cases the name part of the compatible string matches exactly the expected client name so no conversion is needed. Thanks, -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hwmon: (jc42) Fix name to have no illegal characters 2020-04-17 9:55 ` Jean Delvare @ 2020-04-17 10:32 ` Sascha Hauer 2020-04-17 13:47 ` Jean Delvare 2020-04-18 3:14 ` Guenter Roeck 1 sibling, 1 reply; 7+ messages in thread From: Sascha Hauer @ 2020-04-17 10:32 UTC (permalink / raw) To: Jean Delvare; +Cc: linux-hwmon, Guenter Roeck Hi Jean, On Fri, Apr 17, 2020 at 11:55:03AM +0200, Jean Delvare wrote: > Hi Sascha, > > On Fri, 17 Apr 2020 11:28:53 +0200, Sascha Hauer wrote: > > The jc42 driver passes I2C client's name as hwmon device name. In case > > of device tree probed devices this ends up being part of the compatible > > string, "jc-42.4-temp". This name contains hyphens and the hwmon core > > doesn't like this: > > > > jc42 2-0018: hwmon: 'jc-42.4-temp' is not a valid name attribute, please fix > > > > This changes the name to "jc42" which doesn't have any illegal > > characters. > > I don't think "jc-42.4-temp" is a valid i2c client name either. What makes the name invalid then? I am not aware of any constraints of i2c client names. > I believe this should be fixed at the of->i2c level, rather than the > i2c->hwmon level. Are you suggesting a character conversion from hyphens to underscores or similar in the i2c core? > Not sure how other drivers are dealing with that, it > seems that in most cases the name part of the compatible string matches > exactly the expected client name so no conversion is needed. Other i2c hwmon drivers I found do not have any hyphens in their compatible string, so they are at least not hit by this message. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hwmon: (jc42) Fix name to have no illegal characters 2020-04-17 10:32 ` Sascha Hauer @ 2020-04-17 13:47 ` Jean Delvare 2020-04-18 3:19 ` Guenter Roeck 0 siblings, 1 reply; 7+ messages in thread From: Jean Delvare @ 2020-04-17 13:47 UTC (permalink / raw) To: Sascha Hauer; +Cc: linux-hwmon, Guenter Roeck On Fri, 17 Apr 2020 12:32:55 +0200, Sascha Hauer wrote: > On Fri, Apr 17, 2020 at 11:55:03AM +0200, Jean Delvare wrote: > > On Fri, 17 Apr 2020 11:28:53 +0200, Sascha Hauer wrote: > > > The jc42 driver passes I2C client's name as hwmon device name. In case > > > of device tree probed devices this ends up being part of the compatible > > > string, "jc-42.4-temp". This name contains hyphens and the hwmon core > > > doesn't like this: > > > > > > jc42 2-0018: hwmon: 'jc-42.4-temp' is not a valid name attribute, please fix > > > > > > This changes the name to "jc42" which doesn't have any illegal > > > characters. > > > > I don't think "jc-42.4-temp" is a valid i2c client name either. > > What makes the name invalid then? I am not aware of any constraints of > i2c client names. Historically hwmon devices were i2c devices so libsensors would use the i2c client name as the device name, and still does as a fallback if memory serves. Therefore whatever rule applies to hwmon names would also apply to i2c names (in the context of hwmon devices) even though this is not enforced. > > I believe this should be fixed at the of->i2c level, rather than the > > i2c->hwmon level. > > Are you suggesting a character conversion from hyphens to underscores or > similar in the i2c core? No, my point is that from a userspace perspective it shouldn't matter if the device comes from the OF tree or not. So the device name should be the same, i.e. the i2c client should be named "jc42" always. That's what happens for all other devices I checked, simply because it turns out that their OF compatible string is in the form <vendor_name>,<linux_i2c_client_name>, so when you strip the vendor name you get the right name directly. My knowledge of the OF subsystem is fairly limited though, so I have no idea if it is possible to enforce a specific name like that at an early stage. The proper name can't be guessed by i2c-core, so ideally the conversion should be provided by the driver itself. I see that struct of_device_id has a "name" field, can it be used for that purpose? If not, I suppose the "data" field could be used for that. > > Not sure how other drivers are dealing with that, it > > seems that in most cases the name part of the compatible string matches > > exactly the expected client name so no conversion is needed. > > Other i2c hwmon drivers I found do not have any hyphens in their > compatible string, so they are at least not hit by this message. I drew the same conclusion here, and your patch is definitely better than nothing as it fixes a real problem, however it is not prefect due to the reason explained above, plus the fact that it would break if the driver ever supports more than one device type (say JEDEC releases JC43 tomorrow and we add support... you code forces the name to "jc42" even if the OF name was something other than "jc-42.4-temp"). -- Jean Delvare SUSE L3 Support ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hwmon: (jc42) Fix name to have no illegal characters 2020-04-17 13:47 ` Jean Delvare @ 2020-04-18 3:19 ` Guenter Roeck 0 siblings, 0 replies; 7+ messages in thread From: Guenter Roeck @ 2020-04-18 3:19 UTC (permalink / raw) To: Jean Delvare, Sascha Hauer; +Cc: linux-hwmon On 4/17/20 6:47 AM, Jean Delvare wrote: > On Fri, 17 Apr 2020 12:32:55 +0200, Sascha Hauer wrote: >> On Fri, Apr 17, 2020 at 11:55:03AM +0200, Jean Delvare wrote: >>> On Fri, 17 Apr 2020 11:28:53 +0200, Sascha Hauer wrote: >>>> The jc42 driver passes I2C client's name as hwmon device name. In case >>>> of device tree probed devices this ends up being part of the compatible >>>> string, "jc-42.4-temp". This name contains hyphens and the hwmon core >>>> doesn't like this: >>>> >>>> jc42 2-0018: hwmon: 'jc-42.4-temp' is not a valid name attribute, please fix >>>> >>>> This changes the name to "jc42" which doesn't have any illegal >>>> characters. >>> >>> I don't think "jc-42.4-temp" is a valid i2c client name either. >> >> What makes the name invalid then? I am not aware of any constraints of >> i2c client names. > > Historically hwmon devices were i2c devices so libsensors would use > the i2c client name as the device name, and still does as a fallback if > memory serves. Therefore whatever rule applies to hwmon names would > also apply to i2c names (in the context of hwmon devices) even though > this is not enforced. > >>> I believe this should be fixed at the of->i2c level, rather than the >>> i2c->hwmon level. >> >> Are you suggesting a character conversion from hyphens to underscores or >> similar in the i2c core? > > No, my point is that from a userspace perspective it shouldn't matter > if the device comes from the OF tree or not. So the device name should > be the same, i.e. the i2c client should be named "jc42" always. That's > what happens for all other devices I checked, simply because it turns > out that their OF compatible string is in the form > <vendor_name>,<linux_i2c_client_name>, so when you strip the vendor > name you get the right name directly. > > My knowledge of the OF subsystem is fairly limited though, so I have no > idea if it is possible to enforce a specific name like that at an early > stage. The proper name can't be guessed by i2c-core, so ideally the > conversion should be provided by the driver itself. I see that struct > of_device_id has a "name" field, can it be used for that purpose? If > not, I suppose the "data" field could be used for that. > >>> Not sure how other drivers are dealing with that, it >>> seems that in most cases the name part of the compatible string matches >>> exactly the expected client name so no conversion is needed. >> >> Other i2c hwmon drivers I found do not have any hyphens in their >> compatible string, so they are at least not hit by this message. > > I drew the same conclusion here, and your patch is definitely better > than nothing as it fixes a real problem, however it is not prefect due > to the reason explained above, plus the fact that it would break if the > driver ever supports more than one device type (say JEDEC releases JC43 > tomorrow and we add support... you code forces the name to "jc42" even > if the OF name was something other than "jc-42.4-temp"). > The driver should really list all supported devices (not the standard). It could/should have a generic property name to match, but I have no idea what that should be. There are existing compatible properties named "jedec,ufs-1.1" and similar, so maybe the current compatible string isn't as bad as I thought. Still, we need to do something. I am open to suggestions. Guenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hwmon: (jc42) Fix name to have no illegal characters 2020-04-17 9:55 ` Jean Delvare 2020-04-17 10:32 ` Sascha Hauer @ 2020-04-18 3:14 ` Guenter Roeck 1 sibling, 0 replies; 7+ messages in thread From: Guenter Roeck @ 2020-04-18 3:14 UTC (permalink / raw) To: Jean Delvare, Sascha Hauer; +Cc: linux-hwmon On 4/17/20 2:55 AM, Jean Delvare wrote: > Hi Sascha, > > On Fri, 17 Apr 2020 11:28:53 +0200, Sascha Hauer wrote: >> The jc42 driver passes I2C client's name as hwmon device name. In case >> of device tree probed devices this ends up being part of the compatible >> string, "jc-42.4-temp". This name contains hyphens and the hwmon core >> doesn't like this: >> >> jc42 2-0018: hwmon: 'jc-42.4-temp' is not a valid name attribute, please fix >> >> This changes the name to "jc42" which doesn't have any illegal >> characters. > > I don't think "jc-42.4-temp" is a valid i2c client name either. I > believe this should be fixed at the of->i2c level, rather than the > i2c->hwmon level. Not sure how other drivers are dealing with that, it > seems that in most cases the name part of the compatible string matches > exactly the expected client name so no conversion is needed. > The problem here is that the compatible string is simply wrong. It precedes the time when I understood devicetree properties well enough to understand what I was doing. Oh well. No idea what to do at this point other than accepting this patch. Guenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hwmon: (jc42) Fix name to have no illegal characters 2020-04-17 9:28 [PATCH] hwmon: (jc42) Fix name to have no illegal characters Sascha Hauer 2020-04-17 9:55 ` Jean Delvare @ 2020-04-18 17:13 ` Guenter Roeck 1 sibling, 0 replies; 7+ messages in thread From: Guenter Roeck @ 2020-04-18 17:13 UTC (permalink / raw) To: Sascha Hauer; +Cc: linux-hwmon, Jean Delvare On Fri, Apr 17, 2020 at 11:28:53AM +0200, Sascha Hauer wrote: > The jc42 driver passes I2C client's name as hwmon device name. In case > of device tree probed devices this ends up being part of the compatible > string, "jc-42.4-temp". This name contains hyphens and the hwmon core > doesn't like this: > > jc42 2-0018: hwmon: 'jc-42.4-temp' is not a valid name attribute, please fix > > This changes the name to "jc42" which doesn't have any illegal > characters. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> I have not heard a better suggestion, so applied. Thanks, Guenter > --- > drivers/hwmon/jc42.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c > index f2d81b0558e5..e3f1ebee7130 100644 > --- a/drivers/hwmon/jc42.c > +++ b/drivers/hwmon/jc42.c > @@ -506,7 +506,7 @@ static int jc42_probe(struct i2c_client *client, const struct i2c_device_id *id) > } > data->config = config; > > - hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, > + hwmon_dev = devm_hwmon_device_register_with_info(dev, "jc42", > data, &jc42_chip_info, > NULL); > return PTR_ERR_OR_ZERO(hwmon_dev); ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-18 17:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-17 9:28 [PATCH] hwmon: (jc42) Fix name to have no illegal characters Sascha Hauer 2020-04-17 9:55 ` Jean Delvare 2020-04-17 10:32 ` Sascha Hauer 2020-04-17 13:47 ` Jean Delvare 2020-04-18 3:19 ` Guenter Roeck 2020-04-18 3:14 ` Guenter Roeck 2020-04-18 17:13 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).