[PATCH 1/6] iio: accel: HID: hid-sensor-accel-3d: Refactor channel initialization

[Marcelo Schmitt]

On 05/08, Natália Salvino André wrote:
> Clean up the channel initialization loop and replace the local
> accel_3d_adjust_channel_bit_mask() function with a compound literal.

> This follows the maintainer's feedback to use direct struct
> initialization
This can go after the '---'.

> 
> Signed-off-by: Natália Salvino André <natalia.andre em ime.usp.br>
> Co-developed-by: Pietro Di Consolo Gregorio <pietro.gregorio em usp.br>
> Signed-off-by: Pietro Di Consolo Gregorio <pietro.gregorio em usp.br>
> ---
Add the maintainer feedback comment here as part of a change log.

>  drivers/iio/accel/hid-sensor-accel-3d.c | 27 +++++++++----------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> index 2ff591b3458f..99ca67732bbe 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
> @@ -3,6 +3,7 @@
>   * HID Sensors Driver
>   * Copyright (c) 2012, Intel Corporation.
>   */
> +#include <linux/bitops.h>
>  #include <linux/device.h>
>  #include <linux/platform_device.h>
>  #include <linux/module.h>
> @@ -119,17 +120,6 @@ static const struct iio_chan_spec gravity_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP),
>  };
>  
> -/* Adjust channel real bits based on report descriptor */
> -static void accel_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
> -						int channel, int size)
> -{
> -	channels[channel].scan_type.sign = 's';
> -	/* Real storage bits will change based on the report desc. */
> -	channels[channel].scan_type.realbits = size * 8;
> -	/* Maximum size of a sample to capture is u32 */
> -	channels[channel].scan_type.storagebits = sizeof(u32) * 8;
> -}
> -
>  /* Channel read_raw handler */
>  static int accel_3d_read_raw(struct iio_dev *indio_dev,
>  			      struct iio_chan_spec const *chan,
> @@ -297,19 +287,20 @@ static int accel_3d_parse_report(struct platform_device *pdev,
>  				struct accel_3d_state *st)
>  {
>  	int ret;
> -	int i;
>  
> -	for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) {
> +	for (unsigned int ch = CHANNEL_SCAN_INDEX_X; ch <= CHANNEL_SCAN_INDEX_Z; ch++) {
>  		ret = sensor_hub_input_get_attribute_info(hsdev,
>  				HID_INPUT_REPORT,
>  				usage_id,
> -				HID_USAGE_SENSOR_ACCEL_X_AXIS + i,
> -				&st->accel[CHANNEL_SCAN_INDEX_X + i]);
> +				ch,
> +				&st->accel[ch]);
Hmm, there's an entire line just for 'ch,' ?!
IMO, it would look more concise as
		ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
							  usage_id, ch, &st->accel[ch]);

it's probably okay going a bit over 80 columns in this case.

>  		if (ret < 0)
>  			break;
> -		accel_3d_adjust_channel_bit_mask(channels,
> -				CHANNEL_SCAN_INDEX_X + i,
> -				st->accel[CHANNEL_SCAN_INDEX_X + i].size);
> +		channels[ch].scan_type = (struct iio_scan_type) {
> +			.format = 's',
> +			.realbits = BYTES_TO_BITS(st->accel[ch].size),
> +			.storagebits = BITS_PER_TYPE(u32),
> +		};
>  	}
>  	dev_dbg(&pdev->dev, "accel_3d %x:%x, %x:%x, %x:%x\n",
>  			st->accel[0].index,
> -- 
> 2.51.0
> 
> -- 
> kernel mailing list
> kernel em lists.ime.usp.br
> https://lists.ime.usp.br/listinfo/kernel