[ASUS EC Sensors v8 0/3]

Eugene Shalygin posted 3 patches 4 years, 5 months ago
There is a newer version of this series
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
[ASUS EC Sensors v8 0/3]
Posted by Eugene Shalygin 4 years, 5 months ago
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

-- 
2.34.1

Re: [ASUS EC Sensors v8 0/3]
Posted by Eugene Shalygin 4 years, 4 months ago
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
Re: [ASUS EC Sensors v8 0/3]
Posted by Guenter Roeck 4 years, 4 months ago
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
Re: [ASUS EC Sensors v8 0/3]
Posted by Eugene Shalygin 4 years, 4 months ago
> 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
Re: [ASUS EC Sensors v8 0/3]
Posted by Guenter Roeck 4 years, 4 months ago
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
Re: [ASUS EC Sensors v8 0/3]
Posted by Eugene Shalygin 4 years, 4 months ago
> > 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
Re: [ASUS EC Sensors v8 0/3]
Posted by Oleksandr Natalenko 4 years, 4 months ago
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
> 

-- 
Oleksandr Natalenko (post-factum)


Re: [ASUS EC Sensors v8 0/3]
Posted by Guenter Roeck 4 years, 4 months ago
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
>>
> 

Re: [ASUS EC Sensors v8 0/3]
Posted by Eugene Shalygin 4 years, 4 months ago
> >> 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
Re: [ASUS EC Sensors v8 0/3]
Posted by Denis Pauk 4 years, 4 months ago
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.
Re: [ASUS EC Sensors v8 0/3]
Posted by Eugene Shalygin 4 years, 4 months ago
> 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
Re: [ASUS EC Sensors v8 0/3]
Posted by Eugene Shalygin 4 years, 4 months ago
> 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