| Message ID | 20240719085908.3564171-1-danishanwar@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | remoteproc: uclass: Modify uc_pdata->name to use combination of device name and device's parent name | expand |
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/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 | 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. > + 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;
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/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 | 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; >
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; + char *buf; + + buf = malloc(rproc_name_size); + 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;
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/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 | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)