of: property: Add device link support for interrupts

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
 	return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+					    const char *prop_name, int index)
+{
+	struct device_node *sup;
+
+	if (strcmp(prop_name, "interrupts") || index)
+		return NULL;
+
+	of_node_get(np);
+	while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+		np = of_get_next_parent(np);
+	of_node_put(np);
+
+	return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_clocks, },

drivers/of/property.c +1 -0

 	{ .parse_prop = parse_interconnects, },

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
 	{ .parse_prop = parse_pinctrl6, },
 	{ .parse_prop = parse_pinctrl7, },
 	{ .parse_prop = parse_pinctrl8, },
+	{ .parse_prop = parse_interrupts, },
 	{ .parse_prop = parse_regulators, },
 	{ .parse_prop = parse_gpio, },

{ .parse_prop = parse_gpios, },

Quoted 53 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
 	return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+					    const char *prop_name, int index)
+{
+	struct device_node *sup;
+
+	if (strcmp(prop_name, "interrupts") || index)
+		return NULL;
+
+	of_node_get(np);
+	while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+		np = of_get_next_parent(np);
+	of_node_put(np);
+
+	return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_clocks, },

drivers/of/property.c +1 -0

 	{ .parse_prop = parse_interconnects, },

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
 	{ .parse_prop = parse_pinctrl6, },
 	{ .parse_prop = parse_pinctrl7, },
 	{ .parse_prop = parse_pinctrl8, },
+	{ .parse_prop = parse_interrupts, },
 	{ .parse_prop = parse_regulators, },

Quoted 2 lines

drivers/of/property.c +0 -0

 	{ .parse_prop = parse_gpio, },

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 54 lines · Marc Zyngier

Quoted 53 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
 	return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+					    const char *prop_name, int index)
+{
+	struct device_node *sup;
+
+	if (strcmp(prop_name, "interrupts") || index)
+		return NULL;
+
+	of_node_get(np);
+	while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+		np = of_get_next_parent(np);
+	of_node_put(np);
+
+	return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_clocks, },

drivers/of/property.c +1 -0

 	{ .parse_prop = parse_interconnects, },

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
 	{ .parse_prop = parse_pinctrl6, },
 	{ .parse_prop = parse_pinctrl7, },
 	{ .parse_prop = parse_pinctrl8, },
+	{ .parse_prop = parse_interrupts, },
 	{ .parse_prop = parse_regulators, },

Quoted 8 lines

Quoted 2 lines

drivers/of/property.c +0 -0

 	{ .parse_prop = parse_gpio, },

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

We don't need to follow the hierarchy, we just need the immediate dependencies.

But you are right that 'interrupt-map' also needs to be tracked.

I also noticed that we define 'interrupt-parent' as a dependency to parse, but that's wrong. The dependency is where 'interrupts' appears and where 'interrupt-parent' appears is irrelevant.

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Quoted 2 lines

But you are right that 'interrupt-map' also needs to be tracked.

And 'interrupts-extended', while we're at it.

Quoted 4 lines

I also noticed that we define 'interrupt-parent' as a dependency to parse, but that's wrong. The dependency is where 'interrupts' appears and where 'interrupt-parent' appears is irrelevant.

Agreed. Though you need the object the dependency is on, I guess, if you want to be able to have the dependency edge between the device and the interrupt controller. But since the commit message doesn't say much about what this is trying to achieve, I'm only guessing the purpose of this patch.

        M.

--
Without deviation from the norm, progress is not possible.

Quoted 68 lines · Marc Zyngier

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 4 lines

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Thanks Rob for explaining it.

Marc, I wasn't sure if Rob would be okay with including of_irq.h here. Also, I'm trying to keep of/property.c independent of the framework code for now. The long term goal is to see if I can move out most of this into the frameworks. But I want to do that after I sort of some of the larger problems (like getting fw_devlink=on to work on all devices first). Let me know if you have a strong preference for right now, if not, I'd rather keep property.c independent for now.

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Quoted 85 lines · Saravana Kannan

