From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756255AbcIVIfK (ORCPT ); Thu, 22 Sep 2016 04:35:10 -0400 Received: from mail-he1eur01on0084.outbound.protection.outlook.com ([104.47.0.84]:51552 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752250AbcIVIfH (ORCPT ); Thu, 22 Sep 2016 04:35:07 -0400 X-Greylist: delayed 12753 seconds by postgrey-1.27 at vger.kernel.org; Thu, 22 Sep 2016 04:35:06 EDT From: Sriram Dash To: Arnd Bergmann CC: Felipe Balbi , Peter Chen , "Leo Li" , 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" , Suresh Gupta Subject: RE: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev Thread-Topic: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev Thread-Index: AQHRnyfo8hAvuSqBeEa9c0v53Aihx5+byEYAgAAgqICAAWeLAIAAixwAgAADb4CAABurgIAAA/OAgAANzoCAAA2DgIAABMCAgAAChwCAACDQAIAAEMYAgACfpQCAAIBDAIAAAioAgAAA7ACAxohLAIAA0VKAgADBlYCABEgOgIAA+lqAgABD5ICAAU3OgIAAJbAAgAAa1ICAAFPSAIAVtPJQgAAKvACAAAI6QIAAEyWAgAEP5qA= Date: Thu, 22 Sep 2016 05:02:17 +0000 Message-ID: References: <3596831.dckG5peiyF@wuerfel> <20042182.GcPMJken6Z@wuerfel> In-Reply-To: <20042182.GcPMJken6Z@wuerfel> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=sriram.dash@nxp.com; x-originating-ip: [192.88.169.1] x-ms-office365-filtering-correlation-id: 4b83a228-2659-4847-2723-08d3e2a5a06e x-microsoft-exchange-diagnostics: 1;DB6PR0401MB2632;7:REoStl/BcYWwLQyh04WlF9oeHpI5V6eEociQuAGdUs9CJRAR8rxqPOOhtTGnjuCWmJICMQ71ZZl6W+XluWdSibtTKJcNGX0XytPtICsDO1l5itNayWSwgYnSyMC0ToDTfuUej2Yu2WZMkg0h4gdXGrfMKVNd+dy+T7tPoYll7po1xtTNUn9/fDdtL0jz7XIjaBnOhoFl1nZSDpTRsmWYtaTsZdATM6bgWVudn/C7EELueZx8lQiAZciKbX8QxgXXTnIZNtlr3XN5l4k3IQx2q69LLjGPSZ+ctiilFNQdIRd82ok664mLam8l5CdRQ1l+qshsc9rAcIXDTTIc/mi8qw== x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DB6PR0401MB2632; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(185117386973197); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026);SRVR:DB6PR0401MB2632;BCL:0;PCL:0;RULEID:;SRVR:DB6PR0401MB2632; x-forefront-prvs: 0073BFEF03 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(24454002)(377454003)(199003)(189002)(106116001)(105586002)(93886004)(7416002)(575784001)(8936002)(8676002)(86362001)(81156014)(81166006)(5890100001)(106356001)(122556002)(9686002)(19580395003)(97736004)(19580405001)(50986999)(101416001)(76576001)(11100500001)(54356999)(76176999)(189998001)(5002640100001)(66066001)(102836003)(3846002)(6116002)(305945005)(586003)(4326007)(77096005)(92566002)(74316002)(5660300001)(15975445007)(2906002)(87936001)(10400500002)(110136003)(7846002)(3660700001)(7736002)(3280700002)(68736007)(33656002)(2950100001)(7696004)(2900100001);DIR:OUT;SFP:1101;SCL:1;SRVR:DB6PR0401MB2632;H:DB5PR0401MB1925.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Sep 2016 05:02:17.5454 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0401MB2632 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u8M8ZEkF012129 >From: Arnd Bergmann [mailto:arnd@arndb.de] >On Wednesday, September 21, 2016 11:43:59 AM CEST Sriram Dash wrote: >> >From: Arnd Bergmann [mailto:arnd@arndb.de] On Wednesday, September >> >21, 2016 11:06:47 AM CEST Sriram Dash wrote: >> >> ============================================================== >> From 8b0dea1513e9e73a11dcfa802ddc71cca11d66f8 Mon Sep 17 00:00:00 2001 >> From: Sriram Dash >> Date: Wed, 21 Sep 2016 11:39:30 +0530 >> Subject: [PATCH] usb: xhci: Fix the patch inherit dma configuration >> from parent dev >> >> Fixes the patch https://patchwork.kernel.org/patch/9319527/ >> ("usb: dwc3: host: inherit dma configuration from parent dev"). >> >> Signed-off-by: Sriram Dash >> --- >> drivers/usb/host/xhci-mem.c | 12 ++++++------ >> drivers/usb/host/xhci.c | 20 ++++++++++---------- >> 2 files changed, 16 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c >> index 6afe323..79608df 100644 >> --- a/drivers/usb/host/xhci-mem.c >> +++ b/drivers/usb/host/xhci-mem.c > >All the changes you did to this file seem fine, I completely missed that part. > >> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index >> 01d96c9..9a1ff09 100644 >> --- a/drivers/usb/host/xhci.c >> +++ b/drivers/usb/host/xhci.c Yes. Some sanity is required over this patch. >> @@ -231,7 +231,7 @@ static int xhci_free_msi(struct xhci_hcd *xhci) >> static int xhci_setup_msi(struct xhci_hcd *xhci) { >> int ret; >> - struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller); >> + struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev); >> >> ret = pci_enable_msi(pdev); >> if (ret) { > >This one is interesting as I stumbled over some code comment mentioning that for >dwc3-pci, we don't support MSI. I think with this change, we /can/ actually support >MSI, but this could be a separate patch and we'd have to test it on dwc3-pci >hardware. Same for most of this file. > >> @@ -745,7 +745,7 @@ void xhci_shutdown(struct usb_hcd *hcd) >> struct xhci_hcd *xhci = hcd_to_xhci(hcd); >> >> if (xhci->quirks & XHCI_SPURIOUS_REBOOT) >> - usb_disable_xhci_ports(to_pci_dev(hcd->self.controller)); >> + usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev)); >> >> spin_lock_irq(&xhci->lock); >> xhci_halt(xhci); > >This seems obviously correct, but I don't yet see why it currently works. We >probably don't call this function on dwc3. > >> #ifdef CONFIG_PM >> @@ -3605,7 +3605,7 @@ void xhci_free_dev(struct usb_hcd *hcd, struct >usb_device *udev) >> * if no devices remain. >> */ >> if (xhci->quirks & XHCI_RESET_ON_RESUME) >> - pm_runtime_put_noidle(hcd->self.controller); >> + pm_runtime_put_noidle(hcd->self.sysdev); >> #endif >> >> ret = xhci_check_args(hcd, udev, NULL, 0, true, __func__); > >I suspect this one is wrong, based on what Felipe explained earlier: >the power management should propagate down from the child to the parent >device. > >Someone who understands this better than I do should look at it. > >> @@ -3745,7 +3745,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct >usb_device *udev) >> * suspend if there is a device attached. >> */ >> if (xhci->quirks & XHCI_RESET_ON_RESUME) >> - pm_runtime_get_noresume(hcd->self.controller); >> + pm_runtime_get_noresume(hcd->self.sysdev); >> #endif >> >> > >Same here. > >> @@ -4834,7 +4834,7 @@ int xhci_get_frame(struct usb_hcd *hcd) int >> xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) { >> struct xhci_hcd *xhci; >> - struct device *dev = hcd->self.controller; >> + struct device *dev = hcd->self.sysdev; >> int retval; > > >This one calls > > get_quirks(dev, xhci); > >not sure whether this should be called with self.controller or self.sysdev, we should >audit every one of the callers here to be sure: > >drivers/usb/host/xhci-mtk.c: return xhci_gen_setup(hcd, xhci_mtk_quirks); >drivers/usb/host/xhci-pci.c: retval = xhci_gen_setup(hcd, xhci_pci_quirks); >drivers/usb/host/xhci-plat.c: return xhci_gen_setup(hcd, xhci_plat_quirks); >drivers/usb/host/xhci-rcar.c: * xhci_gen_setup(). >drivers/usb/host/xhci-tegra.c: return xhci_gen_setup(hcd, tegra_xhci_quirks); > > Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: sriram.dash@nxp.com (Sriram Dash) Date: Thu, 22 Sep 2016 05:02:17 +0000 Subject: [PATCH] usb: dwc3: host: inherit dma configuration from parent dev In-Reply-To: <20042182.GcPMJken6Z@wuerfel> References: <3596831.dckG5peiyF@wuerfel> <20042182.GcPMJken6Z@wuerfel> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org >From: Arnd Bergmann [mailto:arnd at arndb.de] >On Wednesday, September 21, 2016 11:43:59 AM CEST Sriram Dash wrote: >> >From: Arnd Bergmann [mailto:arnd at arndb.de] On Wednesday, September >> >21, 2016 11:06:47 AM CEST Sriram Dash wrote: >> >> ============================================================== >> From 8b0dea1513e9e73a11dcfa802ddc71cca11d66f8 Mon Sep 17 00:00:00 2001 >> From: Sriram Dash >> Date: Wed, 21 Sep 2016 11:39:30 +0530 >> Subject: [PATCH] usb: xhci: Fix the patch inherit dma configuration >> from parent dev >> >> Fixes the patch https://patchwork.kernel.org/patch/9319527/ >> ("usb: dwc3: host: inherit dma configuration from parent dev"). >> >> Signed-off-by: Sriram Dash >> --- >> drivers/usb/host/xhci-mem.c | 12 ++++++------ >> drivers/usb/host/xhci.c | 20 ++++++++++---------- >> 2 files changed, 16 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c >> index 6afe323..79608df 100644 >> --- a/drivers/usb/host/xhci-mem.c >> +++ b/drivers/usb/host/xhci-mem.c > >All the changes you did to this file seem fine, I completely missed that part. > >> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index >> 01d96c9..9a1ff09 100644 >> --- a/drivers/usb/host/xhci.c >> +++ b/drivers/usb/host/xhci.c Yes. Some sanity is required over this patch. >> @@ -231,7 +231,7 @@ static int xhci_free_msi(struct xhci_hcd *xhci) >> static int xhci_setup_msi(struct xhci_hcd *xhci) { >> int ret; >> - struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller); >> + struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.sysdev); >> >> ret = pci_enable_msi(pdev); >> if (ret) { > >This one is interesting as I stumbled over some code comment mentioning that for >dwc3-pci, we don't support MSI. I think with this change, we /can/ actually support >MSI, but this could be a separate patch and we'd have to test it on dwc3-pci >hardware. Same for most of this file. > >> @@ -745,7 +745,7 @@ void xhci_shutdown(struct usb_hcd *hcd) >> struct xhci_hcd *xhci = hcd_to_xhci(hcd); >> >> if (xhci->quirks & XHCI_SPURIOUS_REBOOT) >> - usb_disable_xhci_ports(to_pci_dev(hcd->self.controller)); >> + usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev)); >> >> spin_lock_irq(&xhci->lock); >> xhci_halt(xhci); > >This seems obviously correct, but I don't yet see why it currently works. We >probably don't call this function on dwc3. > >> #ifdef CONFIG_PM >> @@ -3605,7 +3605,7 @@ void xhci_free_dev(struct usb_hcd *hcd, struct >usb_device *udev) >> * if no devices remain. >> */ >> if (xhci->quirks & XHCI_RESET_ON_RESUME) >> - pm_runtime_put_noidle(hcd->self.controller); >> + pm_runtime_put_noidle(hcd->self.sysdev); >> #endif >> >> ret = xhci_check_args(hcd, udev, NULL, 0, true, __func__); > >I suspect this one is wrong, based on what Felipe explained earlier: >the power management should propagate down from the child to the parent >device. > >Someone who understands this better than I do should look at it. > >> @@ -3745,7 +3745,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct >usb_device *udev) >> * suspend if there is a device attached. >> */ >> if (xhci->quirks & XHCI_RESET_ON_RESUME) >> - pm_runtime_get_noresume(hcd->self.controller); >> + pm_runtime_get_noresume(hcd->self.sysdev); >> #endif >> >> > >Same here. > >> @@ -4834,7 +4834,7 @@ int xhci_get_frame(struct usb_hcd *hcd) int >> xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) { >> struct xhci_hcd *xhci; >> - struct device *dev = hcd->self.controller; >> + struct device *dev = hcd->self.sysdev; >> int retval; > > >This one calls > > get_quirks(dev, xhci); > >not sure whether this should be called with self.controller or self.sysdev, we should >audit every one of the callers here to be sure: > >drivers/usb/host/xhci-mtk.c: return xhci_gen_setup(hcd, xhci_mtk_quirks); >drivers/usb/host/xhci-pci.c: retval = xhci_gen_setup(hcd, xhci_pci_quirks); >drivers/usb/host/xhci-plat.c: return xhci_gen_setup(hcd, xhci_plat_quirks); >drivers/usb/host/xhci-rcar.c: * xhci_gen_setup(). >drivers/usb/host/xhci-tegra.c: return xhci_gen_setup(hcd, tegra_xhci_quirks); > > Arnd