mbox series

[00/10] Add support for Ethernet Boot on SK-AM62

Message ID 20240112064759.1801600-1-s-vadapalli@ti.com
Headers show
Series Add support for Ethernet Boot on SK-AM62 | expand

Message

Siddharth Vadapalli Jan. 12, 2024, 6:47 a.m. UTC
Hello,

This series enables Ethernet Boot on SK-AM62 device.
Product Link: https://www.ti.com/tool/SK-AM62
User Guide: https://www.ti.com/lit/pdf/spruj40

Ethernet Boot flow is as follows:
1. The BOOT MODE pins are configured for Ethernet Boot.
2. On powering on the device, ROM uses the Ethernet Port corresponding
to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot".
3. The TFTP server and DHCP server on the receiver device need to be
configured such that VCI string "TI K3 Bootp Boot" maps to the file
"tiboot3.bin" and the TFTP server should be capable of transferring
it to the device.
4. ROM loads and executes "tiboot3.bin" provided by the TFTP server.
5. The "tiboot3.bin" file is expected to be built using the config:
am62x_evm_r5_ethboot_defconfig
introduced in this series, which shall enable "tispl.bin" to be fetched
over TFTP using "tiboot3.bin".
6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its
NET_VCI_STRING, thereby implying that the DHCP server and TFTP server
need to be configured to transfer "tispl.bin" to the device.
7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP
server.
8. The "tispl.bin" file is expected to be built using the config:
am62x_evm_a53_defconfig
which has been updated in this series to enable Ethernet Boot specific
configs, allowing "u-boot.img" to be fetched over TFTP using
"tispl.bin".
9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its
NET_VCI_STRING. The DHCP server and TFTP server need to be configured to
transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING.
10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and
executes it on the device, completing the process of Ethernet Boot on the
device.

NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode
of operation due to which it is expected that the Link Partner also
supports the same mode of operation.
Additionally, enabling "phy_gmii_sel" node at SPL stage will be
necessary and is not added as a part of this series with the aim of
adding the "bootph-all" property to its counterpart in Linux device-tree
first.

This series is based on commit:
f28a77589e Merge tag 'dm-next-7jan23' of https://source.denx.de/u-boot/custodians/u-boot-dm into next
of the next branch of u-boot.

Regards,
Siddharth.

Andreas Dannenberg (1):
  arm: mach-k3: am62: Update SoC autogenerated data to enable Ethernet
    Boot

Kishon Vijay Abraham I (7):
  board: ti: am62x: Init DRAM size in R5/A53 SPL
  firmware: ti_sci: Add No-OP for "RX_FL_CFG"
  soc: ti: k3-navss-ringacc: Initialize base address of ring cfg
    registers
  dma: ti: k3-udma: Add support for native configuration of chan/flow
  arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
  configs: am62: Add configs for enabling ETHBOOT in R5SPL
  configs: am62x_evm_a53_defconfig: Enable configs required for Ethboot

Siddharth Vadapalli (1):
  arm: dts: k3-am625-r5-sk: Enable DM services for main_pktdma

Vignesh Raghavendra (1):
  soc: ti: k3-navss-ringacc: Fix reset ring API

 arch/arm/dts/k3-am625-r5-sk.dts          |   5 ++
 arch/arm/mach-k3/am625_init.c            |  10 +++
 arch/arm/mach-k3/r5/am62x/clk-data.c     |  79 ++++++++--------
 board/ti/am62x/evm.c                     |   3 +
 configs/am62x_evm_a53_defconfig          |   8 ++
 configs/am62x_evm_r5_ethboot_defconfig   | 110 +++++++++++++++++++++++
 drivers/dma/ti/k3-udma.c                 |   6 ++
 drivers/firmware/ti_sci.c                |   8 +-
 drivers/soc/ti/k3-navss-ringacc-u-boot.c |   9 +-
 drivers/soc/ti/k3-navss-ringacc.c        |   7 +-
 10 files changed, 202 insertions(+), 43 deletions(-)
 create mode 100644 configs/am62x_evm_r5_ethboot_defconfig

Comments

Nishanth Menon Jan. 12, 2024, 12:32 p.m. UTC | #1
On 12:17-20240112, Siddharth Vadapalli wrote:
> Hello,
> 
> This series enables Ethernet Boot on SK-AM62 device.
> Product Link: https://www.ti.com/tool/SK-AM62
> User Guide: https://www.ti.com/lit/pdf/spruj40
> 
> Ethernet Boot flow is as follows:
> 1. The BOOT MODE pins are configured for Ethernet Boot.
> 2. On powering on the device, ROM uses the Ethernet Port corresponding
> to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot".
> 3. The TFTP server and DHCP server on the receiver device need to be
> configured such that VCI string "TI K3 Bootp Boot" maps to the file
> "tiboot3.bin" and the TFTP server should be capable of transferring
> it to the device.
> 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server.
> 5. The "tiboot3.bin" file is expected to be built using the config:
> am62x_evm_r5_ethboot_defconfig
> introduced in this series, which shall enable "tispl.bin" to be fetched
> over TFTP using "tiboot3.bin".
> 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its
> NET_VCI_STRING, thereby implying that the DHCP server and TFTP server
> need to be configured to transfer "tispl.bin" to the device.
> 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP
> server.
> 8. The "tispl.bin" file is expected to be built using the config:
> am62x_evm_a53_defconfig
> which has been updated in this series to enable Ethernet Boot specific
> configs, allowing "u-boot.img" to be fetched over TFTP using
> "tispl.bin".
> 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its
> NET_VCI_STRING. The DHCP server and TFTP server need to be configured to
> transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING.
> 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and
> executes it on the device, completing the process of Ethernet Boot on the
> device.
> 
> NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode
> of operation due to which it is expected that the Link Partner also
> supports the same mode of operation.
> Additionally, enabling "phy_gmii_sel" node at SPL stage will be
> necessary and is not added as a part of this series with the aim of
> adding the "bootph-all" property to its counterpart in Linux device-tree
> first.


NAK - instead of writing all this up in the commit message, why would
you not spend that time updating the excellent documentation we have?

> 
> This series is based on commit:
> f28a77589e Merge tag 'dm-next-7jan23' of https://source.denx.de/u-boot/custodians/u-boot-dm into next
> of the next branch of u-boot.
> 
> Regards,
> Siddharth.
> 
> Andreas Dannenberg (1):
>   arm: mach-k3: am62: Update SoC autogenerated data to enable Ethernet
>     Boot
> 
> Kishon Vijay Abraham I (7):
>   board: ti: am62x: Init DRAM size in R5/A53 SPL
>   firmware: ti_sci: Add No-OP for "RX_FL_CFG"
>   soc: ti: k3-navss-ringacc: Initialize base address of ring cfg
>     registers
>   dma: ti: k3-udma: Add support for native configuration of chan/flow
>   arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
>   configs: am62: Add configs for enabling ETHBOOT in R5SPL
>   configs: am62x_evm_a53_defconfig: Enable configs required for Ethboot
> 
> Siddharth Vadapalli (1):
>   arm: dts: k3-am625-r5-sk: Enable DM services for main_pktdma
> 
> Vignesh Raghavendra (1):
>   soc: ti: k3-navss-ringacc: Fix reset ring API
> 
>  arch/arm/dts/k3-am625-r5-sk.dts          |   5 ++
>  arch/arm/mach-k3/am625_init.c            |  10 +++
>  arch/arm/mach-k3/r5/am62x/clk-data.c     |  79 ++++++++--------
>  board/ti/am62x/evm.c                     |   3 +
>  configs/am62x_evm_a53_defconfig          |   8 ++
>  configs/am62x_evm_r5_ethboot_defconfig   | 110 +++++++++++++++++++++++
>  drivers/dma/ti/k3-udma.c                 |   6 ++
>  drivers/firmware/ti_sci.c                |   8 +-
>  drivers/soc/ti/k3-navss-ringacc-u-boot.c |   9 +-
>  drivers/soc/ti/k3-navss-ringacc.c        |   7 +-
>  10 files changed, 202 insertions(+), 43 deletions(-)
>  create mode 100644 configs/am62x_evm_r5_ethboot_defconfig
> 
> -- 
> 2.34.1
>
Siddharth Vadapalli Jan. 12, 2024, 12:36 p.m. UTC | #2
On 12/01/24 18:02, Nishanth Menon wrote:
> On 12:17-20240112, Siddharth Vadapalli wrote:
>> Hello,
>>
>> This series enables Ethernet Boot on SK-AM62 device.
>> Product Link: https://www.ti.com/tool/SK-AM62
>> User Guide: https://www.ti.com/lit/pdf/spruj40
>>
>> Ethernet Boot flow is as follows:
>> 1. The BOOT MODE pins are configured for Ethernet Boot.
>> 2. On powering on the device, ROM uses the Ethernet Port corresponding
>> to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot".
>> 3. The TFTP server and DHCP server on the receiver device need to be
>> configured such that VCI string "TI K3 Bootp Boot" maps to the file
>> "tiboot3.bin" and the TFTP server should be capable of transferring
>> it to the device.
>> 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server.
>> 5. The "tiboot3.bin" file is expected to be built using the config:
>> am62x_evm_r5_ethboot_defconfig
>> introduced in this series, which shall enable "tispl.bin" to be fetched
>> over TFTP using "tiboot3.bin".
>> 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its
>> NET_VCI_STRING, thereby implying that the DHCP server and TFTP server
>> need to be configured to transfer "tispl.bin" to the device.
>> 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP
>> server.
>> 8. The "tispl.bin" file is expected to be built using the config:
>> am62x_evm_a53_defconfig
>> which has been updated in this series to enable Ethernet Boot specific
>> configs, allowing "u-boot.img" to be fetched over TFTP using
>> "tispl.bin".
>> 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its
>> NET_VCI_STRING. The DHCP server and TFTP server need to be configured to
>> transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING.
>> 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and
>> executes it on the device, completing the process of Ethernet Boot on the
>> device.
>>
>> NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode
>> of operation due to which it is expected that the Link Partner also
>> supports the same mode of operation.
>> Additionally, enabling "phy_gmii_sel" node at SPL stage will be
>> necessary and is not added as a part of this series with the aim of
>> adding the "bootph-all" property to its counterpart in Linux device-tree
>> first.
> 
> 
> NAK - instead of writing all this up in the commit message, why would
> you not spend that time updating the excellent documentation we have?

