[3/3] net: ipa: Grab IMEM slice base/size from DTS

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

This is a detail that differ per chip, and not per IPA version (and
there are cases of the same IPA versions being implemented across very
very very different SoCs).

This region isn't actually used by the driver, but we most definitely
want to iommu-map it, so that IPA can poke at the data within.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/net/ipa/ipa_data.h |  3 +++
 drivers/net/ipa/ipa_mem.c  | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

On Fri, May 23, 2025 at 01:08:34AM +0200, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> This is a detail that differ per chip, and not per IPA version (and
> there are cases of the same IPA versions being implemented across very
> very very different SoCs).
> 
> This region isn't actually used by the driver, but we most definitely
> want to iommu-map it, so that IPA can poke at the data within.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

It looks like these patches are for net-next. For future reference,
it's best to note that in the subject.

  Subject: [PATCH net-next 3/3 v2] ...

> ---
>  drivers/net/ipa/ipa_data.h |  3 +++
>  drivers/net/ipa/ipa_mem.c  | 18 ++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h
> index 2fd03f0799b207833f9f2b421ce043534720d718..a384df91b5ee3ed2db9c7812ad43d03424b82a6f 100644
> --- a/drivers/net/ipa/ipa_data.h
> +++ b/drivers/net/ipa/ipa_data.h
> @@ -185,8 +185,11 @@ struct ipa_resource_data {
>  struct ipa_mem_data {
>  	u32 local_count;
>  	const struct ipa_mem *local;
> +
> +	/* DEPRECATED (now passed via DT) fallback data, varies per chip and not per IPA version */

For Networking code, please restrict lines to 80 columns wide or less where
that can be done without reducing readability (which is the case here).

	/* DEPRECATED (now passed via DT) fallback data,
	 * varies per chip and not per IPA version */

>  	u32 imem_addr;
>  	u32 imem_size;
> +
>  	u32 smem_size;
>  };
>  
> diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c
> index 835a3c9c1fd47167da3396424a1653ebcae81d40..020508ab47d92b5cca9d5b467e3fef46936b4a82 100644
> --- a/drivers/net/ipa/ipa_mem.c
> +++ b/drivers/net/ipa/ipa_mem.c
> @@ -7,6 +7,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/io.h>
>  #include <linux/iommu.h>
> +#include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  #include <linux/types.h>
>  
> @@ -617,7 +618,9 @@ static void ipa_smem_exit(struct ipa *ipa)
>  int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
>  		 const struct ipa_mem_data *mem_data)
>  {
> +	struct device_node *ipa_slice_np;
>  	struct device *dev = &pdev->dev;
> +	u32 imem_base, imem_size;
>  	struct resource *res;
>  	int ret;
>  
> @@ -656,6 +659,21 @@ int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
>  	ipa->mem_addr = res->start;
>  	ipa->mem_size = resource_size(res);
>  
> +	ipa_slice_np = of_parse_phandle(dev->of_node, "sram", 0);
> +	if (ipa_slice_np) {
> +		ret = of_address_to_resource(ipa_slice_np, 0, res);
> +		of_node_put(ipa_slice_np);
> +		if (ret)
> +			return ret;
> +
> +		imem_base = res->start;
> +		imem_size = resource_size(res);
> +	} else {
> +		/* Backwards compatibility for DTs lacking an explicit reference */

Ditto.

> +		imem_base = mem_data->imem_addr;
> +		imem_size = mem_data->imem_size;
> +	}
> +
>  	ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size);

I think you also need to update this line to use the local
variables imem_addr and imem_size.

>  	if (ret)
>  		goto err_unmap;

Please do observe the 24h rule [1] if you post a v2 of this patchset.

[1] https://docs.kernel.org/process/maintainer-netdev.html#resending-after-review

