pci_bus_for_each_resource, transparent bridges and rsrc_nonstatic.c

[Bjorn Helgaas]

On Tuesday 23 March 2010 09:13:01 am Dominik Brodowski wrote:
> From: Dominik Brodowski <linux at dominikbrodowski.net>
> Date: Tue, 23 Mar 2010 16:05:00 +0100
> Subject: [PATCH] pcmcia: do not use ioports < 0x100 on x86
> 
> On x86 systems using ACPI _CRS information -- now the default for
> post-2008 systems -- the PCI root bus no longer pretends to be
> offering the root ioport_resource. To avoid accidentally hitting
> some platform / system device, use only I/O ports >= 0x100 for
> PCMCIA devices on x86.
> 
> Also, use %pR, while we're there.
> 
> Reported-by: Komuro <komurojun-mbn at nifty.com>
> CC: Bjorn Helgaas <bjorn.helgaas at hp.com>
> Signed-off-by: Dominik Brodowski <linux at dominikbrodowski.net>
> 
> diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c
> index 4663b3f..7ae5b0f 100644
> --- a/drivers/pcmcia/rsrc_nonstatic.c
> +++ b/drivers/pcmcia/rsrc_nonstatic.c
> @@ -864,13 +864,21 @@ static int nonstatic_autoadd_resources(struct pcmcia_socket *s)
>  			continue;
>  
>  		if (res->flags & IORESOURCE_IO) {
> +
> +#if defined(CONFIG_X86)
> +			/* on x86, avoid anything < 0x100 for it is often
> +			 * used for legacy platform devices
> +			 */
> +			if (res->start < 0x100)
> +				res->start = 0x100;
> +			if (res->start >= res->end)
> +				continue;
> +#endif

I'm concerned that this changes a resource that PCMCIA doesn't own,
i.e., this struct resource actually lives in the pci_dev of an upstream
bridge or in the host bridge data.

Doesn't PCMCIA maintain its own resource_map structures, and if so,
wouldn't it be cleaner to modify *those* instead, or change the
allocator to avoid these low ports?

> +
>  			if (res == &ioport_resource)
>  				continue;

If you make PCMCIA smart enough to avoid these low ports, do we still
need these &ioport_resource and &iomem_resource checks?  Having two
mechanisms will lead to more complicated behavior and more special
cases.

> -			dev_printk(KERN_INFO, &s->cb_dev->dev,
> -				   "pcmcia: parent PCI bridge I/O "
> -				   "window: 0x%llx - 0x%llx\n",
> -				   (unsigned long long)res->start,
> -				   (unsigned long long)res->end);
> +			dev_info(&s->cb_dev->dev, "pcmcia: parent PCI bridge "
> +				"window: %pR\n", res);

Jesse applied a patch from me to make this %pR change just a few days ago.

Bjorn

>  			if (!adjust_io(s, ADD_MANAGED_RESOURCE, res->start, res->end))
>  				done |= IORESOURCE_IO;
>  
> @@ -879,11 +887,8 @@ static int nonstatic_autoadd_resources(struct pcmcia_socket *s)
>  		if (res->flags & IORESOURCE_MEM) {
>  			if (res == &iomem_resource)
>  				continue;
> -			dev_printk(KERN_INFO, &s->cb_dev->dev,
> -				   "pcmcia: parent PCI bridge Memory "
> -				   "window: 0x%llx - 0x%llx\n",
> -				   (unsigned long long)res->start,
> -				   (unsigned long long)res->end);
> +			dev_info(&s->cb_dev->dev, "pcmcia: parent PCI bridge "
> +				"window: %pR\n", res);
>  			if (!adjust_memory(s, ADD_MANAGED_RESOURCE, res->start, res->end))
>  				done |= IORESOURCE_MEM;
>  		}
>