I plan to document it after the feature is in. The reason for mentioning the
content above is for explaining the flow in case anyone wishes to test and
verify it. Wouldn't documenting it make it appear that the feature is present
when it isn't?

...
Nishanth Menon Jan. 12, 2024, 12:42 p.m. UTC | #3
On 18:06-20240112, Siddharth Vadapalli wrote:
> 
> 
> On 12/01/24 18:02, Nishanth Menon wrote:
> > On 12:17-20240112, Siddharth Vadapalli wrote:
> >> Hello,
> >>
> >> This series enables Ethernet Boot on SK-AM62 device.
> >> Product Link: https://www.ti.com/tool/SK-AM62
> >> User Guide: https://www.ti.com/lit/pdf/spruj40
> >>
> >> Ethernet Boot flow is as follows:
> >> 1. The BOOT MODE pins are configured for Ethernet Boot.
> >> 2. On powering on the device, ROM uses the Ethernet Port corresponding
> >> to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot".
> >> 3. The TFTP server and DHCP server on the receiver device need to be
> >> configured such that VCI string "TI K3 Bootp Boot" maps to the file
> >> "tiboot3.bin" and the TFTP server should be capable of transferring
> >> it to the device.
> >> 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server.
> >> 5. The "tiboot3.bin" file is expected to be built using the config:
> >> am62x_evm_r5_ethboot_defconfig
> >> introduced in this series, which shall enable "tispl.bin" to be fetched
> >> over TFTP using "tiboot3.bin".
> >> 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its
> >> NET_VCI_STRING, thereby implying that the DHCP server and TFTP server
> >> need to be configured to transfer "tispl.bin" to the device.
> >> 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP
> >> server.
> >> 8. The "tispl.bin" file is expected to be built using the config:
> >> am62x_evm_a53_defconfig
> >> which has been updated in this series to enable Ethernet Boot specific
> >> configs, allowing "u-boot.img" to be fetched over TFTP using
> >> "tispl.bin".
> >> 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its
> >> NET_VCI_STRING. The DHCP server and TFTP server need to be configured to
> >> transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING.
> >> 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and
> >> executes it on the device, completing the process of Ethernet Boot on the
> >> device.
> >>
> >> NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode
> >> of operation due to which it is expected that the Link Partner also
> >> supports the same mode of operation.
> >> Additionally, enabling "phy_gmii_sel" node at SPL stage will be
> >> necessary and is not added as a part of this series with the aim of
> >> adding the "bootph-all" property to its counterpart in Linux device-tree
> >> first.
> > 
> > 
> > NAK - instead of writing all this up in the commit message, why would
> > you not spend that time updating the excellent documentation we have?
> 
> I plan to document it after the feature is in. The reason for mentioning the
> content above is for explaining the flow in case anyone wishes to test and
> verify it. Wouldn't documenting it make it appear that the feature is present
> when it isn't?

