From patchwork Thu Sep 8 11:00:13 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Felipe Balbi X-Patchwork-Id: 9321015 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C67196077F for ; Thu, 8 Sep 2016 11:03:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B9721297C4 for ; Thu, 8 Sep 2016 11:03:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AD63D297C6; Thu, 8 Sep 2016 11:03:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E6C4E297C5 for ; Thu, 8 Sep 2016 11:03:08 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bhx59-0004o9-R4; Thu, 08 Sep 2016 11:01:23 +0000 Received: from mga02.intel.com ([134.134.136.20]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bhx52-0004Mb-8e for linux-arm-kernel@lists.infradead.org; Thu, 08 Sep 2016 11:01:20 +0000 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 08 Sep 2016 04:00:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,300,1470726000"; d="asc'?scan'208";a="1026822855" Received: from pipin.fi.intel.com (HELO localhost) ([10.237.68.160]) by orsmga001.jf.intel.com with ESMTP; 08 Sep 2016 04:00:50 -0700 From: Felipe Balbi To: Arnd Bergmann Subject: Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev In-Reply-To: <4934737.egJZVdaLZs@wuerfel> References: <2733202.7lpFC7RnDm@wuerfel> <87lgz2iv6d.fsf@linux.intel.com> <4934737.egJZVdaLZs@wuerfel> User-Agent: Notmuch/0.22.1+63~g994277e (https://notmuchmail.org) Emacs/25.1.3 (x86_64-pc-linux-gnu) Date: Thu, 08 Sep 2016 14:00:13 +0300 Message-ID: <87d1keirlu.fsf@linux.intel.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160908_040116_488716_1C9F5A65 X-CRM114-Status: GOOD ( 28.39 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Grygorii Strashko , Russell King - ARM Linux , Catalin Marinas , Yoshihiro Shimoda , "linux-usb@vger.kernel.org" , Sekhar Nori , lkml , Stuart Yoder , Scott Wood , David Fisher , "Thang Q. Nguyen" , Leo Li , "linux-arm-kernel@lists.infradead.org" , Greg Kroah-Hartman , Alan Stern , Peter Chen Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, Arnd Bergmann writes: > On Thursday, September 8, 2016 12:43:06 PM CEST Felipe Balbi wrote: >> Arnd Bergmann writes: >> > On Thursday, September 8, 2016 11:29:04 AM CEST Felipe Balbi wrote: >> >> > If we do that, we have to put child devices of the dwc3 devices into >> >> > the platform glue, and it also breaks those dwc3 devices that don't >> >> > have a parent driver. >> >> >> >> Well, this is easy to fix: >> >> >> >> if (dwc->dev->parent) { >> >> dwc->sysdev = dwc->dev->parent; >> >> } else { >> >> dev_info(dwc->dev, "Please provide a glue layer!\n"); >> >> dwc->sysdev = dwc->dev; >> >> } >> > >> > I don't understand. Do you mean we should have an extra level of >> > stacking and splitting "static struct platform_driver dwc3_driver" >> > in two so instead of >> > >> > "qcom,dwc3" -> "snps,dwc3" (usb_bus.sysdev) -> "xhci" (usb_bus.dev) >> > >> > we do this? >> > >> > "qcom,dwc3" -> "snps,dwc3" (usb_bus.sysdev) -> "dwc3-glue" -> "xhci" (usb_bus.dev) >> >> no >> >> If we have a parent device, use that as sysdev, otherwise use self as >> sysdev. > > But there is often a parent device in DT, as the xhci device is > attached to some internal bus that gets turned into a platform_device > as well, so checking whether there is a parent will get the wrong > device node. oh, that makes things more interesting :-s >> > That sounds a bit clumsy for the sake of consistency with PCI. >> > The advantage is that xhci can always use the grandparent device >> > as sysdev whenever it isn't probed through PCI or firmware >> > itself, but the purpose of the dwc3-glue is otherwise questionable. >> > >> > How about adding a 'compatible="snps,dwc3-pci"' property for the dwc3 >> > device when that is created from the PCI driver and checking for that >> > with the device property interface instead? If it's "snps,dwc3" >> > we use the device itself while for "snps,dwc3-pci", we use the parent? >> >> Any reason why we wouldn't use e.g. dwc3-omap.dev as sysdev? > > That would be incompatible with the USB binding, as the sysdev > is assumed to be a USB host controller with #address-cells=<1> > and #size-cells=<0> in order to hold the child devices, for > example: > > / { > omap_dwc3_1: omap_dwc3_1@48880000 { > compatible = "ti,dwc3"; > #address-cells = <1>; > #size-cells = <1>; > ranges; > usb1: usb@48890000 { > compatible = "snps,dwc3"; > reg = <0x48890000 0x17000>; > #address-cells = <1>; > #size-cells = <0>; > interrupts = , > , > ; > interrupt-names = "peripheral", > "host", > "otg"; > phys = <&usb2_phy1>, <&usb3_phy1>; > phy-names = "usb2-phy", "usb3-phy"; > > hub@1 { > compatible = "usb5e3,608"; > reg = <1>; > #address-cells = <1>; > #size-cells = <0>; > > ethernet@1 { > compatible = "usb424,ec00"; > mac-address = [00 11 22 33 44 55]; > reg = <1>; > }; > }; > }; > }; > > It's also the node that contains the "phys" properties and > presumably other properties like "otg-rev", "maximum-speed" > etc. > > If we make the sysdev point to the parent, then we can no longer > look up those properties and child devices from the USB core code > by looking at "sysdev->of_node". this also makes things more interesting. I can't of anything other than having some type of flag passed via e.g. device_properties by dwc3-pci.c :-s It's quite a hack, though. I still think that inheriting DMA (or manually initializing a child with parent's DMA bits and pieces) is the best way to go. So we're back to of_dma_configure() and acpi_dma_configure(), right? But this needs to be done before dwc3_probe() executes. For dwc3-pci that's easy, but for DT devices, seems like it should be in of core. Below is, clearly, not enough but should show the idea: diff --git a/drivers/of/device.c b/drivers/of/device.c index fd5cfad7c403..a54610198946 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -94,8 +94,12 @@ void of_dma_configure(struct device *dev, struct device_node *np) * Set default coherent_dma_mask to 32 bit. Drivers are expected to * setup the correct supported mask. */ - if (!dev->coherent_dma_mask) - dev->coherent_dma_mask = DMA_BIT_MASK(32); + if (!dev->coherent_dma_mask) { + if (!dev->parent->coherent_dma_mask) + dev->coherent_dma_mask = DMA_BIT_MASK(32); + else + dev->coherent_dma_mask = dev->parent->coherent_dma_mask; + } /* * Set it to coherent_dma_mask by default if the architecture