From: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
To: Alex Chiang <achiang@hp.com>, Matthew Wilcox <matthew@wil.cx>,
Pierre Ossman <drzeus-list@drzeus.cx>,
Jesse Barnes <jbarnes@virtuousgeek.org>,
Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-pci@vger.kernel.org
Subject: Re: post 2.6.26 requires pciehp_slot_with_bus
Date: Fri, 25 Jul 2008 14:38:38 +0900 [thread overview]
Message-ID: <488966DE.8020307@jp.fujitsu.com> (raw)
In-Reply-To: <20080725044234.GA18511@ldl.fc.hp.com>
Alex Chiang wrote:
> * Matthew Wilcox <matthew@wil.cx>:
>> On Fri, Jul 25, 2008 at 01:29:16AM +0200, Pierre Ossman wrote:
>>> On Thu, 24 Jul 2008 17:08:27 -0600
>>> Alex Chiang <achiang@hp.com> 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.
>
>> 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.
>
> Well, we have this code in pci_hp_register:
>
> /* Check if we have already registered a slot with the same name. */
> if (get_slot_from_name(slot->name))
> return -EEXIST;
>
>> 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".
>>
>> I'm at OLS this week, so no patch from me.
>
> It should be pretty easy for pci_hp_register() to fix up the name
> in the event of a collision.
>
> The hard part is figuring out a convention that we can all agree
> on. ;) I've no strong feelings here, but of the options
> presented, I lean towards "2a", "2b" or "2-1", "2-2".
>
Only my hope is I don't want to look "2a" or "2-1" on normal platform.
It should be "2" on normal platform.
I'd like to try to make a patch, but it will be middle of next week
because of other jobs. Is that ok?
Thanks,
Kenji Kaneshige
next prev parent reply other threads:[~2008-07-25 5:39 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-24 11:47 post 2.6.26 requires pciehp_slot_with_bus Pierre Ossman
2008-07-24 12:38 ` Kenji Kaneshige
2008-07-24 20:39 ` Pierre Ossman
2008-07-24 21:07 ` Jesse Barnes
2008-07-24 21:51 ` Pierre Ossman
2008-07-24 22:06 ` Jesse Barnes
2008-07-24 22:29 ` Alex Chiang
2008-07-24 22:49 ` Pierre Ossman
2008-07-24 23:08 ` Alex Chiang
2008-07-24 23:29 ` Pierre Ossman
2008-07-25 3:29 ` Matthew Wilcox
2008-07-25 4:42 ` Alex Chiang
2008-07-25 5:38 ` Kenji Kaneshige [this message]
2008-07-25 11:18 ` Matthew Wilcox
2008-07-28 18:05 ` Greg KH
2008-07-25 4:57 ` Kenji Kaneshige
2008-07-30 2:38 ` Alex Chiang
2008-07-30 2:42 ` [PATCH 1/2] pciehp: Rename duplicate slot name N as N-1, N-2, N-M Alex Chiang
2008-07-31 10:32 ` Kenji Kaneshige
2008-07-30 2:44 ` [PATCH 2/2] shpchp: " Alex Chiang
2008-07-31 10:32 ` Kenji Kaneshige
2008-07-31 10:31 ` post 2.6.26 requires pciehp_slot_with_bus Kenji Kaneshige
2008-07-31 15:47 ` Alex Chiang
2008-08-01 8:43 ` Kenji Kaneshige
2008-07-25 8:53 ` Kenji Kaneshige
2008-07-25 11:40 ` Matthew Wilcox
2008-07-28 7:21 ` Kenji Kaneshige
2008-07-25 4:50 ` Kenji Kaneshige
2008-07-25 22:18 ` Jesse Barnes
2008-07-26 1:16 ` Matthew Wilcox
2008-07-28 8:58 ` Kenji Kaneshige
2008-07-28 8:44 ` Kenji Kaneshige
2008-07-28 16:16 ` Jesse Barnes
2008-07-29 2:43 ` Kenji Kaneshige
2008-07-29 15:14 ` Jesse Barnes
2008-07-30 2:44 ` Kenji Kaneshige
2008-07-28 16:57 ` Matthew Wilcox
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=488966DE.8020307@jp.fujitsu.com \
--to=kaneshige.kenji@jp.fujitsu.com \
--cc=achiang@hp.com \
--cc=drzeus-list@drzeus.cx \
--cc=jbarnes@virtuousgeek.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=matthew@wil.cx \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox