On Fri, Aug 14, 2020 at 12:21 PM Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> Hi all,
>
> On 2020-07-28 16:36, Jiaxun Yang wrote:
> > 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@xxxxxxxxxxx>
> > Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
>
> 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...
> 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
(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...)
> 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 <maz@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> ---
> 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.
The other option is fixup the live tree with of_add_property() in the
Rockchip PCI driver. Less likely to impact anyone else.
> */
> return of_node_is_type(np, "pci") || of_node_is_type(np, "pciex") ||
> - of_node_is_type(np, "vci") || of_node_is_type(np, "ht");
> + of_node_is_type(np, "vci") || of_node_is_type(np, "ht") ||
> + of_find_property(np, "linux,pci-domain", NULL);
> }
>
> static void of_bus_pci_count_cells(struct device_node *np,
> --
> 2.27.0
>
>
> --
> Jazz is not dead. It just smells funny...
