On 24.01.19 12:56, Lorenzo Pieralisi wrote:
On Mon, Jan 07, 2019 at 02:45:09PM +0100, Koen Vandeputte wrote:
Originally, cns3xxx used it's own functions for mapping, reading and writing registers.
Commit 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors") removed the internal PCI config write function in favor of the generic one:
cns3xxx_pci_write_config() --> pci_generic_config_write()
cns3xxx_pci_write_config() expected aligned addresses, being produced by cns3xxx_pci_map_bus() while the generic one pci_generic_config_write() actually expects the real address as both the function and hardware are capable of byte-aligned writes.
This currently leads to pci_generic_config_write() writing to the wrong registers on some ocasions.
First issue seen due to this:
- driver ath9k gets loaded
- The driver wants to write value 0xA8 to register PCI_LATENCY_TIMER, located at 0x0D
- cns3xxx_pci_map_bus() aligns the address to 0x0C
- pci_generic_config_write() effectively writes 0xA8 into register 0x0C (CACHE_LINE_SIZE)
This seems to cause some slight instability when certain PCI devices are used.
Another issue example caused by this this is the PCI bus numbering, where the primary bus is higher than the secondary, which is impossible.
Before:
00:00.0 PCI bridge: Cavium, Inc. Device 3400 (rev 01) (prog-if 00 [Normal decode]) Flags: bus master, fast devsel, latency 0, IRQ 255 Bus: primary=02, secondary=01, subordinate=ff, sec-latency=0
After fix:
00:00.0 PCI bridge: Cavium, Inc. Device 3400 (rev 01) (prog-if 00 [Normal decode]) Flags: bus master, fast devsel, latency 0, IRQ 255 Bus: primary=00, secondary=01, subordinate=02, sec-latency=0
And very likely some more ..
Fix all by omitting the alignment being done in the mapping function.
Fixes: 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors") Acked-by: Krzysztof Halasa khalasa@piap.pl Acked-by: Tim Harvey tharvey@gateworks.com Signed-off-by: Koen Vandeputte koen.vandeputte@ncentric.com CC: Arnd Bergmann arnd@arndb.de CC: Bjorn Helgaas bhelgaas@google.com CC: Olof Johansson olof@lixom.net CC: Robin Leblon robin.leblon@ncentric.com CC: Rob Herring robh@kernel.org CC: Russell King linux@armlinux.org.uk CC: stable@vger.kernel.org # v4.0+
arch/arm/mach-cns3xxx/pcie.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
I have applied both patches to pci/arm-cns3xxx for v5.1, however I had to reformat and rewrite both logs (commit line wrappings, capitalization, etc.) so have a look.
Thanks, Lorenzo
Hi Lorenzo,
Thank you for taking care of the wrappings etc. it seems my auto-wrap was disabled in my git tooling .. Your adaptions look more than fine.
Purely for my information:
Testing on a lot of devices here shows a huge improvement towards stability. Is it possible to get it merged sooner? Does "queued for 5.1" also mean that backporting to stables only will happen at 5.1_rc1 release?
Thanks again!
Koen