Re: [lm-sensors] [PATCH] hwmon: (max6650) Add support for gpiodef

[Guenter Roeck <linux@roeck-us.net>]

On 11/22/2013 01:23 AM, Laszlo Papp wrote:
> Just to clarify: you want to have ./gpio/gpio-max6650.c?
>
No, I never said that. I wanted you to register the gpio pins
with the gpio subsystem. I didn't ask you to write a separate
driver for it.

Sure, strictly speaking one could write a top level mfd driver
and separate gpio and hwmon drivers, but at least in my opinion
that would be overkill. I also never suggested this; you brought
the term mfd into the discussion.

Guenter

> On Thu, Nov 21, 2013 at 8:52 PM, Laszlo Papp <lpapp@kde.org> wrote:
>> On Thu, Nov 21, 2013 at 5:34 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>>> On Thu, Nov 21, 2013 at 03:20:34PM +0000, Laszlo Papp wrote:
>>>> One week passed since the initial submit. Any feedback from the
>>>> maintainer who accepts patches for this?
>>>>
>>> Last time I checked that was either Jean Delvare or me.
>>>
>>> As I already told you, I won't accept the patch as-is,
>>> and I told you what would need to be changed to have it accepted.
>>>
>>> In general, we don't support adding non-standard sysfs attributes in hwmon
>>> drivers unless really needed and discussed. As I see it, there is no need
>>> for non-standard sysfs attributes in this driver; you _could_ use
>>> the gpio subsystem. You chose not to and provide non-standard sysfs
>>> attributes instead, essentially duplicating gpio subsystem functionality.
>>
>> MFD != gpio subsystem, but for some reason or another you continuously
>> overlook that. You also disregard what Markus wrote: this change is
>> just following the existing convention in there. Basically, your
>> suggestion would lead to a mixed interface where some feature of the
>> chip is exposed in 3-4 other places, and some centrally and in a
>> compact manner in hwmon.
>>
>>> I see it as even more important to use the gpio subsystem for the intended use
>>> case, which is to use gpio pins for fan control. In that case, providing access
>>> through the gpio subsystem would enable using the gpio-fan driver to actually
>>> control the fans.
>>
>> That is clearly incorrect. To write a proper userspace middleware
>> would imply fishing stuff from several subspaces rather than using the
>> same compact interface. I called that a nightmare from end user point
>> of view.
>>
>>> You may consider that to be personal taste nitpicking. I don't.
>>
>> I consider it worse than nitpicking eventually: imho, it is rejecting
>> a validated feature without providing a better change. It is a shame,
>> but we cannot do anything more at this point to provide remedy here
>> without getting financial loss, further time spent on a full rewrite,
>> and relevant study, etc. The kernel will remain without this feature
>> probably. I see it as a loss/loss for both parties. You will save
>> maintaining it (even though it is me who would probably need to
>> maintain this feature for the next few years...) for the cost of not
>> having the feature at all, most likely.
>>
>> Well, I guess we will need to stick to a more feature-rich forked
>> version for us then.
>
>