Hi,
On Sun, Apr 15, 2012 at 04:40:24AM -0700, Ido Shayevitz wrote:
>
> > This will prevent dwc3.ko from ioremapping
> > the entire memory space, thus preventing xHCI
> > from requesting its own part of the address.
> >
> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > ---
> > drivers/usb/dwc3/core.c | 26 +++++++---
> > drivers/usb/dwc3/core.h | 122
> > +++++++++++++++++++++++------------------------
> > 2 files changed, 80 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index e7c853a..3c69d1a 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 resource host_resource[2];
> > struct resource *res;
> > struct dwc3 *dwc;
> > struct device *dev = &pdev->dev;
> > @@ -431,7 +432,24 @@ static int __devinit dwc3_probe(struct
> > platform_device *pdev)
> > return -ENODEV;
> > }
> >
> > - dwc->res = res;
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + dev_err(dev, "missing IRQ\n");
> > + return -ENODEV;
> > + }
> > +
> > + host_resource[0].start = res->start;
> > + host_resource[0].end = res->start + 0x7fff;
> > + host_resource[0].flags = IORESOURCE_MEM;
>
> You set the memory end region to end after 0x7fff, but later you assign it
> to dwc->res , meaning that this is memory for all the dwc3 core and not
> just for the host part.
no dude I guess you misunderstood.
start + 0x7fff is the end of the xHCI address space. dwc->res gets
passed to xHCI driver on host.c
> > +
> > + host_resource[1].start = irq;
> > + host_resource[1].end = irq;
> > + host_resource[1].flags = IORESOURCE_IRQ;
>
> Adding the irq to the dwc->res is nice to have but I don't think it is
> related to this patch and I don't see where you use it. Also it seems
> redundant since of dwc->irq.
xHCI uses it, dwc->res is the resource which will be passed to xHCI
driver.
> > +
> > + dwc->res = host_resource;
>
> I don't like the name host_resource, this array is the resources of all
> the dwc3 not just the host part. Also seems that you set a pointer to
> local scoped array, isn't this pointer will be invalid after probe will
> finish ?
host_resource is only the part of the address which contains the xHCI
address space, nothing more.
> So, instead, maybe create a local array used for non xhci resource:
> struct resource mem_res_exclude_xhci[1];
> ...
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> ...
> mem_res_exclude_xhci[0].start = res->start + 0xc100;
> mem_res_exclude_xhci[0].end = res->end;
> mem_res_exclude_xhci[0].flags = res.flags;
and at which point do we let xHCI know what address it should request
and ioremap ?
--
balbi
Attachment:
signature.asc
Description: Digital signature
