post 2.6.26 requires pciehp_slot_with_bus - Kernel

This is a discussion on post 2.6.26 requires pciehp_slot_with_bus - Kernel ; Matthew Wilcox wrote: > On Fri, Jul 25, 2008 at 05:53:33PM +0900, Kenji Kaneshige wrote: >> IIRC, pciehp uses bridge's secondary bus number for slot name, and >> PCI express downstream port can have only one hotplug slot. I think ...

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2
Results 21 to 37 of 37

Thread: post 2.6.26 requires pciehp_slot_with_bus

  1. Re: post 2.6.26 requires pciehp_slot_with_bus

    Matthew Wilcox wrote:
    > On Fri, Jul 25, 2008 at 05:53:33PM +0900, Kenji Kaneshige wrote:
    >> IIRC, pciehp uses bridge's secondary bus number for slot name, and
    >> PCI express downstream port can have only one hotplug slot. I think
    >> this is why with_bus prameter makes difference.

    >
    > Ahh, I overlooked that last night.
    >
    >> But it doesn't work on the system that has multiple pci segments.

    >
    > Yes, we still have the problem that pciehp does not include the 'chassis
    > number' as part of the name. I no longer have easy access to any
    > systems with multiple chassis. Do your systems have devices which
    > implement the PCI_CAP_ID_SLOTID capcbility? (as root) lspci -vvv will
    > report it:
    >
    > printf("Slot ID: %d slots, First%c, chassis %02x\n",
    > esr & PCI_SID_ESR_NSLOTS,
    > FLAG(esr, PCI_SID_ESR_FIC),
    > chs);
    >


    Unfortunately, I don't have any systems that implement the
    PCI_CAP_ID_SLOTID capability...

    Thanks,
    Kenji Kaneshige


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: post 2.6.26 requires pciehp_slot_with_bus

    Jesse Barnes wrote:
    > 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.
    >>
    >>
    >> 0001_0001, 0002_0002, 0003_0003, 0004_0004, 0005_0005, 000d_0002
    >>
    >>
    >> 1, 2, 3, 4, 5
    >>
    >>
    >> 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...
    >


    As you pointed out, the root cause might not a problem of slot naming,
    but a problem of slots detection, because pciehp driver detects multiple
    PCIe hotplug slots even thought your and Pierre's system seems to have
    only one hotplug slot. So I think we should also consider the problem
    from this view point (slot detection).

    But, I think simply checking for _RMV method first is dangerous because
    I think there are many systems that doesn't implement _RMV for PCIe
    hotplug slots (at least, my system doesn't implement that. Anyway,
    I would like to look at the documents/specifications that mention _RMV
    method for determining whether a given slot is hot pluggable. Do you
    have any information about that? I think PCI Local Bus, PCI Express and
    PCI Firmware specification don't mention that. I think hot pluggable slots
    on your, Pierre's and Matthew's system are ExpressCard slots. So I guess
    ExpressCard specification might define something about this. But
    unfortunately, I don't have ExpressCard specification. Can anyone access
    ExpressCard spec?

    Thanks,
    Kenji Kaneshige


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: post 2.6.26 requires pciehp_slot_with_bus

    Matthew Wilcox wrote:
    > On Fri, Jul 25, 2008 at 03:18:53PM -0700, Jesse Barnes wrote:
    >> 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.

    >
    > I think you're getting distracted from the real problem we're trying to
    > solve here, the reason for introducing the pci_slot driver in the first
    > place: we want to have information on all slots, not just hotplug ones.
    >
    > So while this is growing out of the hotplug system, we need to register
    > all slots, even ones without _RMV.
    >


    I think Jesse's idea is not for breaking pci_slot driver. I think even
    with his idea pci_slot driver will detect all slots, but hotplug driver
    (e.g. pciehp) will not be registered on some of those slots.

    By the way, how is pci_slot driver on your system that has a problem with
    pciehp? Does duplicate slot problem happen also with pci_slot driver?

    Thanks,
    Kenji Kaneshige

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: post 2.6.26 requires pciehp_slot_with_bus

    On Monday, July 28, 2008 1:44 am Kenji Kaneshige wrote:
    > Jesse Barnes wrote:
    > > 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...

    >
    > As you pointed out, the root cause might not a problem of slot naming,
    > but a problem of slots detection, because pciehp driver detects multiple
    > PCIe hotplug slots even thought your and Pierre's system seems to have
    > only one hotplug slot. So I think we should also consider the problem
    > from this view point (slot detection).
    >
    > But, I think simply checking for _RMV method first is dangerous because
    > I think there are many systems that doesn't implement _RMV for PCIe
    > hotplug slots (at least, my system doesn't implement that. Anyway,
    > I would like to look at the documents/specifications that mention _RMV
    > method for determining whether a given slot is hot pluggable. Do you
    > have any information about that? I think PCI Local Bus, PCI Express and
    > PCI Firmware specification don't mention that. I think hot pluggable slots
    > on your, Pierre's and Matthew's system are ExpressCard slots. So I guess
    > ExpressCard specification might define something about this. But
    > unfortunately, I don't have ExpressCard specification. Can anyone access
    > ExpressCard spec?


    Your systems don't have _RMV methods for the hotpluggable PCIe slots in the
    DSDT? That's a shame; the Windows docs I found on PCIe hotplug seemed to
    indicate that _RMV and _OSC (under Vista) were used to detect whether a given
    slot was hot pluggable (I just googled for "windows pcie hotplug" or
    something) so I was hoping that would be a reliable method... Any other
    ideas? I'll go see if I can dig up some ExpressCard info.

    Thanks,
    Jesse
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: post 2.6.26 requires pciehp_slot_with_bus

    On Mon, Jul 28, 2008 at 05:44:28PM +0900, Kenji Kaneshige wrote:
    > But, I think simply checking for _RMV method first is dangerous because
    > I think there are many systems that doesn't implement _RMV for PCIe
    > hotplug slots (at least, my system doesn't implement that. Anyway,
    > I would like to look at the documents/specifications that mention _RMV
    > method for determining whether a given slot is hot pluggable. Do you
    > have any information about that? I think PCI Local Bus, PCI Express and
    > PCI Firmware specification don't mention that. I think hot pluggable slots
    > on your, Pierre's and Matthew's system are ExpressCard slots. So I guess
    > ExpressCard specification might define something about this. But
    > unfortunately, I don't have ExpressCard specification. Can anyone access
    > ExpressCard spec?


    My only externally visible slot is CardBus, not ExpressCard. It's this
    laptop:

    http://store.shopfujitsu.com/ca/Ecom...o?series=P8010

    full lspci:

    00:00.0 Host bridge: Intel Corporation Mobile PM965/GM965/GL960 Memory Controller Hub (rev 03)
    Subsystem: Fujitsu Limited. Device 13f2
    Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0
    Capabilities: [e0] Vendor Specific Information
    Kernel driver in use: agpgart-intel

    00:02.0 VGA compatible controller: Intel Corporation Mobile GM965/GL960 Integrated Graphics Controller (rev 03)
    Subsystem: Fujitsu Limited. Device 13fe
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0
    Interrupt: pin A routed to IRQ 16
    Region 0: Memory at fc000000 (64-bit, non-prefetchable) [size=1M]
    Region 2: Memory at e0000000 (64-bit, prefetchable) [size=256M]
    Region 4: I/O ports at 1800 [size=8]
    Capabilities: [90] Message Signalled Interrupts: Mask- 64bit- Queue=0/0 Enable-
    Address: 00000000 Data: 0000
    Capabilities: [d0] Power Management version 3
    Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Bridge: PM- B3+

    00:02.1 Display controller: Intel Corporation Mobile GM965/GL960 Integrated Graphics Controller (rev 03)
    Subsystem: Fujitsu Limited. Device 13fe
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0
    Region 0: Memory at fc100000 (64-bit, non-prefetchable) [size=1M]
    Capabilities: [d0] Power Management version 3
    Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Bridge: PM- B3+

    00:1a.0 USB Controller: Intel Corporation 82801H (ICH8 Family) USB UHCI Controller #4 (rev 03)
    Subsystem: Fujitsu Limited. Device 1414
    Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
    Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0
    Interrupt: pin A routed to IRQ 22
    Region 4: I/O ports at 1820 [size=32]
    Kernel driver in use: uhci_hcd

    00:1a.1 USB Controller: Intel Corporation 82801H (ICH8 Family) USB UHCI Controller #5 (rev 03)
    Subsystem: Fujitsu Limited. Device 1414
    Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
    Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0
    Interrupt: pin A routed to IRQ 22
    Region 4: I/O ports at 1840 [size=32]
    Kernel driver in use: uhci_hcd

    00:1a.7 USB Controller: Intel Corporation 82801H (ICH8 Family) USB2 EHCI Controller #2 (rev 03) (prog-if 20)
    Subsystem: Fujitsu Limited. Device 1415
    Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0
    Interrupt: pin B routed to IRQ 23
    Region 0: Memory at fc704800 (32-bit, non-prefetchable) [size=1K]
    Capabilities: [50] Power Management version 2
    Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Capabilities: [58] Debug port: BAR=1 offset=00a0
    Kernel driver in use: ehci_hcd

    00:1b.0 Audio device: Intel Corporation 82801H (ICH8 Family) HD Audio Controller (rev 03)
    Subsystem: Fujitsu Limited. Device 142d
    Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0, Cache Line Size: 64 bytes
    Interrupt: pin A routed to IRQ 21
    Region 0: Memory at fc700000 (64-bit, non-prefetchable) [size=16K]
    Capabilities: [50] Power Management version 2
    Flags: PMEClk- DSI- D1- D2- AuxCurrent=55mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Capabilities: [60] Message Signalled Interrupts: Mask- 64bit+ Queue=0/0 Enable-
    Address: 0000000000000000 Data: 0000
    Capabilities: [70] Express (v1) Root Complex Integrated Endpoint, MSI 00
    DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, L1 <1us
    ExtTag- RBE- FLReset-
    DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
    RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop+
    MaxPayload 128 bytes, MaxReadReq 128 bytes
    DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
    LnkCap: Port #0, Speed unknown, Width x0, ASPM unknown, Latency L0 <64ns, L1 <1us
    ClockPM- Suprise- LLActRep- BwNot-
    LnkCtl: ASPM Disabled; Disabled- Retrain- CommClk-
    ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
    LnkSta: Speed unknown, Width x0, TrErr- Train- SlotClk- DLActive- BWMgmt- ABWMgmt-
    Capabilities: [100] Virtual Channel
    Capabilities: [130] Root Complex Link
    Kernel driver in use: HDA Intel

    00:1c.0 PCI bridge: Intel Corporation 82801H (ICH8 Family) PCI Express Port 1 (rev 03)
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0, Cache Line Size: 64 bytes
    Bus: primary=00, secondary=04, subordinate=07, sec-latency=0
    I/O behind bridge: 00002000-00002fff
    Memory behind bridge: fc200000-fc2fffff
    Prefetchable memory behind bridge: 00000000a8000000-00000000a80fffff
    Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- BridgeCtl: Parity- SERR- NoISA+ VGA- MAbort- >Reset- FastB2B-
    PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
    Capabilities: [40] Express (v1) Root Port (Slot+), MSI 00
    DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s unlimited, L1 unlimited
    ExtTag- RBE+ FLReset-
    DevCtl: Report errors: Correctable+ Non-Fatal+ Fatal+ Unsupported+
    RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
    MaxPayload 128 bytes, MaxReadReq 128 bytes
    DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
    LnkCap: Port #1, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <256ns, L1 <4us
    ClockPM- Suprise- LLActRep+ BwNot-
    LnkCtl: ASPM L0s Enabled; RCB 64 bytes Disabled- Retrain- CommClk+
    ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
    LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive+ BWMgmt- ABWMgmt-
    SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surpise+
    Slot # 2, PowerLimit 6.500000; Interlock- NoCompl-
    SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet+ CmdCplt- HPIrq- LinkChg-
    Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock-
    SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet+ Interlock-
    Changed: MRL- PresDet- LinkState-
    RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna- CRSVisible-
    RootCap: CRSVisible-
    RootSta: PME ReqID 0000, PMEStatus- PMEPending-
    Capabilities: [80] Message Signalled Interrupts: Mask- 64bit- Queue=0/0 Enable-
    Address: fee0100c Data: 41b1
    Capabilities: [90] Subsystem: Fujitsu Limited. Device 1416
    Capabilities: [a0] Power Management version 2
    Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Capabilities: [100] Virtual Channel
    Capabilities: [180] Root Complex Link
    Kernel driver in use: pcieport-driver

    00:1c.4 PCI bridge: Intel Corporation 82801H (ICH8 Family) PCI Express Port 5 (rev 03)
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0, Cache Line Size: 64 bytes
    Bus: primary=00, secondary=14, subordinate=1b, sec-latency=0
    I/O behind bridge: 0000f000-00000fff
    Memory behind bridge: fc300000-fc3fffff
    Prefetchable memory behind bridge: 00000000fff00000-00000000000fffff
    Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- BridgeCtl: Parity- SERR- NoISA+ VGA- MAbort- >Reset- FastB2B-
    PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
    Capabilities: [40] Express (v1) Root Port (Slot+), MSI 00
    DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s unlimited, L1 unlimited
    ExtTag- RBE+ FLReset-
    DevCtl: Report errors: Correctable+ Non-Fatal+ Fatal+ Unsupported+
    RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
    MaxPayload 128 bytes, MaxReadReq 128 bytes
    DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
    LnkCap: Port #5, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <256ns, L1 <4us
    ClockPM- Suprise- LLActRep+ BwNot-
    LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- Retrain- CommClk+
    ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
    LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive+ BWMgmt- ABWMgmt-
    SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surpise+
    Slot # 2, PowerLimit 6.500000; Interlock- NoCompl-
    SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet+ CmdCplt- HPIrq- LinkChg-
    Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock-
    SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet+ Interlock-
    Changed: MRL- PresDet- LinkState-
    RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna- CRSVisible-
    RootCap: CRSVisible-
    RootSta: PME ReqID 0000, PMEStatus- PMEPending-
    Capabilities: [80] Message Signalled Interrupts: Mask- 64bit- Queue=0/0 Enable-
    Address: fee0100c Data: 41b9
    Capabilities: [90] Subsystem: Fujitsu Limited. Device 1416
    Capabilities: [a0] Power Management version 2
    Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Capabilities: [100] Virtual Channel
    Capabilities: [180] Root Complex Link
    Kernel driver in use: pcieport-driver

    00:1d.0 USB Controller: Intel Corporation 82801H (ICH8 Family) USB UHCI Controller #1 (rev 03)
    Subsystem: Fujitsu Limited. Device 1414
    Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
    Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0
    Interrupt: pin A routed to IRQ 22
    Region 4: I/O ports at 1860 [size=32]
    Kernel driver in use: uhci_hcd

    00:1d.1 USB Controller: Intel Corporation 82801H (ICH8 Family) USB UHCI Controller #2 (rev 03)
    Subsystem: Fujitsu Limited. Device 1414
    Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
    Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0
    Interrupt: pin A routed to IRQ 22
    Region 4: I/O ports at 1880 [size=32]
    Kernel driver in use: uhci_hcd

    00:1d.7 USB Controller: Intel Corporation 82801H (ICH8 Family) USB2 EHCI Controller #1 (rev 03) (prog-if 20)
    Subsystem: Fujitsu Limited. Device 1415
    Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0
    Interrupt: pin B routed to IRQ 23
    Region 0: Memory at fc704c00 (32-bit, non-prefetchable) [size=1K]
    Capabilities: [50] Power Management version 2
    Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME+
    Capabilities: [58] Debug port: BAR=1 offset=00a0
    Kernel driver in use: ehci_hcd

    00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev f3) (prog-if 01)
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0
    Bus: primary=00, secondary=1c, subordinate=20, sec-latency=32
    I/O behind bridge: 00003000-00003fff
    Memory behind bridge: fc400000-fc4fffff
    Prefetchable memory behind bridge: 00000000fff00000-00000000000fffff
    Secondary status: 66MHz- FastB2B+ ParErr- DEVSEL=medium >TAbort- BridgeCtl: Parity- SERR- NoISA+ VGA- MAbort- >Reset- FastB2B-
    PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
    Capabilities: [50] Subsystem: Fujitsu Limited. Device 140c

    00:1f.0 ISA bridge: Intel Corporation 82801HEM (ICH8M) LPC Interface Controller (rev 03)
    Subsystem: Fujitsu Limited. Device 140e
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0
    Capabilities: [e0] Vendor Specific Information

    00:1f.1 IDE interface: Intel Corporation 82801HBM/HEM (ICH8M/ICH8M-E) IDE Controller (rev 03) (prog-if 8a [Master SecP PriP])
    Subsystem: Fujitsu Limited. Device 140f
    Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
    Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0
    Interrupt: pin A routed to IRQ 20
    Region 0: I/O ports at 01f0 [size=8]
    Region 1: I/O ports at 03f4 [size=1]
    Region 2: I/O ports at 0170 [size=8]
    Region 3: I/O ports at 0374 [size=1]
    Region 4: I/O ports at 1810 [size=16]
    Kernel driver in use: ata_piix

    00:1f.2 SATA controller: Intel Corporation 82801HBM/HEM (ICH8M/ICH8M-E) SATA AHCI Controller (rev 03) (prog-if 01)
    Subsystem: Fujitsu Limited. Device 1411
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
    Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Latency: 0
    Interrupt: pin A routed to IRQ 220
    Region 0: I/O ports at 18e0 [size=8]
    Region 1: I/O ports at 18b4 [size=4]
    Region 2: I/O ports at 18b8 [size=8]
    Region 3: I/O ports at 18b0 [size=4]
    Region 4: I/O ports at 18c0 [size=32]
    Region 5: Memory at fc704000 (32-bit, non-prefetchable) [size=2K]
    Capabilities: [80] Message Signalled Interrupts: Mask- 64bit- Queue=0/2 Enable+
    Address: fee0100c Data: 41d1
    Capabilities: [70] Power Management version 3
    Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot+,D3cold-)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Capabilities: [a8] SATA HBA
    Kernel driver in use: ahci

    00:1f.3 SMBus: Intel Corporation 82801H (ICH8 Family) SMBus Controller (rev 03)
    Subsystem: Fujitsu Limited. Device 1413
    Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Interrupt: pin B routed to IRQ 21
    Region 0: Memory at a8100000 (32-bit, non-prefetchable) [size=256]
    Region 4: I/O ports at 1c00 [size=32]
    Kernel driver in use: i801_smbus

    04:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8055 PCI-E Gigabit Ethernet Controller (rev 14)
    Subsystem: Fujitsu Limited. Device 139a
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0, Cache Line Size: 64 bytes
    Interrupt: pin A routed to IRQ 221
    Region 0: Memory at fc200000 (64-bit, non-prefetchable) [size=16K]
    Region 2: I/O ports at 2000 [size=256]
    [virtual] Expansion ROM at a8000000 [disabled] [size=128K]
    Capabilities: [48] Power Management version 3
    Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Capabilities: [50] Vital Product Data
    Capabilities: [5c] Message Signalled Interrupts: Mask- 64bit+ Queue=0/0 Enable+
    Address: 00000000fee0100c Data: 41c1
    Capabilities: [e0] Express (v1) Legacy Endpoint, MSI 00
    DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s unlimited, L1 unlimited
    ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
    DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
    RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
    MaxPayload 128 bytes, MaxReadReq 512 bytes
    DevSta: CorrErr+ UncorrErr+ FatalErr- UnsuppReq+ AuxPwr+ TransPend-
    LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <256ns, L1 unlimited
    ClockPM+ Suprise- LLActRep- BwNot-
    LnkCtl: ASPM L0s Enabled; RCB 128 bytes Disabled- Retrain- CommClk+
    ExtSynch- ClockPM+ AutWidDis- BWInt- AutBWInt-
    LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
    Capabilities: [100] Advanced Error Reporting
    Kernel driver in use: sky2

    14:00.0 Network controller: Intel Corporation PRO/Wireless 4965 AG or AGN Network Connection (rev 61)
    Subsystem: Intel Corporation Device 1100
    Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0, Cache Line Size: 64 bytes
    Interrupt: pin A routed to IRQ 219
    Region 0: Memory at fc300000 (64-bit, non-prefetchable) [size=8K]
    Capabilities: [c8] Power Management version 3
    Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Capabilities: [d0] Message Signalled Interrupts: Mask- 64bit+ Queue=0/0 Enable+
    Address: 00000000fee0300c Data: 4138
    Capabilities: [e0] Express (v1) Endpoint, MSI 00
    DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 unlimited
    ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
    DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
    RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
    MaxPayload 128 bytes, MaxReadReq 128 bytes
    DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr+ TransPend-
    LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Latency L0 <128ns, L1 <64us
    ClockPM+ Suprise- LLActRep- BwNot-
    LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- Retrain- CommClk+
    ExtSynch- ClockPM+ AutWidDis- BWInt- AutBWInt-
    LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
    Capabilities: [100] Advanced Error Reporting
    Capabilities: [140] Device Serial Number 97-62-84-ff-ff-3b-1f-00
    Kernel driver in use: iwl4965

    1c:03.0 CardBus bridge: O2 Micro, Inc. OZ711SP1 Memory CardBus Controller (rev 01)
    Subsystem: Fujitsu Limited. Device 143d
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping+ SERR- FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=slow >TAbort- SERR- Latency: 168
    Interrupt: pin A routed to IRQ 16
    Region 0: Memory at fc402000 (32-bit, non-prefetchable) [size=4K]
    Bus: primary=1c, secondary=1d, subordinate=20, sec-latency=176
    Memory window 0: a8400000-a87ff000 (prefetchable)
    Memory window 1: ac000000-affff000
    I/O window 0: 00003000-000030ff
    I/O window 1: 00003400-000034ff
    BridgeCtl: Parity- SERR- ISA- VGA- MAbort- >Reset+ 16bInt+ PostWrite+
    16-bit legacy interface ports at 0001
    Kernel driver in use: yenta_cardbus

    1c:03.2 SD Host controller: O2 Micro, Inc. Integrated MMC/SD Controller (rev 02) (prog-if 01)
    Subsystem: Fujitsu Limited. Device 143d
    Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=slow >TAbort- SERR- Latency: 32, Cache Line Size: 64 bytes
    Interrupt: pin A routed to IRQ 16
    Region 0: Memory at fc401800 (32-bit, non-prefetchable) [size=256]
    Capabilities: [a0] Power Management version 2
    Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME-
    Kernel driver in use: sdhci-pci
    Kernel modules: sdhci-pci

    1c:03.4 FireWire (IEEE 1394): O2 Micro, Inc. Firewire (IEEE 1394) (rev 02) (prog-if 10)
    Subsystem: Fujitsu Limited. Device 143e
    Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- SERR- Latency: 32, Cache Line Size: 64 bytes
    Interrupt: pin A routed to IRQ 16
    Region 0: Memory at fc400000 (32-bit, non-prefetchable) [size=4K]
    Region 1: Memory at fc401000 (32-bit, non-prefetchable) [size=2K]
    Capabilities: [60] Power Management version 2
    Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold-)
    Status: D0 PME-Enable- DSel=0 DScale=0 PME+
    Kernel driver in use: ohci1394



    --
    Intel are signing my paycheques ... these opinions are still mine
    "Bill, look, we understand that you're interested in selling us this
    operating system, but compare it to ours. We can't possibly take such
    a retrograde step."
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: post 2.6.26 requires pciehp_slot_with_bus

    On Thu, Jul 24, 2008 at 10:42:34PM -0600, Alex Chiang wrote:
    > * Matthew Wilcox :
    > > On Fri, Jul 25, 2008 at 01:29:16AM +0200, Pierre Ossman wrote:
    > > > On Thu, 24 Jul 2008 17:08:27 -0600
    > > > Alex Chiang wrote:
    > > >
    > > > > Sorry for one more round-trip, but could you turn on debugging
    > > > > for pciehp as well?
    > > > >
    > > >
    > > > Same thing, with debugging:

    > >
    > > I have a laptop with a similar problem (though I don't have pciehp
    > > enabled, so I didn't notice it). Obviously, we need to fix this.
    > >
    > > There is no question in my mind that firmware has programmed the slot
    > > numbers incorrectly. Here's the evidence from lspci -vvv:
    > >
    > > 00:1c.0 PCI bridge: Intel Corporation 82801H (ICH8 Family) PCI Express Port 1 (rev 03)
    > > Capabilities: [40] Express (v1) Root Port (Slot+), MSI 00
    > > SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surpise+
    > > Slot # 2, PowerLimit 6.500000; Interlock- NoCompl-
    > > 00:1c.4 PCI bridge: Intel Corporation 82801H (ICH8 Family) PCI Express Port 5 (rev 03)
    > > Capabilities: [40] Express (v1) Root Port (Slot+), MSI 00
    > > SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surpise+
    > > Slot # 2, PowerLimit 6.500000; Interlock- NoCompl-
    > >
    > > I don't think anyone can credibly argue that this is correct. They're
    > > both PCIe devices, they're both both indicating that they have a slot
    > > (maybe if I get my screwdriver out, I can see if there's really a slot
    > > ...), they're on the same bus (so I don't know how the with_bus
    > > parameter makes any difference).
    > >
    > > I've always hated that with_bus parameter. I don't like it being a
    > > parameter and I don't like the names it produces.
    > >
    > > Part of the problem is the kobject API. It really hates you trying to
    > > register a duplicate name and won't just return -EEXIST and let you try
    > > a new name. Instead it prints an ugly warning and dumps stack. See
    > > kobject_add_internal() in lib/kobject.c.

    >
    > Yeah, I don't really like that part of the kobject API either.


    Then don't register kobjects with the same name of an already existing
    one

    It's pretty simple, you already have a list of all kobjects associated
    with this parent kobject (or driver or class), so search them all before
    registering them if you think you might end up with a duplicate.

    thanks,

    greg k-h
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: post 2.6.26 requires pciehp_slot_with_bus

    Jesse Barnes wrote:
    > On Monday, July 28, 2008 1:44 am Kenji Kaneshige wrote:
    >> Jesse Barnes wrote:
    >>> 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...

    >> As you pointed out, the root cause might not a problem of slot naming,
    >> but a problem of slots detection, because pciehp driver detects multiple
    >> PCIe hotplug slots even thought your and Pierre's system seems to have
    >> only one hotplug slot. So I think we should also consider the problem
    >> from this view point (slot detection).
    >>
    >> But, I think simply checking for _RMV method first is dangerous because
    >> I think there are many systems that doesn't implement _RMV for PCIe
    >> hotplug slots (at least, my system doesn't implement that. Anyway,
    >> I would like to look at the documents/specifications that mention _RMV
    >> method for determining whether a given slot is hot pluggable. Do you
    >> have any information about that? I think PCI Local Bus, PCI Express and
    >> PCI Firmware specification don't mention that. I think hot pluggable slots
    >> on your, Pierre's and Matthew's system are ExpressCard slots. So I guess
    >> ExpressCard specification might define something about this. But
    >> unfortunately, I don't have ExpressCard specification. Can anyone access
    >> ExpressCard spec?

    >
    > Your systems don't have _RMV methods for the hotpluggable PCIe slots in the
    > DSDT? That's a shame; the Windows docs I found on PCIe hotplug seemed to
    > indicate that _RMV and _OSC (under Vista) were used to detect whether a given
    > slot was hot pluggable (I just googled for "windows pcie hotplug" or
    > something) so I was hoping that would be a reliable method... Any other
    > ideas? I'll go see if I can dig up some ExpressCard info.
    >


    My systems don't have _RMV methods for the hot pluggable PCIe slots in the
    DSDT, but I don't think that's a shame. I suppose that the document you are
    referring describes how Windows handles ExpressCard slots. In my
    understanding, Hot Plug Surprise bit in the Slot Capabilities register is
    set to 1b on ExpressCard slots, and I believe that ACPI _RVM method is for
    the device that only supports surprise-style removal. I think this is why
    your system implements _RMV method for slots.

    On the other hand, hot pluggable slots on my servers are *not* ExpressCard
    slots, and all of them have Power Controller instead of surprise-style
    removal (Hot Plug Surprise bit in the Slot Capabilities register is set to
    0b). So I believe there is no reason to implement _RMV methods for the hot
    pluggable PCIe slots on my systems.

    Here is an idea. How about using _RMV method to determine whether a given
    slot is actually hot pluggable when Hot Plug Surprise bit in the Slot
    Capabilities register is set to 1b on the slot? This is based on a little
    rough assumption that all PCIe slots that support surprise-style removal
    have _RMV method, though. Does this work for you?

    Thanks,
    Kenji Kaneshige

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: post 2.6.26 requires pciehp_slot_with_bus

    On Monday, July 28, 2008 7:43 pm Kenji Kaneshige wrote:
    > > Your systems don't have _RMV methods for the hotpluggable PCIe slots in
    > > the DSDT? That's a shame; the Windows docs I found on PCIe hotplug
    > > seemed to indicate that _RMV and _OSC (under Vista) were used to detect
    > > whether a given slot was hot pluggable (I just googled for "windows pcie
    > > hotplug" or something) so I was hoping that would be a reliable method...
    > > Any other ideas? I'll go see if I can dig up some ExpressCard info.

    >
    > My systems don't have _RMV methods for the hot pluggable PCIe slots in the
    > DSDT, but I don't think that's a shame. I suppose that the document you are
    > referring describes how Windows handles ExpressCard slots. In my
    > understanding, Hot Plug Surprise bit in the Slot Capabilities register is
    > set to 1b on ExpressCard slots, and I believe that ACPI _RVM method is for
    > the device that only supports surprise-style removal. I think this is why
    > your system implements _RMV method for slots.


    Yeah, that may be. The document wasn't very clear; I was hoping that
    something simple would be available.

    > On the other hand, hot pluggable slots on my servers are *not* ExpressCard
    > slots, and all of them have Power Controller instead of surprise-style
    > removal (Hot Plug Surprise bit in the Slot Capabilities register is set to
    > 0b). So I believe there is no reason to implement _RMV methods for the hot
    > pluggable PCIe slots on my systems.
    >
    > Here is an idea. How about using _RMV method to determine whether a given
    > slot is actually hot pluggable when Hot Plug Surprise bit in the Slot
    > Capabilities register is set to 1b on the slot? This is based on a little
    > rough assumption that all PCIe slots that support surprise-style removal
    > have _RMV method, though. Does this work for you?


    It's worth a try. We need *some* sort of better method to detect hot
    pluggable slots...

    Thanks,
    Jesse
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: post 2.6.26 requires pciehp_slot_with_bus

    * Kenji Kaneshige :
    > Matthew Wilcox wrote:
    >> So we need a way to find if there's already a slot of this
    >> name. I don't see a kobject routine to do that. Maybe we can
    >> do it internally to the pci slot code.


    pci_hp_register already does this with get_slot_from_name().

    >> Then we need to pick a new name for the kobject if it does
    >> collide. My suggestion is that the second time we find an
    >> object named "2", we call it "2dup1" (the third time "2dup2",
    >> etc.) Other opinions I've seen include "2a", "2b", ... or
    >> "2-1", "2-2", ... or "2-brokenfw1", "2-brokenfw2".

    >
    > That looks quite better than using bus number.


    I went with:

    - first slot to register gets "2"
    - second slot to register gets "2-1"
    - Mth slot to register gets "2-M"

    At first, I thought it would have been better to put this logic
    inside of pci_hp_register, since it knows about the collision,
    and could just fix stuff up for the caller.

    However, the problem is that each hotplug driver can have a
    different length for "name", and it got messy quickly.

    So, I just patched the two drivers that are known to be
    problematic.

    Two patches follow, against 2.6.27-rc1.

    Compile tested only -- I don't have hardware to replicate this.

    I'd say they're somewhere between RFC and requested for
    inclusion. I'm certainly not tied to them, just trying to show
    some code to implement the approach described above. If we decide
    that looking at _RMV + other bits is the way to go, then I'm fine
    with that.

    It would be great if Pierre and Kenji-san could try them out.

    Thanks.

    /ac

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M...

    Commit 3800345f723fd130d50434d4717b99d4a9f383c8 introduces the
    pciehp_slot_with_bus module parameter, which was intended to help
    work around broken firmware that assigns the same name to multiple
    slots.

    Commit 9e4f2e8d4ddb04ad16a3828cd9a369a5a5287009 tells the user to
    use the above parameter in the event of a name collision.

    This approach is sub-optimal because it requires too much work from
    the user.

    Instead, let's rename the slot on behalf of the user. If firmware
    assigns the name N to multiple slots, then:

    The first registered slot is assigned N
    The second registered slot is assigned N-1
    The third registered slot is assigned N-2
    The Mth registered slot becomes N-M

    In the event we overflow the slot->name parameter, we report an
    error to the user.

    Signed-off-by: Alex Chiang
    ---
    drivers/pci/hotplug/pciehp.h | 1 -
    drivers/pci/hotplug/pciehp_core.c | 21 ++++++++++++++-------
    drivers/pci/hotplug/pciehp_hpc.c | 11 +----------
    3 files changed, 15 insertions(+), 18 deletions(-)

    diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
    index e3a1e7e..9e6cec6 100644
    --- a/drivers/pci/hotplug/pciehp.h
    +++ b/drivers/pci/hotplug/pciehp.h
    @@ -43,7 +43,6 @@ extern int pciehp_poll_mode;
    extern int pciehp_poll_time;
    extern int pciehp_debug;
    extern int pciehp_force;
    -extern int pciehp_slot_with_bus;
    extern struct workqueue_struct *pciehp_wq;

    #define dbg(format, arg...) \
    diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
    index 3677495..4fd5355 100644
    --- a/drivers/pci/hotplug/pciehp_core.c
    +++ b/drivers/pci/hotplug/pciehp_core.c
    @@ -41,7 +41,6 @@ int pciehp_debug;
    int pciehp_poll_mode;
    int pciehp_poll_time;
    int pciehp_force;
    -int pciehp_slot_with_bus;
    struct workqueue_struct *pciehp_wq;

    #define DRIVER_VERSION "0.4"
    @@ -56,12 +55,10 @@ module_param(pciehp_debug, bool, 0644);
    module_param(pciehp_poll_mode, bool, 0644);
    module_param(pciehp_poll_time, int, 0644);
    module_param(pciehp_force, bool, 0644);
    -module_param(pciehp_slot_with_bus, bool, 0644);
    MODULE_PARM_DESC(pciehp_debug, "Debugging mode enabled or not");
    MODULE_PARM_DESC(pciehp_poll_mode, "Using polling mechanism for hot-plug events or not");
    MODULE_PARM_DESC(pciehp_poll_time, "Polling mechanism frequency, in seconds");
    MODULE_PARM_DESC(pciehp_force, "Force pciehp, even if _OSC and OSHP are missing");
    -MODULE_PARM_DESC(pciehp_slot_with_bus, "Use bus number in the slot name");

    #define PCIE_MODULE_NAME "pciehp"

    @@ -194,6 +191,7 @@ static int init_slots(struct controller *ctrl)
    struct slot *slot;
    struct hotplug_slot *hotplug_slot;
    struct hotplug_slot_info *info;
    + int len, dup = 1;
    int retval = -ENOMEM;

    list_for_each_entry(slot, &ctrl->slot_list, slot_list) {
    @@ -220,15 +218,24 @@ static int init_slots(struct controller *ctrl)
    dbg("Registering bus=%x dev=%x hp_slot=%x sun=%x "
    "slot_device_offset=%x\n", slot->bus, slot->device,
    slot->hp_slot, slot->number, ctrl->slot_device_offset);
    +duplicate_name:
    retval = pci_hp_register(hotplug_slot,
    ctrl->pci_dev->subordinate,
    slot->device);
    if (retval) {
    + /*
    + * If slot N already exists, we'll try to create
    + * slot N-1, N-2 ... N-M, until we overflow.
    + */
    + if (retval == -EEXIST) {
    + len = snprintf(slot->name, SLOT_NAME_SIZE,
    + "%d-%d", slot->number, dup++);
    + if (len < SLOT_NAME_SIZE)
    + goto duplicate_name;
    + else
    + err("duplicate slot name overflow\n");
    + }
    err("pci_hp_register failed with error %d\n", retval);
    - if (retval == -EEXIST)
    - err("Failed to register slot because of name "
    - "collision. Try \'pciehp_slot_with_bus\' "
    - "module option.\n");
    goto error_info;
    }
    /* create additional sysfs entries */
    diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
    index ad27e9e..ab31f5b 100644
    --- a/drivers/pci/hotplug/pciehp_hpc.c
    +++ b/drivers/pci/hotplug/pciehp_hpc.c
    @@ -1030,15 +1030,6 @@ static void pcie_shutdown_notification(struct controller *ctrl)
    pciehp_free_irq(ctrl);
    }

    -static void make_slot_name(struct slot *slot)
    -{
    - if (pciehp_slot_with_bus)
    - snprintf(slot->name, SLOT_NAME_SIZE, "%04d_%04d",
    - slot->bus, slot->number);
    - else
    - snprintf(slot->name, SLOT_NAME_SIZE, "%d", slot->number);
    -}
    -
    static int pcie_init_slot(struct controller *ctrl)
    {
    struct slot *slot;
    @@ -1053,7 +1044,7 @@ static int pcie_init_slot(struct controller *ctrl)
    slot->device = ctrl->slot_device_offset + slot->hp_slot;
    slot->hpc_ops = ctrl->hpc_ops;
    slot->number = ctrl->first_slot;
    - make_slot_name(slot);
    + snprintf(slot->name, SLOT_NAME_SIZE, "%d", slot->number);
    mutex_init(&slot->lock);
    INIT_DELAYED_WORK(&slot->work, pciehp_queue_pushbutton_work);
    list_add(&slot->slot_list, &ctrl->slot_list);
    --
    1.6.0.rc0.g95f8

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: post 2.6.26 requires pciehp_slot_with_bus

    Jesse Barnes wrote:
    > On Monday, July 28, 2008 7:43 pm Kenji Kaneshige wrote:
    >>> Your systems don't have _RMV methods for the hotpluggable PCIe slots in
    >>> the DSDT? That's a shame; the Windows docs I found on PCIe hotplug
    >>> seemed to indicate that _RMV and _OSC (under Vista) were used to detect
    >>> whether a given slot was hot pluggable (I just googled for "windows pcie
    >>> hotplug" or something) so I was hoping that would be a reliable method...
    >>> Any other ideas? I'll go see if I can dig up some ExpressCard info.

    >> My systems don't have _RMV methods for the hot pluggable PCIe slots in the
    >> DSDT, but I don't think that's a shame. I suppose that the document you are
    >> referring describes how Windows handles ExpressCard slots. In my
    >> understanding, Hot Plug Surprise bit in the Slot Capabilities register is
    >> set to 1b on ExpressCard slots, and I believe that ACPI _RVM method is for
    >> the device that only supports surprise-style removal. I think this is why
    >> your system implements _RMV method for slots.

    >
    > Yeah, that may be. The document wasn't very clear; I was hoping that
    > something simple would be available.
    >
    >> On the other hand, hot pluggable slots on my servers are *not* ExpressCard
    >> slots, and all of them have Power Controller instead of surprise-style
    >> removal (Hot Plug Surprise bit in the Slot Capabilities register is set to
    >> 0b). So I believe there is no reason to implement _RMV methods for the hot
    >> pluggable PCIe slots on my systems.
    >>
    >> Here is an idea. How about using _RMV method to determine whether a given
    >> slot is actually hot pluggable when Hot Plug Surprise bit in the Slot
    >> Capabilities register is set to 1b on the slot? This is based on a little
    >> rough assumption that all PCIe slots that support surprise-style removal
    >> have _RMV method, though. Does this work for you?

    >
    > It's worth a try. We need *some* sort of better method to detect hot
    > pluggable slots...


    OK. I'll try to make a patch.

    According to PCI Express and PCI firmware spec, I think Hot Plug Capable
    bit in the Slot Capabilities register and ACPI _OSC are enough to detect
    hot pluggable slots. But I might be missing something especially about
    ExpressCard, or BIOS is just broken...

    Thanks,
    Kenji Kaneshige


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. [PATCH 2/2] shpchp: Rename duplicate slot name N as N-1, N-2, N-M...

    Commit ef0ff95f136f0f2d035667af5d18b824609de320 introduces the
    shpchp_slot_with_bus module parameter, which was intended to help
    work around broken firmware that assigns the same name to multiple
    slots.

    Commit b3bd307c628af2f0a581c42d5d7e4bcdbbf64b6a tells the user to
    use the above parameter in the event of a name collision.

    This approach is sub-optimal because it requires too much work from
    the user.

    Instead, let's rename the slot on behalf of the user. If firmware
    assigns the name N to multiple slots, then:

    The first registered slot is assigned N
    The second registered slot is assigned N-1
    The third registered slot is assigned N-2
    The Mth registered slot becomes N-M

    In the event we overflow the slot->name parameter, we report an
    error to the user.

    Signed-off-by: Alex Chiang
    ---
    drivers/pci/hotplug/shpchp_core.c | 34 +++++++++++++++-------------------
    1 files changed, 15 insertions(+), 19 deletions(-)

    diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
    index a8cbd03..cc38615 100644
    --- a/drivers/pci/hotplug/shpchp_core.c
    +++ b/drivers/pci/hotplug/shpchp_core.c
    @@ -39,7 +39,6 @@
    int shpchp_debug;
    int shpchp_poll_mode;
    int shpchp_poll_time;
    -static int shpchp_slot_with_bus;
    struct workqueue_struct *shpchp_wq;

    #define DRIVER_VERSION "0.4"
    @@ -53,11 +52,9 @@ MODULE_LICENSE("GPL");
    module_param(shpchp_debug, bool, 0644);
    module_param(shpchp_poll_mode, bool, 0644);
    module_param(shpchp_poll_time, int, 0644);
    -module_param(shpchp_slot_with_bus, bool, 0644);
    MODULE_PARM_DESC(shpchp_debug, "Debugging mode enabled or not");
    MODULE_PARM_DESC(shpchp_poll_mode, "Using polling mechanism for hot-plug events or not");
    MODULE_PARM_DESC(shpchp_poll_time, "Polling mechanism frequency, in seconds");
    -MODULE_PARM_DESC(shpchp_slot_with_bus, "Use bus number in the slot name");

    #define SHPC_MODULE_NAME "shpchp"

    @@ -99,23 +96,13 @@ static void release_slot(struct hotplug_slot *hotplug_slot)
    kfree(slot);
    }

    -static void make_slot_name(struct slot *slot)
    -{
    - if (shpchp_slot_with_bus)
    - snprintf(slot->hotplug_slot->name, SLOT_NAME_SIZE, "%04d_%04d",
    - slot->bus, slot->number);
    - else
    - snprintf(slot->hotplug_slot->name, SLOT_NAME_SIZE, "%d",
    - slot->number);
    -}
    -
    static int init_slots(struct controller *ctrl)
    {
    struct slot *slot;
    struct hotplug_slot *hotplug_slot;
    struct hotplug_slot_info *info;
    int retval = -ENOMEM;
    - int i;
    + int i, len, dup = 1;

    for (i = 0; i < ctrl->num_slots; i++) {
    slot = kzalloc(sizeof(*slot), GFP_KERNEL);
    @@ -146,7 +133,7 @@ static int init_slots(struct controller *ctrl)
    /* register this slot with the hotplug pci core */
    hotplug_slot->private = slot;
    hotplug_slot->release = &release_slot;
    - make_slot_name(slot);
    + snprintf(slot->name, SLOT_NAME_SIZE, "%d", slot->number);
    hotplug_slot->ops = &shpchp_hotplug_slot_ops;

    get_power_status(hotplug_slot, &info->power_status);
    @@ -157,14 +144,23 @@ static int init_slots(struct controller *ctrl)
    dbg("Registering bus=%x dev=%x hp_slot=%x sun=%x "
    "slot_device_offset=%x\n", slot->bus, slot->device,
    slot->hp_slot, slot->number, ctrl->slot_device_offset);
    +duplicate_name:
    retval = pci_hp_register(slot->hotplug_slot,
    ctrl->pci_dev->subordinate, slot->device);
    if (retval) {
    + /*
    + * If slot N already exists, we'll try to create
    + * slot N-1, N-2 ... N-M, until we overflow.
    + */
    + if (retval == -EEXIST) {
    + len = snprintf(slot->name, SLOT_NAME_SIZE,
    + "%d-%d", slot->number, dup++);
    + if (len < SLOT_NAME_SIZE)
    + goto duplicate_name;
    + else
    + err("duplicate slot name overflow\n");
    + }
    err("pci_hp_register failed with error %d\n", retval);
    - if (retval == -EEXIST)
    - err("Failed to register slot because of name "
    - "collision. Try \'shpchp_slot_with_bus\' "
    - "module option.\n");
    goto error_info;
    }

    --
    1.6.0.rc0.g95f8

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M...

    Tested-by & Acked-by: Kenji Kaneshige

    Thnaks,
    Kenji Kaneshige



    Alex Chiang wrote:
    > Commit 3800345f723fd130d50434d4717b99d4a9f383c8 introduces the
    > pciehp_slot_with_bus module parameter, which was intended to help
    > work around broken firmware that assigns the same name to multiple
    > slots.
    >
    > Commit 9e4f2e8d4ddb04ad16a3828cd9a369a5a5287009 tells the user to
    > use the above parameter in the event of a name collision.
    >
    > This approach is sub-optimal because it requires too much work from
    > the user.
    >
    > Instead, let's rename the slot on behalf of the user. If firmware
    > assigns the name N to multiple slots, then:
    >
    > The first registered slot is assigned N
    > The second registered slot is assigned N-1
    > The third registered slot is assigned N-2
    > The Mth registered slot becomes N-M
    >
    > In the event we overflow the slot->name parameter, we report an
    > error to the user.
    >
    > Signed-off-by: Alex Chiang
    > ---
    > drivers/pci/hotplug/pciehp.h | 1 -
    > drivers/pci/hotplug/pciehp_core.c | 21 ++++++++++++++-------
    > drivers/pci/hotplug/pciehp_hpc.c | 11 +----------
    > 3 files changed, 15 insertions(+), 18 deletions(-)
    >
    > diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
    > index e3a1e7e..9e6cec6 100644
    > --- a/drivers/pci/hotplug/pciehp.h
    > +++ b/drivers/pci/hotplug/pciehp.h
    > @@ -43,7 +43,6 @@ extern int pciehp_poll_mode;
    > extern int pciehp_poll_time;
    > extern int pciehp_debug;
    > extern int pciehp_force;
    > -extern int pciehp_slot_with_bus;
    > extern struct workqueue_struct *pciehp_wq;
    >
    > #define dbg(format, arg...) \
    > diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
    > index 3677495..4fd5355 100644
    > --- a/drivers/pci/hotplug/pciehp_core.c
    > +++ b/drivers/pci/hotplug/pciehp_core.c
    > @@ -41,7 +41,6 @@ int pciehp_debug;
    > int pciehp_poll_mode;
    > int pciehp_poll_time;
    > int pciehp_force;
    > -int pciehp_slot_with_bus;
    > struct workqueue_struct *pciehp_wq;
    >
    > #define DRIVER_VERSION "0.4"
    > @@ -56,12 +55,10 @@ module_param(pciehp_debug, bool, 0644);
    > module_param(pciehp_poll_mode, bool, 0644);
    > module_param(pciehp_poll_time, int, 0644);
    > module_param(pciehp_force, bool, 0644);
    > -module_param(pciehp_slot_with_bus, bool, 0644);
    > MODULE_PARM_DESC(pciehp_debug, "Debugging mode enabled or not");
    > MODULE_PARM_DESC(pciehp_poll_mode, "Using polling mechanism for hot-plug events or not");
    > MODULE_PARM_DESC(pciehp_poll_time, "Polling mechanism frequency, in seconds");
    > MODULE_PARM_DESC(pciehp_force, "Force pciehp, even if _OSC and OSHP are missing");
    > -MODULE_PARM_DESC(pciehp_slot_with_bus, "Use bus number in the slot name");
    >
    > #define PCIE_MODULE_NAME "pciehp"
    >
    > @@ -194,6 +191,7 @@ static int init_slots(struct controller *ctrl)
    > struct slot *slot;
    > struct hotplug_slot *hotplug_slot;
    > struct hotplug_slot_info *info;
    > + int len, dup = 1;
    > int retval = -ENOMEM;
    >
    > list_for_each_entry(slot, &ctrl->slot_list, slot_list) {
    > @@ -220,15 +218,24 @@ static int init_slots(struct controller *ctrl)
    > dbg("Registering bus=%x dev=%x hp_slot=%x sun=%x "
    > "slot_device_offset=%x\n", slot->bus, slot->device,
    > slot->hp_slot, slot->number, ctrl->slot_device_offset);
    > +duplicate_name:
    > retval = pci_hp_register(hotplug_slot,
    > ctrl->pci_dev->subordinate,
    > slot->device);
    > if (retval) {
    > + /*
    > + * If slot N already exists, we'll try to create
    > + * slot N-1, N-2 ... N-M, until we overflow.
    > + */
    > + if (retval == -EEXIST) {
    > + len = snprintf(slot->name, SLOT_NAME_SIZE,
    > + "%d-%d", slot->number, dup++);
    > + if (len < SLOT_NAME_SIZE)
    > + goto duplicate_name;
    > + else
    > + err("duplicate slot name overflow\n");
    > + }
    > err("pci_hp_register failed with error %d\n", retval);
    > - if (retval == -EEXIST)
    > - err("Failed to register slot because of name "
    > - "collision. Try \'pciehp_slot_with_bus\' "
    > - "module option.\n");
    > goto error_info;
    > }
    > /* create additional sysfs entries */
    > diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
    > index ad27e9e..ab31f5b 100644
    > --- a/drivers/pci/hotplug/pciehp_hpc.c
    > +++ b/drivers/pci/hotplug/pciehp_hpc.c
    > @@ -1030,15 +1030,6 @@ static void pcie_shutdown_notification(struct controller *ctrl)
    > pciehp_free_irq(ctrl);
    > }
    >
    > -static void make_slot_name(struct slot *slot)
    > -{
    > - if (pciehp_slot_with_bus)
    > - snprintf(slot->name, SLOT_NAME_SIZE, "%04d_%04d",
    > - slot->bus, slot->number);
    > - else
    > - snprintf(slot->name, SLOT_NAME_SIZE, "%d", slot->number);
    > -}
    > -
    > static int pcie_init_slot(struct controller *ctrl)
    > {
    > struct slot *slot;
    > @@ -1053,7 +1044,7 @@ static int pcie_init_slot(struct controller *ctrl)
    > slot->device = ctrl->slot_device_offset + slot->hp_slot;
    > slot->hpc_ops = ctrl->hpc_ops;
    > slot->number = ctrl->first_slot;
    > - make_slot_name(slot);
    > + snprintf(slot->name, SLOT_NAME_SIZE, "%d", slot->number);
    > mutex_init(&slot->lock);
    > INIT_DELAYED_WORK(&slot->work, pciehp_queue_pushbutton_work);
    > list_add(&slot->slot_list, &ctrl->slot_list);



    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [PATCH 2/2] shpchp: Rename duplicate slot name N as N-1, N-2, N-M...

    Tested-by & Acked-by: Kenji Kaneshige

    Thnaks,
    Kenji Kaneshige


    Alex Chiang wrote:
    > Commit ef0ff95f136f0f2d035667af5d18b824609de320 introduces the
    > shpchp_slot_with_bus module parameter, which was intended to help
    > work around broken firmware that assigns the same name to multiple
    > slots.
    >
    > Commit b3bd307c628af2f0a581c42d5d7e4bcdbbf64b6a tells the user to
    > use the above parameter in the event of a name collision.
    >
    > This approach is sub-optimal because it requires too much work from
    > the user.
    >
    > Instead, let's rename the slot on behalf of the user. If firmware
    > assigns the name N to multiple slots, then:
    >
    > The first registered slot is assigned N
    > The second registered slot is assigned N-1
    > The third registered slot is assigned N-2
    > The Mth registered slot becomes N-M
    >
    > In the event we overflow the slot->name parameter, we report an
    > error to the user.
    >
    > Signed-off-by: Alex Chiang
    > ---
    > drivers/pci/hotplug/shpchp_core.c | 34 +++++++++++++++-------------------
    > 1 files changed, 15 insertions(+), 19 deletions(-)
    >
    > diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
    > index a8cbd03..cc38615 100644
    > --- a/drivers/pci/hotplug/shpchp_core.c
    > +++ b/drivers/pci/hotplug/shpchp_core.c
    > @@ -39,7 +39,6 @@
    > int shpchp_debug;
    > int shpchp_poll_mode;
    > int shpchp_poll_time;
    > -static int shpchp_slot_with_bus;
    > struct workqueue_struct *shpchp_wq;
    >
    > #define DRIVER_VERSION "0.4"
    > @@ -53,11 +52,9 @@ MODULE_LICENSE("GPL");
    > module_param(shpchp_debug, bool, 0644);
    > module_param(shpchp_poll_mode, bool, 0644);
    > module_param(shpchp_poll_time, int, 0644);
    > -module_param(shpchp_slot_with_bus, bool, 0644);
    > MODULE_PARM_DESC(shpchp_debug, "Debugging mode enabled or not");
    > MODULE_PARM_DESC(shpchp_poll_mode, "Using polling mechanism for hot-plug events or not");
    > MODULE_PARM_DESC(shpchp_poll_time, "Polling mechanism frequency, in seconds");
    > -MODULE_PARM_DESC(shpchp_slot_with_bus, "Use bus number in the slot name");
    >
    > #define SHPC_MODULE_NAME "shpchp"
    >
    > @@ -99,23 +96,13 @@ static void release_slot(struct hotplug_slot *hotplug_slot)
    > kfree(slot);
    > }
    >
    > -static void make_slot_name(struct slot *slot)
    > -{
    > - if (shpchp_slot_with_bus)
    > - snprintf(slot->hotplug_slot->name, SLOT_NAME_SIZE, "%04d_%04d",
    > - slot->bus, slot->number);
    > - else
    > - snprintf(slot->hotplug_slot->name, SLOT_NAME_SIZE, "%d",
    > - slot->number);
    > -}
    > -
    > static int init_slots(struct controller *ctrl)
    > {
    > struct slot *slot;
    > struct hotplug_slot *hotplug_slot;
    > struct hotplug_slot_info *info;
    > int retval = -ENOMEM;
    > - int i;
    > + int i, len, dup = 1;
    >
    > for (i = 0; i < ctrl->num_slots; i++) {
    > slot = kzalloc(sizeof(*slot), GFP_KERNEL);
    > @@ -146,7 +133,7 @@ static int init_slots(struct controller *ctrl)
    > /* register this slot with the hotplug pci core */
    > hotplug_slot->private = slot;
    > hotplug_slot->release = &release_slot;
    > - make_slot_name(slot);
    > + snprintf(slot->name, SLOT_NAME_SIZE, "%d", slot->number);
    > hotplug_slot->ops = &shpchp_hotplug_slot_ops;
    >
    > get_power_status(hotplug_slot, &info->power_status);
    > @@ -157,14 +144,23 @@ static int init_slots(struct controller *ctrl)
    > dbg("Registering bus=%x dev=%x hp_slot=%x sun=%x "
    > "slot_device_offset=%x\n", slot->bus, slot->device,
    > slot->hp_slot, slot->number, ctrl->slot_device_offset);
    > +duplicate_name:
    > retval = pci_hp_register(slot->hotplug_slot,
    > ctrl->pci_dev->subordinate, slot->device);
    > if (retval) {
    > + /*
    > + * If slot N already exists, we'll try to create
    > + * slot N-1, N-2 ... N-M, until we overflow.
    > + */
    > + if (retval == -EEXIST) {
    > + len = snprintf(slot->name, SLOT_NAME_SIZE,
    > + "%d-%d", slot->number, dup++);
    > + if (len < SLOT_NAME_SIZE)
    > + goto duplicate_name;
    > + else
    > + err("duplicate slot name overflow\n");
    > + }
    > err("pci_hp_register failed with error %d\n", retval);
    > - if (retval == -EEXIST)
    > - err("Failed to register slot because of name "
    > - "collision. Try \'shpchp_slot_with_bus\' "
    > - "module option.\n");
    > goto error_info;
    > }
    >



    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: post 2.6.26 requires pciehp_slot_with_bus

    Alex Chiang wrote:
    > * Kenji Kaneshige :
    >> Matthew Wilcox wrote:
    >>> So we need a way to find if there's already a slot of this
    >>> name. I don't see a kobject routine to do that. Maybe we can
    >>> do it internally to the pci slot code.

    >
    > pci_hp_register already does this with get_slot_from_name().
    >
    >>> Then we need to pick a new name for the kobject if it does
    >>> collide. My suggestion is that the second time we find an
    >>> object named "2", we call it "2dup1" (the third time "2dup2",
    >>> etc.) Other opinions I've seen include "2a", "2b", ... or
    >>> "2-1", "2-2", ... or "2-brokenfw1", "2-brokenfw2".

    >> That looks quite better than using bus number.

    >
    > I went with:
    >
    > - first slot to register gets "2"
    > - second slot to register gets "2-1"
    > - Mth slot to register gets "2-M"
    >
    > At first, I thought it would have been better to put this logic
    > inside of pci_hp_register, since it knows about the collision,
    > and could just fix stuff up for the caller.
    >
    > However, the problem is that each hotplug driver can have a
    > different length for "name", and it got messy quickly.
    >
    > So, I just patched the two drivers that are known to be
    > problematic.
    >
    > Two patches follow, against 2.6.27-rc1.
    >
    > Compile tested only -- I don't have hardware to replicate this.
    >
    > I'd say they're somewhere between RFC and requested for
    > inclusion. I'm certainly not tied to them, just trying to show
    > some code to implement the approach described above. If we decide
    > that looking at _RMV + other bits is the way to go, then I'm fine
    > with that.
    >
    > It would be great if Pierre and Kenji-san could try them out.


    Thank you for patches, Alex-san!

    I've reviewed those patches and tested them on my ia64 machine
    that have both shpc and pcie hotplug slots. Your patch looks
    good.

    As you mentioned, we are considering the problem also from the
    view point of slot detection. But I think your patch is needed
    regardless of that because there might be platforms whose slots
    are detected properly but firmware assigns the physical slot
    number wrongly. I think Alex's patch should go to mainline.

    P.S.: I found a possible improvement, though it is not a big
    problem and we don't not need to fix it soon. I'd like to tell
    you about it just in case. Current pci_hp_register() checks if
    name is duplicated first, before checking if another hotplug
    driver is already registered to the slot. So, if shpchp/pciehp
    driver tries to register hotplug slot that is already registered
    by the other hotplug driver (e.g. acpiphp) with the same name,
    shpchp/pciehp driver will do as follows:

    (1) shpchp/pciehp call pci_hp_register()
    (2) pci_hp_register() returns -EEXIST
    (3) shpchp/pciehp call pci_hp_register() with other name ("M-1")
    (4) pci_hp_register() returns -EBUSY

    if pci_hp_register() checked if another hotplug driver is already
    registered first, step (2) and (3) could be removed.

    Thanks,
    Kenji Kaneshige

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: post 2.6.26 requires pciehp_slot_with_bus

    * Kenji Kaneshige :
    > Thank you for patches, Alex-san!
    >
    > I've reviewed those patches and tested them on my ia64 machine
    > that have both shpc and pcie hotplug slots. Your patch looks
    > good.


    Thank you for reviewing and testing.

    > As you mentioned, we are considering the problem also from the
    > view point of slot detection. But I think your patch is needed
    > regardless of that because there might be platforms whose slots
    > are detected properly but firmware assigns the physical slot
    > number wrongly. I think Alex's patch should go to mainline.


    That is a good point.

    > P.S.: I found a possible improvement, though it is not a big
    > problem and we don't not need to fix it soon. I'd like to tell
    > you about it just in case. Current pci_hp_register() checks if
    > name is duplicated first, before checking if another hotplug
    > driver is already registered to the slot. So, if shpchp/pciehp
    > driver tries to register hotplug slot that is already registered
    > by the other hotplug driver (e.g. acpiphp) with the same name,
    > shpchp/pciehp driver will do as follows:
    >
    > (1) shpchp/pciehp call pci_hp_register()
    > (2) pci_hp_register() returns -EEXIST
    > (3) shpchp/pciehp call pci_hp_register() with other name ("M-1")
    > (4) pci_hp_register() returns -EBUSY
    >
    > if pci_hp_register() checked if another hotplug driver is already
    > registered first, step (2) and (3) could be removed.


    Thanks, that seems pretty easy to do.

    Would you mind testing this patch as well? You should probably
    apply it on top of the other two patches to see how all three
    patches interact.

    Thanks!

    /ac


    From: Alex Chiang
    Subject: [PATCH] PCI hotplug: check for claimed slot before duplicate named slot

    Kenji Kaneshige observes that:

    If shpchp/pciehp driver tries to register hotplug slot that is
    already registered by the other hotplug driver (e.g. acpiphp) with
    the same name, shpchp/pciehp driver will do as follows:

    (1) shpchp/pciehp call pci_hp_register()
    (2) pci_hp_register() returns -EEXIST
    (3) shpchp/pciehp call pci_hp_register() with other name ("M-1")
    (4) pci_hp_register() returns -EBUSY

    If pci_hp_register() checked if another hotplug driver is already
    registered first, step (2) and (3) could be removed.

    This patch does not prevent the *same* driver from attempting
    to register multiple slots with the same name (on systems with
    broken firmware). For that situation, we still need to detect
    a name collision and return -EEXIST if so.

    Signed-off-by: Alex Chiang
    ---
    drivers/pci/hotplug/pci_hotplug_core.c | 11 ++++++-----
    1 files changed, 6 insertions(+), 5 deletions(-)

    diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
    index 5f85b1b..9c379b6 100644
    --- a/drivers/pci/hotplug/pci_hotplug_core.c
    +++ b/drivers/pci/hotplug/pci_hotplug_core.c
    @@ -568,10 +568,6 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
    return -EINVAL;
    }

    - /* Check if we have already registered a slot with the same name. */
    - if (get_slot_from_name(slot->name))
    - return -EEXIST;
    -
    /*
    * No problems if we call this interface from both ACPI_PCI_SLOT
    * driver and call it here again. If we've already created the
    @@ -587,6 +583,12 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
    return -EBUSY;
    }

    + /* Check if we have already registered a slot with the same name. */
    + if (get_slot_from_name(slot->name)) {
    + pci_destroy_slot(pci_slot);
    + return -EEXIST;
    + }
    +
    slot->pci_slot = pci_slot;
    pci_slot->hotplug = slot;

    @@ -609,7 +611,6 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
    kobject_uevent(&pci_slot->kobj, KOBJ_ADD);
    dbg("Added slot %s to the list\n", slot->name);

    -
    return result;
    }

    --
    1.6.0.rc0.g95f8

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: post 2.6.26 requires pciehp_slot_with_bus

    Alex Chiang wrote:
    > * Kenji Kaneshige :
    >> Thank you for patches, Alex-san!
    >>
    >> I've reviewed those patches and tested them on my ia64 machine
    >> that have both shpc and pcie hotplug slots. Your patch looks
    >> good.

    >
    > Thank you for reviewing and testing.
    >
    >> As you mentioned, we are considering the problem also from the
    >> view point of slot detection. But I think your patch is needed
    >> regardless of that because there might be platforms whose slots
    >> are detected properly but firmware assigns the physical slot
    >> number wrongly. I think Alex's patch should go to mainline.

    >
    > That is a good point.
    >
    >> P.S.: I found a possible improvement, though it is not a big
    >> problem and we don't not need to fix it soon. I'd like to tell
    >> you about it just in case. Current pci_hp_register() checks if
    >> name is duplicated first, before checking if another hotplug
    >> driver is already registered to the slot. So, if shpchp/pciehp
    >> driver tries to register hotplug slot that is already registered
    >> by the other hotplug driver (e.g. acpiphp) with the same name,
    >> shpchp/pciehp driver will do as follows:
    >>
    >> (1) shpchp/pciehp call pci_hp_register()
    >> (2) pci_hp_register() returns -EEXIST
    >> (3) shpchp/pciehp call pci_hp_register() with other name ("M-1")
    >> (4) pci_hp_register() returns -EBUSY
    >>
    >> if pci_hp_register() checked if another hotplug driver is already
    >> registered first, step (2) and (3) could be removed.

    >
    > Thanks, that seems pretty easy to do.
    >
    > Would you mind testing this patch as well? You should probably
    > apply it on top of the other two patches to see how all three
    > patches interact.
    >
    > Thanks!
    >
    > /ac
    >
    >
    > From: Alex Chiang
    > Subject: [PATCH] PCI hotplug: check for claimed slot before duplicate named slot
    >
    > Kenji Kaneshige observes that:
    >
    > If shpchp/pciehp driver tries to register hotplug slot that is
    > already registered by the other hotplug driver (e.g. acpiphp) with
    > the same name, shpchp/pciehp driver will do as follows:
    >
    > (1) shpchp/pciehp call pci_hp_register()
    > (2) pci_hp_register() returns -EEXIST
    > (3) shpchp/pciehp call pci_hp_register() with other name ("M-1")
    > (4) pci_hp_register() returns -EBUSY
    >
    > If pci_hp_register() checked if another hotplug driver is already
    > registered first, step (2) and (3) could be removed.
    >
    > This patch does not prevent the *same* driver from attempting
    > to register multiple slots with the same name (on systems with
    > broken firmware). For that situation, we still need to detect
    > a name collision and return -EEXIST if so.
    >
    > Signed-off-by: Alex Chiang
    > ---
    > drivers/pci/hotplug/pci_hotplug_core.c | 11 ++++++-----
    > 1 files changed, 6 insertions(+), 5 deletions(-)
    >
    > diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
    > index 5f85b1b..9c379b6 100644
    > --- a/drivers/pci/hotplug/pci_hotplug_core.c
    > +++ b/drivers/pci/hotplug/pci_hotplug_core.c
    > @@ -568,10 +568,6 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
    > return -EINVAL;
    > }
    >
    > - /* Check if we have already registered a slot with the same name. */
    > - if (get_slot_from_name(slot->name))
    > - return -EEXIST;
    > -
    > /*
    > * No problems if we call this interface from both ACPI_PCI_SLOT
    > * driver and call it here again. If we've already created the
    > @@ -587,6 +583,12 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
    > return -EBUSY;
    > }
    >
    > + /* Check if we have already registered a slot with the same name. */
    > + if (get_slot_from_name(slot->name)) {
    > + pci_destroy_slot(pci_slot);
    > + return -EEXIST;
    > + }
    > +
    > slot->pci_slot = pci_slot;
    > pci_slot->hotplug = slot;
    >
    > @@ -609,7 +611,6 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
    > kobject_uevent(&pci_slot->kobj, KOBJ_ADD);
    > dbg("Added slot %s to the list\n", slot->name);
    >
    > -
    > return result;
    > }
    >


    Unfortunately, we can't simply move the following check after pci_create_slot().

    > - /* Check if we have already registered a slot with the same name. */
    > - if (get_slot_from_name(slot->name))
    > - return -EEXIST;
    > -


    With this change, kobject_init_and_add() called in pci_create_slot() will
    show stack trace if a hotplug driver attempts to register multiple slot with
    the same name. That is, stack trace will be shown on the platform that wrongly
    assing the physical slot number to multiple slots. I'm very sorry, but I don't
    have enough time to consider how to fix it today.

    Thanks,
    Kenji Kaneshige


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2