All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrei Konovalov <akonovalov@ru.mvista.com>
To: John Williams <jwilliams@itee.uq.edu.au>
Cc: linuxppc-dev@ozlabs.org,
	Wolfgang Reissnegger <wolfgang.reissnegger@xilinx.com>,
	"David H. Lynch Jr." <dhlii@dlasys.net>,
	linuxppc-embedded@ozlabs.org
Subject: Re: Xilinx git tree at source.mvista.com
Date: Tue, 29 May 2007 12:07:33 +0400	[thread overview]
Message-ID: <465BDF45.1010904@ru.mvista.com> (raw)
In-Reply-To: <465B6611.1070107@itee.uq.edu.au>

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

  reply	other threads:[~2007-05-29  8:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-24  8:53 Xilinx git tree at source.mvista.com Andrei Konovalov
2007-05-24  9:57 ` David H. Lynch Jr.
2007-05-24 13:08   ` Andrei Konovalov
2007-05-24 15:42     ` Wolfgang Reissnegger
2007-05-25  1:49       ` David H. Lynch Jr.
2007-05-25  4:26         ` Wolfgang Reissnegger
2007-05-28 23:30           ` John Williams
2007-05-29  8:07             ` Andrei Konovalov [this message]
2007-05-25  0:47     ` David H. Lynch Jr.
2007-06-12 10:29 ` Esben Nielsen
2007-06-12 17:09   ` Dale Farnsworth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=465BDF45.1010904@ru.mvista.com \
    --to=akonovalov@ru.mvista.com \
    --cc=dhlii@dlasys.net \
    --cc=jwilliams@itee.uq.edu.au \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=linuxppc-embedded@ozlabs.org \
    --cc=wolfgang.reissnegger@xilinx.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.