[PATCH net-next v3 0/3] net: airoha: npu: Introduce support for Airoha 7583 NPU

[PATCH net-next v3 0/3] net: airoha: npu: Introduce support for Airoha 7583 NPU

[Lorenzo Bianconi]

Introduce support for Airoha 7583 SoC NPU.

---
Changes in v3:
- Rebase on top of net-next
- Link to v2: https://lore.kernel.org/r/20250927-airoha-npu-7583-v2-0-e12fac5cce1f@kernel.org

Changes in v2:
- Introduce airoha_npu_soc_data struct and generalize firmware load
- Link to v1: https://lore.kernel.org/r/20250926-airoha-npu-7583-v1-0-447e5e2df08d@kernel.org

---
Lorenzo Bianconi (3):
      dt-bindings: net: airoha: npu: Add AN7583 support
      net: airoha: npu: Add airoha_npu_soc_data struct
      net: airoha: npu: Add 7583 SoC support

 .../devicetree/bindings/net/airoha,en7581-npu.yaml |  1 +
 drivers/net/ethernet/airoha/airoha_npu.c           | 93 ++++++++++++++++------
 2 files changed, 68 insertions(+), 26 deletions(-)
---
base-commit: 18a7e218cfcdca6666e1f7356533e4c988780b57
change-id: 20250926-airoha-npu-7583-63e41301664c

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>

[PATCH net-next v3 1/3] dt-bindings: net: airoha: npu: Add AN7583 support

[Lorenzo Bianconi]

Introduce AN7583 NPU support to Airoha EN7581 NPU device-tree bindings.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 Documentation/devicetree/bindings/net/airoha,en7581-npu.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/net/airoha,en7581-npu.yaml b/Documentation/devicetree/bindings/net/airoha,en7581-npu.yaml
index c7644e6586d329d8ec2f0a0d8d8e4f4490429dcc..59c57f58116b568092446e6cfb7b6bd3f4f47b82 100644
--- a/Documentation/devicetree/bindings/net/airoha,en7581-npu.yaml
+++ b/Documentation/devicetree/bindings/net/airoha,en7581-npu.yaml
@@ -18,6 +18,7 @@ properties:
   compatible:
     enum:
       - airoha,en7581-npu
+      - airoha,an7583-npu
 
   reg:
     maxItems: 1

-- 
2.51.0

[PATCH net-next v3 2/3] net: airoha: npu: Add airoha_npu_soc_data struct

[Lorenzo Bianconi]

Introduce airoha_npu_soc_data structure in order to generalize per-SoC
NPU firmware info. Introduce airoha_npu_load_firmware utility routine.
This is a preliminary patch in order to introduce AN7583 NPU support.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_npu.c | 77 +++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_npu.c b/drivers/net/ethernet/airoha/airoha_npu.c
index 8c883f2b2d36b74053282bc299f0a0572b2e0e71..41944cc5f6b062129528e9357511fd9e05cbe1e6 100644
--- a/drivers/net/ethernet/airoha/airoha_npu.c
+++ b/drivers/net/ethernet/airoha/airoha_npu.c
@@ -103,6 +103,16 @@ enum {
 	QDMA_WAN_PON_XDSL,
 };
 
+struct airoha_npu_fw {
+	const char *name;
+	int max_size;
+};
+
+struct airoha_npu_soc_data {
+	struct airoha_npu_fw fw_rv32;
+	struct airoha_npu_fw fw_data;
+};
+
 #define MBOX_MSG_FUNC_ID	GENMASK(14, 11)
 #define MBOX_MSG_STATIC_BUF	BIT(5)
 #define MBOX_MSG_STATUS		GENMASK(4, 2)
@@ -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);
 }
 
 static irqreturn_t airoha_npu_mbox_handler(int irq, void *npu_instance)
@@ -597,8 +611,19 @@ void airoha_npu_put(struct airoha_npu *npu)
 }
 EXPORT_SYMBOL_GPL(airoha_npu_put);
 
