[PATCH v3 0/3] Fix static analyser and compiler warnings in int3472

[PATCH v3 0/3] Fix static analyser and compiler warnings in int3472

[Sakari Ailus]

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

[PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print

[Sakari Ailus]

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

[PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning

[Sakari Ailus]

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

[PATCH v3 3/3] platform/x86: int3472: Clean up GPIO parsing

[Sakari Ailus]

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

Re: [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print

[Ricardo Ribalda Delgado]

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

Re: [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning

[Ricardo Ribalda Delgado]

nits:

It would be nice to have the error message that it is fixed in the
commit message (also for 1/3)

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

On Tue, Aug 25, 2026 at 10:36 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Fix a smatch warning about uninitialised err_msg variable, by printing the
> error where it is handled.
>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> 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
>
>


-- 
Ricardo Ribalda

Re: [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning

[Sakari Ailus]

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

Re: [PATCH v3 0/3] Fix static analyser and compiler warnings in int3472

[Rafael J. Wysocki (Intel)]

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!

Re: [PATCH v3 1/3] platform/x86: int3472: Address Coccinelle warning on an error print

[Dan Scally]

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);
>   	}
>   

Re: [PATCH v3 2/3] platform/x86: int3472: Fix uninitialised variable warning

[Dan Scally]

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)

Re: [PATCH v3 3/3] platform/x86: int3472: Clean up GPIO parsing

[Dan Scally]

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)