From patchwork Mon Jan 27 21:42:22 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Yann Droneaud X-Patchwork-Id: 314507 Return-Path: X-Original-To: incoming-imx@patchwork.ozlabs.org Delivered-To: patchwork-incoming-imx@bilbo.ozlabs.org Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:770:15f::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id CD3DC2C00A7 for ; Tue, 28 Jan 2014 08:46:21 +1100 (EST) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1W7tyL-0001Me-DP; Mon, 27 Jan 2014 21:44:01 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1W7tyJ-0002gL-0h; Mon, 27 Jan 2014 21:43:59 +0000 Received: from smtp4-g21.free.fr ([2a01:e0c:1:1599::13]) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1W7tyF-0002g1-LS for linux-arm-kernel@lists.infradead.org; Mon, 27 Jan 2014 21:43:57 +0000 Received: from localhost.localdomain (unknown [IPv6:2a01:e35:2e9f:6ac0:8e70:5aff:fe2f:2d74]) by smtp4-g21.free.fr (Postfix) with ESMTP id 8E9214C80E1; Mon, 27 Jan 2014 22:43:08 +0100 (CET) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by localhost.localdomain (8.14.7/8.14.7) with ESMTP id s0RLh5mN010945; Mon, 27 Jan 2014 22:43:05 +0100 Received: (from ydroneaud@localhost) by localhost.localdomain (8.14.7/8.14.7/Submit) id s0RLh0jS010944; Mon, 27 Jan 2014 22:43:00 +0100 From: Yann Droneaud To: Russell King - ARM Linux Subject: Re: [PATCH] arm64: Add pdev_archdata for dmamask Date: Mon, 27 Jan 2014 22:42:22 +0100 Message-Id: <1390858942-10904-1-git-send-email-ydroneaud@opteya.com> X-Mailer: git-send-email 1.8.5.3 In-Reply-To: <20140127203635.GA15937@n2100.arm.linux.org.uk> References: <1390845177-2626-1-git-send-email-lauraa@codeaurora.org> <20140127181836.GH26766@pengutronix.de> <1390854331.23180.22.camel@localhost.localdomain> <20140127203635.GA15937@n2100.arm.linux.org.uk> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140127_164356_631618_98A88836 X-CRM114-Status: GOOD ( 30.57 ) X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Yann Droneaud , linux-m68k@vger.kernel.org, Laura Abbott , Greg Kroah-Hartman , Catalin Marinas , Dmitry Torokhov , Will Deacon , Geert Uytterhoeven , Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= , sparclinux@vger.kernel.org, "David S. Miller" , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org List-Id: linux-imx-kernel.lists.patchwork.ozlabs.org Hi Russel, Le lundi 27 janvier 2014 à 20:36 +0000, Russell King - ARM Linux a écrit : > On Mon, Jan 27, 2014 at 09:25:31PM +0100, Yann Droneaud wrote: > > ARM, even AAAAARGH64 [1], doesn't need a special treatement regarding > > the infamous dma_mask pointer. So perhaps my solution is better. > > > > This solution (adding dma_mask in pdev_archdata) is already in use in > > powerpc architecture. See arch/powerpc/kernel/setup-common.c > > > > The advantage of this solution is that it makes a dma_mask placeholder > > available to statically allocated platform_device struct, while mine > > only address the problem for platform_device struct allocated with > > platform_device_alloc(). > > As I've already said in this thread, the basic problem comes from DT's > platform device creation. It's the responsibility of the device creator > to set the dma_mask pointer appropriately, and DT doesn't do that. So, > DT needs to be fixed rather than everyone introducing their own > workarounds for this. > Sure proliferation of fixes is not what we want. Note that Uwe added me to the thread not because I tried to address the same issue, but I tried to improve platform_device_register_full() to not allocate and leak an u64 used to hold the dma mask provided as part of platform_device_info. I believe he thought the two issues could be solved at the same time. > > I'm also considering using dma_set_mask_and_coherent() in > > platform_device_register_full() and drivers using > > platform_device_alloc(). > > As the one who introduced dma_set_mask_and_coherent, consider this a > strong NAK on that. The reason is dma_set_mask_and_coherent() is > for drivers to set their requirements, not for the bus requirements > to be set in the first place. > > It also means that drivers which need no DMA support are subjected to > DMA restrictions (in that dma_set_mask_and_coherent can error out if > the platform can't support the DMA mask.) > I'm not sure to understand your point here. If the driver explicitly set a DMA mask in its call to platform_device_register_full(), can't we suppose it should be subject to restriction from the platform ? As I'm not sure to be clear on this particular point, here's the patch I was considering, (sorry of being quite out of topic): ---8<--- Subject: [PATCH] driver core/platform: use dma_set_mask_and_coherent() in platform_device_register_full() dma_set_mask_and_coherent() is a nice function for setting the various DMA masks in struct device. Additionnaly, it checks the mask value against arch limits. It may be used to control the validity of the dma_mask given to platform_device_register_full() and setting the masks in struct platform_device. Note: as dma_set_mask() does nothing on m68k, or returns -EINVAL on sparc (if PCI support not built), etc. this could break in unpleasant ways I'm not yet able to detail ... Cc: Uwe Kleine-König Cc: Dmitry Torokhov Cc: Greg Kroah-Hartman Cc: Geert Uytterhoeven Cc: linux-m68k@lists.linux-m68k.org Cc: "David S. Miller" Cc: sparclinux@vger.kernel.org Signed-off-by: Yann Droneaud --- drivers/base/platform.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/base/platform.c b/drivers/base/platform.c index bc78848dd59a..ada1d366e1b6 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -446,8 +446,10 @@ struct platform_device *platform_device_register_full( if (!pdev->dev.dma_mask) goto err; - *pdev->dev.dma_mask = pdevinfo->dma_mask; - pdev->dev.coherent_dma_mask = pdevinfo->dma_mask; + ret = dma_set_mask_and_coherent(&pdev->dev, + pdevinfo->dma_mask); + if (ret) + goto err; } ret = platform_device_add_resources(pdev,