Re: [PATCH v13 06/12] usb: xhci: use bus->sysdev for DMA configuration

From: Peter Chen
Date: Wed Feb 15 2017 - 20:23:37 EST


On Wed, Feb 15, 2017 at 01:51:31PM +0200, Felipe Balbi wrote:
>
> Hi,
>
> Roger Quadros <rogerq@xxxxxx> writes:
> >>>>>>>> Why are we using sysdev to read DT property? We should be using the
> >>>>>>>> XHCI device (&pdev->dev) here, no?
> >>>>>>>
> >>>>>>> If I remember correctly, this is one of the cases where pdev does not
> >>>>>>> have a device node attached to it because it was created by the driver
> >>>>>>> of the parent device on the fly in case of dwc3. When you have a pure xhci
> >>>>>>> device in DT, the two pointers are the same.
> >>>>>>
> >>>>>> From drivers/usb/dwc3/host.c
> >>>>>>
> >>>>>>> if (dwc->usb3_lpm_capable) {
> >>>>>>> props[0].name = "usb3-lpm-capable";
> >>>>>>> ret = platform_device_add_properties(xhci, props);
> >>>>>>> if (ret) {
> >>>>>>> dev_err(dwc->dev, "failed to add properties to xHCI\n");
> >>>>>>> goto err1;
> >>>>>>> }
> >>>>>>> }
> >>>>>>
> >>>>>> So it is setting the usb3-lpm-capable property into the xhci platform device
> >>>>>> and we should be reading the property from there.
> >>>>
> >>>> Why dwc3 needs another "snps,usb3_lpm_capable"? Why not using
> >>>> "usb3-lpm-capable" at firmware directly?
> >>>
> >>> dwc3 is not setting "snps,usb3_lpm_capable" but "usb3-lpm-capable" for the
> >>> xhci platform device.
> >>>
> >>> What did you mean by firmware? Did you mean something like BIOS?
> >>> At least TI platforms don't use any firmware like BIOS. So dwc3 driver
> >>> needs to create a platform device for xhci on the fly and set the DT properties.
> >>>
> >>
> >> By readying code, the dwc3 calls dwc3_get_properties to set
> >> dwc->usb3_lpm_capable, and at dwc3/host.c, it sets property
> >> "usb3-lpm-capable" according to this flag, why not let common
> >> code xhci-plat.c to get this property from sysdev which is DT
> >> nodes for dwc3?
> >>
> >
> > Felipe, any comments?
>
> Won't work. We have quirk flags which are based on DWC3's revision which
> is not accessible by xhci-plat. Also, we can't call
> device_add_property() because it's not really *adding*. It's *setting*,
> meaning that we would loose all other properties.
>

Sorry, I am not clear by reading the code, here we just discuss
"usb3-lpm-capable" property from DT or other firmwares.

--

Best Regards,
Peter Chen