Duplicated allocation in AU1xxx OHCI driver

Duplicated allocation in AU1xxx OHCI driver

[Karl Lessard]

[-- Attachment #1: Type: text/plain, Size: 388 bytes --]

Hello,

I was looking at the code of the new ohci-au1xxx, and I've figured out 
that operationnal regiters resource
is allocated two times: once when registering the OHCI platform device 
(check in drivers/base/platform.c),
and once in OHCI driver probe.
Is that ok?? I'm kind of surprised that the second allocation doesn't 
failed. Removing it seems to works
well for me.

Thanks,
Karl

[-- Attachment #2: ohci-au1xxx.patch --]
[-- Type: text/plain, Size: 1036 bytes --]

--- linux-mips/drivers/usb/host/ohci-au1xxx.c	Sun Oct 10 13:56:25 2004
+++ linux/drivers/usb/host/ohci-au1xxx.c	Thu Oct 14 15:39:11 2004
@@ -91,13 +91,6 @@ int usb_hcd_au1xxx_probe (const struct h
 	struct usb_hcd *hcd = 0;
 
 	unsigned int *addr = NULL;
-
-	if (!request_mem_region(dev->resource[0].start,
-				dev->resource[0].end
-				- dev->resource[0].start + 1, hcd_name)) {
-		pr_debug("request_mem_region failed");
-		return -EBUSY;
-	}
 	
 	au1xxx_start_hc(dev);
 	
@@ -173,9 +166,6 @@ int usb_hcd_au1xxx_probe (const struct h
 		driver->hcd_free(hcd);
  err1:
 	au1xxx_stop_hc(dev);
-	release_mem_region(dev->resource[0].start,
-				dev->resource[0].end
-			   - dev->resource[0].start + 1);
 	return retval;
 }
 
@@ -219,9 +209,6 @@ void usb_hcd_au1xxx_remove (struct usb_h
 	hcd->driver->hcd_free (hcd);
 
 	au1xxx_stop_hc(dev);
-	release_mem_region(dev->resource[0].start,
-			   dev->resource[0].end
-			   - dev->resource[0].start + 1);
 }
 
 /*-------------------------------------------------------------------------*/

Re: Duplicated allocation in AU1xxx OHCI driver

[Matt Porter]

On Thu, Oct 14, 2004 at 03:45:39PM -0400, Karl Lessard wrote:
> Hello,
> 
> I was looking at the code of the new ohci-au1xxx, and I've figured out 
> that operationnal regiters resource
> is allocated two times: once when registering the OHCI platform device 
> (check in drivers/base/platform.c),
> and once in OHCI driver probe.
> Is that ok?? I'm kind of surprised that the second allocation doesn't 
> failed. Removing it seems to works
> well for me.

These are two different calls. request_resource() versus
request_region() (this is wrappered by request_mem_region()).

The request_resource() call is used by the bus driver to show
allocatable resources.

The request_region() call allocates new busy regions and checks
that the requested regions are not already busy.

The request_mem_region() call is correct and does not fail because
no other call has requested the same region.

If you look at a complex /proc/iomem like on a PCI-equipped system
with many devices, you can see how the bus driver shows allocatable
resources and then some drivers have claimed portions of those
resources as regions (because they are tagged with a driver name).
Here's a dump from such a system:

...
cde00000-cfefffff : PCI Bus #01
  ce000000-ceffffff : 0000:01:00.0
    ce000000-ceffffff : nvidia
cffdd000-cffddfff : 0000:00:03.0
  cffdd000-cffddfff : sis900
cfffdf00-cfffdfff : 0000:00:13.2
  cfffdf00-cfffdfff : ehci_hcd
cfffe000-cfffefff : 0000:00:02.2
  cfffe000-cfffefff : ohci_hcd
cffff000-cfffffff : 0000:00:02.3
  cffff000-cfffffff : ohci_hcd
...

Note that the platform bus code requests the bus allocatable resources
in generic code, whereas for something like PCI it's done in
arch-specific pci code since each arch requires a lot of fixup to
those resources due to the nature of PCI. The platform bus code
doesn't have any inherent knowledge of bus resources beyond what
it gains from a platform_add_device() call.

-Matt