* [PATCH] remoteproc: uclass: Modify uc_pdata->name to use combination of device name and device's parent name
@ 2024-07-19 8:59 MD Danish Anwar
2024-08-05 6:38 ` Roger Quadros
0 siblings, 1 reply; 3+ messages in thread
From: MD Danish Anwar @ 2024-07-19 8:59 UTC (permalink / raw)
To: Dan Carpenter, Nishanth Menon, MD Danish Anwar, Simon Glass,
Tom Rini
Cc: u-boot, srk, Vignesh Raghavendra, Roger Quadros, Andrew Davis
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;
+ 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;
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] remoteproc: uclass: Modify uc_pdata->name to use combination of device name and device's parent name
2024-07-19 8:59 [PATCH] remoteproc: uclass: Modify uc_pdata->name to use combination of device name and device's parent name MD Danish Anwar
@ 2024-08-05 6:38 ` Roger Quadros
2024-08-07 8:41 ` MD Danish Anwar
0 siblings, 1 reply; 3+ messages in thread
From: Roger Quadros @ 2024-08-05 6:38 UTC (permalink / raw)
To: MD Danish Anwar, Dan Carpenter, Nishanth Menon, Simon Glass,
Tom Rini
Cc: u-boot, srk, Vignesh Raghavendra, Andrew Davis
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;
--
cheers,
-roger
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] remoteproc: uclass: Modify uc_pdata->name to use combination of device name and device's parent name
2024-08-05 6:38 ` Roger Quadros
@ 2024-08-07 8:41 ` MD Danish Anwar
0 siblings, 0 replies; 3+ messages in thread
From: MD Danish Anwar @ 2024-08-07 8:41 UTC (permalink / raw)
To: Roger Quadros, Dan Carpenter, Nishanth Menon, Simon Glass,
Tom Rini
Cc: u-boot, srk, Vignesh Raghavendra, Andrew Davis
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;
>
--
Thanks and Regards,
Danish
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-07 8:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-19 8:59 [PATCH] remoteproc: uclass: Modify uc_pdata->name to use combination of device name and device's parent name MD Danish Anwar
2024-08-05 6:38 ` Roger Quadros
2024-08-07 8:41 ` MD Danish Anwar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox