of/irq: create interrupts-extended-2 property

The new interrupts-extended property, which reuses the phandle+arguments pattern used by GPIOs and other core bindings, still have some issue.

If an SoC have already specifiy interrupt and a board want to add specific interrupt such as GPIO (which can be optionnal) be need to re-define interrupts-extended. So allow to have an optionnale interrupts-extended-2 property.

Today the problem is ofen solve by defining a gpio-xxx property and then do a gpio_to_irq in the C code. *Which is wrong!!*. We need to describe the IRQ.

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Cc Grant Likely <[email protected]>

Cc Rob Herring <[email protected]>

---

We have the same issue on pinctrl

 .../devicetree/bindings/interrupt-controller/interrupts.txt    |  4 ++++
 drivers/of/irq.c                                               | 10 ++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/interrupt-controller/interrupts.txt +4 -0

index 1486497..5d559fd 100644
@@ -25,8 +25,12 @@ to reference multiple interrupt parents. Each entry in this property contains
 both the parent phandle and the interrupt specifier. "interrupts-extended"
 should only be used when a device has multiple interrupt parents.
 
+The "interrupts-extended-2" allow to extend at board level node interrupt without
+having to re-define the SoC interrupts.
+
   Example:
 	interrupts-extended = <&intc1 5 1>, <&intc2 1 0>;
+	interrupts-extended-2 = <&intc1 6 1>
 
 A device node may contain either "interrupts" or "interrupts-extended", but not
 both. If both properties are present, then the operating system should log an

drivers/of/irq.c +0 -0

index 786b0b4..bc36710 100644

Quoted 8 lines · Jean-Christophe PLAGNIOL-VILLARD

The new interrupts-extended property, which reuses the phandle+arguments pattern used by GPIOs and other core bindings, still have some issue.

If an SoC have already specifiy interrupt and a board want to add specific interrupt such as GPIO (which can be optionnal) be need to re-define interrupts-extended. So allow to have an optionnale interrupts-extended-2 property.

NAK.

This is a hack that works around a dts organisation issue. This is _not_ a binding or parsing issue.

Properties can be overridden - just describe all of the interrupts in the final dts file.

Quoted 36 lines

Today the problem is ofen solve by defining a gpio-xxx property and then do a gpio_to_irq in the C code. *Which is wrong!!*. We need to describe the IRQ.

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Cc Grant Likely <[email protected]>

Cc Rob Herring <[email protected]>

---

We have the same issue on pinctrl

 .../devicetree/bindings/interrupt-controller/interrupts.txt    |  4 ++++
 drivers/of/irq.c                                               | 10 ++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/interrupt-controller/interrupts.txt +4 -0

index 1486497..5d559fd 100644
@@ -25,8 +25,12 @@ to reference multiple interrupt parents. Each entry in this property contains
 both the parent phandle and the interrupt specifier. "interrupts-extended"
 should only be used when a device has multiple interrupt parents.
 
+The "interrupts-extended-2" allow to extend at board level node interrupt without
+having to re-define the SoC interrupts.
+
   Example:
 	interrupts-extended = <&intc1 5 1>, <&intc2 1 0>;
+	interrupts-extended-2 = <&intc1 6 1>
 
 A device node may contain either "interrupts" or "interrupts-extended", but not
 both. If both properties are present, then the operating system should log an

drivers/of/irq.c +0 -0

index 786b0b4..bc36710 100644

drivers/of/irq.c int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_ar · line 313

Quoted 9 lines

drivers/of/irq.c +4 -2

@@ -307,8 +307,14 @@ int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_ar
 		/* Try the new-style interrupts-extended */
 		res = of_parse_phandle_with_args(device, "interrupts-extended",
 						"#interrupt-cells", index, out_irq);
