From: Kenji Kaneshige <kaneshige.kenji@jp•fujitsu.com>
To: Felix Radensky <felix@embedded-sol•com>
Cc: linux-pci@vger•kernel.org, Alex Chiang <achiang@hp•com>,
"linuxppc-dev@ozlabs•org" <linuxppc-dev@ozlabs•org>
Subject: Re: Problem with PCI bus rescan on 460EX
Date: Mon, 15 Mar 2010 18:00:31 +0900 [thread overview]
Message-ID: <4B9DF72F.3040105@jp.fujitsu.com> (raw)
In-Reply-To: <4B9DCF01.4050709@embedded-sol.com>
Felix Radensky wrote:
> Hello Kenji-san,
>
> Kenji Kaneshige wrote:
>> Felix Radensky wrote:
>>> Hello Kenji-san,
>>>
>>> Kenji Kaneshige wrote:
>>>> I'm not sure, but I guess pci_setup_bridge() didn't update IO
>>>> base/limit
>>>> and Mem base/limit of the bridge (0000:00:02.0) because of the
>>>> following
>>>> lines.
>>>>
>>>> static void pci_setup_bridge(struct pci_bus *bus)
>>>> {
>>>> struct pci_dev *bridge = bus->self;
>>>> struct resource *res;
>>>> struct pci_bus_region region;
>>>> u32 l, bu, lu, io_upper16;
>>>>
>>>> if (pci_is_enabled(bridge))
>>>> <===============================
>>>> return; <===============================
>>>>
>>>> dev_info(&bridge->dev, "PCI bridge to [bus %02x-%02x]\n",
>>>> bus->secondary, bus->subordinate);
>>>> ...
>>>>
>>>> It seems the bridge was already enabled by
>>>> pci_assign_unassigned_resources()
>>>> at boot time. Does removing those lines make any change?
>>>>
>>>
>>> Yes, with these lines removed bridge memory window is properly
>>> allocated.
>>
>> These lines are to prevent updating IO or memory windows when there are
>> some devices working behind the bridge. So please note that removing
>> these lines is just for debugging.
>>
>>
>>> For some reason bridge memory is disabled, but if I enable it via
>>> setpci, and
>>> also enable device memory, then everything works fine. If the system
>>> is booted
>>> when device behind the bridge is plugged in, bridge memory is enabled.
>>>
>>
>> Maybe because of the following lines, I guess.
>>
>> void pci_enable_bridges(struct pci_bus *bus)
>> {
>> struct pci_dev *dev;
>> int retval;
>>
>> list_for_each_entry(dev, &bus->devices, bus_list) {
>> if (dev->subordinate) {
>> if (!pci_is_enabled(dev)) { <=======
>> retval = pci_enable_device(dev);<=======
>> pci_set_master(dev); <=======
>> }
>> ...
>>
>>
>
> Yes, but removing this check is not enough. The bridge is enabled after
> first scan, but it's
> memory is disabled. So simply calling pci_enable_device() will not
> help, it will return without
> enabling memory. I had to call pci_disable_device() before
> pci_enable_device() to make things
> work.
>> One possible cause is that pcibios_enable_device() for the bridge didn't
>> return any error even though it didn't work properly (ex. cannot access
>> to the command register, and so on) on your platform when there is no
>> device behind the bridge. But it is just my guess.
> Should pcibios_enable_device() return an error if it fails to enable
> bridge memory ?
>
I think the device is expected to be ready to work if pci_enable_device()
returns without error. So I think pci_enable_device() should return an
error if it fails to enable the device (device is not ready to work). In
this case, detecting your bridge's failure seems PPC specific to me. So I
thought pcibios_enable_device() was the right to return an error. If
pcibios_enable_device() returned an error, pci_dev->enable_cnt would
decremented by pci_enable_device() (like pci_disable_device() does) and
this problem would not happen.
On the other hand, as Ben suggested, handling this by specific hot-plug
driver would be one of the other candidate to fix the problem.
Thanks,
Kenji Kaneshige
next prev parent reply other threads:[~2010-03-15 9:01 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <4B8E6FA3.70503@embedded-sol.com>
[not found] ` <20100310225100.GB27324@ldl.fc.hp.com>
2010-03-11 7:45 ` Problem with PCI bus rescan on 460EX Felix Radensky
2010-03-11 20:32 ` Benjamin Herrenschmidt
2010-03-11 21:41 ` Felix Radensky
2010-03-11 21:49 ` Benjamin Herrenschmidt
2010-03-11 7:50 ` Felix Radensky
2010-03-12 9:22 ` Kenji Kaneshige
2010-03-12 23:04 ` Felix Radensky
2010-03-15 5:39 ` Kenji Kaneshige
2010-03-15 5:46 ` Benjamin Herrenschmidt
2010-03-15 5:54 ` Benjamin Herrenschmidt
2010-03-15 6:09 ` Felix Radensky
2010-03-15 9:00 ` Kenji Kaneshige [this message]
2010-03-15 11:23 ` Felix Radensky
2010-03-16 5:40 ` Kenji Kaneshige
2010-03-16 8:39 ` Felix Radensky
2010-03-16 21:40 ` Felix Radensky
2010-03-17 1:03 ` Kenji Kaneshige
2010-03-17 7:38 ` Felix Radensky
2010-03-17 7:47 ` Benjamin Herrenschmidt
2010-03-17 7:57 ` Felix Radensky
2010-03-17 23:04 ` Benjamin Herrenschmidt
2010-03-18 0:09 ` Benjamin Herrenschmidt
2010-03-28 9:13 ` Felix Radensky
2010-03-28 9:56 ` Benjamin Herrenschmidt
2010-03-28 13:07 ` Felix Radensky
2010-03-29 0:05 ` Yinghai Lu
2010-03-29 7:01 ` Kenji Kaneshige
2010-03-29 7:35 ` Felix Radensky
2010-03-02 13:27 Felix Radensky
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=4B9DF72F.3040105@jp.fujitsu.com \
--to=kaneshige.kenji@jp$(echo .)fujitsu.com \
--cc=achiang@hp$(echo .)com \
--cc=felix@embedded-sol$(echo .)com \
--cc=linux-pci@vger$(echo .)kernel.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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