Subject: Re: Xilinx git tree at source.mvista.com

From: Andrei Konovalov <[email-masked]>
Date: Tue, 29 May 2007 12:07:33 +0400

John Williams wrote:
> Hi Wolfgang,
> 
> Wolfgang Reissnegger wrote:
>> David H. Lynch Jr. wrote:
>>
>>> For me the most significant issue is the bazillion layers of nested
>>> macro's and includes.
>>
>>
>> I don't see the macros as an issue, just look at the implementation of,
>> for example, spin_lock_irq() and Xilinx's macros seem like child's 
>> play :-)
>> As for includes, yes, there are a few too many header files. But, as
>> time progresses and the need arises they can be merged into fewer files.
> 
> It seems the kernel.org decision has been made re: the style issue. None 
> of the *_i.[ch], *_g.[ch] + adapter.c drivers will make it to mainline.

BTW, all the current drivers I am aware of have been moved to platform bus,
and do not use *_g.[ch] already.

> I understand why Xilinx did it this way, but to be honest never agreed 
> with it myself either.  Style issues aside, three levels of function 
> calls in an interrupt handler might be portable, but it still isn't a 
> good thing!

Another thing we've encountered while moving our current spi driver to the spi
framework is that sometimes there is too much "policy" in the level 1 drivers.
The level 1 spi driver controls the chip select on its own. For this reason
we were not able to use the spi_bitbang stuff. And even then we have to copy
the buffers into a single one to avoid CS toggling in the middle of the EEPROM
write sequence. The latter could probably be worked around, and could be due
to us not making the most of the level 1 driver, but at least this is not
trivial.

Yet another thing is that if one wants just FIFO mode for the TEMAC, he has
to include a bit of SGDMA code too as e.g. XTemac_Start() references
XDmaV3_SgStart() et al. IMHO wider usage of "virtual functions" would
help here (see e.g. how the System ACE driver by Grant L. handles different
bus widths).

Personally and in general, I like the idea of reusable OS independent code.
But it is hard to implement the way everyone would be happy with ;)

Thanks,
Andrei