* [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* 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 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
* [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[parent not found: <CAPybu_1J4z2Z0Xv1PyHZa2U+PFsTWPVyifJr2p8DgkQRTJm6YQ@mail.gmail.com>]
* 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 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
* [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 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
* 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