On 5/22/25 6:08 PM, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> This is a detail that differ per chip, and not per IPA version (and
> there are cases of the same IPA versions being implemented across very
> very very different SoCs).
> 
> This region isn't actually used by the driver, but we most definitely
> want to iommu-map it, so that IPA can poke at the data within.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

You need to fix something here, but it otherwise look good.

Please fix, and assuming you do:

Reviewed-by: Alex Elder <elder@riscstar.com>

> ---
>   drivers/net/ipa/ipa_data.h |  3 +++
>   drivers/net/ipa/ipa_mem.c  | 18 ++++++++++++++++++
>   2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h
> index 2fd03f0799b207833f9f2b421ce043534720d718..a384df91b5ee3ed2db9c7812ad43d03424b82a6f 100644
> --- a/drivers/net/ipa/ipa_data.h
> +++ b/drivers/net/ipa/ipa_data.h
> @@ -185,8 +185,11 @@ struct ipa_resource_data {
>   struct ipa_mem_data {
>   	u32 local_count;
>   	const struct ipa_mem *local;
> +
> +	/* DEPRECATED (now passed via DT) fallback data, varies per chip and not per IPA version */
>   	u32 imem_addr;
>   	u32 imem_size;
> +
>   	u32 smem_size;
>   };
>   
> diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c
> index 835a3c9c1fd47167da3396424a1653ebcae81d40..020508ab47d92b5cca9d5b467e3fef46936b4a82 100644
> --- a/drivers/net/ipa/ipa_mem.c
> +++ b/drivers/net/ipa/ipa_mem.c
> @@ -7,6 +7,7 @@
>   #include <linux/dma-mapping.h>
>   #include <linux/io.h>
>   #include <linux/iommu.h>
> +#include <linux/of_address.h>
>   #include <linux/platform_device.h>
>   #include <linux/types.h>
>   
> @@ -617,7 +618,9 @@ static void ipa_smem_exit(struct ipa *ipa)
>   int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
>   		 const struct ipa_mem_data *mem_data)
>   {
> +	struct device_node *ipa_slice_np;
>   	struct device *dev = &pdev->dev;
> +	u32 imem_base, imem_size;
>   	struct resource *res;
>   	int ret;
>   
> @@ -656,6 +659,21 @@ int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
>   	ipa->mem_addr = res->start;
>   	ipa->mem_size = resource_size(res);
>   
> +	ipa_slice_np = of_parse_phandle(dev->of_node, "sram", 0);
> +	if (ipa_slice_np) {
> +		ret = of_address_to_resource(ipa_slice_np, 0, res);
> +		of_node_put(ipa_slice_np);
> +		if (ret)
> +			return ret;
> +
> +		imem_base = res->start;
> +		imem_size = resource_size(res);
> +	} else {
> +		/* Backwards compatibility for DTs lacking an explicit reference */
> +		imem_base = mem_data->imem_addr;
> +		imem_size = mem_data->imem_size;
> +	}
> +
>   	ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size);

You want to pass imem_base and imem_size here?

					-Alex

>   	if (ret)
>   		goto err_unmap;
>

On 5/23/25 3:17 PM, Simon Horman wrote:
> On Fri, May 23, 2025 at 01:08:34AM +0200, Konrad Dybcio wrote:
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> This is a detail that differ per chip, and not per IPA version (and
>> there are cases of the same IPA versions being implemented across very
>> very very different SoCs).
>>
>> This region isn't actually used by the driver, but we most definitely
>> want to iommu-map it, so that IPA can poke at the data within.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> It looks like these patches are for net-next. For future reference,
> it's best to note that in the subject.
> 
>   Subject: [PATCH net-next 3/3 v2] ...
> 
>> ---

ah, the networking guys and their customs ;)

[...]

>>  	ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size);
> 
> I think you also need to update this line to use the local
> variables imem_addr and imem_size.

I paid great attention to validate that the data I got was good and printed
the value inside the first if branch.. but failed to change it here. Thanks
for catching it!

Konrad

Hi Konrad,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 460178e842c7a1e48a06df684c66eb5fd630bcf7]

