netdev - Re: [PATCH net-next v3 2/3] net: airoha: npu: Add airoha_npu_soc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aO4LL8racazLjjzk@horms.kernel.org>
Date: Tue, 14 Oct 2025 09:34:55 +0100
From: Simon Horman <horms@...nel.org>
To: Lorenzo Bianconi <lorenzo@...nel.org>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH net-next v3 2/3] net: airoha: npu: Add
 airoha_npu_soc_data struct

On Mon, Oct 13, 2025 at 03:58:50PM +0200, Lorenzo Bianconi wrote:

...

> @@ -182,49 +192,53 @@ static int airoha_npu_send_msg(struct airoha_npu *npu, int func_id,
>  	return ret;
>  }
>  
> -static int airoha_npu_run_firmware(struct device *dev, void __iomem *base,
> -				   struct resource *res)
> +static int airoha_npu_load_firmware(struct device *dev, void __iomem *addr,
> +				    const struct airoha_npu_fw *fw_info)
>  {
>  	const struct firmware *fw;
> -	void __iomem *addr;
>  	int ret;
>  
> -	ret = request_firmware(&fw, NPU_EN7581_FIRMWARE_RV32, dev);
> +	ret = request_firmware(&fw, fw_info->name, dev);
>  	if (ret)
>  		return ret == -ENOENT ? -EPROBE_DEFER : ret;
>  
> -	if (fw->size > NPU_EN7581_FIRMWARE_RV32_MAX_SIZE) {
> +	if (fw->size > fw_info->max_size) {
>  		dev_err(dev, "%s: fw size too overlimit (%zu)\n",
> -			NPU_EN7581_FIRMWARE_RV32, fw->size);
> +			fw_info->name, fw->size);
>  		ret = -E2BIG;
>  		goto out;
>  	}
>  
> -	addr = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(addr)) {
> -		ret = PTR_ERR(addr);
> -		goto out;
> -	}
> -
>  	memcpy_toio(addr, fw->data, fw->size);
> +out:
>  	release_firmware(fw);
>  
> -	ret = request_firmware(&fw, NPU_EN7581_FIRMWARE_DATA, dev);
> -	if (ret)
> -		return ret == -ENOENT ? -EPROBE_DEFER : ret;
> +	return ret;
> +}
>  
> -	if (fw->size > NPU_EN7581_FIRMWARE_DATA_MAX_SIZE) {
> -		dev_err(dev, "%s: fw size too overlimit (%zu)\n",
> -			NPU_EN7581_FIRMWARE_DATA, fw->size);
> -		ret = -E2BIG;
> -		goto out;
> -	}
> +static int airoha_npu_run_firmware(struct device *dev, void __iomem *base,
> +				   struct resource *res)
> +{
> +	const struct airoha_npu_soc_data *soc;
> +	void __iomem *addr;
> +	int ret;
>  
> -	memcpy_toio(base + REG_NPU_LOCAL_SRAM, fw->data, fw->size);
> -out:
> -	release_firmware(fw);
> +	soc = of_device_get_match_data(dev);
> +	if (!soc)
> +		return -EINVAL;
>  
> -	return ret;
> +	addr = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(addr))
> +		return PTR_ERR(addr);
> +
> +	/* Load rv32 npu firmware */
> +	ret = airoha_npu_load_firmware(dev, addr, &soc->fw_rv32);
> +	if (ret)
> +		return ret;
> +
> +	/* Load data npu firmware */
> +	return airoha_npu_load_firmware(dev, base + REG_NPU_LOCAL_SRAM,
> +					&soc->fw_data);

Hi Lorenzo,

There are two calls to airoha_npu_load_firmware() above.
And, internally, airoha_npu_load_firmware() will call release_firmware()
if an error is encountered.

But should release_firmware() be called for the firmware requested
by the first call to airoha_npu_load_firmware() if the second call fails?
Such clean-up seems to have been the case prior to this patch.

Also, not strictly related. Should release_firmware() be called (twice)
when the driver is removed?

>  }
>  
>  static irqreturn_t airoha_npu_mbox_handler(int irq, void *npu_instance)

...