Re: [PATCH v3 2/2] hwmon: (asus_wmi_sensors) Support X370 Asus WMI.

["Thomas Weißschuh" <thomas@t-8ch.de>]

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@vger.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.

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.

In any case I think it would make sense to have some sort of written and
well-known policy about this, though.

> > 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.
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.

> > 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