url:    https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/dt-bindings-sram-qcom-imem-Allow-modem-tables/20250523-071359
base:   460178e842c7a1e48a06df684c66eb5fd630bcf7
patch link:    https://lore.kernel.org/r/20250523-topic-ipa_imem-v1-3-b5d536291c7f%40oss.qualcomm.com
patch subject: [PATCH 3/3] net: ipa: Grab IMEM slice base/size from DTS
config: xtensa-randconfig-002-20250524 (https://download.01.org/0day-ci/archive/20250524/202505240200.w0D4DdAY-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250524/202505240200.w0D4DdAY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505240200.w0D4DdAY-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/ipa/ipa_mem.c: In function 'ipa_mem_init':
>> drivers/net/ipa/ipa_mem.c:623:17: warning: variable 'imem_size' set but not used [-Wunused-but-set-variable]
     u32 imem_base, imem_size;
                    ^~~~~~~~~
>> drivers/net/ipa/ipa_mem.c:623:6: warning: variable 'imem_base' set but not used [-Wunused-but-set-variable]
     u32 imem_base, imem_size;
         ^~~~~~~~~


vim +/imem_size +623 drivers/net/ipa/ipa_mem.c

   616	
   617	/* Perform memory region-related initialization */
   618	int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev,
   619			 const struct ipa_mem_data *mem_data)
   620	{
   621		struct device_node *ipa_slice_np;
   622		struct device *dev = &pdev->dev;
 > 623		u32 imem_base, imem_size;
   624		struct resource *res;
   625		int ret;
   626	
   627		/* Make sure the set of defined memory regions is valid */
   628		if (!ipa_mem_valid(ipa, mem_data))
   629			return -EINVAL;
   630	
   631		ipa->mem_count = mem_data->local_count;
   632		ipa->mem = mem_data->local;
   633	
   634		/* Check the route and filter table memory regions */
   635		if (!ipa_table_mem_valid(ipa, false))
   636			return -EINVAL;
   637		if (!ipa_table_mem_valid(ipa, true))
   638			return -EINVAL;
   639	
   640		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
   641		if (ret) {
   642			dev_err(dev, "error %d setting DMA mask\n", ret);
   643			return ret;
   644		}
   645	
   646		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ipa-shared");
   647		if (!res) {
   648			dev_err(dev,
   649				"DT error getting \"ipa-shared\" memory property\n");
   650			return -ENODEV;
   651		}
   652	
   653		ipa->mem_virt = memremap(res->start, resource_size(res), MEMREMAP_WC);
   654		if (!ipa->mem_virt) {
   655			dev_err(dev, "unable to remap \"ipa-shared\" memory\n");
   656			return -ENOMEM;
   657		}
   658	
   659		ipa->mem_addr = res->start;
   660		ipa->mem_size = resource_size(res);
   661	
   662		ipa_slice_np = of_parse_phandle(dev->of_node, "sram", 0);
   663		if (ipa_slice_np) {
   664			ret = of_address_to_resource(ipa_slice_np, 0, res);
   665			of_node_put(ipa_slice_np);
   666			if (ret)
   667				return ret;
   668	
   669			imem_base = res->start;
   670			imem_size = resource_size(res);
   671		} else {
   672			/* Backwards compatibility for DTs lacking an explicit reference */
   673			imem_base = mem_data->imem_addr;
   674			imem_size = mem_data->imem_size;
   675		}
   676	
   677		ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size);
   678		if (ret)
   679			goto err_unmap;
   680	
   681		ret = ipa_smem_init(ipa, mem_data->smem_size);
   682		if (ret)
   683			goto err_imem_exit;
   684	
   685		return 0;
   686	
   687	err_imem_exit:
   688		ipa_imem_exit(ipa);
   689	err_unmap:
   690		memunmap(ipa->mem_virt);
   691	
   692		return ret;
   693	}
   694