RE: [PATCH] usb: dwc3: core: modify IO memory resource afterdeferred probe completes

From: Paul Zimmerman
Date: Fri Jul 26 2013 - 18:02:20 EST


> From: Felipe Balbi [mailto:balbi@xxxxxx]
> Sent: Friday, July 26, 2013 1:32 PM
>
> On Fri, Jul 26, 2013 at 06:44:23PM +0000, Paul Zimmerman wrote:
> > > From: Felipe Balbi [mailto:balbi@xxxxxx]
> > > Sent: Friday, July 26, 2013 2:54 AM
> > > > > > Also, this is not *modifying* what was passed, just skipping the xHCI
> > > > > > address space so we don't request_mem_region() an area we won't really
> > > > > > handle and prevent xhci-hcd.ko from probing.
> > > > >
> > > > > Hmm? platform_get_resource() returns a pointer to an entry in the
> > > > > platform_device's resource[] array. And "res->start +=" modifies the
> > > > > entry pointed at. If it didn't, the bug fixed by this patch wouldn't
> > > > > have happened.
> > > > >
> > > > > Are you sure this code will work OK if you build the driver as a module,
> > > > > modprobe it, rmmod it, and then modprobe it again? Seems like it won't,
> > > > > unless the dev->resource[] array gets reinitialized in between somehow.
> > >
> > > gotta try that one... Perhaps the correct way would be to copy the
> > > resource to a private struct resource and modify that one, leaving
> > > pdev->resources untouched.
> >
> > Maybe this is a dumb question, but why can't the driver that is going
> > to use the resource after this just "know" that it has to add
> > DWC3_GLOBALS_REGS_START to the start address? Are there some versions
> > of the core where that is not the case?
>
> that won't work, because dwc3.ko will already have request_mem_region()
> the entire region and a subsequent request_mem_region() for xHCI space
> only would fail.
>
> > Or, maybe there should be two sets of resources?
>
> maybe we should require two sets of resources, yes... but then there's
> no point in having any host initialization whatsoever in dwc3.ko.

Right. For a platform with a DWC3 controller, have DT or whatever set up
a second memory resource in the platform device, and have xhci_plat_probe()
look for that one first. If it finds it, it uses that resource instead
of the first one. If it doesn't find the second resource (not a DWC3
platform) then it uses the first one like it does today.

Then you don't need to have dwc3 set up the xhci resource like it does
today. Seems cleaner to me. 'Course it's incompatible with what you have
today, I guess that's the major drawback.

--
Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/