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

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

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 

[...]

> > > +     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