[staging] driver for adis16255 gyroscope

[Jiri Slaby]

On 04/29/2010 10:35 AM, matthias wrote:
> 2010/4/29 Jiri Slaby <jirislaby at gmail.com>:
>> On 04/28/2010 08:45 PM, matthias wrote:
>>> --- /dev/null
>>> +++ b/drivers/staging/adis16255/adis16255.c
>>> +static int spi_adis16255_read_data(struct spi_adis16255_data *spiadis,
>>> +               u8 adr,
>>> +               u8 *rbuf)
>>> +{
>>> +   struct spi_device *spi = spiadis->spi;
>>> +   struct spi_message msg;
>>> +   struct spi_transfer xfer1, xfer2;
>>> +   u8 *buf, *rx;
>>> +   int ret;
>>> +
>>> +   buf = kmalloc(4, GFP_KERNEL);
>>> +   if (!buf)
>>> +      return -ENOMEM;
>>> +
>>> +   rx = kzalloc(4, GFP_KERNEL);
>>> +   if (!rx)
>>> +      return -ENOMEM;
>>
>> buf leaks here. You can just use buffers on stack for 8 bytes (unless it
>> DMAs there).
> 
> I'm not sure if I understand you right. A buffer can not be put on the
> stack if it is not multiple of 8. Why? (sorry, maybe off topic here)

No, I meant you alloc 4+4 bytes which can be safely be on stack. UNLESS
the SPI layer does DMA to/from that memory. In that case you should
better handle failures (a leak) in this code.

>>> +   schedule_work(&spiadis->wq);
>>
>> So this can be threaded irq with NULL handler, no?
> 
> Why? I request the interrupt in the probe function, passing spiadis struct.
> Should I check for spiadis, for defensive programming sake?

You know what is threaded irq, don't you? You just request_threaded_irq
with NULL for irq handler and directly spi_workqueue as a thread
function (with changed prototype). It will save you from messing up with
workqueues.