+static const struct airoha_npu_soc_data en7581_npu_soc_data = {
+	.fw_rv32 = {
+		.name = NPU_EN7581_FIRMWARE_RV32,
+		.max_size = NPU_EN7581_FIRMWARE_RV32_MAX_SIZE,
+	},
+	.fw_data = {
+		.name = NPU_EN7581_FIRMWARE_DATA,
+		.max_size = NPU_EN7581_FIRMWARE_DATA_MAX_SIZE,
+	},
+};
+
 static const struct of_device_id of_airoha_npu_match[] = {
-	{ .compatible = "airoha,en7581-npu" },
+	{ .compatible = "airoha,en7581-npu", .data = &en7581_npu_soc_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, of_airoha_npu_match);

-- 
2.51.0

[PATCH net-next v3 3/3] net: airoha: npu: Add 7583 SoC support

[Lorenzo Bianconi]

Introduce support for Airoha 7583 SoC NPU selecting proper firmware images.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_npu.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/airoha/airoha_npu.c b/drivers/net/ethernet/airoha/airoha_npu.c
index 41944cc5f6b062129528e9357511fd9e05cbe1e6..68b7f9684dc7f3912493876ae937207f55b81330 100644
--- a/drivers/net/ethernet/airoha/airoha_npu.c
+++ b/drivers/net/ethernet/airoha/airoha_npu.c
@@ -16,6 +16,8 @@
 
 #define NPU_EN7581_FIRMWARE_DATA		"airoha/en7581_npu_data.bin"
 #define NPU_EN7581_FIRMWARE_RV32		"airoha/en7581_npu_rv32.bin"
+#define NPU_AN7583_FIRMWARE_DATA		"airoha/an7583_npu_data.bin"
+#define NPU_AN7583_FIRMWARE_RV32		"airoha/an7583_npu_rv32.bin"
 #define NPU_EN7581_FIRMWARE_RV32_MAX_SIZE	0x200000
 #define NPU_EN7581_FIRMWARE_DATA_MAX_SIZE	0x10000
 #define NPU_DUMP_SIZE				512
@@ -622,8 +624,20 @@ static const struct airoha_npu_soc_data en7581_npu_soc_data = {
 	},
 };
 
+static const struct airoha_npu_soc_data an7583_npu_soc_data = {
+	.fw_rv32 = {
+		.name = NPU_AN7583_FIRMWARE_RV32,
+		.max_size = NPU_EN7581_FIRMWARE_RV32_MAX_SIZE,
+	},
+	.fw_data = {
+		.name = NPU_AN7583_FIRMWARE_DATA,
+		.max_size = NPU_EN7581_FIRMWARE_DATA_MAX_SIZE,
+	},
+};
+
 static const struct of_device_id of_airoha_npu_match[] = {
 	{ .compatible = "airoha,en7581-npu", .data = &en7581_npu_soc_data },
+	{ .compatible = "airoha,an7583-npu", .data = &an7583_npu_soc_data },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, of_airoha_npu_match);
@@ -762,6 +776,8 @@ module_platform_driver(airoha_npu_driver);
 
 MODULE_FIRMWARE(NPU_EN7581_FIRMWARE_DATA);
 MODULE_FIRMWARE(NPU_EN7581_FIRMWARE_RV32);
+MODULE_FIRMWARE(NPU_AN7583_FIRMWARE_DATA);
+MODULE_FIRMWARE(NPU_AN7583_FIRMWARE_RV32);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Lorenzo Bianconi <lorenzo@kernel.org>");
 MODULE_DESCRIPTION("Airoha Network Processor Unit driver");

-- 
2.51.0

Re: [PATCH net-next v3 2/3] net: airoha: npu: Add airoha_npu_soc_data struct

[Simon Horman]

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)

...

Re: [PATCH net-next v3 3/3] net: airoha: npu: Add 7583 SoC support

[Simon Horman]

On Mon, Oct 13, 2025 at 03:58:51PM +0200, Lorenzo Bianconi wrote:
> Introduce support for Airoha 7583 SoC NPU selecting proper firmware images.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v3 2/3] net: airoha: npu: Add airoha_npu_soc_data struct

