In message <200007181831.DAA03913@srapc342.sra.co.jp>, Noriyuki Soda writes:
>I hope that GENERIC configuration has the following options.
>(i.e. almost all PCIBIOS options enabled.)

Err, the GENERIC configuration has all of the options commented out.
They don't indicate why they're commend out, so one can only assume
they are commented out because of instability or bugs (are there open
PRs?).

Incidently, it would be nice to understand whether you were attempting
to have this ready for 1.5? I think it'd be nice, but I don't have a
good feeling for the stability.

>Uchiyama-san said PCIBIOS_ADDR_FIXUP may still have problem with
>pccbb (cardbus bridge), so probably it is a bit early to to enable 
>this for now.

Well, GENERIC does not have cardbus support right now. Perhaps
enabling it in GENERIC but not in CARDBUS?

>About PCIBIOS_INTR_FIXUP, I believe that pci_intr_fixup.c revision 1.7 
>is more robust than previous version. So, please try to enable 
>this option with the revision 1.7. I hope this option enabled 
>in NetBSD-1.5.

Ah, see above. When you say "hope," I'm not sure what you mean.
Are you going to commit the change and submit a pullup request?

>Anyway, the description of pcibios.4 should be improved.
>For example, I think pcibios.4 should have the following descriptions.

OK.

>(Please correct if there is incorrect or misleading description,
> also please correct mistakes about English expression.)

OK. I don't have the best understanding of the pcibios code,
but I will review the text for clarity and English now.

>------------------------------------------------------------------------
>PCIBIOS_ADDR_FIXUP
>	Some BIOS doesn't allocate I/O space and memory space for 

"Some" takes a plural, but behaves like a singular. The plural
of BIOS is of some debate, you may choose "BIOSes" or "BIOS
implementations", but that should be "Some BIOSes don't" or
"Some BIOS implementations don't".

>	some PCI devices. Especially, BIOS which is PnP OS mode 
>	enabled shows this behavior.

s/BIOS/a BIOS/
Probably you should put "Pnp OS mode enabled" in quotation marks,
via .Dq in mandoc.

>       Also many BIOS leaves CardBus bridge's space unallocated.

"BIOSes" or "BIOS implementations", and since it is plural,
"leave" instead of "leaves". You want the plural possessive
of "bridge" rather than the singular possesive, so "bridges'"
(apostrophe following the s).

>       Since necessary space isn't allocated, those devices will not
>       work in this situation.

s/in this situation/without special handling/
The current wording implies "this situation" is "with PCIBIOS_ADDR_FIXUP",
which is not what you mean. Lots of other ways to fix it, too, of course,
just my recommendation.

>       This option allocates the I/O space and memory space
>       instead of such BIOS.

s/instead of such BIOS/instead of relying upon the BIOS to do so/

>       If the space is already correctly assigned to devices,
>       this option leaves I/O space and memory space as is,
>
>PCIBIOS_BUS_FIXUP
>       Each PCI bus and CardBus should have unique bus number. 

s/unique/a unique/

>       But some BIOS doesn't assign bus number for subordinate 

"some BIOSes don't assign a bus number"

>       PCI buses. And many BIOS doesn't assign bus number for 
>       CardBuses.

"many BIOSes don't assign a bus number"

>       Typical symptom of this is the following boot message:

s/Typical/A typical/

>               cardbus0 at cardslot0: bus 0 device 0...
>       Please note that this cardbus0 has a bus number `0',
>       but normally the bus number 0 is used by the machine's
>       primary PCI bus, thus, this bus number for cardbus is
>       incorrect (not assigned).

Sentence break before thus. s/bus, thus,/bus. Thus,/

>       In this situation, a device located in cardbus0 doesn't show
>       correct device ID, because it's bus number 0 incorrectly

"it's" is a contraction for "it is", the possessive of "it" is "its".
So s/it's/its/

>       refers primary PCI bus, a device ID in the primary PCI bus are

s/refers/refers to the/
s/a device/and a device/
s/are/is/

>       shown in boot message instead of the device's ID in the
>       cardbus0.

s/boot message/the boot message/

QUESTION: Should the cardbus code print out a warning when it is
detected at bus 0, suggesting pcibios(4) be consulted?

>       This option assigns bus numbers for all subordinate
>       PCI buses and CardBuses.
>
>       Since this option renumbers all PCI buses and CardBuses,
>       all bus numbers of subordinate buses becomes different
>       when this option enabled.

s/becomes/become/

>PCIBIOS_INTR_FIXUP
>       Some BIOS doesn't assign interrupt for some devices.

"Some BIOSes don't assign"
Either "interrupts" or "an interrupt". I guess the latter.

>       This option assigns interrupt for such devices instead
"an interrupt"

>       of such BIOS.

"instead of relying upon the BIOS to do so."

>       This option leaves already assigned interrupt as is.

Not sure what this means. 

"If the BIOS has already assigned an interrupt to a device, this
option leaves the interrupt assigned."

>PCIBIOS_IRQS_HINT
>       PCIBIOS_INTR_FIXUP tries to detect a IRQ which can be used

Because "IRQ" is pronounced "eye are cue" and not "irk",
it is "an IRQ" not "a IRQ."

>       for a PCI device. This option specifies a hint which is 
>       used when the automatic IRQ detection doesn't work.

>       Ths value is a logical-or of power-of-2s of allowable interrupts:

s/Ths/The/
s/logical-or/logical or/

>       IRQ Val      IRQ Val      IRQ Val       IRQ Val
>        0  0x0001    4  0x0010    8  0x0100    12  0x1000
>        1  0x0002    5  0x0020    9  0x0200    13  0x2000
>        2  0x0004    6  0x0040   10  0x0400    14  0x4000
>        3  0x0008    7  0x0080   11  0x0800    15  0x8000
>       For example, "options PCIBIOS_IRQS_HINT=0x0a00" allows 
>       irq 9 and irq 11.
>
>       Kernel global variable `pcibios_irqs_hint' holds this value,

s/Kernel/The kernel/

>       so a user can override this value without kernel recompilation.
>       For example:
>       - To specify this value on the fly, type "boot -d" on
>         boot prompt to drop into DDB (the in-kernel debuger, 

s/on boot prompt/at the boot prompt/
s/debuger/debugger;/ (spelling, and use a semicolon instead of a comma
because you are seperating two independant thoughts)

>         you have to specify "options DDB" to make kernel with DDB).
>         Specify "write pcibios_irqs_hint 0x0a00" on "db>" prompt 

s/on/at the/

>         and type "c" to continue to boot.
>       - To modify kernel image without kernel recompilation,
>         run "gdb --write /netbsd" on shell, 
>         type "set pcibios_irqs_hint=0xa00" on "(gdb)" prompt, 
>         and type "quit" 

>PCIBIOS_INTR_FIXUP_FORCE
>       Some buggy BIOS provides inconsistent information between
>       PCI Interrupt Configuration Register and PCI Interrupt

s/PCI/the PCI/g

>       Routing table.


>       In such case, PCI Interrupt Configuration 
s/PCI/the PCI/
>       Register takes precedence by default. If this happens,
>       a kernel with PCIBIOSVERBOSE shows "WARNING: leave irq XX"
>       in the PCI routing table.

I'm not sure how to parse "WARNING: leave irq XX". Is that intended
to be an imperative (i.e. "Leave IRQ XX alone!") or present tense
indicative ("leaving IRQ XX" i.e. "not changing IRQ XX")?
I'm unclear on what that message is trying to say, it can probably
be improved. Perhaps "preserving" instead of "leaving"?

>
>       If PCIBIOS_INTR_FIXUP_FORCE is specified in addtion to
s/addtion/addition/

>       PCIBIOS_INTR_FIXUP, PCI Interrupt Routing table takes
s/PCI I/the PCI I/

>       precedence. In this case, a kernel with PCIBIOSVERBOSE 
>       shows "WARNING: override irq XX" in the PCI routing table.

Probably should be "overriding".


Under what circumstances would we want to use this option?

>
>PCIINTR_DEBUG
>       Display detailed information about PCIBIOS_INTR_FIXUP processing.
>
>PCIBIOS
>       If one of the above PCIBIOS_* options is used, this option
>       should be specified, too.

Well, that's not quite right. It should be listed first,
and the existing description is probably OK.


>PCIBIOSVERBOSE
>       Display information about PCIBIOS processing.
>------------------------------------------------------------------------

I think these options need to indicate whether they ought to be
enabled as part of the normal case, or only enabled under special
circumstanecs (when needed).


Thanks!

--jhawk