On 05/08/24 12:08 pm, Roger Quadros wrote:
On 19/07/2024 11:59, 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@ti.com
Cc: Andrew Davis afd@ti.com
Failure Example: In k3-am64-main.dtsi, both pru0_0 [1] and pru1_0 [2] will have dev->name as "pru@34000" although their parent name is different.
pru0_0 has dev->name as "pru@34000" and parent name as "icssg@30000000" pru1_0 has dev->name as "pru@34000" and parent name as "icssg@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@ti.com [4]
Failure Logs: rproc_pre_probe: 'pru@34000': using fdt rproc_pre_probe: 'pru@34000': using fdt rproc_pre_probe: pru@34000 duplicate name 'pru@34000' _rproc_probe_dev: pru@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@34000:icssg@30000000" pru1_0 uc->pdata->name = "pru@34000:icssg@30080000"
Both the names are unique, thus rproc_init() succeeds.
[1] https://elixir.bootlin.com/u-boot/v2024.07-rc3/source/dts/upstream/src/arm64... [2] https://elixir.bootlin.com/u-boot/v2024.07-rc3/source/dts/upstream/src/arm64... [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 | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c index 3ba2b40dca..e550292dda 100644 --- a/drivers/remoteproc/rproc-uclass.c +++ b/drivers/remoteproc/rproc-uclass.c @@ -158,9 +158,15 @@ 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) {
int rproc_name_size = 256;Instead of hardcoding 256 why not use buf = malloc(strlen(dev->name) + strlen(dev->parent->name) + 2); // 1 for null and one for '-'
char *buf;buf = malloc(rproc_name_size);need to check/error out for malloc failure.
Sure Roger, I will make these changes and update the patch. Thanks for the review.
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;