diff mbox series

[v3,4/4] arm64: dts: rockchip: Update PCI host bridge window to 32-bit address memory

Message ID 20210607112856.3499682-5-punitagrawal@gmail.com (mailing list archive)
State Superseded
Delegated to: Bjorn Helgaas
Headers show
Series PCI: of: Improvements to handle 64-bit attribute for non-prefetchable ranges | expand

Commit Message

Punit Agrawal June 7, 2021, 11:28 a.m. UTC
The PCIe host bridge on RK3399 advertises a single 64-bit memory
address range even though it lies entirely below 4GB.

Previously the OF PCI range parser treated 64-bit ranges more
leniently (i.e., as 32-bit), but since commit 9d57e61bf723 ("of/pci:
Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
the code takes a stricter view and treats the ranges as advertised in
the device tree (i.e, as 64-bit).

The change in behaviour causes failure when allocating bus addresses
to devices connected behind a PCI-to-PCI bridge that require
non-prefetchable memory ranges. The allocation failure was observed
for certain Samsung NVMe drives connected to RockPro64 boards.

Update the host bridge window attributes to treat it as 32-bit address
memory. This fixes the allocation failure observed since commit
9d57e61bf723.

Reported-by: Alexandru Elisei <alexandru.elisei@arm.com>
Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Rob Herring <robh+dt@kernel.org>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Heiko Stuebner June 10, 2021, 9:50 p.m. UTC | #1
Hi,

Am Montag, 7. Juni 2021, 13:28:56 CEST schrieb Punit Agrawal:
> The PCIe host bridge on RK3399 advertises a single 64-bit memory
> address range even though it lies entirely below 4GB.
> 
> Previously the OF PCI range parser treated 64-bit ranges more
> leniently (i.e., as 32-bit), but since commit 9d57e61bf723 ("of/pci:
> Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
> the code takes a stricter view and treats the ranges as advertised in
> the device tree (i.e, as 64-bit).
> 
> The change in behaviour causes failure when allocating bus addresses
> to devices connected behind a PCI-to-PCI bridge that require
> non-prefetchable memory ranges. The allocation failure was observed
> for certain Samsung NVMe drives connected to RockPro64 boards.
> 
> Update the host bridge window attributes to treat it as 32-bit address
> memory. This fixes the allocation failure observed since commit
> 9d57e61bf723.
> 
> Reported-by: Alexandru Elisei <alexandru.elisei@arm.com>
> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Rob Herring <robh+dt@kernel.org>

just for clarity, should I just pick this patch separately for 5.13-rc to
make it easy for people using current kernel devicetrees, or should
this wait for the update mentioned in the cover-letter response
and should go all together through the PCI tree?

If so, I can provide an
Acked-by: Heiko Stuebner <heiko@sntech.de>



> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 634a91af8e83..4b854eb21f72 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -227,7 +227,7 @@ pcie0: pcie@f8000000 {
>  		       <&pcie_phy 2>, <&pcie_phy 3>;
>  		phy-names = "pcie-phy-0", "pcie-phy-1",
>  			    "pcie-phy-2", "pcie-phy-3";
> -		ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
> +		ranges = <0x82000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
>  			 <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
>  		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
>  			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
>
Punit Agrawal June 11, 2021, 2:38 p.m. UTC | #2
Hi Heiko,

Heiko Stübner <heiko@sntech.de> writes:

> Hi,
>
> Am Montag, 7. Juni 2021, 13:28:56 CEST schrieb Punit Agrawal:
>> The PCIe host bridge on RK3399 advertises a single 64-bit memory
>> address range even though it lies entirely below 4GB.
>> 
>> Previously the OF PCI range parser treated 64-bit ranges more
>> leniently (i.e., as 32-bit), but since commit 9d57e61bf723 ("of/pci:
>> Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
>> the code takes a stricter view and treats the ranges as advertised in
>> the device tree (i.e, as 64-bit).
>> 
>> The change in behaviour causes failure when allocating bus addresses
>> to devices connected behind a PCI-to-PCI bridge that require
>> non-prefetchable memory ranges. The allocation failure was observed
>> for certain Samsung NVMe drives connected to RockPro64 boards.
>> 
>> Update the host bridge window attributes to treat it as 32-bit address
>> memory. This fixes the allocation failure observed since commit
>> 9d57e61bf723.
>> 
>> Reported-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> Suggested-by: Robin Murphy <robin.murphy@arm.com>
>> Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
>> Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> Cc: Heiko Stuebner <heiko@sntech.de>
>> Cc: Rob Herring <robh+dt@kernel.org>
>
> just for clarity, should I just pick this patch separately for 5.13-rc to
> make it easy for people using current kernel devicetrees, or should
> this wait for the update mentioned in the cover-letter response
> and should go all together through the PCI tree?

The device tree change is independent of the other patches in the
series. It would be great if you can pick this one - I am waiting on
feedback from Rob before sending an update on the remaining patches.

Thanks,
Punit

> If so, I can provide an
> Acked-by: Heiko Stuebner <heiko@sntech.de>
>
>
>
>> ---
>>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> index 634a91af8e83..4b854eb21f72 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
>> @@ -227,7 +227,7 @@ pcie0: pcie@f8000000 {
>>  		       <&pcie_phy 2>, <&pcie_phy 3>;
>>  		phy-names = "pcie-phy-0", "pcie-phy-1",
>>  			    "pcie-phy-2", "pcie-phy-3";
>> -		ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
>> +		ranges = <0x82000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
>>  			 <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
>>  		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
>>  			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,
>> 
>
>
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
Rob Herring June 15, 2021, 9:29 p.m. UTC | #3
On Thu, Jun 10, 2021 at 3:50 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi,
>
> Am Montag, 7. Juni 2021, 13:28:56 CEST schrieb Punit Agrawal:
> > The PCIe host bridge on RK3399 advertises a single 64-bit memory
> > address range even though it lies entirely below 4GB.
> >
> > Previously the OF PCI range parser treated 64-bit ranges more
> > leniently (i.e., as 32-bit), but since commit 9d57e61bf723 ("of/pci:
> > Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
> > the code takes a stricter view and treats the ranges as advertised in
> > the device tree (i.e, as 64-bit).
> >
> > The change in behaviour causes failure when allocating bus addresses
> > to devices connected behind a PCI-to-PCI bridge that require
> > non-prefetchable memory ranges. The allocation failure was observed
> > for certain Samsung NVMe drives connected to RockPro64 boards.
> >
> > Update the host bridge window attributes to treat it as 32-bit address
> > memory. This fixes the allocation failure observed since commit
> > 9d57e61bf723.
> >
> > Reported-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
> > Suggested-by: Robin Murphy <robin.murphy@arm.com>
> > Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> > Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: Rob Herring <robh+dt@kernel.org>
>
> just for clarity, should I just pick this patch separately for 5.13-rc to
> make it easy for people using current kernel devicetrees, or should
> this wait for the update mentioned in the cover-letter response
> and should go all together through the PCI tree?

This was dropped from v4, but should still be applied IMO.

Acked-by: Rob Herring <robh@kernel.org>
Heiko Stuebner June 15, 2021, 9:49 p.m. UTC | #4
Am Dienstag, 15. Juni 2021, 23:29:12 CEST schrieb Rob Herring:
> On Thu, Jun 10, 2021 at 3:50 PM Heiko Stübner <heiko@sntech.de> wrote:
> >
> > Hi,
> >
> > Am Montag, 7. Juni 2021, 13:28:56 CEST schrieb Punit Agrawal:
> > > The PCIe host bridge on RK3399 advertises a single 64-bit memory
> > > address range even though it lies entirely below 4GB.
> > >
> > > Previously the OF PCI range parser treated 64-bit ranges more
> > > leniently (i.e., as 32-bit), but since commit 9d57e61bf723 ("of/pci:
> > > Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
> > > the code takes a stricter view and treats the ranges as advertised in
> > > the device tree (i.e, as 64-bit).
> > >
> > > The change in behaviour causes failure when allocating bus addresses
> > > to devices connected behind a PCI-to-PCI bridge that require
> > > non-prefetchable memory ranges. The allocation failure was observed
> > > for certain Samsung NVMe drives connected to RockPro64 boards.
> > >
> > > Update the host bridge window attributes to treat it as 32-bit address
> > > memory. This fixes the allocation failure observed since commit
> > > 9d57e61bf723.
> > >
> > > Reported-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
> > > Suggested-by: Robin Murphy <robin.murphy@arm.com>
> > > Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
> > > Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > Cc: Heiko Stuebner <heiko@sntech.de>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> >
> > just for clarity, should I just pick this patch separately for 5.13-rc to
> > make it easy for people using current kernel devicetrees, or should
> > this wait for the update mentioned in the cover-letter response
> > and should go all together through the PCI tree?
> 
> This was dropped from v4, but should still be applied IMO.

It was probably dropped because I applied it ;-)

It's part of armsoc already [0] and should make its way into
5.13 shortly.


Heiko


[0] https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/commit/?h=arm/fixes&id=8efe01b4386ab38a36b99cfdc1dc02c38a8898c3

> 
> Acked-by: Rob Herring <robh@kernel.org>
>
Punit Agrawal June 16, 2021, 1 p.m. UTC | #5
Heiko Stübner <heiko@sntech.de> writes:

> Am Dienstag, 15. Juni 2021, 23:29:12 CEST schrieb Rob Herring:
>> On Thu, Jun 10, 2021 at 3:50 PM Heiko Stübner <heiko@sntech.de> wrote:
>> >
>> > Hi,
>> >
>> > Am Montag, 7. Juni 2021, 13:28:56 CEST schrieb Punit Agrawal:
>> > > The PCIe host bridge on RK3399 advertises a single 64-bit memory
>> > > address range even though it lies entirely below 4GB.
>> > >
>> > > Previously the OF PCI range parser treated 64-bit ranges more
>> > > leniently (i.e., as 32-bit), but since commit 9d57e61bf723 ("of/pci:
>> > > Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses")
>> > > the code takes a stricter view and treats the ranges as advertised in
>> > > the device tree (i.e, as 64-bit).
>> > >
>> > > The change in behaviour causes failure when allocating bus addresses
>> > > to devices connected behind a PCI-to-PCI bridge that require
>> > > non-prefetchable memory ranges. The allocation failure was observed
>> > > for certain Samsung NVMe drives connected to RockPro64 boards.
>> > >
>> > > Update the host bridge window attributes to treat it as 32-bit address
>> > > memory. This fixes the allocation failure observed since commit
>> > > 9d57e61bf723.
>> > >
>> > > Reported-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> > > Link: https://lore.kernel.org/r/7a1e2ebc-f7d8-8431-d844-41a9c36a8911@arm.com
>> > > Suggested-by: Robin Murphy <robin.murphy@arm.com>
>> > > Signed-off-by: Punit Agrawal <punitagrawal@gmail.com>
>> > > Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> > > Cc: Heiko Stuebner <heiko@sntech.de>
>> > > Cc: Rob Herring <robh+dt@kernel.org>
>> >
>> > just for clarity, should I just pick this patch separately for 5.13-rc to
>> > make it easy for people using current kernel devicetrees, or should
>> > this wait for the update mentioned in the cover-letter response
>> > and should go all together through the PCI tree?
>> 
>> This was dropped from v4, but should still be applied IMO.
>
> It was probably dropped because I applied it ;-)
>
> It's part of armsoc already [0] and should make its way into
> 5.13 shortly.

Thanks for sending the patch along. I left a note to the effect in v4
but it's easy to miss.

Hopefully all sorted now.

[...]
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 634a91af8e83..4b854eb21f72 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -227,7 +227,7 @@  pcie0: pcie@f8000000 {
 		       <&pcie_phy 2>, <&pcie_phy 3>;
 		phy-names = "pcie-phy-0", "pcie-phy-1",
 			    "pcie-phy-2", "pcie-phy-3";
-		ranges = <0x83000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
+		ranges = <0x82000000 0x0 0xfa000000 0x0 0xfa000000 0x0 0x1e00000>,
 			 <0x81000000 0x0 0xfbe00000 0x0 0xfbe00000 0x0 0x100000>;
 		resets = <&cru SRST_PCIE_CORE>, <&cru SRST_PCIE_MGMT>,
 			 <&cru SRST_PCIE_MGMT_STICKY>, <&cru SRST_PCIE_PIPE>,