RE: [PATCH 7/7 v2] usb: dwc3: add the hibernation code itself

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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.

> 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.

-- 
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux