* Re: [PATCH] Restore PCI IO space behind Freescale pseudo-bridge
[not found] <200901271813.n0RIDBHq023872@pogo.cesa.opbu.xerox.com>
@ 2009-01-28 5:37 ` Kumar Gala
2009-01-28 9:31 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 5+ messages in thread
From: Kumar Gala @ 2009-01-28 5:37 UTC (permalink / raw)
To: Andrew Klossner; +Cc: linuxppc-dev list
On Jan 27, 2009, at 12:13 PM, Andrew Klossner wrote:
> Kumar, you wrote:
>
>> Any resolution to all this.. Not exactly sure what change impacts
>> things and what part of our screwy HW is causing issues.
>
> The change in question is commit
> b556151110ff003ce77d84597400c84824690ccf.
>
> The fundamental problem is that the "PCI Bus Command" register, the 16
> bits at offset 4 in config space for both the PCI and PCI-Express bus
> interfaces, does not populate PCI_COMMAND_IO, the least-significant
> bit. The PCI bridge standard says that software must set this bit to
> 1 before the bridge can respond as a slave to I/O space access,
> including access to devices on the subordinate bus. The FSL parts
> ignore this requirement and control response to I/O space with the
> (vendor-specific) outbound window attributes registers (POWARn.)
> 2.6.28 differs from 2.6.27 in that it examines this bit and sees that
> it's set to 0, then observes that the I/O space window starts at
> offset 0 and concludes that no I/O space resource should be allocated
> for the bridge.
>
> We are using the patch I emailed. It's ugly in that it hard-codes the
> Freescale vendor ID, but it works for us.
I guess I'm confused what the actual breakage is beyond some printk
messages.
We "fixup" the resources in fsl_pcibios_fixup_bus() so Ben's change
shouldn't have an impact. Does something actually stop functioning?
> Another option would have been to configure our I/O space window to
> start at an offset other than 0. This would take me some time to
> achieve, and it feels ugly.
agreed, that's not the right solution.
- k
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Restore PCI IO space behind Freescale pseudo-bridge
2009-01-28 5:37 ` [PATCH] Restore PCI IO space behind Freescale pseudo-bridge Kumar Gala
@ 2009-01-28 9:31 ` Benjamin Herrenschmidt
2009-01-28 15:59 ` Kumar Gala
0 siblings, 1 reply; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2009-01-28 9:31 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev list, Andrew Klossner
> > We are using the patch I emailed. It's ugly in that it hard-codes the
> > Freescale vendor ID, but it works for us.
>
> I guess I'm confused what the actual breakage is beyond some printk
> messages.
>
> We "fixup" the resources in fsl_pcibios_fixup_bus() so Ben's change
> shouldn't have an impact. Does something actually stop functioning?
I think the problem comes from my patch that "detects" closed bridge
windows, which uses the PCI_COMMAND register as a clue, and it seems
that your bridges don't properly support IO enable in there..
Ben.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Restore PCI IO space behind Freescale pseudo-bridge
2009-01-28 9:31 ` Benjamin Herrenschmidt
@ 2009-01-28 15:59 ` Kumar Gala
0 siblings, 0 replies; 5+ messages in thread
From: Kumar Gala @ 2009-01-28 15:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list, Andrew Klossner
On Jan 28, 2009, at 3:31 AM, Benjamin Herrenschmidt wrote:
>
>>> We are using the patch I emailed. It's ugly in that it hard-codes
>>> the
>>> Freescale vendor ID, but it works for us.
>>
>> I guess I'm confused what the actual breakage is beyond some printk
>> messages.
>>
>> We "fixup" the resources in fsl_pcibios_fixup_bus() so Ben's change
>> shouldn't have an impact. Does something actually stop functioning?
>
> I think the problem comes from my patch that "detects" closed bridge
> windows, which uses the PCI_COMMAND register as a clue, and it seems
> that your bridges don't properly support IO enable in there..
Yeah but I don't see why that's an issue.
Because after your code (pcibios_uninitialized_bridge_resource) we'll
call ppc_md.pcibios_fixup_bus() and that will fix things up.
So I'm wondering if this is really a bug or just annoying printk's
that are confusing.
- k
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Restore PCI IO space behind Freescale pseudo-bridge
2009-01-14 19:48 Andrew Klossner
@ 2009-01-14 20:37 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2009-01-14 20:37 UTC (permalink / raw)
To: Andrew Klossner; +Cc: linuxppc-dev
On Wed, 2009-01-14 at 11:48 -0800, Andrew Klossner wrote:
> When I moved from 2.6.27 to 2.6.28, one of my devices didn't work
> until I devised the attached patch. The kernel disallowed PCI I/O
> space resources behind the pseudo-bridge in the Freescale MPC8545
> because that bridge's config-space registers incorrectly report that
> it doesn't forward I/O space transactions.
>
> Checking for a specific vendor ID is hacky, but it got my system going
> again with only a few perturbed lines. Is there a more correct way to
> achieve this?
A few things yes.
First, can't you just set the PCI_COMMAND_IO bit from a quirk or at init
time rather than adding to code to cope with it being cleared later on ?
Also, the fact that it's detected as uninitialized shouldn't have broken
access to your device. If there is need for some IO space, for example
because a device requests some, the kernel should have re-opened that
window of the bridge.
Thus if that failed, it would be useful to track it down a bit more. You
can try enabling DEBUG in pci-common.c and pci-64.c, boot with "debug"
on the kernel command line and and send the resulting dmesg log
Ben.
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index f36936d..f9f0048 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -985,6 +985,7 @@ static int __devinit pcibios_uninitialized_bridge_resource(struct pci_bus *bus,
> struct pci_dev *dev = bus->self;
> resource_size_t offset;
> u16 command;
> + u16 vendor;
> int i;
>
> /* We don't do anything if PCI_PROBE_ONLY is set */
> @@ -1030,7 +1031,16 @@ static int __devinit pcibios_uninitialized_bridge_resource(struct pci_bus *bus,
> * starting at low addresses -is- valid. What we do instead if that
> * we consider as unassigned anything that doesn't have IO enabled
> * in the PCI command register, and that's it.
> + * However, we don't do that if the bridge is internal to a Freescale
> + * CPU, as such bridges break the rules by, for example, not populating
> + * the PCI_COMMAND_IO bit.
> */
> + pci_read_config_word(dev, PCI_VENDOR_ID, &vendor);
> + if (vendor == PCI_VENDOR_ID_MOTOROLA ||
> + vendor == PCI_VENDOR_ID_FREESCALE) {
> + return 0;
> + }
> +
> pci_read_config_word(dev, PCI_COMMAND, &command);
> if (command & PCI_COMMAND_IO)
> return 0;
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Restore PCI IO space behind Freescale pseudo-bridge
@ 2009-01-14 19:48 Andrew Klossner
2009-01-14 20:37 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Klossner @ 2009-01-14 19:48 UTC (permalink / raw)
To: linuxppc-dev
When I moved from 2.6.27 to 2.6.28, one of my devices didn't work
until I devised the attached patch. The kernel disallowed PCI I/O
space resources behind the pseudo-bridge in the Freescale MPC8545
because that bridge's config-space registers incorrectly report that
it doesn't forward I/O space transactions.
Checking for a specific vendor ID is hacky, but it got my system going
again with only a few perturbed lines. Is there a more correct way to
achieve this?
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index f36936d..f9f0048 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -985,6 +985,7 @@ static int __devinit pcibios_uninitialized_bridge_resource(struct pci_bus *bus,
struct pci_dev *dev = bus->self;
resource_size_t offset;
u16 command;
+ u16 vendor;
int i;
/* We don't do anything if PCI_PROBE_ONLY is set */
@@ -1030,7 +1031,16 @@ static int __devinit pcibios_uninitialized_bridge_resource(struct pci_bus *bus,
* starting at low addresses -is- valid. What we do instead if that
* we consider as unassigned anything that doesn't have IO enabled
* in the PCI command register, and that's it.
+ * However, we don't do that if the bridge is internal to a Freescale
+ * CPU, as such bridges break the rules by, for example, not populating
+ * the PCI_COMMAND_IO bit.
*/
+ pci_read_config_word(dev, PCI_VENDOR_ID, &vendor);
+ if (vendor == PCI_VENDOR_ID_MOTOROLA ||
+ vendor == PCI_VENDOR_ID_FREESCALE) {
+ return 0;
+ }
+
pci_read_config_word(dev, PCI_COMMAND, &command);
if (command & PCI_COMMAND_IO)
return 0;
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-01-28 15:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <200901271813.n0RIDBHq023872@pogo.cesa.opbu.xerox.com>
2009-01-28 5:37 ` [PATCH] Restore PCI IO space behind Freescale pseudo-bridge Kumar Gala
2009-01-28 9:31 ` Benjamin Herrenschmidt
2009-01-28 15:59 ` Kumar Gala
2009-01-14 19:48 Andrew Klossner
2009-01-14 20:37 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).