Messages in this thread | | | Date | Fri, 22 Nov 2013 06:35:36 -0800 | From | Guenter Roeck <> | Subject | Re: [lm-sensors] [PATCH] hwmon: (max6650) Add support for gpiodef |
| |
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. > >
| |