[Lorenzo Bianconi]

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

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

Hi Simon,

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

release_firmware() is intended to release the resources allocated by the
corresponding call to request_firmware() in airoha_npu_load_firmware().
According to my understanding we always run release_firmware() in
airoha_npu_load_firmware() before returning to the caller. Even before this
patch we run release_firmware() on the 'first' firmware image before requesting
the second one. Am I missing something?

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

For the above reasons, it is not important to call release_firmware() removing
the module. Agree?

Regards,
Lorenzo

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v3 2/3] net: airoha: npu: Add airoha_npu_soc_data struct

[Simon Horman]

On Tue, Oct 14, 2025 at 11:23:37AM +0200, Lorenzo Bianconi wrote:
> > 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,
> 
> Hi Simon,
> 
> > 
> > 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.
> 
> release_firmware() is intended to release the resources allocated by the
> corresponding call to request_firmware() in airoha_npu_load_firmware().
> According to my understanding we always run release_firmware() in
> airoha_npu_load_firmware() before returning to the caller. Even before this
> patch we run release_firmware() on the 'first' firmware image before requesting
> the second one. Am I missing something?
> 
> > 
> > Also, not strictly related. Should release_firmware() be called (twice)
> > when the driver is removed?
> 
> For the above reasons, it is not important to call release_firmware() removing
> the module. Agree?

Thanks, agreed.

For some reason I missed that release_firmware() is called in
airoha_npu_load_firmware() regardless of error - I thought it was only
in error paths for some reason.

So I agree that the firmware is always released by the time
airoha_npu_load_firmware() is returned. As thus there is never
a need to release it afterwards.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v3 2/3] net: airoha: npu: Add airoha_npu_soc_data struct

[Lorenzo Bianconi]

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

On Oct 14, Simon Horman wrote:
> On Tue, Oct 14, 2025 at 11:23:37AM +0200, Lorenzo Bianconi wrote:
> > > 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,
> > 
> > Hi Simon,
> > 
> > > 
> > > 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.
> > 
> > release_firmware() is intended to release the resources allocated by the
> > corresponding call to request_firmware() in airoha_npu_load_firmware().
> > According to my understanding we always run release_firmware() in
> > airoha_npu_load_firmware() before returning to the caller. Even before this
> > patch we run release_firmware() on the 'first' firmware image before requesting
> > the second one. Am I missing something?
> > 
> > > 
> > > Also, not strictly related. Should release_firmware() be called (twice)
> > > when the driver is removed?
> > 
> > For the above reasons, it is not important to call release_firmware() removing
> > the module. Agree?
> 
> Thanks, agreed.
> 
> For some reason I missed that release_firmware() is called in
> airoha_npu_load_firmware() regardless of error - I thought it was only
> in error paths for some reason.
> 
> So I agree that the firmware is always released by the time
> airoha_npu_load_firmware() is returned. As thus there is never
> a need to release it afterwards.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> 

ack, thx for the review.

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v3 0/3] net: airoha: npu: Introduce support for Airoha 7583 NPU

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 13 Oct 2025 15:58:48 +0200 you wrote:
> Introduce support for Airoha 7583 SoC NPU.
> 
> ---
> Changes in v3:
> - Rebase on top of net-next
> - Link to v2: https://lore.kernel.org/r/20250927-airoha-npu-7583-v2-0-e12fac5cce1f@kernel.org
> 
> [...]

Here is the summary with links:
  - [net-next,v3,1/3] dt-bindings: net: airoha: npu: Add AN7583 support
    https://git.kernel.org/netdev/net-next/c/9fbafbfa5b99
  - [net-next,v3,2/3] net: airoha: npu: Add airoha_npu_soc_data struct
    https://git.kernel.org/netdev/net-next/c/0850ae496d53
  - [net-next,v3,3/3] net: airoha: npu: Add 7583 SoC support
    https://git.kernel.org/netdev/net-next/c/4478596f71d9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html