This series of patches does some PNP housecleaning and
consolidation.

PNP currently uses a fixed-size table (pnp_resource_table)
to track the IO, MMIO, IRQ, and DMA resources used by a
device.  Some motherboard devices have many resources, so
we've been plagued by table overflows and we've had to
drastically increase the table size, which wastes a lot
of memory.

The end goal is to replace that fixed-size table with something
more dynamic.  These patches don't go that far, but they do make
pnp_resource_table private to the PNP core and centralize all
references to it in a small set of shared functions.

In addition, this series contains a number of related
cleanups, like centralized allocation of struct pnp_dev,
conversion to dev_printk when possible, removing many
PNP core internal functions from the public interface,
and alignment of the ISAPNP, PNPBIOS, and PNPACPI backends.

Changes between first post and v2:

  - export pnp_get_resource()
  - fix EISA ID conversion and make a common function for ISAPNP/PNPBIOS
  - fix typos in pnp_check_{port,mem,etc} that made resource assign fail

  - the following fixes should precede this series (they're in -mm already):
	- parport_pc: wrap PNP probe code in CONFIG_PNP
	- radio-cadet: wrap PNP probe code in CONFIG_PNP
	- smsc-ircc2: wrap PNP probe code in CONFIG_PNP
	- nsc-ircc: wrap PNP probe code in CONFIG_PNP

Thanks to Rene Herman for finding and fixing the EISA ID and resource
check bugs.

Bjorn
-- 
--

Quite a series...

[patch 01/37] ISAPNP: move config register addresses out of isapnp.h

   Acked-By: Rene Herman <rene.herman@gmail.com>

[patch 02/37] PNPACPI: continue after _CRS and _PRS errors

   No opinion.

[patch 03/37] PNP: make pnp_add_id() internal to PNP core

   Acked-By: Rene Herman <rene.herman@gmail.com>

[patch 04/37] PNP: change pnp_add_id() to allocate its own pnp_id structures

   Acked-By: Rene Herman <rene.herman@gmail.com>

[patch 05/37] PNP: add pnp_eisa_id_to_string()

   +void pnp_eisa_id_to_string(u32 id, char *str)
   +{
   +       id = be32_to_cpu(id);
   +       str[0] = '@' + ((id >> 26) & 0x1f);
   +       str[1] = '@' + ((id >> 21) & 0x1f);
   +       str[2] = '@' + ((id >> 16) & 0x1f);
   +       str[3] = hex_asc((id >> 12) & 0xf);
   +       str[4] = hex_asc((id >>  8) & 0xf);
   +       str[5] = hex_asc((id >>  4) & 0xf);
   +       str[6] = hex_asc((id >>  0) & 0xf);
   +       str[7] = '\0';
   +}

   I'd much prefer 'A' - 1 over '@'. While no doubt not a practical issue,
   it's more portable that way and more importantly, clearer.

   By the way, the original isapnp_parse_id explicitly encodes the top _6_
   bits in str[0] (& 0x3f) which seems odd. Bit 31 had better be 0 indeed,
   but I wonder why the original didn't just assume such.

   Other than that,

   Acked-By: Rene Herman <rene.herman@gmail.com>

[patch 06/37] PNP: add pnp_alloc_dev()

   Acked-By: Rene Herman <rene.herman@gmail.com>

[patch 07/37] PNP: make pnp_add_card_id() internal to PNP core

   Acked-By: Rene Herman <rene.herman@gmail.com>

[patch 08/37] PNP: change pnp_add_card_id() to allocate its own pnp_id 
structures

   Same problem with hexadecimal as before. Bisection would get a bogus
   card id here, but fixed in 09/37.

   Acked-By: Rene Herman <rene.herman@gmail.com>

[patch 09/37] ISAPNP: pull pnp_add_card_id() out of i...
[ message continues ]

Thanks again for your detailed review and for fixing my ISAPNP

I copied that from acpi_ex_eisa_id_to_string(), but I agree that

Yes, I wonder about that, too.  Including that bit would mean that
the first character of PNP IDs could include characters at offsets
0x20-0x3f, i.e., "`a..z{|}~" and DEL.  I poked around and found
some IDs that seem to depend on that, e.g., "nEC8241" in the 8250_pnp
serial driver.

I changed this to include six bits for the first character, and
masked off the top bit in PNPBIOS.  I think that should preserve

Thanks for pointing that out.  I changed it to use hex_asc() so bisection



One would expect that "mem.write_protect == 1" would mean read-only.
Unfortunately, I'm too lazy to un-obfuscate the ACPI CA logic that
deals with mem.write_protect, since it seems to be all table-driven.
In the absence of understanding, I tried to preserve the existing
behavior.  I think I did, i.e., if "write_protect == ACPI_READ_WRITE_MEMORY",
we add in IORESOURCE_MEM_WRITEABLE, otherwise do nothing.  If I
goofed that up, let me know.

I have an ISA question here, too: previously isapnp_read_resources()
set only res.start for IO and MMIO resources and left res.end unset
(should be zero, I think).  I don't think ISA tells you the size, so
I assumed "1", but I don't know if that's the right thing to do.  My
reasoning was "zero is obviously wrong, two could be too big and

No good reason; I think I was just being lazy and making the code
shorter.  I changed them all to static inlines.  (After making this
change, I did trip over a use of pnp_mem_flags() as an lvalue, so I
added a patch that removed that usage.)

I also made the _len() functions inlines and restructured the logic in
both _len() and _valid() functions.  I couldn't stand the thought of all
those extra list traversals in there :-)  I'd appreciate a double-check