-		if (res)
-			return -EINVAL;
+		if (res) {
+			/* Try the new-style interrupts-extended-2 */
+			res = of_parse_phandle_with_args(device, "interrupts-extended-2",
+							"#interrupt-cells", index, out_irq);

This also breaks error reporting.

What if you have an interrupt in the middle of interrupts-extended which fails to parse? This will jump to the interrupts-extended-2 property and grab the wrong interrupt rather than propagating the error.

Thanks,
Mark.

Quoted 10 lines · Mark Rutland

Quoted 8 lines · Jean-Christophe PLAGNIOL-VILLARD

The new interrupts-extended property, which reuses the phandle+arguments pattern used by GPIOs and other core bindings, still have some issue.

If an SoC have already specifiy interrupt and a board want to add specific interrupt such as GPIO (which can be optionnal) be need to re-define interrupts-extended. So allow to have an optionnale interrupts-extended-2 property.

Quoted 4 lines

NAK.

This is a hack that works around a dts organisation issue. This is _not_ a binding or parsing issue.

So the DT is stupid. Yes w have board and SoC information
so we do not want to duplicate them. Having a way to descript SoC vs board specific information need to be provided.

Quoted 4 lines

Properties can be overridden - just describe all of the interrupts in the final dts file.

this is wrong, which mean you duplicate informaation and duplicate bug and fixes

Quoted 11 lines · Jean-Christophe PLAGNIOL-VILLARD

Quoted 10 lines · Mark Rutland

Quoted 8 lines · Jean-Christophe PLAGNIOL-VILLARD

The new interrupts-extended property, which reuses the phandle+arguments pattern used by GPIOs and other core bindings, still have some issue.

If an SoC have already specifiy interrupt and a board want to add specific interrupt such as GPIO (which can be optionnal) be need to re-define interrupts-extended. So allow to have an optionnale interrupts-extended-2 property.

Quoted 8 lines

Quoted 4 lines

NAK.

This is a hack that works around a dts organisation issue. This is _not_ a binding or parsing issue.

So the DT is stupid. Yes w have board and SoC information
so we do not want to duplicate them. Having a way to descript SoC vs board specific information need to be provided.

I agree that the current situation isn't fantastic. That doesn't make the DT stupid. There are other solutions to this problem.

Quoted 4 lines

Quoted 4 lines

Properties can be overridden - just describe all of the interrupts in the final dts file.

Quoted 2 lines

this is wrong, which mean you duplicate informaation and duplicate bug and fixes

It's certainly not nice, but it doesn't make it wrong.

The same problem can be found with any other list property that varies per-board. Hacking the parsing of each property is not a solution.

Today the only way to implement what you want is to have the list in the final file.

One way of working with that would be to have the common interrupts described in a shared macro in the soc file:

#define SOC_COMMON_INTERRUPTS <&intc1 5 1>, <&intc2 1 0>

Then the soc file can have:

	interrupts = SOC_COMMON_INTERRUPTS;

And the board can have:

	interrupts = SOC_COMMON_INTERRUPTS, <&intc1 6 1>;

That gains functionality equivalent to your proposal, without introducing new code or new edge cases in interrupt parsing. However, it does make it unclear how many interrupts are described, and it won't interact well with interrupt-names.

Another, more invasive option would be extend the dts syntax and teach dtc to handle property appending. Then the soc dts could stay as it is, and the board dts could have something like:

	/append-property/ interrupts = <&intc1 6 1>;
	/append-property/ interrupt-names = "board-specific-irq";

Both these options solve the issue at the source, are general to other properties, and allow more than one level of hierarchy (the proposed interrupts-extended-2 only allows one level).

Thanks,
Mark.

Quoted 11 lines · Jean-Christophe PLAGNIOL-VILLARD

Quoted 10 lines · Mark Rutland

Quoted 8 lines · Jean-Christophe PLAGNIOL-VILLARD

The new interrupts-extended property, which reuses the phandle+arguments pattern used by GPIOs and other core bindings, still have some issue.

If an SoC have already specifiy interrupt and a board want to add specific interrupt such as GPIO (which can be optionnal) be need to re-define interrupts-extended. So allow to have an optionnale interrupts-extended-2 property.

Quoted 8 lines

Quoted 4 lines

NAK.

This is a hack that works around a dts organisation issue. This is _not_ a binding or parsing issue.

So the DT is stupid. Yes w have board and SoC information
so we do not want to duplicate them. Having a way to descript SoC vs board specific information need to be provided.

I agree that the current situation isn't fantastic. That doesn't make the DT stupid. There are other solutions to this problem.

Quoted 6 lines

Quoted 4 lines

Properties can be overridden - just describe all of the interrupts in the final dts file.

this is wrong, which mean you duplicate informaation and duplicate bug and fixes

It's certainly not nice, but it doesn't make it wrong.

The same problem can be found with any other list property that varies per-board. Hacking the parsing of each property is not a solution.

Today the only way to implement what you want is to have the list in the final file.

One way of working with that would be to have the common interrupts described in a shared macro in the soc file:

#define SOC_COMMON_INTERRUPTS <&intc1 5 1>, <&intc2 1 0>

Then the soc file can have:

        interrupts = SOC_COMMON_INTERRUPTS;

And the board can have:

        interrupts = SOC_COMMON_INTERRUPTS, <&intc1 6 1>;

Quoted 36 lines · Mark Rutland

Quoted 11 lines · Jean-Christophe PLAGNIOL-VILLARD

Quoted 10 lines · Mark Rutland

Quoted 8 lines · Jean-Christophe PLAGNIOL-VILLARD

The new interrupts-extended property, which reuses the phandle+arguments pattern used by GPIOs and other core bindings, still have some issue.

If an SoC have already specifiy interrupt and a board want to add specific interrupt such as GPIO (which can be optionnal) be need to re-define interrupts-extended. So allow to have an optionnale interrupts-extended-2 property.

Quoted 8 lines

Quoted 4 lines

NAK.

This is a hack that works around a dts organisation issue. This is _not_ a binding or parsing issue.

So the DT is stupid. Yes w have board and SoC information
so we do not want to duplicate them. Having a way to descript SoC vs board specific information need to be provided.

I agree that the current situation isn't fantastic. That doesn't make the DT stupid. There are other solutions to this problem.

Quoted 6 lines

Quoted 4 lines

Properties can be overridden - just describe all of the interrupts in the final dts file.

this is wrong, which mean you duplicate informaation and duplicate bug and fixes

It's certainly not nice, but it doesn't make it wrong.

The same problem can be found with any other list property that varies per-board. Hacking the parsing of each property is not a solution.

Quoted 7 lines

Today the only way to implement what you want is to have the list in the final file.

One way of working with that would be to have the common interrupts described in a shared macro in the soc file:

#define SOC_COMMON_INTERRUPTS <&intc1 5 1>, <&intc2 1 0>

Yuck. Please, let's not encourage this pattern.

Quoted 7 lines

Then the soc file can have:

        interrupts = SOC_COMMON_INTERRUPTS;

And the board can have:

        interrupts = SOC_COMMON_INTERRUPTS, <&intc1 6 1>;

Unless this is solved in dtc include mechanism, just duplicate the interrupts in each board file. I'd rather have the duplication than continued churn of refactoring dtsi files.

Rob

Another, more invasive option would be extend the dts syntax and teach dtc to handle property appending. Then the soc dts could stay as it is, and the board dts could have something like:

	/append-property/ interrupts = <&intc1 6 1>;
	/append-property/ interrupt-names = "board-specific-irq";

Both these options solve the issue at the source, are general to other properties, and allow more than one level of hierarchy (the proposed interrupts-extended-2 only allows one level).

From 88be0036b6a966bd7506f58e3cb9ce9ea4c5ac48 Mon Sep 17 00:00:00 2001

Quoted 8 lines

Another, more invasive option would be extend the dts syntax and teach dtc to handle property appending. Then the soc dts could stay as it is, and the board dts could have something like:

	/append-property/ interrupts = <&intc1 6 1>;
	/append-property/ interrupt-names = "board-specific-irq";

Quoted 3 lines

Both these options solve the issue at the source, are general to other properties, and allow more than one level of hierarchy (the proposed interrupts-extended-2 only allows one level).

I've just had a go at implementing the append-property mechanism above in dtc, and it was far easier than I expected (patch below).

Does anyone have any issues with the /append-property/ idea?

Thanks,
Mark.

---->8----

From 88be0036b6a966bd7506f58e3cb9ce9ea4c5ac48 Mon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Wed, 15 Jan 2014 15:43:51 +0000
Subject: [PATCH] dtc: add ability to append properties

When dealing with hierarchies of dtsi files, handling minute differences between individual boards can become very painful. Adding a single board-specific interrupt requires duplicating the entire interrupts property, which requires duplication of common values. This makes bug fixing painful and if not handled very carefully files diverge rapdily.

To ameliorate this, this patch adds the ability to append properties, allowing board files to describe only the additional values required in a property. This functionality also works with strings, so parallel properties like interupts and interrupt-names stay in sync. Properties may be appended multiple times, and deleting properties clears all previous appended values.

To append a property, secondary definitions must be prefixed with /append-property/. This is longer than a possible '+=' syntax, but makes it far easier to spot when appending behaviour is requested, and so hopefully will lead to fewer buggy dts files.

For example, if the following dts fragements are compiled together:

  / {
      interrupts = <0>, <1>;
      interrupt-names = "zero", "one";
  };

  / {
      /append-property/ interrupts = <2>, <3>;
      /append-property/ interrupt-names = "two", "three";
  };

They will result in a dtb equivalent to the following dts fragment:

  / {
      interrupts = <0>, <1>, <2>, <3>;
      interrupt-names = "zero", "one", "two", "three";
  };

Signed-off-by Mark Rutland <[email protected]>

---
 dtc-lexer.l  |  7 +++++++
 dtc-parser.y |  5 +++++
 dtc.h        |  2 ++
 livetree.c   | 15 ++++++++++++++-
 4 files changed, 28 insertions(+), 1 deletion(-)

dtc-lexer.l +7 -0

index 0cd7e67..7989abe 100644
@@ -131,6 +131,13 @@ static bool pop_input_file(void);
 			return DT_DEL_PROP;
 		}
 
+<*>"/append-property/"	{
+			DPRINT("Keyword: /append-property/\n");
+			DPRINT("<PROPNODENAME>\n");
+			BEGIN(PROPNODENAME);
+			return DT_APPEND_PROP;
+		}
+
 <*>"/delete-node/"	{
 			DPRINT("Keyword: /delete-node/\n");
 			DPRINT("<PROPNODENAME>\n");

dtc-parser.y +5 -0

index 4864631..a8409ed 100644
@@ -63,6 +63,7 @@ static unsigned char eval_char_literal(const char *s);
 %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
 %token DT_BITS
 %token DT_DEL_PROP
+%token DT_APPEND_PROP
 %token DT_DEL_NODE
 %token <propnodename> DT_PROPNODENAME
 %token <literal> DT_LITERAL

@@ -195,6 +196,10 @@ propdef:
 		{
 			$$ = build_property($1, empty_data);
 		}
+	| DT_APPEND_PROP DT_PROPNODENAME '=' propdata ';'
+		{
+			$$ = build_property_append($2, $4);
+		}
 	| DT_DEL_PROP DT_PROPNODENAME ';'
 		{
 			$$ = build_property_delete($2);

dtc.h +2 -0

index 20e4d56..8687530 100644
@@ -133,6 +133,7 @@ struct label {
 };
 
 struct property {
+	bool appended;
 	bool deleted;
 	char *name;
 	struct data val;

@@ -186,6 +187,7 @@ void delete_labels(struct label **labels);
 
 struct property *build_property(char *name, struct data val);
 struct property *build_property_delete(char *name);
+struct property *build_property_append(char *name, struct data val);
 struct property *chain_property(struct property *first, struct property *list);
 struct property *reverse_properties(struct property *first);
 

livetree.c +14 -1

index b61465f..894e42b 100644
@@ -74,6 +74,15 @@ struct property *build_property_delete(char *name)
 	return new;
 }
 
+struct property *build_property_append(char *name, struct data val)
+{
+	struct property *new = build_property(name, val);
+
+	new->appended = 1;
+
+	return new;
+}
+
 struct property *chain_property(struct property *first, struct property *list)
 {
 	assert(first->next == NULL);

@@ -167,7 +176,11 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 				for_each_label_withdel(new_prop->labels, l)
 					add_label(&old_prop->labels, l->label);
 
-				old_prop->val = new_prop->val;
+				if (new_prop->appended)
+					old_prop->val = data_merge(old_prop->val, new_prop->val);
+				else
+					old_prop->val = new_prop->val;
+
 				old_prop->deleted = 0;
 				free(new_prop);
 				new_prop = NULL;

Signature · 1 line

1.8.1.1

Quoted 8 lines

Another, more invasive option would be extend the dts syntax and teach dtc to handle property appending. Then the soc dts could stay as it is, and the board dts could have something like:

	/append-property/ interrupts = <&intc1 6 1>;
	/append-property/ interrupt-names = "board-specific-irq";

Quoted 3 lines

Both these options solve the issue at the source, are general to other properties, and allow more than one level of hierarchy (the proposed interrupts-extended-2 only allows one level).

I've just had a go at implementing the append-property mechanism above in dtc, and it was far easier than I expected (patch below).

Does anyone have any issues with the /append-property/ idea?

Thanks,
Mark.

---->8----

From 88be0036b6a966bd7506f58e3cb9ce9ea4c5ac48 Mon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Wed, 15 Jan 2014 15:43:51 +0000
Subject: [PATCH] dtc: add ability to append properties

When dealing with hierarchies of dtsi files, handling minute differences between individual boards can become very painful. Adding a single board-specific interrupt requires duplicating the entire interrupts property, which requires duplication of common values. This makes bug fixing painful and if not handled very carefully files diverge rapdily.

To ameliorate this, this patch adds the ability to append properties, allowing board files to describe only the additional values required in a property. This functionality also works with strings, so parallel properties like interupts and interrupt-names stay in sync. Properties may be appended multiple times, and deleting properties clears all previous appended values.

To append a property, secondary definitions must be prefixed with /append-property/. This is longer than a possible '+=' syntax, but makes it far easier to spot when appending behaviour is requested, and so hopefully will lead to fewer buggy dts files.

For example, if the following dts fragements are compiled together:

  / {
      interrupts = <0>, <1>;
      interrupt-names = "zero", "one";
  };

  / {
      /append-property/ interrupts = <2>, <3>;
      /append-property/ interrupt-names = "two", "three";
  };

They will result in a dtb equivalent to the following dts fragment:

  / {
      interrupts = <0>, <1>, <2>, <3>;
      interrupt-names = "zero", "one", "two", "three";
  };

Signed-off-by Mark Rutland <[email protected]>

---
 dtc-lexer.l  |  7 +++++++
 dtc-parser.y |  5 +++++
 dtc.h        |  2 ++
 livetree.c   | 15 ++++++++++++++-
 4 files changed, 28 insertions(+), 1 deletion(-)

dtc-lexer.l +7 -0

index 0cd7e67..7989abe 100644
@@ -131,6 +131,13 @@ static bool pop_input_file(void);
 			return DT_DEL_PROP;
 		}
 
+<*>"/append-property/"	{
+			DPRINT("Keyword: /append-property/\n");
+			DPRINT("<PROPNODENAME>\n");
+			BEGIN(PROPNODENAME);
+			return DT_APPEND_PROP;
+		}
+
 <*>"/delete-node/"	{
 			DPRINT("Keyword: /delete-node/\n");
 			DPRINT("<PROPNODENAME>\n");

dtc-parser.y +5 -0

index 4864631..a8409ed 100644
@@ -63,6 +63,7 @@ static unsigned char eval_char_literal(const char *s);
 %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
 %token DT_BITS
 %token DT_DEL_PROP
+%token DT_APPEND_PROP
 %token DT_DEL_NODE
 %token <propnodename> DT_PROPNODENAME
 %token <literal> DT_LITERAL

@@ -195,6 +196,10 @@ propdef:
 		{
 			$$ = build_property($1, empty_data);
 		}
+	| DT_APPEND_PROP DT_PROPNODENAME '=' propdata ';'
+		{
+			$$ = build_property_append($2, $4);
+		}
 	| DT_DEL_PROP DT_PROPNODENAME ';'
 		{
 			$$ = build_property_delete($2);

dtc.h +2 -0

index 20e4d56..8687530 100644
@@ -133,6 +133,7 @@ struct label {
 };
 
 struct property {
+	bool appended;
 	bool deleted;
 	char *name;
 	struct data val;

@@ -186,6 +187,7 @@ void delete_labels(struct label **labels);
 
 struct property *build_property(char *name, struct data val);
 struct property *build_property_delete(char *name);
+struct property *build_property_append(char *name, struct data val);
 struct property *chain_property(struct property *first, struct property *list);
 struct property *reverse_properties(struct property *first);
 

livetree.c +14 -1

index b61465f..894e42b 100644
@@ -74,6 +74,15 @@ struct property *build_property_delete(char *name)
 	return new;
 }
 
+struct property *build_property_append(char *name, struct data val)
+{
+	struct property *new = build_property(name, val);
+
+	new->appended = 1;
+
+	return new;
+}
+
 struct property *chain_property(struct property *first, struct property *list)
 {
 	assert(first->next == NULL);

@@ -167,7 +176,11 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 				for_each_label_withdel(new_prop->labels, l)
 					add_label(&old_prop->labels, l->label);
 
-				old_prop->val = new_prop->val;
+				if (new_prop->appended)
+					old_prop->val = data_merge(old_prop->val, new_prop->val);
+				else
+					old_prop->val = new_prop->val;
+
 				old_prop->deleted = 0;
 				free(new_prop);
 				new_prop = NULL;

Signature · 1 line

1.8.1.1

Quoted 12 lines · Mark Rutland

Quoted 11 lines

Another, more invasive option would be extend the dts syntax and teach dtc to handle property appending. Then the soc dts could stay as it is, and the board dts could have something like:

    /append-property/ interrupts = <&intc1 6 1>;
    /append-property/ interrupt-names = "board-specific-irq";

Both these options solve the issue at the source, are general to other properties, and allow more than one level of hierarchy (the proposed interrupts-extended-2 only allows one level).

Quoted 4 lines

I've just had a go at implementing the append-property mechanism above in dtc, and it was far easier than I expected (patch below).

Does anyone have any issues with the /append-property/ idea?

I think that is reasonable.

Quoted 13 lines · Grant Likely

Quoted 12 lines · Mark Rutland

Quoted 11 lines

Another, more invasive option would be extend the dts syntax and teach dtc to handle property appending. Then the soc dts could stay as it is, and the board dts could have something like:

    /append-property/ interrupts = <&intc1 6 1>;
    /append-property/ interrupt-names = "board-specific-irq";

Both these options solve the issue at the source, are general to other properties, and allow more than one level of hierarchy (the proposed interrupts-extended-2 only allows one level).

Quoted 6 lines

Quoted 4 lines

I've just had a go at implementing the append-property mechanism above in dtc, and it was far easier than I expected (patch below).

Does anyone have any issues with the /append-property/ idea?

I think that is reasonable.

The main problem with this (same for clocks) is if you need to append something with a name when the original didn't have any.

Reordering entries might not work for interrupts, since the bindings might have requirements on order.

I'm not aware of a good solution for this. Suggestions welcome.

Quoted 16 lines · Grant Likely

Quoted 15 lines · Mark Rutland

Quoted 11 lines

Another, more invasive option would be extend the dts syntax and teach dtc to handle property appending. Then the soc dts could stay as it is, and the board dts could have something like:

    /append-property/ interrupts = <&intc1 6 1>;
    /append-property/ interrupt-names = "board-specific-irq";

Both these options solve the issue at the source, are general to other properties, and allow more than one level of hierarchy (the proposed interrupts-extended-2 only allows one level).

I've just had a go at implementing the append-property mechanism above in dtc, and it was far easier than I expected (patch below).

Quoted 3 lines

Quoted 1 line

Does anyone have any issues with the /append-property/ idea?

I think that is reasonable.

The main problem with this (same for clocks) is if you need to append something with a name when the original didn't have any.

Reordering entries might not work for interrupts, since the bindings might have requirements on order.

I'm not aware of a good solution for this. Suggestions welcome.

Quoted 17 lines · Olof Johansson

Quoted 16 lines · Grant Likely

Quoted 15 lines · Mark Rutland

Quoted 11 lines

Another, more invasive option would be extend the dts syntax and teach dtc to handle property appending. Then the soc dts could stay as it is, and the board dts could have something like:

    /append-property/ interrupts = <&intc1 6 1>;
    /append-property/ interrupt-names = "board-specific-irq";

Both these options solve the issue at the source, are general to other properties, and allow more than one level of hierarchy (the proposed interrupts-extended-2 only allows one level).

I've just had a go at implementing the append-property mechanism above in dtc, and it was far easier than I expected (patch below).

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Does anyone have any issues with the /append-property/ idea?

I think that is reasonable.

The main problem with this (same for clocks) is if you need to append something with a name when the original didn't have any.

Can you not just add a name in the original file? I assume we're not going to use this to adjust dts files we're not already in full control of.

Quoted 3 lines

Reordering entries might not work for interrupts, since the bindings might have requirements on order.

That's a fair point.

Do we currently have any optional/board-specific interrupts which must appear at the start or middle of the list?

For those, could we add names? The kernel should be abel to fall back to ordering if names aren't present, and we can recommend a particular ordering for compatiblity with older kernels.

As a general preventative measure it would be nice to have named elements whenever elements can be optional.

Quoted 3 lines

I'm not aware of a good solution for this. Suggestions welcome.

Me neither. Prepending and appending is easy.

Inserting and/or modifying the list requires knowledge of the size of each element (and for variable-sized entries requires knowledge of the particular binding, which we cannot embed in dtc).

I suspect adding richer syntax for modifying properties in arbitrary ways will devolve into a turing tarpit.

Thanks,
Mark.

Quoted 18 lines · Grant Likely

Quoted 16 lines · Mark Rutland

Quoted 11 lines

Another, more invasive option would be extend the dts syntax and teach dtc to handle property appending. Then the soc dts could stay as it is, and the board dts could have something like:

    /append-property/ interrupts = <&intc1 6 1>;
    /append-property/ interrupt-names = "board-specific-irq";

Both these options solve the issue at the source, are general to other properties, and allow more than one level of hierarchy (the proposed interrupts-extended-2 only allows one level).

I've just had a go at implementing the append-property mechanism above in dtc, and it was far easier than I expected (patch below).

Does anyone have any issues with the /append-property/ idea?

Quoted 1 line

I think that is reasonable.

The main problem with this (same for clocks) is if you need to append something with a name when the original didn't have any.

Reordering entries might not work for interrupts, since the bindings might have requirements on order.

Quoted 8 lines · Jean-Christophe PLAGNIOL-VILLARD

The new interrupts-extended property, which reuses the phandle+arguments pattern used by GPIOs and other core bindings, still have some issue.

If an SoC have already specifiy interrupt and a board want to add specific interrupt such as GPIO (which can be optionnal) be need to re-define interrupts-extended. So allow to have an optionnale interrupts-extended-2 property.

NAK.

This is a hack that works around a dts organisation issue. This is _not_ a binding or parsing issue.

Properties can be overridden - just describe all of the interrupts in the final dts file.