Hi,
This series convert reservation of Loongson64 Logic PIO into DeviceTree based
method.
It can be used to replace Huacai's
"MIPS: Loongson64: Reserve legacy MMIO space according to bridge type".
Thanks.
v2:
- Address Rob and Huacai's review comments.
v3:
- Address Rob, Thomas's review comments.
v4:
- Fix typo & grammar issue according to Xuerui's suggestion.
Jiaxun Yang (5):
of_address: Add bus type match for pci ranges parser
MIPS: Loongson64: Process ISA Node in DeviceTree
MIPS: Loongson64: Enlarge IO_SPACE_LIMIT
MIPS: Loongson64: DTS: Fix ISA and PCI I/O ranges for RS780E PCH
MIPS: Loongson64: Add ISA node for LS7A PCH
arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 ++
arch/mips/boot/dts/loongson/rs780e-pch.dtsi | 4 +-
arch/mips/include/asm/io.h | 2 -
arch/mips/include/asm/mach-generic/spaces.h | 4 +
.../mips/include/asm/mach-loongson64/spaces.h | 3 +-
arch/mips/loongson64/init.c | 87 +++++++++++++------
drivers/of/address.c | 29 ++++---
include/linux/of_address.h | 4 +
8 files changed, 97 insertions(+), 43 deletions(-)
--
2.28.0.rc1
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 <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
--
v2: Drop useless check, fix some na for bus_addr
add define of of_range_parser_init according to
Rob's suggestion.
v3: Abstract out has_flags. simplify define.
---
drivers/of/address.c | 29 +++++++++++++++++------------
include/linux/of_address.h | 4 ++++
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 8eea3f6e29a4..813936d419ad 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -49,6 +49,7 @@ struct of_bus {
u64 (*map)(__be32 *addr, const __be32 *range,
int na, int ns, int pna);
int (*translate)(__be32 *addr, u64 offset, int na);
+ bool has_flags;
unsigned int (*get_flags)(const __be32 *addr);
};
@@ -364,6 +365,7 @@ static struct of_bus of_busses[] = {
.count_cells = of_bus_pci_count_cells,
.map = of_bus_pci_map,
.translate = of_bus_pci_translate,
+ .has_flags = true,
.get_flags = of_bus_pci_get_flags,
},
#endif /* CONFIG_PCI */
@@ -375,6 +377,7 @@ static struct of_bus of_busses[] = {
.count_cells = of_bus_isa_count_cells,
.map = of_bus_isa_map,
.translate = of_bus_isa_translate,
+ .has_flags = true,
.get_flags = of_bus_isa_get_flags,
},
/* Default */
@@ -698,9 +701,10 @@ static int parser_init(struct of_pci_range_parser *parser,
parser->node = node;
parser->pna = of_n_addr_cells(node);
- parser->na = of_bus_n_addr_cells(node);
- parser->ns = of_bus_n_size_cells(node);
parser->dma = !strcmp(name, "dma-ranges");
+ parser->bus = of_match_bus(node);
+
+ parser->bus->count_cells(parser->node, &parser->na, &parser->ns);
parser->range = of_get_property(node, name, &rlen);
if (parser->range == NULL)
@@ -732,6 +736,7 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
int na = parser->na;
int ns = parser->ns;
int np = parser->pna + na + ns;
+ int busflag_na = 0;
if (!range)
return NULL;
@@ -739,12 +744,13 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
if (!parser->range || parser->range + np > parser->end)
return NULL;
- if (parser->na == 3)
- range->flags = of_bus_pci_get_flags(parser->range);
- else
- range->flags = 0;
+ range->flags = parser->bus->get_flags(parser->range);
+
+ /* A extra cell for resource flags */
+ if (parser->bus->has_flags)
+ busflag_na = 1;
- range->pci_addr = of_read_number(parser->range, na);
+ range->bus_addr = of_read_number(parser->range + busflag_na, na - busflag_na);
if (parser->dma)
range->cpu_addr = of_translate_dma_address(parser->node,
@@ -759,11 +765,10 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
/* Now consume following elements while they are contiguous */
while (parser->range + np <= parser->end) {
u32 flags = 0;
- u64 pci_addr, cpu_addr, size;
+ u64 bus_addr, cpu_addr, size;
- if (parser->na == 3)
- flags = of_bus_pci_get_flags(parser->range);
- pci_addr = of_read_number(parser->range, na);
+ flags = parser->bus->get_flags(parser->range);
+ bus_addr = of_read_number(parser->range + busflag_na, na - busflag_na);
if (parser->dma)
cpu_addr = of_translate_dma_address(parser->node,
parser->range + na);
@@ -774,7 +779,7 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser,
if (flags != range->flags)
break;
- if (pci_addr != range->pci_addr + range->size ||
+ if (bus_addr != range->bus_addr + range->size ||
cpu_addr != range->cpu_addr + range->size)
break;
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 763022ed3456..88bc943405cd 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -6,8 +6,11 @@
#include <linux/of.h>
#include <linux/io.h>
+struct of_bus;
+
struct of_pci_range_parser {
struct device_node *node;
+ struct of_bus *bus;
const __be32 *range;
const __be32 *end;
int na;
@@ -119,6 +122,7 @@ static inline void __iomem *of_iomap(struct device_node *device, int index)
return NULL;
}
#endif
+#define of_range_parser_init of_pci_range_parser_init
#if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_PCI)
extern const __be32 *of_get_pci_address(struct device_node *dev, int bar_no,
--
2.28.0.rc1
Previously, we're hardcoding reserved ISA I/O Space in, now
we're processing it I/O via DeviceTree directly.
The ranges property if ISA node is used to determine the size and address
of reserved I/O space.
Signed-off-by: Jiaxun Yang <[email protected]>
--
v2: Use range_parser instead of pci_range_parser
v4: Fix typos & grammar problem thanks to Xuerui.
---
arch/mips/loongson64/init.c | 87 ++++++++++++++++++++++++++-----------
1 file changed, 62 insertions(+), 25 deletions(-)
diff --git a/arch/mips/loongson64/init.c b/arch/mips/loongson64/init.c
index 59ddadace83f..8ba22c30f312 100644
--- a/arch/mips/loongson64/init.c
+++ b/arch/mips/loongson64/init.c
@@ -7,6 +7,8 @@
#include <linux/irqchip.h>
#include <linux/logic_pio.h>
#include <linux/memblock.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
#include <asm/bootinfo.h>
#include <asm/traps.h>
#include <asm/smp-ops.h>
@@ -63,41 +65,76 @@ void __init prom_free_prom_memory(void)
{
}
-static __init void reserve_pio_range(void)
+static int __init add_legacy_isa_io(struct fwnode_handle *fwnode, resource_size_t hw_start,
+ resource_size_t size)
{
+ int ret = 0;
struct logic_pio_hwaddr *range;
+ unsigned long vaddr;
range = kzalloc(sizeof(*range), GFP_ATOMIC);
if (!range)
- return;
+ return -ENOMEM;
- range->fwnode = &of_root->fwnode;
- range->size = MMIO_LOWER_RESERVED;
- range->hw_start = LOONGSON_PCIIO_BASE;
+ range->fwnode = fwnode;
+ range->size = size;
+ range->hw_start = hw_start;
range->flags = LOGIC_PIO_CPU_MMIO;
- if (logic_pio_register_range(range)) {
- pr_err("Failed to reserve PIO range for legacy ISA\n");
- goto free_range;
+ ret = logic_pio_register_range(range);
+ if (ret) {
+ kfree(range);
+ return ret;
+ }
+
+ /* Legacy ISA must placed at the start of PCI_IOBASE */
+ if (range->io_start != 0) {
+ logic_pio_unregister_range(range);
+ kfree(range);
+ return -EINVAL;
}
- if (WARN(range->io_start != 0,
- "Reserved PIO range does not start from 0\n"))
- goto unregister;
-
- /*
- * i8259 would access I/O space, so mapping must be done here.
- * Please remove it when all drivers can be managed by logic_pio.
- */
- ioremap_page_range(PCI_IOBASE, PCI_IOBASE + MMIO_LOWER_RESERVED,
- LOONGSON_PCIIO_BASE,
- pgprot_device(PAGE_KERNEL));
-
- return;
-unregister:
- logic_pio_unregister_range(range);
-free_range:
- kfree(range);
+ vaddr = PCI_IOBASE + range->io_start;
+
+ ioremap_page_range(vaddr, vaddr + size, hw_start, pgprot_device(PAGE_KERNEL));
+
+ return 0;
+}
+
+static __init void reserve_pio_range(void)
+{
+ struct device_node *np;
+
+ for_each_node_by_name(np, "isa") {
+ struct of_range range;
+ struct of_range_parser parser;
+
+ pr_info("ISA Bridge: %pOF\n", np);
+
+ if (of_range_parser_init(&parser, np)) {
+ pr_info("Failed to parse resources.\n");
+ break;
+ }
+
+ for_each_of_range(&parser, &range) {
+ switch (range.flags & IORESOURCE_TYPE_BITS) {
+ case IORESOURCE_IO:
+ pr_info(" IO 0x%016llx..0x%016llx -> 0x%016llx\n",
+ range.cpu_addr,
+ range.cpu_addr + range.size - 1,
+ range.bus_addr);
+ if (add_legacy_isa_io(&np->fwnode, range.cpu_addr, range.size))
+ pr_warn("Failed to reserve legacy IO in Logic PIO\n");
+ break;
+ case IORESOURCE_MEM:
+ pr_info(" MEM 0x%016llx..0x%016llx -> 0x%016llx\n",
+ range.cpu_addr,
+ range.cpu_addr + range.size - 1,
+ range.bus_addr);
+ break;
+ }
+ }
+ }
}
void __init arch_init_irq(void)
--
2.28.0.rc1
Although currently we're not enabling any ISA device in devicetree,
but this node is required to express the ranges of address reserved
for ISA.
Signed-off-by: Jiaxun Yang <[email protected]>
---
arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
index 1c286bb8c703..e574a062dfae 100644
--- a/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
+++ b/arch/mips/boot/dts/loongson/ls7a-pch.dtsi
@@ -367,5 +367,12 @@ pci_bridge@14,0 {
interrupt-map = <0 0 0 0 &pic 39 IRQ_TYPE_LEVEL_HIGH>;
};
};
+
+ isa {
+ compatible = "isa";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ ranges = <1 0 0 0x18000000 0x20000>;
+ };
};
};
--
2.28.0.rc1
It can be very big on LS7A PCH systems.
Signed-off-by: Jiaxun Yang <[email protected]>
--
v3: Move IO_SPACE_LIMIT to spaces.h
---
arch/mips/include/asm/io.h | 2 --
arch/mips/include/asm/mach-generic/spaces.h | 4 ++++
arch/mips/include/asm/mach-loongson64/spaces.h | 3 +--
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index 346fffd9e972..8ca53f17c7c2 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -51,8 +51,6 @@
/* ioswab[bwlq], __mem_ioswab[bwlq] are defined in mangle-port.h */
-#define IO_SPACE_LIMIT 0xffff
-
/*
* On MIPS I/O ports are memory mapped, so we access them using normal
* load/store instructions. mips_io_port_base is the virtual address to
diff --git a/arch/mips/include/asm/mach-generic/spaces.h b/arch/mips/include/asm/mach-generic/spaces.h
index ee5ebe98f6cf..c3ac06a6acd2 100644
--- a/arch/mips/include/asm/mach-generic/spaces.h
+++ b/arch/mips/include/asm/mach-generic/spaces.h
@@ -14,6 +14,10 @@
#include <asm/mipsregs.h>
+#ifndef IO_SPACE_LIMIT
+#define IO_SPACE_LIMIT 0xffff
+#endif
+
/*
* This gives the physical RAM offset.
*/
diff --git a/arch/mips/include/asm/mach-loongson64/spaces.h b/arch/mips/include/asm/mach-loongson64/spaces.h
index 3de0ac9d8829..ce04e998a37b 100644
--- a/arch/mips/include/asm/mach-loongson64/spaces.h
+++ b/arch/mips/include/asm/mach-loongson64/spaces.h
@@ -11,8 +11,7 @@
#define PCI_IOSIZE SZ_16M
#define MAP_BASE (PCI_IOBASE + PCI_IOSIZE)
-/* Reserved at the start of PCI_IOBASE for legacy drivers */
-#define MMIO_LOWER_RESERVED 0x10000
+#define IO_SPACE_LIMIT (PCI_IOSIZE - 1)
#include <asm/mach-generic/spaces.h>
#endif
--
2.28.0.rc1
Ranges should express the actual physical address on bus.
Also enlarge the PCI I/O size to the actual hardware limit.
Signed-off-by: Jiaxun Yang <[email protected]>
---
arch/mips/boot/dts/loongson/rs780e-pch.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/boot/dts/loongson/rs780e-pch.dtsi b/arch/mips/boot/dts/loongson/rs780e-pch.dtsi
index d4c803d74036..871c866e0423 100644
--- a/arch/mips/boot/dts/loongson/rs780e-pch.dtsi
+++ b/arch/mips/boot/dts/loongson/rs780e-pch.dtsi
@@ -17,7 +17,7 @@ pci@1a000000 {
reg = <0 0x1a000000 0 0x02000000>;
- ranges = <0x01000000 0 0x00004000 0 0x18004000 0 0x00008000>,
+ ranges = <0x01000000 0 0x00004000 0 0x18004000 0 0x0000c000>,
<0x02000000 0 0x40000000 0 0x40000000 0 0x40000000>;
};
@@ -25,7 +25,7 @@ isa {
compatible = "isa";
#address-cells = <2>;
#size-cells = <1>;
- ranges = <1 0 0 0 0x4000>;
+ ranges = <1 0 0 0x18000000 0x4000>;
rtc0: rtc@70 {
compatible = "motorola,mc146818";
--
2.28.0.rc1
On Tue, Jul 28, 2020 at 11:36:54PM +0800, Jiaxun Yang wrote:
> Hi,
>
> This series convert reservation of Loongson64 Logic PIO into DeviceTree based
> method.
>
> It can be used to replace Huacai's
> "MIPS: Loongson64: Reserve legacy MMIO space according to bridge type".
>
> Thanks.
>
> v2:
> - Address Rob and Huacai's review comments.
> v3:
> - Address Rob, Thomas's review comments.
> v4:
> - Fix typo & grammar issue according to Xuerui's suggestion.
>
> Jiaxun Yang (5):
> of_address: Add bus type match for pci ranges parser
> MIPS: Loongson64: Process ISA Node in DeviceTree
> MIPS: Loongson64: Enlarge IO_SPACE_LIMIT
> MIPS: Loongson64: DTS: Fix ISA and PCI I/O ranges for RS780E PCH
> MIPS: Loongson64: Add ISA node for LS7A PCH
>
> arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 ++
> arch/mips/boot/dts/loongson/rs780e-pch.dtsi | 4 +-
> arch/mips/include/asm/io.h | 2 -
> arch/mips/include/asm/mach-generic/spaces.h | 4 +
> .../mips/include/asm/mach-loongson64/spaces.h | 3 +-
> arch/mips/loongson64/init.c | 87 +++++++++++++------
> drivers/of/address.c | 29 ++++---
> include/linux/of_address.h | 4 +
> 8 files changed, 97 insertions(+), 43 deletions(-)
series applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
?? 2020/7/29 4:52, Thomas Bogendoerfer ะด??:
> On Tue, Jul 28, 2020 at 11:36:54PM +0800, Jiaxun Yang wrote:
>> Hi,
>>
>> This series convert reservation of Loongson64 Logic PIO into DeviceTree based
>> method.
>>
>> It can be used to replace Huacai's
>> "MIPS: Loongson64: Reserve legacy MMIO space according to bridge type".
>>
>> Thanks.
>>
>> v2:
>> - Address Rob and Huacai's review comments.
>> v3:
>> - Address Rob, Thomas's review comments.
>> v4:
>> - Fix typo & grammar issue according to Xuerui's suggestion.
>>
>> Jiaxun Yang (5):
>> of_address: Add bus type match for pci ranges parser
>> MIPS: Loongson64: Process ISA Node in DeviceTree
>> MIPS: Loongson64: Enlarge IO_SPACE_LIMIT
>> MIPS: Loongson64: DTS: Fix ISA and PCI I/O ranges for RS780E PCH
>> MIPS: Loongson64: Add ISA node for LS7A PCH
>>
>> arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 7 ++
>> arch/mips/boot/dts/loongson/rs780e-pch.dtsi | 4 +-
>> arch/mips/include/asm/io.h | 2 -
>> arch/mips/include/asm/mach-generic/spaces.h | 4 +
>> .../mips/include/asm/mach-loongson64/spaces.h | 3 +-
>> arch/mips/loongson64/init.c | 87 +++++++++++++------
>> drivers/of/address.c | 29 ++++---
>> include/linux/of_address.h | 4 +
>> 8 files changed, 97 insertions(+), 43 deletions(-)
> series applied to mips-next.
+ Xuerui & Bibo,
This series should fix the issue that RTC on RS780E chipset failed
to be probed, please kindly test latest mips-next.
Thanks!
- Jiaxun
>
> Thomas.
>
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 <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
This patch, although it looks correct, breaks the RK3399-based
systems, as they miss the (now required) 'device_type = "pci";'
property.
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).
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 <[email protected]>
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 <[email protected]>
---
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.
*/
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...
On Fri, Aug 14, 2020 at 12:21 PM Marc Zyngier <[email protected]> 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 <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
>
> 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 <[email protected]>
> 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 <[email protected]>
> ---
> 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...
On 2020-08-14 23:51, Rob Herring wrote:
> On Fri, Aug 14, 2020 at 12:21 PM Marc Zyngier <[email protected]> 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 <[email protected]>
>> > Reviewed-by: Rob Herring <[email protected]>
>>
>> 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.
>
>> 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.
>
>> 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 <[email protected]>
>> 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 <[email protected]>
>> ---
>> 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...