| Message ID | 20240522063652.3759497-2-danishanwar@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Enable ICSSG Driver for AM64x | expand |
On 5/22/24 1:36 AM, MD Danish Anwar wrote: > Add remoteproc-name property for PRU cores. > > Signed-off-by: MD Danish Anwar <danishanwar@ti.com> > --- > arch/arm/dts/k3-am642-evm-u-boot.dtsi | 44 +++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/arch/arm/dts/k3-am642-evm-u-boot.dtsi b/arch/arm/dts/k3-am642-evm-u-boot.dtsi > index 705b3baa81..25ddace74e 100644 > --- a/arch/arm/dts/k3-am642-evm-u-boot.dtsi > +++ b/arch/arm/dts/k3-am642-evm-u-boot.dtsi > @@ -11,6 +11,50 @@ > }; > }; > > +&pru0_0 { > + remoteproc-name = "pru0_0"; Why do we need all these? Looks like we fallback to using `dev->name` if these are not set, does that not work for us here? Andrew > +}; > + > +&rtu0_0 { > + remoteproc-name = "rtu0_0"; > +}; > + > +&tx_pru0_0 { > + remoteproc-name = "tx_pru0_0"; > +}; > + > +&pru0_1 { > + remoteproc-name = "pru0_1"; > +}; > + > +&rtu0_1 { > + remoteproc-name = "rtu0_1"; > +}; > + > +&tx_pru0_1 { > + remoteproc-name = "tx_pru0_1"; > +}; > + > +&pru1_0 { > + remoteproc-name = "pru1_0"; > +}; > + > +&rtu1_0 { > + remoteproc-name = "rtu1_0"; > +}; > + > +&tx_pru1_0 { > + remoteproc-name = "tx_pru1_0"; > +}; > + > +&pru1_1 { > + remoteproc-name = "pru1_1"; > +}; > + > +&rtu1_1 { > + remoteproc-name = "rtu1_1"; > +}; > + > &main_timer0 { > clock-frequency = <200000000>; > };
Hi Andrew, On 5/22/2024 9:38 PM, Andrew Davis wrote: > On 5/22/24 1:36 AM, MD Danish Anwar wrote: >> Add remoteproc-name property for PRU cores. >> >> Signed-off-by: MD Danish Anwar <danishanwar@ti.com> >> --- >> arch/arm/dts/k3-am642-evm-u-boot.dtsi | 44 +++++++++++++++++++++++++++ >> 1 file changed, 44 insertions(+) >> >> diff --git a/arch/arm/dts/k3-am642-evm-u-boot.dtsi >> b/arch/arm/dts/k3-am642-evm-u-boot.dtsi >> index 705b3baa81..25ddace74e 100644 >> --- a/arch/arm/dts/k3-am642-evm-u-boot.dtsi >> +++ b/arch/arm/dts/k3-am642-evm-u-boot.dtsi >> @@ -11,6 +11,50 @@ >> }; >> }; >> +&pru0_0 { >> + remoteproc-name = "pru0_0"; > > Why do we need all these? Looks like we fallback to using `dev->name` if > these are not set, does that not work for us here? > Yes we will fallback to `dev->name` if `remoteproc-name` is not set but in our case two different PRU cores will have same `dev->name`. As a result the API rproc_name_is_unique() will return false and as a result the probe will fail for the PRU cores. Example: In k3-am64-main.dtsi, both pru0_0 [1] and pru1_0 [2] will have dev->name as "pru@34000" same goes for rtu and txpru as well. pru0_0: pru@34000 { compatible = "ti,am642-pru"; reg = <0x34000 0x3000>, <0x22000 0x100>, <0x22400 0x100>; reg-names = "iram", "control", "debug"; firmware-name = "am64x-pru0_0-fw"; }; pru1_0: pru@34000 { compatible = "ti,am642-pru"; reg = <0x34000 0x4000>, <0x22000 0x100>, <0x22400 0x100>; reg-names = "iram", "control", "debug"; firmware-name = "am64x-pru1_0-fw"; }; To avoid this, we are setting the "remoteproc-name" property in -u-boot.dtsi. Same is done for AM65x as well [3]. [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://elixir.bootlin.com/u-boot/v2024.07-rc3/source/arch/arm/dts/k3-am654-base-board-u-boot.dtsi#L168 > Andrew > >> +}; >> + >> +&rtu0_0 { >> + remoteproc-name = "rtu0_0"; >> +}; >> + >> +&tx_pru0_0 { >> + remoteproc-name = "tx_pru0_0"; >> +}; >> + >> +&pru0_1 { >> + remoteproc-name = "pru0_1"; >> +}; >> + >> +&rtu0_1 { >> + remoteproc-name = "rtu0_1"; >> +}; >> + >> +&tx_pru0_1 { >> + remoteproc-name = "tx_pru0_1"; >> +}; >> + >> +&pru1_0 { >> + remoteproc-name = "pru1_0"; >> +}; >> + >> +&rtu1_0 { >> + remoteproc-name = "rtu1_0"; >> +}; >> + >> +&tx_pru1_0 { >> + remoteproc-name = "tx_pru1_0"; >> +}; >> + >> +&pru1_1 { >> + remoteproc-name = "pru1_1"; >> +}; >> + >> +&rtu1_1 { >> + remoteproc-name = "rtu1_1"; >> +}; >> + >> &main_timer0 { >> clock-frequency = <200000000>; >> };
On 5/23/24 1:33 AM, Anwar, Md Danish wrote: > Hi Andrew, > > On 5/22/2024 9:38 PM, Andrew Davis wrote: >> On 5/22/24 1:36 AM, MD Danish Anwar wrote: >>> Add remoteproc-name property for PRU cores. >>> >>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com> >>> --- >>> arch/arm/dts/k3-am642-evm-u-boot.dtsi | 44 +++++++++++++++++++++++++++ >>> 1 file changed, 44 insertions(+) >>> >>> diff --git a/arch/arm/dts/k3-am642-evm-u-boot.dtsi >>> b/arch/arm/dts/k3-am642-evm-u-boot.dtsi >>> index 705b3baa81..25ddace74e 100644 >>> --- a/arch/arm/dts/k3-am642-evm-u-boot.dtsi >>> +++ b/arch/arm/dts/k3-am642-evm-u-boot.dtsi >>> @@ -11,6 +11,50 @@ >>> }; >>> }; >>> +&pru0_0 { >>> + remoteproc-name = "pru0_0"; >> >> Why do we need all these? Looks like we fallback to using `dev->name` if >> these are not set, does that not work for us here? >> > > Yes we will fallback to `dev->name` if `remoteproc-name` is not set but > in our case two different PRU cores will have same `dev->name`. As a > result the API rproc_name_is_unique() will return false and as a result > the probe will fail for the PRU cores. > If `dev->name` is not unique, then combine it with something that is, or remove the requirement for the name to be unique. Right now this looks like just a hack around a framework requirement and a driver problem. If you really think this is something that should be solved in DT then go convince the DT maintainers and get it into the kernel DT. We should be reducing the deltas we carry in -u-boot.dtsi files, not adding more workarounds. Andrew > Example: In k3-am64-main.dtsi, both pru0_0 [1] and pru1_0 [2] will have > dev->name as "pru@34000" same goes for rtu and txpru as well. > > pru0_0: pru@34000 { > compatible = "ti,am642-pru"; > reg = <0x34000 0x3000>, > <0x22000 0x100>, > <0x22400 0x100>; > reg-names = "iram", "control", "debug"; > firmware-name = "am64x-pru0_0-fw"; > }; > > pru1_0: pru@34000 { > compatible = "ti,am642-pru"; > reg = <0x34000 0x4000>, > <0x22000 0x100>, > <0x22400 0x100>; > reg-names = "iram", "control", "debug"; > firmware-name = "am64x-pru1_0-fw"; > }; > > To avoid this, we are setting the "remoteproc-name" property in > -u-boot.dtsi. Same is done for AM65x as well [3]. > > [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://elixir.bootlin.com/u-boot/v2024.07-rc3/source/arch/arm/dts/k3-am654-base-board-u-boot.dtsi#L168 > >> Andrew >> >>> +}; >>> + >>> +&rtu0_0 { >>> + remoteproc-name = "rtu0_0"; >>> +}; >>> + >>> +&tx_pru0_0 { >>> + remoteproc-name = "tx_pru0_0"; >>> +}; >>> + >>> +&pru0_1 { >>> + remoteproc-name = "pru0_1"; >>> +}; >>> + >>> +&rtu0_1 { >>> + remoteproc-name = "rtu0_1"; >>> +}; >>> + >>> +&tx_pru0_1 { >>> + remoteproc-name = "tx_pru0_1"; >>> +}; >>> + >>> +&pru1_0 { >>> + remoteproc-name = "pru1_0"; >>> +}; >>> + >>> +&rtu1_0 { >>> + remoteproc-name = "rtu1_0"; >>> +}; >>> + >>> +&tx_pru1_0 { >>> + remoteproc-name = "tx_pru1_0"; >>> +}; >>> + >>> +&pru1_1 { >>> + remoteproc-name = "pru1_1"; >>> +}; >>> + >>> +&rtu1_1 { >>> + remoteproc-name = "rtu1_1"; >>> +}; >>> + >>> &main_timer0 { >>> clock-frequency = <200000000>; >>> }; >
diff --git a/arch/arm/dts/k3-am642-evm-u-boot.dtsi b/arch/arm/dts/k3-am642-evm-u-boot.dtsi index 705b3baa81..25ddace74e 100644 --- a/arch/arm/dts/k3-am642-evm-u-boot.dtsi +++ b/arch/arm/dts/k3-am642-evm-u-boot.dtsi @@ -11,6 +11,50 @@ }; }; +&pru0_0 { + remoteproc-name = "pru0_0"; +}; + +&rtu0_0 { + remoteproc-name = "rtu0_0"; +}; + +&tx_pru0_0 { + remoteproc-name = "tx_pru0_0"; +}; + +&pru0_1 { + remoteproc-name = "pru0_1"; +}; + +&rtu0_1 { + remoteproc-name = "rtu0_1"; +}; + +&tx_pru0_1 { + remoteproc-name = "tx_pru0_1"; +}; + +&pru1_0 { + remoteproc-name = "pru1_0"; +}; + +&rtu1_0 { + remoteproc-name = "rtu1_0"; +}; + +&tx_pru1_0 { + remoteproc-name = "tx_pru1_0"; +}; + +&pru1_1 { + remoteproc-name = "pru1_1"; +}; + +&rtu1_1 { + remoteproc-name = "rtu1_1"; +}; + &main_timer0 { clock-frequency = <200000000>; };
Add remoteproc-name property for PRU cores. Signed-off-by: MD Danish Anwar <danishanwar@ti.com> --- arch/arm/dts/k3-am642-evm-u-boot.dtsi | 44 +++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)