* [PATCH v3] of: property: Add fw_devlink support for interrupt-map property
@ 2024-04-30 4:08 Anup Patel
2024-05-08 20:54 ` Saravana Kannan
0 siblings, 1 reply; 3+ messages in thread
From: Anup Patel @ 2024-04-30 4:08 UTC (permalink / raw
To: Rob Herring, Saravana Kannan
Cc: Palmer Dabbelt, Paul Walmsley, Atish Patra, Andrew Jones,
Sunil V L, Anup Patel, linux-riscv, linux-kernel, devicetree,
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. This the only case where
the interrupts are not described in DT.
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 affects every platform 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.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
Changes since v2:
- No need for a loop to find #interrupt-cells property value
- Fix node de-reference leak when index is greater than number
of entries in interrupt-map property
Changes since v1:
- Updated commit description based on Rob's suggestion
- Use of_irq_parse_raw() for parsing interrupt-map DT property
---
drivers/of/property.c | 50 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index a6358ee99b74..7326ca07adfe 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1311,6 +1311,55 @@ 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)
+{
+ const __be32 *imap, *imap_end, *addr;
+ struct of_phandle_args sup_args;
+ u32 addrcells, intcells;
+ int i, imaplen;
+
+ if (!IS_ENABLED(CONFIG_OF_IRQ))
+ return NULL;
+
+ if (strcmp(prop_name, "interrupt-map"))
+ return NULL;
+
+ if (of_property_read_u32(np, "#interrupt-cells", &intcells))
+ return NULL;
+ addrcells = of_bus_n_addr_cells(np);
+
+ imap = of_get_property(np, "interrupt-map", &imaplen);
+ if (!imap || imaplen <= (addrcells + intcells))
+ return NULL;
+ imap_end = imap + imaplen;
+
+ sup_args.np = NULL;
+ while (imap < imap_end) {
+ addr = imap;
+ imap += addrcells;
+
+ sup_args.np = np;
+ sup_args.args_count = intcells;
+ for (i = 0; i < intcells; i++)
+ sup_args.args[i] = be32_to_cpu(imap[i]);
+ imap += intcells;
+
+ if (of_irq_parse_raw(addr, &sup_args))
+ return NULL;
+
+ if (!index)
+ return sup_args.np;
+
+ of_node_put(sup_args.np);
+ sup_args.np = NULL;
+ imap += sup_args.args_count + 1;
+ index--;
+ }
+
+ return NULL;
+}
+
static struct device_node *parse_remote_endpoint(struct device_node *np,
const char *prop_name,
int index)
@@ -1359,6 +1408,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, },
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] of: property: Add fw_devlink support for interrupt-map property
2024-04-30 4:08 [PATCH v3] of: property: Add fw_devlink support for interrupt-map property Anup Patel
@ 2024-05-08 20:54 ` Saravana Kannan
2024-05-09 12:07 ` Anup Patel
0 siblings, 1 reply; 3+ messages in thread
From: Saravana Kannan @ 2024-05-08 20:54 UTC (permalink / raw
To: Anup Patel
Cc: Rob Herring, Palmer Dabbelt, Paul Walmsley, Atish Patra,
Andrew Jones, Sunil V L, Anup Patel, linux-riscv, linux-kernel,
devicetree
On Mon, Apr 29, 2024 at 9:08 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> 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. This the only case where
> the interrupts are not described in DT.
>
> 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 affects every platform 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.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> Changes since v2:
> - No need for a loop to find #interrupt-cells property value
> - Fix node de-reference leak when index is greater than number
> of entries in interrupt-map property
> Changes since v1:
> - Updated commit description based on Rob's suggestion
> - Use of_irq_parse_raw() for parsing interrupt-map DT property
> ---
> drivers/of/property.c | 50 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index a6358ee99b74..7326ca07adfe 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1311,6 +1311,55 @@ 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)
> +{
> + const __be32 *imap, *imap_end, *addr;
> + struct of_phandle_args sup_args;
> + u32 addrcells, intcells;
> + int i, imaplen;
> +
> + if (!IS_ENABLED(CONFIG_OF_IRQ))
> + return NULL;
> +
> + if (strcmp(prop_name, "interrupt-map"))
> + return NULL;
> +
> + if (of_property_read_u32(np, "#interrupt-cells", &intcells))
> + return NULL;
> + addrcells = of_bus_n_addr_cells(np);
> +
> + imap = of_get_property(np, "interrupt-map", &imaplen);
> + if (!imap || imaplen <= (addrcells + intcells))
> + return NULL;
> + imap_end = imap + imaplen;
> +
> + sup_args.np = NULL;
> + while (imap < imap_end) {
> + addr = imap;
> + imap += addrcells;
> +
> + sup_args.np = np;
> + sup_args.args_count = intcells;
> + for (i = 0; i < intcells; i++)
> + sup_args.args[i] = be32_to_cpu(imap[i]);
> + imap += intcells;
> +
> + if (of_irq_parse_raw(addr, &sup_args))
Can you leave a comment above this call saying of_irq_parse_raw()
updates sup_args.np? It's really a problem with the function IMO, but
a comment here would be helpful.
> + return NULL;
> +
> + if (!index)
> + return sup_args.np;
> +
> + of_node_put(sup_args.np);
> + sup_args.np = NULL;
Why do you need to set it to NULL? Can we just delete this line?
If you take care of these minor comments, then
Reviewed-by: Saravana Kannan <saravanak@google.com>
-Saravana
> + imap += sup_args.args_count + 1;
> + index--;
> + }
> +
> + return NULL;
> +}
> +
> static struct device_node *parse_remote_endpoint(struct device_node *np,
> const char *prop_name,
> int index)
> @@ -1359,6 +1408,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, },
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] of: property: Add fw_devlink support for interrupt-map property
2024-05-08 20:54 ` Saravana Kannan
@ 2024-05-09 12:07 ` Anup Patel
0 siblings, 0 replies; 3+ messages in thread
From: Anup Patel @ 2024-05-09 12:07 UTC (permalink / raw
To: Saravana Kannan
Cc: Anup Patel, Rob Herring, Palmer Dabbelt, Paul Walmsley,
Atish Patra, Andrew Jones, Sunil V L, linux-riscv, linux-kernel,
devicetree
On Thu, May 9, 2024 at 2:25 AM Saravana Kannan <saravanak@google.com> wrote:
>
> On Mon, Apr 29, 2024 at 9:08 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > 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. This the only case where
> > the interrupts are not described in DT.
> >
> > 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 affects every platform 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.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> > Changes since v2:
> > - No need for a loop to find #interrupt-cells property value
> > - Fix node de-reference leak when index is greater than number
> > of entries in interrupt-map property
> > Changes since v1:
> > - Updated commit description based on Rob's suggestion
> > - Use of_irq_parse_raw() for parsing interrupt-map DT property
> > ---
> > drivers/of/property.c | 50 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> >
> > diff --git a/drivers/of/property.c b/drivers/of/property.c
> > index a6358ee99b74..7326ca07adfe 100644
> > --- a/drivers/of/property.c
> > +++ b/drivers/of/property.c
> > @@ -1311,6 +1311,55 @@ 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)
> > +{
> > + const __be32 *imap, *imap_end, *addr;
> > + struct of_phandle_args sup_args;
> > + u32 addrcells, intcells;
> > + int i, imaplen;
> > +
> > + if (!IS_ENABLED(CONFIG_OF_IRQ))
> > + return NULL;
> > +
> > + if (strcmp(prop_name, "interrupt-map"))
> > + return NULL;
> > +
> > + if (of_property_read_u32(np, "#interrupt-cells", &intcells))
> > + return NULL;
> > + addrcells = of_bus_n_addr_cells(np);
> > +
> > + imap = of_get_property(np, "interrupt-map", &imaplen);
> > + if (!imap || imaplen <= (addrcells + intcells))
> > + return NULL;
> > + imap_end = imap + imaplen;
> > +
> > + sup_args.np = NULL;
> > + while (imap < imap_end) {
> > + addr = imap;
> > + imap += addrcells;
> > +
> > + sup_args.np = np;
> > + sup_args.args_count = intcells;
> > + for (i = 0; i < intcells; i++)
> > + sup_args.args[i] = be32_to_cpu(imap[i]);
> > + imap += intcells;
> > +
> > + if (of_irq_parse_raw(addr, &sup_args))
> Can you leave a comment above this call saying of_irq_parse_raw()
> updates sup_args.np? It's really a problem with the function IMO, but
> a comment here would be helpful.
Sure, I will add a comment about this.
>
> > + return NULL;
> > +
> > + if (!index)
> > + return sup_args.np;
> > +
> > + of_node_put(sup_args.np);
> > + sup_args.np = NULL;
>
> Why do you need to set it to NULL? Can we just delete this line?
These NULL assignments are residue from previous revision. I will
remove these assignments.
>
> If you take care of these minor comments, then
>
> Reviewed-by: Saravana Kannan <saravanak@google.com>
>
> -Saravana
>
> > + imap += sup_args.args_count + 1;
> > + index--;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > static struct device_node *parse_remote_endpoint(struct device_node *np,
> > const char *prop_name,
> > int index)
> > @@ -1359,6 +1408,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, },
> > --
> > 2.34.1
> >
Thanks,
Anup
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-09 12:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-30 4:08 [PATCH v3] of: property: Add fw_devlink support for interrupt-map property Anup Patel
2024-05-08 20:54 ` Saravana Kannan
2024-05-09 12:07 ` Anup Patel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).