Fixed, thanks.

I'll add in a couple more patches to (finally) remove the fixed-size
pnp_resource_table and post a v3 in a day or two.

...
[ message continues ]

Yes, it should. I checked the ISAPnP specification and it explicitly fixes 
bit 31 at 0 (and defines the "compressed ASCII" as 5 bits). Given what you 
describe you probably don't have a good place to stash a comment but with 6 
bits being non-spec something like "appease broken ISAPnP hardware" would 


Yes, as far as I'm aware the actual value is of no consequence. The size is 
not a setable parameter; to hardware they're only base address registers, It 
used to be kept simply at -1 (in an unsigned sort of way) and as far as I'm 
aware, we're also not interested yet at this level.

However, now that you made me look closer and in context -- there's actually 
a possibly somewhat serious problem here.

isapnp_read_resources() stores the resources as read from the hardware at 
the index in the table that matches the actual index in the hardware and 
isapnp_set_resources() stores them back into those same hardware indices.

Now by using pnp_add_foo_resource() which just scans for the first _UNSET 
resource, the resources might not end up in the same linear position in 
table/list if intermediate resources were unset in hardware (!ret). A 
subsequent isapnp_set_resources() would them restore the value to the wrong 
hardware index.

The IORESOURCE_ flags currently reserve too few bits (IORESOURCE_BITS,  8) 
to be able to store the hardware index: IORESOURCE_MEM and IORESOURCE_DMA 
need 2 and 1 respectively and there are 1 and 0 available respectively. It's 
ofcourse possible to hijack a few more bits in IORESOURCE_ flags but you're 
turning this into a list. I suppose the idea is to make it a simple list of 
struct resource, but perhaps a resource-private "driver_data" sort of field 
comes in handy for more than this already? Swiping more of IORESOURCE_ is a 
bit ugly...

In any case, I missed this, but ISAPnP is still (at least in principle) 

Will do.

Rene.
--

I want to understand this better.  I think the case we're concerned
about is this:

  Memory descriptor 0 is not assigned, i.e., its base and limit/range
  registers starting at 0x40 contain zeroes, but Descriptor 1, starting
  at 0x48, *is* assigned.

The 2.6.25 "get_resources" code doesn't touch the resource table for
Descriptor 0, so its entry remains "unset".  The "set_resources" code
skips Descriptor 0 because its resource table entry is "unset" and
writes Descriptor 1.

When I convert the table to a list, I have to make sure that we write
the Descriptor 1 resources to the correct place starting at 0x48, not
to the Descriptor 0 registers.  To do this, I made "get_resources" set
the pnp_resource.index field to the current descriptor index, and
"set_resources" uses pnp_resource.index to compute the register address.

However, PNPBIOS, PNPACPI, and even ISAPNP Resource Data is all based
on the ordinal position in list (see the fourth paragraph of section
4.6.1 of the ISA spec).  Having pnp_resource.index in addition to a
list position adds a lot of confusion.

I think a better solution would be to get rid of pnp_resource.index
and have "get_resources" add a "disabled" resource for Descriptor 0,
so the Nth MEM resource in the list would always correspond to the
Nth Memory Descriptor register.

Does this make sense?

Bjorn
--

I agree. Got confused/uneasy about the difference myself looking at the 

It does. Ofcourse, you can than also not reuse _UNSET resources as you did 
previously but that's for the best anyway.

In trying to come up with problems I'm only finding a difference in an added 
failure mode with respect to the static array if we run out of memory at a 
bad time and this is quite unserious.

Yes, I'd say to just do that. It might appear a bit clumsy from an 
implementation standpoint but the only thing this stuff should be doing is 
enable inane amounts of possible resources for one device without forcing 
them on all.

Rene
--

I mean, this would be an added mode when just releasing and rbuilding teh 
list as opposed to teh reusing, it's not a diffeerence with respect to the 
.index setup.

So, going to completely slash struct pnp_resource again? :) Would improve 
things...

Rene.
--

It would be nice to get rid of struct pnp_resource, but then I
wouldn't have a place to keep the list pointer.  But I think
it will disappear from most of the interfaces, e.g,. we can
get rid of pnp_get_pnp_resource(), etc.

Bjorn
--

Hmm...  you're right.  And I think it could bite PNPBIOS and PNPACPI
as well -- they don't read/write hardware registers directly, but the
firmware still depends on preserving the resource order.  I'll have to
ponder that for a while.

Bjorn
--

Both PNPBIOS and PNPACPI should be fine it seems:

pnpbios_get_resources()
   pnpbios_read_resources_from_node()
     pnpbios_parse_allocated_resource_data()
       pnpbios_parse_allocated_irqresource()
       pnpbios_parse_allocated_dmaresource()
       pnpbios_parse_allocated_ioresource()
       pnpbios_parse_allocated_memresource()

where the latter do the same scan for the first _UNSET resource as the new 
code does. Same thing for ACPI in the path

pnpacpi_get_resources()
   pnpacpi_parse_allocated_resource()
      pnpacpi_allocated_resource()
        pnpacpi_parse_allocated_irqresource()
        pnpacpi_parse_allocated_dmaresource()
        pnpacpi_parse_allocated_ioresource()
        pnpacpi_parse_allocated_memresource()
        pnpacpi_parse_allocated_address_space()
          pnpacpi_parse_allocated_memresource()

Rene.
--

While I was there a small cleanup by the way.

Rene.