of: property: Add fw_devlink support for interrupt-map property
Subsystems (2)
PATCH 1/1 of: property: Add fw_devlink support for interrupt-map property
Some of the PCI controllers (such as generic PCI host controller) use "interrupt-map" DT property to describe the mapping between PCI endpoints and PCI interrupt pins.
Currently, there is no fw_devlink created based on "interrupt-map" DT property so interrupt controller is not guaranteed to be probed before PCI host controller. This mainly affects RISC-V platforms where both PCI host controller and interrupt controllers are probed as regular platform devices.
This creates fw_devlink between consumers (PCI host controller) and supplier (interrupt controller) based on "interrupt-map" DT property.
--- drivers/of/property.c | 53 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
drivers/of/property.c +53 -0
@@ -1311,6 +1311,58 @@ static struct device_node *parse_interrupts(struct device_node *np,return of_irq_parse_one (np, index, &sup_args) ?NULL : sup_args.np; } +static struct device_node *parse_interrupt_map (struct device_node *np, +const char *prop_name,int index) +{ +struct device_node *tn, *ipar, *supnp =NULL ; +u32 addrcells, intcells, cells; +const __be32 *imap, *imap_end; +int i, imaplen; + +if (!IS_ENABLED (CONFIG_OF_IRQ )) +return NULL ; + +if (strcmp (prop_name,"interrupt-map" )) +return NULL ; + + ipar =of_node_get (np); +do { +if (!of_property_read_u32 (ipar,"#interrupt-cells" , &intcells)) +break ; + tn = ipar; + ipar =of_irq_find_parent (ipar); +of_node_put (tn); + }while (ipar); +if (!ipar) +return NULL ; + addrcells =of_bus_n_addr_cells (ipar); +of_node_put (ipar); + + imap =of_get_property (np,"interrupt-map" , &imaplen); +if (!imap || imaplen <= (addrcells + intcells)) +return NULL ; + imap_end = imap + imaplen; + +for (i =0 ; i <= index && imap < imap_end; i++) { +if (supnp) +of_node_put (supnp); + + imap += addrcells; + imap += intcells; + + supnp =of_find_node_by_phandle (be32_to_cpu (imap[0 ])); +if (!supnp) +return NULL ; + imap +=1 ; + +if (of_property_read_u32 (supnp,"#interrupt-cells" , &cells)) +return NULL ; + imap += cells; + } + +return supnp; +} +static struct device_node *parse_remote_endpoint (struct device_node *np,const char *prop_name,int index)
@@ -1359,6 +1411,7 @@ static const struct supplier_bindings of_supplier_bindings[] = { { .parse_prop = parse_msi_parent, }, { .parse_prop = parse_gpio_compat, }, { .parse_prop = parse_interrupts, }, + { .parse_prop = parse_interrupt_map, }, { .parse_prop = parse_regulators, }, { .parse_prop = parse_gpio, }, { .parse_prop = parse_gpios, },
Signature · 1 line
2.34.1
Rob Herring
maintainerQuoted 4 lines · Anup Patel
Some of the PCI controllers (such as generic PCI host controller) use "interrupt-map" DT property to describe the mapping between PCI endpoints and PCI interrupt pins.
I would go as far as saying that's the only case as that's the only case where the interrupts are not described in DT.
Quoted 5 lines
Currently, there is no
fw_devlinkcreated based on "interrupt-map" DT property so interrupt controller is not guaranteed to be probed before PCI host controller. This mainly affects RISC-V platforms where both PCI host controller and interrupt controllers are probed as regular platform devices.
That's *every* system with PCI really.
Quoted 53 lines
This creates
fw_devlinkbetween consumers (PCI host controller) and supplier (interrupt controller) based on "interrupt-map" DT property.Signed-off-by Anup Patel <[email protected]>--- drivers/of/property.c | 53 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)drivers/of/property.c +37
-0@@ -1311,6 +1311,58 @@ static struct device_node *parse_interrupts(struct device_node *np,return of_irq_parse_one (np, index, &sup_args) ?NULL : sup_args.np; } +static struct device_node *parse_interrupt_map (struct device_node *np, +const char *prop_name,int index) +{ +struct device_node *tn, *ipar, *supnp =NULL ; +u32 addrcells, intcells, cells; +const __be32 *imap, *imap_end; +int i, imaplen; + +if (!IS_ENABLED (CONFIG_OF_IRQ )) +return NULL ; + +if (strcmp (prop_name,"interrupt-map" )) +return NULL ; + + ipar =of_node_get (np); +do { +if (!of_property_read_u32 (ipar,"#interrupt-cells" , &intcells)) +break ; + tn = ipar; + ipar =of_irq_find_parent (ipar); +of_node_put (tn); + }while (ipar); +if (!ipar) +return NULL ; + addrcells =of_bus_n_addr_cells (ipar); +of_node_put (ipar); + + imap =of_get_property (np,"interrupt-map" , &imaplen); +if (!imap || imaplen <= (addrcells + intcells)) +return NULL ; + imap_end = imap + imaplen; + +for (i =0 ; i <= index && imap < imap_end; i++) { +if (supnp) +of_node_put (supnp); + + imap += addrcells;
drivers/of/property.c static struct device_node *parse_interrupts(struct device_node *np, · line 1360
Quoted 10 lines
drivers/of/property.c +10
-0+ imap += intcells; + + supnp =of_find_node_by_phandle (be32_to_cpu (imap[0 ])); +if (!supnp) +return NULL ; + imap +=1 ; + +if (of_property_read_u32 (supnp,"#interrupt-cells" , &cells)) +return NULL ; + imap += cells;
This is wrong. Technically, you can have #address-cells too.
The bigger problem I have is this creates 2 sets of 'interrupt-map' parsing code. Your version skips a lot of things like whether the interrupt controller is available and there's the list of 'interrupt-map' abusers to think about.
Rob
Quoted 9 lines · Rob Herring
Quoted 4 lines · Anup Patel
Some of the PCI controllers (such as generic PCI host controller) use "interrupt-map" DT property to describe the mapping between PCI endpoints and PCI interrupt pins.
I would go as far as saying that's the only case as that's the only case where the interrupts are not described in DT.
Sure, I will update the text.
Quoted 8 lines
Quoted 6 lines
Currently, there is no
fw_devlinkcreated based on "interrupt-map" DT property so interrupt controller is not guaranteed to be probed before PCI host controller. This mainly affects RISC-V platforms where both PCI host controller and interrupt controllers are probed as regular platform devices.That's *every* system with PCI really.
Okay, I will re-word.
Quoted 67 lines
Quoted 54 lines
This creates
fw_devlinkbetween consumers (PCI host controller) and supplier (interrupt controller) based on "interrupt-map" DT property.Signed-off-by Anup Patel <[email protected]>--- drivers/of/property.c | 53 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)drivers/of/property.c +37
-0@@ -1311,6 +1311,58 @@ static struct device_node *parse_interrupts(struct device_node *np,return of_irq_parse_one (np, index, &sup_args) ?NULL : sup_args.np; } +static struct device_node *parse_interrupt_map (struct device_node *np, +const char *prop_name,int index) +{ +struct device_node *tn, *ipar, *supnp =NULL ; +u32 addrcells, intcells, cells; +const __be32 *imap, *imap_end; +int i, imaplen; + +if (!IS_ENABLED (CONFIG_OF_IRQ )) +return NULL ; + +if (strcmp (prop_name,"interrupt-map" )) +return NULL ; + + ipar =of_node_get (np); +do { +if (!of_property_read_u32 (ipar,"#interrupt-cells" , &intcells)) +break ; + tn = ipar; + ipar =of_irq_find_parent (ipar); +of_node_put (tn); + }while (ipar); +if (!ipar) +return NULL ; + addrcells =of_bus_n_addr_cells (ipar); +of_node_put (ipar); + + imap =of_get_property (np,"interrupt-map" , &imaplen); +if (!imap || imaplen <= (addrcells + intcells)) +return NULL ; + imap_end = imap + imaplen; + +for (i =0 ; i <= index && imap < imap_end; i++) { +if (supnp) +of_node_put (supnp); + + imap += addrcells;drivers/of/property.c static struct device_node *parse_interrupts(struct device_node *np, · line 1360
Quoted 10 lines
drivers/of/property.c +10
-0+ imap += intcells; + + supnp =of_find_node_by_phandle (be32_to_cpu (imap[0 ])); +if (!supnp) +return NULL ; + imap +=1 ; + +if (of_property_read_u32 (supnp,"#interrupt-cells" , &cells)) +return NULL ; + imap += cells;This is wrong. Technically, you can have #address-cells too.
Quoted 5 lines
The bigger problem I have is this creates 2 sets of 'interrupt-map' parsing code. Your version skips a lot of things like whether the interrupt controller is available and there's the list of 'interrupt-map' abusers to think about.
In my first attempt, I was trying to keep it simple but I guess I over-simplified it.
I have an alternate approach to use of_irq_parse_raw() over here (similar to of_irq_parse_pci() in drivers/pci/of.c). Are you okay with this approach ?
Regards,
Anup