mbox series

[ASUS,EC,Sensors,v8,0/3]

Message ID 20220124015658.687309-1-eugene.shalygin@gmail.com (mailing list archive)
Headers show
Series [ASUS,EC,Sensors,v8,1/3] hwmon: (asus-ec-sensors) add driver for ASUS EC | expand

Message

Eugene Shalygin Jan. 24, 2022, 1:56 a.m. UTC
This patchset replaces the HWMON asus_wmi_ec_sensors driver with
an implementation that does not use WMI but queries the embedded
controller directly.

That provides two enhancements: sensor reading became quicker (on some
systems or kernel configuration it took almost a full second to read
all the sensors, that transfers less than 15 bytes of data), the driver
became more fexible. The driver now relies on ACPI mutex to lock access
to the EC, in the same way as the WMI DSDT code does.

Changes in v8:
 - Fixed formatting (removed doc comments, removed redundant new line).
 - Changed hwmon device name (asus_ec_sensors -> asusec) removing
	 unnecessary "sensors" word.

Changes in v7:
 - Add suport for the ROG STRIX X570-F GAMING board.
 - Add the __init attribute to two more functions.

Changes in v6:
 - Fixed hwmon device name replacing dashes with underscores.
 - Removed module verion.
 - Fixed condition for asus_wmi_ec_Sensors in KBuild.

Changes in v5:
 - Place the sensors bitset directly into the driver_data field of the
   dmi_system_id struct.
 - Replace doc comments with regular ones.

Changes in v4:
 - Deprecate the wmi driver rather than removing it.

Changes in v3:
 - Remove BIOS version checks and BIOS version dependent mutex path.

Changes in v2:
 - Replace sensor flags enum with bitset
 - Replace module init/probe functions with module_platform_driver_probe
   and ask the platform drivers framework to load the driver when ACPI
   EC is found (ACPI ID "PNP0C09").
 - Extend board data with BIOS version attribute for the mutex path to be
   BIOS version dependent.
 - Add module parameter to override the mutex path.

Eugene Shalygin (3):
  hwmon: (asus-ec-sensors) add driver for ASUS EC
  hwmon: (asus-ec-sensors) update documentation
  hwmon: deprecate asis_wmi_ec_sensors driver

 Documentation/hwmon/asus_ec_sensors.rst     |  52 ++
 Documentation/hwmon/asus_wmi_ec_sensors.rst |  38 --
 MAINTAINERS                                 |   6 +
 drivers/hwmon/Kconfig                       |  16 +-
 drivers/hwmon/Makefile                      |   1 +
 drivers/hwmon/asus-ec-sensors.c             | 694 ++++++++++++++++++++
 6 files changed, 768 insertions(+), 39 deletions(-)
 create mode 100644 Documentation/hwmon/asus_ec_sensors.rst
 delete mode 100644 Documentation/hwmon/asus_wmi_ec_sensors.rst
 create mode 100644 drivers/hwmon/asus-ec-sensors.c

Comments

Eugene Shalygin Feb. 2, 2022, 11:58 p.m. UTC | #1
On Mon, 24 Jan 2022 at 02:57, Eugene Shalygin <eugene.shalygin@gmail.com> wrote:
>
> This patchset replaces the HWMON asus_wmi_ec_sensors driver with
> an implementation that does not use WMI but queries the embedded
> controller directly.

Günter, I would like to add support for one more board model. What
should I do? Another version update or could you, please, merge this
patchset already?

Thank you,
Eugene
Guenter Roeck Feb. 3, 2022, 1:10 a.m. UTC | #2
On 2/2/22 15:58, Eugene Shalygin wrote:
> On Mon, 24 Jan 2022 at 02:57, Eugene Shalygin <eugene.shalygin@gmail.com> wrote:
>>
>> This patchset replaces the HWMON asus_wmi_ec_sensors driver with
>> an implementation that does not use WMI but queries the embedded
>> controller directly.
> 
> Günter, I would like to add support for one more board model. What
> should I do? Another version update or could you, please, merge this
> patchset already?
> 
> Thank you,
> Eugene

I was waiting for someone to send me a Tested-by: for the series,
since you dropped the previous feedback. Presumably that means that
the changes from previous versions warrants another round of testing
and/or review.