So you are saying this series does NOT work! why are you sending patches
then? If you are introducing a feature and enabling it, ensure you send
documentation along with it allowing people to be able to actually use
the feature.
Siddharth Vadapalli Jan. 12, 2024, 12:47 p.m. UTC | #4
On 12/01/24 18:12, Nishanth Menon wrote:
> On 18:06-20240112, Siddharth Vadapalli wrote:
>>
>>
>> On 12/01/24 18:02, Nishanth Menon wrote:
>>> On 12:17-20240112, Siddharth Vadapalli wrote:
>>>> Hello,
>>>>
>>>> This series enables Ethernet Boot on SK-AM62 device.
>>>> Product Link: https://www.ti.com/tool/SK-AM62
>>>> User Guide: https://www.ti.com/lit/pdf/spruj40
>>>>
>>>> Ethernet Boot flow is as follows:
>>>> 1. The BOOT MODE pins are configured for Ethernet Boot.
>>>> 2. On powering on the device, ROM uses the Ethernet Port corresponding
>>>> to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot".
>>>> 3. The TFTP server and DHCP server on the receiver device need to be
>>>> configured such that VCI string "TI K3 Bootp Boot" maps to the file
>>>> "tiboot3.bin" and the TFTP server should be capable of transferring
>>>> it to the device.
>>>> 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server.
>>>> 5. The "tiboot3.bin" file is expected to be built using the config:
>>>> am62x_evm_r5_ethboot_defconfig
>>>> introduced in this series, which shall enable "tispl.bin" to be fetched
>>>> over TFTP using "tiboot3.bin".
>>>> 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its
>>>> NET_VCI_STRING, thereby implying that the DHCP server and TFTP server
>>>> need to be configured to transfer "tispl.bin" to the device.
>>>> 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP
>>>> server.
>>>> 8. The "tispl.bin" file is expected to be built using the config:
>>>> am62x_evm_a53_defconfig
>>>> which has been updated in this series to enable Ethernet Boot specific
>>>> configs, allowing "u-boot.img" to be fetched over TFTP using
>>>> "tispl.bin".
>>>> 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its
>>>> NET_VCI_STRING. The DHCP server and TFTP server need to be configured to
>>>> transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING.
>>>> 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and
>>>> executes it on the device, completing the process of Ethernet Boot on the
>>>> device.
>>>>
>>>> NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode
>>>> of operation due to which it is expected that the Link Partner also
>>>> supports the same mode of operation.
>>>> Additionally, enabling "phy_gmii_sel" node at SPL stage will be
>>>> necessary and is not added as a part of this series with the aim of
>>>> adding the "bootph-all" property to its counterpart in Linux device-tree
>>>> first.
>>>
>>>
>>> NAK - instead of writing all this up in the commit message, why would
>>> you not spend that time updating the excellent documentation we have?
>>
>> I plan to document it after the feature is in. The reason for mentioning the
>> content above is for explaining the flow in case anyone wishes to test and
>> verify it. Wouldn't documenting it make it appear that the feature is present
>> when it isn't?
> 
> So you are saying this series does NOT work! why are you sending patches
> then? If you are introducing a feature and enabling it, ensure you send
> documentation along with it allowing people to be able to actually use
> the feature.

I have mentioned in the "NOTE" above that enabling "phy_gmii_sel" node at SPL
stage by adding the "bootph-all" property is necessary to verify this series.
I cannot post that with this series since Linux device-tree needs to have the
property added first and the merge window is closed now. Once it is in the Linux
device-tree, syncing U-Boot device-tree with Linux device-tree will enable
Ethernet Boot which is when the feature will work. That is when I was planning
to document it. However, based on your feedback, in the next version for this
series I will add the documentation as well along with the note that
"phy_gmii_sel" needs to be enabled at SPL stage for the feature to work.

Please let me know if that is acceptable.

