On Fri, Feb 24, 2012 at 11:16:00AM -0800, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > Sent: Friday, February 24, 2012 1:58 AM > > > > On Thu, Feb 23, 2012 at 01:17:13PM -0800, Paul Zimmerman wrote: > > > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > > > Sent: Thursday, February 23, 2012 2:26 AM > > > > > > > > On Wed, Feb 15, 2012 at 06:57:02PM -0800, Paul Zimmerman wrote: > > > > > @@ -56,7 +55,7 @@ struct dwc3_pci { > > > > > static int __devinit dwc3_pci_probe(struct pci_dev *pci, > > > > > const struct pci_device_id *id) > > > > > { > > > > > - struct resource res[2]; > > > > > + struct resource res[3]; > > > > > struct platform_device *dwc3; > > > > > struct dwc3_pci *glue; > > > > > int ret = -ENOMEM; > > > > > @@ -102,6 +101,16 @@ static int __devinit dwc3_pci_probe(struct pci_dev *pci, > > > > > res[1].name = "dwc_usb3"; > > > > > res[1].flags = IORESOURCE_IRQ; > > > > > > > > > > + if (pci->vendor == PCI_VENDOR_ID_SYNOPSYS && > > > > > + pci->device == PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3) { > > > > > + /* Add dummy resource to tell dwc3 driver this is HAPS */ > > > > > + dev_vdbg(&pci->dev, "adding dummy resource for HAPS\n"); > > > > > + res[2].start = 0; > > > > > + res[2].end = 1; > > > > > + res[2].name = "dwc_usb3_haps"; > > > > > + res[2].flags = IORESOURCE_MEM; > > > > > + } > > > > > > > > instead, you need to add a dev_pm_ops on this file too (as I said on the > > > > last mail), then you can move this check to dwc3-pci.c's > > > > runtime_resume/suspend and call haps specific stuff from there. Just > > > > make a big code comment stating that HAPS is a special situation because > > > > it's non-standard FPGA-only platform. > > > > > > > > This will also let you remove the "is_haps" flag from core.c which > > > > doesn't make much sense anyway. If I let "is_haps" go in, soon we will > > > > have > > > > is_my_perfect_special_architecture_which_is_better_then_the_rest_of_you > > > > for every single user and that makes frightens me quite a lot ;-) > > > > > > Unfortunately, I don't see a way to make that work. The way the > > > interface between the modules is designed, dwc3-pci does not have > > > access to the DWC3 registers. Plus, dwc3-pci does not have access to > > > the 'struct dwc3' pointer either, so it has no way to see the software > > > state of the dwc3 driver. > > > > > > We could redesign that, so the register space is mapped and the > > > 'struct dwc3' is allocated in the glue layer instead of in dwc3. > > > Would you be up for that? If so, I could prepare an RFC patch for > > > that. > > > > no no, that'll cause problems when removing modules. If you remove > > dwc3-pci.ko before dwc3.ko, the latter could try to access already freed > > memory. > > No, I was thinking that dwc3 would export an init() and exit() function. > When dwc3-pci was removed, it would call the exit() function, and after > that dwc3 would no longer be active. That would look like a violation of the driver core. Well, it would look a lot like MUSB and that frightens me to death ;-) > > You could split the PCI address space into DWC USB3 IP specific and the > > rest (which is HAPS specific), unless you're mapping your HAPS-specific > > part on an unused area of the DWC USB3 IP address space, then, well, we > > need to think how to handle it. > > I played around with that, but it was really ugly, and there were still > problems because 'struct dwc3' was not accessible from dwc3-pci. > > Let me cook up an RFC patch for the modifications I have in mind. Code > talks, after all :) It might take me a few days, I have some higher- > priority stuff going on right now. Sure, go ahead ;-) I'll go over the other patches you sent meanwhile. -- balbi
Attachment:
signature.asc
Description: Digital signature