Quoted 68 lines · Marc Zyngier

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 4 lines

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Thanks Rob for explaining it.

Marc, I wasn't sure if Rob would be okay with including of_irq.h here. Also, I'm trying to keep of/property.c independent of the framework code for now. The long term goal is to see if I can move out most of this into the frameworks. But I want to do that after I sort of some of the larger problems (like getting fw_devlink=on to work on all devices first). Let me know if you have a strong preference for right now, if not, I'd rather keep property.c independent for now.

Quoted 3 lines

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Hmm... I looked at Documentation/devicetree/bindings/interrupt-controller/interrupts.txt and it has no documentation for interrupt-map. There's a bunch of references to it in device specific DT binding documentation but I don't want to rely on those for correctness.

Quoted 91 lines · Saravana Kannan

Quoted 88 lines · Saravana Kannan

Quoted 68 lines · Marc Zyngier

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 4 lines

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Thanks Rob for explaining it.

Marc, I wasn't sure if Rob would be okay with including of_irq.h here. Also, I'm trying to keep of/property.c independent of the framework code for now. The long term goal is to see if I can move out most of this into the frameworks. But I want to do that after I sort of some of the larger problems (like getting fw_devlink=on to work on all devices first). Let me know if you have a strong preference for right now, if not, I'd rather keep property.c independent for now.

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Quoted 5 lines

Hmm... I looked at Documentation/devicetree/bindings/interrupt-controller/interrupts.txt and it has no documentation for interrupt-map. There's a bunch of references to it in device specific DT binding documentation but I don't want to rely on those for correctness.

See the DT spec and there's also details on elinux.org. It's only existed since the 1990s.

Quoted 4 lines · Rob Herring

Quoted 2 lines · Saravana Kannan

On Wed, Jan 6, 2021 at 10:52 AM Saravana Kannan <[email protected]> wrote:

Quoted 10 lines

Quoted 9 lines

Quoted 3 lines

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Hmm... I looked at Documentation/devicetree/bindings/interrupt-controller/interrupts.txt and it has no documentation for interrupt-map. There's a bunch of references to it in device specific DT binding documentation but I don't want to rely on those for correctness.

Quoted 2 lines

See the DT spec and there's also details on elinux.org. It's only existed since the 1990s.

Thanks :) Will try to find it there.

Quoted 68 lines · Marc Zyngier

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 4 lines

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Thanks Rob for explaining it.

Marc, I wasn't sure if Rob would be okay with including of_irq.h here. Also, I'm trying to keep of/property.c independent of the framework code for now. The long term goal is to see if I can move out most of this into the frameworks. But I want to do that after I sort of some of the larger problems (like getting fw_devlink=on to work on all devices first). Let me know if you have a strong preference for right now, if not, I'd rather keep property.c independent for now.

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Quoted 4 lines

Quoted 2 lines

But you are right that 'interrupt-map' also needs to be tracked.

And 'interrupts-extended', while we're at it.

This is already handled.

Quoted 1 line

Quoted 1 line

Quoted 3 lines

Quoted 3 lines

I also noticed that we define 'interrupt-parent' as a dependency to parse, but that's wrong. The dependency is where 'interrupts' appears and where 'interrupt-parent' appears is irrelevant.

No, the interrupt-parent parsing is correct and it's needed for interrupt controllers to probe in the right order. But interrupt-parent is also needs to be looked at for parsing "interrupts".

Quoted 101 lines · Saravana Kannan

Quoted 68 lines · Marc Zyngier

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 4 lines

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Thanks Rob for explaining it.

Marc, I wasn't sure if Rob would be okay with including of_irq.h here. Also, I'm trying to keep of/property.c independent of the framework code for now. The long term goal is to see if I can move out most of this into the frameworks. But I want to do that after I sort of some of the larger problems (like getting fw_devlink=on to work on all devices first). Let me know if you have a strong preference for right now, if not, I'd rather keep property.c independent for now.

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Quoted 4 lines

