> From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Felipe Balbi > Sent: Monday, February 27, 2012 12:14 AM > > 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: > > > > > > > > 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. Hi Felipe, I think I found a less invasive way to do what I need, by defining a platform_data struct for the dwc3 device, so that it can share function pointers and the dwc3 context with the bus glue. Below is a POC patch to show roughly what I plan to do. Is this an acceptable use of platform data? If so, maybe we could also remove the dwc3_{get|put}_device_id functions and just have dwc3_probe write the devid into the platform_data? Then we could get rid of those two exported functions. diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 6c7945b..3f5d0dc 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -529,6 +529,18 @@ struct dwc3_request { unsigned queued:1; }; +/* + * Platform data (used for hibernation control) + */ +struct dwc3_pdata { + /* set by dwc3 module */ + struct dwc3 *dwc; + + /* set by bus glue module */ + void (*power_control)(struct dwc3 *, int); + int (*interrupt_hook)(struct dwc3 *); +}; + /** * struct dwc3 - representation of our controller * @ctrl_req: usb control request which is used for ep0 @@ -584,6 +596,7 @@ struct dwc3 { struct platform_device *xhci; struct resource *res; + struct dwc3_pdata *pdata; struct dwc3_event_buffer **ev_buffs; struct dwc3_ep *eps[DWC3_ENDPOINTS_NUM]; diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c index a81b30e..e647570 100644 --- a/drivers/usb/dwc3/dwc3-pci.c +++ b/drivers/usb/dwc3/dwc3-pci.c @@ -51,8 +51,38 @@ struct dwc3_pci { struct device *dev; struct platform_device *dwc3; + struct dwc3_pdata pdata; }; +/** + * dwc3_haps_power_ctl - platform-specific power control routine, for HAPS board + * + * @dwc: pointer to our controller context structure + * @on: 1 to turn the power on, 0 to turn it off + */ +static void dwc3_haps_power_ctl(struct dwc3 *dwc, int on) +{ + /* ... */ +} + +/** + * dwc3_haps_intr_hook - platform-specific interrupt handling, for HAPS board + * + * If this routine returns false, the caller should handle the interrupt + * as usual. + * + * If this routine returns true, the caller should exit the interrupt handler + * immediately, without touching the dwc3 registers. + * + * @dwc: pointer to our controller context structure + */ +static int dwc3_haps_intr_hook(struct dwc3 *dwc) +{ + /* ... */ + + return 0; +} + static int __devinit dwc3_pci_probe(struct pci_dev *pci, const struct pci_device_id *id) { @@ -109,12 +139,16 @@ static int __devinit dwc3_pci_probe(struct pci_dev *pci, goto err2; } + glue->pdata.power_control = dwc3_haps_power_ctl; + glue->pdata.interrupt_hook = dwc3_haps_intr_hook; + pci_set_drvdata(pci, glue); dma_set_coherent_mask(&dwc3->dev, dev->coherent_dma_mask); dwc3->dev.dma_mask = dev->dma_mask; dwc3->dev.dma_parms = dev->dma_parms; + dwc3->dev.platform_data = &glue->pdata; dwc3->dev.parent = dev; glue->dwc3 = dwc3; diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index fd6917b..ad8529a 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -405,6 +405,7 @@ static void dwc3_core_exit(struct dwc3 *dwc) static int __devinit dwc3_probe(struct platform_device *pdev) { struct device_node *node = pdev->dev.of_node; + struct dwc3_pdata *pdata = pdev->dev.platform_data; struct resource *res; struct dwc3 *dwc; struct device *dev = &pdev->dev; @@ -459,6 +460,10 @@ static int __devinit dwc3_probe(struct platform_device *pdev) dwc->regs_size = resource_size(res); dwc->dev = dev; dwc->irq = irq; + dwc->pdata = pdata; + + if (pdata) + pdata->dwc = dwc; if (!strncmp("super", maximum_speed, 5)) dwc->maximum_speed = DWC3_DCFG_SUPERSPEED; -- 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
