Hi, Sriram Dash <sriram.dash@xxxxxxx> writes: >>From: Felipe Balbi [mailto:felipe.balbi@xxxxxxxxxxxxxxx] >> >> >>Hi, > > Hello Felipe, > >> >>Sriram Dash <sriram.dash@xxxxxxx> writes: >>> From: Arnd Bergmann <arnd@xxxxxxxx> >>> >>> 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@xxxxxxxx> >>> Signed-off-by: Sriram Dash <sriram.dash@xxxxxxx> >>> --- >>> 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
Attachment:
signature.asc
Description: PGP signature
