On 2020-08-14 23:51, Rob Herring wrote:

On Fri, Aug 14, 2020 at 12:21 PM Marc Zyngier [off-list ref] wrote:

quoted

Hi all,

On 2020-07-28 16:36, Jiaxun Yang wrote:

quoted

So the parser can be used to parse range property of ISA bus.

As they're all using PCI-like method of range property, there is no
need
start a new parser.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Rob Herring <robh@kernel.org>

This patch, although it looks correct, breaks the RK3399-based
systems, as they miss the (now required) 'device_type = "pci";'
property.

Required since 1990 something...

And yet... The fact that it has been broken from day-1 (3.5 years
ago) shows how good we are at enforcing those requirements.

quoted

We can fix the in-tree DT, but that's not really an option
if someone relies on the DT being provided by the firmware
(I for one definitely do).

Perhaps time to pay attention to schema errors:

arch/arm64/boot/dts/rockchip/rk3399-sapphire-excavator.dt.yaml:
pcie@f8000000: 'device_type' is a required property

As long as this isn't part of the normal build flow, these errors
will get ignored. I don't even know how to trigger this validation,
TBH.

(I thought dtc would also catch this, but there we look for
device_type and then do PCI checks like node name. I guess we needed
to check for either device_type or the node name...)

That would be much better. At least this *does* run at build time.

quoted

I came up with the following hack, which solves the issue for
me. Definitely not my finest hour, but I really need this box
to keep going. I will post a patch fixing the DT separately.

Thanks,

         M.

 From ceef5fd9c4d2005eb577505c68868ebe527c098f Mon Sep 17 00:00:00 
2001
 From: Marc Zyngier [off-list ref]
Date: Fri, 14 Aug 2020 19:10:12 +0100
Subject: [PATCH] of: address: Workaround broken DTs missing the
'device_type =
  "pci"' property

Recent changes to the PCI bus parsing made it mandatory for
device trees nodes describing a PCI controller to have the
'device_type = "pci"' property for the node to be matched.

Although this is compliant with the specification, it breaks
existing device-trees that have been working fine for years
(the Rockchip rk3399-based systems being a prime example of
such breakage).

In order to paper over the blunder, let's also match nodes
that have the "linux,pci-domain" property, as they are
pretty likely to be PCI nodes. This fixes the issue for
systems such as the above platforms.

Fixes: 2f96593ecc37 ("of_address: Add bus type match for pci ranges
parser")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
  drivers/of/address.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 590493e04b01..712e03781a2a 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c

@@ -134,9 +134,12 @@ static int of_bus_pci_match(struct device_node 

*np)
         * "pciex" is PCI Express
         * "vci" is for the /chaos bridge on 1st-gen PCI powermacs
         * "ht" is hypertransport
+        * "linux,pci-domain" is a workaround for broken device trees
+        * lacking the required "device_type" property.

I would suggest looking for 'pci' or 'pcie' node name instead.

You should remove linux,pci-domain from rk3399 as it is pointless when
there's a single PCI host bridge.

Indeed. I was clutching at straws here.

The other option is fixup the live tree with of_add_property() in the
Rockchip PCI driver. Less likely to impact anyone else.

That's actually a much better solution, thanks for pointing this out.
At least I can shout about broken DTs while fixing it up, and the
new fix is fairly neat.

I'll post new patches shortly.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...