Hi, On 11/24/20 7:11 PM, Mark Pearson wrote: > Add support to thinkpad_acpi for returning the status of the palm > sensor. > > This patch builds on the work done previously for the input device > implementation (which was not needed). Both lap and palm sensor are using > sysfs and they are combined into the proxsensor block. > > Note: On some platforms, because of an issue in the HW implementation, > the palm sensor presence may be incorrectly advertised as always > enabled even if a palm sensor is not present. The palm sensor is > intended for WWAN transmission power control and should be available > and correct on all WWAN enabled systems. It is not recommended to use > this interface for other use cases. > > Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans > --- > drivers/platform/x86/thinkpad_acpi.c | 162 +++++++++++++++++---------- > 1 file changed, 103 insertions(+), 59 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index e3810675090a..6a4c54db38fb 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -4021,6 +4021,7 @@ static bool hotkey_notify_usrevent(const u32 hkey, > } > > static void thermal_dump_all_sensors(void); > +static void palmsensor_refresh(void); > > static bool hotkey_notify_6xxx(const u32 hkey, > bool *send_acpi_ev, > @@ -4087,8 +4088,8 @@ static bool hotkey_notify_6xxx(const u32 hkey, > > case TP_HKEY_EV_PALM_DETECTED: > case TP_HKEY_EV_PALM_UNDETECTED: > - /* palm detected hovering the keyboard, forward to user-space > - * via netlink for consumption */ > + /* palm detected - pass on to event handler */ > + palmsensor_refresh(); > return true; > > default: > @@ -9828,102 +9829,146 @@ static struct ibm_struct lcdshadow_driver_data = { > }; > > /************************************************************************* > - * DYTC subdriver, for the Lenovo lapmode feature > + * Thinkpad sensor interfaces > */ > > #define DYTC_CMD_GET 2 /* To get current IC function and mode */ > #define DYTC_GET_LAPMODE_BIT 17 /* Set when in lapmode */ > > -static bool dytc_lapmode; > +#define PALMSENSOR_PRESENT_BIT 0 /* Determine if psensor present */ > +#define PALMSENSOR_ON_BIT 1 /* psensor status */ > > -static void dytc_lapmode_notify_change(void) > -{ > - sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_lapmode"); > -} > +static bool has_palmsensor; > +static bool has_lapsensor; > +static bool palm_state; > +static bool lap_state; > > -static int dytc_command(int command, int *output) > +static int lapsensor_get(bool *present, bool *state) > { > acpi_handle dytc_handle; > + int output; > > - if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) { > - /* Platform doesn't support DYTC */ > + *present = false; > + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) > return -ENODEV; > - } > - if (!acpi_evalf(dytc_handle, output, NULL, "dd", command)) > + if (!acpi_evalf(dytc_handle, &output, NULL, "dd", DYTC_CMD_GET)) > return -EIO; > + > + *present = true; /*If we get his far, we have lapmode support*/ > + *state = output & BIT(DYTC_GET_LAPMODE_BIT) ? true : false; > return 0; > } > > -static int dytc_lapmode_get(bool *state) > +static int palmsensor_get(bool *present, bool *state) > { > - int output, err; > + acpi_handle psensor_handle; > + int output; > > - err = dytc_command(DYTC_CMD_GET, &output); > - if (err) > - return err; > - *state = output & BIT(DYTC_GET_LAPMODE_BIT) ? true : false; > + *present = false; > + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "GPSS", &psensor_handle))) > + return -ENODEV; > + if (!acpi_evalf(psensor_handle, &output, NULL, "d")) > + return -EIO; > + > + *present = output & BIT(PALMSENSOR_PRESENT_BIT) ? true : false; > + *state = output & BIT(PALMSENSOR_ON_BIT) ? true : false; > return 0; > } > > -static void dytc_lapmode_refresh(void) > +static void lapsensor_refresh(void) > { > - bool new_state; > + bool state; > int err; > > - err = dytc_lapmode_get(&new_state); > - if (err || (new_state == dytc_lapmode)) > - return; > + if (has_lapsensor) { > + err = lapsensor_get(&has_lapsensor, &state); > + if (err) > + return; > + if (lap_state != state) { > + lap_state = state; > + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "dytc_lapmode"); > + } > + } > +} > > - dytc_lapmode = new_state; > - dytc_lapmode_notify_change(); > +static void palmsensor_refresh(void) > +{ > + bool state; > + int err; > + > + if (has_palmsensor) { > + err = palmsensor_get(&has_palmsensor, &state); > + if (err) > + return; > + if (palm_state != state) { > + palm_state = state; > + sysfs_notify(&tpacpi_pdev->dev.kobj, NULL, "palmsensor"); > + } > + } > } > > -/* sysfs lapmode entry */ > static ssize_t dytc_lapmode_show(struct device *dev, > struct device_attribute *attr, > char *buf) > { > - return snprintf(buf, PAGE_SIZE, "%d\n", dytc_lapmode); > + if (has_lapsensor) > + return sysfs_emit(buf, "%d\n", lap_state); > + return sysfs_emit(buf, "\n"); > } > - > static DEVICE_ATTR_RO(dytc_lapmode); > > -static struct attribute *dytc_attributes[] = { > - &dev_attr_dytc_lapmode.attr, > - NULL, > -}; > - > -static const struct attribute_group dytc_attr_group = { > - .attrs = dytc_attributes, > -}; > +static ssize_t palmsensor_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + if (has_palmsensor) > + return sysfs_emit(buf, "%d\n", palm_state); > + return sysfs_emit(buf, "\n"); > +} > +static DEVICE_ATTR_RO(palmsensor); > > -static int tpacpi_dytc_init(struct ibm_init_struct *iibm) > +static int tpacpi_proxsensor_init(struct ibm_init_struct *iibm) > { > - int err; > + int palm_err, lap_err, err; > > - err = dytc_lapmode_get(&dytc_lapmode); > - /* If support isn't available (ENODEV) then don't return an error > - * but just don't create the sysfs group > + palm_err = palmsensor_get(&has_palmsensor, &palm_state); > + lap_err = lapsensor_get(&has_lapsensor, &lap_state); > + /* > + * If support isn't available (ENODEV) for both devices then quit, but > + * don't return an error. > */ > - if (err == -ENODEV) > + if ((palm_err == -ENODEV) && (lap_err == -ENODEV)) > return 0; > - /* For all other errors we can flag the failure */ > - if (err) > - return err; > - > - /* Platform supports this feature - create the group */ > - err = sysfs_create_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group); > - return err; > + /* Otherwise, if there was an error return it */ > + if (palm_err && (palm_err != ENODEV)) > + return palm_err; > + if (lap_err && (lap_err != ENODEV)) > + return lap_err; > + > + if (has_palmsensor) { > + err = sysfs_create_file(&tpacpi_pdev->dev.kobj, &dev_attr_palmsensor.attr); > + if (err) > + return err; > + } > + if (has_lapsensor) { > + err = sysfs_create_file(&tpacpi_pdev->dev.kobj, &dev_attr_dytc_lapmode.attr); > + if (err) > + return err; > + } > + return 0; > } > > -static void dytc_exit(void) > +static void proxsensor_exit(void) > { > - sysfs_remove_group(&tpacpi_pdev->dev.kobj, &dytc_attr_group); > + if (has_lapsensor) > + sysfs_remove_file(&tpacpi_pdev->dev.kobj, &dev_attr_dytc_lapmode.attr); > + if (has_palmsensor) > + sysfs_remove_file(&tpacpi_pdev->dev.kobj, &dev_attr_palmsensor.attr); > } > > -static struct ibm_struct dytc_driver_data = { > - .name = "dytc", > - .exit = dytc_exit, > +static struct ibm_struct proxsensor_driver_data = { > + .name = "proximity-sensor", > + .exit = proxsensor_exit, > }; > > /**************************************************************************** > @@ -9975,8 +10020,7 @@ static void tpacpi_driver_event(const unsigned int hkey_event) > } > > if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED) > - dytc_lapmode_refresh(); > - > + lapsensor_refresh(); > } > > static void hotkey_driver_event(const unsigned int scancode) > @@ -10416,8 +10460,8 @@ static struct ibm_init_struct ibms_init[] __initdata = { > .data = &lcdshadow_driver_data, > }, > { > - .init = tpacpi_dytc_init, > - .data = &dytc_driver_data, > + .init = tpacpi_proxsensor_init, > + .data = &proxsensor_driver_data, > }, > }; > >