Re: [PATCH 2/3] fastboot net: implement fastboot over UDP

["Daniel Glöckner" <dg@emlix.com>]

Am 03/05/20 um 08:50 schrieb Sascha Hauer:
>> +		const char *fastboot_files = getenv("nv.fastboot.files");
> 
> Please don't use nv variables directly, always use the corresponding
> global variable.
> 
> For USB fastboot we have a variable global.usbgadget.fastboot_function
> for the same purpose. We should probably use the same variable for both
> fastboot variants and maybe also for DFU. How about moving the variable
> name out of that usbgadget namespace and use something like
> global.update_partitions? There might be better names...

Since there is also the question what to do with global.usbgadget
.fastboot_max_download_size, we could introduce a new namespace for
fastboot and call the variables global.fastboot.update_partitions
and global.fastboot.max_download_size. Although I would prefer
"partitions" to "update_partitions".

> Interestingly this only became a problem with my change, I tested your
> fastboot net patches as-is and it works fluently.

Maybe it works with the fastboot code because the poller is registered
in a very late initcall. Or did it cause problems for you after all
initcalls had run?

> My
> favourite solution would be to move net_poll() inside a poller indeed,
> but I currently have no good idea how to fix this mdio read problem.

How about wrapping the body of mdiobus_read, mdiobus_write, eth_send,
eth_rx, eth_check_open, and maybe even mdiobus_register, eth_register,
and eth_unregister in

net_entered++;
...
net_entered--;

and then immediately returning from net_poll if net_entered != 0 on
entry? That's similar to how poller_call prevents reentrance.


Now that I have shifted the focus to poller_call, how about changing that
function to allow being entered N times (with N configurable since stack
space is limited) to allow fastboot to send keep alive messages and the
watchdog to be serviced? We could add a flag to poller_struct that tells
poller_call to skip this poller when entered again when its function is
already running. The keep alives would then be sent from another poller.

Hm, I think I'll rewrite fastboot_poll to return while waiting for more
download data. Then only the command execution can trigger a watchdog
reset.

Best regards,

  Daniel

-- 
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11,
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke
Ust-IdNr.: DE 205 198 055

emlix - your embedded linux partner

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox