Re: [patch 01/15] serial: when guessing, check only active resources, not options

[Rene Herman <rene.herman@keyaccess.nl>]

On 31-05-08 00:48, Bjorn Helgaas wrote:

> Given a completely unknown PNP device, if it happens to have
> a modem-like string in its name and it matches a COM port
> address, we assume it's a modem.
> 
> We used to check the address against all the possible resource
> options for the device.  But the device is already configured
> and enabled, so we only need to check the resources it is
> actually using.  If we matched an address that wasn't currently
> enabled, we would fail anyway as soon as we attempted to touch
> the device at that address.
> 
> This removes a reference to the struct pnp_dev.{independent,dependent}
> fields, which I will soon make private to the PNP subsystem.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

This isn't quite right it seems. Or changes behaviour at least:

> Index: work10/drivers/serial/8250_pnp.c
> ===================================================================
> --- work10.orig/drivers/serial/8250_pnp.c	2008-05-13 11:28:48.000000000 -0600
> +++ work10/drivers/serial/8250_pnp.c	2008-05-13 11:29:16.000000000 -0600
> @@ -383,22 +383,19 @@ static int __devinit check_name(char *na
>  	return 0;
>  }
>  
> -static int __devinit check_resources(struct pnp_option *option)
> +static int __devinit check_resources(struct pnp_dev *dev)
>  {
> -	struct pnp_option *tmp;
> -	if (!option)
> +	resource_size_t port, size;
> +
> +	if (!pnp_port_valid(dev, 0))
>  		return 0;
>  
> -	for (tmp = option; tmp; tmp = tmp->next) {
> -		struct pnp_port *port;
> -		for (port = tmp->port; port; port = port->next)
> -			if ((port->size == 8) &&
> -			    ((port->min == 0x2f8) ||
> -			     (port->min == 0x3f8) ||
> -			     (port->min == 0x2e8) ||
> -			     (port->min == 0x3e8)))
> -				return 1;
> -	}
> +	port = pnp_port_start(dev, 0);
> +	size = pnp_port_len(dev, 0);
> +
> +	if (size == 8 &&
> +	    (port == 0x2f8 || port == 0x3f8 || port == 0x2e8 || port == 0x3e8))
> +		return 1;
>  
>  	return 0;
>  }
> @@ -420,10 +417,7 @@ static int __devinit serial_pnp_guess_bo
>  		(dev->card && check_name(dev->card->name))))
>  			return -ENODEV;
>  
> -	if (check_resources(dev->independent))
> -		return 0;
> -
> -	if (check_resources(dev->dependent))
> +	if (check_resources(dev))
>  		return 0;
>  
>  	return -ENODEV;

We used to cry "modem" if the device _could_ be configured using a COM 
address while after this change we'd only do so if it _was_ configured 
using one.

My old analog modem for example had 5 dependent sets -- the first with 
the regular COM port addresses (0x3f8, 0x2f8, 0x3e8, 0x2e8) and a 5th 
one listing a port range of (IIRC, something like that at least) 0x100 
to 0xff8.

So say I'd have a PC with the regular two onboard COM ports at COM1 and 
COM2 (0x3f8 and 0x2f8) and an additional serial controller providing 
COM3 and COM4 (0x3e8 and 0x2e8). My modem would then be configured to 
use 0x100 and this code would no longer trigger while it did use to.

Not that I care deeply or anything but whomever wrote this did no doubt 
intend it to work this way...

Rene.