Quoted 2 lines

But you are right that 'interrupt-map' also needs to be tracked.

And 'interrupts-extended', while we're at it.

This is already handled.

Quoted 4 lines

Quoted 4 lines

I also noticed that we define 'interrupt-parent' as a dependency to parse, but that's wrong. The dependency is where 'interrupts' appears and where 'interrupt-parent' appears is irrelevant.

Quoted 4 lines

No, the interrupt-parent parsing is correct and it's needed for interrupt controllers to probe in the right order. But interrupt-parent is also needs to be looked at for parsing "interrupts".

If you parse 'interrupts' for interrupt controllers (which in turn will use 'interrupt-parent'), then you aren't going to need to track 'interrupt-parent' by itself.

Quoted 108 lines · Rob Herring

Quoted 105 lines · Saravana Kannan

Quoted 68 lines · Marc Zyngier

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 4 lines

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Thanks Rob for explaining it.

Marc, I wasn't sure if Rob would be okay with including of_irq.h here. Also, I'm trying to keep of/property.c independent of the framework code for now. The long term goal is to see if I can move out most of this into the frameworks. But I want to do that after I sort of some of the larger problems (like getting fw_devlink=on to work on all devices first). Let me know if you have a strong preference for right now, if not, I'd rather keep property.c independent for now.

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Quoted 4 lines

Quoted 2 lines

But you are right that 'interrupt-map' also needs to be tracked.

And 'interrupts-extended', while we're at it.

This is already handled.

Quoted 4 lines

Quoted 4 lines

I also noticed that we define 'interrupt-parent' as a dependency to parse, but that's wrong. The dependency is where 'interrupts' appears and where 'interrupt-parent' appears is irrelevant.

No, the interrupt-parent parsing is correct and it's needed for interrupt controllers to probe in the right order. But interrupt-parent is also needs to be looked at for parsing "interrupts".

Quoted 3 lines

If you parse 'interrupts' for interrupt controllers (which in turn will use 'interrupt-parent'), then you aren't going to need to track 'interrupt-parent' by itself.

Do all interrupt controllers (that are not the root interrupt controller) need to have "interrupts" property? If yes, then yeah, that makes sense. But I vaguely remember that this wasn't the case for some DT I saw.

Ah, here's one I found.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/mt2701.dtsi#n209

Quoted 114 lines · Saravana Kannan

Quoted 111 lines · Rob Herring

Quoted 97 lines · Saravana Kannan

Quoted 68 lines · Marc Zyngier

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 4 lines

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Thanks Rob for explaining it.

Marc, I wasn't sure if Rob would be okay with including of_irq.h here. Also, I'm trying to keep of/property.c independent of the framework code for now. The long term goal is to see if I can move out most of this into the frameworks. But I want to do that after I sort of some of the larger problems (like getting fw_devlink=on to work on all devices first). Let me know if you have a strong preference for right now, if not, I'd rather keep property.c independent for now.

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Quoted 4 lines

Quoted 2 lines

But you are right that 'interrupt-map' also needs to be tracked.

And 'interrupts-extended', while we're at it.

This is already handled.

Quoted 1 line

Quoted 1 line

Quoted 8 lines

Quoted 3 lines

Quoted 3 lines

I also noticed that we define 'interrupt-parent' as a dependency to parse, but that's wrong. The dependency is where 'interrupts' appears and where 'interrupt-parent' appears is irrelevant.

No, the interrupt-parent parsing is correct and it's needed for interrupt controllers to probe in the right order. But interrupt-parent is also needs to be looked at for parsing "interrupts".

If you parse 'interrupts' for interrupt controllers (which in turn will use 'interrupt-parent'), then you aren't going to need to track 'interrupt-parent' by itself.

Quoted 4 lines

Do all interrupt controllers (that are not the root interrupt controller) need to have "interrupts" property? If yes, then yeah, that makes sense. But I vaguely remember that this wasn't the case for some DT I saw.

There are some cases of stacked controllers where it's implicit.

Quoted 3 lines

Ah, here's one I found.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/mt2701.dtsi#n209

Right, so this is one of several cases of custom interrupt mapping properties (mediatek,ext-irq-range). Really, 'interrupts' or 'interrupt-map' should have been used here, but 'interrupt-map' doesn't really scale well if you have large ranges of interrupts.

To handle the dependency with just 'interrupt-parent', you need to find nodes that are themselves an 'interrupt-parent' and then find their 'interrupt-parent'.

Quoted 121 lines · Rob Herring

Quoted 118 lines · Saravana Kannan

Quoted 103 lines · Rob Herring

Quoted 101 lines · Saravana Kannan

Quoted 68 lines · Marc Zyngier

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 4 lines

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Thanks Rob for explaining it.

Marc, I wasn't sure if Rob would be okay with including of_irq.h here. Also, I'm trying to keep of/property.c independent of the framework code for now. The long term goal is to see if I can move out most of this into the frameworks. But I want to do that after I sort of some of the larger problems (like getting fw_devlink=on to work on all devices first). Let me know if you have a strong preference for right now, if not, I'd rather keep property.c independent for now.

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Quoted 4 lines

Quoted 2 lines

But you are right that 'interrupt-map' also needs to be tracked.

And 'interrupts-extended', while we're at it.

This is already handled.

Quoted 4 lines

Quoted 4 lines

I also noticed that we define 'interrupt-parent' as a dependency to parse, but that's wrong. The dependency is where 'interrupts' appears and where 'interrupt-parent' appears is irrelevant.

Quoted 8 lines

Quoted 4 lines

No, the interrupt-parent parsing is correct and it's needed for interrupt controllers to probe in the right order. But interrupt-parent is also needs to be looked at for parsing "interrupts".

If you parse 'interrupts' for interrupt controllers (which in turn will use 'interrupt-parent'), then you aren't going to need to track 'interrupt-parent' by itself.

Do all interrupt controllers (that are not the root interrupt controller) need to have "interrupts" property? If yes, then yeah, that makes sense. But I vaguely remember that this wasn't the case for some DT I saw.

Quoted 1 line

There are some cases of stacked controllers where it's implicit.

In that case, I think it's good to track interrupt-parent explicitly. Doesn't really hurt anything. We already protect for stuff like making sure a parent doesn't depend on its child, etc.

Quoted 4 lines

Quoted 3 lines

Ah, here's one I found.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/mt2701.dtsi#n209

Quoted 8 lines

Right, so this is one of several cases of custom interrupt mapping properties (mediatek,ext-irq-range). Really, 'interrupts' or 'interrupt-map' should have been used here, but 'interrupt-map' doesn't really scale well if you have large ranges of interrupts.

To handle the dependency with just 'interrupt-parent', you need to find nodes that are themselves an 'interrupt-parent' and then find their 'interrupt-parent'.

Not sure I understand this.

On a side note, if I'm adding device links between a device and the "interrupt-parent" it's pointing to, at worst, I'm having it depend on an interrupt controller its child devices would depend on. This combined with the fact that "weird" links like "parent depending on child",

"non-device node having interrupt-parent",

etc are already
ignored, seems safe to leave in "interrupt-parent" to catch these cases where interrupt controllers don't specify "interrupts" or "interrupt-map"? I don't mind removing it, but maybe we can wait till we get fw_devlink=on and then remove it to see if anything breaks? If nothing breaks, we can remove explicit interrupt-parent parsing?

Going back to interrupt-map, I understand the syntax now. I'm trying to see if I can break up of_irq_parse_raw() into smaller pieces and reuse (call into) some of that code. While doing that I see that when "address-cells" isn't present, the "addrsize" is initialized to 2 [1] but when "address-cells" isn't present, the "newaddrsize" is initialized to 0 [1]. Why is the default value of #address-cells two different numbers?

Quoted 123 lines · Saravana Kannan

Quoted 121 lines · Rob Herring

Quoted 118 lines · Saravana Kannan

