On 6/3/25 16:06, Tom Rini wrote:
> On Tue, Jun 03, 2025 at 03:49:05PM +0200, Jerome Forissier wrote:
>> Hi Heinrich,
>>
>> On 6/2/25 23:23, Heinrich Schuchardt wrote:
>>> Am 21. Mai 2025 17:14:42 MESZ schrieb Jerome Forissier
>>> <jerome.foriss...@linaro.org>:
>>>> Implement the sntp command when NET_LWIP=y.
>>>>
>>>> Signed-off-by: Jerome Forissier <jerome.foriss...@linaro.org>
>>>> ---
>>>>
>>>> cmd/Kconfig | 13 ++--
>>>> cmd/net-lwip.c | 5 ++
>>>> include/net-common.h | 11 ++++
>>>> lib/lwip/Makefile | 1 +
>>>> lib/lwip/u-boot/arch/cc.h | 4 ++
>>>> lib/lwip/u-boot/lwipopts.h | 4 ++
>>>> net/lwip/Makefile | 1 +
>>>> net/lwip/sntp.c | 128 +++++++++++++++++++++++++++++++++++++
>>>> 8 files changed, 161 insertions(+), 6 deletions(-)
>>>> create mode 100644 net/lwip/sntp.c
>>>>
>>>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>>>> index f21d27cb27f..58f629e1c34 100644
>>>> --- a/cmd/Kconfig
>>>> +++ b/cmd/Kconfig
>>>> @@ -2061,12 +2061,6 @@ config CMD_CDP
>>>> and to retrieve configuration data including the VLAN id using the
>>>> proprietary Cisco Discovery Protocol (CDP).
>>>>
>>>> -config CMD_SNTP
>>>> - bool "sntp"
>>>> - select PROT_UDP
>>>> - help
>>>> - Synchronize RTC via network
>>>> -
>>>> config CMD_LINK_LOCAL
>>>> bool "linklocal"
>>>> depends on (LIB_RAND || LIB_HW_RAND)
>>>> @@ -2144,6 +2138,13 @@ config CMD_PING
>>>> help
>>>> Send ICMP ECHO_REQUEST to network host
>>>>
>>>> +config CMD_SNTP
>>>
>>> The command should depend on RTC support.
>>>
>>>> + bool "sntp"
>>>> + select PROT_UDP if NET
>>>> + select PROT_UDP_LWIP if NET_LWIP
>>>> + help
>>>> + Synchronize RTC via network
>>>> +
>>>> config CMD_TFTPBOOT
>>>> bool "tftp"
>>>> select PROT_UDP_LWIP if NET_LWIP
>>>> diff --git a/cmd/net-lwip.c b/cmd/net-lwip.c
>>>> index cecf8d02555..2ffee64f97e 100644
>>>> --- a/cmd/net-lwip.c
>>>> +++ b/cmd/net-lwip.c
>>>
>>> Please, put every command into a separate file instead of duplicating the
>>> #ifdef chaos of the old network stack.
>>
>> While I agree that cmd/net.c is painful re. #ifdef's, I believe
>> cmd/net-lwip.c is not bad at all. In fact I took care of having only the
>> command wrappers in there (U_BOOT_CMD(...)), so it's just 55 lines at
>> the moment. The implementations are in
>> net/lwip/{dhcp,dns,ping,sntp,tftp,wget}.c. So I don't think there is
>> a need to change that. Or I could move the U_BOOT_CMD(...) into
>> net/lwip/*.c as well, leaving only the non-net commands under cmd/ (apart
>> from the legacy NET stuff).
>>
>> Any opinions on that?
>
> While there are commands outside of cmd/ they are the exception (and
> generally ought to be moved) so if we go the route of breaking up
> cmd/net-lwip.c I'd rather see cmd/lwip/${cmd}.c instead.
To be clear, you mean moving net/lwip/${cmd}.c into cmd/lwip/ in addition to
splitting cmd/net-lwip.c?
Thanks,
--
Jerome
