Re: [PATCH v2 0/2] Fix MSHW0040 button driver selection on AMD Surface devices

[Antonio Vázquez Blanco]

Hi Andy,

Thanks for the review.

On the tags, agreed. In v3 the acpidumps reference moves into the tag block
as

  Link: https://github.com/linux-surface/acpidumps [1]

and I will keep the v1 thread, marked [2] and referenced from the text,
since Dmitry's review there is where the "0 is a valid revision" design
comes from.

On the documentation: if the maintainers would rather not take it with
b4 shazam -M, I am happy to put the model table in a comment next to the
check in both drivers instead.

I will wait for Dmitry and Hans before sending v3, so it can cover anything
they raise as well.

Thanks!
Antón


El lun, 9 nov 2026 a las 8:10, Andy Shevchenko
(<andy.shevchenko@xxxxxxxxx>) escribió:
>
> On Sun, Nov 8, 2026 at 11:37 PM Antonio Vázquez Blanco
> <antoniovazquezblanco@xxxxxxxxx> wrote:
> >
> > This is a rebase and rework of Sachi King's series from November 2021 [1].
> > It was reviewed then, the review was never acted on, and the thread went
> > quiet in March 2022. Sachi has confirmed they are happy for it to be
> > carried and no longer have the hardware. Both patches keep their original
> > authorship and sign-offs.
> >
> > The problem, restated, because Hans asked for this in [1] and never got a
> > clear answer:
> >
> > The AMD variants of the Surface Laptop 3 and 4 use the same MSHW0040 ACPI
> > device ID as the Intel Surface devices from the 5th generation on. Both
> > soc_button_array and surfacepro3_button match that ID and tell the groups
> > apart inside probe, by evaluating function 2 of the MSHW0040 _DSM, the OEM
> > Platform Revision, and treating a returned 0 as proof that the method is
> > absent. The AMD variants implement that function and legitimately return 0,
> > so both drivers draw the wrong conclusion, and surfacepro3_button binds
> > where soc_button_array should.
> >
> > The sound test is whether the device implements that function at all, which
> > function 0 of the same _DSM advertises and acpi_check_dsm() reports.
> >
> > To answer the question of which models are affected, every ACPI dump in
> > linux-surface/acpidumps [2] carrying MSHW0040 was surveyed, 32 models, for
> > UUID 6fd05c69-cde3-49f4-95ed-ab1665498035 revision 1:
> >
> >   Model                        fn 0 mask fn 2 advertised
> >   Surface Book 1               0x03      no
> >   Surface Book 2               0x0d      yes
> >   Surface Book 3               0x0f      yes
> >   Surface Laptop 1             0x05      yes
> >   Surface Laptop 2             0x05      yes
> >   Surface Laptop 3 AMD         0x0f      yes
> >   Surface Laptop 3 Intel       0x0f      yes
> >   Surface Laptop 4 AMD         0x0f      yes
> >   Surface Laptop 4 Intel       0x0f      yes
> >   Surface Laptop 5             0x0f      yes
> >   Surface Laptop 6             0x0f      yes
> >   Surface Laptop 7 Qualcomm    0x0f      yes
> >   Surface Laptop Go 1          0x0f      yes
> >   Surface Laptop Go 2          0x0f      yes
> >   Surface Laptop Go 3          0x0f      yes
> >   Surface Laptop SE            0x0d      yes
> >   Surface Laptop Studio 1      0x0f      yes
> >   Surface Laptop Studio 2      0x0f      yes
> >   Surface Pro 10 Business      0x0f      yes
> >   Surface Pro 11 Qualcomm      0x0f      yes
> >   Surface Pro 4                no _DSM   no
> >   Surface Pro 5                0x0f      yes
> >   Surface Pro 6                0x0f      yes
> >   Surface Pro 7                0x0f      yes
> >   Surface Pro 7 Plus           0x0f      yes
> >   Surface Pro 8                0x0f      yes
> >   Surface Pro 9 5G             0x0f      yes
> >   Surface Pro 9 Intel          0x0f      yes
> >   Surface Pro X SQ1            0x0f      yes
> >   Surface Pro X SQ2            0x0f      yes
> >   Surface Studio 2             0x0d      yes
> >   Windows Dev Kit 2023         0x0f      yes
> > Exactly two do not advertise function 2, and they are exactly the two that
> > need surfacepro3_button:
> >
> >   - Surface Pro 4: its MSHW0040 device carries no _DSM at all, only _HID
> >     and _STA.
> >   - Surface Book 1: it does carry the _DSM with this UUID, but function 0
> >     returns {0x03,0,0,0}, advertising functions 0 and 1 only. Function 1
> >     there is dGPU base attach and detach handling.
> >
> > Worth noting that v1 said the devices needing surfacepro3_button "do not
> > have the _DSM". That is not true of the Surface Book 1, and the inaccuracy
> > is probably why the problem was hard to follow. The commit messages now
> > describe what is actually tested.
>
> ...
>
> > On merging: please do not separate the two patches. Both drivers match
> > MSHW0040 and discriminate only in probe, so with only one of them applied,
> > which driver claims the device on the AMD variants depends on module load
> > order. Hans asked for the same arrangement for his own two patch series in
> > 2022, so either tree with an ack from the other would work.
>
> If we go with this series close to its current shape, I really ask
> maintainers to use `b4 shazam -M` for it to make a cover letter
> (without list of files and changelogs) be present in the Git history
> (as a "merge" commit message) as it supplies nice piece of
> documentation (see the part above ...). Alternatively we might want to
> see this be listed somewhere in the code as a comment, or in some file
> under Documentation. The rationale is that it is always hard to find
> people who may test something on a variety of the HW and having a
> technical background provide a valuable summary.
>
> --
> With Best Regards,
> Andy Shevchenko