Quoted 103 lines · Rob Herring

Quoted 101 lines · Saravana Kannan

Quoted 68 lines · Marc Zyngier

Quoted 65 lines · Rob Herring

Quoted 62 lines · Marc Zyngier

Quoted 54 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

---
Rob/Greg,

This might need to go into driver-core to avoid conflict due to fw_devlink refactor series that merged there.

Thanks,
Saravana

 drivers/of/property.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/of/property.c +16 -0

index 5f9eed79a8aa..e56a5eae0a0b 100644
@@ -1271,6 +1271,22 @@ static struct device_node
 *parse_iommu_maps(struct device_node *np,
  return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_interrupts(struct device_node *np,
+                                     const char *prop_name, int index)
+{
+ struct device_node *sup;
+
+ if (strcmp(prop_name, "interrupts") || index)
+         return NULL;
+
+ of_node_get(np);
+ while (np && !(sup = of_parse_phandle(np, "interrupt-parent", 0)))
+         np = of_get_next_parent(np);
+ of_node_put(np);
+
+ return sup;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
  { .parse_prop = parse_clocks, },

{ .parse_prop = parse_interconnects, },

drivers/of/property.c +1 -0

@@ -1296,6 +1312,7 @@ static const struct supplier_bindings
 of_supplier_bindings[] = {
  { .parse_prop = parse_pinctrl6, },
  { .parse_prop = parse_pinctrl7, },
  { .parse_prop = parse_pinctrl8, },
+ { .parse_prop = parse_interrupts, },
  { .parse_prop = parse_regulators, },
  { .parse_prop = parse_gpio, },

Quoted 1 line

{ .parse_prop = parse_gpios, },

You don't really describe what this is for so I'm only guessing from the context. If you want to follow the interrupt hierarchy, "interrupt-parent" isn't enough. You also need to track things like interrupt-map, or anything that carries a phandle to an interrupt controller.

Quoted 4 lines

Quoted 2 lines

We don't need to follow the hierarchy, we just need the immediate dependencies.

Indeed. I also wonder why this isn't just a irq_find_parent() call, TBH.

Thanks Rob for explaining it.

Marc, I wasn't sure if Rob would be okay with including of_irq.h here. Also, I'm trying to keep of/property.c independent of the framework code for now. The long term goal is to see if I can move out most of this into the frameworks. But I want to do that after I sort of some of the larger problems (like getting fw_devlink=on to work on all devices first). Let me know if you have a strong preference for right now, if not, I'd rather keep property.c independent for now.

I wasn't aware of interrupt-map until a few weeks ago and didn't know it carried phandles. I can add support for that too. There's no reason for all of them to go in one patch though.

Quoted 4 lines

Quoted 2 lines

But you are right that 'interrupt-map' also needs to be tracked.

And 'interrupts-extended', while we're at it.

This is already handled.

Quoted 4 lines

Quoted 4 lines

I also noticed that we define 'interrupt-parent' as a dependency to parse, but that's wrong. The dependency is where 'interrupts' appears and where 'interrupt-parent' appears is irrelevant.

Quoted 8 lines

Quoted 4 lines

No, the interrupt-parent parsing is correct and it's needed for interrupt controllers to probe in the right order. But interrupt-parent is also needs to be looked at for parsing "interrupts".

If you parse 'interrupts' for interrupt controllers (which in turn will use 'interrupt-parent'), then you aren't going to need to track 'interrupt-parent' by itself.

Do all interrupt controllers (that are not the root interrupt controller) need to have "interrupts" property? If yes, then yeah, that makes sense. But I vaguely remember that this wasn't the case for some DT I saw.

Quoted 5 lines

Quoted 1 line

There are some cases of stacked controllers where it's implicit.

In that case, I think it's good to track interrupt-parent explicitly. Doesn't really hurt anything. We already protect for stuff like making sure a parent doesn't depend on its child, etc.

Given the interrupt controller is probably always going to be there, it probably doesn't really matter. But the dependency is wrong.

Does this work:

bus {
interrupt-parent = <&root_int>;

  root_int: interrupt-controller {
  };

  device {
    interrupts = <1>;

};
};

The only dependency here should be /bus/device -> root_int.

Quoted 13 lines

Quoted 4 lines

Quoted 3 lines

Ah, here's one I found.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/mt2701.dtsi#n209

Quoted 8 lines

Right, so this is one of several cases of custom interrupt mapping properties (mediatek,ext-irq-range). Really, 'interrupts' or 'interrupt-map' should have been used here, but 'interrupt-map' doesn't really scale well if you have large ranges of interrupts.

To handle the dependency with just 'interrupt-parent', you need to find nodes that are themselves an 'interrupt-parent' and then find their 'interrupt-parent'.

Quoted 7 lines

Not sure I understand this.

On a side note, if I'm adding device links between a device and the "interrupt-parent" it's pointing to, at worst, I'm having it depend on an interrupt controller its child devices would depend on. This combined with the fact that "weird" links like "parent depending on child",

The above would be this case if you treat root_int as a dependency of bus. It's not, but I guess that plus the child dependency on parent makes interrupt dependencies mostly work.

Quoted 1 line

"non-device node having interrupt-parent",

Yes, that's probably 95% of interrupt-parent cases. It's either at the root or bus node.

Quoted 6 lines

etc are already
ignored, seems safe to leave in "interrupt-parent" to catch these cases where interrupt controllers don't specify "interrupts" or "interrupt-map"? I don't mind removing it, but maybe we can wait till we get fw_devlink=on and then remove it to see if anything breaks? If nothing breaks, we can remove explicit interrupt-parent parsing?

Okay, as long as we're breaking platforms once potentially.

Quoted 7 lines

Going back to interrupt-map, I understand the syntax now. I'm trying to see if I can break up of_irq_parse_raw() into smaller pieces and reuse (call into) some of that code. While doing that I see that when "address-cells" isn't present, the "addrsize" is initialized to 2 [1] but when "address-cells" isn't present, the "newaddrsize" is initialized to 0 [1]. Why is the default value of #address-cells two different numbers?

It's some ancient code that needs to handle quirks of ancient systems.

The first one is the #address-cells for the device's bus. The second is #address-cells in the interrupt-controller/map. It's complicated because interrupt hierarchies used to follow the bus hierarchies. Generally in modern systems #address-cells used for interrupt-map purposes (the 2nd case) will always be 0.

Note that there's generic nexus code to support gpio-map. It works similarly though without the address-cells part. Any refactoring should take that into account.

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

Hi Saravana,

Quoted 5 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

Thanks for your patch!

This does not seem to add all links. I see links being created to the secondary interrupt controller (e61c0000 "renesas,irqc"), but not to the primary interrupt controller (GIC)

Which is good, as the GIC driver is not a platform_driver, and thus creating links would break everything ;-)

BTW, I'd _love_ the GIC driver to be a platform_driver, as the GIC is part of a power/and or clock domain on Renesas SoCs...

Quoted 5 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

Thanks for your patch!

This does not seem to add all links. I see links being created to the secondary interrupt controller (e61c0000 "renesas,irqc"), but not to the primary interrupt controller (GIC)

Which is good, as the GIC driver is not a platform_driver, and thus creating links would break everything ;-)

Hi Saravana,

Quoted 13 lines · Geert Uytterhoeven

Quoted 5 lines · Saravana Kannan

Add support for creating device links out of interrupts property.

Cc Marc Zyngier <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Saravana Kannan <[email protected]>

Thanks for your patch!

This does not seem to add all links. I see links being created to the secondary interrupt controller (e61c0000 "renesas,irqc"), but not to the primary interrupt controller (GIC)

Quoted 2 lines

Which is good, as the GIC driver is not a platform_driver, and thus creating links would break everything ;-)

of_link_to_phandle() ignores device nodes where OF_POPULATED is set, and of_irq_init() sets that flag. Hence the GIC is ignored.