[PATCH 5/5] realtek: restructure rtl_table_read/write

Sander Vanheule sander at svanheule.net
Sun Oct 23 13:09:44 PDT 2022


On Fri, 2022-10-14 at 23:06 +0200, Jan Hoffmann wrote:
> These two functions are identical apart from writing different values to
> the read/write bit. Create a new function rtl_table_exec to reduce code
> duplication.
> 
> Also replace the unbounded busy-waiting loop. As the hardware usually
> responds very quickly, always call cond_resched, so that callers do not
> need to worry about blocking for too long. This is especially important
> for the L2 table, as a full loop (as in rtl83xx_port_fdb_dump) takes
> about 20ms on RTL839x (and that function actually ends up being called
> in a loop for every port).
> 
> So far, polling timeout errors are only handled by logging an error, but
> a return value is added to allow proper handling in the future.
> 
> Signed-off-by: Jan Hoffmann <jan at 3e8.eu>
> ---
> 
> I'm not really sure if putting cond_resched in this place is really the
> best solution. An alternative would be to place it directly in the loops
> (i.e. rtl83xx_port_fdb_dump and l2_table_show) instead.

I'm no expert on this, but it does appear to be more common to call
cond_resched() from loops. If rtl_table_exec() once is not an issue, but calling
it repeatedly is, then it would make more sense to me to put this in the loops.
If anyone want to argue otherwise, I'd be happy to hear so though.

Otherwise this patch looks good to me. Putting upper bounds on loop durations,
however unlikely to fail, is not something I'm going to refuse :)

> 
> About error handling: Actually implementing that for all calls is going
> to require large changes. And doing it in a proper way which is better
> than just printing an error (i.e. actually trying to leave a consistent
> state) is non-trivial. I started to work on that only for fdb/mdb
> access, but that is only a part of where these tables are used:
> 
> https://github.com/janh/openwrt/commit/e45403299208d49675a0403ad94349ebdff2a374
> https://github.com/janh/openwrt/commit/fa68b23cab542683832696f039cf160b96da83db
> https://github.com/janh/openwrt/commit/92abfc2e4925e5bc1a0e25e464ca1d08833c9b30

Thanks for your work on improving this driver!

Best,
Sander



More information about the openwrt-devel mailing list