Re: [RFC] SPI core -- revisited

Vitaly Wool
2005-06-27T10:20:53Z

Russell King wrote:

>On Mon, Jun 27, 2005 at 01:13:44PM +0400, Vitaly Wool wrote:
>
>
>>Arjan van de Ven wrote:
>>
>>
>>>how is that??
>>>
>>>
>>These functions are not exactly *wrappers*, there's some little
>>additional logic inside.
>>spi-pnx0105_atmel.c uses spi_pnx_msg_buff_t structure to embed physical
>>and virtual address and length of the memory area allocated by
>>consistent_alloc, so if we just get rid of the alloc/free functions,
>>we'll copy wrong data from the userspace and nothing'll work.
>>
>>Let's look at it from another point. When a read request comes from the
>>userspace to spi-dev, spi-dev should allocate memory and copy the user
>>data in there. The problem is it is not (and shouldn't be) aware whether
>>the transfer is gonna be DMA or not so spi-dev can't choose
>>theappropriate method of memory allocation. Therefore it's reasonable to
>>let algorithm provide routines to do that.
>>
>>
>
>Sorry, this isn't making sense. Are you implying that you want to use
>DMA to copy data from user to kernel space? Or something else?
>
>In any case, I do hope that you are aware that copy_to_user/copy_from_user
>have rather important side effects other than just copying data? They
>cause page faults which ensure that the user space pages are paged in
>and/or copied if written to as appropriate.
>
>
Looks like you've misunderstood me. I was talking only about mem-to-dev
and dev-to-mem DMA transfers, where dev is actually an SPI device. The
thing I was talking about was that the 'wrappers' are necessary to call
*real* copy_from_user/copy_to_user with the appropriate parameters. I
hope this can also be derived by looking in the code.

>In another post, you mention that this patch was not provided for review.
>However, it _is_ effectively provided for review because it's an
>illustration of _how_ the interfaces are used - and we're reviewing
>the interfaces provided by the SPI core.
>
>
>
Ok, readily agreeing to that.
But -- does the interfaces review imply checking wthether the figure
brackets are complying to K&R? ;)