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

From: Laszlo Papp
Date: Fri Nov 22 2013 - 04:23:44 EST


Just to clarify: you want to have ./gpio/gpio-max6650.c?

On Thu, Nov 21, 2013 at 8:52 PM, Laszlo Papp <lpapp@xxxxxxx> wrote:
> On Thu, Nov 21, 2013 at 5:34 PM, Guenter Roeck <linux@xxxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/