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

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

[Sakari Ailus]

Hi folks,

These two patches address smatch and compiler warnings in int3472.

Just compile tested for now. :-)

Sakari Ailus (2):
  platform/x86: int3472: Address Coccinelle warning on an error print
  platform/x86: int3472: Fix uninitialised variable warning

 drivers/platform/x86/intel/int3472/discrete.c | 21 +++++++++----------
 drivers/platform/x86/intel/int3472/tps68470.c |  2 +-
 2 files changed, 11 insertions(+), 12 deletions(-)

-- 
2.47.3

[PATCH 1/2] 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>
---
 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 2/2] 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 | 21 +++++++++----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
index 6c729fcfce5d..cf8a58ef963b 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,7 @@ 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(int3472->dev, "Failed to map GPIO pin to sensor\n");
 
 		break;
 	case INT3472_GPIO_TYPE_CLK_ENABLE:
@@ -387,7 +386,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(int3472->dev, "Failed to get GPIO\n");
 			break;
 		}
 
@@ -395,14 +394,14 @@ 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(int3472->dev, "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(int3472->dev, "Failed to register LED\n");
 
 			break;
 		case INT3472_GPIO_TYPE_POWER_ENABLE:
@@ -413,7 +412,7 @@ 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(int3472->dev, "Failed to register regulator\n");
 
 			break;
 		default: /* Never reached */
@@ -436,11 +435,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

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

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]

On Thu, 20 Aug 2026, Sakari Ailus wrote:

> Fix the following Coccinelle warning:
> 
> ./tps68470.c:164:58-65: WARNING: Consider using %pe to print PTR_ERR()

It seems coccinelle scripts too employ scare tactics. :-/

The change is fine,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

> by using %pe specifier for printing an error code.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@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);
>  	}
>  
> 

-- 
 i.

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

[Ilpo Järvinen]

On Thu, 20 Aug 2026, 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>
> ---
>  drivers/platform/x86/intel/int3472/discrete.c | 21 +++++++++----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> index 6c729fcfce5d..cf8a58ef963b 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,7 @@ 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(int3472->dev, "Failed to map GPIO pin to sensor\n");
>  
>  		break;
>  	case INT3472_GPIO_TYPE_CLK_ENABLE:
> @@ -387,7 +386,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(int3472->dev, "Failed to get GPIO\n");
>  			break;
>  		}
>  

ssahiko is not happy about dev_err_probe() -> dev_err() conversion.

> @@ -395,14 +394,14 @@ 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(int3472->dev, "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(int3472->dev, "Failed to register LED\n");
>  
>  			break;
>  		case INT3472_GPIO_TYPE_POWER_ENABLE:
> @@ -413,7 +412,7 @@ 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(int3472->dev, "Failed to register regulator\n");
>  
>  			break;
>  		default: /* Never reached */
> @@ -436,11 +435,11 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
>  	int3472->ngpios++;
>  	ACPI_FREE(obj);

As a general comment to logic in this function, it would be preferrable to 
arrange code such that the error cases inside the switch could just return 
directly.

To realize that, one would need to

1) use __free() for obj

2) handle that ngpios++ earlier; but I'm not even entire sure about ngpios 
correctness and function, it seems to be only used for indexing in
acpi_evaluate_dsm_typed() (successive calls into 
skl_int3472_handle_gpio_resources()?) but then I don't understand why 
the !obj early return does not increment ngpios too.

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

-- 
 i.

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

[Sakari Ailus]

Moi,

Thanks for the review.

On Fri, Aug 21, 2026 at 11:34:06AM +0300, Ilpo Järvinen wrote:
> On Thu, 20 Aug 2026, 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>
> > ---
> >  drivers/platform/x86/intel/int3472/discrete.c | 21 +++++++++----------
> >  1 file changed, 10 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/intel/int3472/discrete.c b/drivers/platform/x86/intel/int3472/discrete.c
> > index 6c729fcfce5d..cf8a58ef963b 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,7 @@ 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(int3472->dev, "Failed to map GPIO pin to sensor\n");
> >  
> >  		break;
> >  	case INT3472_GPIO_TYPE_CLK_ENABLE:
> > @@ -387,7 +386,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(int3472->dev, "Failed to get GPIO\n");
> >  			break;
> >  		}
> >  
> 
> ssahiko is not happy about dev_err_probe() -> dev_err() conversion.

I was wondering, too, whether I should keep it, but then again returning
the error won't help here (as of now at least) and it is probably not
possible -EPROBE_DEFER would be returned in these cases. I can keep using
dev_err_probe() though if you prefer that.

> 
> > @@ -395,14 +394,14 @@ 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(int3472->dev, "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(int3472->dev, "Failed to register LED\n");
> >  
> >  			break;
> >  		case INT3472_GPIO_TYPE_POWER_ENABLE:
> > @@ -413,7 +412,7 @@ 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(int3472->dev, "Failed to register regulator\n");
> >  
> >  			break;
> >  		default: /* Never reached */
> > @@ -436,11 +435,11 @@ static int skl_int3472_handle_gpio_resources(struct acpi_resource *ares,
> >  	int3472->ngpios++;
> >  	ACPI_FREE(obj);
> 
> As a general comment to logic in this function, it would be preferrable to 
> arrange code such that the error cases inside the switch could just return 
> directly.
> 
> To realize that, one would need to
> 
> 1) use __free() for obj
> 
> 2) handle that ngpios++ earlier; but I'm not even entire sure about ngpios 
> correctness and function, it seems to be only used for indexing in
> acpi_evaluate_dsm_typed() (successive calls into 
> skl_int3472_handle_gpio_resources()?) but then I don't understand why 
> the !obj early return does not increment ngpios too.

"ngpios" is in fact an iterator. In case of an error incrementing it
doesn't really matter and !obj signals the end.

I'll see how to refactor this a little in another patch. The intent here
was just to fix the smatch warning as we can't currently add int3472 to
Media CI because of it.

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

-- 
Kind regards,

Sakari Ailus