Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 9:57 AM, Lee Jones 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 10:22 AM, Laszlo Papp wrote: > On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones wrote: >>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: >>> >> > Additionally, dashes are explicitly forbidden in hwmon >>> >> > device names. >>> >> >>> >> Also, where is that documented? >>> > >>> > In Documentation/hwmon/sysfs-interface: >>> > >>> > * >>> > * Global attributes * >>> > * >>> > >>> > nameThe 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) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones wrote: >> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: >> >> > Additionally, dashes are explicitly forbidden in hwmon >> >> > device names. >> >> >> >> Also, where is that documented? >> > >> > In Documentation/hwmon/sysfs-interface: >> > >> > * >> > * Global attributes * >> > * >> > >> > nameThe 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
> >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 9:47 AM, Lee Jones wrote: >> >> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: >> >> >> > Additionally, dashes are explicitly forbidden in hwmon >> >> >> > device names. >> >> >> >> >> >> Also, where is that documented? >> >> > >> >> > In Documentation/hwmon/sysfs-interface: >> >> > >> >> > * >> >> > * Global attributes * >> >> > * >> >> > >> >> > nameThe 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
> >> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: > >> >> > Additionally, dashes are explicitly forbidden in hwmon > >> >> > device names. > >> >> > >> >> Also, where is that documented? > >> > > >> > In Documentation/hwmon/sysfs-interface: > >> > > >> > * > >> > * Global attributes * > >> > * > >> > > >> > nameThe 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 8:58 AM, Laszlo Papp wrote: > On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones wrote: >>> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: >>> >> > Additionally, dashes are explicitly forbidden in hwmon >>> >> > device names. >>> >> >>> >> Also, where is that documented? >>> > >>> > In Documentation/hwmon/sysfs-interface: >>> > >>> > * >>> > * Global attributes * >>> > * >>> > >>> > nameThe 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 8:49 AM, Jean Delvare wrote: > Le Tuesday 11 February 2014 à 08:28 +, Laszlo Papp a écrit : >> On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare wrote: >> > Hi Laszlo, >> > >> > On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote: >> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: >> >> > Additionally, dashes are explicitly forbidden in hwmon >> >> > device names. >> >> >> >> Also, where is that documented? >> > >> > In Documentation/hwmon/sysfs-interface: >> > >> > * >> > * Global attributes * >> > * >> > >> > nameThe 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? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones wrote: >> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: >> >> > Additionally, dashes are explicitly forbidden in hwmon >> >> > device names. >> >> >> >> Also, where is that documented? >> > >> > In Documentation/hwmon/sysfs-interface: >> > >> > * >> > * Global attributes * >> > * >> > >> > nameThe 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? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
> >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: > >> > Additionally, dashes are explicitly forbidden in hwmon > >> > device names. > >> > >> Also, where is that documented? > > > > In Documentation/hwmon/sysfs-interface: > > > > * > > * Global attributes * > > * > > > > nameThe 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Le Tuesday 11 February 2014 à 08:28 +, Laszlo Papp a écrit : > On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare wrote: > > Hi Laszlo, > > > > On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote: > >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: > >> > Additionally, dashes are explicitly forbidden in hwmon > >> > device names. > >> > >> Also, where is that documented? > > > > In Documentation/hwmon/sysfs-interface: > > > > * > > * Global attributes * > > * > > > > nameThe 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare wrote: > Hi Laszlo, > > On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote: >> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote: >> > Additionally, dashes are explicitly forbidden in hwmon >> > device names. >> >> Also, where is that documented? > > In Documentation/hwmon/sysfs-interface: > > * > * Global attributes * > * > > nameThe 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? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare wrote:
> Hi Laszlo,
>
> On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote:
>> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote:
>> > Additionally, dashes are explicitly forbidden in hwmon
>> > device names.
>>
>> Also, where is that documented?
>
> In Documentation/hwmon/sysfs-interface:
>
> *
> * Global attributes *
> *
>
> nameThe 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
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare jdelv...@suse.de wrote:
Hi Laszlo,
On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote:
On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote:
Additionally, dashes are explicitly forbidden in hwmon
device names.
Also, where is that documented?
In Documentation/hwmon/sysfs-interface:
*
* Global attributes *
*
nameThe 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
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare jdelv...@suse.de wrote: Hi Laszlo, On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote: On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Le Tuesday 11 February 2014 à 08:28 +, Laszlo Papp a écrit : On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare jdelv...@suse.de wrote: Hi Laszlo, On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote: On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones lee.jo...@linaro.org wrote: On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 8:49 AM, Jean Delvare jdelv...@suse.de wrote: Le Tuesday 11 February 2014 à 08:28 +, Laszlo Papp a écrit : On Tue, Feb 11, 2014 at 7:50 AM, Jean Delvare jdelv...@suse.de wrote: Hi Laszlo, On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote: On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 8:58 AM, Laszlo Papp lp...@kde.org wrote: On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones lee.jo...@linaro.org wrote: On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 9:47 AM, Lee Jones lee.jo...@linaro.org wrote: On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones lee.jo...@linaro.org wrote: On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 10:22 AM, Laszlo Papp lp...@kde.org wrote: On Tue, Feb 11, 2014 at 8:50 AM, Lee Jones lee.jo...@linaro.org wrote: On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote: Additionally, dashes are explicitly forbidden in hwmon device names. Also, where is that documented? In Documentation/hwmon/sysfs-interface: * * Global attributes * * nameThe 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) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 9:57 AM, Lee Jones lee.jo...@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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Hi Laszlo,
On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote:
> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote:
> > Additionally, dashes are explicitly forbidden in hwmon
> > device names.
>
> Also, where is that documented?
In Documentation/hwmon/sysfs-interface:
*
* Global attributes *
*
nameThe 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 3:23 AM, Laszlo Papp wrote: > On Mon, Feb 10, 2014 at 11:10 PM, Guenter Roeck wrote: >> On Mon, Feb 10, 2014 at 06:59:55PM +, 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.) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 11:10 PM, Guenter Roeck wrote: > On Mon, Feb 10, 2014 at 06:59:55PM +, 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! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 06:59:55PM +, Laszlo Papp wrote: > On Mon, Feb 10, 2014 at 4:53 PM, wrote: > > Quoting Jean Delvare : > > > >> > >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 4:53 PM, wrote: > Quoting Jean Delvare : > >> >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, 10 Feb 2014 18:27:07 +, Laszlo Papp wrote: > On Mon, Feb 10, 2014 at 5:43 PM, Jean Delvare wrote: > > On Mon, 10 Feb 2014 16:58:53 +, 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
> > > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, 10 Feb 2014 18:01:59 +, 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 5:06 PM, Laszlo Papp wrote:
> On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote:
>> Hi Lee, Laszlo,
>>
>> On Mon, 10 Feb 2014 16:08:42 +, 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
>>> > ---
>>> > 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare wrote:
> Hi Lee, Laszlo,
>
> On Mon, 10 Feb 2014 16:08:42 +, 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
>> > ---
>> > 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 ...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
> > > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Quoting Jean Delvare : 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Hi Lee, Laszlo,
On Mon, 10 Feb 2014 16:08:42 +, 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
> > ---
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Hi Lee, Laszlo,
On Mon, 10 Feb 2014 16:08:42 +, 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 lp...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Quoting Jean Delvare jdelv...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote:
Hi Lee, Laszlo,
On Mon, 10 Feb 2014 16:08:42 +, 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 lp...@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 ...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 5:06 PM, Laszlo Papp lp...@kde.org wrote:
On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote:
Hi Lee, Laszlo,
On Mon, 10 Feb 2014 16:08:42 +, 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 lp...@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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, 10 Feb 2014 18:01:59 +, 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, 10 Feb 2014 18:27:07 +, Laszlo Papp wrote: On Mon, Feb 10, 2014 at 5:43 PM, Jean Delvare jdelv...@suse.de wrote: On Mon, 10 Feb 2014 16:58:53 +, 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 4:53 PM, li...@roeck-us.net wrote: Quoting Jean Delvare jdelv...@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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 06:59:55PM +, Laszlo Papp wrote: On Mon, Feb 10, 2014 at 4:53 PM, li...@roeck-us.net wrote: Quoting Jean Delvare jdelv...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Mon, Feb 10, 2014 at 11:10 PM, Guenter Roeck li...@roeck-us.net wrote: On Mon, Feb 10, 2014 at 06:59:55PM +, 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! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
On Tue, Feb 11, 2014 at 3:23 AM, Laszlo Papp lp...@kde.org wrote: On Mon, Feb 10, 2014 at 11:10 PM, Guenter Roeck li...@roeck-us.net wrote: On Mon, Feb 10, 2014 at 06:59:55PM +, 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.) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [lm-sensors] [PATCH] hwmon: (max6650) Rename the device ids to contain the hwmon suffix
Hi Laszlo,
On Tue, 11 Feb 2014 03:13:37 +, Laszlo Papp wrote:
On Mon, Feb 10, 2014 at 4:38 PM, Jean Delvare jdelv...@suse.de wrote:
Additionally, dashes are explicitly forbidden in hwmon
device names.
Also, where is that documented?
In Documentation/hwmon/sysfs-interface:
*
* Global attributes *
*
nameThe 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
