Re: [PATCH 5/7] swiotlb: (re)Create swiotlb_unmap_single

[FUJITA Tomonori]

On Tue, 7 Apr 2009 10:32:20 -0500
Kumar Gala <galak@xxxxxxxxxxxxxxxxxxx> wrote:

> 
> On Apr 7, 2009, at 4:09 AM, FUJITA Tomonori wrote:
> 
> > On Tue, 7 Apr 2009 01:34:44 -0500
> > Kumar Gala <galak@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> >>
> >> On Apr 6, 2009, at 9:24 PM, FUJITA Tomonori wrote:
> >>
> >>> On Fri,  3 Apr 2009 20:56:47 -0500
> >>> Becky Bruce <beckyb@xxxxxxxxxxxxxxxxxxx> wrote:
> >>>
> >>>> This mirrors the current swiotlb_sync_single() setup
> >>>> where the swiotlb_unmap_single() function is static to this
> >>>> file and contains the logic required to determine if we need
> >>>> to call actual sync_single.  Previously, swiotlb_unmap_page
> >>>> and swiotlb_unmap_sg were duplicating very similar code.
> >>>> The duplicated code has also been reformatted for
> >>>> readability.
> >>>>
> >>>> Note that the swiotlb_unmap_sg code was previously doing
> >>>> a complicated comparison to determine if an addresses needed
> >>>> to be unmapped where a simple is_swiotlb_buffer() call
> >>>> would have sufficed.
> >>>>
> >>>> Signed-off-by: Becky Bruce <beckyb@xxxxxxxxxxxxxxxxxxx>
> >>>> ---
> >>>> lib/swiotlb.c |   36 +++++++++++++++++++++++-------------
> >>>> 1 files changed, 23 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> >>>> index af2ec25..602315b 100644
> >>>> --- a/lib/swiotlb.c
> >>>> +++ b/lib/swiotlb.c
> >>>
> >>> I don't think 'swiotlb_unmap_single' name is appropriate.
> >>>
> >>> swiotlb_unmap_single sounds like an exported function that IOMMUs  
> >>> can
> >>> use (and it was) however it should not be.
> >>
> >> What do you suggest we call it?  __swiotlb_unmap_single.
> >
> > I think that __swiotlb_unmap_single is better because the name implies
> > that it's an internal function. It's fine by me.
> >
> > If it is odd that __swiotlb_unmap_single() is just a wrapper function
> > of unmap_single(), which does the real job to unmap a dma mapping, it
> > might be another possible option to rename unmap_single to
> > do_unamp_single and use unmap_single.
> 
> I think you lost me here.  I'd prefer to just use  
> __swiotlb_unmap_single at this point and get this code into the tree  
> and work on such renaming after the fact (if that's ok).

If you are rushing to merge this right now, the original patchset is
fine by me (I thought that you missed this merge window). I'll rename
it later.
--
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/