Re: [PATCH] platform/x86: thinkpad_acpi: Add palm sensor support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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,
>  	},
>  };
>  
> 




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux