Linux Input/HID development
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Sanjay Chitroda via B4 Relay
	<devnull+sanjayembeddedse.gmail.com@kernel.org>
Cc: sanjayembeddedse@gmail.com,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Jiri Kosina" <jikos@kernel.org>,
	"Srinivas Pandruvada" <srinivas.pandruvada@linux.intel.com>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-input@vger.kernel.org
Subject: Re: [PATCH v2 2/6] iio: hid-sensors: align function parenthesis for readability
Date: Thu, 2 Jul 2026 18:20:15 +0100	[thread overview]
Message-ID: <20260702182015.303db93c@jic23-huawei> (raw)
In-Reply-To: <20260702-15-jun-hid-iio-alignment-v2-2-b87f01f5efbc@gmail.com>

On Thu, 02 Jul 2026 21:47:59 +0530
Sanjay Chitroda via B4 Relay <devnull+sanjayembeddedse.gmail.com@kernel.org> wrote:

> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> 
> Adjust alignment of parentheses across HID sensor IIO drivers to
> improve readability and maintain consistency with kernel coding style.
> 
> While updating the formatting, group related arguments consistently in
> multi-line function signatures where appropriate.
> 
> No functional change intended.
> 
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Hi Sanjay,

Whilst I appreciate this code isn't quite in line with standards
and usually like that stuff to be fixed up, in this particular case
this is a massive amount of churn.  That churn will make backporting
fixes etc messier, so I'd like input on whether others consider this
one worthwhile.  Jiri, Srinivas, Andy etc. What do you think?

A few comments on inconsistencies inline.

Jonathan

> ---
>  drivers/iio/accel/hid-sensor-accel-3d.c            | 46 ++++++-----
>  .../iio/common/hid-sensors/hid-sensor-attributes.c | 88 +++++++++++-----------
>  .../iio/common/hid-sensors/hid-sensor-trigger.c    |  2 +-
>  .../iio/common/hid-sensors/hid-sensor-trigger.h    |  2 +-
>  drivers/iio/gyro/hid-sensor-gyro-3d.c              | 54 ++++++-------
>  drivers/iio/humidity/hid-sensor-humidity.c         | 47 ++++++------
>  drivers/iio/light/hid-sensor-als.c                 | 32 ++++----
>  drivers/iio/light/hid-sensor-prox.c                | 29 +++----
>  drivers/iio/magnetometer/hid-sensor-magn-3d.c      | 74 +++++++++---------
>  drivers/iio/orientation/hid-sensor-incl-3d.c       | 31 ++++----
>  drivers/iio/orientation/hid-sensor-rotation.c      | 24 +++---
>  .../iio/position/hid-sensor-custom-intel-hinge.c   | 13 ++--
>  drivers/iio/pressure/hid-sensor-press.c            | 35 ++++-----
>  drivers/iio/temperature/hid-sensor-temperature.c   | 38 +++++-----
>  14 files changed, 241 insertions(+), 274 deletions(-)


> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> index 2f0a1ea42f48..13fa55e966f4 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
...

