[lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

In the preparation of creating an mfd driver and then refactor this one into a
platform driver in order to add some pinctrl functionality to the chip, it is
necessary to start the series with this change so that the mfd driver can refer
to the proper name in the subsequent change without making changes in more than
one driver later.

This was a request from Lee Jones, the MFD subsystem maintainer.

Signed-off-by: Laszlo Papp <lpapp@kde.org>
---
 drivers/hwmon/max6650.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
index 0cafc39..3c36edc 100644
--- a/drivers/hwmon/max6650.c
+++ b/drivers/hwmon/max6650.c
@@ -116,8 +116,8 @@ static struct max6650_data *max6650_update_device(struct device *dev);
  */
 
 static const struct i2c_device_id max6650_id[] = {
-	{ "max6650", 1 },
-	{ "max6651", 4 },
+	{ "max6650-hwmon", 1 },
+	{ "max6651-hwmon", 4 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, max6650_id);
-- 
1.8.5.3


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

[PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

In the preparation of creating an mfd driver and then refactor this one into a
platform driver in order to add some pinctrl functionality to the chip, it is
necessary to start the series with this change so that the mfd driver can refer
to the proper name in the subsequent change without making changes in more than
one driver later.

This was a request from Lee Jones, the MFD subsystem maintainer.

Signed-off-by: Laszlo Papp <lpapp@kde.org>
---
 drivers/hwmon/max6650.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
index 0cafc39..3c36edc 100644
--- a/drivers/hwmon/max6650.c
+++ b/drivers/hwmon/max6650.c
@@ -116,8 +116,8 @@ static struct max6650_data *max6650_update_device(struct device *dev);
  */
 
 static const struct i2c_device_id max6650_id[] = {
-	{ "max6650", 1 },
-	{ "max6651", 4 },
+	{ "max6650-hwmon", 1 },
+	{ "max6651-hwmon", 4 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, max6650_id);
-- 
1.8.5.3

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

PiBJbiB0aGUgcHJlcGFyYXRpb24gb2YgY3JlYXRpbmcgYW4gbWZkIGRyaXZlciBhbmQgdGhlbiBy
ZWZhY3RvciB0aGlzIG9uZSBpbnRvIGEKPiBwbGF0Zm9ybSBkcml2ZXIgaW4gb3JkZXIgdG8gYWRk
IHNvbWUgcGluY3RybCBmdW5jdGlvbmFsaXR5IHRvIHRoZSBjaGlwLCBpdCBpcwo+IG5lY2Vzc2Fy
eSB0byBzdGFydCB0aGUgc2VyaWVzIHdpdGggdGhpcyBjaGFuZ2Ugc28gdGhhdCB0aGUgbWZkIGRy
aXZlciBjYW4gcmVmZXIKPiB0byB0aGUgcHJvcGVyIG5hbWUgaW4gdGhlIHN1YnNlcXVlbnQgY2hh
bmdlIHdpdGhvdXQgbWFraW5nIGNoYW5nZXMgaW4gbW9yZSB0aGFuCj4gb25lIGRyaXZlciBsYXRl
ci4KPiAKPiBUaGlzIHdhcyBhIHJlcXVlc3QgZnJvbSBMZWUgSm9uZXMsIHRoZSBNRkQgc3Vic3lz
dGVtIG1haW50YWluZXIuCj4gCj4gU2lnbmVkLW9mZi1ieTogTGFzemxvIFBhcHAgPGxwYXBwQGtk
ZS5vcmc+Cj4gLS0tCj4gIGRyaXZlcnMvaHdtb24vbWF4NjY1MC5jIHwgNCArKy0tCj4gIDEgZmls
ZSBjaGFuZ2VkLCAyIGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pCj4gCj4gZGlmZiAtLWdp
dCBhL2RyaXZlcnMvaHdtb24vbWF4NjY1MC5jIGIvZHJpdmVycy9od21vbi9tYXg2NjUwLmMKPiBp
bmRleCAwY2FmYzM5Li4zYzM2ZWRjIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvaHdtb24vbWF4NjY1
MC5jCj4gKysrIGIvZHJpdmVycy9od21vbi9tYXg2NjUwLmMKPiBAQCAtMTE2LDggKzExNiw4IEBA
IHN0YXRpYyBzdHJ1Y3QgbWF4NjY1MF9kYXRhICptYXg2NjUwX3VwZGF0ZV9kZXZpY2Uoc3RydWN0
IGRldmljZSAqZGV2KTsKPiAgICovCj4gIAo+ICBzdGF0aWMgY29uc3Qgc3RydWN0IGkyY19kZXZp
Y2VfaWQgbWF4NjY1MF9pZFtdID0gewo+IC0JeyAibWF4NjY1MCIsIDEgfSwKPiAtCXsgIm1heDY2
NTEiLCA0IH0sCj4gKwl7ICJtYXg2NjUwLWh3bW9uIiwgMSB9LAo+ICsJeyAibWF4NjY1MS1od21v
biIsIDQgfSwKCk1pZ2h0IGJlIHdvcnRoIHRha2luZyB0aGUgb3Bwb3J0dW5pdHkgdG8gc3dhcCBv
dXQgdGhlc2UgbWFnaWMgbnVtYmVycwpub3cuCgo+ICAJeyB9Cj4gIH07Cj4gIE1PRFVMRV9ERVZJ
Q0VfVEFCTEUoaTJjLCBtYXg2NjUwX2lkKTsKCi0tIApMZWUgSm9uZXMKTGluYXJvIFNUTWljcm9l
bGVjdHJvbmljcyBMYW5kaW5nIFRlYW0gTGVhZApMaW5hcm8ub3JnIOKUgiBPcGVuIHNvdXJjZSBz
b2Z0d2FyZSBmb3IgQVJNIFNvQ3MKRm9sbG93IExpbmFybzogRmFjZWJvb2sgfCBUd2l0dGVyIHwg
QmxvZwoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbG0t
c2Vuc29ycyBtYWlsaW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1zZW5zb3JzLm9yZwpodHRwOi8vbGlz
dHMubG0tc2Vuc29ycy5vcmcvbWFpbG1hbi9saXN0aW5mby9sbS1zZW5zb3Jz

Re: [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

> In the preparation of creating an mfd driver and then refactor this one into a
> platform driver in order to add some pinctrl functionality to the chip, it is
> necessary to start the series with this change so that the mfd driver can refer
> to the proper name in the subsequent change without making changes in more than
> one driver later.
> 
> This was a request from Lee Jones, the MFD subsystem maintainer.
> 
> Signed-off-by: Laszlo Papp <lpapp@kde.org>
> ---
>  drivers/hwmon/max6650.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
> index 0cafc39..3c36edc 100644
> --- a/drivers/hwmon/max6650.c
> +++ b/drivers/hwmon/max6650.c
> @@ -116,8 +116,8 @@ static struct max6650_data *max6650_update_device(struct device *dev);
>   */
>  
>  static const struct i2c_device_id max6650_id[] = {
> -	{ "max6650", 1 },
> -	{ "max6651", 4 },
> +	{ "max6650-hwmon", 1 },
> +	{ "max6651-hwmon", 4 },

Might be worth taking the opportunity to swap out these magic numbers
now.

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, max6650_id);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

Hi Lee, Laszlo,

On Mon, 10 Feb 2014 16:08:42 +0000, Lee Jones wrote:
> > In the preparation of creating an mfd driver and then refactor this one into a
> > platform driver in order to add some pinctrl functionality to the chip, it is
> > necessary to start the series with this change so that the mfd driver can refer
> > to the proper name in the subsequent change without making changes in more than
> > one driver later.
> > 
> > This was a request from Lee Jones, the MFD subsystem maintainer.
> > 
> > Signed-off-by: Laszlo Papp <lpapp@kde.org>
> > ---
> >  drivers/hwmon/max6650.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
> > index 0cafc39..3c36edc 100644
> > --- a/drivers/hwmon/max6650.c
> > +++ b/drivers/hwmon/max6650.c
> > @@ -116,8 +116,8 @@ static struct max6650_data *max6650_update_device(struct device *dev);
> >   */
> >  
> >  static const struct i2c_device_id max6650_id[] = {
> > -	{ "max6650", 1 },
> > -	{ "max6651", 4 },
> > +	{ "max6650-hwmon", 1 },
> > +	{ "max6651-hwmon", 4 },

No, this is not acceptable, sorry. This will change the name of the
hwmon device as seen from user-space, breaking any configuration file
referring to it. Additionally, dashes are explicitly forbidden in hwmon
device names. And lastly this will break any explicit instantiation of
theses devices (which is the only way, as the driver doesn't support
device auto-detection), be it in the kernel itself or from user-space.

The change doesn't make sense anyway. If you move to the MFD framework,
the core driver will be an I2C driver binding to the I2C device, and it
will spawn the logical devices, presumably in the form of platform
devices. That's what the current max6650 driver would have to bind to.
Just renaming the device won't work, you also need to change the type.

If you want to turn this into an MFD driver, I believe you must first
convert the hwmon part to register using
devm_hwmon_device_register_with_groups(). This will dissociate the i2c
device name from the hwmon device name and create a clean name-space
for each function. Guenter, maybe you had a plan to do so already
anyway?

That being said, going with MFD in this case seems quite overkill to
me. MFD makes a lot of sense when each function has its own resources.
As this isn't the case here, a single driver registering both an hwmon
interface and a pinctrl interface would seem sufficient to me. But I
think Guenter already discussed this in the past so I'll let him
continue and decide.

> Might be worth taking the opportunity to swap out these magic numbers
> now.

There's nothing magic about them, they tell the driver how many fans
each device supports. If you don't pass them as driver_data you'll have
to derive them from the device name in the probe function.

> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(i2c, max6650_id);
> 


-- 
Jean Delvare
Suse L3 Support

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

Hi Lee, Laszlo,

On Mon, 10 Feb 2014 16:08:42 +0000, Lee Jones wrote:
> > In the preparation of creating an mfd driver and then refactor this one into a
> > platform driver in order to add some pinctrl functionality to the chip, it is
> > necessary to start the series with this change so that the mfd driver can refer
> > to the proper name in the subsequent change without making changes in more than
> > one driver later.
> > 
> > This was a request from Lee Jones, the MFD subsystem maintainer.
> > 
> > Signed-off-by: Laszlo Papp <lpapp@kde.org>
> > ---
> >  drivers/hwmon/max6650.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
> > index 0cafc39..3c36edc 100644
> > --- a/drivers/hwmon/max6650.c
> > +++ b/drivers/hwmon/max6650.c
> > @@ -116,8 +116,8 @@ static struct max6650_data *max6650_update_device(struct device *dev);
> >   */
> >  
> >  static const struct i2c_device_id max6650_id[] = {
> > -	{ "max6650", 1 },
> > -	{ "max6651", 4 },
> > +	{ "max6650-hwmon", 1 },
> > +	{ "max6651-hwmon", 4 },

No, this is not acceptable, sorry. This will change the name of the
hwmon device as seen from user-space, breaking any configuration file
referring to it. Additionally, dashes are explicitly forbidden in hwmon
device names. And lastly this will break any explicit instantiation of
theses devices (which is the only way, as the driver doesn't support
device auto-detection), be it in the kernel itself or from user-space.

The change doesn't make sense anyway. If you move to the MFD framework,
the core driver will be an I2C driver binding to the I2C device, and it
will spawn the logical devices, presumably in the form of platform
devices. That's what the current max6650 driver would have to bind to.
Just renaming the device won't work, you also need to change the type.

If you want to turn this into an MFD driver, I believe you must first
convert the hwmon part to register using
devm_hwmon_device_register_with_groups(). This will dissociate the i2c
device name from the hwmon device name and create a clean name-space
for each function. Guenter, maybe you had a plan to do so already
anyway?

That being said, going with MFD in this case seems quite overkill to
me. MFD makes a lot of sense when each function has its own resources.
As this isn't the case here, a single driver registering both an hwmon
interface and a pinctrl interface would seem sufficient to me. But I
think Guenter already discussed this in the past so I'll let him
continue and decide.

> Might be worth taking the opportunity to swap out these magic numbers
> now.

There's nothing magic about them, they tell the driver how many fans
each device supports. If you don't pass them as driver_data you'll have
to derive them from the device name in the probe function.

> >  	{ }
> >  };
> >  MODULE_DEVICE_TABLE(i2c, max6650_id);
> 


-- 
Jean Delvare
Suse L3 Support

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[linux]

Quoting Jean Delvare <jdelvare@suse.de>:

>
> That being said, going with MFD in this case seems quite overkill to
> me. MFD makes a lot of sense when each function has its own resources.
> As this isn't the case here, a single driver registering both an hwmon
> interface and a pinctrl interface would seem sufficient to me. But I
> think Guenter already discussed this in the past so I'll let him
> continue and decide.
>

That is what I had suggested as well (though we were talking gpio
at the time). Laszlo didn't want to do it this way for some reason.
Right now I don't really have an idea what to do.

Guenter


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[linux]

Quoting Jean Delvare <jdelvare@suse.de>:

>
> That being said, going with MFD in this case seems quite overkill to
> me. MFD makes a lot of sense when each function has its own resources.
> As this isn't the case here, a single driver registering both an hwmon
> interface and a pinctrl interface would seem sufficient to me. But I
> think Guenter already discussed this in the past so I'll let him
> continue and decide.
>

That is what I had suggested as well (though we were talking gpio
at the time). Laszlo didn't want to do it this way for some reason.
Right now I don't really have an idea what to do.

Guenter

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 4:53 PM,  <linux@roeck-us.net> wrote:
> Quoting Jean Delvare <jdelvare@suse.de>:
>
>>
>> That being said, going with MFD in this case seems quite overkill to
>> me. MFD makes a lot of sense when each function has its own resources.
>> As this isn't the case here, a single driver registering both an hwmon
>> interface and a pinctrl interface would seem sufficient to me. But I
>> think Guenter already discussed this in the past so I'll let him
>> continue and decide.
>>
>
> That is what I had suggested as well (though we were talking gpio
> at the time). Laszlo didn't want to do it this way for some reason.
> Right now I don't really have an idea what to do.

Right now I do not really have an idea what the concern here is.

I will quote you:

"Please explain, for my education, what makes you believe that I would
object to or reject to anyone submitting such a driver."

and then the next one in the thread:

"> > Works for me. Should I apply the gpio and mfd drivers separately or in
> > one single patch?
>
> s/apply/send/
>
Separately."

This happened about two months ago, and after two months of man work,
several reviews from various people, while you have been *explicitly*
included in the threads, are claiming that it is unacceptable? Do you
see how much time waste that would be for everyone who have been
involved.

What I currently do not understand is the point for rejecting the
contribution that does not have API drawback, etc, if you do not
provide anything better. You are more than welcome to rewrite my work
once the feature works, but I guess it is very likely that you would
not do that.

So, let me ask it: shall we continue the bike-shedding after months,
or there is a definite decision from the maintainers? Disagreement is
not a problem because people can move on if the maintainers actually
make it clear what is acceptable and what not, but here that did not
really happen. We are where we were months ago.

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 4:53 PM,  <linux@roeck-us.net> wrote:
> Quoting Jean Delvare <jdelvare@suse.de>:
>
>>
>> That being said, going with MFD in this case seems quite overkill to
>> me. MFD makes a lot of sense when each function has its own resources.
>> As this isn't the case here, a single driver registering both an hwmon
>> interface and a pinctrl interface would seem sufficient to me. But I
>> think Guenter already discussed this in the past so I'll let him
>> continue and decide.
>>
>
> That is what I had suggested as well (though we were talking gpio
> at the time). Laszlo didn't want to do it this way for some reason.
> Right now I don't really have an idea what to do.

Right now I do not really have an idea what the concern here is.

I will quote you:

"Please explain, for my education, what makes you believe that I would
object to or reject to anyone submitting such a driver."

and then the next one in the thread:

"> > Works for me. Should I apply the gpio and mfd drivers separately or in
> > one single patch?
>
> s/apply/send/
>
Separately."

This happened about two months ago, and after two months of man work,
several reviews from various people, while you have been *explicitly*
included in the threads, are claiming that it is unacceptable? Do you
see how much time waste that would be for everyone who have been
involved.

What I currently do not understand is the point for rejecting the
contribution that does not have API drawback, etc, if you do not
provide anything better. You are more than welcome to rewrite my work
once the feature works, but I guess it is very likely that you would
not do that.

So, let me ask it: shall we continue the bike-shedding after months,
or there is a definite decision from the maintainers? Disagreement is
not a problem because people can move on if the maintainers actually
make it clear what is acceptable and what not, but here that did not
really happen. We are where we were months ago.

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Guenter Roeck]

On Mon, Feb 10, 2014 at 06:59:55PM +0000, Laszlo Papp wrote:
> On Mon, Feb 10, 2014 at 4:53 PM,  <linux@roeck-us.net> wrote:
> > Quoting Jean Delvare <jdelvare@suse.de>:
> >
> >>
> >> That being said, going with MFD in this case seems quite overkill to
> >> me. MFD makes a lot of sense when each function has its own resources.
> >> As this isn't the case here, a single driver registering both an hwmon
> >> interface and a pinctrl interface would seem sufficient to me. But I
> >> think Guenter already discussed this in the past so I'll let him
> >> continue and decide.
> >>
> >
> > That is what I had suggested as well (though we were talking gpio
> > at the time). Laszlo didn't want to do it this way for some reason.
> > Right now I don't really have an idea what to do.
> 
> Right now I do not really have an idea what the concern here is.
> 
> I will quote you:
> 
> "Please explain, for my education, what makes you believe that I would
> object to or reject to anyone submitting such a driver."
> 
> and then the next one in the thread:
> 
> "> > Works for me. Should I apply the gpio and mfd drivers separately or in
> > > one single patch?
> >
> > s/apply/send/
> >
> Separately."
> 
> This happened about two months ago, and after two months of man work,
> several reviews from various people, while you have been *explicitly*
> included in the threads, are claiming that it is unacceptable? Do you
> see how much time waste that would be for everyone who have been
> involved.
> 
> What I currently do not understand is the point for rejecting the
> contribution that does not have API drawback, etc, if you do not
> provide anything better. You are more than welcome to rewrite my work
> once the feature works, but I guess it is very likely that you would
> not do that.
> 
> So, let me ask it: shall we continue the bike-shedding after months,
> or there is a definite decision from the maintainers? Disagreement is
> not a problem because people can move on if the maintainers actually
> make it clear what is acceptable and what not, but here that did not
> really happen. We are where we were months ago.
> 

I think I'll let Jean handle this one.

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Guenter Roeck]

On Mon, Feb 10, 2014 at 06:59:55PM +0000, Laszlo Papp wrote:
> On Mon, Feb 10, 2014 at 4:53 PM,  <linux@roeck-us.net> wrote:
> > Quoting Jean Delvare <jdelvare@suse.de>:
> >
> >>
> >> That being said, going with MFD in this case seems quite overkill to
> >> me. MFD makes a lot of sense when each function has its own resources.
> >> As this isn't the case here, a single driver registering both an hwmon
> >> interface and a pinctrl interface would seem sufficient to me. But I
> >> think Guenter already discussed this in the past so I'll let him
> >> continue and decide.
> >>
> >
> > That is what I had suggested as well (though we were talking gpio
> > at the time). Laszlo didn't want to do it this way for some reason.
> > Right now I don't really have an idea what to do.
> 
> Right now I do not really have an idea what the concern here is.
> 
> I will quote you:
> 
> "Please explain, for my education, what makes you believe that I would
> object to or reject to anyone submitting such a driver."
> 
> and then the next one in the thread:
> 
> "> > Works for me. Should I apply the gpio and mfd drivers separately or in
> > > one single patch?
> >
> > s/apply/send/
> >
> Separately."
> 
> This happened about two months ago, and after two months of man work,
> several reviews from various people, while you have been *explicitly*
> included in the threads, are claiming that it is unacceptable? Do you
> see how much time waste that would be for everyone who have been
> involved.
> 
> What I currently do not understand is the point for rejecting the
> contribution that does not have API drawback, etc, if you do not
> provide anything better. You are more than welcome to rewrite my work
> once the feature works, but I guess it is very likely that you would
> not do that.
> 
> So, let me ask it: shall we continue the bike-shedding after months,
> or there is a definite decision from the maintainers? Disagreement is
> not a problem because people can move on if the maintainers actually
> make it clear what is acceptable and what not, but here that did not
> really happen. We are where we were months ago.
> 

I think I'll let Jean handle this one.

Guenter

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 11:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Mon, Feb 10, 2014 at 06:59:55PM +0000, Laszlo Papp wrote:
> I think I'll let Jean handle this one.

Guys, please be a bit more definite.

We should get over this long ping-pong game. It has been clearly
stated that either way is fine, and there was no objection for months
to either way either, and now the feature is just not moving forward.
This is the first time I have this sorrow experience that I am having
here in hwmon, unfortunately. A lot of effort spent, and nothing got
done.

As I said before, disagreeing is fine and natural. What is not fine is
wasting people's time for months and not making it clear what the
hwmon maintainers want, and months later after the work, the opposite
is claimed than before.

I am sorry if it sounds harsh, but currently this is how I see the
situation. If the MFD solution gets rejected, I consider it a huge
maintainer mistake since both of you were involved, and have never
ever spoken up for months that this would not be acceptable. In fact,
as quoted earlier in this thread, the opposite had been said.

I would like to get over the hwmon situation as soon as possible, so
please guys kindly advise what you would *really* like to see. I do
not really mind who makes the decision, but rejecting months of work
due to miscommunication is still better than continuing the same!

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 11:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Mon, Feb 10, 2014 at 06:59:55PM +0000, Laszlo Papp wrote:
> I think I'll let Jean handle this one.

Guys, please be a bit more definite.

We should get over this long ping-pong game. It has been clearly
stated that either way is fine, and there was no objection for months
to either way either, and now the feature is just not moving forward.
This is the first time I have this sorrow experience that I am having
here in hwmon, unfortunately. A lot of effort spent, and nothing got
done.

As I said before, disagreeing is fine and natural. What is not fine is
wasting people's time for months and not making it clear what the
hwmon maintainers want, and months later after the work, the opposite
is claimed than before.

I am sorry if it sounds harsh, but currently this is how I see the
situation. If the MFD solution gets rejected, I consider it a huge
maintainer mistake since both of you were involved, and have never
ever spoken up for months that this would not be acceptable. In fact,
as quoted earlier in this thread, the opposite had been said.

I would like to get over the hwmon situation as soon as possible, so
please guys kindly advise what you would *really* like to see. I do
not really mind who makes the decision, but rejecting months of work
due to miscommunication is still better than continuing the same!

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 3:23 AM, Laszlo Papp <lpapp@kde.org> wrote:
> On Mon, Feb 10, 2014 at 11:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On Mon, Feb 10, 2014 at 06:59:55PM +0000, Laszlo Papp wrote:
>> I think I'll let Jean handle this one.
>
> Guys, please be a bit more definite.
>
> We should get over this long ping-pong game. It has been clearly
> stated that either way is fine, and there was no objection for months
> to either way either, and now the feature is just not moving forward.
> This is the first time I have this sorrow experience that I am having
> here in hwmon, unfortunately. A lot of effort spent, and nothing got
> done.
>
> As I said before, disagreeing is fine and natural. What is not fine is
> wasting people's time for months and not making it clear what the
> hwmon maintainers want, and months later after the work, the opposite
> is claimed than before.
>
> I am sorry if it sounds harsh, but currently this is how I see the
> situation. If the MFD solution gets rejected, I consider it a huge
> maintainer mistake since both of you were involved, and have never
> ever spoken up for months that this would not be acceptable. In fact,
> as quoted earlier in this thread, the opposite had been said.
>
> I would like to get over the hwmon situation as soon as possible, so
> please guys kindly advise what you would *really* like to see. I do
> not really mind who makes the decision, but rejecting months of work
> due to miscommunication is still better than continuing the same!

(Alternatively, who is the higher-level decision maker over the
drivers (and hence driver subsystem maintainers) to ask for making a
final decision if you cannot make it?
I would hope for this being the last resort, but there is a point
where it is necessary to remain productive, in my opinion.)

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 3:23 AM, Laszlo Papp <lpapp@kde.org> wrote:
> On Mon, Feb 10, 2014 at 11:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On Mon, Feb 10, 2014 at 06:59:55PM +0000, Laszlo Papp wrote:
>> I think I'll let Jean handle this one.
>
> Guys, please be a bit more definite.
>
> We should get over this long ping-pong game. It has been clearly
> stated that either way is fine, and there was no objection for months
> to either way either, and now the feature is just not moving forward.
> This is the first time I have this sorrow experience that I am having
> here in hwmon, unfortunately. A lot of effort spent, and nothing got
> done.
>
> As I said before, disagreeing is fine and natural. What is not fine is
> wasting people's time for months and not making it clear what the
> hwmon maintainers want, and months later after the work, the opposite
> is claimed than before.
>
> I am sorry if it sounds harsh, but currently this is how I see the
> situation. If the MFD solution gets rejected, I consider it a huge
> maintainer mistake since both of you were involved, and have never
> ever spoken up for months that this would not be acceptable. In fact,
> as quoted earlier in this thread, the opposite had been said.
>
> I would like to get over the hwmon situation as soon as possible, so
> please guys kindly advise what you would *really* like to see. I do
> not really mind who makes the decision, but rejecting months of work
> due to miscommunication is still better than continuing the same!

(Alternatively, who is the higher-level decision maker over the
drivers (and hence driver subsystem maintainers) to ask for making a
final decision if you cannot make it?
I would hope for this being the last resort, but there is a point
where it is necessary to remain productive, in my opinion.)

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

PiA+ID4gIHN0YXRpYyBjb25zdCBzdHJ1Y3QgaTJjX2RldmljZV9pZCBtYXg2NjUwX2lkW10gPSB7
Cj4gPiA+IC0JeyAibWF4NjY1MCIsIDEgfSwKPiA+ID4gLQl7ICJtYXg2NjUxIiwgNCB9LAo+ID4g
PiArCXsgIm1heDY2NTAtaHdtb24iLCAxIH0sCj4gPiA+ICsJeyAibWF4NjY1MS1od21vbiIsIDQg
fSwKPiAKPiBObywgdGhpcyBpcyBub3QgYWNjZXB0YWJsZSwgc29ycnkuIFRoaXMgd2lsbCBjaGFu
Z2UgdGhlIG5hbWUgb2YgdGhlCj4gaHdtb24gZGV2aWNlIGFzIHNlZW4gZnJvbSB1c2VyLXNwYWNl
LCBicmVha2luZyBhbnkgY29uZmlndXJhdGlvbiBmaWxlCj4gcmVmZXJyaW5nIHRvIGl0LiBBZGRp
dGlvbmFsbHksIGRhc2hlcyBhcmUgZXhwbGljaXRseSBmb3JiaWRkZW4gaW4gaHdtb24KPiBkZXZp
Y2UgbmFtZXMuIEFuZCBsYXN0bHkgdGhpcyB3aWxsIGJyZWFrIGFueSBleHBsaWNpdCBpbnN0YW50
aWF0aW9uIG9mCj4gdGhlc2VzIGRldmljZXMgKHdoaWNoIGlzIHRoZSBvbmx5IHdheSwgYXMgdGhl
IGRyaXZlciBkb2Vzbid0IHN1cHBvcnQKPiBkZXZpY2UgYXV0by1kZXRlY3Rpb24pLCBiZSBpdCBp
biB0aGUga2VybmVsIGl0c2VsZiBvciBmcm9tIHVzZXItc3BhY2UuCj4gCj4gVGhlIGNoYW5nZSBk
b2Vzbid0IG1ha2Ugc2Vuc2UgYW55d2F5LiBJZiB5b3UgbW92ZSB0byB0aGUgTUZEIGZyYW1ld29y
aywKPiB0aGUgY29yZSBkcml2ZXIgd2lsbCBiZSBhbiBJMkMgZHJpdmVyIGJpbmRpbmcgdG8gdGhl
IEkyQyBkZXZpY2UsIGFuZCBpdAo+IHdpbGwgc3Bhd24gdGhlIGxvZ2ljYWwgZGV2aWNlcywgcHJl
c3VtYWJseSBpbiB0aGUgZm9ybSBvZiBwbGF0Zm9ybQo+IGRldmljZXMuIFRoYXQncyB3aGF0IHRo
ZSBjdXJyZW50IG1heDY2NTAgZHJpdmVyIHdvdWxkIGhhdmUgdG8gYmluZCB0by4KPiBKdXN0IHJl
bmFtaW5nIHRoZSBkZXZpY2Ugd29uJ3Qgd29yaywgeW91IGFsc28gbmVlZCB0byBjaGFuZ2UgdGhl
IHR5cGUuCj4gCj4gSWYgeW91IHdhbnQgdG8gdHVybiB0aGlzIGludG8gYW4gTUZEIGRyaXZlciwg
SSBiZWxpZXZlIHlvdSBtdXN0IGZpcnN0Cj4gY29udmVydCB0aGUgaHdtb24gcGFydCB0byByZWdp
c3RlciB1c2luZwo+IGRldm1faHdtb25fZGV2aWNlX3JlZ2lzdGVyX3dpdGhfZ3JvdXBzKCkuIFRo
aXMgd2lsbCBkaXNzb2NpYXRlIHRoZSBpMmMKPiBkZXZpY2UgbmFtZSBmcm9tIHRoZSBod21vbiBk
ZXZpY2UgbmFtZSBhbmQgY3JlYXRlIGEgY2xlYW4gbmFtZS1zcGFjZQo+IGZvciBlYWNoIGZ1bmN0
aW9uLiBHdWVudGVyLCBtYXliZSB5b3UgaGFkIGEgcGxhbiB0byBkbyBzbyBhbHJlYWR5Cj4gYW55
d2F5Pwo+IAo+IFRoYXQgYmVpbmcgc2FpZCwgZ29pbmcgd2l0aCBNRkQgaW4gdGhpcyBjYXNlIHNl
ZW1zIHF1aXRlIG92ZXJraWxsIHRvCj4gbWUuIE1GRCBtYWtlcyBhIGxvdCBvZiBzZW5zZSB3aGVu
IGVhY2ggZnVuY3Rpb24gaGFzIGl0cyBvd24gcmVzb3VyY2VzLgo+IEFzIHRoaXMgaXNuJ3QgdGhl
IGNhc2UgaGVyZSwgYSBzaW5nbGUgZHJpdmVyIHJlZ2lzdGVyaW5nIGJvdGggYW4gaHdtb24KPiBp
bnRlcmZhY2UgYW5kIGEgcGluY3RybCBpbnRlcmZhY2Ugd291bGQgc2VlbSBzdWZmaWNpZW50IHRv
IG1lLiBCdXQgSQo+IHRoaW5rIEd1ZW50ZXIgYWxyZWFkeSBkaXNjdXNzZWQgdGhpcyBpbiB0aGUg
cGFzdCBzbyBJJ2xsIGxldCBoaW0KPiBjb250aW51ZSBhbmQgZGVjaWRlLgoKSSdsbCBnZXQgeW91
IGd1eXMgZGVjaWRlIGlmIHlvdSB3YW50IHRvIGdvIHRoZSBNRkQgcm91dGUgb3Igbm90LgoKRWl0
aGVyIGlzIG9rYXkgd2l0aCBtZSwgYnV0IGlmIHlvdSBkbyBkZWNpZGUgaW4gZmF2b3VyLCBhIG5h
bWUgY2hhbmdlCndpdGggdGhlIGRldmljZSB0eXBlIGFwcGVuZGVkIHdvdWxkIGJlIHByZWZlcnJl
ZC4gRWxzZSB0aGUgY29yZSBkZXZpY2UKd291bGQgaGF2ZSB0aGUgc2FtZSBuYW1lIGFzIGFsbCBv
ZiBpdHMgY2hpbGRyZW4gd2hpY2ggd291bGQgYmUgcXVpdGUKdW53b3JrYWJsZS4KCj4gPiBNaWdo
dCBiZSB3b3J0aCB0YWtpbmcgdGhlIG9wcG9ydHVuaXR5IHRvIHN3YXAgb3V0IHRoZXNlIG1hZ2lj
IG51bWJlcnMKPiA+IG5vdy4KPiAKPiBUaGVyZSdzIG5vdGhpbmcgbWFnaWMgYWJvdXQgdGhlbSwg
dGhleSB0ZWxsIHRoZSBkcml2ZXIgaG93IG1hbnkgZmFucwo+IGVhY2ggZGV2aWNlIHN1cHBvcnRz
LiBJZiB5b3UgZG9uJ3QgcGFzcyB0aGVtIGFzIGRyaXZlcl9kYXRhIHlvdSdsbCBoYXZlCj4gdG8g
ZGVyaXZlIHRoZW0gZnJvbSB0aGUgZGV2aWNlIG5hbWUgaW4gdGhlIHByb2JlIGZ1bmN0aW9uLgoK
VGhleSdyZSBtYWdpYyBpbiB0aGF0IHRoZXkncmUgbm90IGVhc2lseSBpZGVudGlmaWFibGUuIElu
IHRoZSBmZXcKbW9tZW50cyB0aGF0IEkgbG9va2VkIGF0IHRoZSBwYXRjaCBJIGFzc3VtZWQgdGhl
eSB3ZXJlIGRldmljZQpJRHMuIFRoZXkgc2hvdWxkIGJlIGNsZWFybHkgZGVmaW5lZC4KCj4gPiA+
ICAJeyB9Cj4gPiA+ICB9Owo+ID4gPiAgTU9EVUxFX0RFVklDRV9UQUJMRShpMmMsIG1heDY2NTBf
aWQpOwoKLS0gCkxlZSBKb25lcwpMaW5hcm8gU1RNaWNyb2VsZWN0cm9uaWNzIExhbmRpbmcgVGVh
bSBMZWFkCkxpbmFyby5vcmcg4pSCIE9wZW4gc291cmNlIHNvZnR3YXJlIGZvciBBUk0gU29DcwpG
b2xsb3cgTGluYXJvOiBGYWNlYm9vayB8IFR3aXR0ZXIgfCBCbG9nCgpfX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpsbS1zZW5zb3JzIG1haWxpbmcgbGlzdAps
bS1zZW5zb3JzQGxtLXNlbnNvcnMub3JnCmh0dHA6Ly9saXN0cy5sbS1zZW5zb3JzLm9yZy9tYWls
bWFuL2xpc3RpbmZvL2xtLXNlbnNvcnM

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

> > >  static const struct i2c_device_id max6650_id[] = {
> > > -	{ "max6650", 1 },
> > > -	{ "max6651", 4 },
> > > +	{ "max6650-hwmon", 1 },
> > > +	{ "max6651-hwmon", 4 },
> 
> No, this is not acceptable, sorry. This will change the name of the
> hwmon device as seen from user-space, breaking any configuration file
> referring to it. Additionally, dashes are explicitly forbidden in hwmon
> device names. And lastly this will break any explicit instantiation of
> theses devices (which is the only way, as the driver doesn't support
> device auto-detection), be it in the kernel itself or from user-space.
> 
> The change doesn't make sense anyway. If you move to the MFD framework,
> the core driver will be an I2C driver binding to the I2C device, and it
> will spawn the logical devices, presumably in the form of platform
> devices. That's what the current max6650 driver would have to bind to.
> Just renaming the device won't work, you also need to change the type.
> 
> If you want to turn this into an MFD driver, I believe you must first
> convert the hwmon part to register using
> devm_hwmon_device_register_with_groups(). This will dissociate the i2c
> device name from the hwmon device name and create a clean name-space
> for each function. Guenter, maybe you had a plan to do so already
> anyway?
> 
> That being said, going with MFD in this case seems quite overkill to
> me. MFD makes a lot of sense when each function has its own resources.
> As this isn't the case here, a single driver registering both an hwmon
> interface and a pinctrl interface would seem sufficient to me. But I
> think Guenter already discussed this in the past so I'll let him
> continue and decide.

I'll get you guys decide if you want to go the MFD route or not.

Either is okay with me, but if you do decide in favour, a name change
with the device type appended would be preferred. Else the core device
would have the same name as all of its children which would be quite
unworkable.

> > Might be worth taking the opportunity to swap out these magic numbers
> > now.
> 
> There's nothing magic about them, they tell the driver how many fans
> each device supports. If you don't pass them as driver_data you'll have
> to derive them from the device name in the probe function.

They're magic in that they're not easily identifiable. In the few
moments that I looked at the patch I assumed they were device
IDs. They should be clearly defined.

> > >  	{ }
> > >  };
> > >  MODULE_DEVICE_TABLE(i2c, max6650_id);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

Hi Lee,

On Mon, 10 Feb 2014 16:58:53 +0000, Lee Jones wrote:
> > > >  static const struct i2c_device_id max6650_id[] = {
> > > > -	{ "max6650", 1 },
> > > > -	{ "max6651", 4 },
> > > > +	{ "max6650-hwmon", 1 },
> > > > +	{ "max6651-hwmon", 4 },
> > 
> > No, this is not acceptable, sorry. This will change the name of the
> > hwmon device as seen from user-space, breaking any configuration file
> > referring to it. Additionally, dashes are explicitly forbidden in hwmon
> > device names. And lastly this will break any explicit instantiation of
> > theses devices (which is the only way, as the driver doesn't support
> > device auto-detection), be it in the kernel itself or from user-space.
> > 
> > The change doesn't make sense anyway. If you move to the MFD framework,
> > the core driver will be an I2C driver binding to the I2C device, and it
> > will spawn the logical devices, presumably in the form of platform
> > devices. That's what the current max6650 driver would have to bind to.
> > Just renaming the device won't work, you also need to change the type.
> > 
> > If you want to turn this into an MFD driver, I believe you must first
> > convert the hwmon part to register using
> > devm_hwmon_device_register_with_groups(). This will dissociate the i2c
> > device name from the hwmon device name and create a clean name-space
> > for each function. Guenter, maybe you had a plan to do so already
> > anyway?
> > 
> > That being said, going with MFD in this case seems quite overkill to
> > me. MFD makes a lot of sense when each function has its own resources.
> > As this isn't the case here, a single driver registering both an hwmon
> > interface and a pinctrl interface would seem sufficient to me. But I
> > think Guenter already discussed this in the past so I'll let him
> > continue and decide.
> 
> I'll get you guys decide if you want to go the MFD route or not.
> 
> Either is okay with me, but if you do decide in favour, a name change
> with the device type appended would be preferred. Else the core device
> would have the same name as all of its children which would be quite
> unworkable.

No problem with that. The main (I2C) device should be named max6650 (or
max6651), the subdevices can be named whatever you want, as long as the
hwmon (class) device's name attribute is also "max6650" (or "max6651".)
As stated above, this is required to preserve compatibility with
existing users.

> > > Might be worth taking the opportunity to swap out these magic numbers
> > > now.
> > 
> > There's nothing magic about them, they tell the driver how many fans
> > each device supports. If you don't pass them as driver_data you'll have
> > to derive them from the device name in the probe function.
> 
> They're magic in that they're not easily identifiable. In the few
> moments that I looked at the patch I assumed they were device
> IDs. They should be clearly defined.

They could have been device IDs, some drivers do that, and that would
have been equally fine. driver_data can be anything. Best thing to do
is to document it right above the device id array if you really find it
confusing (I don't.) I don't know what else exactly you had in mind,
but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
strike me as the best coding practice.

-- 
Jean Delvare
Suse L3 Support

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

Hi Lee,

On Mon, 10 Feb 2014 16:58:53 +0000, Lee Jones wrote:
> > > >  static const struct i2c_device_id max6650_id[] = {
> > > > -	{ "max6650", 1 },
> > > > -	{ "max6651", 4 },
> > > > +	{ "max6650-hwmon", 1 },
> > > > +	{ "max6651-hwmon", 4 },
> > 
> > No, this is not acceptable, sorry. This will change the name of the
> > hwmon device as seen from user-space, breaking any configuration file
> > referring to it. Additionally, dashes are explicitly forbidden in hwmon
> > device names. And lastly this will break any explicit instantiation of
> > theses devices (which is the only way, as the driver doesn't support
> > device auto-detection), be it in the kernel itself or from user-space.
> > 
> > The change doesn't make sense anyway. If you move to the MFD framework,
> > the core driver will be an I2C driver binding to the I2C device, and it
> > will spawn the logical devices, presumably in the form of platform
> > devices. That's what the current max6650 driver would have to bind to.
> > Just renaming the device won't work, you also need to change the type.
> > 
> > If you want to turn this into an MFD driver, I believe you must first
> > convert the hwmon part to register using
> > devm_hwmon_device_register_with_groups(). This will dissociate the i2c
> > device name from the hwmon device name and create a clean name-space
> > for each function. Guenter, maybe you had a plan to do so already
> > anyway?
> > 
> > That being said, going with MFD in this case seems quite overkill to
> > me. MFD makes a lot of sense when each function has its own resources.
> > As this isn't the case here, a single driver registering both an hwmon
> > interface and a pinctrl interface would seem sufficient to me. But I
> > think Guenter already discussed this in the past so I'll let him
> > continue and decide.
> 
> I'll get you guys decide if you want to go the MFD route or not.
> 
> Either is okay with me, but if you do decide in favour, a name change
> with the device type appended would be preferred. Else the core device
> would have the same name as all of its children which would be quite
> unworkable.

No problem with that. The main (I2C) device should be named max6650 (or
max6651), the subdevices can be named whatever you want, as long as the
hwmon (class) device's name attribute is also "max6650" (or "max6651".)
As stated above, this is required to preserve compatibility with
existing users.

> > > Might be worth taking the opportunity to swap out these magic numbers
> > > now.
> > 
> > There's nothing magic about them, they tell the driver how many fans
> > each device supports. If you don't pass them as driver_data you'll have
> > to derive them from the device name in the probe function.
> 
> They're magic in that they're not easily identifiable. In the few
> moments that I looked at the patch I assumed they were device
> IDs. They should be clearly defined.

They could have been device IDs, some drivers do that, and that would
have been equally fine. driver_data can be anything. Best thing to do
is to document it right above the device id array if you really find it
confusing (I don't.) I don't know what else exactly you had in mind,
but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
strike me as the best coding practice.

-- 
Jean Delvare
Suse L3 Support

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

PiA+ID4gPiBNaWdodCBiZSB3b3J0aCB0YWtpbmcgdGhlIG9wcG9ydHVuaXR5IHRvIHN3YXAgb3V0
IHRoZXNlIG1hZ2ljIG51bWJlcnMKPiA+ID4gPiBub3cuCj4gPiA+IAo+ID4gPiBUaGVyZSdzIG5v
dGhpbmcgbWFnaWMgYWJvdXQgdGhlbSwgdGhleSB0ZWxsIHRoZSBkcml2ZXIgaG93IG1hbnkgZmFu
cwo+ID4gPiBlYWNoIGRldmljZSBzdXBwb3J0cy4gSWYgeW91IGRvbid0IHBhc3MgdGhlbSBhcyBk
cml2ZXJfZGF0YSB5b3UnbGwgaGF2ZQo+ID4gPiB0byBkZXJpdmUgdGhlbSBmcm9tIHRoZSBkZXZp
Y2UgbmFtZSBpbiB0aGUgcHJvYmUgZnVuY3Rpb24uCj4gPiAKPiA+IFRoZXkncmUgbWFnaWMgaW4g
dGhhdCB0aGV5J3JlIG5vdCBlYXNpbHkgaWRlbnRpZmlhYmxlLiBJbiB0aGUgZmV3Cj4gPiBtb21l
bnRzIHRoYXQgSSBsb29rZWQgYXQgdGhlIHBhdGNoIEkgYXNzdW1lZCB0aGV5IHdlcmUgZGV2aWNl
Cj4gPiBJRHMuIFRoZXkgc2hvdWxkIGJlIGNsZWFybHkgZGVmaW5lZC4KPiAKPiBUaGV5IGNvdWxk
IGhhdmUgYmVlbiBkZXZpY2UgSURzLCBzb21lIGRyaXZlcnMgZG8gdGhhdCwgYW5kIHRoYXQgd291
bGQKPiBoYXZlIGJlZW4gZXF1YWxseSBmaW5lLiBkcml2ZXJfZGF0YSBjYW4gYmUgYW55dGhpbmcu
IEJlc3QgdGhpbmcgdG8gZG8KPiBpcyB0byBkb2N1bWVudCBpdCByaWdodCBhYm92ZSB0aGUgZGV2
aWNlIGlkIGFycmF5IGlmIHlvdSByZWFsbHkgZmluZCBpdAo+IGNvbmZ1c2luZyAoSSBkb24ndC4p
IEkgZG9uJ3Qga25vdyB3aGF0IGVsc2UgZXhhY3RseSB5b3UgaGFkIGluIG1pbmQsCj4gYnV0ICNk
ZWZpbmluZyBGT1VSX0ZBTlMgYXMgNCBhbmQgT05FX0ZBTiBhcyAxIGFuZCB1c2luZyB0aGF0IGRv
ZXNuJ3QKPiBzdHJpa2UgbWUgYXMgdGhlIGJlc3QgY29kaW5nIHByYWN0aWNlLgoKT24gdGhlIGNv
bnRyYXJ5LiBQZXJoYXBzIHRoZSBub21lbmNsYXR1cmUgY2FuIGJlIHdvcmtlZCBvbiBhIGxpdHRs
ZSwKYnV0IGlmIEkgc2F3IHRoZSBhZm9yZW1lbnRpb25lZCBkZWZpbmVzIEkgd291bGQgaGF2ZSBr
bm93biBpbnN0YW50bHkKd2hhdCB3YXMgYmVpbmcgZGVmaW5lZCB3aXRob3V0IHNlYXJjaGluZyBm
b3IgY28tbG9jYXRlZCBjb21tZW50cy4gVGh1cwplbGV2YXRpbmcgdGhlIHJlcXVpcmVtZW50IGZv
ciBtZSB0byBldmVuIG1lbnRpb24gaXQuIEV2ZW4gd2hlbiB3ZQp1c2UgdGhlIC5kYXRhIGVsZW1l
bnQgZm9yIHZlcnkgc2ltcGxlIGluZm9ybWF0aW9uIHN1Y2ggYXMgZGV2aWNlIElEcwp3ZSBkbyBz
byB3aXRoIGEgI2RlZmluZS4KCi0tIApMZWUgSm9uZXMKTGluYXJvIFNUTWljcm9lbGVjdHJvbmlj
cyBMYW5kaW5nIFRlYW0gTGVhZApMaW5hcm8ub3JnIOKUgiBPcGVuIHNvdXJjZSBzb2Z0d2FyZSBm
b3IgQVJNIFNvQ3MKRm9sbG93IExpbmFybzogRmFjZWJvb2sgfCBUd2l0dGVyIHwgQmxvZwoKX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbG0tc2Vuc29ycyBt
YWlsaW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1zZW5zb3JzLm9yZwpodHRwOi8vbGlzdHMubG0tc2Vu
c29ycy5vcmcvbWFpbG1hbi9saXN0aW5mby9sbS1zZW5zb3Jz

Re: [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

> > > > Might be worth taking the opportunity to swap out these magic numbers
> > > > now.
> > > 
> > > There's nothing magic about them, they tell the driver how many fans
> > > each device supports. If you don't pass them as driver_data you'll have
> > > to derive them from the device name in the probe function.
> > 
> > They're magic in that they're not easily identifiable. In the few
> > moments that I looked at the patch I assumed they were device
> > IDs. They should be clearly defined.
> 
> They could have been device IDs, some drivers do that, and that would
> have been equally fine. driver_data can be anything. Best thing to do
> is to document it right above the device id array if you really find it
> confusing (I don't.) I don't know what else exactly you had in mind,
> but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
> strike me as the best coding practice.

On the contrary. Perhaps the nomenclature can be worked on a little,
but if I saw the aforementioned defines I would have known instantly
what was being defined without searching for co-located comments. Thus
elevating the requirement for me to even mention it. Even when we
use the .data element for very simple information such as device IDs
we do so with a #define.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

On Mon, 10 Feb 2014 18:01:59 +0000, Lee Jones wrote:
> > > > > Might be worth taking the opportunity to swap out these magic numbers
> > > > > now.
> > > > 
> > > > There's nothing magic about them, they tell the driver how many fans
> > > > each device supports. If you don't pass them as driver_data you'll have
> > > > to derive them from the device name in the probe function.
> > > 
> > > They're magic in that they're not easily identifiable. In the few
> > > moments that I looked at the patch I assumed they were device
> > > IDs. They should be clearly defined.
> > 
> > They could have been device IDs, some drivers do that, and that would
> > have been equally fine. driver_data can be anything. Best thing to do
> > is to document it right above the device id array if you really find it
> > confusing (I don't.) I don't know what else exactly you had in mind,
> > but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
> > strike me as the best coding practice.
> 
> On the contrary. Perhaps the nomenclature can be worked on a little,
> but if I saw the aforementioned defines I would have known instantly
> what was being defined without searching for co-located comments. Thus
> elevating the requirement for me to even mention it. Even when we
> use the .data element for very simple information such as device IDs
> we do so with a #define.

Right, you have a point here.

I suppose it was deemed unneeded for a ~750 lines driver nobody really
cared about. But if the driver is becoming more complex and popular
then indeed it makes sense to clean it up a little. Starting with
reordering functions to kill forward declarations ^^

-- 
Jean Delvare
Suse L3 Support

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

On Mon, 10 Feb 2014 18:01:59 +0000, Lee Jones wrote:
> > > > > Might be worth taking the opportunity to swap out these magic numbers
> > > > > now.
> > > > 
> > > > There's nothing magic about them, they tell the driver how many fans
> > > > each device supports. If you don't pass them as driver_data you'll have
> > > > to derive them from the device name in the probe function.
> > > 
> > > They're magic in that they're not easily identifiable. In the few
> > > moments that I looked at the patch I assumed they were device
> > > IDs. They should be clearly defined.
> > 
> > They could have been device IDs, some drivers do that, and that would
> > have been equally fine. driver_data can be anything. Best thing to do
> > is to document it right above the device id array if you really find it
> > confusing (I don't.) I don't know what else exactly you had in mind,
> > but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
> > strike me as the best coding practice.
> 
> On the contrary. Perhaps the nomenclature can be worked on a little,
> but if I saw the aforementioned defines I would have known instantly
> what was being defined without searching for co-located comments. Thus
> elevating the requirement for me to even mention it. Even when we
> use the .data element for very simple information such as device IDs
> we do so with a #define.

Right, you have a point here.

I suppose it was deemed unneeded for a ~750 lines driver nobody really
cared about. But if the driver is becoming more complex and popular
then indeed it makes sense to clean it up a little. Starting with
reordering functions to kill forward declarations ^^

-- 
Jean Delvare
Suse L3 Support

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

PiA+ID4gPiA+ID4gTWlnaHQgYmUgd29ydGggdGFraW5nIHRoZSBvcHBvcnR1bml0eSB0byBzd2Fw
IG91dCB0aGVzZSBtYWdpYyBudW1iZXJzCj4gPiA+ID4gPiA+IG5vdy4KPiA+ID4gPiA+IAo+ID4g
PiA+ID4gVGhlcmUncyBub3RoaW5nIG1hZ2ljIGFib3V0IHRoZW0sIHRoZXkgdGVsbCB0aGUgZHJp
dmVyIGhvdyBtYW55IGZhbnMKPiA+ID4gPiA+IGVhY2ggZGV2aWNlIHN1cHBvcnRzLiBJZiB5b3Ug
ZG9uJ3QgcGFzcyB0aGVtIGFzIGRyaXZlcl9kYXRhIHlvdSdsbCBoYXZlCj4gPiA+ID4gPiB0byBk
ZXJpdmUgdGhlbSBmcm9tIHRoZSBkZXZpY2UgbmFtZSBpbiB0aGUgcHJvYmUgZnVuY3Rpb24uCj4g
PiA+ID4gCj4gPiA+ID4gVGhleSdyZSBtYWdpYyBpbiB0aGF0IHRoZXkncmUgbm90IGVhc2lseSBp
ZGVudGlmaWFibGUuIEluIHRoZSBmZXcKPiA+ID4gPiBtb21lbnRzIHRoYXQgSSBsb29rZWQgYXQg
dGhlIHBhdGNoIEkgYXNzdW1lZCB0aGV5IHdlcmUgZGV2aWNlCj4gPiA+ID4gSURzLiBUaGV5IHNo
b3VsZCBiZSBjbGVhcmx5IGRlZmluZWQuCj4gPiA+IAo+ID4gPiBUaGV5IGNvdWxkIGhhdmUgYmVl
biBkZXZpY2UgSURzLCBzb21lIGRyaXZlcnMgZG8gdGhhdCwgYW5kIHRoYXQgd291bGQKPiA+ID4g
aGF2ZSBiZWVuIGVxdWFsbHkgZmluZS4gZHJpdmVyX2RhdGEgY2FuIGJlIGFueXRoaW5nLiBCZXN0
IHRoaW5nIHRvIGRvCj4gPiA+IGlzIHRvIGRvY3VtZW50IGl0IHJpZ2h0IGFib3ZlIHRoZSBkZXZp
Y2UgaWQgYXJyYXkgaWYgeW91IHJlYWxseSBmaW5kIGl0Cj4gPiA+IGNvbmZ1c2luZyAoSSBkb24n
dC4pIEkgZG9uJ3Qga25vdyB3aGF0IGVsc2UgZXhhY3RseSB5b3UgaGFkIGluIG1pbmQsCj4gPiA+
IGJ1dCAjZGVmaW5pbmcgRk9VUl9GQU5TIGFzIDQgYW5kIE9ORV9GQU4gYXMgMSBhbmQgdXNpbmcg
dGhhdCBkb2Vzbid0Cj4gPiA+IHN0cmlrZSBtZSBhcyB0aGUgYmVzdCBjb2RpbmcgcHJhY3RpY2Uu
Cj4gPiAKPiA+IE9uIHRoZSBjb250cmFyeS4gUGVyaGFwcyB0aGUgbm9tZW5jbGF0dXJlIGNhbiBi
ZSB3b3JrZWQgb24gYSBsaXR0bGUsCj4gPiBidXQgaWYgSSBzYXcgdGhlIGFmb3JlbWVudGlvbmVk
IGRlZmluZXMgSSB3b3VsZCBoYXZlIGtub3duIGluc3RhbnRseQo+ID4gd2hhdCB3YXMgYmVpbmcg
ZGVmaW5lZCB3aXRob3V0IHNlYXJjaGluZyBmb3IgY28tbG9jYXRlZCBjb21tZW50cy4gVGh1cwo+
ID4gZWxldmF0aW5nIHRoZSByZXF1aXJlbWVudCBmb3IgbWUgdG8gZXZlbiBtZW50aW9uIGl0LiBF
dmVuIHdoZW4gd2UKPiA+IHVzZSB0aGUgLmRhdGEgZWxlbWVudCBmb3IgdmVyeSBzaW1wbGUgaW5m
b3JtYXRpb24gc3VjaCBhcyBkZXZpY2UgSURzCj4gPiB3ZSBkbyBzbyB3aXRoIGEgI2RlZmluZS4K
PiAKPiBSaWdodCwgeW91IGhhdmUgYSBwb2ludCBoZXJlLgo+IAo+IEkgc3VwcG9zZSBpdCB3YXMg
ZGVlbWVkIHVubmVlZGVkIGZvciBhIH43NTAgbGluZXMgZHJpdmVyIG5vYm9keSByZWFsbHkKPiBj
YXJlZCBhYm91dC4gQnV0IGlmIHRoZSBkcml2ZXIgaXMgYmVjb21pbmcgbW9yZSBjb21wbGV4IGFu
ZCBwb3B1bGFyCj4gdGhlbiBpbmRlZWQgaXQgbWFrZXMgc2Vuc2UgdG8gY2xlYW4gaXQgdXAgYSBs
aXR0bGUuIFN0YXJ0aW5nIHdpdGgKPiByZW9yZGVyaW5nIGZ1bmN0aW9ucyB0byBraWxsIGZvcndh
cmQgZGVjbGFyYXRpb25zIF5eCgpBbm90aGVyIHdvcnRod2hpbGUgZW5kZWF2b3VyLiA6KQoKLS0g
CkxlZSBKb25lcwpMaW5hcm8gU1RNaWNyb2VsZWN0cm9uaWNzIExhbmRpbmcgVGVhbSBMZWFkCkxp
bmFyby5vcmcg4pSCIE9wZW4gc291cmNlIHNvZnR3YXJlIGZvciBBUk0gU29DcwpGb2xsb3cgTGlu
YXJvOiBGYWNlYm9vayB8IFR3aXR0ZXIgfCBCbG9nCgpfX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fXwpsbS1zZW5zb3JzIG1haWxpbmcgbGlzdApsbS1zZW5zb3Jz
QGxtLXNlbnNvcnMub3JnCmh0dHA6Ly9saXN0cy5sbS1zZW5zb3JzLm9yZy9tYWlsbWFuL2xpc3Rp
bmZvL2xtLXNlbnNvcnM

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

> > > > > > Might be worth taking the opportunity to swap out these magic numbers
> > > > > > now.
> > > > > 
> > > > > There's nothing magic about them, they tell the driver how many fans
> > > > > each device supports. If you don't pass them as driver_data you'll have
> > > > > to derive them from the device name in the probe function.
> > > > 
> > > > They're magic in that they're not easily identifiable. In the few
> > > > moments that I looked at the patch I assumed they were device
> > > > IDs. They should be clearly defined.
> > > 
> > > They could have been device IDs, some drivers do that, and that would
> > > have been equally fine. driver_data can be anything. Best thing to do
> > > is to document it right above the device id array if you really find it
> > > confusing (I don't.) I don't know what else exactly you had in mind,
> > > but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
> > > strike me as the best coding practice.
> > 
> > On the contrary. Perhaps the nomenclature can be worked on a little,
> > but if I saw the aforementioned defines I would have known instantly
> > what was being defined without searching for co-located comments. Thus
> > elevating the requirement for me to even mention it. Even when we
> > use the .data element for very simple information such as device IDs
> > we do so with a #define.
> 
> Right, you have a point here.
> 
> I suppose it was deemed unneeded for a ~750 lines driver nobody really
> cared about. But if the driver is becoming more complex and popular
> then indeed it makes sense to clean it up a little. Starting with
> reordering functions to kill forward declarations ^^

Another worthwhile endeavour. :)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 5:43 PM, Jean Delvare <jdelvare@suse.de> wrote:
> Hi Lee,
>
> On Mon, 10 Feb 2014 16:58:53 +0000, Lee Jones wrote:
>> > > >  static const struct i2c_device_id max6650_id[] = {
>> > > > -       { "max6650", 1 },
>> > > > -       { "max6651", 4 },
>> > > > +       { "max6650-hwmon", 1 },
>> > > > +       { "max6651-hwmon", 4 },
>> >
>> > No, this is not acceptable, sorry. This will change the name of the
>> > hwmon device as seen from user-space, breaking any configuration file
>> > referring to it. Additionally, dashes are explicitly forbidden in hwmon
>> > device names. And lastly this will break any explicit instantiation of
>> > theses devices (which is the only way, as the driver doesn't support
>> > device auto-detection), be it in the kernel itself or from user-space.
>> >
>> > The change doesn't make sense anyway. If you move to the MFD framework,
>> > the core driver will be an I2C driver binding to the I2C device, and it
>> > will spawn the logical devices, presumably in the form of platform
>> > devices. That's what the current max6650 driver would have to bind to.
>> > Just renaming the device won't work, you also need to change the type.
>> >
>> > If you want to turn this into an MFD driver, I believe you must first
>> > convert the hwmon part to register using
>> > devm_hwmon_device_register_with_groups(). This will dissociate the i2c
>> > device name from the hwmon device name and create a clean name-space
>> > for each function. Guenter, maybe you had a plan to do so already
>> > anyway?
>> >
>> > That being said, going with MFD in this case seems quite overkill to
>> > me. MFD makes a lot of sense when each function has its own resources.
>> > As this isn't the case here, a single driver registering both an hwmon
>> > interface and a pinctrl interface would seem sufficient to me. But I
>> > think Guenter already discussed this in the past so I'll let him
>> > continue and decide.
>>
>> I'll get you guys decide if you want to go the MFD route or not.
>>
>> Either is okay with me, but if you do decide in favour, a name change
>> with the device type appended would be preferred. Else the core device
>> would have the same name as all of its children which would be quite
>> unworkable.
>
> No problem with that. The main (I2C) device should be named max6650 (or
> max6651), the subdevices can be named whatever you want, as long as the
> hwmon (class) device's name attribute is also "max6650" (or "max6651".)
> As stated above, this is required to preserve compatibility with
> existing users.
>
>> > > Might be worth taking the opportunity to swap out these magic numbers
>> > > now.
>> >
>> > There's nothing magic about them, they tell the driver how many fans
>> > each device supports. If you don't pass them as driver_data you'll have
>> > to derive them from the device name in the probe function.
>>
>> They're magic in that they're not easily identifiable. In the few
>> moments that I looked at the patch I assumed they were device
>> IDs. They should be clearly defined.
>
> They could have been device IDs, some drivers do that, and that would
> have been equally fine. driver_data can be anything. Best thing to do
> is to document it right above the device id array if you really find it
> confusing (I don't.) I don't know what else exactly you had in mind,
> but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
> strike me as the best coding practice.

Err... no. 1/4 fan is not the only difference between max6650 and
max6651 ... (might be worth looking up the datasheet).

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 5:43 PM, Jean Delvare <jdelvare@suse.de> wrote:
> Hi Lee,
>
> On Mon, 10 Feb 2014 16:58:53 +0000, Lee Jones wrote:
>> > > >  static const struct i2c_device_id max6650_id[] = {
>> > > > -       { "max6650", 1 },
>> > > > -       { "max6651", 4 },
>> > > > +       { "max6650-hwmon", 1 },
>> > > > +       { "max6651-hwmon", 4 },
>> >
>> > No, this is not acceptable, sorry. This will change the name of the
>> > hwmon device as seen from user-space, breaking any configuration file
>> > referring to it. Additionally, dashes are explicitly forbidden in hwmon
>> > device names. And lastly this will break any explicit instantiation of
>> > theses devices (which is the only way, as the driver doesn't support
>> > device auto-detection), be it in the kernel itself or from user-space.
>> >
>> > The change doesn't make sense anyway. If you move to the MFD framework,
>> > the core driver will be an I2C driver binding to the I2C device, and it
>> > will spawn the logical devices, presumably in the form of platform
>> > devices. That's what the current max6650 driver would have to bind to.
>> > Just renaming the device won't work, you also need to change the type.
>> >
>> > If you want to turn this into an MFD driver, I believe you must first
>> > convert the hwmon part to register using
>> > devm_hwmon_device_register_with_groups(). This will dissociate the i2c
>> > device name from the hwmon device name and create a clean name-space
>> > for each function. Guenter, maybe you had a plan to do so already
>> > anyway?
>> >
>> > That being said, going with MFD in this case seems quite overkill to
>> > me. MFD makes a lot of sense when each function has its own resources.
>> > As this isn't the case here, a single driver registering both an hwmon
>> > interface and a pinctrl interface would seem sufficient to me. But I
>> > think Guenter already discussed this in the past so I'll let him
>> > continue and decide.
>>
>> I'll get you guys decide if you want to go the MFD route or not.
>>
>> Either is okay with me, but if you do decide in favour, a name change
>> with the device type appended would be preferred. Else the core device
>> would have the same name as all of its children which would be quite
>> unworkable.
>
> No problem with that. The main (I2C) device should be named max6650 (or
> max6651), the subdevices can be named whatever you want, as long as the
> hwmon (class) device's name attribute is also "max6650" (or "max6651".)
> As stated above, this is required to preserve compatibility with
> existing users.
>
>> > > Might be worth taking the opportunity to swap out these magic numbers
>> > > now.
>> >
>> > There's nothing magic about them, they tell the driver how many fans
>> > each device supports. If you don't pass them as driver_data you'll have
>> > to derive them from the device name in the probe function.
>>
>> They're magic in that they're not easily identifiable. In the few
>> moments that I looked at the patch I assumed they were device
>> IDs. They should be clearly defined.
>
> They could have been device IDs, some drivers do that, and that would
> have been equally fine. driver_data can be anything. Best thing to do
> is to document it right above the device id array if you really find it
> confusing (I don't.) I don't know what else exactly you had in mind,
> but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
> strike me as the best coding practice.

Err... no. 1/4 fan is not the only difference between max6650 and
max6651 ... (might be worth looking up the datasheet).

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

On Mon, 10 Feb 2014 18:27:07 +0000, Laszlo Papp wrote:
> On Mon, Feb 10, 2014 at 5:43 PM, Jean Delvare <jdelvare@suse.de> wrote:
> > On Mon, 10 Feb 2014 16:58:53 +0000, Lee Jones wrote:
> >> > > Might be worth taking the opportunity to swap out these magic numbers
> >> > > now.
> >> >
> >> > There's nothing magic about them, they tell the driver how many fans
> >> > each device supports. If you don't pass them as driver_data you'll have
> >> > to derive them from the device name in the probe function.
> >>
> >> They're magic in that they're not easily identifiable. In the few
> >> moments that I looked at the patch I assumed they were device
> >> IDs. They should be clearly defined.
> >
> > They could have been device IDs, some drivers do that, and that would
> > have been equally fine. driver_data can be anything. Best thing to do
> > is to document it right above the device id array if you really find it
> > confusing (I don't.) I don't know what else exactly you had in mind,
> > but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
> > strike me as the best coding practice.
> 
> Err... no. 1/4 fan is not the only difference between max6650 and
> max6651 ... (might be worth looking up the datasheet).

This is the only difference the driver cared about so far, so the code
made sense. If the driver is extended to support features which differ
between the MAX6650 and MAX6651 then it will make sense to revisit, of
course.

-- 
Jean Delvare
Suse L3 Support

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

On Mon, 10 Feb 2014 18:27:07 +0000, Laszlo Papp wrote:
> On Mon, Feb 10, 2014 at 5:43 PM, Jean Delvare <jdelvare@suse.de> wrote:
> > On Mon, 10 Feb 2014 16:58:53 +0000, Lee Jones wrote:
> >> > > Might be worth taking the opportunity to swap out these magic numbers
> >> > > now.
> >> >
> >> > There's nothing magic about them, they tell the driver how many fans
> >> > each device supports. If you don't pass them as driver_data you'll have
> >> > to derive them from the device name in the probe function.
> >>
> >> They're magic in that they're not easily identifiable. In the few
> >> moments that I looked at the patch I assumed they were device
> >> IDs. They should be clearly defined.
> >
> > They could have been device IDs, some drivers do that, and that would
> > have been equally fine. driver_data can be anything. Best thing to do
> > is to document it right above the device id array if you really find it
> > confusing (I don't.) I don't know what else exactly you had in mind,
> > but #defining FOUR_FANS as 4 and ONE_FAN as 1 and using that doesn't
> > strike me as the best coding practice.
> 
> Err... no. 1/4 fan is not the only difference between max6650 and
> max6651 ... (might be worth looking up the datasheet).

This is the only difference the driver cared about so far, so the code
made sense. If the driver is extended to support features which differ
between the MAX6650 and MAX6651 then it will make sense to revisit, of
course.

-- 
Jean Delvare
Suse L3 Support

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
> Hi Lee, Laszlo,
>
> On Mon, 10 Feb 2014 16:08:42 +0000, Lee Jones wrote:
>> > In the preparation of creating an mfd driver and then refactor this one into a
>> > platform driver in order to add some pinctrl functionality to the chip, it is
>> > necessary to start the series with this change so that the mfd driver can refer
>> > to the proper name in the subsequent change without making changes in more than
>> > one driver later.
>> >
>> > This was a request from Lee Jones, the MFD subsystem maintainer.
>> >
>> > Signed-off-by: Laszlo Papp <lpapp@kde.org>
>> > ---
>> >  drivers/hwmon/max6650.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
>> > index 0cafc39..3c36edc 100644
>> > --- a/drivers/hwmon/max6650.c
>> > +++ b/drivers/hwmon/max6650.c
>> > @@ -116,8 +116,8 @@ static struct max6650_data *max6650_update_device(struct device *dev);
>> >   */
>> >
>> >  static const struct i2c_device_id max6650_id[] = {
>> > -   { "max6650", 1 },
>> > -   { "max6651", 4 },
>> > +   { "max6650-hwmon", 1 },
>> > +   { "max6651-hwmon", 4 },
>
> No, this is not acceptable, sorry. This will change the name of the
> hwmon device as seen from user-space, breaking any configuration file
> referring to it. Additionally, dashes are explicitly forbidden in hwmon
> device names. And lastly this will break any explicit instantiation of
> theses devices (which is the only way, as the driver doesn't support
> device auto-detection), be it in the kernel itself or from user-space.
>
> The change doesn't make sense anyway. If you move to the MFD framework,
> the core driver will be an I2C driver binding to the I2C device, and it
> will spawn the logical devices, presumably in the form of platform
> devices. That's what the current max6650 driver would have to bind to.
> Just renaming the device won't work, you also need to change the type.

Hmm, this paragraph seems to indicate that you have not seen my
previous patch set. I tried to summarize in this commit message that
the type in this subdriver would need to change, yes.

I am fine with not renaming, but appending if such a thing is
possible. What does not make sense to me is acquiring a "global"
max665x name in a sub-device driver. The children have to be
distinguished somehow!

> If you want to turn this into an MFD driver, I believe you must first
> convert the hwmon part to register using
> devm_hwmon_device_register_with_groups(). This will dissociate the i2c
> device name from the hwmon device name and create a clean name-space
> for each function. Guenter, maybe you had a plan to do so already
> anyway?
>
> That being said, going with MFD in this case seems quite overkill to
> me. MFD makes a lot of sense when each function has its own resources.
> As this isn't the case here, a single driver registering both an hwmon
> interface and a pinctrl interface would seem sufficient to me. But I
> think Guenter already discussed this in the past so I'll let him
> continue and decide.

Exactly. This had been overdiscussed. I took my personal preference
without any technical drawback. I prefer clean separation just like
several other mfd drivers are doing, really.

Tell me this is unacceptable, and I will stop helping with getting the
required functionality into the kernel. Frankly, I am getting tired of
having worked on it for a few months now, and we are still at
discussing personal preferences rather than getting features in ...

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
> Hi Lee, Laszlo,
>
> On Mon, 10 Feb 2014 16:08:42 +0000, Lee Jones wrote:
>> > In the preparation of creating an mfd driver and then refactor this one into a
>> > platform driver in order to add some pinctrl functionality to the chip, it is
>> > necessary to start the series with this change so that the mfd driver can refer
>> > to the proper name in the subsequent change without making changes in more than
>> > one driver later.
>> >
>> > This was a request from Lee Jones, the MFD subsystem maintainer.
>> >
>> > Signed-off-by: Laszlo Papp <lpapp@kde.org>
>> > ---
>> >  drivers/hwmon/max6650.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
>> > index 0cafc39..3c36edc 100644
>> > --- a/drivers/hwmon/max6650.c
>> > +++ b/drivers/hwmon/max6650.c
>> > @@ -116,8 +116,8 @@ static struct max6650_data *max6650_update_device(struct device *dev);
>> >   */
>> >
>> >  static const struct i2c_device_id max6650_id[] = {
>> > -   { "max6650", 1 },
>> > -   { "max6651", 4 },
>> > +   { "max6650-hwmon", 1 },
>> > +   { "max6651-hwmon", 4 },
>
> No, this is not acceptable, sorry. This will change the name of the
> hwmon device as seen from user-space, breaking any configuration file
> referring to it. Additionally, dashes are explicitly forbidden in hwmon
> device names. And lastly this will break any explicit instantiation of
> theses devices (which is the only way, as the driver doesn't support
> device auto-detection), be it in the kernel itself or from user-space.
>
> The change doesn't make sense anyway. If you move to the MFD framework,
> the core driver will be an I2C driver binding to the I2C device, and it
> will spawn the logical devices, presumably in the form of platform
> devices. That's what the current max6650 driver would have to bind to.
> Just renaming the device won't work, you also need to change the type.

Hmm, this paragraph seems to indicate that you have not seen my
previous patch set. I tried to summarize in this commit message that
the type in this subdriver would need to change, yes.

I am fine with not renaming, but appending if such a thing is
possible. What does not make sense to me is acquiring a "global"
max665x name in a sub-device driver. The children have to be
distinguished somehow!

> If you want to turn this into an MFD driver, I believe you must first
> convert the hwmon part to register using
> devm_hwmon_device_register_with_groups(). This will dissociate the i2c
> device name from the hwmon device name and create a clean name-space
> for each function. Guenter, maybe you had a plan to do so already
> anyway?
>
> That being said, going with MFD in this case seems quite overkill to
> me. MFD makes a lot of sense when each function has its own resources.
> As this isn't the case here, a single driver registering both an hwmon
> interface and a pinctrl interface would seem sufficient to me. But I
> think Guenter already discussed this in the past so I'll let him
> continue and decide.

Exactly. This had been overdiscussed. I took my personal preference
without any technical drawback. I prefer clean separation just like
several other mfd drivers are doing, really.

Tell me this is unacceptable, and I will stop helping with getting the
required functionality into the kernel. Frankly, I am getting tired of
having worked on it for a few months now, and we are still at
discussing personal preferences rather than getting features in ...

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 5:06 PM, Laszlo Papp <lpapp@kde.org> wrote:
> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> Hi Lee, Laszlo,
>>
>> On Mon, 10 Feb 2014 16:08:42 +0000, Lee Jones wrote:
>>> > In the preparation of creating an mfd driver and then refactor this one into a
>>> > platform driver in order to add some pinctrl functionality to the chip, it is
>>> > necessary to start the series with this change so that the mfd driver can refer
>>> > to the proper name in the subsequent change without making changes in more than
>>> > one driver later.
>>> >
>>> > This was a request from Lee Jones, the MFD subsystem maintainer.
>>> >
>>> > Signed-off-by: Laszlo Papp <lpapp@kde.org>
>>> > ---
>>> >  drivers/hwmon/max6650.c | 4 ++--
>>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
>>> > index 0cafc39..3c36edc 100644
>>> > --- a/drivers/hwmon/max6650.c
>>> > +++ b/drivers/hwmon/max6650.c
>>> > @@ -116,8 +116,8 @@ static struct max6650_data *max6650_update_device(struct device *dev);
>>> >   */
>>> >
>>> >  static const struct i2c_device_id max6650_id[] = {
>>> > -   { "max6650", 1 },
>>> > -   { "max6651", 4 },
>>> > +   { "max6650-hwmon", 1 },
>>> > +   { "max6651-hwmon", 4 },
>>
>> No, this is not acceptable, sorry. This will change the name of the
>> hwmon device as seen from user-space, breaking any configuration file
>> referring to it. Additionally, dashes are explicitly forbidden in hwmon
>> device names. And lastly this will break any explicit instantiation of
>> theses devices (which is the only way, as the driver doesn't support
>> device auto-detection), be it in the kernel itself or from user-space.
>>
>> The change doesn't make sense anyway. If you move to the MFD framework,
>> the core driver will be an I2C driver binding to the I2C device, and it
>> will spawn the logical devices, presumably in the form of platform
>> devices. That's what the current max6650 driver would have to bind to.
>> Just renaming the device won't work, you also need to change the type.
>
> Hmm, this paragraph seems to indicate that you have not seen my
> previous patch set. I tried to summarize in this commit message that
> the type in this subdriver would need to change, yes.
>
> I am fine with not renaming, but appending if such a thing is
> possible. What does not make sense to me is acquiring a "global"
> max665x name in a sub-device driver. The children have to be
> distinguished somehow!
>
>> If you want to turn this into an MFD driver, I believe you must first
>> convert the hwmon part to register using
>> devm_hwmon_device_register_with_groups(). This will dissociate the i2c
>> device name from the hwmon device name and create a clean name-space
>> for each function. Guenter, maybe you had a plan to do so already
>> anyway?
>>
>> That being said, going with MFD in this case seems quite overkill to
>> me. MFD makes a lot of sense when each function has its own resources.
>> As this isn't the case here, a single driver registering both an hwmon
>> interface and a pinctrl interface would seem sufficient to me. But I
>> think Guenter already discussed this in the past so I'll let him
>> continue and decide.
>
> Exactly. This had been overdiscussed. I took my personal preference
> without any technical drawback. I prefer clean separation just like
> several other mfd drivers are doing, really.
>
> Tell me this is unacceptable, and I will stop helping with getting the
> required functionality into the kernel. Frankly, I am getting tired of
> having worked on it for a few months now, and we are still at
> discussing personal preferences rather than getting features in ...

Moreover, I really do not see how this is an overkill. In my opinion,
quite the opposite, putting MFD functionality for different areas into
one particular area is more than peculiar.

Even more, Guenter has been included in the long
gpio/pinctrl/mfd/hwmon patchset discussions. Could you please make up
your mind finally? I am sorry if this sounds harsh, but I hope you
understand my position. Trying to help this stuff moving forward, and
even after months of discussion we are still hitting the same
opinionated gates.

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 5:06 PM, Laszlo Papp <lpapp@kde.org> wrote:
> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> Hi Lee, Laszlo,
>>
>> On Mon, 10 Feb 2014 16:08:42 +0000, Lee Jones wrote:
>>> > In the preparation of creating an mfd driver and then refactor this one into a
>>> > platform driver in order to add some pinctrl functionality to the chip, it is
>>> > necessary to start the series with this change so that the mfd driver can refer
>>> > to the proper name in the subsequent change without making changes in more than
>>> > one driver later.
>>> >
>>> > This was a request from Lee Jones, the MFD subsystem maintainer.
>>> >
>>> > Signed-off-by: Laszlo Papp <lpapp@kde.org>
>>> > ---
>>> >  drivers/hwmon/max6650.c | 4 ++--
>>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c
>>> > index 0cafc39..3c36edc 100644
>>> > --- a/drivers/hwmon/max6650.c
>>> > +++ b/drivers/hwmon/max6650.c
>>> > @@ -116,8 +116,8 @@ static struct max6650_data *max6650_update_device(struct device *dev);
>>> >   */
>>> >
>>> >  static const struct i2c_device_id max6650_id[] = {
>>> > -   { "max6650", 1 },
>>> > -   { "max6651", 4 },
>>> > +   { "max6650-hwmon", 1 },
>>> > +   { "max6651-hwmon", 4 },
>>
>> No, this is not acceptable, sorry. This will change the name of the
>> hwmon device as seen from user-space, breaking any configuration file
>> referring to it. Additionally, dashes are explicitly forbidden in hwmon
>> device names. And lastly this will break any explicit instantiation of
>> theses devices (which is the only way, as the driver doesn't support
>> device auto-detection), be it in the kernel itself or from user-space.
>>
>> The change doesn't make sense anyway. If you move to the MFD framework,
>> the core driver will be an I2C driver binding to the I2C device, and it
>> will spawn the logical devices, presumably in the form of platform
>> devices. That's what the current max6650 driver would have to bind to.
>> Just renaming the device won't work, you also need to change the type.
>
> Hmm, this paragraph seems to indicate that you have not seen my
> previous patch set. I tried to summarize in this commit message that
> the type in this subdriver would need to change, yes.
>
> I am fine with not renaming, but appending if such a thing is
> possible. What does not make sense to me is acquiring a "global"
> max665x name in a sub-device driver. The children have to be
> distinguished somehow!
>
>> If you want to turn this into an MFD driver, I believe you must first
>> convert the hwmon part to register using
>> devm_hwmon_device_register_with_groups(). This will dissociate the i2c
>> device name from the hwmon device name and create a clean name-space
>> for each function. Guenter, maybe you had a plan to do so already
>> anyway?
>>
>> That being said, going with MFD in this case seems quite overkill to
>> me. MFD makes a lot of sense when each function has its own resources.
>> As this isn't the case here, a single driver registering both an hwmon
>> interface and a pinctrl interface would seem sufficient to me. But I
>> think Guenter already discussed this in the past so I'll let him
>> continue and decide.
>
> Exactly. This had been overdiscussed. I took my personal preference
> without any technical drawback. I prefer clean separation just like
> several other mfd drivers are doing, really.
>
> Tell me this is unacceptable, and I will stop helping with getting the
> required functionality into the kernel. Frankly, I am getting tired of
> having worked on it for a few months now, and we are still at
> discussing personal preferences rather than getting features in ...

Moreover, I really do not see how this is an overkill. In my opinion,
quite the opposite, putting MFD functionality for different areas into
one particular area is more than peculiar.

Even more, Guenter has been included in the long
gpio/pinctrl/mfd/hwmon patchset discussions. Could you please make up
your mind finally? I am sorry if this sounds harsh, but I hope you
understand my position. Trying to help this stuff moving forward, and
even after months of discussion we are still hitting the same
opinionated gates.

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
> Additionally, dashes are explicitly forbidden in hwmon
> device names.

Also, where is that documented?

I do not think you can make such a decision, and you will realize that
once you begin to think a bit out of the box and look around. See how
other children are managed for MFD devices. "driver-subsystem" is a
pretty common a schema. I do not really see any point in forbidding
dashes currently. Please do elaborate about the reasons, and the fact
that why it is undocuemented.

Also, I currently do not understand what you are suggesting: just
leave this technically unreasonable situation as is for compatibility
reasons? There is no better support in place for appending further
alternative names?

In any case, at the very least, I hope the lesson is learnt for the
future from this past mistake. If a chip is MFD'ish, a subdevice
driver should not ever be added with such an id.

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
> Additionally, dashes are explicitly forbidden in hwmon
> device names.

Also, where is that documented?

I do not think you can make such a decision, and you will realize that
once you begin to think a bit out of the box and look around. See how
other children are managed for MFD devices. "driver-subsystem" is a
pretty common a schema. I do not really see any point in forbidding
dashes currently. Please do elaborate about the reasons, and the fact
that why it is undocuemented.

Also, I currently do not understand what you are suggesting: just
leave this technically unreasonable situation as is for compatibility
reasons? There is no better support in place for appending further
alternative names?

In any case, at the very least, I hope the lesson is learnt for the
future from this past mistake. If a chip is MFD'ish, a subdevice
driver should not ever be added with such an id.

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

Hi Laszlo,

On Tue, 11 Feb 2014 03:13:37 +0000, Laszlo Papp wrote:
> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
> > Additionally, dashes are explicitly forbidden in hwmon
> > device names.
> 
> Also, where is that documented?

In Documentation/hwmon/sysfs-interface:

*********************
* Global attributes *
*********************

name		The chip name.
		This should be a short, lowercase string, not containing
		spaces nor dashes, representing the chip name. This is
		the only mandatory attribute.
		I2C devices get this attribute created automatically.
		RO

> I do not think you can make such a decision, and you will realize that
> once you begin to think a bit out of the box and look around. See how
> other children are managed for MFD devices. "driver-subsystem" is a
> pretty common a schema. I do not really see any point in forbidding
> dashes currently. Please do elaborate about the reasons, and the fact
> that why it is undocuemented.

It is documented since August 2007 [1], and stop with this tone,
please. The more you talk, the less I want to help you. Guenter already
gave up on you, I might as well do the same.

I did not "make" the decision, it is a limitation implied by the way
libsensors creates unique and persistent identifiers for hwmon devices.
These identifiers are of the form ${name}-${bus}-${address}, where
${bus} can (but doesn't have to) be of the form ${type}-${number}. If
${name} contains a dash then parsing the identifier becomes ambiguous,
as you can no longer easily tell where ${name} ends and where ${bus}
begins.

> Also, I currently do not understand what you are suggesting: just
> leave this technically unreasonable situation as is for compatibility
> reasons? There is no better support in place for appending further
> alternative names?

There's nothing unreasonable about the situation. Yes, compatibility
matters, it matters a lot even, and we do our best to guarantee that
users can move to a more recent kernel without breaking anything that
was previously working. I certainly hope other open source project
maintainers and developers do the same.

I already explained (but apparently you were to busy yelling at Guenter
and myself to notice) that the hwmon device name (which is the one we
can't change) can now be dissociated from the i2c (or platform) device
name, thanks to Guenter's excellent work. This is exactly the solution
to your problem, so there's no need sound dramatic.

> In any case, at the very least, I hope the lesson is learnt for the
> future from this past mistake. If a chip is MFD'ish, a subdevice
> driver should not ever be added with such an id.

You can't always anticipate this kind of thing. If you wait until
you're certain your design (and code) is perfect and will never need
to be changed, then you'll never release any piece of software. That
code you just wrote and submitted, and you believe is perfect, guess
what? Next year someone will call it crap and blame it on you for not
putting enough thinking into it.

There was little reason to believe that the max6650 driver would become
a MFD driver at the time is was written (10 years ago [2].) The concept
of MFD driver did not even exist then. We have several hwmon drivers
implementing side features (such as watchdog) without using the MFD
framework. I still believe using the MFD framework to support the
MAX6650 and MAX6651 is overkill, BTW, so even today I wouldn't
anticipate it. Which does _not_ mean I'll reject the attempt to do so,
contrary to what you repeatedly claimed in various posts of this
discussion.

You sent one patch, I reviewed it, found it was broken, and explained
why. In great details, I believe. Did you actually read my review? Did
you understand it?

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/hwmon/sysfs-interface?id\x176544dc55b0a912a2e1bacb9f48ccbd4aabd55f
[2] http://www.lm-sensors.org/browser/lm-sensors/trunk/kernel/chips/max6650.c?rev\x1987

-- 
Jean Delvare
Suse L3 Support

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

Hi Laszlo,

On Tue, 11 Feb 2014 03:13:37 +0000, Laszlo Papp wrote:
> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
> > Additionally, dashes are explicitly forbidden in hwmon
> > device names.
> 
> Also, where is that documented?

In Documentation/hwmon/sysfs-interface:

*********************
* Global attributes *
*********************

name		The chip name.
		This should be a short, lowercase string, not containing
		spaces nor dashes, representing the chip name. This is
		the only mandatory attribute.
		I2C devices get this attribute created automatically.
		RO

> I do not think you can make such a decision, and you will realize that
> once you begin to think a bit out of the box and look around. See how
> other children are managed for MFD devices. "driver-subsystem" is a
> pretty common a schema. I do not really see any point in forbidding
> dashes currently. Please do elaborate about the reasons, and the fact
> that why it is undocuemented.

It is documented since August 2007 [1], and stop with this tone,
please. The more you talk, the less I want to help you. Guenter already
gave up on you, I might as well do the same.

I did not "make" the decision, it is a limitation implied by the way
libsensors creates unique and persistent identifiers for hwmon devices.
These identifiers are of the form ${name}-${bus}-${address}, where
${bus} can (but doesn't have to) be of the form ${type}-${number}. If
${name} contains a dash then parsing the identifier becomes ambiguous,
as you can no longer easily tell where ${name} ends and where ${bus}
begins.

> Also, I currently do not understand what you are suggesting: just
> leave this technically unreasonable situation as is for compatibility
> reasons? There is no better support in place for appending further
> alternative names?

There's nothing unreasonable about the situation. Yes, compatibility
matters, it matters a lot even, and we do our best to guarantee that
users can move to a more recent kernel without breaking anything that
was previously working. I certainly hope other open source project
maintainers and developers do the same.

I already explained (but apparently you were to busy yelling at Guenter
and myself to notice) that the hwmon device name (which is the one we
can't change) can now be dissociated from the i2c (or platform) device
name, thanks to Guenter's excellent work. This is exactly the solution
to your problem, so there's no need sound dramatic.

> In any case, at the very least, I hope the lesson is learnt for the
> future from this past mistake. If a chip is MFD'ish, a subdevice
> driver should not ever be added with such an id.

You can't always anticipate this kind of thing. If you wait until
you're certain your design (and code) is perfect and will never need
to be changed, then you'll never release any piece of software. That
code you just wrote and submitted, and you believe is perfect, guess
what? Next year someone will call it crap and blame it on you for not
putting enough thinking into it.

There was little reason to believe that the max6650 driver would become
a MFD driver at the time is was written (10 years ago [2].) The concept
of MFD driver did not even exist then. We have several hwmon drivers
implementing side features (such as watchdog) without using the MFD
framework. I still believe using the MFD framework to support the
MAX6650 and MAX6651 is overkill, BTW, so even today I wouldn't
anticipate it. Which does _not_ mean I'll reject the attempt to do so,
contrary to what you repeatedly claimed in various posts of this
discussion.

You sent one patch, I reviewed it, found it was broken, and explained
why. In great details, I believe. Did you actually read my review? Did
you understand it?

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/hwmon/sysfs-interface?id=176544dc55b0a912a2e1bacb9f48ccbd4aabd55f
[2] http://www.lm-sensors.org/browser/lm-sensors/trunk/kernel/chips/max6650.c?rev=1987

-- 
Jean Delvare
Suse L3 Support

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare <jdelvare@suse.de> wrote:
> Hi Laszlo,
>
> On Tue, 11 Feb 2014 03:13:37 +0000, Laszlo Papp wrote:
>> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> > Additionally, dashes are explicitly forbidden in hwmon
>> > device names.
>>
>> Also, where is that documented?
>
> In Documentation/hwmon/sysfs-interface:
>
> *********************
> * Global attributes *
> *********************
>
> name            The chip name.
>                 This should be a short, lowercase string, not containing
>                 spaces nor dashes, representing the chip name. This is
>                 the only mandatory attribute.
>                 I2C devices get this attribute created automatically.
>                 RO
>
>> I do not think you can make such a decision, and you will realize that
>> once you begin to think a bit out of the box and look around. See how
>> other children are managed for MFD devices. "driver-subsystem" is a
>> pretty common a schema. I do not really see any point in forbidding
>> dashes currently. Please do elaborate about the reasons, and the fact
>> that why it is undocuemented.
>
> It is documented since August 2007 [1], and stop with this tone,
> please. The more you talk, the less I want to help you. Guenter already
> gave up on you, I might as well do the same.

My reply is reflecting the frustration coming from the wasted man
months caused by the miscommunication. I cannot step over it easily I
am afraid. Please assume good faith though... I was writing this in
the hope of this situation never ever happening again, so everyone
feels better in the future.

If I can also drop in my two cents, I would like to note that I also
feel bad about the no sign of regret.

> I did not "make" the decision, it is a limitation implied by the way
> libsensors creates unique and persistent identifiers for hwmon devices.
> These identifiers are of the form ${name}-${bus}-${address}, where
> ${bus} can (but doesn't have to) be of the form ${type}-${number}. If
> ${name} contains a dash then parsing the identifier becomes ambiguous,
> as you can no longer easily tell where ${name} ends and where ${bus}
> begins.
>
>> Also, I currently do not understand what you are suggesting: just
>> leave this technically unreasonable situation as is for compatibility
>> reasons? There is no better support in place for appending further
>> alternative names?
>
> There's nothing unreasonable about the situation. Yes, compatibility
> matters, it matters a lot even, and we do our best to guarantee that
> users can move to a more recent kernel without breaking anything that
> was previously working. I certainly hope other open source project
> maintainers and developers do the same.

My post was not actually about compatibility... It was about the fact
that there was no alternative way presented what should be done. That
is, not in an understandable way for me at least.

In any case, please consider rejecting "global names" for MFD chips in
the future for hwmon. This is my personal two cents.

> I already explained (but apparently you were to busy yelling at Guenter
> and myself to notice) that the hwmon device name (which is the one we
> can't change) can now be dissociated from the i2c (or platform) device
> name, thanks to Guenter's excellent work. This is exactly the solution
> to your problem, so there's no need sound dramatic.

Sorry, but I wished to raise the maintainer issue regardless how you
take them. I can assure you that I have done it only with good faith,
so the same mistake never ever happens again.

That being said, I really do not understand what you are writing, so I
guess I would need more details to proceed. Please give a more
thorough explanation that newcomers can also understand. Currently, it
is unclear to me what you are trying to write.

I already asked in the beginning of this thread if it is possible to
add the alternative ids to the list instead of renaming them. I do not
seem to have gotten replies to that.

>> In any case, at the very least, I hope the lesson is learnt for the
>> future from this past mistake. If a chip is MFD'ish, a subdevice
>> driver should not ever be added with such an id.
>
> You can't always anticipate this kind of thing. If you wait until
> you're certain your design (and code) is perfect and will never need
> to be changed, then you'll never release any piece of software. That
> code you just wrote and submitted, and you believe is perfect, guess
> what? Next year someone will call it crap and blame it on you for not
> putting enough thinking into it.

I personally think you are underestimating the issue here. As far as I
can tell, the datasheet is pretty short for this chip, and someone
reading it through surely gets the idea this chip is not at all about
just hwmon. I am not saying people do not do mistakes, but I am
encouraging you to learn the lesson with good intention so that this
can be avoided in the future.

Perhaps the datasheet was not read completely, and the feature was
added haskily... I do not know, but currently I cannot imagine it
otherwise.

> There was little reason to believe that the max6650 driver would become
> a MFD driver at the time is was written (10 years ago [2].) The concept
> of MFD driver did not even exist then. We have several hwmon drivers
> implementing side features (such as watchdog) without using the MFD
> framework. I still believe using the MFD framework to support the
> MAX6650 and MAX6651 is overkill, BTW, so even today I wouldn't
> anticipate it. Which does _not_ mean I'll reject the attempt to do so,
> contrary to what you repeatedly claimed in various posts of this
> discussion.
>
> You sent one patch, I reviewed it, found it was broken, and explained
> why. In great details, I believe. Did you actually read my review? Did
> you understand it?

No, I have not understand it, sorry. Could you please elaborate more?

> [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/hwmon/sysfs-interface?id\x176544dc55b0a912a2e1bacb9f48ccbd4aabd55f
> [2] http://www.lm-sensors.org/browser/lm-sensors/trunk/kernel/chips/max6650.c?rev\x1987
>
> --
> Jean Delvare
> Suse L3 Support

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare <jdelvare@suse.de> wrote:
> Hi Laszlo,
>
> On Tue, 11 Feb 2014 03:13:37 +0000, Laszlo Papp wrote:
>> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> > Additionally, dashes are explicitly forbidden in hwmon
>> > device names.
>>
>> Also, where is that documented?
>
> In Documentation/hwmon/sysfs-interface:
>
> *********************
> * Global attributes *
> *********************
>
> name            The chip name.
>                 This should be a short, lowercase string, not containing
>                 spaces nor dashes, representing the chip name. This is
>                 the only mandatory attribute.
>                 I2C devices get this attribute created automatically.
>                 RO
>
>> I do not think you can make such a decision, and you will realize that
>> once you begin to think a bit out of the box and look around. See how
>> other children are managed for MFD devices. "driver-subsystem" is a
>> pretty common a schema. I do not really see any point in forbidding
>> dashes currently. Please do elaborate about the reasons, and the fact
>> that why it is undocuemented.
>
> It is documented since August 2007 [1], and stop with this tone,
> please. The more you talk, the less I want to help you. Guenter already
> gave up on you, I might as well do the same.

My reply is reflecting the frustration coming from the wasted man
months caused by the miscommunication. I cannot step over it easily I
am afraid. Please assume good faith though... I was writing this in
the hope of this situation never ever happening again, so everyone
feels better in the future.

If I can also drop in my two cents, I would like to note that I also
feel bad about the no sign of regret.

> I did not "make" the decision, it is a limitation implied by the way
> libsensors creates unique and persistent identifiers for hwmon devices.
> These identifiers are of the form ${name}-${bus}-${address}, where
> ${bus} can (but doesn't have to) be of the form ${type}-${number}. If
> ${name} contains a dash then parsing the identifier becomes ambiguous,
> as you can no longer easily tell where ${name} ends and where ${bus}
> begins.
>
>> Also, I currently do not understand what you are suggesting: just
>> leave this technically unreasonable situation as is for compatibility
>> reasons? There is no better support in place for appending further
>> alternative names?
>
> There's nothing unreasonable about the situation. Yes, compatibility
> matters, it matters a lot even, and we do our best to guarantee that
> users can move to a more recent kernel without breaking anything that
> was previously working. I certainly hope other open source project
> maintainers and developers do the same.

My post was not actually about compatibility... It was about the fact
that there was no alternative way presented what should be done. That
is, not in an understandable way for me at least.

In any case, please consider rejecting "global names" for MFD chips in
the future for hwmon. This is my personal two cents.

> I already explained (but apparently you were to busy yelling at Guenter
> and myself to notice) that the hwmon device name (which is the one we
> can't change) can now be dissociated from the i2c (or platform) device
> name, thanks to Guenter's excellent work. This is exactly the solution
> to your problem, so there's no need sound dramatic.

Sorry, but I wished to raise the maintainer issue regardless how you
take them. I can assure you that I have done it only with good faith,
so the same mistake never ever happens again.

That being said, I really do not understand what you are writing, so I
guess I would need more details to proceed. Please give a more
thorough explanation that newcomers can also understand. Currently, it
is unclear to me what you are trying to write.

I already asked in the beginning of this thread if it is possible to
add the alternative ids to the list instead of renaming them. I do not
seem to have gotten replies to that.

>> In any case, at the very least, I hope the lesson is learnt for the
>> future from this past mistake. If a chip is MFD'ish, a subdevice
>> driver should not ever be added with such an id.
>
> You can't always anticipate this kind of thing. If you wait until
> you're certain your design (and code) is perfect and will never need
> to be changed, then you'll never release any piece of software. That
> code you just wrote and submitted, and you believe is perfect, guess
> what? Next year someone will call it crap and blame it on you for not
> putting enough thinking into it.

I personally think you are underestimating the issue here. As far as I
can tell, the datasheet is pretty short for this chip, and someone
reading it through surely gets the idea this chip is not at all about
just hwmon. I am not saying people do not do mistakes, but I am
encouraging you to learn the lesson with good intention so that this
can be avoided in the future.

Perhaps the datasheet was not read completely, and the feature was
added haskily... I do not know, but currently I cannot imagine it
otherwise.

> There was little reason to believe that the max6650 driver would become
> a MFD driver at the time is was written (10 years ago [2].) The concept
> of MFD driver did not even exist then. We have several hwmon drivers
> implementing side features (such as watchdog) without using the MFD
> framework. I still believe using the MFD framework to support the
> MAX6650 and MAX6651 is overkill, BTW, so even today I wouldn't
> anticipate it. Which does _not_ mean I'll reject the attempt to do so,
> contrary to what you repeatedly claimed in various posts of this
> discussion.
>
> You sent one patch, I reviewed it, found it was broken, and explained
> why. In great details, I believe. Did you actually read my review? Did
> you understand it?

No, I have not understand it, sorry. Could you please elaborate more?

> [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/hwmon/sysfs-interface?id=176544dc55b0a912a2e1bacb9f48ccbd4aabd55f
> [2] http://www.lm-sensors.org/browser/lm-sensors/trunk/kernel/chips/max6650.c?rev=1987
>
> --
> Jean Delvare
> Suse L3 Support

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare <jdelvare@suse.de> wrote:
> Hi Laszlo,
>
> On Tue, 11 Feb 2014 03:13:37 +0000, Laszlo Papp wrote:
>> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> > Additionally, dashes are explicitly forbidden in hwmon
>> > device names.
>>
>> Also, where is that documented?
>
> In Documentation/hwmon/sysfs-interface:
>
> *********************
> * Global attributes *
> *********************
>
> name            The chip name.
>                 This should be a short, lowercase string, not containing
>                 spaces nor dashes, representing the chip name. This is
>                 the only mandatory attribute.
>                 I2C devices get this attribute created automatically.
>                 RO

Time to revisit this decision....

So, based on the fact that children device names usually contain
dashes, I do not understand why hwmon would be any special in this
regard. It is possible that the hwmon developers have not faced much
MFD situation before, and so, this was not considered to be handled
like in other subsystems.

I am proposing to change this "rule"...  Any objection?

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare <jdelvare@suse.de> wrote:
> Hi Laszlo,
>
> On Tue, 11 Feb 2014 03:13:37 +0000, Laszlo Papp wrote:
>> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> > Additionally, dashes are explicitly forbidden in hwmon
>> > device names.
>>
>> Also, where is that documented?
>
> In Documentation/hwmon/sysfs-interface:
>
> *********************
> * Global attributes *
> *********************
>
> name            The chip name.
>                 This should be a short, lowercase string, not containing
>                 spaces nor dashes, representing the chip name. This is
>                 the only mandatory attribute.
>                 I2C devices get this attribute created automatically.
>                 RO

Time to revisit this decision....

So, based on the fact that children device names usually contain
dashes, I do not understand why hwmon would be any special in this
regard. It is possible that the hwmon developers have not faced much
MFD situation before, and so, this was not considered to be handled
like in other subsystems.

I am proposing to change this "rule"...  Any objection?

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

TGUgVHVlc2RheSAxMSBGZWJydWFyeSAyMDE0IMOgIDA4OjI4ICswMDAwLCBMYXN6bG8gUGFwcCBh
IMOpY3JpdCA6Cj4gT24gVHVlLCBGZWIgMTEsIDIwMTQgYXQgNzo1MCBBTSwgSmVhbiBEZWx2YXJl
IDxqZGVsdmFyZUBzdXNlLmRlPiB3cm90ZToKPiA+IEhpIExhc3psbywKPiA+Cj4gPiBPbiBUdWUs
IDExIEZlYiAyMDE0IDAzOjEzOjM3ICswMDAwLCBMYXN6bG8gUGFwcCB3cm90ZToKPiA+PiBPbiBN
b24sIEZlYiAxMCwgMjAxNCBhdCA0OjM4IFBNLCBKZWFuIERlbHZhcmUgPGpkZWx2YXJlQHN1c2Uu
ZGU+IHdyb3RlOgo+ID4+ID4gQWRkaXRpb25hbGx5LCBkYXNoZXMgYXJlIGV4cGxpY2l0bHkgZm9y
YmlkZGVuIGluIGh3bW9uCj4gPj4gPiBkZXZpY2UgbmFtZXMuCj4gPj4KPiA+PiBBbHNvLCB3aGVy
ZSBpcyB0aGF0IGRvY3VtZW50ZWQ/Cj4gPgo+ID4gSW4gRG9jdW1lbnRhdGlvbi9od21vbi9zeXNm
cy1pbnRlcmZhY2U6Cj4gPgo+ID4gKioqKioqKioqKioqKioqKioqKioqCj4gPiAqIEdsb2JhbCBh
dHRyaWJ1dGVzICoKPiA+ICoqKioqKioqKioqKioqKioqKioqKgo+ID4KPiA+IG5hbWUgICAgICAg
ICAgICBUaGUgY2hpcCBuYW1lLgo+ID4gICAgICAgICAgICAgICAgIFRoaXMgc2hvdWxkIGJlIGEg
c2hvcnQsIGxvd2VyY2FzZSBzdHJpbmcsIG5vdCBjb250YWluaW5nCj4gPiAgICAgICAgICAgICAg
ICAgc3BhY2VzIG5vciBkYXNoZXMsIHJlcHJlc2VudGluZyB0aGUgY2hpcCBuYW1lLiBUaGlzIGlz
Cj4gPiAgICAgICAgICAgICAgICAgdGhlIG9ubHkgbWFuZGF0b3J5IGF0dHJpYnV0ZS4KPiA+ICAg
ICAgICAgICAgICAgICBJMkMgZGV2aWNlcyBnZXQgdGhpcyBhdHRyaWJ1dGUgY3JlYXRlZCBhdXRv
bWF0aWNhbGx5Lgo+ID4gICAgICAgICAgICAgICAgIFJPCj4gCj4gVGltZSB0byByZXZpc2l0IHRo
aXMgZGVjaXNpb24uLi4uCj4gCj4gU28sIGJhc2VkIG9uIHRoZSBmYWN0IHRoYXQgY2hpbGRyZW4g
ZGV2aWNlIG5hbWVzIHVzdWFsbHkgY29udGFpbgo+IGRhc2hlcywgSSBkbyBub3QgdW5kZXJzdGFu
ZCB3aHkgaHdtb24gd291bGQgYmUgYW55IHNwZWNpYWwgaW4gdGhpcwo+IHJlZ2FyZC4gSXQgaXMg
cG9zc2libGUgdGhhdCB0aGUgaHdtb24gZGV2ZWxvcGVycyBoYXZlIG5vdCBmYWNlZCBtdWNoCj4g
TUZEIHNpdHVhdGlvbiBiZWZvcmUsIGFuZCBzbywgdGhpcyB3YXMgbm90IGNvbnNpZGVyZWQgdG8g
YmUgaGFuZGxlZAo+IGxpa2UgaW4gb3RoZXIgc3Vic3lzdGVtcy4KPiAKPiBJIGFtIHByb3Bvc2lu
ZyB0byBjaGFuZ2UgdGhpcyAicnVsZSIuLi4gIEFueSBvYmplY3Rpb24/CgpJJ20gZ2l2aW5nIHVw
IGhlcmUsIHNvcnJ5LiBUaGVyZSdzIG5vIHBvaW50IGluIG1lIHdyaXRpbmcgYW55IG1vcmUgb24K
dGhlIHRvcGljIGFzIHlvdSBhcmUgbm90IGxpc3RlbmluZyB0byBtZS4gSSBkbyBub3QgaGF2ZSB0
aGUgaW1wcmVzc2lvbgp5b3UgYXJlIGRvaW5nIGFueSBlZmZvcnQgdG8gdW5kZXJzdGFuZCB3aGF0
IEknbSBzYXlpbmcuIFBsdXMgeW91IGtlZXAKZm9jdXNpbmcgb24gdGhpbmdzIHRoYXQgZG8gbm90
IG1hdHRlciBhbmQgcHJvYmxlbXMgZm9yIHdoaWNoIGEgc29sdXRpb24KaGFzIGFscmVhZHkgYmVl
biBwcm92aWRlZC4gU28gd2hlcmV2ZXIgeW91J3JlIGdvaW5nLCB0aGlzIGlzIHdpdGhvdXQgbWUu
CgotLSAKSmVhbiBEZWx2YXJlClN1c2UgTDMgU3VwcG9ydAoKCl9fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fCmxtLXNlbnNvcnMgbWFpbGluZyBsaXN0CmxtLXNl
bnNvcnNAbG0tc2Vuc29ycy5vcmcKaHR0cDovL2xpc3RzLmxtLXNlbnNvcnMub3JnL21haWxtYW4v
bGlzdGluZm8vbG0tc2Vuc29ycw=

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Jean Delvare]

Le Tuesday 11 February 2014 à 08:28 +0000, Laszlo Papp a écrit :
> On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare <jdelvare@suse.de> wrote:
> > Hi Laszlo,
> >
> > On Tue, 11 Feb 2014 03:13:37 +0000, Laszlo Papp wrote:
> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
> >> > Additionally, dashes are explicitly forbidden in hwmon
> >> > device names.
> >>
> >> Also, where is that documented?
> >
> > In Documentation/hwmon/sysfs-interface:
> >
> > *********************
> > * Global attributes *
> > *********************
> >
> > name            The chip name.
> >                 This should be a short, lowercase string, not containing
> >                 spaces nor dashes, representing the chip name. This is
> >                 the only mandatory attribute.
> >                 I2C devices get this attribute created automatically.
> >                 RO
> 
> Time to revisit this decision....
> 
> So, based on the fact that children device names usually contain
> dashes, I do not understand why hwmon would be any special in this
> regard. It is possible that the hwmon developers have not faced much
> MFD situation before, and so, this was not considered to be handled
> like in other subsystems.
> 
> I am proposing to change this "rule"...  Any objection?

I'm giving up here, sorry. There's no point in me writing any more on
the topic as you are not listening to me. I do not have the impression
you are doing any effort to understand what I'm saying. Plus you keep
focusing on things that do not matter and problems for which a solution
has already been provided. So wherever you're going, this is without me.

-- 
Jean Delvare
Suse L3 Support

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 8:49 AM, Jean Delvare <jdelvare@suse.de> wrote:
> Le Tuesday 11 February 2014 à 08:28 +0000, Laszlo Papp a écrit :
>> On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare <jdelvare@suse.de> wrote:
>> > Hi Laszlo,
>> >
>> > On Tue, 11 Feb 2014 03:13:37 +0000, Laszlo Papp wrote:
>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> >> > Additionally, dashes are explicitly forbidden in hwmon
>> >> > device names.
>> >>
>> >> Also, where is that documented?
>> >
>> > In Documentation/hwmon/sysfs-interface:
>> >
>> > *********************
>> > * Global attributes *
>> > *********************
>> >
>> > name            The chip name.
>> >                 This should be a short, lowercase string, not containing
>> >                 spaces nor dashes, representing the chip name. This is
>> >                 the only mandatory attribute.
>> >                 I2C devices get this attribute created automatically.
>> >                 RO
>>
>> Time to revisit this decision....
>>
>> So, based on the fact that children device names usually contain
>> dashes, I do not understand why hwmon would be any special in this
>> regard. It is possible that the hwmon developers have not faced much
>> MFD situation before, and so, this was not considered to be handled
>> like in other subsystems.
>>
>> I am proposing to change this "rule"...  Any objection?
>
> I'm giving up here, sorry. There's no point in me writing any more on
> the topic as you are not listening to me. I do not have the impression
> you are doing any effort to understand what I'm saying. Plus you keep
> focusing on things that do not matter and problems for which a solution
> has already been provided.

Does it mean the people cannot come up with ideas if they think
something may be better than some provided way? Surely, software
evolves over time and people are sometimes right, and sometimes wrong
right?

I can assure you that I do listen, please assume good faith with ideas
thrown in. If you think it is that bad an idea, you could have
explained why so, so that I and others can also understand it who do
not yet, right?

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 8:49 AM, Jean Delvare <jdelvare@suse.de> wrote:
> Le Tuesday 11 February 2014 à 08:28 +0000, Laszlo Papp a écrit :
>> On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare <jdelvare@suse.de> wrote:
>> > Hi Laszlo,
>> >
>> > On Tue, 11 Feb 2014 03:13:37 +0000, Laszlo Papp wrote:
>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> >> > Additionally, dashes are explicitly forbidden in hwmon
>> >> > device names.
>> >>
>> >> Also, where is that documented?
>> >
>> > In Documentation/hwmon/sysfs-interface:
>> >
>> > *********************
>> > * Global attributes *
>> > *********************
>> >
>> > name            The chip name.
>> >                 This should be a short, lowercase string, not containing
>> >                 spaces nor dashes, representing the chip name. This is
>> >                 the only mandatory attribute.
>> >                 I2C devices get this attribute created automatically.
>> >                 RO
>>
>> Time to revisit this decision....
>>
>> So, based on the fact that children device names usually contain
>> dashes, I do not understand why hwmon would be any special in this
>> regard. It is possible that the hwmon developers have not faced much
>> MFD situation before, and so, this was not considered to be handled
>> like in other subsystems.
>>
>> I am proposing to change this "rule"...  Any objection?
>
> I'm giving up here, sorry. There's no point in me writing any more on
> the topic as you are not listening to me. I do not have the impression
> you are doing any effort to understand what I'm saying. Plus you keep
> focusing on things that do not matter and problems for which a solution
> has already been provided.

Does it mean the people cannot come up with ideas if they think
something may be better than some provided way? Surely, software
evolves over time and people are sometimes right, and sometimes wrong
right?

I can assure you that I do listen, please assume good faith with ideas
thrown in. If you think it is that bad an idea, you could have
explained why so, so that I and others can also understand it who do
not yet, right?

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

PiA+PiBUaW1lIHRvIHJldmlzaXQgdGhpcyBkZWNpc2lvbi4uLi4KPiA+Pgo+ID4+IFNvLCBiYXNl
ZCBvbiB0aGUgZmFjdCB0aGF0IGNoaWxkcmVuIGRldmljZSBuYW1lcyB1c3VhbGx5IGNvbnRhaW4K
PiA+PiBkYXNoZXMsIEkgZG8gbm90IHVuZGVyc3RhbmQgd2h5IGh3bW9uIHdvdWxkIGJlIGFueSBz
cGVjaWFsIGluIHRoaXMKPiA+PiByZWdhcmQuIEl0IGlzIHBvc3NpYmxlIHRoYXQgdGhlIGh3bW9u
IGRldmVsb3BlcnMgaGF2ZSBub3QgZmFjZWQgbXVjaAo+ID4+IE1GRCBzaXR1YXRpb24gYmVmb3Jl
LCBhbmQgc28sIHRoaXMgd2FzIG5vdCBjb25zaWRlcmVkIHRvIGJlIGhhbmRsZWQKPiA+PiBsaWtl
IGluIG90aGVyIHN1YnN5c3RlbXMuCj4gPj4KPiA+PiBJIGFtIHByb3Bvc2luZyB0byBjaGFuZ2Ug
dGhpcyAicnVsZSIuLi4gIEFueSBvYmplY3Rpb24/Cj4gPgo+ID4gSSdtIGdpdmluZyB1cCBoZXJl
LCBzb3JyeS4gVGhlcmUncyBubyBwb2ludCBpbiBtZSB3cml0aW5nIGFueSBtb3JlIG9uCj4gPiB0
aGUgdG9waWMgYXMgeW91IGFyZSBub3QgbGlzdGVuaW5nIHRvIG1lLiBJIGRvIG5vdCBoYXZlIHRo
ZSBpbXByZXNzaW9uCj4gPiB5b3UgYXJlIGRvaW5nIGFueSBlZmZvcnQgdG8gdW5kZXJzdGFuZCB3
aGF0IEknbSBzYXlpbmcuIFBsdXMgeW91IGtlZXAKPiA+IGZvY3VzaW5nIG9uIHRoaW5ncyB0aGF0
IGRvIG5vdCBtYXR0ZXIgYW5kIHByb2JsZW1zIGZvciB3aGljaCBhIHNvbHV0aW9uCj4gPiBoYXMg
YWxyZWFkeSBiZWVuIHByb3ZpZGVkLgo+IAo+IERvZXMgaXQgbWVhbiB0aGUgcGVvcGxlIGNhbm5v
dCBjb21lIHVwIHdpdGggaWRlYXMgaWYgdGhleSB0aGluawo+IHNvbWV0aGluZyBtYXkgYmUgYmV0
dGVyIHRoYW4gc29tZSBwcm92aWRlZCB3YXk/IFN1cmVseSwgc29mdHdhcmUKPiBldm9sdmVzIG92
ZXIgdGltZSBhbmQgcGVvcGxlIGFyZSBzb21ldGltZXMgcmlnaHQsIGFuZCBzb21ldGltZXMgd3Jv
bmcKPiByaWdodD8KPiAKPiBJIGNhbiBhc3N1cmUgeW91IHRoYXQgSSBkbyBsaXN0ZW4sIHBsZWFz
ZSBhc3N1bWUgZ29vZCBmYWl0aCB3aXRoIGlkZWFzCj4gdGhyb3duIGluLiBJZiB5b3UgdGhpbmsg
aXQgaXMgdGhhdCBiYWQgYW4gaWRlYSwgeW91IGNvdWxkIGhhdmUKPiBleHBsYWluZWQgd2h5IHNv
LCBzbyB0aGF0IEkgYW5kIG90aGVycyBjYW4gYWxzbyB1bmRlcnN0YW5kIGl0IHdobyBkbwo+IG5v
dCB5ZXQsIHJpZ2h0PwoKSW4gdGhlIHNtYWxsIGFtb3VudCBvZiB0aW1lIHlvdSd2ZSBiZWVuIHdv
cmtpbmcgaW4gdGhlIGtlcm5lbCBJJ3ZlCnNlZW4gMy00IE1haW50YWluZXJzIGNvbXBsZXRlbHkg
Z2l2ZSB1cCBvbiB5b3UgYW5kIDItMyBkcml2ZW4gY2xvc2UgdG8KdGhlIGVuZCBvZiB0aGVpciB0
ZXRoZXIgKG15c2VsZiBpbmNsdWRlZCkuCgpSYW50aW5nIGFuZCBkZW1hbmRpbmcgd2lsbCBub3Qg
d29yayB3ZWxsIGZvciB5b3UgaGVyZS4gUGxlYXNlLCBmb3IKeW91ciBvd24gc2FrZSwgbGVhcm4g
c29tZSBkaXBsb21hY3kgYW5kIGJlIGNvdXJ0ZW91cyB0byBvdGhlcnMgb24gdGhlCmxpc3QuIElm
IHBlb3BsZSBhcmUgcmVwbHlpbmcgaXQgbWVhbnMgdGhleSdyZSB0cnlpbmcgdG8gaGVscC4gSnVt
cGluZwpkb3duIHRoZWlyIHRocm9hdHMgaXMgbm90IGNvbmR1Y2l2ZSB0byB5b3VyIHN1Y2Nlc3Mg
YW5kIHdpbGwgbm90CnJlc3VsdCBpbiBhIHdpbi4gIAoKLS0gCkxlZSBKb25lcwpMaW5hcm8gU1RN
aWNyb2VsZWN0cm9uaWNzIExhbmRpbmcgVGVhbSBMZWFkCkxpbmFyby5vcmcg4pSCIE9wZW4gc291
cmNlIHNvZnR3YXJlIGZvciBBUk0gU29DcwpGb2xsb3cgTGluYXJvOiBGYWNlYm9vayB8IFR3aXR0
ZXIgfCBCbG9nCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
XwpsbS1zZW5zb3JzIG1haWxpbmcgbGlzdApsbS1zZW5zb3JzQGxtLXNlbnNvcnMub3JnCmh0dHA6
Ly9saXN0cy5sbS1zZW5zb3JzLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xtLXNlbnNvcnM

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

> >> Time to revisit this decision....
> >>
> >> So, based on the fact that children device names usually contain
> >> dashes, I do not understand why hwmon would be any special in this
> >> regard. It is possible that the hwmon developers have not faced much
> >> MFD situation before, and so, this was not considered to be handled
> >> like in other subsystems.
> >>
> >> I am proposing to change this "rule"...  Any objection?
> >
> > I'm giving up here, sorry. There's no point in me writing any more on
> > the topic as you are not listening to me. I do not have the impression
> > you are doing any effort to understand what I'm saying. Plus you keep
> > focusing on things that do not matter and problems for which a solution
> > has already been provided.
> 
> Does it mean the people cannot come up with ideas if they think
> something may be better than some provided way? Surely, software
> evolves over time and people are sometimes right, and sometimes wrong
> right?
> 
> I can assure you that I do listen, please assume good faith with ideas
> thrown in. If you think it is that bad an idea, you could have
> explained why so, so that I and others can also understand it who do
> not yet, right?

In the small amount of time you've been working in the kernel I've
seen 3-4 Maintainers completely give up on you and 2-3 driven close to
the end of their tether (myself included).

Ranting and demanding will not work well for you here. Please, for
your own sake, learn some diplomacy and be courteous to others on the
list. If people are replying it means they're trying to help. Jumping
down their throats is not conducive to your success and will not
result in a win.  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 9:57 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> Time to revisit this decision....
>> >>
>> >> So, based on the fact that children device names usually contain
>> >> dashes, I do not understand why hwmon would be any special in this
>> >> regard. It is possible that the hwmon developers have not faced much
>> >> MFD situation before, and so, this was not considered to be handled
>> >> like in other subsystems.
>> >>
>> >> I am proposing to change this "rule"...  Any objection?
>> >
>> > I'm giving up here, sorry. There's no point in me writing any more on
>> > the topic as you are not listening to me. I do not have the impression
>> > you are doing any effort to understand what I'm saying. Plus you keep
>> > focusing on things that do not matter and problems for which a solution
>> > has already been provided.
>>
>> Does it mean the people cannot come up with ideas if they think
>> something may be better than some provided way? Surely, software
>> evolves over time and people are sometimes right, and sometimes wrong
>> right?
>>
>> I can assure you that I do listen, please assume good faith with ideas
>> thrown in. If you think it is that bad an idea, you could have
>> explained why so, so that I and others can also understand it who do
>> not yet, right?
>
> In the small amount of time you've been working in the kernel I've
> seen 3-4 Maintainers completely give up on you and 2-3 driven close to
> the end of their tether (myself included).
>
> Ranting and demanding will not work well for you here. Please, for
> your own sake, learn some diplomacy and be courteous to others on the
> list. If people are replying it means they're trying to help. Jumping
> down their throats is not conducive to your success and will not
> result in a win.

I have not intended to demand anything, but I must confess that I was
frustrated about my long work being wasted, and I could probably have
spent my time with sipping some tea instead of replying. :-)

Apologies if I had hurt anyone involved. It is far from my intention.
I just really wished to proceed with this feature. Will try to grab my
cup next time earlier. Cheers.

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 9:57 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> Time to revisit this decision....
>> >>
>> >> So, based on the fact that children device names usually contain
>> >> dashes, I do not understand why hwmon would be any special in this
>> >> regard. It is possible that the hwmon developers have not faced much
>> >> MFD situation before, and so, this was not considered to be handled
>> >> like in other subsystems.
>> >>
>> >> I am proposing to change this "rule"...  Any objection?
>> >
>> > I'm giving up here, sorry. There's no point in me writing any more on
>> > the topic as you are not listening to me. I do not have the impression
>> > you are doing any effort to understand what I'm saying. Plus you keep
>> > focusing on things that do not matter and problems for which a solution
>> > has already been provided.
>>
>> Does it mean the people cannot come up with ideas if they think
>> something may be better than some provided way? Surely, software
>> evolves over time and people are sometimes right, and sometimes wrong
>> right?
>>
>> I can assure you that I do listen, please assume good faith with ideas
>> thrown in. If you think it is that bad an idea, you could have
>> explained why so, so that I and others can also understand it who do
>> not yet, right?
>
> In the small amount of time you've been working in the kernel I've
> seen 3-4 Maintainers completely give up on you and 2-3 driven close to
> the end of their tether (myself included).
>
> Ranting and demanding will not work well for you here. Please, for
> your own sake, learn some diplomacy and be courteous to others on the
> list. If people are replying it means they're trying to help. Jumping
> down their throats is not conducive to your success and will not
> result in a win.

I have not intended to demand anything, but I must confess that I was
frustrated about my long work being wasted, and I could probably have
spent my time with sipping some tea instead of replying. :-)

Apologies if I had hurt anyone involved. It is far from my intention.
I just really wished to proceed with this feature. Will try to grab my
cup next time earlier. Cheers.

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

PiA+PiBPbiBNb24sIEZlYiAxMCwgMjAxNCBhdCA0OjM4IFBNLCBKZWFuIERlbHZhcmUgPGpkZWx2
YXJlQHN1c2UuZGU+IHdyb3RlOgo+ID4+ID4gQWRkaXRpb25hbGx5LCBkYXNoZXMgYXJlIGV4cGxp
Y2l0bHkgZm9yYmlkZGVuIGluIGh3bW9uCj4gPj4gPiBkZXZpY2UgbmFtZXMuCj4gPj4KPiA+PiBB
bHNvLCB3aGVyZSBpcyB0aGF0IGRvY3VtZW50ZWQ/Cj4gPgo+ID4gSW4gRG9jdW1lbnRhdGlvbi9o
d21vbi9zeXNmcy1pbnRlcmZhY2U6Cj4gPgo+ID4gKioqKioqKioqKioqKioqKioqKioqCj4gPiAq
IEdsb2JhbCBhdHRyaWJ1dGVzICoKPiA+ICoqKioqKioqKioqKioqKioqKioqKgo+ID4KPiA+IG5h
bWUgICAgICAgICAgICBUaGUgY2hpcCBuYW1lLgo+ID4gICAgICAgICAgICAgICAgIFRoaXMgc2hv
dWxkIGJlIGEgc2hvcnQsIGxvd2VyY2FzZSBzdHJpbmcsIG5vdCBjb250YWluaW5nCj4gPiAgICAg
ICAgICAgICAgICAgc3BhY2VzIG5vciBkYXNoZXMsIHJlcHJlc2VudGluZyB0aGUgY2hpcCBuYW1l
LiBUaGlzIGlzCj4gPiAgICAgICAgICAgICAgICAgdGhlIG9ubHkgbWFuZGF0b3J5IGF0dHJpYnV0
ZS4KPiA+ICAgICAgICAgICAgICAgICBJMkMgZGV2aWNlcyBnZXQgdGhpcyBhdHRyaWJ1dGUgY3Jl
YXRlZCBhdXRvbWF0aWNhbGx5Lgo+ID4gICAgICAgICAgICAgICAgIFJPCj4gCj4gVGltZSB0byBy
ZXZpc2l0IHRoaXMgZGVjaXNpb24uLi4uCj4gCj4gU28sIGJhc2VkIG9uIHRoZSBmYWN0IHRoYXQg
Y2hpbGRyZW4gZGV2aWNlIG5hbWVzIHVzdWFsbHkgY29udGFpbgo+IGRhc2hlcywgSSBkbyBub3Qg
dW5kZXJzdGFuZCB3aHkgaHdtb24gd291bGQgYmUgYW55IHNwZWNpYWwgaW4gdGhpcwo+IHJlZ2Fy
ZC4gSXQgaXMgcG9zc2libGUgdGhhdCB0aGUgaHdtb24gZGV2ZWxvcGVycyBoYXZlIG5vdCBmYWNl
ZCBtdWNoCj4gTUZEIHNpdHVhdGlvbiBiZWZvcmUsIGFuZCBzbywgdGhpcyB3YXMgbm90IGNvbnNp
ZGVyZWQgdG8gYmUgaGFuZGxlZAo+IGxpa2UgaW4gb3RoZXIgc3Vic3lzdGVtcy4KPiAKPiBJIGFt
IHByb3Bvc2luZyB0byBjaGFuZ2UgdGhpcyAicnVsZSIuLi4gIEFueSBvYmplY3Rpb24/CgpQcmlv
ciB0byBwcm9wb3Npbmcgc3VjaCBhbiBpbnZhc2l2ZSBjaGFuZ2Ugd2hpY2ggaXMgaGlnaGx5IGxp
a2VseSB0bwpjb21lIHVwIGFnYWluc3QgaGVhdnkgb3Bwb3NpdGlvbiwgd2h5IGRvbid0IHlvdSB0
cnkgdG8gd29yayBfd2l0aF8gdGhlCmN1cnJlbnQgcnVsaW5nIGFuZCB0aGUgTWFpbnRhaW5lcnMg
dG8gc2VlIHdoYXQgb3RoZXJzIGhhdmUgZG9uZSB0bwpzb2x2ZSB0aGUgcHJvYmxlbS4gIEkgaGln
aGx5IGRvdWJ0IHlvdSBhcmUgdGhlIGZpcnN0L29ubHkgZGV2ZWxvcGVyIHdobwpoYXMgaGFkIHRo
aXMgaXNzdWUuCgpEbzoKICBgZ2l0IGdyZXAgIlwtaHdtb24iYAoKLi4uIGFuZCBoYXZlIGEgZ29v
ZCBsb29rIGFyb3VuZCBmb3IgYW4gYWNjZXB0ZWQgc29sdXRpb24uCgotLSAKTGVlIEpvbmVzCkxp
bmFybyBTVE1pY3JvZWxlY3Ryb25pY3MgTGFuZGluZyBUZWFtIExlYWQKTGluYXJvLm9yZyDilIIg
T3BlbiBzb3VyY2Ugc29mdHdhcmUgZm9yIEFSTSBTb0NzCkZvbGxvdyBMaW5hcm86IEZhY2Vib29r
IHwgVHdpdHRlciB8IEJsb2cKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fCmxtLXNlbnNvcnMgbWFpbGluZyBsaXN0CmxtLXNlbnNvcnNAbG0tc2Vuc29ycy5v
cmcKaHR0cDovL2xpc3RzLmxtLXNlbnNvcnMub3JnL21haWxtYW4vbGlzdGluZm8vbG0tc2Vuc29y
cw=

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
> >> > Additionally, dashes are explicitly forbidden in hwmon
> >> > device names.
> >>
> >> Also, where is that documented?
> >
> > In Documentation/hwmon/sysfs-interface:
> >
> > *********************
> > * Global attributes *
> > *********************
> >
> > name            The chip name.
> >                 This should be a short, lowercase string, not containing
> >                 spaces nor dashes, representing the chip name. This is
> >                 the only mandatory attribute.
> >                 I2C devices get this attribute created automatically.
> >                 RO
> 
> Time to revisit this decision....
> 
> So, based on the fact that children device names usually contain
> dashes, I do not understand why hwmon would be any special in this
> regard. It is possible that the hwmon developers have not faced much
> MFD situation before, and so, this was not considered to be handled
> like in other subsystems.
> 
> I am proposing to change this "rule"...  Any objection?

Prior to proposing such an invasive change which is highly likely to
come up against heavy opposition, why don't you try to work _with_ the
current ruling and the Maintainers to see what others have done to
solve the problem.  I highly doubt you are the first/only developer who
has had this issue.

Do:
  `git grep "\-hwmon"`

... and have a good look around for an accepted solution.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> >> > Additionally, dashes are explicitly forbidden in hwmon
>> >> > device names.
>> >>
>> >> Also, where is that documented?
>> >
>> > In Documentation/hwmon/sysfs-interface:
>> >
>> > *********************
>> > * Global attributes *
>> > *********************
>> >
>> > name            The chip name.
>> >                 This should be a short, lowercase string, not containing
>> >                 spaces nor dashes, representing the chip name. This is
>> >                 the only mandatory attribute.
>> >                 I2C devices get this attribute created automatically.
>> >                 RO
>>
>> Time to revisit this decision....
>>
>> So, based on the fact that children device names usually contain
>> dashes, I do not understand why hwmon would be any special in this
>> regard. It is possible that the hwmon developers have not faced much
>> MFD situation before, and so, this was not considered to be handled
>> like in other subsystems.
>>
>> I am proposing to change this "rule"...  Any objection?
>
> Prior to proposing such an invasive change which is highly likely to
> come up against heavy opposition,

It is possible that someone does not understand why you think it may
be invasive, right? Could you please explain the reason for that?

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> >> > Additionally, dashes are explicitly forbidden in hwmon
>> >> > device names.
>> >>
>> >> Also, where is that documented?
>> >
>> > In Documentation/hwmon/sysfs-interface:
>> >
>> > *********************
>> > * Global attributes *
>> > *********************
>> >
>> > name            The chip name.
>> >                 This should be a short, lowercase string, not containing
>> >                 spaces nor dashes, representing the chip name. This is
>> >                 the only mandatory attribute.
>> >                 I2C devices get this attribute created automatically.
>> >                 RO
>>
>> Time to revisit this decision....
>>
>> So, based on the fact that children device names usually contain
>> dashes, I do not understand why hwmon would be any special in this
>> regard. It is possible that the hwmon developers have not faced much
>> MFD situation before, and so, this was not considered to be handled
>> like in other subsystems.
>>
>> I am proposing to change this "rule"...  Any objection?
>
> Prior to proposing such an invasive change which is highly likely to
> come up against heavy opposition,

It is possible that someone does not understand why you think it may
be invasive, right? Could you please explain the reason for that?

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 8:58 AM, Laszlo Papp <lpapp@kde.org> wrote:
> On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
>>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>>> >> > Additionally, dashes are explicitly forbidden in hwmon
>>> >> > device names.
>>> >>
>>> >> Also, where is that documented?
>>> >
>>> > In Documentation/hwmon/sysfs-interface:
>>> >
>>> > *********************
>>> > * Global attributes *
>>> > *********************
>>> >
>>> > name            The chip name.
>>> >                 This should be a short, lowercase string, not containing
>>> >                 spaces nor dashes, representing the chip name. This is
>>> >                 the only mandatory attribute.
>>> >                 I2C devices get this attribute created automatically.
>>> >                 RO
>>>
>>> Time to revisit this decision....
>>>
>>> So, based on the fact that children device names usually contain
>>> dashes, I do not understand why hwmon would be any special in this
>>> regard. It is possible that the hwmon developers have not faced much
>>> MFD situation before, and so, this was not considered to be handled
>>> like in other subsystems.
>>>
>>> I am proposing to change this "rule"...  Any objection?
>>
>> Prior to proposing such an invasive change which is highly likely to
>> come up against heavy opposition,
>
> It is possible that someone does not understand why you think it may
> be invasive, right? Could you please explain the reason for that?

Perhaps you think I would like to refactor all the existing
interfaces? That would be intrusive yes, but changing rules is
orthogonal to keeping compatibility in my opinion. It would be
possible to sanitize this for the feature and make consistent with
other subsystems while keeping the old drivers around as they are.

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 8:58 AM, Laszlo Papp <lpapp@kde.org> wrote:
> On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
>>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>>> >> > Additionally, dashes are explicitly forbidden in hwmon
>>> >> > device names.
>>> >>
>>> >> Also, where is that documented?
>>> >
>>> > In Documentation/hwmon/sysfs-interface:
>>> >
>>> > *********************
>>> > * Global attributes *
>>> > *********************
>>> >
>>> > name            The chip name.
>>> >                 This should be a short, lowercase string, not containing
>>> >                 spaces nor dashes, representing the chip name. This is
>>> >                 the only mandatory attribute.
>>> >                 I2C devices get this attribute created automatically.
>>> >                 RO
>>>
>>> Time to revisit this decision....
>>>
>>> So, based on the fact that children device names usually contain
>>> dashes, I do not understand why hwmon would be any special in this
>>> regard. It is possible that the hwmon developers have not faced much
>>> MFD situation before, and so, this was not considered to be handled
>>> like in other subsystems.
>>>
>>> I am proposing to change this "rule"...  Any objection?
>>
>> Prior to proposing such an invasive change which is highly likely to
>> come up against heavy opposition,
>
> It is possible that someone does not understand why you think it may
> be invasive, right? Could you please explain the reason for that?

Perhaps you think I would like to refactor all the existing
interfaces? That would be intrusive yes, but changing rules is
orthogonal to keeping compatibility in my opinion. It would be
possible to sanitize this for the feature and make consistent with
other subsystems while keeping the old drivers around as they are.

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

PiA+PiA+PiBPbiBNb24sIEZlYiAxMCwgMjAxNCBhdCA0OjM4IFBNLCBKZWFuIERlbHZhcmUgPGpk
ZWx2YXJlQHN1c2UuZGU+IHdyb3RlOgo+ID4+ID4+ID4gQWRkaXRpb25hbGx5LCBkYXNoZXMgYXJl
IGV4cGxpY2l0bHkgZm9yYmlkZGVuIGluIGh3bW9uCj4gPj4gPj4gPiBkZXZpY2UgbmFtZXMuCj4g
Pj4gPj4KPiA+PiA+PiBBbHNvLCB3aGVyZSBpcyB0aGF0IGRvY3VtZW50ZWQ/Cj4gPj4gPgo+ID4+
ID4gSW4gRG9jdW1lbnRhdGlvbi9od21vbi9zeXNmcy1pbnRlcmZhY2U6Cj4gPj4gPgo+ID4+ID4g
KioqKioqKioqKioqKioqKioqKioqCj4gPj4gPiAqIEdsb2JhbCBhdHRyaWJ1dGVzICoKPiA+PiA+
ICoqKioqKioqKioqKioqKioqKioqKgo+ID4+ID4KPiA+PiA+IG5hbWUgICAgICAgICAgICBUaGUg
Y2hpcCBuYW1lLgo+ID4+ID4gICAgICAgICAgICAgICAgIFRoaXMgc2hvdWxkIGJlIGEgc2hvcnQs
IGxvd2VyY2FzZSBzdHJpbmcsIG5vdCBjb250YWluaW5nCj4gPj4gPiAgICAgICAgICAgICAgICAg
c3BhY2VzIG5vciBkYXNoZXMsIHJlcHJlc2VudGluZyB0aGUgY2hpcCBuYW1lLiBUaGlzIGlzCj4g
Pj4gPiAgICAgICAgICAgICAgICAgdGhlIG9ubHkgbWFuZGF0b3J5IGF0dHJpYnV0ZS4KPiA+PiA+
ICAgICAgICAgICAgICAgICBJMkMgZGV2aWNlcyBnZXQgdGhpcyBhdHRyaWJ1dGUgY3JlYXRlZCBh
dXRvbWF0aWNhbGx5Lgo+ID4+ID4gICAgICAgICAgICAgICAgIFJPCj4gPj4KPiA+PiBUaW1lIHRv
IHJldmlzaXQgdGhpcyBkZWNpc2lvbi4uLi4KPiA+Pgo+ID4+IFNvLCBiYXNlZCBvbiB0aGUgZmFj
dCB0aGF0IGNoaWxkcmVuIGRldmljZSBuYW1lcyB1c3VhbGx5IGNvbnRhaW4KPiA+PiBkYXNoZXMs
IEkgZG8gbm90IHVuZGVyc3RhbmQgd2h5IGh3bW9uIHdvdWxkIGJlIGFueSBzcGVjaWFsIGluIHRo
aXMKPiA+PiByZWdhcmQuIEl0IGlzIHBvc3NpYmxlIHRoYXQgdGhlIGh3bW9uIGRldmVsb3BlcnMg
aGF2ZSBub3QgZmFjZWQgbXVjaAo+ID4+IE1GRCBzaXR1YXRpb24gYmVmb3JlLCBhbmQgc28sIHRo
aXMgd2FzIG5vdCBjb25zaWRlcmVkIHRvIGJlIGhhbmRsZWQKPiA+PiBsaWtlIGluIG90aGVyIHN1
YnN5c3RlbXMuCj4gPj4KPiA+PiBJIGFtIHByb3Bvc2luZyB0byBjaGFuZ2UgdGhpcyAicnVsZSIu
Li4gIEFueSBvYmplY3Rpb24/Cj4gPgo+ID4gUHJpb3IgdG8gcHJvcG9zaW5nIHN1Y2ggYW4gaW52
YXNpdmUgY2hhbmdlIHdoaWNoIGlzIGhpZ2hseSBsaWtlbHkgdG8KPiA+IGNvbWUgdXAgYWdhaW5z
dCBoZWF2eSBvcHBvc2l0aW9uLAo+IAo+IEl0IGlzIHBvc3NpYmxlIHRoYXQgc29tZW9uZSBkb2Vz
IG5vdCB1bmRlcnN0YW5kIHdoeSB5b3UgdGhpbmsgaXQgbWF5Cj4gYmUgaW52YXNpdmUsIHJpZ2h0
PyBDb3VsZCB5b3UgcGxlYXNlIGV4cGxhaW4gdGhlIHJlYXNvbiBmb3IgdGhhdD8KClRoZSByZWFz
b24gaXMgYSBnb29kIG9uZS4gSW4gdGhlIGtlcm5lbCB3ZSBtYWtlIGV2ZXJ5IGF0dGVtcHQgbm90
IHRvCmJyZWFrIHVzZXJzcGFjZS4gQnkgdGhhdCBJIG1lYW4gX2FueV8gdXNlcnNwYWNlIGFwcGxp
Y2F0aW9uLiBVc2Vyc3BhY2UKYXBwbGljYXRpb25zIHdoaWNoIGludGVyZmFjZSB3aXRoIHRoZSBr
ZXJuZWwgY2FuIGRvIHNvIHZpYSBhIHZhcmlldHkgb2YKbWV0aG9kcy4gT25lIG9mIHRob3NlIGlz
IFN5c2ZzLCB3aGVyZSB0aGlzIG5hbWUgeW91IGFyZSBhdHRlbXB0aW5nIHRvCmNoYW5nZSBhcHBl
YXJzLiBUaGUgdXNlcnNwYWNlIGFwcGxpY2F0aW9ucyBhbHJlYWR5IG1lbnRpb25lZCBwYXJzZSBm
b3IKdGhlc2UgZGV2aWNlcywgcmVnZXg6aW5nIGZvciAnLScgc2VwYXJhdG9ycy4gSWYgeW91IGFk
ZCBhbiBhZGRpdGlvbmFsCictJyBzZXBhcmF0b3IgdGhlcmUgaXMgYSBjaGFuY2UgdGhhdCB0aGVz
ZSBhcHBsaWNhdGlvbnMgd2lsbCBnZXQKY29uZnVzZWQgYW5kIGJyZWFrIHdpdGhvdXQgd2Fybmlu
Zy4gCgotLSAKTGVlIEpvbmVzCkxpbmFybyBTVE1pY3JvZWxlY3Ryb25pY3MgTGFuZGluZyBUZWFt
IExlYWQKTGluYXJvLm9yZyDilIIgT3BlbiBzb3VyY2Ugc29mdHdhcmUgZm9yIEFSTSBTb0NzCkZv
bGxvdyBMaW5hcm86IEZhY2Vib29rIHwgVHdpdHRlciB8IEJsb2cKCl9fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxtLXNlbnNvcnMgbWFpbGluZyBsaXN0Cmxt
LXNlbnNvcnNAbG0tc2Vuc29ycy5vcmcKaHR0cDovL2xpc3RzLmxtLXNlbnNvcnMub3JnL21haWxt
YW4vbGlzdGluZm8vbG0tc2Vuc29ycw=

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Lee Jones]

> >> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
> >> >> > Additionally, dashes are explicitly forbidden in hwmon
> >> >> > device names.
> >> >>
> >> >> Also, where is that documented?
> >> >
> >> > In Documentation/hwmon/sysfs-interface:
> >> >
> >> > *********************
> >> > * Global attributes *
> >> > *********************
> >> >
> >> > name            The chip name.
> >> >                 This should be a short, lowercase string, not containing
> >> >                 spaces nor dashes, representing the chip name. This is
> >> >                 the only mandatory attribute.
> >> >                 I2C devices get this attribute created automatically.
> >> >                 RO
> >>
> >> Time to revisit this decision....
> >>
> >> So, based on the fact that children device names usually contain
> >> dashes, I do not understand why hwmon would be any special in this
> >> regard. It is possible that the hwmon developers have not faced much
> >> MFD situation before, and so, this was not considered to be handled
> >> like in other subsystems.
> >>
> >> I am proposing to change this "rule"...  Any objection?
> >
> > Prior to proposing such an invasive change which is highly likely to
> > come up against heavy opposition,
> 
> It is possible that someone does not understand why you think it may
> be invasive, right? Could you please explain the reason for that?

The reason is a good one. In the kernel we make every attempt not to
break userspace. By that I mean _any_ userspace application. Userspace
applications which interface with the kernel can do so via a variety of
methods. One of those is Sysfs, where this name you are attempting to
change appears. The userspace applications already mentioned parse for
these devices, regex:ing for '-' separators. If you add an additional
'-' separator there is a chance that these applications will get
confused and break without warning. 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

T24gVHVlLCBGZWIgMTEsIDIwMTQgYXQgOTo0NyBBTSwgTGVlIEpvbmVzIDxsZWUuam9uZXNAbGlu
YXJvLm9yZz4gd3JvdGU6Cj4+ID4+ID4+IE9uIE1vbiwgRmViIDEwLCAyMDE0IGF0IDQ6MzggUE0s
IEplYW4gRGVsdmFyZSA8amRlbHZhcmVAc3VzZS5kZT4gd3JvdGU6Cj4+ID4+ID4+ID4gQWRkaXRp
b25hbGx5LCBkYXNoZXMgYXJlIGV4cGxpY2l0bHkgZm9yYmlkZGVuIGluIGh3bW9uCj4+ID4+ID4+
ID4gZGV2aWNlIG5hbWVzLgo+PiA+PiA+Pgo+PiA+PiA+PiBBbHNvLCB3aGVyZSBpcyB0aGF0IGRv
Y3VtZW50ZWQ/Cj4+ID4+ID4KPj4gPj4gPiBJbiBEb2N1bWVudGF0aW9uL2h3bW9uL3N5c2ZzLWlu
dGVyZmFjZToKPj4gPj4gPgo+PiA+PiA+ICoqKioqKioqKioqKioqKioqKioqKgo+PiA+PiA+ICog
R2xvYmFsIGF0dHJpYnV0ZXMgKgo+PiA+PiA+ICoqKioqKioqKioqKioqKioqKioqKgo+PiA+PiA+
Cj4+ID4+ID4gbmFtZSAgICAgICAgICAgIFRoZSBjaGlwIG5hbWUuCj4+ID4+ID4gICAgICAgICAg
ICAgICAgIFRoaXMgc2hvdWxkIGJlIGEgc2hvcnQsIGxvd2VyY2FzZSBzdHJpbmcsIG5vdCBjb250
YWluaW5nCj4+ID4+ID4gICAgICAgICAgICAgICAgIHNwYWNlcyBub3IgZGFzaGVzLCByZXByZXNl
bnRpbmcgdGhlIGNoaXAgbmFtZS4gVGhpcyBpcwo+PiA+PiA+ICAgICAgICAgICAgICAgICB0aGUg
b25seSBtYW5kYXRvcnkgYXR0cmlidXRlLgo+PiA+PiA+ICAgICAgICAgICAgICAgICBJMkMgZGV2
aWNlcyBnZXQgdGhpcyBhdHRyaWJ1dGUgY3JlYXRlZCBhdXRvbWF0aWNhbGx5Lgo+PiA+PiA+ICAg
ICAgICAgICAgICAgICBSTwo+PiA+Pgo+PiA+PiBUaW1lIHRvIHJldmlzaXQgdGhpcyBkZWNpc2lv
bi4uLi4KPj4gPj4KPj4gPj4gU28sIGJhc2VkIG9uIHRoZSBmYWN0IHRoYXQgY2hpbGRyZW4gZGV2
aWNlIG5hbWVzIHVzdWFsbHkgY29udGFpbgo+PiA+PiBkYXNoZXMsIEkgZG8gbm90IHVuZGVyc3Rh
bmQgd2h5IGh3bW9uIHdvdWxkIGJlIGFueSBzcGVjaWFsIGluIHRoaXMKPj4gPj4gcmVnYXJkLiBJ
dCBpcyBwb3NzaWJsZSB0aGF0IHRoZSBod21vbiBkZXZlbG9wZXJzIGhhdmUgbm90IGZhY2VkIG11
Y2gKPj4gPj4gTUZEIHNpdHVhdGlvbiBiZWZvcmUsIGFuZCBzbywgdGhpcyB3YXMgbm90IGNvbnNp
ZGVyZWQgdG8gYmUgaGFuZGxlZAo+PiA+PiBsaWtlIGluIG90aGVyIHN1YnN5c3RlbXMuCj4+ID4+
Cj4+ID4+IEkgYW0gcHJvcG9zaW5nIHRvIGNoYW5nZSB0aGlzICJydWxlIi4uLiAgQW55IG9iamVj
dGlvbj8KPj4gPgo+PiA+IFByaW9yIHRvIHByb3Bvc2luZyBzdWNoIGFuIGludmFzaXZlIGNoYW5n
ZSB3aGljaCBpcyBoaWdobHkgbGlrZWx5IHRvCj4+ID4gY29tZSB1cCBhZ2FpbnN0IGhlYXZ5IG9w
cG9zaXRpb24sCj4+Cj4+IEl0IGlzIHBvc3NpYmxlIHRoYXQgc29tZW9uZSBkb2VzIG5vdCB1bmRl
cnN0YW5kIHdoeSB5b3UgdGhpbmsgaXQgbWF5Cj4+IGJlIGludmFzaXZlLCByaWdodD8gQ291bGQg
eW91IHBsZWFzZSBleHBsYWluIHRoZSByZWFzb24gZm9yIHRoYXQ/Cj4KPiBUaGUgcmVhc29uIGlz
IGEgZ29vZCBvbmUuIEluIHRoZSBrZXJuZWwgd2UgbWFrZSBldmVyeSBhdHRlbXB0IG5vdCB0bwo+
IGJyZWFrIHVzZXJzcGFjZS4gQnkgdGhhdCBJIG1lYW4gX2FueV8gdXNlcnNwYWNlIGFwcGxpY2F0
aW9uLiBVc2Vyc3BhY2UKPiBhcHBsaWNhdGlvbnMgd2hpY2ggaW50ZXJmYWNlIHdpdGggdGhlIGtl
cm5lbCBjYW4gZG8gc28gdmlhIGEgdmFyaWV0eSBvZgo+IG1ldGhvZHMuIE9uZSBvZiB0aG9zZSBp
cyBTeXNmcywgd2hlcmUgdGhpcyBuYW1lIHlvdSBhcmUgYXR0ZW1wdGluZyB0bwo+IGNoYW5nZSBh
cHBlYXJzLiBUaGUgdXNlcnNwYWNlIGFwcGxpY2F0aW9ucyBhbHJlYWR5IG1lbnRpb25lZCBwYXJz
ZSBmb3IKPiB0aGVzZSBkZXZpY2VzLCByZWdleDppbmcgZm9yICctJyBzZXBhcmF0b3JzLiBJZiB5
b3UgYWRkIGFuIGFkZGl0aW9uYWwKPiAnLScgc2VwYXJhdG9yIHRoZXJlIGlzIGEgY2hhbmNlIHRo
YXQgdGhlc2UgYXBwbGljYXRpb25zIHdpbGwgZ2V0Cj4gY29uZnVzZWQgYW5kIGJyZWFrIHdpdGhv
dXQgd2FybmluZy4KCk9ubHkgZm9yIG5ldyBkZXZpY2VzIHRoYXQgdGhleSBoYXZlIG5vdCBzdXBw
b3J0ZWQgYmVmb3JlLCByaWdodD8KUHJvdmlkZWQsIHdlIGFwcGx5IHRoZSBsb2dpYyBvbmx5IGZv
ciBuZXcgZHJpdmVycy4KCklzIGl0IHVuYWNjZXB0YWJsZSB0byBoYXZlIGFuIGludGVyaW0gIm9i
c29sZXRlIiBwZXJpb2QgZm9yIGEgd2hpbGUKYW5kIHRoZSByZWNvbW1lbmRhdGlvbiBpcyB0aGUg
ZGFzaCBmb3IgdGhlIGxvbmcgZnV0dXJlPwoKQWxzbywgaG93IGFib3V0IGFwcGxpY2F0aW9ucyB0
aGF0IHdvdWxkIGV4cGVjdCBod21vbiB0byBiZWhhdmUgdGhlCnNhbWUgd2F5IGxpa2UgYWxsIHRo
ZSByZXN0PyBUaGV5IGFsc28gYnJlYWsgd2l0aCB0aGUgY3VycmVudCBod21vbgpzY2hlbWEsIHJp
Z2h0PwoKPgo+IC0tCj4gTGVlIEpvbmVzCj4gTGluYXJvIFNUTWljcm9lbGVjdHJvbmljcyBMYW5k
aW5nIFRlYW0gTGVhZAo+IExpbmFyby5vcmcg4pSCIE9wZW4gc291cmNlIHNvZnR3YXJlIGZvciBB
Uk0gU29Dcwo+IEZvbGxvdyBMaW5hcm86IEZhY2Vib29rIHwgVHdpdHRlciB8IEJsb2cKCl9fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxtLXNlbnNvcnMgbWFp
bGluZyBsaXN0CmxtLXNlbnNvcnNAbG0tc2Vuc29ycy5vcmcKaHR0cDovL2xpc3RzLmxtLXNlbnNv
cnMub3JnL21haWxtYW4vbGlzdGluZm8vbG0tc2Vuc29ycw=

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 9:47 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> >> >> > Additionally, dashes are explicitly forbidden in hwmon
>> >> >> > device names.
>> >> >>
>> >> >> Also, where is that documented?
>> >> >
>> >> > In Documentation/hwmon/sysfs-interface:
>> >> >
>> >> > *********************
>> >> > * Global attributes *
>> >> > *********************
>> >> >
>> >> > name            The chip name.
>> >> >                 This should be a short, lowercase string, not containing
>> >> >                 spaces nor dashes, representing the chip name. This is
>> >> >                 the only mandatory attribute.
>> >> >                 I2C devices get this attribute created automatically.
>> >> >                 RO
>> >>
>> >> Time to revisit this decision....
>> >>
>> >> So, based on the fact that children device names usually contain
>> >> dashes, I do not understand why hwmon would be any special in this
>> >> regard. It is possible that the hwmon developers have not faced much
>> >> MFD situation before, and so, this was not considered to be handled
>> >> like in other subsystems.
>> >>
>> >> I am proposing to change this "rule"...  Any objection?
>> >
>> > Prior to proposing such an invasive change which is highly likely to
>> > come up against heavy opposition,
>>
>> It is possible that someone does not understand why you think it may
>> be invasive, right? Could you please explain the reason for that?
>
> The reason is a good one. In the kernel we make every attempt not to
> break userspace. By that I mean _any_ userspace application. Userspace
> applications which interface with the kernel can do so via a variety of
> methods. One of those is Sysfs, where this name you are attempting to
> change appears. The userspace applications already mentioned parse for
> these devices, regex:ing for '-' separators. If you add an additional
> '-' separator there is a chance that these applications will get
> confused and break without warning.

Only for new devices that they have not supported before, right?
Provided, we apply the logic only for new drivers.

Is it unacceptable to have an interim "obsolete" period for a while
and the recommendation is the dash for the long future?

Also, how about applications that would expect hwmon to behave the
same way like all the rest? They also break with the current hwmon
schema, right?

>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> >> > Additionally, dashes are explicitly forbidden in hwmon
>> >> > device names.
>> >>
>> >> Also, where is that documented?
>> >
>> > In Documentation/hwmon/sysfs-interface:
>> >
>> > *********************
>> > * Global attributes *
>> > *********************
>> >
>> > name            The chip name.
>> >                 This should be a short, lowercase string, not containing
>> >                 spaces nor dashes, representing the chip name. This is
>> >                 the only mandatory attribute.
>> >                 I2C devices get this attribute created automatically.
>> >                 RO
>>
>> Time to revisit this decision....
>>
>> So, based on the fact that children device names usually contain
>> dashes, I do not understand why hwmon would be any special in this
>> regard. It is possible that the hwmon developers have not faced much
>> MFD situation before, and so, this was not considered to be handled
>> like in other subsystems.
>>
>> I am proposing to change this "rule"...  Any objection?
>
> Prior to proposing such an invasive change which is highly likely to
> come up against heavy opposition, why don't you try to work _with_ the
> current ruling and the Maintainers to see what others have done to
> solve the problem.  I highly doubt you are the first/only developer who
> has had this issue.
>
> Do:
>   `git grep "\-hwmon"`
>
> ... and have a good look around for an accepted solution.

Well, this did not bring up relevant results, at least for my
understanding, but I will send a new patch with the
devm_hwmon_device_register_
with_groups() stuff based on my little understanding without further help.

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>> >> > Additionally, dashes are explicitly forbidden in hwmon
>> >> > device names.
>> >>
>> >> Also, where is that documented?
>> >
>> > In Documentation/hwmon/sysfs-interface:
>> >
>> > *********************
>> > * Global attributes *
>> > *********************
>> >
>> > name            The chip name.
>> >                 This should be a short, lowercase string, not containing
>> >                 spaces nor dashes, representing the chip name. This is
>> >                 the only mandatory attribute.
>> >                 I2C devices get this attribute created automatically.
>> >                 RO
>>
>> Time to revisit this decision....
>>
>> So, based on the fact that children device names usually contain
>> dashes, I do not understand why hwmon would be any special in this
>> regard. It is possible that the hwmon developers have not faced much
>> MFD situation before, and so, this was not considered to be handled
>> like in other subsystems.
>>
>> I am proposing to change this "rule"...  Any objection?
>
> Prior to proposing such an invasive change which is highly likely to
> come up against heavy opposition, why don't you try to work _with_ the
> current ruling and the Maintainers to see what others have done to
> solve the problem.  I highly doubt you are the first/only developer who
> has had this issue.
>
> Do:
>   `git grep "\-hwmon"`
>
> ... and have a good look around for an accepted solution.

Well, this did not bring up relevant results, at least for my
understanding, but I will send a new patch with the
devm_hwmon_device_register_
with_groups() stuff based on my little understanding without further help.

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 10:22 AM, Laszlo Papp <lpapp@kde.org> wrote:
> On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
>>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>>> >> > Additionally, dashes are explicitly forbidden in hwmon
>>> >> > device names.
>>> >>
>>> >> Also, where is that documented?
>>> >
>>> > In Documentation/hwmon/sysfs-interface:
>>> >
>>> > *********************
>>> > * Global attributes *
>>> > *********************
>>> >
>>> > name            The chip name.
>>> >                 This should be a short, lowercase string, not containing
>>> >                 spaces nor dashes, representing the chip name. This is
>>> >                 the only mandatory attribute.
>>> >                 I2C devices get this attribute created automatically.
>>> >                 RO
>>>
>>> Time to revisit this decision....
>>>
>>> So, based on the fact that children device names usually contain
>>> dashes, I do not understand why hwmon would be any special in this
>>> regard. It is possible that the hwmon developers have not faced much
>>> MFD situation before, and so, this was not considered to be handled
>>> like in other subsystems.
>>>
>>> I am proposing to change this "rule"...  Any objection?
>>
>> Prior to proposing such an invasive change which is highly likely to
>> come up against heavy opposition, why don't you try to work _with_ the
>> current ruling and the Maintainers to see what others have done to
>> solve the problem.  I highly doubt you are the first/only developer who
>> has had this issue.
>>
>> Do:
>>   `git grep "\-hwmon"`
>>
>> ... and have a good look around for an accepted solution.
>
> Well, this did not bring up relevant results, at least for my
> understanding, but I will send a new patch with the
> devm_hwmon_device_register_
> with_groups() stuff based on my little understanding without further help.

https://lkml.org/lkml/2014/2/11/155

but I do not know how to enable the different devices conditionally.
What is the best practice for this in the kernel? (We can continue it
in there)

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix

[Laszlo Papp]

On Tue, Feb 11, 2014 at 10:22 AM, Laszlo Papp <lpapp@kde.org> wrote:
> On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
>>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare <jdelvare@suse.de> wrote:
>>> >> > Additionally, dashes are explicitly forbidden in hwmon
>>> >> > device names.
>>> >>
>>> >> Also, where is that documented?
>>> >
>>> > In Documentation/hwmon/sysfs-interface:
>>> >
>>> > *********************
>>> > * Global attributes *
>>> > *********************
>>> >
>>> > name            The chip name.
>>> >                 This should be a short, lowercase string, not containing
>>> >                 spaces nor dashes, representing the chip name. This is
>>> >                 the only mandatory attribute.
>>> >                 I2C devices get this attribute created automatically.
>>> >                 RO
>>>
>>> Time to revisit this decision....
>>>
>>> So, based on the fact that children device names usually contain
>>> dashes, I do not understand why hwmon would be any special in this
>>> regard. It is possible that the hwmon developers have not faced much
>>> MFD situation before, and so, this was not considered to be handled
>>> like in other subsystems.
>>>
>>> I am proposing to change this "rule"...  Any objection?
>>
>> Prior to proposing such an invasive change which is highly likely to
>> come up against heavy opposition, why don't you try to work _with_ the
>> current ruling and the Maintainers to see what others have done to
>> solve the problem.  I highly doubt you are the first/only developer who
>> has had this issue.
>>
>> Do:
>>   `git grep "\-hwmon"`
>>
>> ... and have a good look around for an accepted solution.
>
> Well, this did not bring up relevant results, at least for my
> understanding, but I will send a new patch with the
> devm_hwmon_device_register_
> with_groups() stuff based on my little understanding without further help.

https://lkml.org/lkml/2014/2/11/155

but I do not know how to enable the different devices conditionally.
What is the best practice for this in the kernel? (We can continue it
in there)