Guenter
Eugene Shalygin Feb. 3, 2022, 1:16 a.m. UTC | #3
> I was waiting for someone to send me a Tested-by: for the series,

Oleksandr sent an informal one already.

> since you dropped the previous feedback.

Does it mean it is possible to update patches while keeping it?

Eugene
Guenter Roeck Feb. 3, 2022, 3:41 a.m. UTC | #4
On 2/2/22 17:16, Eugene Shalygin wrote:
>> I was waiting for someone to send me a Tested-by: for the series,
> 
> Oleksandr sent an informal one already.
> 

He wrote:

"Given minor changes against v7, I think my "Tested-by:" should have been preserved."

which doesn't mean he tested again, only that in his opinion
the tags should have been preserved.

>> since you dropped the previous feedback.
> 
> Does it mean it is possible to update patches while keeping it?
> 

See above. You are the one to make the call, and you made the call that
the series should be re-tested.

That means that I am left with either accepting the series without any
Tested-by: and/or Reviewed-by: tags, or I have to wait for some. I guess
you are telling me that I won't get any additional tags, so I'll have to
go in myself and have a closer look. I'll try to do that in the next week
or two.

Guenter
Eugene Shalygin Feb. 3, 2022, 3:48 a.m. UTC | #5
> > Oleksandr sent an informal one already.
> >
>
> He wrote:
>
> "Given minor changes against v7, I think my "Tested-by:" should have been preserved."
>
> which doesn't mean he tested again, only that in his opinion
> the tags should have been preserved.

Oleksandre, could you, please, let us know did you actually test the
v8 code and if so provide us with the Tested-by: tag?

> That means that I am left with either accepting the series without any
> Tested-by: and/or Reviewed-by: tags, or I have to wait for some. I guess
> you are telling me that I won't get any additional tags, so I'll have to
> go in myself and have a closer look. I'll try to do that in the next week
> or two.

Thank you, I understand now. If Oleksandr does not reply in a few days
I will send the update with another board
(fully duplicating information for its base variant), tested by a
Libre Hardware Monitor user.

Best regards,
Eugene
Oleksandr Natalenko Feb. 3, 2022, 7:09 a.m. UTC | #6
Hello.

On čtvrtek 3. února 2022 4:48:53 CET Eugene Shalygin wrote:
> > > Oleksandr sent an informal one already.
> > >
> >
> > He wrote:
> >
> > "Given minor changes against v7, I think my "Tested-by:" should have been preserved."
> >
> > which doesn't mean he tested again, only that in his opinion
> > the tags should have been preserved.

This is not what I meant, but my wording could be better, yes.

BTW, the changes were not of that kind to drop Tested-by: tag, really.

> Oleksandre, could you, please, let us know did you actually test the
> v8 code and if so provide us with the Tested-by: tag?

Yes, I do run this version now, and it works fine for me.

Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>

> > That means that I am left with either accepting the series without any
> > Tested-by: and/or Reviewed-by: tags, or I have to wait for some. I guess
> > you are telling me that I won't get any additional tags, so I'll have to
> > go in myself and have a closer look. I'll try to do that in the next week
> > or two.
> 
> Thank you, I understand now. If Oleksandr does not reply in a few days
> I will send the update with another board
> (fully duplicating information for its base variant), tested by a
> Libre Hardware Monitor user.

Thanks.

> Best regards,
> Eugene
>
Guenter Roeck Feb. 3, 2022, 6:54 p.m. UTC | #7
On 2/2/22 23:09, Oleksandr Natalenko wrote:
> Hello.
> 
> On čtvrtek 3. února 2022 4:48:53 CET Eugene Shalygin wrote:
>>>> Oleksandr sent an informal one already.
>>>>
>>>
>>> He wrote:
>>>
>>> "Given minor changes against v7, I think my "Tested-by:" should have been preserved."
>>>
>>> which doesn't mean he tested again, only that in his opinion
>>> the tags should have been preserved.
> 
> This is not what I meant, but my wording could be better, yes.
> 
> BTW, the changes were not of that kind to drop Tested-by: tag, really.
> 
>> Oleksandre, could you, please, let us know did you actually test the
>> v8 code and if so provide us with the Tested-by: tag?
> 
> Yes, I do run this version now, and it works fine for me.
> 
> Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>
> 

