linux-kernel - RE: [PATCH v3 5/6] usb: dwc3: use bus->sysdev for DMA configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Nov 2016 12:57:53 +0200
From:   Felipe Balbi <felipe.balbi@...ux.intel.com>
To:     Sriram Dash <sriram.dash@....com>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb\@vger.kernel.org" <linux-usb@...r.kernel.org>
Cc:     "mathias.nyman\@intel.com" <mathias.nyman@...el.com>,
        "gregkh\@linuxfoundation.org" <gregkh@...uxfoundation.org>,
        Suresh Gupta <suresh.gupta@....com>,
        "stern\@rowland.harvard.edu" <stern@...land.harvard.edu>,
        "pku.leo\@gmail.com" <pku.leo@...il.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: RE: [PATCH v3 5/6] usb: dwc3: use bus->sysdev for DMA configuration


Hi,

Sriram Dash <sriram.dash@....com> writes:
>>From: Felipe Balbi [mailto:felipe.balbi@...ux.intel.com]
>>
>>
>>Hi,
>
> Hello Felipe,
>
>>
>>Sriram Dash <sriram.dash@....com> writes:
>>> From: Arnd Bergmann <arnd@...db.de>
>>>
>>> The dma ops for dwc3 devices are not set properly. So, use a physical
>>> device sysdev, which will be inherited from parent, to set the
>>> hardware / firmware parameters like dma.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@...db.de>
>>> Signed-off-by: Sriram Dash <sriram.dash@....com>
>>> ---
>>> Changes in v3:
>>>   - No update
>>>
>>> Changes in v2:
>>>   - integrate dwc3 driver changes together
>>>
>>>  drivers/usb/dwc3/core.c   | 28 +++++++++++++++-------------
>>>  drivers/usb/dwc3/core.h   |  1 +
>>>  drivers/usb/dwc3/ep0.c    |  8 ++++----
>>>  drivers/usb/dwc3/gadget.c | 37 +++++++++++++++++++------------------
>>>  drivers/usb/dwc3/host.c   | 12 ++++--------
>>>  5 files changed, 43 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
>>> 7287a76..0af0dc0 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -25,6 +25,7 @@
>>>  #include <linux/slab.h>
>>>  #include <linux/spinlock.h>
>>>  #include <linux/platform_device.h>
>>> +#include <linux/pci.h>
>>
>>I'd prefer to add a device property instead of checking for PCI bus type.
>>
>>> @@ -943,6 +944,13 @@ static int dwc3_probe(struct platform_device *pdev)
>>>  	dwc = PTR_ALIGN(mem, DWC3_ALIGN_MASK + 1);
>>>  	dwc->mem = mem;
>>>  	dwc->dev = dev;
>>> +#ifdef CONFIG_PCI
>>> +	/* TODO: or some other way of detecting this? */
>>> +	if (dwc->dev->parent && dwc->dev->parent->bus == &pci_bus_type)
>>> +		dwc->sysdev = dwc->dev->parent;
>>> +	else
>>> +#endif
>>
>>IOW:
>>
>>        dwc->sysdev_is_parent = device_property_read_bool(dev,
>>        		"linux,sysdev_is_parent");
>>
>>[...]
>>
>>	if (dwc->sysdev_is_parent)
>>        	dwc->sysdev = dwc->dev->parent;
>>	else
>>        	dwc->sysdev = dwc->dev;
>>
>
> I am with you in the fact that the core should not worry about pci.
> This change you proposed is also appealing. But, if we are going with
> this solution, all the clients which are using dwc3 pci have to 
> mention the dts property "linux,sysdev_is_parent". This will be requiring

PCI doesn't use DTS ;-) You're gonna need something like so:

1 file changed, 10 insertions(+)
drivers/usb/dwc3/dwc3-pci.c | 10 ++++++++++

modified   drivers/usb/dwc3/dwc3-pci.c
@@ -73,6 +73,16 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc)
 {
 	struct platform_device		*dwc3 = dwc->dwc3;
 	struct pci_dev			*pdev = dwc->pci;
+	int				ret;
+
+	struct property_entry sysdev_property[] = {
+		PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
+		{ },
+	};
+
+	ret = platform_device_add_properties(dwc3, sysdev_property);
+	if (ret)
+		return ret;
 
 	if (pdev->vendor == PCI_VENDOR_ID_AMD &&
 	    pdev->device == PCI_DEVICE_ID_AMD_NL_USB) {

> a lot of testing and proper changes from the dts, or it might break the
> functionality for dwc3 pci clients. So, IMO, we could postpone this change
> and try it out in future.

not taking any ifdefs, sorry.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (801 bytes)