mbox series

[0/3] AMD_SFH: Allow overriding sensor-mask by module-param or DMI-quirk

Message ID 20210128121219.6381-1-hdegoede@redhat.com (mailing list archive)
Headers show
Series AMD_SFH: Allow overriding sensor-mask by module-param or DMI-quirk | expand

Message

Hans de Goede Jan. 28, 2021, 12:12 p.m. UTC
Hi All,

There are several bug-reports about the new AMD_SFH driver not working
on various HP ENVY x360 Convertible models. The problem is that the
driver expects the BIOS to program which sensors are present into the
active bits of the AMD_P2C_MSG3 register; and the BIOS on these models
does not do this:

https://bugzilla.kernel.org/show_bug.cgi?id=199715
https://bugzilla.redhat.com/show_bug.cgi?id=1651886

This patch-set adds a module-parameter + DMI-quirk mechanism to override
the settings read back from the AMD_P2C_MSG3 register, to work around
this problem. The DMI-quirk table is populated with 2 HP ENVY x360
Convertible models which are know to need this workaround.

There also is a much larger refactoring patch-set pending from
Richard Neumann, who is also involved in the bugzilla.kernel.org bug.

But it looks to me like that will need a bit more more work before
it is ready for merging, where as (IMHO) this set is ready for
merging now. So IMHO it would be good to first merge this patch-set
to get this fix into the hands of end-users of these devices.

Note there still is an open issue on these devices where the
sensors stop working after a suspend/resume cycle.

I wonder if the driver should perhaps not only not use the
active bits of the AMD_P2C_MSG3 register for determining which
sensors are there, but if it should actually write to those bots
with the correct settings.

Sandeep, do you have any ideas what might be the problem here?
Should I ask the reporters to test a patch which actually
updates the active bits?

Regards,

Hans



Hans de Goede (3):
  AMD_SFH: Removed unused activecontrolstatus member from the
    amd_mp2_dev struct
  AMD_SFH: Add sensor_mask module parameter
  AMD_SFH: Add DMI quirk table for BIOS-es which don't set the
    activestatus bits

 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 40 ++++++++++++++++++++++++--
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.h |  1 -
 2 files changed, 37 insertions(+), 4 deletions(-)

Comments

Richard Neumann Jan. 28, 2021, 1:24 p.m. UTC | #1
Hi Hans,

sorry, I missed to include you in the recipients of the patch I
submitted yesterday: 
https://patchwork.kernel.org/project/linux-input/list/?series=423055

I was focused on the recipient returned by the get_maintainer.pl
script.
My patch, however, does the DMI matching a little differently and does
not utilize the driver_data field of the DMI IDs.
Also I did not include a patch to add a module parameter.

I have too few knowledge which one is better. I don't think that we
need the module parameter right now, if we have the DMI quirks, but
I'll leave this decision to the experts.

It is true, that my fully refactored patch will need more review, and I
changed some things already since I submitted it. But I will wait for
some comments before I consider submitting it again.

Kind regards,

Richard

Am Donnerstag, dem 28.01.2021 um 13:12 +0100 schrieb Hans de Goede:
> Hi All,
> 
> There are several bug-reports about the new AMD_SFH driver not
> working
> on various HP ENVY x360 Convertible models. The problem is that the
> driver expects the BIOS to program which sensors are present into the
> active bits of the AMD_P2C_MSG3 register; and the BIOS on these
> models
> does not do this:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=199715
> https://bugzilla.redhat.com/show_bug.cgi?id=1651886
> 
> This patch-set adds a module-parameter + DMI-quirk mechanism to
> override
> the settings read back from the AMD_P2C_MSG3 register, to work around
> this problem. The DMI-quirk table is populated with 2 HP ENVY x360
> Convertible models which are know to need this workaround.
> 
> There also is a much larger refactoring patch-set pending from
> Richard Neumann, who is also involved in the bugzilla.kernel.org bug.
> 
> But it looks to me like that will need a bit more more work before
> it is ready for merging, where as (IMHO) this set is ready for
> merging now. So IMHO it would be good to first merge this patch-set
> to get this fix into the hands of end-users of these devices.
> 
> Note there still is an open issue on these devices where the
> sensors stop working after a suspend/resume cycle.
> 
> I wonder if the driver should perhaps not only not use the
> active bits of the AMD_P2C_MSG3 register for determining which
> sensors are there, but if it should actually write to those bots
> with the correct settings.
> 
> Sandeep, do you have any ideas what might be the problem here?
> Should I ask the reporters to test a patch which actually
> updates the active bits?
> 
> Regards,
> 
> Hans
> 
> 
> 
> Hans de Goede (3):
>   AMD_SFH: Removed unused activecontrolstatus member from the
>     amd_mp2_dev struct
>   AMD_SFH: Add sensor_mask module parameter
>   AMD_SFH: Add DMI quirk table for BIOS-es which don't set the
>     activestatus bits
> 
>  drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 40
> ++++++++++++++++++++++++--
>  drivers/hid/amd-sfh-hid/amd_sfh_pcie.h |  1 -
>  2 files changed, 37 insertions(+), 4 deletions(-)
>
Jiri Kosina March 8, 2021, 3:33 p.m. UTC | #2
On Thu, 28 Jan 2021, Hans de Goede wrote:

> Hi All,
> 
> There are several bug-reports about the new AMD_SFH driver not working
> on various HP ENVY x360 Convertible models. The problem is that the
> driver expects the BIOS to program which sensors are present into the
> active bits of the AMD_P2C_MSG3 register; and the BIOS on these models
> does not do this:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=199715
> https://bugzilla.redhat.com/show_bug.cgi?id=1651886
> 
> This patch-set adds a module-parameter + DMI-quirk mechanism to override
> the settings read back from the AMD_P2C_MSG3 register, to work around
> this problem. The DMI-quirk table is populated with 2 HP ENVY x360
> Convertible models which are know to need this workaround.

Applied, thanks.