Re: [PATCHv3] block: Add support for Sony SxS cards

[Jens Axboe <axboe@kernel.dk>]

On 09/28/2014 08:39 AM, Kieran Kunhya wrote:
>> So a few questions here... This device only does reads? And it seems to 
>> be assuming that only reads end up in the request handler? How so?
> 
> I have only reverse engineered the read-part of the device. Yes, the 
> driver should check whether the request is a read. 

And you should also deny any writeable opens, in that case. Add an open
method and check for FMODE_WRITE. Then add a check ala:

	if (bio_data_dir(bio)) {
		bio_endio(bio, -EROFS);
		return;
	}

and the top of your sxs_request().

>> Second question. IO never fails? There's no status checking and IO is 
>> always ended successfully. This too seems odd.
> 
> I will fix this.
> 
>> Third, this wong work as-is, at least not of HIGHMEM is used. After the 
>> bvec_kmap_irq(), irqs will be disabled. But your sxs_memcpy_read() 
>> invokes schedule through the completion waits, that will instantly go bad.
> 
> Is there a better way of doing this? I spent a long time trying to get DMA 
> to work but couldn't so had to resort to this ugly hack. I assume the memcpy 
> also needs to have some locking somewhere?

It really is pretty horrible and slow, but if you can't get DMA working,
then so be it. And yes, you should add a mutex that is held for the
duration of the memcpy_read(), otherwise things will go bad very fast if
two or more processes attempt to read from it at the same time. A quick
work-around for the irq issue would be to have the block layer bounce
the highmem pages for you. That's actually the default behavior, but I
would make it explicit with a call to:

	blk_queue_bounce_limit(q, BLK_BOUNCE_HIGH);

after the blk_queue_make_request() call. If you do that, then get rid of
the bvec_kmap_irq(), and just do:

	buffer = page_address(bvec.bv_page) + bvec.bv_offset;

to get the destination for your read.

-- 
Jens Axboe