Re: pci_bus_for_each_resource, transparent bridges and rsrc_nonstatic.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday 23 March 2010 09:13:01 am Dominik Brodowski wrote:
> From: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx>
> 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@xxxxxxxxx>
> CC: Bjorn Helgaas <bjorn.helgaas@xxxxxx>
> Signed-off-by: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx>
> 
> 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;
>  		}
> 


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

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux