Linux Media Controller development
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-media@vger.kernel.org,
	Daniel Scally <dan.scally@ideasonboard.com>,
	 Hans de Goede <hansg@kernel.org>,
	platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH 2/2] platform/x86: int3472: Fix uninitialised variable warning
Date: Fri, 21 Aug 2026 11:34:06 +0300 (EEST)	[thread overview]
Message-ID: <ca97b754-795e-e579-8705-9f2e3d66287b@linux.intel.com> (raw)
In-Reply-To: <20260820205148.2910639-3-sakari.ailus@linux.intel.com>

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.


      reply	other threads:[~2026-08-21  8:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-20 20:51 [PATCH 0/2] Fix static analyser and compiler warnings in int3472 Sakari Ailus
2026-08-20 20:51 ` [PATCH 1/2] platform/x86: int3472: Address Coccinelle warning on an error print Sakari Ailus
2026-08-21  8:14   ` Ilpo Järvinen
2026-08-20 20:51 ` [PATCH 2/2] platform/x86: int3472: Fix uninitialised variable warning Sakari Ailus
2026-08-21  8:34   ` Ilpo Järvinen [this message]

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=ca97b754-795e-e579-8705-9f2e3d66287b@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=dan.scally@ideasonboard.com \
    --cc=hansg@kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

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

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