Date:	Fri, 25 Jul 2008 15:18:53 -0700
From:	Jesse Barnes <jbarnes@...tuousgeek.org>
To:	Kenji Kaneshige <kaneshige.kenji@...fujitsu.com>
Cc:	Pierre Ossman <drzeus-list@...eus.cx>,
	Alex Chiang <achiang@...com>,
	LKML <linux-kernel@...r.kernel.org>, linux-pci@...r.kernel.org,
	Kristen Accardi <kristen.c.accardi@...el.com>
Subject: Re: post 2.6.26 requires pciehp_slot_with_bus

On Thursday, July 24, 2008 9:50 pm Kenji Kaneshige wrote:
> Thank you for debug info, Pierre.
>
> According to the debugging output, five slots are detected (five
> slots on laptop!?) and two of them have the same physical slots
> number '2'. This is the reason why Pierre's machine needs
> 'pciehp_slot_with_bus' option.
>
> Before 2.6.26 (from 2.6.xx), pciehp did the workaround for the
> problem (some platform wrongly assign the same physical slot
> number to multiple slots) by default. But this was not a good
> idea because of the several reasons like follows:
>
>   - Slot name should be a physical identifier of physical slot
>     on the system. Using bus number as a part of slot name is
>     not a idea because bus number is logical number and it can
>     be changed.
>
>   - As Jesse explained, some hotplug slot can be handled through
>     several type of controllers. For example, some hotplug slot
>     can be handled by either acpiphp or pciehp. But those drivers
>     must not handle the same slot at the same time. The pci
>     hotplug core is checking this by checking duplicate names.
>     This check didn't work because pciehp had started using bus
>     number as a part of slot name and slot names became different
>     between acpiphp and pciehp.
>
> About the former, I'm ok with using bus number as a part of slot
> name on the problematic platform. But it should not be used on
> the normal platform.
>
> About the latter, IIRC, thanks to Alex's pci slot framework from
> 2.6.26, pci hotplug core can check if multiple drivers attempts
> to handle the same slot even if those drivers uses the different
> names.
>
> Based on my thought above, I have a following idea to remove
> "pciehp_slot_with_bus".
>
>   - Try to use physical slot number as a slot name, first.
>
>   - If pci_hp_register() success, no problem.
>
>   - If pci_hp_register() returns -EBUSY, that means another
>     hotplug driver already handling the slot. So return as error.
>
>   - If pci_hp_register() returns -EEXIST, that means there is a
>     existing slot with the same name. In this case, retry to
>     register slots with logical name (bus number + physical slot
>     number, or other).
>
> With this idea, slots names will become as follows on Pierre's
> machine.
>
> <Before 2.6.26>
> 0001_0001, 0002_0002, 0003_0003, 0004_0004, 0005_0005, 000d_0002
>
> <Current>
> 1, 2, 3, 4, 5
>
> <With my idea>
> 1, 2, 3, 4, 5, 000d_0002
>
>
> Please give me comments.

I think that's fine (automatically creating duplicate devices with names to 
differentiate them), but I think we should also try harder to avoid adding 
duplicates.

In Pierre's case, and on my T61, there's only one actual hotplug slot 
available, but the firmware creates duplicate physical slot numbers and sets 
the HP_CAP bit on everything, both of which are obviously wrong (well I 
suppose you could pop these chips off the board, but it's not very 
practical).  However, afaict that "other" OS uses the _RMV method to 
determine whether a given slot is actually hot pluggable.  On my T61 at 
least, this seems to be accurate: only one of my EXP* objects has a _RMV 
method.

So maybe the PCIe hotplug driver should be checking for that method when ACPI 
is available?  We already try to use _OSC etc., so checking for _RMV first 
would make sense...

I tried and failed to do this (naive patch attached), I think somehow I've got 
to traverse child devices too... (and of course add the appropriate #ifdef 
ACPI etc stuff).

Kenji-san and Alex, maybe you can take a look and clue me in?

Thanks,
Jesse

View attachment "pciehp-detect-fixes.patch" of type "text/x-diff" (3972 bytes)

Powered by blists - more mailing lists