Linux Media Controller development
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472
@ 2026-08-25 20:35 Sakari Ailus
  2026-08-25 20:35 ` [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print Sakari Ailus
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Sakari Ailus @ 2026-08-25 20:35 UTC (permalink / raw)
  To: linux-media
  Cc: Rafael J. Wysocki, linux-acpi, Len Brown, Daniel Scally,
	Hans de Goede, Ilpo Järvinen, platform-driver-x86

Hi folks,

The two first patches address smatch and compiler warnings in int3472. The
third patch clean up GPIO parsing in the int3472 driver.

since v2:

- Drop the __free(ACPI_FREE) change.

- Move ACPI_FREE() earlier in the function and save the object's integer
  value into a local variable.

- Fix return values -- 0 was returned by v2 in some cases where 1 was the
  right value.

Sakari Ailus (3):
  platform/x86: int3472: Address Coccinelle warning on an error print
  platform/x86: int3472: Fix uninitialised variable warning
  platform/x86: int3472: Clean up GPIO parsing

 drivers/platform/x86/intel/int3472/discrete.c | 51 +++++++++----------
 drivers/platform/x86/intel/int3472/tps68470.c |  2 +-
 2 files changed, 25 insertions(+), 28 deletions(-)


base-commit: 56c29fa3ee666197516a231e75aed789ae9c530d
-- 
2.47.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print
  2026-08-25 20:35 [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472 Sakari Ailus
@ 2026-08-25 20:35 ` Sakari Ailus
  2026-08-25 21:01   ` Ricardo Ribalda Delgado
  2026-08-27  8:25   ` Dan Scally
  2026-08-25 20:35 ` [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning Sakari Ailus
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: Sakari Ailus @ 2026-08-25 20:35 UTC (permalink / raw)
  To: linux-media
  Cc: Rafael J. Wysocki, linux-acpi, Len Brown, Daniel Scally,
	Hans de Goede, Ilpo Järvinen, platform-driver-x86

Fix the following Coccinelle warning:

./tps68470.c:164:58-65: WARNING: Consider using %pe to print PTR_ERR()

by using %pe specifier for printing an error code.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/platform/x86/intel/int3472/tps68470.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
index a77ed32abe55..dc777dbac61f 100644
--- a/drivers/platform/x86/intel/int3472/tps68470.c
+++ b/drivers/platform/x86/intel/int3472/tps68470.c
@@ -161,7 +161,7 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
 
 	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
 	if (IS_ERR(regmap)) {
-		dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
+		dev_err(&client->dev, "Failed to create regmap: %pe\n", regmap);
 		return PTR_ERR(regmap);
 	}
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning
  2026-08-25 20:35 [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472 Sakari Ailus
  2026-08-25 20:35 ` [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print Sakari Ailus
@ 2026-08-25 20:35 ` Sakari Ailus
       [not found]   ` <CAPybu_1J4z2Z0Xv1PyHZa2U+PFsTWPVyifJr2p8DgkQRTJm6YQ@mail.gmail.com>
  2026-08-27 11:16   ` Dan Scally
  2026-08-25 20:35 ` [PATCH v3 3/3] platform/x86: int3472: Clean up GPIO parsing Sakari Ailus
  2026-08-26 10:41 ` [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472 Rafael J. Wysocki (Intel)
  3 siblings, 2 replies; 10+ messages in thread
From: Sakari Ailus @ 2026-08-25 20:35 UTC (permalink / raw)
  To: linux-media
  Cc: Rafael J. Wysocki, linux-acpi, Len Brown, Daniel Scally,
	Hans de Goede, Ilpo Järvinen, platform-driver-x86

Fix a smatch warning about uninitialised err_msg variable, by printing the
error where it is handled.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/platform/x86/intel/int3472/discrete.c | 25 +++++++++++--------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 6c729fcfce5d..9cd4f78875ab 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -329,7 +329,6 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 	unsigned long gpio_flags;
 	union acpi_object *obj;
 	struct gpio_desc *gpio;
-	const char *err_msg;
 	const char *con_id;
 	int ret;
 
@@ -375,7 +374,8 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 	case INT3472_GPIO_TYPE_HOTPLUG_DETECT:
 		ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, con_id, gpio_flags);
 		if (ret)
-			err_msg = "Failed to map GPIO pin to sensor\n";
+			dev_err_probe(int3472->dev, ret,
+				      "Failed to map GPIO pin to sensor\n");
 
 		break;
 	case INT3472_GPIO_TYPE_CLK_ENABLE:
@@ -387,7 +387,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
 		if (IS_ERR(gpio)) {
 			ret = PTR_ERR(gpio);
-			err_msg = "Failed to get GPIO\n";
+			dev_err_probe(int3472->dev, ret, "Failed to get GPIO\n");
 			break;
 		}
 
@@ -395,14 +395,16 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 		case INT3472_GPIO_TYPE_CLK_ENABLE:
 			ret = skl_int3472_register_gpio_clock(int3472, gpio);
 			if (ret)
-				err_msg = "Failed to register clock\n";
+				dev_err_probe(int3472->dev, ret,
+					      "Failed to register clock\n");
 
 			break;
 		case INT3472_GPIO_TYPE_PRIVACY_LED:
 		case INT3472_GPIO_TYPE_STROBE:
 			ret = skl_int3472_register_led(int3472, gpio, con_id);
 			if (ret)
-				err_msg = "Failed to register LED\n";
+				dev_err_probe(int3472->dev, ret,
+					      "Failed to register LED\n");
 
 			break;
 		case INT3472_GPIO_TYPE_POWER_ENABLE:
@@ -413,7 +415,8 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 			ret = skl_int3472_register_regulator(int3472, gpio, enable_time_us,
 							     con_id, second_sensor);
 			if (ret)
-				err_msg = "Failed to register regulator\n";
+				dev_err_probe(int3472->dev, ret,
+					      "Failed to register regulator\n");
 
 			break;
 		default: /* Never reached */
@@ -436,11 +439,11 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 	int3472->ngpios++;
 	ACPI_FREE(obj);
 
-	if (ret < 0)
-		return dev_err_probe(int3472->dev, ret, err_msg);
-
-	/* Tell acpi_dev_get_resources() to not make a copy of the resource */
-	return 1;
+	/*
+	 * Either return an error or tell acpi_dev_get_resources() to not make a
+	 * copy of the resource.
+	 */
+	return ret < 0 ? ret : 1;
 }
 
 int int3472_discrete_parse_crs(struct int3472_discrete_device *int3472)
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/3] platform/x86: int3472: Clean up GPIO parsing
  2026-08-25 20:35 [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472 Sakari Ailus
  2026-08-25 20:35 ` [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print Sakari Ailus
  2026-08-25 20:35 ` [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning Sakari Ailus
@ 2026-08-25 20:35 ` Sakari Ailus
  2026-08-27 11:39   ` Dan Scally
  2026-08-26 10:41 ` [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472 Rafael J. Wysocki (Intel)
  3 siblings, 1 reply; 10+ messages in thread
From: Sakari Ailus @ 2026-08-25 20:35 UTC (permalink / raw)
  To: linux-media
  Cc: Rafael J. Wysocki, linux-acpi, Len Brown, Daniel Scally,
	Hans de Goede, Ilpo Järvinen, platform-driver-x86

In skl_int3472_handle_gpio_resources(), only a single integer value from
the ACPI object is used so release it early and avoid having to call
ACPI_FREE() on it later on. This makes it possible to return an error
where it happens, except when we're holding a reference to a GPIO. ngpios
is also incremented earlier on but that does not introduce a functional
change.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/platform/x86/intel/int3472/discrete.c | 42 ++++++++-----------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 9cd4f78875ab..1f8751a7db0c 100644
--- a/drivers/platform/x86/intel/int3472/discrete.c
+++ b/drivers/platform/x86/intel/int3472/discrete.c
@@ -328,6 +328,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 	u8 active_value, pin, type;
 	unsigned long gpio_flags;
 	union acpi_object *obj;
+	unsigned int obj_value;
 	struct gpio_desc *gpio;
 	const char *con_id;
 	int ret;
@@ -343,24 +344,27 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 				      &int3472_gpio_guid, 0x00,
 				      int3472->ngpios + 2,
 				      NULL, ACPI_TYPE_INTEGER);
-
 	if (!obj) {
 		dev_warn(int3472->dev, "No _DSM entry for GPIO pin %u\n",
 			 agpio->pin_table[0]);
 		return 1;
 	}
 
-	type = FIELD_GET(INT3472_GPIO_DSM_TYPE, obj->integer.value);
+	obj_value = obj->integer.value;
+
+	ACPI_FREE(obj);
+
+	type = FIELD_GET(INT3472_GPIO_DSM_TYPE, obj_value);
 
 	int3472_get_con_id_and_polarity(int3472, &type, &con_id, &gpio_flags, &enable_time_us);
 
-	pin = FIELD_GET(INT3472_GPIO_DSM_PIN, obj->integer.value);
+	pin = FIELD_GET(INT3472_GPIO_DSM_PIN, obj_value);
 	/* Pin field is not really used under Windows and wraps around at 8 bits */
 	if (pin != (agpio->pin_table[0] & 0xff))
 		dev_dbg(int3472->dev, FW_BUG "%s %s pin number mismatch _DSM %d resource %d\n",
 			con_id, agpio->resource_source.string_ptr, pin, agpio->pin_table[0]);
 
-	active_value = FIELD_GET(INT3472_GPIO_DSM_SENSOR_ON_VAL, obj->integer.value);
+	active_value = FIELD_GET(INT3472_GPIO_DSM_SENSOR_ON_VAL, obj_value);
 	if (!active_value)
 		gpio_flags ^= GPIO_ACTIVE_LOW;
 
@@ -368,16 +372,18 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 		agpio->resource_source.string_ptr, agpio->pin_table[0],
 		str_high_low(gpio_flags == GPIO_ACTIVE_HIGH));
 
+	int3472->ngpios++;
+
 	switch (type) {
 	case INT3472_GPIO_TYPE_RESET:
 	case INT3472_GPIO_TYPE_POWERDOWN:
 	case INT3472_GPIO_TYPE_HOTPLUG_DETECT:
 		ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, con_id, gpio_flags);
 		if (ret)
-			dev_err_probe(int3472->dev, ret,
-				      "Failed to map GPIO pin to sensor\n");
+			return dev_err_probe(int3472->dev, ret,
+					     "Failed to map GPIO pin to sensor\n");
 
-		break;
+		return 1;
 	case INT3472_GPIO_TYPE_CLK_ENABLE:
 	case INT3472_GPIO_TYPE_PRIVACY_LED:
 	case INT3472_GPIO_TYPE_STROBE:
@@ -385,11 +391,9 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 	case INT3472_GPIO_TYPE_DOVDD:
 	case INT3472_GPIO_TYPE_HANDSHAKE:
 		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
-		if (IS_ERR(gpio)) {
-			ret = PTR_ERR(gpio);
-			dev_err_probe(int3472->dev, ret, "Failed to get GPIO\n");
-			break;
-		}
+		if (IS_ERR(gpio))
+			return dev_err_probe(int3472->dev, PTR_ERR(gpio),
+					     "Failed to get GPIO\n");
 
 		switch (type) {
 		case INT3472_GPIO_TYPE_CLK_ENABLE:
@@ -427,23 +431,13 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
 		if (ret)
 			gpiod_put(gpio);
 
-		break;
+		return ret < 0 ? ret : 1;
 	default:
 		dev_warn(int3472->dev,
 			 "GPIO type 0x%02x unknown; the sensor may not work\n",
 			 type);
-		ret = 1;
-		break;
+		return 1;
 	}
-
-	int3472->ngpios++;
-	ACPI_FREE(obj);
-
-	/*
-	 * Either return an error or tell acpi_dev_get_resources() to not make a
-	 * copy of the resource.
-	 */
-	return ret < 0 ? ret : 1;
 }
 
 int int3472_discrete_parse_crs(struct int3472_discrete_device *int3472)
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print
  2026-08-25 20:35 ` [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print Sakari Ailus
@ 2026-08-25 21:01   ` Ricardo Ribalda Delgado
  2026-08-27  8:25   ` Dan Scally
  1 sibling, 0 replies; 10+ messages in thread
From: Ricardo Ribalda Delgado @ 2026-08-25 21:01 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, Rafael J. Wysocki, linux-acpi, Len Brown,
	Daniel Scally, Hans de Goede, Ilpo Järvinen,
	platform-driver-x86

On Tue, Aug 25, 2026 at 10:36 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Fix the following Coccinelle warning:
>
> ./tps68470.c:164:58-65: WARNING: Consider using %pe to print PTR_ERR()
>
> by using %pe specifier for printing an error code.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/platform/x86/intel/int3472/tps68470.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
> index a77ed32abe55..dc777dbac61f 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470.c
> @@ -161,7 +161,7 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
>
>         regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
>         if (IS_ERR(regmap)) {
> -               dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
> +               dev_err(&client->dev, "Failed to create regmap: %pe\n", regmap);
>                 return PTR_ERR(regmap);
>         }
>
> --
> 2.47.3
>
>


-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning
       [not found]   ` <CAPybu_1J4z2Z0Xv1PyHZa2U+PFsTWPVyifJr2p8DgkQRTJm6YQ@mail.gmail.com>
@ 2026-08-26  7:04     ` Sakari Ailus
  0 siblings, 0 replies; 10+ messages in thread
From: Sakari Ailus @ 2026-08-26  7:04 UTC (permalink / raw)
  To: Ricardo Ribalda Delgado
  Cc: linux-media, Rafael J. Wysocki, linux-acpi, Len Brown,
	Daniel Scally, Hans de Goede, Ilpo Järvinen,
	platform-driver-x86

Hi Ricardo,

Thanks for the review.

On Tue, Aug 25, 2026 at 11:04:51PM +0200, Ricardo Ribalda Delgado wrote:
> nits:
> 
> It would be nice to have the error message that it is fixed in the
> commit message (also for 1/3)

There's actually a compiler warning on this, too. I'll reword this being:

Fix a compiler warning about an uninitialised err_msg variable by printing
the error where it is handled:

drivers/platform/x86/intel/int3472/discrete.c:440:24: warning: ‘err_msg’ may be used uninitialized [-Wmaybe-uninitialized]
  440 |                 return dev_err_probe(int3472->dev, ret, err_msg);

> 
> Would you consider adding a Fixes? This patch fixes a compilation
> error (with the right kernel config)

Fixes: 5de691bffe57 ("platform/x86: Add intel_skl_int3472 driver")
Cc: stable

It seems it's always been there but I think it's a false positive: the
compiler probably just can't figure out err_msg is unused if ret is
non-zero.

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472
  2026-08-25 20:35 [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472 Sakari Ailus
                   ` (2 preceding siblings ...)
  2026-08-25 20:35 ` [PATCH v3 3/3] platform/x86: int3472: Clean up GPIO parsing Sakari Ailus
@ 2026-08-26 10:41 ` Rafael J. Wysocki (Intel)
  3 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-08-26 10:41 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, Rafael J. Wysocki, linux-acpi, Len Brown,
	Daniel Scally, Hans de Goede, Ilpo Järvinen,
	platform-driver-x86

