* Re: [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-15 16:12 ` Mark Rutland
0 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2014-01-15 16:12 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: devicetree@vger.kernel.org, Linus Walleij,
grant.likely@linaro.org, linux-arm-kernel@lists.infradead.org,
David Gibson
>
> 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?
Thanks,
Mark.
---->8----
>From 88be0036b6a966bd7506f58e3cb9ce9ea4c5ac48 Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
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 <mark.rutland@arm.com>
---
dtc-lexer.l | 7 +++++++
dtc-parser.y | 5 +++++
dtc.h | 2 ++
livetree.c | 15 ++++++++++++++-
4 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/dtc-lexer.l b/dtc-lexer.l
index 0cd7e67..7989abe 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -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");
diff --git a/dtc-parser.y b/dtc-parser.y
index 4864631..a8409ed 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -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);
diff --git a/dtc.h b/dtc.h
index 20e4d56..8687530 100644
--- a/dtc.h
+++ b/dtc.h
@@ -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);
diff --git a/livetree.c b/livetree.c
index b61465f..894e42b 100644
--- a/livetree.c
+++ b/livetree.c
@@ -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;
--
1.8.1.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
2014-01-15 16:12 ` Mark Rutland
@ 2014-01-15 18:08 ` Jean-Christophe PLAGNIOL-VILLARD
-1 siblings, 0 replies; 25+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-15 18:08 UTC (permalink / raw)
To: linux-arm-kernel
On 16:12 Wed 15 Jan , Mark Rutland wrote:
> >
> > 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?
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
you finish it before me :(
Best Regards,
J.
>
> Thanks,
> Mark.
>
> ---->8----
>
> From 88be0036b6a966bd7506f58e3cb9ce9ea4c5ac48 Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> 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 <mark.rutland@arm.com>
> ---
> dtc-lexer.l | 7 +++++++
> dtc-parser.y | 5 +++++
> dtc.h | 2 ++
> livetree.c | 15 ++++++++++++++-
> 4 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 0cd7e67..7989abe 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -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");
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 4864631..a8409ed 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -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);
> diff --git a/dtc.h b/dtc.h
> index 20e4d56..8687530 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -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);
>
> diff --git a/livetree.c b/livetree.c
> index b61465f..894e42b 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -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;
> --
> 1.8.1.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-15 18:08 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 25+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-15 18:08 UTC (permalink / raw)
To: Mark Rutland
Cc: devicetree@vger.kernel.org, Linus Walleij,
grant.likely@linaro.org, linux-arm-kernel@lists.infradead.org,
David Gibson
On 16:12 Wed 15 Jan , Mark Rutland wrote:
> >
> > 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?
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
you finish it before me :(
Best Regards,
J.
>
> Thanks,
> Mark.
>
> ---->8----
>
> From 88be0036b6a966bd7506f58e3cb9ce9ea4c5ac48 Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> 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 <mark.rutland@arm.com>
> ---
> dtc-lexer.l | 7 +++++++
> dtc-parser.y | 5 +++++
> dtc.h | 2 ++
> livetree.c | 15 ++++++++++++++-
> 4 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 0cd7e67..7989abe 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -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");
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 4864631..a8409ed 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -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);
> diff --git a/dtc.h b/dtc.h
> index 20e4d56..8687530 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -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);
>
> diff --git a/livetree.c b/livetree.c
> index b61465f..894e42b 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -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;
> --
> 1.8.1.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-20 22:47 ` Grant Likely
0 siblings, 0 replies; 25+ messages in thread
From: Grant Likely @ 2014-01-20 22:47 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > 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?
I think that is reasonable.
g.
>
> Thanks,
> Mark.
>
> ---->8----
>
> From 88be0036b6a966bd7506f58e3cb9ce9ea4c5ac48 Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> 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 <mark.rutland@arm.com>
> ---
> dtc-lexer.l | 7 +++++++
> dtc-parser.y | 5 +++++
> dtc.h | 2 ++
> livetree.c | 15 ++++++++++++++-
> 4 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 0cd7e67..7989abe 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -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");
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 4864631..a8409ed 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -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);
> diff --git a/dtc.h b/dtc.h
> index 20e4d56..8687530 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -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);
>
> diff --git a/livetree.c b/livetree.c
> index b61465f..894e42b 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -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;
> --
> 1.8.1.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-20 22:47 ` Grant Likely
0 siblings, 0 replies; 25+ messages in thread
From: Grant Likely @ 2014-01-20 22:47 UTC (permalink / raw)
To: Mark Rutland, Jean-Christophe PLAGNIOL-VILLARD
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Linus Walleij, David Gibson
On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> >
> > 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?
I think that is reasonable.
g.
>
> Thanks,
> Mark.
>
> ---->8----
>
> From 88be0036b6a966bd7506f58e3cb9ce9ea4c5ac48 Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> 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 <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> ---
> dtc-lexer.l | 7 +++++++
> dtc-parser.y | 5 +++++
> dtc.h | 2 ++
> livetree.c | 15 ++++++++++++++-
> 4 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index 0cd7e67..7989abe 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -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");
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 4864631..a8409ed 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -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);
> diff --git a/dtc.h b/dtc.h
> index 20e4d56..8687530 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -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);
>
> diff --git a/livetree.c b/livetree.c
> index b61465f..894e42b 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -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;
> --
> 1.8.1.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread* [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-21 1:03 ` Olof Johansson
0 siblings, 0 replies; 25+ messages in thread
From: Olof Johansson @ 2014-01-21 1:03 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely@linaro.org> wrote:
> On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
>> >
>> > 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?
>
> 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.
-Olof
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-21 1:03 ` Olof Johansson
0 siblings, 0 replies; 25+ messages in thread
From: Olof Johansson @ 2014-01-21 1:03 UTC (permalink / raw)
To: Grant Likely
Cc: Mark Rutland, Jean-Christophe PLAGNIOL-VILLARD,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Linus Walleij, David Gibson
On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
>> >
>> > 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?
>
> 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.
-Olof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-21 8:57 ` David Gibson
0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2014-01-21 8:57 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 20, 2014 at 05:03:23PM -0800, Olof Johansson wrote:
> On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely@linaro.org> wrote:
> > On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
> >> >
> >> > 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?
> >
> > 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.
So, in principle my preferred way to handle things like this would be
to add richer expression support, including a token for "previous
value" when overlaying.
But although I've recently had another look at it, that's still a ways
off, so I wouldn't object to another approach as a stopgap, as long as
its not too hideous.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140121/a8e3ce63/attachment.sig>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-21 8:57 ` David Gibson
0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2014-01-21 8:57 UTC (permalink / raw)
To: Olof Johansson
Cc: Grant Likely, Mark Rutland, Jean-Christophe PLAGNIOL-VILLARD,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Linus Walleij
[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]
On Mon, Jan 20, 2014 at 05:03:23PM -0800, Olof Johansson wrote:
> On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> >> >
> >> > 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?
> >
> > 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.
So, in principle my preferred way to handle things like this would be
to add richer expression support, including a token for "previous
value" when overlaying.
But although I've recently had another look at it, that's still a ways
off, so I wouldn't object to another approach as a stopgap, as long as
its not too hideous.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-21 12:02 ` Mark Rutland
0 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2014-01-21 12:02 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 21, 2014 at 01:03:23AM +0000, Olof Johansson wrote:
> On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely@linaro.org> wrote:
> > On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
> >> >
> >> > 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?
> >
> > 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.
>
> 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.
>
> 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.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-21 12:02 ` Mark Rutland
0 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2014-01-21 12:02 UTC (permalink / raw)
To: Olof Johansson
Cc: grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
Jean-Christophe PLAGNIOL-VILLARD,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Linus Walleij, David Gibson
On Tue, Jan 21, 2014 at 01:03:23AM +0000, Olof Johansson wrote:
> On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> >> >
> >> > 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?
> >
> > 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.
>
> 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.
>
> 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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
2014-01-21 12:02 ` Mark Rutland
@ 2014-01-21 12:46 ` Jean-Christophe PLAGNIOL-VILLARD
-1 siblings, 0 replies; 25+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-21 12:46 UTC (permalink / raw)
To: linux-arm-kernel
On 12:02 Tue 21 Jan , Mark Rutland wrote:
> On Tue, Jan 21, 2014 at 01:03:23AM +0000, Olof Johansson wrote:
> > On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely@linaro.org> wrote:
> > > On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
> > >> >
> > >> > 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?
> > >
> > > 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.
>
> >
> > 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.
I never was a fanof index search I do agree the names irq is the best way
>
> >
> > 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.
no this need to stay simple if too much complexe => mess up, unmaintainable
if you really have complex stuff duplicate the info
Best Regards,
J.
>
> Thanks,
> Mark.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-21 12:46 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 25+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-01-21 12:46 UTC (permalink / raw)
To: Mark Rutland
Cc: devicetree@vger.kernel.org, Linus Walleij,
grant.likely@linaro.org, Olof Johansson,
linux-arm-kernel@lists.infradead.org, David Gibson
On 12:02 Tue 21 Jan , Mark Rutland wrote:
> On Tue, Jan 21, 2014 at 01:03:23AM +0000, Olof Johansson wrote:
> > On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely@linaro.org> wrote:
> > > On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
> > >> >
> > >> > 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?
> > >
> > > 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.
>
> >
> > 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.
I never was a fanof index search I do agree the names irq is the best way
>
> >
> > 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.
no this need to stay simple if too much complexe => mess up, unmaintainable
if you really have complex stuff duplicate the info
Best Regards,
J.
>
> Thanks,
> Mark.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-24 17:39 ` Grant Likely
0 siblings, 0 replies; 25+ messages in thread
From: Grant Likely @ 2014-01-24 17:39 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 20 Jan 2014 17:03:23 -0800, Olof Johansson <olof@lixom.net> wrote:
> On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely@linaro.org> wrote:
> > On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland@arm.com> wrote:
> >> >
> >> > 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?
> >
> > 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.
Hmmm, yes. to handle that case would need a higher level construct that
keeps the name and interrupt value together. Maybe something like this?
device {
add-interrupt(27);
add-interrupt(29, 5, controller=&intc1);
add-interrupt(2, 4, name="error");
};
I'm using python-like syntax here where controller and name are optional
arguments.
We could get DTC to generate the appropriate interrupt-parent,
interrupts, interrupts-extended and interrupt-names properties as
needed. Of course the shared context would be lost when the tree is
compiled, but it would be useful for building up parsing data. It would
also work for gpio, clocks and similar bindings. Kind of like an
intelligent macro.
g.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [RFC PATCH 1/1] of/irq: create interrupts-extended-2 property
@ 2014-01-24 17:39 ` Grant Likely
0 siblings, 0 replies; 25+ messages in thread
From: Grant Likely @ 2014-01-24 17:39 UTC (permalink / raw)
To: Olof Johansson
Cc: Mark Rutland, Jean-Christophe PLAGNIOL-VILLARD,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Linus Walleij, David Gibson
On Mon, 20 Jan 2014 17:03:23 -0800, Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org> wrote:
> On Mon, Jan 20, 2014 at 2:47 PM, Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Wed, 15 Jan 2014 16:12:24 +0000, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> >> >
> >> > 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?
> >
> > 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.
Hmmm, yes. to handle that case would need a higher level construct that
keeps the name and interrupt value together. Maybe something like this?
device {
add-interrupt(27);
add-interrupt(29, 5, controller=&intc1);
add-interrupt(2, 4, name="error");
};
I'm using python-like syntax here where controller and name are optional
arguments.
We could get DTC to generate the appropriate interrupt-parent,
interrupts, interrupts-extended and interrupt-names properties as
needed. Of course the shared context would be lost when the tree is
compiled, but it would be useful for building up parsing data. It would
also work for gpio, clocks and similar bindings. Kind of like an
intelligent macro.
g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 25+ messages in thread