Hi,
On Mon, Dec 02, 2013 at 04:24:31PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Thursday 28 November 2013 09:29 PM, Heikki Krogerus wrote:
<snip>
> > diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> > index dd17601..8bb82bc 100644
> > --- a/drivers/usb/dwc3/Makefile
> > +++ b/drivers/usb/dwc3/Makefile
> > @@ -13,6 +13,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) $(CONFIG_USB_DWC3_DUAL_ROLE)),)
> > dwc3-y += gadget.o ep0.o
> > endif
> >
> > +ifneq ($(CONFIG_USB_DWC3_ULPI),)
> > + dwc3-y += ulpi.o
> > +endif
> > +
> > ifneq ($(CONFIG_DEBUG_FS),)
> > dwc3-y += debugfs.o
> > endif
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 1c0a69a..94927b2 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -505,6 +505,12 @@ static int dwc3_probe(struct platform_device *pdev)
> > dwc->regs_size = resource_size(res);
> > dwc->dev = dev;
> >
> > + if (!dwc->usb2_generic_phy) {
> > + ret = dwc3_ulpi_init(dwc);
>
> shouldn't this be called from dwc3-pci (or any other dwc3 glue)?
It's not going to be possible to put it to the dwc3-pci, but my
motivation for that is actually up and coming ACPI support. I do not
want to create glue driver for it just because this. With ACPI DSDT
there is no guarantee to get device entries for PHYs I'm afraid. You
will have them on some platforms, but on others you don't :(.
The issue with PCI is the device name, which is used when matching the
phy. We don't know it before the device is registered. We can't live
with the assumption there is only one instance in case of PCI. The
damn thing can be hotpluged from some weird dock, like exchangeable
back cover or something similar. So you will be able to register the
phy only after the core.c has probed, and requested the phys, which is
too late.
And besides. The glue drivers should only have platform specific code.
This is definitely platform agnostic feature of the controller.
> > + if (ret)
> > + return ret;
> > + }
> > +
> > dev->dma_mask = dev->parent->dma_mask;
> > dev->dma_parms = dev->parent->dma_parms;
> > dma_set_coherent_mask(dev, dev->parent->coherent_dma_mask);
<snip>
> > +{
> > + int ret;
> > +
> > + /* First check if there is ULPI PHY */
> > + ret = dwc3_readl(dwc->regs, DWC3_GHWPARAMS3);
> > + if (!(ret & DWC3_ULPI_HSPHY))
> > + return 0;
> > +
> > + /* Register the interface */
> > + dwc->ulpi = ulpi_new_interface(dwc->dev,
> > + dwc3_ulpi_read, dwc3_ulpi_write, dwc);
> > + if (IS_ERR(dwc->ulpi)) {
> > + dev_err(dwc->dev, "failed to register ULPI interface");
> > + return PTR_ERR(dwc->ulpi);
> > + }
> > +
> > + /* Get the ULPI phy instance
> > + * REVISIT: There should be no need to get it separately here.
> > + */
> > + dwc->usb2_generic_phy = devm_phy_get(dwc->dev, ULPI_PORT_NAME);
>
> You shouldn't be needing this. It should get the phy from dwc3 core probe itself.
You don't need this here. I made it like this now just because we
don't yet have your final version of the patches where you introduce
the generic phy support in dwc3.
I want to change the core.c so it set's the dwc->dev and maps the
iomem before requesting the phys. And of course put dwc3_ulpi_init in
between.
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
