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

[Nishanth Menon]

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.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D