> From: Felipe Balbi [mailto:balbi@xxxxxx] > Sent: Wednesday, February 20, 2013 12:50 AM > > On Tue, Feb 19, 2013 at 06:50:07PM -0800, Paul Zimmerman wrote: > > This file contains the PCI bus interface "glue" for the DWC2 > > driver > > > > Signed-off-by: Paul Zimmerman <paulz@xxxxxxxxxxxx> > > before anything, out of curiosity, do you have DWC2 PCI configured with > OTG support ? I remember that on HAPS6x configuring DWC3 PCI OTG would > take too much space on the FPGA and that would cause difficulties on > timing closure, but perhaps dwc2 is smaller ?!? Yes, our standard HSOTG FPGA image is configured with OTG support. I'm not sure how difficult it is to synthesize, that work is done by a team on another continent ;) > > +static void dwc2_driver_remove(struct pci_dev *dev) > > +{ > > + struct dwc2_device *otg_dev = pci_get_drvdata(dev); > > + > > + dev_dbg(&dev->dev, "%s(%p)\n", __func__, dev); > > + > > + if (!otg_dev) { > > + dev_dbg(&dev->dev, "%s: otg_dev NULL!\n", __func__); > > + return; > > + } > > this check can be removed, drivers core guarantees that pci_dev *dev > will always be true and, since you clearly and unconditionally call > pci_set_drvdata(), otg_dev will always be valid. This function is called as part of the failure path from dwc2_driver_probe(), and it's possible that pci_set_drvdata() has not been called yet. > > + > > + dev_dbg(&dev->dev, "otg_dev->hsotg = %p\n", otg_dev->hsotg); > > + if (otg_dev->hsotg) > > + dwc2_hcd_remove(&dev->dev, otg_dev); > > + else > > + dev_dbg(&dev->dev, "%s: otg_dev->hsotg NULL!\n", __func__); > > this check can be removed I guess. You shouldn't be able to remove what > wasn't registered. Ditto. > > + /* Map the DWC_otg Core memory into virtual address space */ > > + dev->current_state = PCI_D0; > > + dev->dev.power.power_state = PMSG_ON; > > no no no... you shouldn't access these directly. > > Plese use pci_set_state(dev, PCI_D0) and > dev_pm_qos_constraints_init(&dev->dev); I will switch to pci_set_power_state(dev, PCI_D0), I guess that's what you meant? I can't get dev_pm_qos_constraints_init() to compile, so I think I'll leave that out for now. I don't have any PM support in the driver yet, anyway. For the rest of your comments I will implement the changes that you suggested. Thanks. -- Paul -- 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
