Re: [PATCH 2/3] IIO: hid-sensor-als: Use generic usage

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 17 Nov 2022 00:19:46 +0100
Philipp Jungkamp <p.jungkamp@gmx.net> wrote:

> Use a generic 'hsdev->usage' instead of the HID_USAGE_SENSOR_ALS to
> allow this driver to drive the Lenvo custom ambient light sensor,

Lenovo

> which is registered under a 'custom' usage and not HID_USAGE_SENSOR_ALS.
> 
> Add the Lenovo Intelligent Sensing Solution (LISS) ambient light sensor
> to the platform device ids.
> 
> Signed-off-by: Philipp Jungkamp <p.jungkamp@gmx.net>

It's a small patch, but in the ideal world, first patch should be a noop
patch that makes the hid-sensor-als stuff able to use a provided
usage ID.  Second patch then adds the new one.

> ---
>  drivers/hid/hid-sensor-custom.c    | 13 +++++++++++++
>  drivers/iio/light/hid-sensor-als.c | 24 +++++++++++++++++-------
>  2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
> index cb21f9178830..124493b8abaf 100644
> --- a/drivers/hid/hid-sensor-custom.c
> +++ b/drivers/hid/hid-sensor-custom.c
> @@ -782,6 +782,19 @@ static const struct hid_sensor_custom_match hid_sensor_custom_known_table[] = {
>  		.luid = "020B000000000000",
>  		.manufacturer = "INTEL",
>  	},
> +	/*
> +	 * Lenovo Intelligent Sensing Solution (LISS)
> +	 */
> +	{	/* ambient light */
> +		.tag = "LISS",
> +		.luid = "0041010200000082",
> +		.model = "STK3X3X Sensor",
> +		.manufacturer = "Vendor 258",

They should definitely rebrand their laptops as this :)


> +		.check_dmi = true,
> +		.dmi.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		}
> +	},
>  	{}
>  };
> 
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index 5a1a625d8d16..0ada158582a9 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -86,6 +86,7 @@ static int als_read_raw(struct iio_dev *indio_dev,
>  			      long mask)
>  {
>  	struct als_state *als_state = iio_priv(indio_dev);
> +	struct hid_sensor_hub_device *hsdev = als_state->common_attributes.hsdev;
>  	int report_id = -1;
>  	u32 address;
>  	int ret_type;
> @@ -110,8 +111,9 @@ static int als_read_raw(struct iio_dev *indio_dev,
>  			hid_sensor_power_state(&als_state->common_attributes,
>  						true);
>  			*val = sensor_hub_input_attr_get_raw_value(
> -					als_state->common_attributes.hsdev,
> -					HID_USAGE_SENSOR_ALS, address,
> +					hsdev,
> +					hsdev->usage,
> +					address,
>  					report_id,
>  					SENSOR_HUB_SYNC,
>  					min < 0);
> @@ -260,7 +262,7 @@ static int als_parse_report(struct platform_device *pdev,
>  			st->als_illum.report_id);
> 
>  	st->scale_precision = hid_sensor_format_scale(
> -				HID_USAGE_SENSOR_ALS,
> +				usage_id,
>  				&st->als_illum,
>  				&st->scale_pre_decml, &st->scale_post_decml);
> 
> @@ -285,7 +287,8 @@ static int hid_als_probe(struct platform_device *pdev)
>  	als_state->common_attributes.hsdev = hsdev;
>  	als_state->common_attributes.pdev = pdev;
> 
> -	ret = hid_sensor_parse_common_attributes(hsdev, HID_USAGE_SENSOR_ALS,
> +	ret = hid_sensor_parse_common_attributes(hsdev,
> +					hsdev->usage,
>  					&als_state->common_attributes,
>  					als_sensitivity_addresses,
>  					ARRAY_SIZE(als_sensitivity_addresses));
> @@ -303,7 +306,8 @@ static int hid_als_probe(struct platform_device *pdev)
> 
>  	ret = als_parse_report(pdev, hsdev,
>  			       (struct iio_chan_spec *)indio_dev->channels,
> -			       HID_USAGE_SENSOR_ALS, als_state);
> +			       hsdev->usage,
> +			       als_state);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to setup attributes\n");
>  		return ret;
> @@ -333,7 +337,8 @@ static int hid_als_probe(struct platform_device *pdev)
>  	als_state->callbacks.send_event = als_proc_event;
>  	als_state->callbacks.capture_sample = als_capture_sample;
>  	als_state->callbacks.pdev = pdev;
> -	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_ALS,
> +	ret = sensor_hub_register_callback(hsdev,
> +					hsdev->usage,
>  					&als_state->callbacks);

Where possible, put multiple params on a line up to the 80 chars limit.
If it improves readability for some reason to go a bit beyond 80 chars,
that is fine too.

>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "callback reg failed\n");
> @@ -356,7 +361,8 @@ static int hid_als_remove(struct platform_device *pdev)
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct als_state *als_state = iio_priv(indio_dev);
> 
> -	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_ALS);
> +	sensor_hub_remove_callback(hsdev,
> +				hsdev->usage);

Oddly short line wrap.

>  	iio_device_unregister(indio_dev);
>  	hid_sensor_remove_trigger(indio_dev, &als_state->common_attributes);
> 
> @@ -368,6 +374,10 @@ static const struct platform_device_id hid_als_ids[] = {
>  		/* Format: HID-SENSOR-usage_id_in_hex_lowercase */
>  		.name = "HID-SENSOR-200041",
>  	},
> +	{
> +		/* Format: HID-SENSOR-custom_sensor_tag-usage_id_in_hex_lowercase */
> +		.name = "HID-SENSOR-LISS-0041",
> +	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(platform, hid_als_ids);
> --
> 2.38.1
>