Re: [spi-devel-general] Re: [PATCH 2.6-git] SPI core refresh

From: Vitaly Wool
Date: Sun Dec 11 2005 - 17:13:54 EST


David Brownell wrote:

But this solves the problem only partially since this technique fits only the synchronous transfers.



Synchronous transfers can easily use stack allocation for
the descriptions, yes.

Not that they need to ... the ads7846 driver allocates its
spi_message and spi_transfer objects on the heap both for
synchronous operations (temperature and voltage sensing) and
for asynch ones (touchscreen tracking from timer and irq).


So are you talking about only kzalloc vs kmalloc?
I was trying to compare the approaches in a somehow deeper way.
That said, I meant that not exposing any structures you don't have to expose was usually a right way to do things.
We don't expose SPI message and you do.
The only advantage of such exposure I could think of was possibilily to allocate messages on stack but this approach has some limitations we've just agrred upon.
On the other hand, our approach is flexible in means of message allocation. I. e. a small memory allocation library can be implemented transparently to device drivers that handles message allocation. It's very easy (and lightweight :)) since the message structure is always of a same length... Agree?

That said ... I know some people _do_ like krefcounted APIs that
do that kind of stuff. Strongly. Greg's been silent here other
than pointing out that your request alloc was too fat to inline.
Mine is trivially inlined, but not refcounted. Likely there's a
happy middle ground, maybe

mesg = spi_message_alloc(struct spi_device *, unsigned ntrans);
mesg = spi_message_get(mesg);
spi_message_put();


Well, it's pretty much what we do in the latest one...
Though we use one-by-one chaining and not specifying the number of messages in chain.
I'm not sure which approach is better, really. Maybe an API like
struct spi_mjsg *spi_message_alloc(struct spi_device *, unsigned ntrans);
spi_add_transfer(struct spi_msg *, ...);
...
is better than what we've got now (see patch sent 12/05; I plan to post the update tomorrow).
I would just like to say that defining such an API looks better thing to me than dealing with SPI message structure explicitly.

Vitaly
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/