Date:   Sun, 10 Oct 2021 09:21:03 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Thomas Weißschuh <thomas@...ch.de>
Cc:     Denis Pauk <pauk.denis@...il.com>, eugene.shalygin@...il.com,
        andy.shevchenko@...il.com, Ed Brindley <kernel@...davale.org>,
        Jean Delvare <jdelvare@...e.com>,
        Jonathan Corbet <corbet@....net>, linux-hwmon@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] hwmon: (asus_wmi_sensors) Support X370 Asus WMI.

On 10/10/21 8:28 AM, Thomas Weißschuh wrote:
> On 2021-10-10T07:56-0700, Guenter Roeck wrote:
>> On 10/10/21 7:10 AM, Thomas Weißschuh wrote:
>>> On 2021-10-10T06:38-0700, Guenter Roeck wrote:
>>>> On 10/10/21 3:20 AM, Thomas Weißschuh wrote:
>>>>> Hi,
>>>>>
>>>>> for WMI drivers the list platform-driver-x86@...r.kernel.org should probably be
>>>>> on CC too.
>>>>> Also all other WMI drivers, even for hwmon stuff are located in
>>>>> drivers/platform/x86 so it may be better to put it there, too.
>>>>>
>>>>
>>>> Not really. If any of those other drivers are pure hwmon drivers, they
>>>> should reside in drivers/hwmon instead. And, yes, that really includes
>>>> the gigabyte-wmi driver. We don't have arbitrary drivers in drivers/pci
>>>> either just because they are drivers for pci devices.
>>>
>>> Fair enough.
>>> I suppose it would be too much churn to move gigabyte-wmi to
>>> hwmon now though, correct?
>>>
>>
>> Is it ? I don't recall the reason why it was added to drivers/platform/x86
>> in the first place. I see other single-use wmi drivers in that directory
>> as well (eg xiaomi-wmi, which should be in input). Is there some unwritten
>> rule stating that all wmi drivers shall reside in drivers/platform/x86,
>> no matter what subsystem they touch ?
> 
> There was no specific reason. I saw all the other WMI drivers in
> drivers/platform/x86 and added mine there and sent it to the recipients as
> reported by get_maintainer.pl.
> 

Sorry, that is not a valid reason or argument for me.

> You mentioned that it could move to hwmon but Hans said there are other
> single-use wmi drivers in drivers/platform/x86 so I left it as is.
> 
> If you want me to move it, I'd be happy to do so.
> 

At this point this is out of my control.

> In any case I think it would make sense to have some sort of written and
> well-known policy about this, though.
> 
Absolutely agree. Historically single-use drivers resided in subsystem directories,
and for multi-use drivers it was handled on a case-by-case basis. If that is being
changed (meaning subsystem driver control/maintenance/review is taken away from
subsystem maintainers), it should for sure be documented.

>>> Having the platform-driver-x86 on Cc would still be useful as they can provide
>>> guidance about using the ACPI/WMI/platform APIs.
>>>
>>
>> Sure, but that is unrelated to the driver location, and the opposite argument
>> can be made as well (that drivers implementing subsystem code should be reviewed
>> by subsystem maintainers). That is a much stronger argument in my opinion.
>>
>> Guenter
> 
> Absolutely. I wanted to make two different points in my mail:
> 
> 1) Maybe the driver should be moved into drivers/platform/x86 as the other
> (single-use) WMI drivers are living there.

Doing something wrong is neither a reason nor an argument to keep doing it.

> I don't know about any rule demanding that but was mentioning this so it stays
> consistent.
> 
> 2) The patch should *also* be reviewed by pdx86 as it is using their
> infrastructure.
> This was not meant to replace any of the hwmon involvement.
> 
> For example when I submitted gigabyte-wmi to pdx86 the maintainers they told
> me to also solicit feedback from you as the hwmon maintainer.
> And in the end both the hwmon parts (thank you!) and the wmi parts
> (platform-device vs WMI bus, same as here) were much better than in the
> first version.
> 

Sure. Unfortunately that is not always the case. Many drivers registering
with hwmon were never reviewed by a hwmon maintainer, and some of those make
me shiver when I look at them. Since v5.12, get_maintainer.pl lists hwmon
maintainers as reviewers if a driver makes a hwmon API call, so hopefully
that will help a bit.

Guenter

>>> For example by using the WMI bus as mentioned in my other mail would allow
>>> to completely remove the manually maintained DMI list and instead directly bind
>>> to the WMI GUID for any device that supports this GUID.
>>> (This is possible as this WMI API seems to be self-describing, so all
>>> specific parameters can be discovered by the driver)
> 
> Thomas
> 

Powered by blists - more mailing lists