Received-SPF: pass (google.com: domain of linux-kernel+bounces-204017-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 6 Jun 2024 12:34:31 +0300
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Javier Carrasco <javier.carrasco@wolfvision.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] usb: typec: tipd: drop second firmware name read
Message-ID: <ZmGCp8G4nK2ljRU8@kuha.fi.intel.com>
References: <20240606-tps6598x_fw_update_log-v1-0-2b5b8369a0ba@wolfvision.net>
 <20240606-tps6598x_fw_update_log-v1-1-2b5b8369a0ba@wolfvision.net>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240606-tps6598x_fw_update_log-v1-1-2b5b8369a0ba@wolfvision.net>

On Thu, Jun 06, 2024 at 11:03:55AM +0200, Javier Carrasco wrote:
> tps_request_firmware() reads the firmware name and there is no need to
> repeat the action in the device-specific implementations of the firmware
> update mechanism.
> 
> Provide the firmware name as a parameter in tps_request_firmware() to
> avoid repetitive operations in the device-specific implementations.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tipd/core.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index ad76dbd20e65..851b0d02727a 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -892,19 +892,19 @@ tps6598x_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode)
>  	return 0;
>  }
>  
> -static int tps_request_firmware(struct tps6598x *tps, const struct firmware **fw)
> +static int tps_request_firmware(struct tps6598x *tps, const struct firmware **fw,
> +				const char **firmware_name)
>  {
> -	const char *firmware_name;
>  	int ret;
>  
>  	ret = device_property_read_string(tps->dev, "firmware-name",
> -					  &firmware_name);
> +					  firmware_name);
>  	if (ret)
>  		return ret;
>  
> -	ret = request_firmware(fw, firmware_name, tps->dev);
> +	ret = request_firmware(fw, *firmware_name, tps->dev);
>  	if (ret) {
> -		dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name);
> +		dev_err(tps->dev, "failed to retrieve \"%s\"\n", *firmware_name);
>  		return ret;
>  	}
>  
> @@ -999,12 +999,7 @@ static int tps25750_start_patch_burst_mode(struct tps6598x *tps)
>  	u32 addr;
>  	struct device_node *np = tps->dev->of_node;
>  
> -	ret = device_property_read_string(tps->dev, "firmware-name",
> -					  &firmware_name);
> -	if (ret)
> -		return ret;
> -
> -	ret = tps_request_firmware(tps, &fw);
> +	ret = tps_request_firmware(tps, &fw, &firmware_name);
>  	if (ret)
>  		return ret;
>  
> @@ -1155,12 +1150,7 @@ static int tps6598x_apply_patch(struct tps6598x *tps)
>  	const char *firmware_name;
>  	int ret;
>  
> -	ret = device_property_read_string(tps->dev, "firmware-name",
> -					  &firmware_name);
> -	if (ret)
> -		return ret;
> -
> -	ret = tps_request_firmware(tps, &fw);
> +	ret = tps_request_firmware(tps, &fw, &firmware_name);
>  	if (ret)
>  		return ret;
>  
> 
> -- 
> 2.40.1

-- 
heikki