Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753021AbbBMPUk (ORCPT ); Fri, 13 Feb 2015 10:20:40 -0500 Received: from mail-lb0-f179.google.com ([209.85.217.179]:61177 "EHLO mail-lb0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752123AbbBMPUj (ORCPT ); Fri, 13 Feb 2015 10:20:39 -0500 MIME-Version: 1.0 In-Reply-To: <54DE11FA.6050702@xs4all.nl> References: <1423650827-16232-1-git-send-email-ricardo.ribalda@gmail.com> <54DE11FA.6050702@xs4all.nl> From: Ricardo Ribalda Delgado Date: Fri, 13 Feb 2015 16:20:16 +0100 Message-ID: Subject: Re: [PATCH v2 1/3] media/videobuf2-dma-sg: Fix handling of sg_table structure To: Hans Verkuil Cc: Pawel Osciak , Marek Szyprowski , Kyungmin Park , Mauro Carvalho Chehab , linux-media , LKML , Hans Verkuil , martin.petersen@oracle.com, hch@lst.de, tonyb@cybernetics.com, axboe@fb.com, Stephen Rothwell , lauraa@codeaurora.org, Benjamin Herrenschmidt , webbnh@hp.com, hare@suse.de, Andrew Morton , Greg Kroah-Hartman Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2310 Lines: 59 Hello Hans On Fri, Feb 13, 2015 at 4:02 PM, Hans Verkuil wrote: > Hi Ricardo, Marek, > > I have a few questions, mostly to improve my own understanding. > > First of all, is this solving an actual bug for you, or did you just find > it while reviewing code? And if it solves a bug, then which architecture > are you using? ARM? Intel? > My arch is intel based (AMD APU). I found it while doing review. While updating our kernel to 3.19 I had to patch some of my out of tree drivers, and then I gave a look to the file. >> dma_map_sg returns the number of areas mapped by the hardware, >> which could be different than the areas given as an input. >> The output must be saved to nent. >> >> The output of dma_map, should be used to transverse the scatter list. >> >> dma_unmap_sg needs the value passed to dma_map_sg (nents_orig). > > I noticed that few dma_unmap_sg calls actually use orig_nents. It makes > me wonder if the dma_unmap_sg documentation is actually correct. It does > clearly state that orig_nents should be used, and it might well be that > the only reason this hasn't led to problems is that very few architectures > actually seem to return nents < orig_nents. It is not the most clear API to use :(. Some of the prototypes do not make a lot of sense, and it is documented outside the code. I have sent these two patches: https://lkml.org/lkml/2015/2/11/231 https://lkml.org/lkml/2015/2/11/232 >> + sgt->nents = dma_map_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents, >> + buf->dma_dir, &attrs); > > Is a driver free to change sgt->nents? It's unclear from the documentation > or code that that is actually the purpose of sgt->nents. Most drivers seem > to store the result of dma_map_sg into a driver-specific struct. As I understand it, this is the purpose of the struct scatter list, have at hand the three values that you need, the sgl, nents and orig_ents. But it would be great if the maintaner of the dma-api speaks up :) I am putting get_maintainer.pl in cc Thanks Hans! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/