Re: [PATCH v9 01/26] remoteproc: k3-r5: Re-order internal memory initialization function

[Andrew Davis <afd@ti.com>]

On 3/17/25 7:05 AM, Beleswar Padhi wrote:
> The core's internal memory data structure will be refactored to be part
> of the k3_r5_rproc structure in a future commit. As a result, internal
> memory initialization will need to be performed inside
> k3_r5_cluster_rproc_init() after rproc_alloc().
> 
> Therefore, move the internal memory initialization function,
> k3_r5_core_of_get_internal_memories() above k3_r5_rproc_init() so that
> it can be invoked from there.
> 
> Signed-off-by: Beleswar Padhi <b-padhi@ti.com>
> ---

Just to keep things organized, does it make sense to also move
the other k3_r5_core_of_get_*_memories() up with this?

Also, you move k3_r5_release_tsp() up too but don't mention
that in the commit message.

Andrew

>   drivers/remoteproc/ti_k3_r5_remoteproc.c | 158 +++++++++++------------
>   1 file changed, 79 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> index dbc513c5569c..b2738b9a1b2d 100644
> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> @@ -1199,6 +1199,85 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc *kproc)
>   	return ret;
>   }
>   
> +static int k3_r5_core_of_get_internal_memories(struct platform_device *pdev,
> +					       struct k3_r5_core *core)
> +{
> +	static const char * const mem_names[] = {"atcm", "btcm"};
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	int num_mems;
> +	int i;
> +
> +	num_mems = ARRAY_SIZE(mem_names);
> +	core->mem = devm_kcalloc(dev, num_mems, sizeof(*core->mem), GFP_KERNEL);
> +	if (!core->mem)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < num_mems; i++) {
> +		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +						   mem_names[i]);
> +		if (!res) {
> +			dev_err(dev, "found no memory resource for %s\n",
> +				mem_names[i]);
> +			return -EINVAL;
> +		}
> +		if (!devm_request_mem_region(dev, res->start,
> +					     resource_size(res),
> +					     dev_name(dev))) {
> +			dev_err(dev, "could not request %s region for resource\n",
> +				mem_names[i]);
> +			return -EBUSY;
> +		}
> +
> +		/*
> +		 * TCMs are designed in general to support RAM-like backing
> +		 * memories. So, map these as Normal Non-Cached memories. This
> +		 * also avoids/fixes any potential alignment faults due to
> +		 * unaligned data accesses when using memcpy() or memset()
> +		 * functions (normally seen with device type memory).
> +		 */
> +		core->mem[i].cpu_addr = devm_ioremap_wc(dev, res->start,
> +							resource_size(res));
> +		if (!core->mem[i].cpu_addr) {
> +			dev_err(dev, "failed to map %s memory\n", mem_names[i]);
> +			return -ENOMEM;
> +		}
> +		core->mem[i].bus_addr = res->start;
> +
> +		/*
> +		 * TODO:
> +		 * The R5F cores can place ATCM & BTCM anywhere in its address
> +		 * based on the corresponding Region Registers in the System
> +		 * Control coprocessor. For now, place ATCM and BTCM at
> +		 * addresses 0 and 0x41010000 (same as the bus address on AM65x
> +		 * SoCs) based on loczrama setting
> +		 */
> +		if (!strcmp(mem_names[i], "atcm")) {
> +			core->mem[i].dev_addr = core->loczrama ?
> +							0 : K3_R5_TCM_DEV_ADDR;
> +		} else {
> +			core->mem[i].dev_addr = core->loczrama ?
> +							K3_R5_TCM_DEV_ADDR : 0;
> +		}
> +		core->mem[i].size = resource_size(res);
> +
> +		dev_dbg(dev, "memory %5s: bus addr %pa size 0x%zx va %pK da 0x%x\n",
> +			mem_names[i], &core->mem[i].bus_addr,
> +			core->mem[i].size, core->mem[i].cpu_addr,
> +			core->mem[i].dev_addr);
> +	}
> +	core->num_mems = num_mems;
> +
> +	return 0;
> +}
> +
> +static void k3_r5_release_tsp(void *data)
> +{
> +	struct ti_sci_proc *tsp = data;
> +
> +	ti_sci_proc_release(tsp);
> +}
> +
>   static int k3_r5_cluster_rproc_init(struct platform_device *pdev)
>   {
>   	struct k3_r5_cluster *cluster = platform_get_drvdata(pdev);
> @@ -1358,78 +1437,6 @@ static void k3_r5_cluster_rproc_exit(void *data)
>   	}
>   }
>   
> -static int k3_r5_core_of_get_internal_memories(struct platform_device *pdev,
> -					       struct k3_r5_core *core)
> -{
> -	static const char * const mem_names[] = {"atcm", "btcm"};
> -	struct device *dev = &pdev->dev;
> -	struct resource *res;
> -	int num_mems;
> -	int i;
> -
> -	num_mems = ARRAY_SIZE(mem_names);
> -	core->mem = devm_kcalloc(dev, num_mems, sizeof(*core->mem), GFP_KERNEL);
> -	if (!core->mem)
> -		return -ENOMEM;
> -
> -	for (i = 0; i < num_mems; i++) {
> -		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> -						   mem_names[i]);
> -		if (!res) {
> -			dev_err(dev, "found no memory resource for %s\n",
> -				mem_names[i]);
> -			return -EINVAL;
> -		}
> -		if (!devm_request_mem_region(dev, res->start,
> -					     resource_size(res),
> -					     dev_name(dev))) {
> -			dev_err(dev, "could not request %s region for resource\n",
> -				mem_names[i]);
> -			return -EBUSY;
> -		}
> -
> -		/*
> -		 * TCMs are designed in general to support RAM-like backing
> -		 * memories. So, map these as Normal Non-Cached memories. This
> -		 * also avoids/fixes any potential alignment faults due to
> -		 * unaligned data accesses when using memcpy() or memset()
> -		 * functions (normally seen with device type memory).
> -		 */
> -		core->mem[i].cpu_addr = devm_ioremap_wc(dev, res->start,
> -							resource_size(res));
> -		if (!core->mem[i].cpu_addr) {
> -			dev_err(dev, "failed to map %s memory\n", mem_names[i]);
> -			return -ENOMEM;
> -		}
> -		core->mem[i].bus_addr = res->start;
> -
> -		/*
> -		 * TODO:
> -		 * The R5F cores can place ATCM & BTCM anywhere in its address
> -		 * based on the corresponding Region Registers in the System
> -		 * Control coprocessor. For now, place ATCM and BTCM at
> -		 * addresses 0 and 0x41010000 (same as the bus address on AM65x
> -		 * SoCs) based on loczrama setting
> -		 */
> -		if (!strcmp(mem_names[i], "atcm")) {
> -			core->mem[i].dev_addr = core->loczrama ?
> -							0 : K3_R5_TCM_DEV_ADDR;
> -		} else {
> -			core->mem[i].dev_addr = core->loczrama ?
> -							K3_R5_TCM_DEV_ADDR : 0;
> -		}
> -		core->mem[i].size = resource_size(res);
> -
> -		dev_dbg(dev, "memory %5s: bus addr %pa size 0x%zx va %pK da 0x%x\n",
> -			mem_names[i], &core->mem[i].bus_addr,
> -			core->mem[i].size, core->mem[i].cpu_addr,
> -			core->mem[i].dev_addr);
> -	}
> -	core->num_mems = num_mems;
> -
> -	return 0;
> -}
> -
>   static int k3_r5_core_of_get_sram_memories(struct platform_device *pdev,
>   					   struct k3_r5_core *core)
>   {
> @@ -1487,13 +1494,6 @@ static int k3_r5_core_of_get_sram_memories(struct platform_device *pdev,
>   	return 0;
>   }
>   
> -static void k3_r5_release_tsp(void *data)
> -{
> -	struct ti_sci_proc *tsp = data;
> -
> -	ti_sci_proc_release(tsp);
> -}
> -
>   static int k3_r5_core_of_init(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;