Re: [PATCH v3 1/4] libata: add R-Car SATA driver

[Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>]

On 28-05-2013 4:09, Tejun Heo wrote:

>>> Yeah, that's exactly where I'm confused.  ATA_DMA_BOUNDARY is 64k
>>> which becomes both queue_segment_boundary and dma seg_boundary.
>>> AFAICS, both __blk_segment_map_sg() and dma mapping won't merge across
>>> seg_boundary and as each bvec is a single page at most, we shouldn't
>>> need to worry about getting sg's which cross 64k boundaries in bmdma
>>> controllers.  Hmmmmm...... I gotta be missing something.  What am I
>>> missing here?

>>      Probably nothing... maybe Jeff just copied that from ATADRVR
>> (which was his inspiration IIRC).

    Maybe we should ask him instead of guessing. :-)

> Yeah, maybe.  I can't find any reason why ata_bmdma_fill_sg() would
> need to worry about 64k boundary.  The dumb variant is putting an
> extra restriction on it but the plain one doesn't seem to be adding
> anything.  Would you be interested in submitting a patch to remove
> those?

    Hm, at least not right now. I'm too busy.

>>     BTW, I've just done some experimentation with my R-Car target
>> and ATA_DEBUG/ATA_VERBOSE_DEBUG #define'd and it turned out
>> that block layer didn't merge any segments at all... :-/

> Even the ones with contiguous physical addresses?

     Exactly. All DMA segments were 4K in size, regardless of whether 
the addresses were adjacent or not.

> It could just be
> that the physical pages are disjoint as we don't really do anything to
> allocate pages contiguously and iommus don't kick in / merge unless
> necessary (it's not like modern hardware struggle with sglists
> anyway), so it could just be that there's nothing to merge.

    There were mergeable segments, for sure.

> Thanks.

MBR, Sergei