>
Nishanth Menon Jan. 12, 2024, 1:01 p.m. UTC | #5
On 18:17-20240112, Siddharth Vadapalli wrote:
> 
> 
> On 12/01/24 18:12, Nishanth Menon wrote:
> > On 18:06-20240112, Siddharth Vadapalli wrote:
> >>
> >>
> >> On 12/01/24 18:02, Nishanth Menon wrote:
> >>> On 12:17-20240112, Siddharth Vadapalli wrote:
> >>>> Hello,
> >>>>
> >>>> This series enables Ethernet Boot on SK-AM62 device.
> >>>> Product Link: https://www.ti.com/tool/SK-AM62
> >>>> User Guide: https://www.ti.com/lit/pdf/spruj40
> >>>>
> >>>> Ethernet Boot flow is as follows:
> >>>> 1. The BOOT MODE pins are configured for Ethernet Boot.
> >>>> 2. On powering on the device, ROM uses the Ethernet Port corresponding
> >>>> to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot".
> >>>> 3. The TFTP server and DHCP server on the receiver device need to be
> >>>> configured such that VCI string "TI K3 Bootp Boot" maps to the file
> >>>> "tiboot3.bin" and the TFTP server should be capable of transferring
> >>>> it to the device.
> >>>> 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server.
> >>>> 5. The "tiboot3.bin" file is expected to be built using the config:
> >>>> am62x_evm_r5_ethboot_defconfig
> >>>> introduced in this series, which shall enable "tispl.bin" to be fetched
> >>>> over TFTP using "tiboot3.bin".
> >>>> 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its
> >>>> NET_VCI_STRING, thereby implying that the DHCP server and TFTP server
> >>>> need to be configured to transfer "tispl.bin" to the device.
> >>>> 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP
> >>>> server.
> >>>> 8. The "tispl.bin" file is expected to be built using the config:
> >>>> am62x_evm_a53_defconfig
> >>>> which has been updated in this series to enable Ethernet Boot specific
> >>>> configs, allowing "u-boot.img" to be fetched over TFTP using
> >>>> "tispl.bin".
> >>>> 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its
> >>>> NET_VCI_STRING. The DHCP server and TFTP server need to be configured to
> >>>> transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING.
> >>>> 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and
> >>>> executes it on the device, completing the process of Ethernet Boot on the
> >>>> device.
> >>>>
> >>>> NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode
> >>>> of operation due to which it is expected that the Link Partner also
> >>>> supports the same mode of operation.
> >>>> Additionally, enabling "phy_gmii_sel" node at SPL stage will be
> >>>> necessary and is not added as a part of this series with the aim of
> >>>> adding the "bootph-all" property to its counterpart in Linux device-tree
> >>>> first.
> >>>
> >>>
> >>> NAK - instead of writing all this up in the commit message, why would
> >>> you not spend that time updating the excellent documentation we have?
> >>
> >> I plan to document it after the feature is in. The reason for mentioning the
> >> content above is for explaining the flow in case anyone wishes to test and
> >> verify it. Wouldn't documenting it make it appear that the feature is present
> >> when it isn't?
> > 
> > So you are saying this series does NOT work! why are you sending patches
> > then? If you are introducing a feature and enabling it, ensure you send
> > documentation along with it allowing people to be able to actually use
> > the feature.
> 
> I have mentioned in the "NOTE" above that enabling "phy_gmii_sel" node at SPL
> stage by adding the "bootph-all" property is necessary to verify this series.
> I cannot post that with this series since Linux device-tree needs to have the
> property added first and the merge window is closed now. Once it is in the Linux
> device-tree, syncing U-Boot device-tree with Linux device-tree will enable
> Ethernet Boot which is when the feature will work. That is when I was planning
> to document it. However, based on your feedback, in the next version for this
> series I will add the documentation as well along with the note that
> "phy_gmii_sel" needs to be enabled at SPL stage for the feature to work.
> 

considered first posting a patch to kernel.org (merge window has
nothing to do with posting and having patches reviewed) and in the
interim, doing that in u-boot.dtsi so that the next sync will drop it
from u-boot.dtsi?

OR hold the series back till it is merged into kernel.org master?

Either way, please do not send patches to the list that does not work.

