Thanks Henrique
On 6/1/2020 6:42 PM, Henrique de Moraes Holschuh wrote:
On Mon, 01 Jun 2020, Mark Pearson wrote:
Newer Lenovo Thinkpad platforms have support to identify whether the
system is on-lap or not using an ACPI DYTC event from the firmware.
This patch provides the ability to retrieve the current mode via sysfs
entrypoints and will be used by userspace for thermal mode and WWAN
functionality
Co-developed-by: Nitin Joshi <njoshi1@xxxxxxxxxx>
Signed-off-by: Nitin Joshi <njoshi1@xxxxxxxxxx>
Reviewed-by: Sugumaran <slacshiminar@xxxxxxxxxx>
Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx>
We need to take this through the kernel platform-driver-x86 ML.
It would be nice to have standard events for "latop on a surface you
don't want to burn ("lap"), and the input people might want to suggest
something, too, so I'd also ask the input maintainer.
> Please resend, cc to:
platform-driver-x86@xxxxxxxxxxxxxxx
No problem. Always happy to have more input and I'll do that with the
updated version after fixing the below.
While at it there is something I noticed right away:
+static int dytc_command(int command)
+{
+ acpi_handle dytc_handle;
+ int output;
+
+ if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) {
+ pr_warn("Thinkpad ACPI has no DYTC interface.\n");
+ return -ENODEV;
+ }
+ if (!acpi_evalf(dytc_handle, &output, NULL, "dd", command))
+ return -EIO;
+ return output;
+}
dytc_command() is called by dytc_lapmode_get():
+static int dytc_lapmode_get(void)
+{
+ int output;
+
+ output = dytc_command(DYTC_CMD_GET);
+ if ((output == -ENODEV) || (output == -EIO))
+ return output;
+
+ return ((output >> DYTC_GET_LAPMODE_SHIFT) &
+ DYTC_GET_ENABLE_MASK);
+}
Which is used by dytc_lapmode_init():
+static int tpacpi_dytc_init(struct ibm_init_struct *iibm)
+{
+ int res;
+
+ dytc_available = false;
+ dytc_lapmode = dytc_lapmode_get();
+
+ if (dytc_lapmode < 0 && dytc_lapmode != -ENODEV)
+ return dytc_lapmode;
+
+ dytc_available = true;
+
+ res = sysfs_create_group(&tpacpi_pdev->dev.kobj,
+ &dytc_attr_group);
+
+ return res;
+}
Looks like this code flow is going to spam people with pr_warn() on an
older thinkpad laptop that doesn't have DYTC. Please fix this, it is
not strange for an older thinkpad to not have DYTC (even if it is a
decade's old thinkpad).
There is a thinkpad-acpi driver-level debug facility you can use to send
developer-level debug info (such as the init function of a subdriver did
not find what it wanted), if you want.
Also, if the code flow goes through dytc_init fine and registers the
sub-driver, you likely don't have to optimize the rest of the code flow
for DYTC disappearing from APCI tables ;-) ENODEV silently would be
fine for something so unlikely to happen.
Agreed - will fix and I'll test on an older platform.
As an aside only a few of last years platforms have this implementation
(and then all of this years) so it would impact a lot of devices. Good
catch.
Thanks
Mark
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
