From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756139AbcIGIs1 (ORCPT ); Wed, 7 Sep 2016 04:48:27 -0400 Received: from mout.kundenserver.de ([212.227.17.10]:63911 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751021AbcIGIsX (ORCPT ); Wed, 7 Sep 2016 04:48:23 -0400 From: Arnd Bergmann To: Peter Chen Cc: Leo Li , Felipe Balbi , 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" , Alan Stern , Greg Kroah-Hartman , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev Date: Wed, 07 Sep 2016 10:48:06 +0200 Message-ID: <12021424.cItk3A7CfE@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-34-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <20160907063313.GA13903@b29397-desktop> References: <5844875.KucAoPMrXi@wuerfel> <20160907063313.GA13903@b29397-desktop> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:3M78YPvi47+Mg2J9x233iT2VWBhup/klc9Rs+q4YUuChT7go70G WH0Bw2HzVmbKrO+dNtVo4chwhNqdDZrLmwn7dF6acSKtgBybx8pCiotpIsgb7kutlugvJ5p ovB83oyHsbryygH6F8nnKTgJPDNbFPrA/PbRf7sgV8M+eFnyEsckdofjQq5sBktPNLqcD/U RAHs7vTpkDhWjrNeIK2MA== X-UI-Out-Filterresults: notjunk:1;V01:K0:8SfXgd29YYo=:5XgbjkT5G3hv2JQKnfnopk FuP1uWbnNgqKvbSu8THvx9e8pw1MSxUw/e3gHijcCnDdRXv0fHxXeYp6/ATOVIUKjSg7uiwS0 94JxoxumW90Kh/qFOUbhCYat+AjjZF4Y3wFVn0wZytNTXcOut0C5iwEc/yDzfltdSWdVh+HRL RTyJ9Er9wPGJDnLvb9JtG39KflztUjN9TtmpYHNPMVvsqWKKBNNAznAYqq/jxFTxRLmA7FbX3 2mJfrOXR2MToYXtCFPgcPzcOs725OKTPy0fcnN9vcq+RQ54p/hOs1jplP14eIgQK6F0ezf597 6Obhr78fadXAKYO1CRDdGsUgwZmP4qoOHCJux3YheuP2G82FfzJaJrYbYKovz3NIXJZjFTZte Iy/3EL/u9gJ5AtGab6Oqj1RGbLuAFP/C07VaQK/gsfviMwilgbKmSQ9ebLogDowsBLM5a+/Lt +iWcWOf/9wGg2kU8bRWabCGXygHN0rcqH4W2RI8ob7F8BqTZ2u+8D20ENtCoXVfNQSNkeiWKn 7pBY+wpPhTmef52mvcl1rLdKkhUdAUo2PAPonYzFLs8C5rm7t2UkU2AoA79mhwpu8Vw3ls3/d PC+4LNCVEY038A1suBPUuqTI8foYb8tRiNT4lQmAEI71eEQSGz7N6HQAgReLBIyVD0TZA8kVs d5P7E1o7KsoPoJS1R/E2ty4dHS1eMQ3WsXHHcNQbSquj0BxAV/mPDvcWHFCJHzfeutUe3X6hc 1S8c6e3uyGC0L1Fw Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, September 7, 2016 2:33:13 PM CEST Peter Chen wrote: > > > > Right, that should make it work with iommu as well. However, it does > > not solve the other issue I mentioned above, with boards that have > > USB devices hardwired to a chipidea host controller that need > > configuration from DT. For that, we still need to come up with another > > way to associate the DT hierarchy in the host bridge node with > > the Linux platform_device. > > > > Why? The DMA configuration is for host controller, not for USB device. > No matter there is hardwired or hotplug devices, the DMA configuration > for host controller are both inherited from glue layer platform devices, > current implementation is at function ci_hdrc_add_device, > drivers/usb/chipidea/core.c. I wasn't referring to DMA configuration there, but only to how we set the of_node pointer in register_root_hub, which you added in dc5878abf49 ("usb: core: move root hub's device node assignment after it is added to bus") as: usb_dev->dev.of_node = parent_dev->of_node; As I understand, parent_dev (aka hcd->self.controller) here refers to the device that you add in ci_hdrc_add_device(), which does not have an of_node pointer, so we actually want parent_dev->parent->of_node. I'm sure you understand that code better than me, so let me know what my mistake is if this indeed works correctly. Regarding the DMA configuration that you mention in ci_hdrc_add_device(), I think we should replace pdev->dev.dma_mask = dev->dma_mask; pdev->dev.dma_parms = dev->dma_parms; dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask); with of_dma_configure(), which has the chance to configure more than just those three, as the dma API might look into different aspects: - iommu specific configuration - cache coherency information - bus type - dma offset - dma_map_ops pointer We try to handle everything in of_dma_configure() at configuration time, and that would be the place to add anything else that we might need in the future. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Wed, 07 Sep 2016 10:48:06 +0200 Subject: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev In-Reply-To: <20160907063313.GA13903@b29397-desktop> References: <5844875.KucAoPMrXi@wuerfel> <20160907063313.GA13903@b29397-desktop> Message-ID: <12021424.cItk3A7CfE@wuerfel> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wednesday, September 7, 2016 2:33:13 PM CEST Peter Chen wrote: > > > > Right, that should make it work with iommu as well. However, it does > > not solve the other issue I mentioned above, with boards that have > > USB devices hardwired to a chipidea host controller that need > > configuration from DT. For that, we still need to come up with another > > way to associate the DT hierarchy in the host bridge node with > > the Linux platform_device. > > > > Why? The DMA configuration is for host controller, not for USB device. > No matter there is hardwired or hotplug devices, the DMA configuration > for host controller are both inherited from glue layer platform devices, > current implementation is at function ci_hdrc_add_device, > drivers/usb/chipidea/core.c. I wasn't referring to DMA configuration there, but only to how we set the of_node pointer in register_root_hub, which you added in dc5878abf49 ("usb: core: move root hub's device node assignment after it is added to bus") as: usb_dev->dev.of_node = parent_dev->of_node; As I understand, parent_dev (aka hcd->self.controller) here refers to the device that you add in ci_hdrc_add_device(), which does not have an of_node pointer, so we actually want parent_dev->parent->of_node. I'm sure you understand that code better than me, so let me know what my mistake is if this indeed works correctly. Regarding the DMA configuration that you mention in ci_hdrc_add_device(), I think we should replace pdev->dev.dma_mask = dev->dma_mask; pdev->dev.dma_parms = dev->dma_parms; dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask); with of_dma_configure(), which has the chance to configure more than just those three, as the dma API might look into different aspects: - iommu specific configuration - cache coherency information - bus type - dma offset - dma_map_ops pointer We try to handle everything in of_dma_configure() at configuration time, and that would be the place to add anything else that we might need in the future. Arnd