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

[Vitaly Wool]

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_msg *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/