Since it is now hard to trust your patches without detailed cover letter
analysis, next time you are updating the series post test logs as well
with just the patches applied and no additional changes.
Siddharth Vadapalli Jan. 15, 2024, 8:16 a.m. UTC | #6
On 12/01/24 18:31, Nishanth Menon wrote:
> On 18:17-20240112, Siddharth Vadapalli wrote:
>>
>>
>> On 12/01/24 18:12, Nishanth Menon wrote:
>>> On 18:06-20240112, Siddharth Vadapalli wrote:
>>>>
>>>>
>>>> On 12/01/24 18:02, Nishanth Menon wrote:
>>>>> On 12:17-20240112, Siddharth Vadapalli wrote:
>>>>>> Hello,
>>>>>>
>>>>>> This series enables Ethernet Boot on SK-AM62 device.
>>>>>> Product Link: https://www.ti.com/tool/SK-AM62
>>>>>> User Guide: https://www.ti.com/lit/pdf/spruj40
>>>>>>
>>>>>> Ethernet Boot flow is as follows:
>>>>>> 1. The BOOT MODE pins are configured for Ethernet Boot.
>>>>>> 2. On powering on the device, ROM uses the Ethernet Port corresponding
>>>>>> to CPSW3G's MAC Port 1 to transmit "TI K3 Bootp Boot".
>>>>>> 3. The TFTP server and DHCP server on the receiver device need to be
>>>>>> configured such that VCI string "TI K3 Bootp Boot" maps to the file
>>>>>> "tiboot3.bin" and the TFTP server should be capable of transferring
>>>>>> it to the device.
>>>>>> 4. ROM loads and executes "tiboot3.bin" provided by the TFTP server.
>>>>>> 5. The "tiboot3.bin" file is expected to be built using the config:
>>>>>> am62x_evm_r5_ethboot_defconfig
>>>>>> introduced in this series, which shall enable "tispl.bin" to be fetched
>>>>>> over TFTP using "tiboot3.bin".
>>>>>> 6. "tiboot3.bin" is configured to transmit "AM62X U-Boot R5 SPL" as its
>>>>>> NET_VCI_STRING, thereby implying that the DHCP server and TFTP server
>>>>>> need to be configured to transfer "tispl.bin" to the device.
>>>>>> 7. "tiboot3.bin" loads and executes "tispl.bin" provided by the TFTP
>>>>>> server.
>>>>>> 8. The "tispl.bin" file is expected to be built using the config:
>>>>>> am62x_evm_a53_defconfig
>>>>>> which has been updated in this series to enable Ethernet Boot specific
>>>>>> configs, allowing "u-boot.img" to be fetched over TFTP using
>>>>>> "tispl.bin".
>>>>>> 9. "tispl.bin" is configured to transmit "AM62X U-Boot A53 SPL" as its
>>>>>> NET_VCI_STRING. The DHCP server and TFTP server need to be configured to
>>>>>> transfer "u-boot.img" to the device for the aforementioned NET_VCI_STRING.
>>>>>> 10. "tispl.bin" then fetches "u-boot.img" using TFTP and loads and
>>>>>> executes it on the device, completing the process of Ethernet Boot on the
>>>>>> device.
>>>>>>
>>>>>> NOTE: ROM configures CPSW3G's MAC Port 1 for 100 Mbps full-duplex mode
>>>>>> of operation due to which it is expected that the Link Partner also
>>>>>> supports the same mode of operation.
>>>>>> Additionally, enabling "phy_gmii_sel" node at SPL stage will be
>>>>>> necessary and is not added as a part of this series with the aim of
>>>>>> adding the "bootph-all" property to its counterpart in Linux device-tree
>>>>>> first.
>>>>>
>>>>>
>>>>> NAK - instead of writing all this up in the commit message, why would
>>>>> you not spend that time updating the excellent documentation we have?
>>>>
>>>> I plan to document it after the feature is in. The reason for mentioning the
>>>> content above is for explaining the flow in case anyone wishes to test and
>>>> verify it. Wouldn't documenting it make it appear that the feature is present
>>>> when it isn't?
>>>
>>> So you are saying this series does NOT work! why are you sending patches
>>> then? If you are introducing a feature and enabling it, ensure you send
>>> documentation along with it allowing people to be able to actually use
>>> the feature.
>>
>> I have mentioned in the "NOTE" above that enabling "phy_gmii_sel" node at SPL
>> stage by adding the "bootph-all" property is necessary to verify this series.
>> I cannot post that with this series since Linux device-tree needs to have the
>> property added first and the merge window is closed now. Once it is in the Linux
>> device-tree, syncing U-Boot device-tree with Linux device-tree will enable
>> Ethernet Boot which is when the feature will work. That is when I was planning
>> to document it. However, based on your feedback, in the next version for this
>> series I will add the documentation as well along with the note that
>> "phy_gmii_sel" needs to be enabled at SPL stage for the feature to work.
>>
> 
> considered first posting a patch to kernel.org (merge window has
> nothing to do with posting and having patches reviewed) and in the
> interim, doing that in u-boot.dtsi so that the next sync will drop it
> from u-boot.dtsi?
> 
> OR hold the series back till it is merged into kernel.org master?
> 
> Either way, please do not send patches to the list that does not work.
> 
> Since it is now hard to trust your patches without detailed cover letter
> analysis, next time you are updating the series post test logs as well
> with just the patches applied and no additional changes.

Thank you for clarifying regarding the approach to be taken. I shall include the
logs when I post the v2 series, in addition to the Documentation update.

>