Re: [RFC/PATCH] remove gratuitous reads from maple pci config space methods
Well, I had already checked with Ben, who wrote the code, and my understanding is that the reads are intended to work around some misbehaving Apple bridges, None of the PCI interfaces on the U3 or U4 bridges have that problem as far as I know. I think the workaround was copied from code for older Apple bridges? Okay, then the change should be fine for maple. Yes. Of course, as usual, testing is needed, yada yada. but that a sync after the write (implied by releasing pci_lock in the generic pci code) should suffice for those. I don't see how a sync could help here at all, not more than an eieio anyway? Alright, well, maybe take it up with Ben when I post the patch for powermac, since that's where it could actually matter. It should be fine on PowerMac as well -- all G5s use U3/U4, the workaround is for certain older Apple bridge chips. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC/PATCH] remove gratuitous reads from maple pci config space methods
It might be worth checking that there isn't a particular reason for these. Just because posting writes are forbidden doesn't mean a particular bridge won't screw it up... Well, I had already checked with Ben, who wrote the code, and my understanding is that the reads are intended to work around some misbehaving Apple bridges, None of the PCI interfaces on the U3 or U4 bridges have that problem as far as I know. I think the workaround was copied from code for older Apple bridges? but that a sync after the write (implied by releasing pci_lock in the generic pci code) should suffice for those. I don't see how a sync could help here at all, not more than an eieio anyway? Segher ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC/PATCH] remove gratuitous reads from maple pci config space methods
On Wed, Aug 08, 2007 at 08:04:32PM -0500, Nathan Lynch wrote: Benjamin Herrenschmidt wrote: On Wed, 2007-08-08 at 19:50 -0500, Nathan Lynch wrote: Remove the gratuitous reads from u3_agp_write_config, u3_ht_write_config, and u4_pcie_write_config. Signed-off-by: Nathan Lynch [EMAIL PROTECTED] Acked-by: Benjamin Herrenschmidt [EMAIL PROTECTED] --- Thanks ! Care to fix powermac too ? :-) Sure, I'll get it tomorrow... looks like pasemi cribbed the powermac code too :) Yeah, it originated either from powermac or maple so it's there too. Feel free to fix it too while you're at it. -Olof ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC/PATCH] remove gratuitous reads from maple pci config space methods
Segher Boessenkool wrote: It might be worth checking that there isn't a particular reason for these. Just because posting writes are forbidden doesn't mean a particular bridge won't screw it up... Well, I had already checked with Ben, who wrote the code, and my understanding is that the reads are intended to work around some misbehaving Apple bridges, None of the PCI interfaces on the U3 or U4 bridges have that problem as far as I know. I think the workaround was copied from code for older Apple bridges? Okay, then the change should be fine for maple. but that a sync after the write (implied by releasing pci_lock in the generic pci code) should suffice for those. I don't see how a sync could help here at all, not more than an eieio anyway? Alright, well, maybe take it up with Ben when I post the patch for powermac, since that's where it could actually matter. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC/PATCH] remove gratuitous reads from maple pci config space methods
Benjamin Herrenschmidt wrote: On Wed, 2007-08-08 at 19:50 -0500, Nathan Lynch wrote: Remove the gratuitous reads from u3_agp_write_config, u3_ht_write_config, and u4_pcie_write_config. Signed-off-by: Nathan Lynch [EMAIL PROTECTED] Acked-by: Benjamin Herrenschmidt [EMAIL PROTECTED] --- Thanks ! Care to fix powermac too ? :-) Sure, I'll get it tomorrow... looks like pasemi cribbed the powermac code too :) ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC/PATCH] remove gratuitous reads from maple pci config space methods
On Wed, Aug 08, 2007 at 07:50:44PM -0500, Nathan Lynch wrote: The maple pci configuration space write methods read the written location immediately after the write is performed, presumably in order to flush the write. However, configuration space writes are not allowed to be posted, making these reads gratuitous. It might be worth checking that there isn't a particular reason for these. Just because posting writes are forbidden doesn't mean a particular bridge won't screw it up... Furthermore, this behavior potentially causes us to violate the PCI PM spec when changing between e.g. D0 and D3 states, because a delay of up to 10ms may be required before the OS accesses configuration space after the write which initiates the transition. It definitely causes a system hang for me with a Broadcom 5721 PCIE network adapter, which is fixed by this change. Remove the gratuitous reads from u3_agp_write_config, u3_ht_write_config, and u4_pcie_write_config. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC/PATCH] remove gratuitous reads from maple pci config space methods
David Gibson wrote: On Wed, Aug 08, 2007 at 07:50:44PM -0500, Nathan Lynch wrote: The maple pci configuration space write methods read the written location immediately after the write is performed, presumably in order to flush the write. However, configuration space writes are not allowed to be posted, making these reads gratuitous. It might be worth checking that there isn't a particular reason for these. Just because posting writes are forbidden doesn't mean a particular bridge won't screw it up... Well, I had already checked with Ben, who wrote the code, and my understanding is that the reads are intended to work around some misbehaving Apple bridges, but that a sync after the write (implied by releasing pci_lock in the generic pci code) should suffice for those. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
