linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment
@ 2019-01-07 13:45 Koen Vandeputte
  2019-01-07 13:45 ` [PATCH v2 2/2] arm: cns3xxx: use actual size reads for PCIe Koen Vandeputte
  2019-01-24 11:56 ` [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment Lorenzo Pieralisi
  0 siblings, 2 replies; 11+ messages in thread
From: Koen Vandeputte @ 2019-01-07 13:45 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Rob Herring, Arnd Bergmann, linux-pci, Russell King, stable,
	Robin Leblon, Olof Johansson, Koen Vandeputte, Bjorn Helgaas

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(-)


V2:
--> resend to be in sync with new second patch
--> added acked-by's based on patch comments

diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
index 318394ed5c7a..5e11ad3164e0 100644
--- a/arch/arm/mach-cns3xxx/pcie.c
+++ b/arch/arm/mach-cns3xxx/pcie.c
@@ -83,7 +83,7 @@ static void __iomem *cns3xxx_pci_map_bus(struct pci_bus *bus,
 	} else /* remote PCI bus */
 		base = cnspci->cfg1_regs + ((busno & 0xf) << 20);
 
-	return base + (where & 0xffc) + (devfn << 12);
+	return base + where + (devfn << 12);
 }
 
 static int cns3xxx_pci_read_config(struct pci_bus *bus, unsigned int devfn,
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] arm: cns3xxx: use actual size reads for PCIe
  2019-01-07 13:45 [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment Koen Vandeputte
@ 2019-01-07 13:45 ` Koen Vandeputte
  2019-01-08  6:10   ` Krzysztof Hałasa
  2019-01-25 10:55   ` Krzysztof Hałasa
  2019-01-24 11:56 ` [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment Lorenzo Pieralisi
  1 sibling, 2 replies; 11+ messages in thread
From: Koen Vandeputte @ 2019-01-07 13:45 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Rob Herring, Arnd Bergmann, linux-pci, Tim Harvey, Russell King,
	stable, Robin Leblon, Krzysztof Halasa, Koen Vandeputte,
	Olof Johansson

commit 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")
reimplemented cns3xxx_pci_read_config() using pci_generic_config_read32(),
which preserved the property of only doing 32-bit reads.

It also replaced cns3xxx_pci_write_config() with pci_generic_config_write(),
so it changed writes from always being 32 bits to being the actual size,
which works just fine.

Due to:
- The documentation does not mention that only 32 bit access is allowed.
- Writes are already executed using the actual size
- Extensive testing shows that 8b, 16b and 32b reads work as intended

It makes perfectly sense to also swap 32 bit reading in favor of actual size.

Fixes: 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Krzysztof Halasa <khalasa@piap.pl>
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: Tim Harvey <tharvey@gateworks.com>
CC: stable@vger.kernel.org # v4.0+
---
 arch/arm/mach-cns3xxx/pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
index 5e11ad3164e0..95a11d5b3587 100644
--- a/arch/arm/mach-cns3xxx/pcie.c
+++ b/arch/arm/mach-cns3xxx/pcie.c
@@ -93,7 +93,7 @@ static int cns3xxx_pci_read_config(struct pci_bus *bus, unsigned int devfn,
 	u32 mask = (0x1ull << (size * 8)) - 1;
 	int shift = (where % 4) * 8;
 
-	ret = pci_generic_config_read32(bus, devfn, where, size, val);
+	ret = pci_generic_config_read(bus, devfn, where, size, val);
 
 	if (ret == PCIBIOS_SUCCESSFUL && !bus->number && !devfn &&
 	    (where & 0xffc) == PCI_CLASS_REVISION)
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] arm: cns3xxx: use actual size reads for PCIe
  2019-01-07 13:45 ` [PATCH v2 2/2] arm: cns3xxx: use actual size reads for PCIe Koen Vandeputte
@ 2019-01-08  6:10   ` Krzysztof Hałasa
  2019-01-25 10:55   ` Krzysztof Hałasa
  1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Hałasa @ 2019-01-08  6:10 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Rob Herring, Arnd Bergmann, linux-pci, Tim Harvey, Russell King,
	stable, Robin Leblon, Olof Johansson, linux-arm-kernel

Koen Vandeputte <koen.vandeputte@ncentric.com> writes:

> commit 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")
> reimplemented cns3xxx_pci_read_config() using pci_generic_config_read32(),
> which preserved the property of only doing 32-bit reads.
>
> It also replaced cns3xxx_pci_write_config() with pci_generic_config_write(),
> so it changed writes from always being 32 bits to being the actual size,
> which works just fine.
>
> Due to:
> - The documentation does not mention that only 32 bit access is allowed.
> - Writes are already executed using the actual size
> - Extensive testing shows that 8b, 16b and 32b reads work as intended
>
> It makes perfectly sense to also swap 32 bit reading in favor of actual size.

Acked-by: Krzysztof Halasa <khalasa@piap.pl>

> --- a/arch/arm/mach-cns3xxx/pcie.c
> +++ b/arch/arm/mach-cns3xxx/pcie.c
> @@ -93,7 +93,7 @@ static int cns3xxx_pci_read_config(struct pci_bus *bus, unsigned int devfn,
>  	u32 mask = (0x1ull << (size * 8)) - 1;
>  	int shift = (where % 4) * 8;
>  
> -	ret = pci_generic_config_read32(bus, devfn, where, size, val);
> +	ret = pci_generic_config_read(bus, devfn, where, size, val);
>  
>  	if (ret == PCIBIOS_SUCCESSFUL && !bus->number && !devfn &&
>  	    (where & 0xffc) == PCI_CLASS_REVISION)

-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment
  2019-01-07 13:45 [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment Koen Vandeputte
  2019-01-07 13:45 ` [PATCH v2 2/2] arm: cns3xxx: use actual size reads for PCIe Koen Vandeputte
@ 2019-01-24 11:56 ` Lorenzo Pieralisi
  2019-01-24 15:23   ` Koen Vandeputte
  1 sibling, 1 reply; 11+ messages in thread
From: Lorenzo Pieralisi @ 2019-01-24 11:56 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Rob Herring, Arnd Bergmann, linux-pci, Russell King, stable,
	Robin Leblon, Olof Johansson, Bjorn Helgaas, linux-arm-kernel

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

> 
> 
> V2:
> --> resend to be in sync with new second patch
> --> added acked-by's based on patch comments
> 
> diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
> index 318394ed5c7a..5e11ad3164e0 100644
> --- a/arch/arm/mach-cns3xxx/pcie.c
> +++ b/arch/arm/mach-cns3xxx/pcie.c
> @@ -83,7 +83,7 @@ static void __iomem *cns3xxx_pci_map_bus(struct pci_bus *bus,
>  	} else /* remote PCI bus */
>  		base = cnspci->cfg1_regs + ((busno & 0xf) << 20);
>  
> -	return base + (where & 0xffc) + (devfn << 12);
> +	return base + where + (devfn << 12);
>  }
>  
>  static int cns3xxx_pci_read_config(struct pci_bus *bus, unsigned int devfn,
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment
  2019-01-24 11:56 ` [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment Lorenzo Pieralisi
@ 2019-01-24 15:23   ` Koen Vandeputte
  2019-01-24 16:27     ` Lorenzo Pieralisi
  0 siblings, 1 reply; 11+ messages in thread
From: Koen Vandeputte @ 2019-01-24 15:23 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Rob Herring, Arnd Bergmann, linux-pci, Russell King, stable,
	Robin Leblon, Olof Johansson, Bjorn Helgaas, linux-arm-kernel


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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment
  2019-01-24 15:23   ` Koen Vandeputte
@ 2019-01-24 16:27     ` Lorenzo Pieralisi
  2019-01-30 22:08       ` Arnd Bergmann
  0 siblings, 1 reply; 11+ messages in thread
From: Lorenzo Pieralisi @ 2019-01-24 16:27 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Rob Herring, Arnd Bergmann, linux-pci, Russell King, stable,
	Robin Leblon, Olof Johansson, Bjorn Helgaas, linux-arm-kernel

On Thu, Jan 24, 2019 at 04:23:05PM +0100, Koen Vandeputte wrote:
> 
> 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?

Yes, I will ask Bjorn if we can send them for one of the upcoming -rc*
(so effectively you will get them in v5.0 and propagated to stable
earlier), I do not think it is that urgent either though, let me handle
that.

Thanks,
Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] arm: cns3xxx: use actual size reads for PCIe
  2019-01-07 13:45 ` [PATCH v2 2/2] arm: cns3xxx: use actual size reads for PCIe Koen Vandeputte
  2019-01-08  6:10   ` Krzysztof Hałasa
@ 2019-01-25 10:55   ` Krzysztof Hałasa
  1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Hałasa @ 2019-01-25 10:55 UTC (permalink / raw)
  To: Koen Vandeputte
  Cc: Rob Herring, Arnd Bergmann, linux-pci, Tim Harvey, Russell King,
	stable, Robin Leblon, Olof Johansson, linux-arm-kernel

Koen Vandeputte <koen.vandeputte@ncentric.com> writes:

> commit 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")
> reimplemented cns3xxx_pci_read_config() using pci_generic_config_read32(),
> which preserved the property of only doing 32-bit reads.
>
> It also replaced cns3xxx_pci_write_config() with pci_generic_config_write(),
> so it changed writes from always being 32 bits to being the actual size,
> which works just fine.
>
> Due to:
> - The documentation does not mention that only 32 bit access is allowed.
> - Writes are already executed using the actual size
> - Extensive testing shows that 8b, 16b and 32b reads work as intended
>
> It makes perfectly sense to also swap 32 bit reading in favor of actual size.
>
> Fixes: 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Koen Vandeputte <koen.vandeputte@ncentric.com>

Acked-by: Krzysztof Halasa <khalasa@piap.pl>

> --- a/arch/arm/mach-cns3xxx/pcie.c
> +++ b/arch/arm/mach-cns3xxx/pcie.c
> @@ -93,7 +93,7 @@ static int cns3xxx_pci_read_config(struct pci_bus *bus, unsigned int devfn,
>  	u32 mask = (0x1ull << (size * 8)) - 1;
>  	int shift = (where % 4) * 8;
>  
> -	ret = pci_generic_config_read32(bus, devfn, where, size, val);
> +	ret = pci_generic_config_read(bus, devfn, where, size, val);
>  
>  	if (ret == PCIBIOS_SUCCESSFUL && !bus->number && !devfn &&
>  	    (where & 0xffc) == PCI_CLASS_REVISION)

-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment
  2019-01-24 16:27     ` Lorenzo Pieralisi
@ 2019-01-30 22:08       ` Arnd Bergmann
  2019-01-30 23:06         ` Bjorn Helgaas
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2019-01-30 22:08 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Rob Herring, linux-pci, Russell King, # 3.4.x, Robin Leblon,
	Olof Johansson, Koen Vandeputte, Bjorn Helgaas, Linux ARM

On Thu, Jan 24, 2019 at 5:29 PM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Thu, Jan 24, 2019 at 04:23:05PM +0100, Koen Vandeputte wrote:
> > On 24.01.19 12:56, Lorenzo Pieralisi wrote:
> > >On Mon, Jan 07, 2019 at 02:45:09PM +0100, Koen Vandeputte wrote:

> >
> > 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?
>
> Yes, I will ask Bjorn if we can send them for one of the upcoming -rc*
> (so effectively you will get them in v5.0 and propagated to stable
> earlier), I do not think it is that urgent either though, let me handle
> that.

We can take them through the soc tree if that's easier, but
going through Bjorn's tree is also fine.

      Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment
  2019-01-30 22:08       ` Arnd Bergmann
@ 2019-01-30 23:06         ` Bjorn Helgaas
  2019-01-31  8:00           ` Arnd Bergmann
  0 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2019-01-30 23:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Rob Herring, Lorenzo Pieralisi, linux-pci, Russell King, # 3.4.x,
	Robin Leblon, Koen Vandeputte, Olof Johansson, Linux ARM

On Wed, Jan 30, 2019 at 11:08:04PM +0100, Arnd Bergmann wrote:
> On Thu, Jan 24, 2019 at 5:29 PM Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On Thu, Jan 24, 2019 at 04:23:05PM +0100, Koen Vandeputte wrote:
> > > On 24.01.19 12:56, Lorenzo Pieralisi wrote:
> > > >On Mon, Jan 07, 2019 at 02:45:09PM +0100, Koen Vandeputte wrote:
> > > 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?
> >
> > Yes, I will ask Bjorn if we can send them for one of the upcoming -rc*
> > (so effectively you will get them in v5.0 and propagated to stable
> > earlier), I do not think it is that urgent either though, let me handle
> > that.
> 
> We can take them through the soc tree if that's easier, but
> going through Bjorn's tree is also fine.

I have the following on my for-linus branch and I'll ask Linus to pull them
this week, so they will appear in v5.0:

  b8b592a3a8d1 ARM: cns3xxx: use actual size reads for PCIe
  b3a32f359397 ARM: cns3xxx: fix writing to wrong PCI registers after alignment

Neither is currently marked for stable, but I'll add that if you like.
They're already both marked as:

  Fixes: 802b7c06adc7 ("ARM: cns3xxx: Convert PCI to use generic config accessors")

which I think appeared in v4.0.

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment
  2019-01-30 23:06         ` Bjorn Helgaas
@ 2019-01-31  8:00           ` Arnd Bergmann
  2019-01-31 21:16             ` Bjorn Helgaas
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2019-01-31  8:00 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rob Herring, Lorenzo Pieralisi, linux-pci, Russell King, # 3.4.x,
	Robin Leblon, Koen Vandeputte, Olof Johansson, Linux ARM

On Thu, Jan 31, 2019 at 12:06 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Wed, Jan 30, 2019 at 11:08:04PM +0100, Arnd Bergmann wrote:
> > On Thu, Jan 24, 2019 at 5:29 PM Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> > > On Thu, Jan 24, 2019 at 04:23:05PM +0100, Koen Vandeputte wrote:
> > > > On 24.01.19 12:56, Lorenzo Pieralisi wrote:
> > > > >On Mon, Jan 07, 2019 at 02:45:09PM +0100, Koen Vandeputte wrote:
> > > > 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?
> > >
> > > Yes, I will ask Bjorn if we can send them for one of the upcoming -rc*
> > > (so effectively you will get them in v5.0 and propagated to stable
> > > earlier), I do not think it is that urgent either though, let me handle
> > > that.
> >
> > We can take them through the soc tree if that's easier, but
> > going through Bjorn's tree is also fine.
>
> I have the following on my for-linus branch and I'll ask Linus to pull them
> this week, so they will appear in v5.0:
>
>   b8b592a3a8d1 ARM: cns3xxx: use actual size reads for PCIe
>   b3a32f359397 ARM: cns3xxx: fix writing to wrong PCI registers after alignment

Ok, thanks!

> Neither is currently marked for stable, but I'll add that if you like.

Yes, I think that would be good, along with

Acked-by: Arnd Bergmann <arnd@arndb.de>

     Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment
  2019-01-31  8:00           ` Arnd Bergmann
@ 2019-01-31 21:16             ` Bjorn Helgaas
  0 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2019-01-31 21:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Rob Herring, Lorenzo Pieralisi, linux-pci, Russell King, # 3.4.x,
	Robin Leblon, Koen Vandeputte, Olof Johansson, Linux ARM

On Thu, Jan 31, 2019 at 09:00:30AM +0100, Arnd Bergmann wrote:
> On Thu, Jan 31, 2019 at 12:06 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Wed, Jan 30, 2019 at 11:08:04PM +0100, Arnd Bergmann wrote:
> > > On Thu, Jan 24, 2019 at 5:29 PM Lorenzo Pieralisi
> > > <lorenzo.pieralisi@arm.com> wrote:
> > > > On Thu, Jan 24, 2019 at 04:23:05PM +0100, Koen Vandeputte wrote:
> > > > > On 24.01.19 12:56, Lorenzo Pieralisi wrote:
> > > > > >On Mon, Jan 07, 2019 at 02:45:09PM +0100, Koen Vandeputte wrote:
> > > > > 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?
> > > >
> > > > Yes, I will ask Bjorn if we can send them for one of the upcoming -rc*
> > > > (so effectively you will get them in v5.0 and propagated to stable
> > > > earlier), I do not think it is that urgent either though, let me handle
> > > > that.
> > >
> > > We can take them through the soc tree if that's easier, but
> > > going through Bjorn's tree is also fine.
> >
> > I have the following on my for-linus branch and I'll ask Linus to pull them
> > this week, so they will appear in v5.0:
> >
> >   b8b592a3a8d1 ARM: cns3xxx: use actual size reads for PCIe
> >   b3a32f359397 ARM: cns3xxx: fix writing to wrong PCI registers after alignment
> 
> Ok, thanks!
> 
> > Neither is currently marked for stable, but I'll add that if you like.
> 
> Yes, I think that would be good, along with
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Added, thanks!

Actually I was mistaken: the "use actual size reads" patch *was* marked for
stable, but the "fix writing" one was not.  I suspect this was intended to
be the other way around because AFAIK the "fix writing" patch fixes
problems and it makes sense to put it in stable, while the "use actual size
reads" patch is more of a cleanup and I don't think there's a benefit to
putting *it* in stable.

So I added your ack to both and marked only the "fix writing" patch for stable.

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-01-31 21:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 13:45 [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment Koen Vandeputte
2019-01-07 13:45 ` [PATCH v2 2/2] arm: cns3xxx: use actual size reads for PCIe Koen Vandeputte
2019-01-08  6:10   ` Krzysztof Hałasa
2019-01-25 10:55   ` Krzysztof Hałasa
2019-01-24 11:56 ` [PATCH v2 1/2] arm: cns3xxx: fix writing to wrong PCI registers after alignment Lorenzo Pieralisi
2019-01-24 15:23   ` Koen Vandeputte
2019-01-24 16:27     ` Lorenzo Pieralisi
2019-01-30 22:08       ` Arnd Bergmann
2019-01-30 23:06         ` Bjorn Helgaas
2019-01-31  8:00           ` Arnd Bergmann
2019-01-31 21:16             ` Bjorn Helgaas

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).