On 9/8/20 8:40 PM, Andy Shevchenko wrote:
On Tue, Sep 8, 2020 at 8:20 PM Maximilian Luz <luzmaximilian@xxxxxxxxx> wrote:
...
+ .gpe_number = 0x17,
+ .gpe_number = 0x4D,
+ .gpe_number = 0x4F,
+ .gpe_number = 0x57,
From where these numbers come from? Can we get them from firmware (ACPI)?
Yes, they are obtained from ACPI/the DSDT. Specifically from the name of
the GPE handler notifying the lid device. See [1] for a repo full of
Surface ACPI dumps (source for this). I'll add a comment pointing this out
in v2.
[1]: https://github.com/linux-surface/acpidumps
...
+static int surface_gpe_probe(struct platform_device *pdev)
+{
+ const struct surface_lid_device *lid;
+ int status;
+
+ lid = dev_get_platdata(&pdev->dev);
+ if (!lid)
+ return -ENODEV;
Can we use software nodes?
As far as I can tell this would work via fwnode_create_software_node /
fwnode_remove_software_node and device properties? I don't seem to find
much documentation on this (there doesn't seem to be an entry for
software nodes in the official docs?), but I think I should be able to
make this work.
+ status = acpi_mark_gpe_for_wake(NULL, lid->gpe_number);
+ if (status) {
+ dev_err(&pdev->dev, "failed to mark GPE for wake: %d\n", status);
+ return -EINVAL;
+ }
+
+ status = acpi_enable_gpe(NULL, lid->gpe_number);
Did I miss anything or all calls of enable / disable GPE are using
NULL as a first parameter? What the point in such case?
As far as I can tell, some of the more generic uses have a non-NULL
gpe_device parameter (acpi/device_pm.c, acpi/wakeup.c) and NULL just
means index-0/main device? Not an expert on that though, so probably
just ignore me here and let the ACPI guys answer this.
...
+MODULE_ALIAS("dmi:*:svnMicrosoftCorporation:pnSurfacePro:*");
+MODULE_ALIAS("dmi:*:svnMicrosoftCorporation:pnSurfacePro4:*");
Can simply
MODULE_ALIAS("dmi:*:svnMicrosoftCorporation:pnSurface*:*");
work?
Depends on your preference, really. That would also auto-load the module
on Surface Pro 3 and earlier devices (just won't do anything on those).
So it's a trade-off between unnecessary loading of the module and
maintainability/readability. Let me know what you prefer and I'll switch
to that.
Style and other issues are noted, I'll fix them for v2.
Regards,
Max