| Message ID | 20231009070926.21629-2-r-gunasekaran@ti.com |
|---|---|
| State | Rejected |
| Delegated to: | Ryan Eatmon |
| Headers | show |
| Series | Remove PRU Ethernet firmwares for legacy devices | expand |
On Mon, Oct 09, 2023 at 12:39:25PM +0530, Ravi Gunasekaran via lists.yoctoproject.org wrote: > PRU-ICSS on AM33x, AM43x, supports different ethernet modes such > as Ethernet, HSR and PRP by means of different firmwares and the > driver loads one of the these firmware. The PRU-ICSS linux driver > may not be available for all kernel versions. > > So add an .inc file that avoids the packaging of such firmwares > in the SDK, so that it can be referenced by kernel recipes that > don't support PRU-ICSS. > > Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> > --- > meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc | 3 +++ > 1 file changed, 3 insertions(+) > create mode 100644 meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc > > diff --git a/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc > new file mode 100644 > index 00000000..6698c9ed > --- /dev/null > +++ b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc > @@ -0,0 +1,3 @@ > +# Do not package firmwares for kernels that don't support PRUETH > +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti43x = " prueth-fw pruhsr-fw pruprp-fw" > +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti33x = " prueth-fw pruhsr-fw pruprp-fw" This is asking for trouble - there's no way to un-do :remove downstream (e.g. any of the Distro, SDK, Product Line or Customer layers) It is not recommended to use :remove (and :append to a lesser degree) on a regular basis when it can be done differently. Moreover, what is a difference from v2? It is a more convoluted way to drop those FW images from getting installed, but it is still done unconditionally here in meta-ti - are there any benefits of doing it this way?
On 10/9/2023 3:20 PM, Denys Dmytriyenko wrote: > On Mon, Oct 09, 2023 at 12:39:25PM +0530, Ravi Gunasekaran via lists.yoctoproject.org wrote: >> PRU-ICSS on AM33x, AM43x, supports different ethernet modes such >> as Ethernet, HSR and PRP by means of different firmwares and the >> driver loads one of the these firmware. The PRU-ICSS linux driver >> may not be available for all kernel versions. >> >> So add an .inc file that avoids the packaging of such firmwares >> in the SDK, so that it can be referenced by kernel recipes that >> don't support PRU-ICSS. >> >> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> >> --- >> meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc | 3 +++ >> 1 file changed, 3 insertions(+) >> create mode 100644 meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >> >> diff --git a/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >> new file mode 100644 >> index 00000000..6698c9ed >> --- /dev/null >> +++ b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >> @@ -0,0 +1,3 @@ >> +# Do not package firmwares for kernels that don't support PRUETH >> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti43x = " prueth-fw pruhsr-fw pruprp-fw" >> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti33x = " prueth-fw pruhsr-fw pruprp-fw" > > This is asking for trouble - there's no way to un-do :remove downstream (e.g. > any of the Distro, SDK, Product Line or Customer layers) > > It is not recommended to use :remove (and :append to a lesser degree) on a > regular basis when it can be done differently. > > Moreover, what is a difference from v2? It is a more convoluted way to drop > those FW images from getting installed, but it is still done unconditionally > here in meta-ti - are there any benefits of doing it this way? From my understanding... This firmware was supported under 5.10. It is not being supported for 6.1 but will be supported next year some time. So I was trying to communicate via the recipes that overall it is supported, but for this blip it is removed. Either way we are adding some odd conditionals to support the logic. Two choices: 1) Enabled by default, disable in the kernels were it is not supported. 2) Disabled by default, enable in 5.10 and in the future. Thoughts?
On 10/9/23 4:25 PM, Ryan Eatmon wrote: > > > On 10/9/2023 3:20 PM, Denys Dmytriyenko wrote: >> On Mon, Oct 09, 2023 at 12:39:25PM +0530, Ravi Gunasekaran via lists.yoctoproject.org wrote: >>> PRU-ICSS on AM33x, AM43x, supports different ethernet modes such >>> as Ethernet, HSR and PRP by means of different firmwares and the >>> driver loads one of the these firmware. The PRU-ICSS linux driver >>> may not be available for all kernel versions. >>> >>> So add an .inc file that avoids the packaging of such firmwares >>> in the SDK, so that it can be referenced by kernel recipes that >>> don't support PRU-ICSS. >>> >>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> >>> --- >>> meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc | 3 +++ >>> 1 file changed, 3 insertions(+) >>> create mode 100644 meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>> >>> diff --git a/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>> new file mode 100644 >>> index 00000000..6698c9ed >>> --- /dev/null >>> +++ b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>> @@ -0,0 +1,3 @@ >>> +# Do not package firmwares for kernels that don't support PRUETH >>> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti43x = " prueth-fw pruhsr-fw pruprp-fw" >>> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti33x = " prueth-fw pruhsr-fw pruprp-fw" >> >> This is asking for trouble - there's no way to un-do :remove downstream (e.g. >> any of the Distro, SDK, Product Line or Customer layers) >> >> It is not recommended to use :remove (and :append to a lesser degree) on a >> regular basis when it can be done differently. >> >> Moreover, what is a difference from v2? It is a more convoluted way to drop >> those FW images from getting installed, but it is still done unconditionally >> here in meta-ti - are there any benefits of doing it this way? > > From my understanding... This firmware was supported under 5.10. It is not being supported for 6.1 but will be supported next year some time. > > So I was trying to communicate via the recipes that overall it is supported, but for this blip it is removed. > > Either way we are adding some odd conditionals to support the logic. Two choices: > > 1) Enabled by default, disable in the kernels were it is not supported. > 2) Disabled by default, enable in 5.10 and in the future. > 3) Install firmware unconditionally linux-firmware has plenty of firmware shipped that different version of the kernel can/cannot use. Why is this one little firmware so important to *not* ship right now? Andrew > Thoughts? > >
On 10/10/23 3:03 AM, Andrew Davis wrote: > On 10/9/23 4:25 PM, Ryan Eatmon wrote: >> >> >> On 10/9/2023 3:20 PM, Denys Dmytriyenko wrote: >>> On Mon, Oct 09, 2023 at 12:39:25PM +0530, Ravi Gunasekaran via lists.yoctoproject.org wrote: >>>> PRU-ICSS on AM33x, AM43x, supports different ethernet modes such >>>> as Ethernet, HSR and PRP by means of different firmwares and the >>>> driver loads one of the these firmware. The PRU-ICSS linux driver >>>> may not be available for all kernel versions. >>>> >>>> So add an .inc file that avoids the packaging of such firmwares >>>> in the SDK, so that it can be referenced by kernel recipes that >>>> don't support PRU-ICSS. >>>> >>>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> >>>> --- >>>> meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> create mode 100644 meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>>> >>>> diff --git a/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>>> new file mode 100644 >>>> index 00000000..6698c9ed >>>> --- /dev/null >>>> +++ b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>>> @@ -0,0 +1,3 @@ >>>> +# Do not package firmwares for kernels that don't support PRUETH >>>> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti43x = " prueth-fw pruhsr-fw pruprp-fw" >>>> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti33x = " prueth-fw pruhsr-fw pruprp-fw" >>> >>> This is asking for trouble - there's no way to un-do :remove downstream (e.g. >>> any of the Distro, SDK, Product Line or Customer layers) >>> >>> It is not recommended to use :remove (and :append to a lesser degree) on a >>> regular basis when it can be done differently. >>> >>> Moreover, what is a difference from v2? It is a more convoluted way to drop >>> those FW images from getting installed, but it is still done unconditionally >>> here in meta-ti - are there any benefits of doing it this way? >> >> From my understanding... This firmware was supported under 5.10. It is not being supported for 6.1 but will be supported next year some time. >> >> So I was trying to communicate via the recipes that overall it is supported, but for this blip it is removed. >> >> Either way we are adding some odd conditionals to support the logic. Two choices: >> >> 1) Enabled by default, disable in the kernels were it is not supported. >> 2) Disabled by default, enable in 5.10 and in the future. >> > > 3) Install firmware unconditionally > > linux-firmware has plenty of firmware shipped that different version of the > kernel can/cannot use. Why is this one little firmware so important to *not* > ship right now? > The concern was raised by the Customer Apps team as the feature is not supported. And the presence of the firmwares may give an impression to the customer that feature is supported. > Andrew > >> Thoughts? >> >>
On 10/9/2023 3:20 PM, Denys Dmytriyenko wrote: > On Mon, Oct 09, 2023 at 12:39:25PM +0530, Ravi Gunasekaran via lists.yoctoproject.org wrote: >> PRU-ICSS on AM33x, AM43x, supports different ethernet modes such >> as Ethernet, HSR and PRP by means of different firmwares and the >> driver loads one of the these firmware. The PRU-ICSS linux driver >> may not be available for all kernel versions. >> >> So add an .inc file that avoids the packaging of such firmwares >> in the SDK, so that it can be referenced by kernel recipes that >> don't support PRU-ICSS. >> >> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> >> --- >> meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc | 3 +++ >> 1 file changed, 3 insertions(+) >> create mode 100644 meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >> >> diff --git a/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >> new file mode 100644 >> index 00000000..6698c9ed >> --- /dev/null >> +++ b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >> @@ -0,0 +1,3 @@ >> +# Do not package firmwares for kernels that don't support PRUETH >> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti43x = " prueth-fw pruhsr-fw pruprp-fw" >> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti33x = " prueth-fw pruhsr-fw pruprp-fw" > > This is asking for trouble - there's no way to un-do :remove downstream (e.g. > any of the Distro, SDK, Product Line or Customer layers) > > It is not recommended to use :remove (and :append to a lesser degree) on a > regular basis when it can be done differently. > > Moreover, what is a difference from v2? It is a more convoluted way to drop > those FW images from getting installed, but it is still done unconditionally > here in meta-ti - are there any benefits of doing it this way? Ravi, I hate to ask this of you as most of the waffling is from me. But I agree with Denys. I think what we need is the v2 patch, but with better wording (like what you have in v3) that explains the situation. Can you put together a new v4 that looks like that?
On 10/21/23 3:50 AM, Ryan Eatmon wrote: > > > On 10/9/2023 3:20 PM, Denys Dmytriyenko wrote: >> On Mon, Oct 09, 2023 at 12:39:25PM +0530, Ravi Gunasekaran via lists.yoctoproject.org wrote: >>> PRU-ICSS on AM33x, AM43x, supports different ethernet modes such >>> as Ethernet, HSR and PRP by means of different firmwares and the >>> driver loads one of the these firmware. The PRU-ICSS linux driver >>> may not be available for all kernel versions. >>> >>> So add an .inc file that avoids the packaging of such firmwares >>> in the SDK, so that it can be referenced by kernel recipes that >>> don't support PRU-ICSS. >>> >>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> >>> --- >>> meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc | 3 +++ >>> 1 file changed, 3 insertions(+) >>> create mode 100644 meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>> >>> diff --git a/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>> new file mode 100644 >>> index 00000000..6698c9ed >>> --- /dev/null >>> +++ b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>> @@ -0,0 +1,3 @@ >>> +# Do not package firmwares for kernels that don't support PRUETH >>> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti43x = " prueth-fw pruhsr-fw pruprp-fw" >>> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti33x = " prueth-fw pruhsr-fw pruprp-fw" >> >> This is asking for trouble - there's no way to un-do :remove downstream (e.g. >> any of the Distro, SDK, Product Line or Customer layers) >> >> It is not recommended to use :remove (and :append to a lesser degree) on a >> regular basis when it can be done differently. >> >> Moreover, what is a difference from v2? It is a more convoluted way to drop >> those FW images from getting installed, but it is still done unconditionally >> here in meta-ti - are there any benefits of doing it this way? > > > Ravi, I hate to ask this of you as most of the waffling is from me. > > But I agree with Denys. I think what we need is the v2 patch, but with better wording (like what you have in v3) that explains the situation. > > Can you put together a new v4 that looks like that? > > Thanks for revisiting this patch. Based on Andrew's comment on installing the firmware unconditionally, I informed the concerned parties that approach to selectively package firmwares is little messy. It's been more than a week and haven't heard back anything. So I would like to drop this patch, unless there is a strong request for it. > >
On 10/22/2023 11:49 PM, Ravi Gunasekaran wrote: > > > On 10/21/23 3:50 AM, Ryan Eatmon wrote: >> >> >> On 10/9/2023 3:20 PM, Denys Dmytriyenko wrote: >>> On Mon, Oct 09, 2023 at 12:39:25PM +0530, Ravi Gunasekaran via lists.yoctoproject.org wrote: >>>> PRU-ICSS on AM33x, AM43x, supports different ethernet modes such >>>> as Ethernet, HSR and PRP by means of different firmwares and the >>>> driver loads one of the these firmware. The PRU-ICSS linux driver >>>> may not be available for all kernel versions. >>>> >>>> So add an .inc file that avoids the packaging of such firmwares >>>> in the SDK, so that it can be referenced by kernel recipes that >>>> don't support PRU-ICSS. >>>> >>>> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> >>>> --- >>>> meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> create mode 100644 meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>>> >>>> diff --git a/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>>> new file mode 100644 >>>> index 00000000..6698c9ed >>>> --- /dev/null >>>> +++ b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc >>>> @@ -0,0 +1,3 @@ >>>> +# Do not package firmwares for kernels that don't support PRUETH >>>> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti43x = " prueth-fw pruhsr-fw pruprp-fw" >>>> +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti33x = " prueth-fw pruhsr-fw pruprp-fw" >>> >>> This is asking for trouble - there's no way to un-do :remove downstream (e.g. >>> any of the Distro, SDK, Product Line or Customer layers) >>> >>> It is not recommended to use :remove (and :append to a lesser degree) on a >>> regular basis when it can be done differently. >>> >>> Moreover, what is a difference from v2? It is a more convoluted way to drop >>> those FW images from getting installed, but it is still done unconditionally >>> here in meta-ti - are there any benefits of doing it this way? >> >> >> Ravi, I hate to ask this of you as most of the waffling is from me. >> >> But I agree with Denys. I think what we need is the v2 patch, but with better wording (like what you have in v3) that explains the situation. >> >> Can you put together a new v4 that looks like that? >> >> > > Thanks for revisiting this patch. > > Based on Andrew's comment on installing the firmware unconditionally, I informed the > concerned parties that approach to selectively package firmwares is little messy. > It's been more than a week and haven't heard back anything. > > So I would like to drop this patch, unless there is a strong request for it. Sounds good. We can revisit later if needed.
diff --git a/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc new file mode 100644 index 00000000..6698c9ed --- /dev/null +++ b/meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc @@ -0,0 +1,3 @@ +# Do not package firmwares for kernels that don't support PRUETH +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti43x = " prueth-fw pruhsr-fw pruprp-fw" +RDEPENDS:${KERNEL_PACKAGE_NAME}-base:remove:ti33x = " prueth-fw pruhsr-fw pruprp-fw"
PRU-ICSS on AM33x, AM43x, supports different ethernet modes such as Ethernet, HSR and PRP by means of different firmwares and the driver loads one of the these firmware. The PRU-ICSS linux driver may not be available for all kernel versions. So add an .inc file that avoids the packaging of such firmwares in the SDK, so that it can be referenced by kernel recipes that don't support PRU-ICSS. Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> --- meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 meta-ti-bsp/recipes-kernel/linux/prueth-fw.inc