On Tue, Aug 25, 2026 at 10:35 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi folks,
>
> The two first patches address smatch and compiler warnings in int3472. The
> third patch clean up GPIO parsing in the int3472 driver.
>
> since v2:
>
> - Drop the __free(ACPI_FREE) change.
>
> - Move ACPI_FREE() earlier in the function and save the object's integer
>   value into a local variable.
>
> - Fix return values -- 0 was returned by v2 in some cases where 1 was the
>   right value.
>
> Sakari Ailus (3):
>   platform/x86: int3472: Address Coccinelle warning on an error print
>   platform/x86: int3472: Fix uninitialised variable warning
>   platform/x86: int3472: Clean up GPIO parsing
>
>  drivers/platform/x86/intel/int3472/discrete.c | 51 +++++++++----------
>  drivers/platform/x86/intel/int3472/tps68470.c |  2 +-
>  2 files changed, 25 insertions(+), 28 deletions(-)
>
>
> base-commit: 56c29fa3ee666197516a231e75aed789ae9c530d
> --

Acked-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

for the series.

Thanks!

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print
  2026-08-25 20:35 ` [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print Sakari Ailus
  2026-08-25 21:01   ` Ricardo Ribalda Delgado
@ 2026-08-27  8:25   ` Dan Scally
  1 sibling, 0 replies; 10+ messages in thread
From: Dan Scally @ 2026-08-27  8:25 UTC (permalink / raw)
  To: Sakari Ailus, linux-media
  Cc: Rafael J. Wysocki, linux-acpi, Len Brown, Hans de Goede,
	Ilpo Järvinen, platform-driver-x86

Hi Sakari - thanks for the patches

On 25/08/2026 21:35, Sakari Ailus wrote:
> Fix the following Coccinelle warning:
> 
> ./tps68470.c:164:58-65: WARNING: Consider using %pe to print PTR_ERR()
> 
> by using %pe specifier for printing an error code.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/platform/x86/intel/int3472/tps68470.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
> index a77ed32abe55..dc777dbac61f 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470.c
> @@ -161,7 +161,7 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
>   
>   	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
>   	if (IS_ERR(regmap)) {
> -		dev_err(&client->dev, "Failed to create regmap: %ld\n", PTR_ERR(regmap));
> +		dev_err(&client->dev, "Failed to create regmap: %pe\n", regmap);
>   		return PTR_ERR(regmap);
>   	}
>   


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning
  2026-08-25 20:35 ` [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning Sakari Ailus
       [not found]   ` <CAPybu_1J4z2Z0Xv1PyHZa2U+PFsTWPVyifJr2p8DgkQRTJm6YQ@mail.gmail.com>
@ 2026-08-27 11:16   ` Dan Scally
  1 sibling, 0 replies; 10+ messages in thread
From: Dan Scally @ 2026-08-27 11:16 UTC (permalink / raw)
  To: Sakari Ailus, linux-media
  Cc: Rafael J. Wysocki, linux-acpi, Len Brown, Hans de Goede,
	Ilpo Järvinen, platform-driver-x86

Hi Sakari - thanks for the patch

On 25/08/2026 21:35, Sakari Ailus wrote:
> Fix a smatch warning about uninitialised err_msg variable, by printing the
> error where it is handled.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---

Oops, wonder why I didn't spot the compiler warning.

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/platform/x86/intel/int3472/discrete.c | 25 +++++++++++--------
>   1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 6c729fcfce5d..9cd4f78875ab 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -329,7 +329,6 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   	unsigned long gpio_flags;
>   	union acpi_object *obj;
>   	struct gpio_desc *gpio;
> -	const char *err_msg;
>   	const char *con_id;
>   	int ret;
>   
> @@ -375,7 +374,8 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   	case INT3472_GPIO_TYPE_HOTPLUG_DETECT:
>   		ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, con_id, gpio_flags);
>   		if (ret)
> -			err_msg = "Failed to map GPIO pin to sensor\n";
> +			dev_err_probe(int3472->dev, ret,
> +				      "Failed to map GPIO pin to sensor\n");
>   
>   		break;
>   	case INT3472_GPIO_TYPE_CLK_ENABLE:
> @@ -387,7 +387,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
>   		if (IS_ERR(gpio)) {
>   			ret = PTR_ERR(gpio);
> -			err_msg = "Failed to get GPIO\n";
> +			dev_err_probe(int3472->dev, ret, "Failed to get GPIO\n");
>   			break;
>   		}
>   
> @@ -395,14 +395,16 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   		case INT3472_GPIO_TYPE_CLK_ENABLE:
>   			ret = skl_int3472_register_gpio_clock(int3472, gpio);
>   			if (ret)
> -				err_msg = "Failed to register clock\n";
> +				dev_err_probe(int3472->dev, ret,
> +					      "Failed to register clock\n");
>   
>   			break;
>   		case INT3472_GPIO_TYPE_PRIVACY_LED:
>   		case INT3472_GPIO_TYPE_STROBE:
>   			ret = skl_int3472_register_led(int3472, gpio, con_id);
>   			if (ret)
> -				err_msg = "Failed to register LED\n";
> +				dev_err_probe(int3472->dev, ret,
> +					      "Failed to register LED\n");
>   
>   			break;
>   		case INT3472_GPIO_TYPE_POWER_ENABLE:
> @@ -413,7 +415,8 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   			ret = skl_int3472_register_regulator(int3472, gpio, enable_time_us,
>   							     con_id, second_sensor);
>   			if (ret)
> -				err_msg = "Failed to register regulator\n";
> +				dev_err_probe(int3472->dev, ret,
> +					      "Failed to register regulator\n");
>   
>   			break;
>   		default: /* Never reached */
> @@ -436,11 +439,11 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   	int3472->ngpios++;
>   	ACPI_FREE(obj);
>   
> -	if (ret < 0)
> -		return dev_err_probe(int3472->dev, ret, err_msg);
> -
> -	/* Tell acpi_dev_get_resources() to not make a copy of the resource */
> -	return 1;
> +	/*
> +	 * Either return an error or tell acpi_dev_get_resources() to not make a
> +	 * copy of the resource.
> +	 */
> +	return ret < 0 ? ret : 1;
>   }
>   
>   int int3472_discrete_parse_crs(struct int3472_discrete_device *int3472)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] platform/x86: int3472: Clean up GPIO parsing
  2026-08-25 20:35 ` [PATCH v3 3/3] platform/x86: int3472: Clean up GPIO parsing Sakari Ailus
@ 2026-08-27 11:39   ` Dan Scally
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Scally @ 2026-08-27 11:39 UTC (permalink / raw)
  To: Sakari Ailus, linux-media
  Cc: Rafael J. Wysocki, linux-acpi, Len Brown, Hans de Goede,
	Ilpo Järvinen, platform-driver-x86

Hi Sakari, thanks for the patch

On 25/08/2026 21:35, Sakari Ailus wrote:
> In skl_int3472_handle_gpio_resources(), only a single integer value from
> the ACPI object is used so release it early and avoid having to call
> ACPI_FREE() on it later on. This makes it possible to return an error
> where it happens, except when we're holding a reference to a GPIO. ngpios
> is also incremented earlier on but that does not introduce a functional
> change.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---

Looks good to me:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/platform/x86/intel/int3472/discrete.c | 42 ++++++++-----------
>   1 file changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 9cd4f78875ab..1f8751a7db0c 100644
> --- a/drivers/platform/x86/intel/int3472/discrete.c
> +++ b/drivers/platform/x86/intel/int3472/discrete.c
> @@ -328,6 +328,7 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   	u8 active_value, pin, type;
>   	unsigned long gpio_flags;
>   	union acpi_object *obj;
> +	unsigned int obj_value;
>   	struct gpio_desc *gpio;
>   	const char *con_id;
>   	int ret;
> @@ -343,24 +344,27 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   				      &int3472_gpio_guid, 0x00,
>   				      int3472->ngpios + 2,
>   				      NULL, ACPI_TYPE_INTEGER);
> -
>   	if (!obj) {
>   		dev_warn(int3472->dev, "No _DSM entry for GPIO pin %u\n",
>   			 agpio->pin_table[0]);
>   		return 1;
>   	}
>   
> -	type = FIELD_GET(INT3472_GPIO_DSM_TYPE, obj->integer.value);
> +	obj_value = obj->integer.value;
> +
> +	ACPI_FREE(obj);
> +
> +	type = FIELD_GET(INT3472_GPIO_DSM_TYPE, obj_value);
>   
>   	int3472_get_con_id_and_polarity(int3472, &type, &con_id, &gpio_flags, &enable_time_us);
>   
> -	pin = FIELD_GET(INT3472_GPIO_DSM_PIN, obj->integer.value);
> +	pin = FIELD_GET(INT3472_GPIO_DSM_PIN, obj_value);
>   	/* Pin field is not really used under Windows and wraps around at 8 bits */
>   	if (pin != (agpio->pin_table[0] & 0xff))
>   		dev_dbg(int3472->dev, FW_BUG "%s %s pin number mismatch _DSM %d resource %d\n",
>   			con_id, agpio->resource_source.string_ptr, pin, agpio->pin_table[0]);
>   
> -	active_value = FIELD_GET(INT3472_GPIO_DSM_SENSOR_ON_VAL, obj->integer.value);
> +	active_value = FIELD_GET(INT3472_GPIO_DSM_SENSOR_ON_VAL, obj_value);
>   	if (!active_value)
>   		gpio_flags ^= GPIO_ACTIVE_LOW;
>   
> @@ -368,16 +372,18 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   		agpio->resource_source.string_ptr, agpio->pin_table[0],
>   		str_high_low(gpio_flags == GPIO_ACTIVE_HIGH));
>   
> +	int3472->ngpios++;
> +
>   	switch (type) {
>   	case INT3472_GPIO_TYPE_RESET:
>   	case INT3472_GPIO_TYPE_POWERDOWN:
>   	case INT3472_GPIO_TYPE_HOTPLUG_DETECT:
>   		ret = skl_int3472_map_gpio_to_sensor(int3472, agpio, con_id, gpio_flags);
>   		if (ret)
> -			dev_err_probe(int3472->dev, ret,
> -				      "Failed to map GPIO pin to sensor\n");
> +			return dev_err_probe(int3472->dev, ret,
> +					     "Failed to map GPIO pin to sensor\n");
>   
> -		break;
> +		return 1;
>   	case INT3472_GPIO_TYPE_CLK_ENABLE:
>   	case INT3472_GPIO_TYPE_PRIVACY_LED:
>   	case INT3472_GPIO_TYPE_STROBE:
> @@ -385,11 +391,9 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   	case INT3472_GPIO_TYPE_DOVDD:
>   	case INT3472_GPIO_TYPE_HANDSHAKE:
>   		gpio = skl_int3472_gpiod_get_from_temp_lookup(int3472, agpio, con_id, gpio_flags);
> -		if (IS_ERR(gpio)) {
> -			ret = PTR_ERR(gpio);
> -			dev_err_probe(int3472->dev, ret, "Failed to get GPIO\n");
> -			break;
> -		}
> +		if (IS_ERR(gpio))
> +			return dev_err_probe(int3472->dev, PTR_ERR(gpio),
> +					     "Failed to get GPIO\n");
>   
>   		switch (type) {
>   		case INT3472_GPIO_TYPE_CLK_ENABLE:
> @@ -427,23 +431,13 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>   		if (ret)
>   			gpiod_put(gpio);
>   
> -		break;
> +		return ret < 0 ? ret : 1;
>   	default:
>   		dev_warn(int3472->dev,
>   			 "GPIO type 0x%02x unknown; the sensor may not work\n",
>   			 type);
> -		ret = 1;
> -		break;
> +		return 1;
>   	}
> -
> -	int3472->ngpios++;
> -	ACPI_FREE(obj);
> -
> -	/*
> -	 * Either return an error or tell acpi_dev_get_resources() to not make a
> -	 * copy of the resource.
> -	 */
> -	return ret < 0 ? ret : 1;
>   }
>   
>   int int3472_discrete_parse_crs(struct int3472_discrete_device *int3472)


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-08-27 11:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-25 20:35 [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472 Sakari Ailus
2026-08-25 20:35 ` [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print Sakari Ailus
2026-08-25 21:01   ` Ricardo Ribalda Delgado
2026-08-27  8:25   ` Dan Scally
2026-08-25 20:35 ` [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning Sakari Ailus
     [not found]   ` <CAPybu_1J4z2Z0Xv1PyHZa2U+PFsTWPVyifJr2p8DgkQRTJm6YQ@mail.gmail.com>
2026-08-26  7:04     ` Sakari Ailus
2026-08-27 11:16   ` Dan Scally
2026-08-25 20:35 ` [PATCH v3 3/3] platform/x86: int3472: Clean up GPIO parsing Sakari Ailus
2026-08-27 11:39   ` Dan Scally
2026-08-26 10:41 ` [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472 Rafael J. Wysocki (Intel)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox