[PATCHv5 11/13] net/lwip: connection between cmd and lwip apps

[Maxim Uvarov]

On Wed, 9 Aug 2023 at 00:13, Ilias Apalodimas <ilias.apalodimas at linaro.org>
wrote:

> Hi Maxim
>
> [...]
>
> > > > +static int lwip_empty_tmo(void) { return 0; };
> > > > +int (*ulwip_tmo)(void) = lwip_empty_tmo;
> > > > +void ulwip_set_tmo(int (*tmo)(void))
> > > > +{
> > > > +     ulwip_tmo = tmo;
> > > > +}
> > > > +
> > > > +static void ulwip_clear_tmo(void)
> > > > +{
> > > > +     ulwip_tmo = lwip_empty_tmo;
> > > > +}
> > > > +
> > > > +static void ulwip_timeout_handler(void)
> > > > +{
> > > > +     eth_halt();
> > > > +     ulwip_tmo();
> > > > +     net_set_state(NETLOOP_FAIL);    /* we did not get the reply */
> > >
> > > I am not sure what I am reading here.  You use callbacks a few lines
> above
> > > to set a timeout function.  But only set it for dhcp.  On top of that
> the
> > > function for DHCP has a case for a *successful* asignment of ip
> addresses.
> > > Why are we setting the state to fail? And why are we complicating this
> by
> > > assigning and removing callbacks if it's only used for dhcp?
> > >
> > >
> > I need two time out callbacks here:
> > 1. Trap rx polling loop if lwip application works too long. It is used
> when
> > code goes to net_loop() code to poll rx packets
> > and nobody interrupts this loop. This timeout is used for all cmds (lwip
> > apps).
> >
> > 2. Trap lwip application after specific timeout and then check some
> state.
> > That is case for DHCP, where LWIP DHCP does not have
> > callback for changing state. And I need to know when to stop polling
> loop.
>
> Yes but is there a reason to reassing those callback to a function ptr?
> Just define them and use them
>
>
I added a more detailed description to this time out function for the next
version. The reason
here is the following -  there is no scheduler. cmd just starts lwip
application. But in fact it does
only initialization and in most cases you need to go to the polling loop.
Then after polling
loop you need to return to 1. back to U-boot console. or 2. back to lwip
application to print some message,
or get some lwip state or run something  else. This return is done with
this timeout callback. Which
looks logical to me to have a timeout callback in the same file as
application code.


> [...]
>
> > > > +     ret = ulwip_dhcp();
> > > > +
> > > > +     net_set_timeout_handler(2000UL, ulwip_timeout_handler);
> > > > +
> > > > +     ulwip_loop();
> > > > +     if (IS_ENABLED(CONFIG_CMD_TFTPBOOT)) {
> > > > +             ulwip_clear_tmo();
> > > > +
> > > > +             filename = env_get("bootfile");
> > > > +             if (!filename) {
> > > > +                     printf("no bootfile\n");
> > > > +                     return CMD_RET_FAILURE;
> > >
> > > Why is this a failure?  You just have the tftp command enabled but dont
> > > want to download anything
> > >
> > > thanks, if dhcp did not return filename, but only IP, then nothing to
> > download. It's not an error.
>
> Yes but downloading a file is not mandatory, it depends on a DHCP option.
> If you
> want to emulate this behaviour,  you need to fail only if 'bootfile' is
> set but
> cant be downloaded.
>
> >
> >
> > > > +             }
> > > > +
>
>
> Regards
> /Ilias
>