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.