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

[Siddharth Vadapalli]

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.

> 

-- 
Regards,
Siddharth.