Re: [PATCH] iio: cros_ec: Add lid angle driver

[Lee Jones <lee.jones@linaro.org>]

On Wed, 27 Feb 2019, Gwendal Grignou wrote:

> Add a IIO driver that reports the angle between the lid and the base for
> ChromeOS convertible device.
> 
> Tested on eve with ToT EC firmware.
> Check driver is loaded and lid angle is correct.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
>  drivers/iio/common/cros_ec_sensors/Kconfig    |   9 +
>  drivers/iio/common/cros_ec_sensors/Makefile   |   1 +
>  .../cros_ec_sensors/cros_ec_lid_angle.c       | 164 ++++++++++++++++++

>  drivers/mfd/cros_ec_dev.c                     |  15 +-

It doesn't look like there are build dependencies between the changes
in these different subsystems.  If my assumption is correct, please
pull the MFD changes out into a separate patch.

>  4 files changed, 186 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c

[...]

> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index d275deaecb12..11b5b2fd1f33 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -297,13 +297,17 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>  
>  	resp = (struct ec_response_motion_sense *)msg->data;
>  	sensor_num = resp->dump.sensor_count;
> -	/* Allocate 1 extra sensors in FIFO are needed */
> -	sensor_cells = kcalloc(sensor_num + 1, sizeof(struct mfd_cell),
> +	/*
> +	 * Allocate extra sensors if lid angle sensor or FIFO are needed.
> +	 */
> +#define NUM_EXTRA_SENSORS 2

Please refrain from placing #defines in the middle of functions.

Move it to the top of the file.

> +	sensor_cells = kcalloc(sensor_num + NUM_EXTRA_SENSORS,

Actually, this isn't something that needs defining.

Just change the comment to:

  "Allocate 2 extra sensors if lid angle sensor or FIFO are needed."

... and drop the define.

> +			       sizeof(struct mfd_cell),
>  			       GFP_KERNEL);
>  	if (sensor_cells == NULL)
>  		goto error;
>  
> -	sensor_platforms = kcalloc(sensor_num + 1,
> +	sensor_platforms = kcalloc(sensor_num,
>  				   sizeof(struct cros_ec_sensor_platform),
>  				   GFP_KERNEL);
>  	if (sensor_platforms == NULL)
> @@ -363,6 +367,11 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>  		sensor_cells[id].name = "cros-ec-ring";
>  		id++;
>  	}
> +	if (cros_ec_check_features(ec,
> +				EC_FEATURE_REFINED_TABLET_MODE_HYSTERESIS)) {
> +		sensor_cells[id].name = "cros-ec-lid-angle";
> +		id++;
> +	}
>  
>  	ret = mfd_add_devices(ec->dev, 0, sensor_cells, id,
>  			      NULL, 0, NULL);

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog