RE: Serial.c BUG 2.4.x-2.5x

[Ed Vance <EdV@macrolink.com>]

On Thu Mar 07, 2002, Roman Kurakin wrote:
> 
> On Wed Mar 06, 2002, Russell King wrote:
> >
> >The patch does fine for the most part, but I have two worries:
> >
> >1. the possibilities of pushing through changes in the IO or memory space
> >   by changing the other space at the same time. (ie, port = 1, iomem =
> >   0xfe007c00 and you already have a line at port = 0, iomem =
0xfe007c00).
> >   I dealt with this properly using the resource management subsystem.
> >
> I think such code could solve this problem ...
> 
> - 	    (rs_table[i].port == new_port) &&
> + 	    ((rs_table[i].port && rs_table[i].port == new_port) ||
> +	    ((rs_table[i].iomem_base && rs_table[i].iomem_base == new_mem))
&&

Indeed it would solve this problem, but I'm not sure there is a problem to
solve here. Have not found a case where ->port and ->iomem_base fields can
both be non-zero. If one of them is always zero then the previous patch hunk
in the "address in use" test at about line 2146 is well enough:

            if ((state != &rs_table[i]) &&
                (rs_table[i].port == new_port) &&
+               (rs_table[i].iomem_base == new_mem) &&
                rs_table[i].type)
                    return -EADDRINUSE;

Assuming one of the two fields is always zero, demanding both to match for
the in use condition works anyway. If the non-zero field matches, then they
both must match. The following hunk at the bottom of function get_pci_port()
at about line 3931 seems to guarantee that they start out this way:

	[[ The req struct is memset() to zero at about line 4009 in 
	  function start_pci_pnp_board(). ]]

        if (IS_PCI_REGION_IOPORT(dev, base_idx)) {
                req->port = port;
                if (HIGH_BITS_OFFSET)
                        req->port_high = port >> HIGH_BITS_OFFSET;
                else
                        req->port_high = 0;
                return 0;
        }
        req->io_type = SERIAL_IO_MEM;
        req->iomem_base = ioremap(port, board->uart_offset);
        req->iomem_reg_shift = board->reg_shift;
        req->port = 0;
        return 0;

Does anybody see a need to add the code anyway? Did I miss a lurker? 

Best to all,

---------------------------------------------------------------- 
Ed Vance              serial24@macrolink.com
Macrolink, Inc.       1500 N. Kellogg Dr  Anaheim, CA  92807
----------------------------------------------------------------