Ok, based on that I'll apply the series on top of hwmon-next with your
Tested-by:.

Guenter

>>> That means that I am left with either accepting the series without any
>>> Tested-by: and/or Reviewed-by: tags, or I have to wait for some. I guess
>>> you are telling me that I won't get any additional tags, so I'll have to
>>> go in myself and have a closer look. I'll try to do that in the next week
>>> or two.
>>
>> Thank you, I understand now. If Oleksandr does not reply in a few days
>> I will send the update with another board
>> (fully duplicating information for its base variant), tested by a
>> Libre Hardware Monitor user.
> 
> Thanks.
> 
>> Best regards,
>> Eugene
>>
>
Eugene Shalygin Feb. 3, 2022, 8:01 p.m. UTC | #8
> >> Oleksandre, could you, please, let us know did you actually test the
> >> v8 code and if so provide us with the Tested-by: tag?
> >
> > Yes, I do run this version now, and it works fine for me.
> >
> > Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>
> >
>
> Ok, based on that I'll apply the series on top of hwmon-next with your
> Tested-by:.
>

Great! Thank you both!

Eugene
Denis Pauk Feb. 3, 2022, 8:24 p.m. UTC | #9
On Thu, 3 Feb 2022 21:01:32 +0100
Eugene Shalygin <eugene.shalygin@gmail.com> wrote:

> > >> Oleksandre, could you, please, let us know did you actually test
> > >> the v8 code and if so provide us with the Tested-by: tag?  
> > >
> > > Yes, I do run this version now, and it works fine for me.
> > >
> > > Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>
> > >  
> >
> > Ok, based on that I'll apply the series on top of hwmon-next with
> > your Tested-by:.
> >  
> 
> Great! Thank you both!
> 
> Eugene

I have also retested code, it works for my case.

Tested-by: Denis Pauk <pauk.denis@gmail.com>

What about other B550/X570 boards?

We have such candidates with same WMI methods in nct6775:
	"ROG STRIX B550-A GAMING",
	"ROG STRIX B550-E GAMING",
	"ROG STRIX B550-F GAMING",
	"ROG STRIX B550-F GAMING (WI-FI)",
	"ROG STRIX B550-I GAMING",

B550-A does not support asus-wmi-ec-interface("ERROR: Can't get value
of subfeature fan1_input: I/O error"), but what about others? 

Best regards,
             Denis.
Eugene Shalygin Feb. 3, 2022, 8:55 p.m. UTC | #10
> I have also retested code, it works for my case.
>
> Tested-by: Denis Pauk <pauk.denis@gmail.com>

Thanks!

> What about other B550/X570 boards?
>
> We have such candidates with same WMI methods in nct6775:
>         "ROG STRIX B550-A GAMING",
>         "ROG STRIX B550-E GAMING",
>         "ROG STRIX B550-F GAMING",
>         "ROG STRIX B550-F GAMING (WI-FI)",
>         "ROG STRIX B550-I GAMING",
>
> B550-A does not support asus-wmi-ec-interface("ERROR: Can't get value
> of subfeature fan1_input: I/O error"), but what about others?

I don't have DSDT for these boards, but most of them have the
T_Sensor, according to the specs at the ASUS web-site, so, probably,
their EC should report something, because most of their boards report
T_Sensor reading via the EC.

Best regards,
Eugene
Eugene Shalygin Feb. 4, 2022, 8:13 p.m. UTC | #11
> What about other B550/X570 boards?
My previous reply is incorrect, in fact we already have information
for some of them, it is just me who can't remember or distinguish
those board names.

> We have such candidates with same WMI methods in nct6775:
>         "ROG STRIX B550-A GAMING",
No data.

>         "ROG STRIX B550-E GAMING",
Already included.

>         "ROG STRIX B550-F GAMING",
No data, the X570-F differs significantly from X570-E, maybe this one
is not like other B550 models too.

>         "ROG STRIX B550-F GAMING (WI-FI)",
Probably is identical to the non-wifi model.

>         "ROG STRIX B550-I GAMING",
Already included.

Best regards,
Eugene