[PATCH v2] remoteproc: uclass: Modify uc_pdata->name to use combination of device name and device's parent name

[Andrew Davis]

On 8/7/24 4:08 AM, MD Danish Anwar wrote:
> uc_pdata->name is populated from device tree property "remoteproc-name".
> For those devcices that don't set "remoteproc-name", uc_pdata->name
> falls back to dev->name.
> 
> If two devices have same name, this will result into uc_pdata->name not
> being unique and rproc_init() will fail.
> 
> Fix this by using combination of dev->name and dev->parent->name instead
> of using just the dev->name to populate uc_pdata->name.
> 
> Signed-off-by: MD Danish Anwar <danishanwar at ti.com>
> ---

Reviewed-by: Andrew Davis <afd at ti.com>

> Cc: Andrew Davis <afd at ti.com>
> 
> Failure Example:
> In k3-am64-main.dtsi, both pru0_0 [1] and pru1_0 [2] will have
> dev->name as "pru at 34000" although their parent name is different.
> 
> pru0_0 has dev->name as "pru at 34000" and parent name as "icssg at 30000000"
> pru1_0 has dev->name as "pru at 34000" and parent name as "icssg at 30080000"
> 
> rproc_init() fails for pru1_0 as the the uc_pdata->name becomes same as the pru0_0.
> More details on this issue can be found here [3]. It was suggested to use a
> different combination if `dev->name` is not unique by Andrew Davis <afd at ti.com> [4]
> 
> Failure Logs:
> 	rproc_pre_probe: 'pru at 34000': using fdt
> 	rproc_pre_probe: 'pru at 34000': using fdt
> 	rproc_pre_probe: pru at 34000 duplicate name 'pru at 34000'
> 	_rproc_probe_dev: pru at 34000: Failed to initialize - -22
> 	rproc_boot: rproc_init() failed: -22
> 
> To fix it, this commit uses combination of dev and dev's parent name.
> 
> After this commit,
> pru0_0 uc->pdata->name = "pru at 34000-icssg@30000000"
> pru1_0 uc->pdata->name = "pru at 34000-icssg@30080000"
> 
> Both the names are unique, thus rproc_init() succeeds.
> 
> Changes since v1:
> *) Instead of hardcoding rproc_name_size to 256, changed it to be equal to
>     strlen(dev->name) + strlen(dev->parent->name) + 2 as suggested by
>     Roger Quadros <rogerq at kernel.org>
>   *) Added check for malloc failure as suggested by Roger Quadros <rogerq at kernel.org>
> 
> [v1] https://lore.kernel.org/all/20240719085908.3564171-1-danishanwar@ti.com/
> [1] https://elixir.bootlin.com/u-boot/v2024.07-rc3/source/dts/upstream/src/arm64/ti/k3-am64-main.dtsi#L1276
> [2] https://elixir.bootlin.com/u-boot/v2024.07-rc3/source/dts/upstream/src/arm64/ti/k3-am64-main.dtsi#L1417
> [3] https://lore.kernel.org/all/5cda289f-1d14-41f6-84e3-ff1d1034be13@ti.com/
> [4] https://lore.kernel.org/all/e48f5818-182c-47ab-b384-379659830d38@ti.com/
> 
>   drivers/remoteproc/rproc-uclass.c | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c
> index e64354dd52..3233ff8041 100644
> --- a/drivers/remoteproc/rproc-uclass.c
> +++ b/drivers/remoteproc/rproc-uclass.c
> @@ -158,9 +158,19 @@ static int rproc_pre_probe(struct udevice *dev)
>   		uc_pdata->driver_plat_data = pdata->driver_plat_data;
>   	}
>   
> -	/* Else try using device Name */
> -	if (!uc_pdata->name)
> -		uc_pdata->name = dev->name;
> +	/* Else try using a combination of device Name and devices's parent's name */
> +	if (!uc_pdata->name) {
> +		/* 2 in the rproc_name_size indicates 1 for null and one for '-' */
> +		int rproc_name_size = strlen(dev->name) + strlen(dev->parent->name) + 2;
> +		char *buf;
> +
> +		buf = malloc(rproc_name_size);
> +		if (!buf)
> +			return -ENOMEM;
> +
> +		snprintf(buf, rproc_name_size, "%s-%s", dev->name, dev->parent->name);
> +		uc_pdata->name = buf;
> +	}
>   	if (!uc_pdata->name) {
>   		debug("Unnamed device!");
>   		return -EINVAL;
> 
> base-commit: 6f4c31c2b658358628b5b0fa801f55c7477c7585