>  }
>  
>  int hid_sensor_format_scale(u32 usage_id,
> -			struct hid_sensor_hub_attribute_info *attr_info,
> -			int *val0, int *val1)
> +			    struct hid_sensor_hub_attribute_info *attr_info,
> +			    int *val0, int *val1)
>  {
>  	int i;
>  	int exp;
> @@ -414,9 +413,8 @@ int hid_sensor_format_scale(u32 usage_id,
>  
>  	for (i = 0; i < ARRAY_SIZE(unit_conversion); ++i) {
>  		if (unit_conversion[i].usage_id == usage_id &&
> -			unit_conversion[i].unit == attr_info->units) {
> -			exp  = hid_sensor_convert_exponent(
> -						attr_info->unit_expo);
> +		    unit_conversion[i].unit == attr_info->units) {
> +			exp  = hid_sensor_convert_exponent(attr_info->unit_expo);
>  			adjust_exponent_nano(val0, val1,
>  					unit_conversion[i].scale_val0,
>  					unit_conversion[i].scale_val1, exp);
Why did this one get left alone?

> @@ -511,14 +510,14 @@ int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
>  	hid_sensor_get_reporting_interval(hsdev, usage_id, st);
>  
>  	sensor_hub_input_get_attribute_info(hsdev,
> -					HID_FEATURE_REPORT, usage_id,
> -					HID_USAGE_SENSOR_PROP_REPORT_STATE,
> -					&st->report_state);
> +			HID_FEATURE_REPORT, usage_id,
> +			HID_USAGE_SENSOR_PROP_REPORT_STATE,
> +			&st->report_state);
>  
>  	sensor_hub_input_get_attribute_info(hsdev,
> -					HID_FEATURE_REPORT, usage_id,
> -					HID_USAGE_SENSOR_PROY_POWER_STATE,
> -					&st->power_state);
> +			HID_FEATURE_REPORT, usage_id,
> +			HID_USAGE_SENSOR_PROY_POWER_STATE,
> +			&st->power_state);

See below.

> @@ -526,7 +525,7 @@ int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
>  	sensor_hub_input_get_attribute_info(hsdev,
>  			HID_FEATURE_REPORT, usage_id,
>  			HID_USAGE_SENSOR_PROP_SENSITIVITY_ABS,
> -			 &st->sensitivity);
> +			&st->sensitivity);

Some cases like this one are locally inconsistent so we'd want to fix those
even if leaving the majority alone.

> diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> index bbca2111e79b..0dba475845d7 100644
> --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c

> @@ -240,11 +235,12 @@ static int gyro_3d_parse_report(struct platform_device *pdev,
>  	int ret;
>  
>  	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_ANGL_VELOCITY_X_AXIS + ch,
> -				&st->gyro[ch]);
> +		ret = sensor_hub_input_get_attribute_info(
> +			hsdev,
> +			HID_INPUT_REPORT,
> +			usage_id,
> +			HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXIS + ch,
> +			&st->gyro[ch]);

See below.

> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> index 23884825eb00..ad10fa20fae0 100644
> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
...
> @@ -353,11 +348,12 @@ static int magn_3d_parse_report(struct platform_device *pdev,
>  		u32 address = magn_3d_addresses[i];
>  
>  		/* Check if usage attribute exists in the sensor hub device */
> -		status = sensor_hub_input_get_attribute_info(hsdev,
> -			HID_INPUT_REPORT,
> -			usage_id,
> -			address,
> -			&(st->magn[i]));
> +		status = sensor_hub_input_get_attribute_info(
> +				hsdev,
> +				HID_INPUT_REPORT,
> +				usage_id,
> +				address,
> +				&(st->magn[i]));

See below.

> @@ -442,7 +436,8 @@ static int magn_3d_parse_report(struct platform_device *pdev,
>  			&st->rot_attr.scale_post_decml);
>  
>  	if (st->rot_attributes.sensitivity.index < 0) {
> -		sensor_hub_input_get_attribute_info(hsdev,
> +		sensor_hub_input_get_attribute_info(
> +			hsdev,
>  			HID_FEATURE_REPORT, usage_id,
>  			HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
>  			HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH,
See below.
> @@ -477,11 +472,12 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
>  	magn_state->magn_flux_attributes.hsdev = hsdev;
>  	magn_state->magn_flux_attributes.pdev = pdev;
>  
> -	ret = hid_sensor_parse_common_attributes(hsdev,
> -				HID_USAGE_SENSOR_COMPASS_3D,
> -				&magn_state->magn_flux_attributes,
> -				magn_3d_sensitivity_addresses,
> -				ARRAY_SIZE(magn_3d_sensitivity_addresses));
> +	ret = hid_sensor_parse_common_attributes(
> +		hsdev,
> +		HID_USAGE_SENSOR_COMPASS_3D,
> +		&magn_state->magn_flux_attributes,
> +		magn_3d_sensitivity_addresses,
> +		ARRAY_SIZE(magn_3d_sensitivity_addresses));

See below - the movement of hsdev and the single tab vs two tab
difference was why I pointed out that this patch is inconsistent.
As noted there, I'd be tempted to leave these ones alone, even
if we are cleaning up most of the other indentation.

> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> index a8d3a15f9c53..18da85e6c60e 100644
> --- a/drivers/iio/temperature/hid-sensor-temperature.c
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c

...

> @@ -206,10 +206,10 @@ static int hid_temperature_probe(struct platform_device *pdev)
>  	temp_st->common_attributes.pdev = pdev;
>  
>  	ret = hid_sensor_parse_common_attributes(hsdev,
> -					HID_USAGE_SENSOR_TEMPERATURE,
> -					&temp_st->common_attributes,
> -					temperature_sensitivity_addresses,
> -					ARRAY_SIZE(temperature_sensitivity_addresses));
> +			HID_USAGE_SENSOR_TEMPERATURE,
> +			&temp_st->common_attributes,
> +			temperature_sensitivity_addresses,
> +			ARRAY_SIZE(temperature_sensitivity_addresses));

There aren't hard rules on how to format these cases where a break is needed.
That is kind of reflected in inconsistency in this set.  I'd be tempted
to leave these ones alone.

Jonathan

> 


  reply	other threads:[~2026-07-02 17:20 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-02 16:17 [PATCH v2 0/6] HID: iio: warning clean up and prefer kernel coding style Sanjay Chitroda via B4 Relay
2026-07-02 16:17 ` [PATCH v2 1/6] iio: hid-sensors: add/remove blank line Sanjay Chitroda via B4 Relay
2026-07-02 17:30   ` Jonathan Cameron
2026-07-02 16:17 ` [PATCH v2 2/6] iio: hid-sensors: align function parenthesis for readability Sanjay Chitroda via B4 Relay
2026-07-02 17:20   ` Jonathan Cameron [this message]
2026-07-03 12:52     ` Andy Shevchenko
2026-07-04  1:09       ` srinivas pandruvada
2026-07-02 16:18 ` [PATCH v2 3/6] iio: hid-sensors: Use implicit NULL pointer checks Sanjay Chitroda via B4 Relay
2026-07-02 17:22   ` Jonathan Cameron
2026-07-02 16:18 ` [PATCH v2 4/6] iio: humidity: hid-sensor-humidity: use common device for devres Sanjay Chitroda via B4 Relay
2026-07-02 17:03   ` sashiko-bot
2026-07-02 17:26   ` Jonathan Cameron
2026-07-02 16:18 ` [PATCH v2 5/6] iio: position: hid-sensor-custom-intel-hinge: " Sanjay Chitroda via B4 Relay
2026-07-02 17:18   ` sashiko-bot
2026-07-02 17:26   ` Jonathan Cameron
2026-07-02 16:18 ` [PATCH v2 6/6] iio: temperature: hid-sensor-temperature: " Sanjay Chitroda via B4 Relay
2026-07-02 17:28   ` sashiko-bot
2026-07-02 17:29   ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260702182015.303db93c@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=andy@kernel.org \
    --cc=devnull+sanjayembeddedse.gmail.com@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jikos@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=sanjayembeddedse@gmail.com \
    --cc=srinivas.pandruvada@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox