Re: [00/02] MPC5200 Bestcomm platform driver

[Sylvain Munaut <tnt@246tNt.com>]

Hi Andrey,

Andrey Volkov wrote:
> Hi Sylvain
> 
> This is first part of "platformizied" bestcomm/fec drivers.
> 
> Comments/Commit?

Obviously I haven't yet had the time to review all the code but the
glance I had looked good ! I'll review it deeper and test it and come
back to you asap.

Still, some "preliminary" comments :

 - I never really liked to have multiple "type" of buffer descriptors
depending of the number of pointers in them. "standard" task have
either 1 or 2 pointers true but I have custom tasks with 3 so I need a
subtmitbuffer3 ... Not very extensible imho. I think there is no problem
as defining the descriptor structure with an array of pointer and then
just allocate the good size at init. Whoever use them must anyway know
the number of pointer to fill.

 - When I started to clean up bescomm a while ago, the only thing I
really got done was a rewrite of the SRAM allocator that supports the
freeing of block at little overcost. I'll try to find it and send it to you.

 - I like the separation of phys/virt ;)

 - sdma_clear_irq(struct sdma *s) is useless, interrupt acking for the
SDMA is already done in mpc52xx_irq.c

 - I thought of separating bestcomm.h in two headers : one public for
the drivers that use the SDMA like the fec. one private for the
bestcomm.c and the tasks implementation. I think it makes sense but I
never deeply looked it one wouldn't end up almost empty.



... to be continued ;)



	Sylvain