* [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width @ 2013-04-08 8:08 Guennadi Liakhovetski 2013-04-08 9:56 ` Laurent Pinchart 2013-04-09 22:23 ` Magnus Damm 0 siblings, 2 replies; 9+ messages in thread From: Guennadi Liakhovetski @ 2013-04-08 8:08 UTC (permalink / raw) To: Magnus Damm Cc: Simon Horman, SH-Linux, devicetree-discuss, Thomas Gleixner, linux-arm-kernel Most Renesas irqpin controllers have 4-bit sense fields, however, some have different widths. This patch adds a DT binding to optionally specify such non-standard values. Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> --- v3: move the code to a common location, where device configuration parameters are retrieved .../interrupt-controller/renesas,intc-irqpin.txt | 13 +++++++++++++ drivers/irqchip/irq-renesas-intc-irqpin.c | 4 ++++ 2 files changed, 17 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt new file mode 100644 index 0000000..c6f09b7 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt @@ -0,0 +1,13 @@ +DT bindings for the R-/SH-Mobile irqpin controller + +Required properties: + +- compatible: has to be "renesas,intc-irqpin" +- #interrupt-cells: has to be <2> + +Optional properties: + +- any properties, listed in interrupts.txt in this directory, and any standard + resource allocation properties +- sense-bitfield-width: width of a single sense bitfield in the SENSE register, + if different from the default 4 bits diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c index 5a68e5a..4aca1b2 100644 --- a/drivers/irqchip/irq-renesas-intc-irqpin.c +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c @@ -18,6 +18,7 @@ */ #include <linux/init.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/spinlock.h> #include <linux/interrupt.h> @@ -349,6 +350,9 @@ static int intc_irqpin_probe(struct platform_device *pdev) /* deal with driver instance configuration */ if (pdata) memcpy(&p->config, pdata, sizeof(*pdata)); + else + of_property_read_u32(pdev->dev.of_node, "sense-bitfield-width", + &p->config.sense_bitfield_width); if (!p->config.sense_bitfield_width) p->config.sense_bitfield_width = 4; /* default to 4 bits */ -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width 2013-04-08 8:08 [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width Guennadi Liakhovetski @ 2013-04-08 9:56 ` Laurent Pinchart 2013-04-08 11:25 ` Guennadi Liakhovetski 2013-04-09 22:23 ` Magnus Damm 1 sibling, 1 reply; 9+ messages in thread From: Laurent Pinchart @ 2013-04-08 9:56 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Magnus Damm, Simon Horman, SH-Linux, devicetree-discuss, Thomas Gleixner, linux-arm-kernel Hi Guennadi, Thanks for the patch. On Monday 08 April 2013 10:08:40 Guennadi Liakhovetski wrote: > Most Renesas irqpin controllers have 4-bit sense fields, however, some > have different widths. This patch adds a DT binding to optionally > specify such non-standard values. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > --- > > v3: move the code to a common location, where device configuration > parameters are retrieved > > .../interrupt-controller/renesas,intc-irqpin.txt | 13 +++++++++++++ > drivers/irqchip/irq-renesas-intc-irqpin.c | 4 ++++ > 2 files changed, 17 insertions(+), 0 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin. > txt > > diff --git > a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpi > n.txt > b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpi > n.txt new file mode 100644 > index 0000000..c6f09b7 > --- /dev/null > +++ > b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpi > n.txt @@ -0,0 +1,13 @@ > +DT bindings for the R-/SH-Mobile irqpin controller > + > +Required properties: > + > +- compatible: has to be "renesas,intc-irqpin" > +- #interrupt-cells: has to be <2> > + > +Optional properties: > + > +- any properties, listed in interrupts.txt in this directory, and any > standard > + resource allocation properties > +- sense-bitfield-width: width of a single sense bitfield in the SENSE > register, > + if different from the default 4 bits Wouldn't it be better to define per-SoC compatible strings, and infer the sense bitfield width from that ? > diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c > b/drivers/irqchip/irq-renesas-intc-irqpin.c index 5a68e5a..4aca1b2 100644 > --- a/drivers/irqchip/irq-renesas-intc-irqpin.c > +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c > @@ -18,6 +18,7 @@ > */ > > #include <linux/init.h> > +#include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/spinlock.h> > #include <linux/interrupt.h> > @@ -349,6 +350,9 @@ static int intc_irqpin_probe(struct platform_device > *pdev) /* deal with driver instance configuration */ > if (pdata) > memcpy(&p->config, pdata, sizeof(*pdata)); > + else > + of_property_read_u32(pdev->dev.of_node, "sense-bitfield-width", > + &p->config.sense_bitfield_width); > if (!p->config.sense_bitfield_width) > p->config.sense_bitfield_width = 4; /* default to 4 bits */ -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width 2013-04-08 9:56 ` Laurent Pinchart @ 2013-04-08 11:25 ` Guennadi Liakhovetski [not found] ` <Pine.LNX.4.64.1304081324370.29945-0199iw4Nj15frtckUFj5Ag@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Guennadi Liakhovetski @ 2013-04-08 11:25 UTC (permalink / raw) To: Laurent Pinchart Cc: SH-Linux, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Magnus Damm, Simon Horman, Thomas Gleixner, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Mon, 8 Apr 2013, Laurent Pinchart wrote: > Hi Guennadi, > > Thanks for the patch. > > On Monday 08 April 2013 10:08:40 Guennadi Liakhovetski wrote: > > Most Renesas irqpin controllers have 4-bit sense fields, however, some > > have different widths. This patch adds a DT binding to optionally > > specify such non-standard values. > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > --- > > > > v3: move the code to a common location, where device configuration > > parameters are retrieved > > > > .../interrupt-controller/renesas,intc-irqpin.txt | 13 +++++++++++++ > > drivers/irqchip/irq-renesas-intc-irqpin.c | 4 ++++ > > 2 files changed, 17 insertions(+), 0 deletions(-) > > create mode 100644 > > Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin. > > txt > > > > diff --git > > a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpi > > n.txt > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpi > > n.txt new file mode 100644 > > index 0000000..c6f09b7 > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpi > > n.txt @@ -0,0 +1,13 @@ > > +DT bindings for the R-/SH-Mobile irqpin controller > > + > > +Required properties: > > + > > +- compatible: has to be "renesas,intc-irqpin" > > +- #interrupt-cells: has to be <2> > > + > > +Optional properties: > > + > > +- any properties, listed in interrupts.txt in this directory, and any > > standard > > + resource allocation properties > > +- sense-bitfield-width: width of a single sense bitfield in the SENSE > > register, > > + if different from the default 4 bits > > Wouldn't it be better to define per-SoC compatible strings, and infer the > sense bitfield width from that ? This is not a boolean, it is an integer, I don't think defining compatibility strings for 1, 2, 3, 4, 5,... bits is better than having one integer property. Thanks Guennadi > > > diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c > > b/drivers/irqchip/irq-renesas-intc-irqpin.c index 5a68e5a..4aca1b2 100644 > > --- a/drivers/irqchip/irq-renesas-intc-irqpin.c > > +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c > > @@ -18,6 +18,7 @@ > > */ > > > > #include <linux/init.h> > > +#include <linux/of.h> > > #include <linux/platform_device.h> > > #include <linux/spinlock.h> > > #include <linux/interrupt.h> > > @@ -349,6 +350,9 @@ static int intc_irqpin_probe(struct platform_device > > *pdev) /* deal with driver instance configuration */ > > if (pdata) > > memcpy(&p->config, pdata, sizeof(*pdata)); > > + else > > + of_property_read_u32(pdev->dev.of_node, "sense-bitfield-width", > > + &p->config.sense_bitfield_width); > > if (!p->config.sense_bitfield_width) > > p->config.sense_bitfield_width = 4; /* default to 4 bits */ > -- > Regards, > > Laurent Pinchart > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <Pine.LNX.4.64.1304081324370.29945-0199iw4Nj15frtckUFj5Ag@public.gmane.org>]
* Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width [not found] ` <Pine.LNX.4.64.1304081324370.29945-0199iw4Nj15frtckUFj5Ag@public.gmane.org> @ 2013-04-08 11:37 ` Laurent Pinchart 2013-04-08 12:15 ` Guennadi Liakhovetski 0 siblings, 1 reply; 9+ messages in thread From: Laurent Pinchart @ 2013-04-08 11:37 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: SH-Linux, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Magnus Damm, Simon Horman, Thomas Gleixner, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Hi Guennadi, On Monday 08 April 2013 13:25:46 Guennadi Liakhovetski wrote: > On Mon, 8 Apr 2013, Laurent Pinchart wrote: > > On Monday 08 April 2013 10:08:40 Guennadi Liakhovetski wrote: > > > Most Renesas irqpin controllers have 4-bit sense fields, however, some > > > have different widths. This patch adds a DT binding to optionally > > > specify such non-standard values. > > > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > > --- > > > > > > v3: move the code to a common location, where device configuration > > > parameters are retrieved > > > > > > .../interrupt-controller/renesas,intc-irqpin.txt | 13 +++++++++++++ > > > drivers/irqchip/irq-renesas-intc-irqpin.c | 4 ++++ > > > 2 files changed, 17 insertions(+), 0 deletions(-) > > > create mode 100644 > > > > > > Documentation/devicetree/bindings/interrupt-controller/renesas,intc- > > > irqpin.txt > > > diff --git a/Documentation/devicetree/bindings/interrupt- > > > controller/renesas,intc-irqpin.txt > > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc- > > > irqpin.txt > > > new file mode 100644 > > > index 0000000..c6f09b7 > > > --- /dev/null > > > +++ > > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc- > > > irqpin.txt > > > @@ -0,0 +1,13 @@ > > > +DT bindings for the R-/SH-Mobile irqpin controller > > > + > > > +Required properties: > > > + > > > +- compatible: has to be "renesas,intc-irqpin" > > > +- #interrupt-cells: has to be <2> > > > + > > > +Optional properties: > > > + > > > +- any properties, listed in interrupts.txt in this directory, and any > > > standard > > > + resource allocation properties > > > +- sense-bitfield-width: width of a single sense bitfield in the SENSE > > > register, > > > + if different from the default 4 bits > > > > Wouldn't it be better to define per-SoC compatible strings, and infer the > > sense bitfield width from that ? > > This is not a boolean, it is an integer, I don't think defining > compatibility strings for 1, 2, 3, 4, 5,... bits is better than having one > integer property. I'm not advocating for compatibility strings for a given number of bits, but for per-SoC compatibility strings from which to infer the width. For instance, if the r8a7779 has a 6-bit sense field, you would use compatible = "r8a7779,intc-irqpin"; and map that to 6 in the irqpin driver. > > > diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c > > > b/drivers/irqchip/irq-renesas-intc-irqpin.c index 5a68e5a..4aca1b2 > > > 100644 > > > --- a/drivers/irqchip/irq-renesas-intc-irqpin.c > > > +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c > > > @@ -18,6 +18,7 @@ > > > */ > > > > > > #include <linux/init.h> > > > +#include <linux/of.h> > > > #include <linux/platform_device.h> > > > #include <linux/spinlock.h> > > > #include <linux/interrupt.h> > > > > > > @@ -349,6 +350,9 @@ static int intc_irqpin_probe(struct platform_device > > > *pdev) /* deal with driver instance configuration */ > > > if (pdata) > > > memcpy(&p->config, pdata, sizeof(*pdata)); > > > + else > > > + of_property_read_u32(pdev->dev.of_node, "sense-bitfield-width", > > > + &p->config.sense_bitfield_width); > > > > > > if (!p->config.sense_bitfield_width) > > > p->config.sense_bitfield_width = 4; /* default to 4 bits */ -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width 2013-04-08 11:37 ` Laurent Pinchart @ 2013-04-08 12:15 ` Guennadi Liakhovetski 2013-04-08 23:22 ` Laurent Pinchart 0 siblings, 1 reply; 9+ messages in thread From: Guennadi Liakhovetski @ 2013-04-08 12:15 UTC (permalink / raw) To: Laurent Pinchart Cc: Magnus Damm, Simon Horman, SH-Linux, devicetree-discuss, Thomas Gleixner, linux-arm-kernel On Mon, 8 Apr 2013, Laurent Pinchart wrote: > Hi Guennadi, > > On Monday 08 April 2013 13:25:46 Guennadi Liakhovetski wrote: > > On Mon, 8 Apr 2013, Laurent Pinchart wrote: > > > On Monday 08 April 2013 10:08:40 Guennadi Liakhovetski wrote: > > > > Most Renesas irqpin controllers have 4-bit sense fields, however, some > > > > have different widths. This patch adds a DT binding to optionally > > > > specify such non-standard values. > > > > > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > > > > --- > > > > > > > > v3: move the code to a common location, where device configuration > > > > parameters are retrieved > > > > > > > > .../interrupt-controller/renesas,intc-irqpin.txt | 13 +++++++++++++ > > > > drivers/irqchip/irq-renesas-intc-irqpin.c | 4 ++++ > > > > 2 files changed, 17 insertions(+), 0 deletions(-) > > > > create mode 100644 > > > > > > > > Documentation/devicetree/bindings/interrupt-controller/renesas,intc- > > > > irqpin.txt > > > > diff --git a/Documentation/devicetree/bindings/interrupt- > > > > controller/renesas,intc-irqpin.txt > > > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc- > > > > irqpin.txt > > > > new file mode 100644 > > > > index 0000000..c6f09b7 > > > > --- /dev/null > > > > +++ > > > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc- > > > > irqpin.txt > > > > @@ -0,0 +1,13 @@ > > > > +DT bindings for the R-/SH-Mobile irqpin controller > > > > + > > > > +Required properties: > > > > + > > > > +- compatible: has to be "renesas,intc-irqpin" > > > > +- #interrupt-cells: has to be <2> > > > > + > > > > +Optional properties: > > > > + > > > > +- any properties, listed in interrupts.txt in this directory, and any > > > > standard > > > > + resource allocation properties > > > > +- sense-bitfield-width: width of a single sense bitfield in the SENSE > > > > register, > > > > + if different from the default 4 bits > > > > > > Wouldn't it be better to define per-SoC compatible strings, and infer the > > > sense bitfield width from that ? > > > > This is not a boolean, it is an integer, I don't think defining > > compatibility strings for 1, 2, 3, 4, 5,... bits is better than having one > > integer property. > > I'm not advocating for compatibility strings for a given number of bits, but > for per-SoC compatibility strings from which to infer the width. > > For instance, if the r8a7779 has a 6-bit sense field, you would use > > compatible = "r8a7779,intc-irqpin"; > > and map that to 6 in the irqpin driver. I understand what you mean, still I don't think mapping N SoC-compatibility strings to various values of a single integer is better than having a single property. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width 2013-04-08 12:15 ` Guennadi Liakhovetski @ 2013-04-08 23:22 ` Laurent Pinchart 2013-04-09 8:44 ` Simon Horman 0 siblings, 1 reply; 9+ messages in thread From: Laurent Pinchart @ 2013-04-08 23:22 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Magnus Damm, Simon Horman, SH-Linux, devicetree-discuss, Thomas Gleixner, linux-arm-kernel Hi Guennadi, On Monday 08 April 2013 14:15:39 Guennadi Liakhovetski wrote: > On Mon, 8 Apr 2013, Laurent Pinchart wrote: > > On Monday 08 April 2013 13:25:46 Guennadi Liakhovetski wrote: > > > On Mon, 8 Apr 2013, Laurent Pinchart wrote: > > > > On Monday 08 April 2013 10:08:40 Guennadi Liakhovetski wrote: > > > > > Most Renesas irqpin controllers have 4-bit sense fields, however, > > > > > some have different widths. This patch adds a DT binding to > > > > > optionally specify such non-standard values. > > > > > > > > > > Signed-off-by: Guennadi Liakhovetski > > > > > <g.liakhovetski+renesas@gmail.com> > > > > > --- > > > > > > > > > > v3: move the code to a common location, where device configuration > > > > > parameters are retrieved > > > > > > > > > > .../interrupt-controller/renesas,intc-irqpin.txt | 13 +++++++++ > > > > > drivers/irqchip/irq-renesas-intc-irqpin.c | 4 ++++ > > > > > 2 files changed, 17 insertions(+), 0 deletions(-) > > > > > create mode 100644 > > > > > > > > > > Documentation/devicetree/bindings/interrupt-controller/renesas,intc- > > > > > irqpin.txt > > > > > diff --git a/Documentation/devicetree/bindings/interrupt- > > > > > controller/renesas,intc-irqpin.txt > > > > > b/Documentation/devicetree/bindings/interrupt-controller/ > > > > > renesas,intc-irqpin.txt > > > > > new file mode 100644 > > > > > index 0000000..c6f09b7 > > > > > --- /dev/null > > > > > +++ > > > > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,int > > > > > c- > > > > > irqpin.txt > > > > > @@ -0,0 +1,13 @@ > > > > > +DT bindings for the R-/SH-Mobile irqpin controller > > > > > + > > > > > +Required properties: > > > > > + > > > > > +- compatible: has to be "renesas,intc-irqpin" > > > > > +- #interrupt-cells: has to be <2> > > > > > + > > > > > +Optional properties: > > > > > + > > > > > +- any properties, listed in interrupts.txt in this directory, and > > > > > any standard > > > > > + resource allocation properties > > > > > +- sense-bitfield-width: width of a single sense bitfield in the > > > > > SENSE register, > > > > > + if different from the default 4 bits > > > > > > > > Wouldn't it be better to define per-SoC compatible strings, and infer > > > > the sense bitfield width from that ? > > > > > > This is not a boolean, it is an integer, I don't think defining > > > compatibility strings for 1, 2, 3, 4, 5,... bits is better than having > > > one integer property. > > > > I'm not advocating for compatibility strings for a given number of bits, > > but for per-SoC compatibility strings from which to infer the width. > > > > For instance, if the r8a7779 has a 6-bit sense field, you would use > > > > compatible = "r8a7779,intc-irqpin"; > > > > and map that to 6 in the irqpin driver. > > I understand what you mean, still I don't think mapping N SoC-compatibility > strings to various values of a single integer is better than having a single > property. The way I understand (or maybe rather feel) the DT bindings philosophy is that hardware information for on-SoC IP cores that depend on the IP core version should be conveyed through the compatible property. I might be wrong though. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width 2013-04-08 23:22 ` Laurent Pinchart @ 2013-04-09 8:44 ` Simon Horman 0 siblings, 0 replies; 9+ messages in thread From: Simon Horman @ 2013-04-09 8:44 UTC (permalink / raw) To: Laurent Pinchart Cc: Guennadi Liakhovetski, Magnus Damm, SH-Linux, devicetree-discuss, Thomas Gleixner, linux-arm-kernel On Tue, Apr 09, 2013 at 01:22:29AM +0200, Laurent Pinchart wrote: > Hi Guennadi, > > On Monday 08 April 2013 14:15:39 Guennadi Liakhovetski wrote: > > On Mon, 8 Apr 2013, Laurent Pinchart wrote: > > > On Monday 08 April 2013 13:25:46 Guennadi Liakhovetski wrote: > > > > On Mon, 8 Apr 2013, Laurent Pinchart wrote: > > > > > On Monday 08 April 2013 10:08:40 Guennadi Liakhovetski wrote: > > > > > > Most Renesas irqpin controllers have 4-bit sense fields, however, > > > > > > some have different widths. This patch adds a DT binding to > > > > > > optionally specify such non-standard values. > > > > > > > > > > > > Signed-off-by: Guennadi Liakhovetski > > > > > > <g.liakhovetski+renesas@gmail.com> > > > > > > --- > > > > > > > > > > > > v3: move the code to a common location, where device configuration > > > > > > parameters are retrieved > > > > > > > > > > > > .../interrupt-controller/renesas,intc-irqpin.txt | 13 +++++++++ > > > > > > drivers/irqchip/irq-renesas-intc-irqpin.c | 4 ++++ > > > > > > 2 files changed, 17 insertions(+), 0 deletions(-) > > > > > > create mode 100644 > > > > > > > > > > > > Documentation/devicetree/bindings/interrupt-controller/renesas,intc- > > > > > > irqpin.txt > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt- > > > > > > controller/renesas,intc-irqpin.txt > > > > > > b/Documentation/devicetree/bindings/interrupt-controller/ > > > > > > renesas,intc-irqpin.txt > > > > > > new file mode 100644 > > > > > > index 0000000..c6f09b7 > > > > > > --- /dev/null > > > > > > +++ > > > > > > b/Documentation/devicetree/bindings/interrupt-controller/renesas,int > > > > > > c- > > > > > > irqpin.txt > > > > > > @@ -0,0 +1,13 @@ > > > > > > +DT bindings for the R-/SH-Mobile irqpin controller > > > > > > + > > > > > > +Required properties: > > > > > > + > > > > > > +- compatible: has to be "renesas,intc-irqpin" > > > > > > +- #interrupt-cells: has to be <2> > > > > > > + > > > > > > +Optional properties: > > > > > > + > > > > > > +- any properties, listed in interrupts.txt in this directory, and > > > > > > any standard > > > > > > + resource allocation properties > > > > > > +- sense-bitfield-width: width of a single sense bitfield in the > > > > > > SENSE register, > > > > > > + if different from the default 4 bits > > > > > > > > > > Wouldn't it be better to define per-SoC compatible strings, and infer > > > > > the sense bitfield width from that ? > > > > > > > > This is not a boolean, it is an integer, I don't think defining > > > > compatibility strings for 1, 2, 3, 4, 5,... bits is better than having > > > > one integer property. > > > > > > I'm not advocating for compatibility strings for a given number of bits, > > > but for per-SoC compatibility strings from which to infer the width. > > > > > > For instance, if the r8a7779 has a 6-bit sense field, you would use > > > > > > compatible = "r8a7779,intc-irqpin"; > > > > > > and map that to 6 in the irqpin driver. > > > > I understand what you mean, still I don't think mapping N SoC-compatibility > > strings to various values of a single integer is better than having a single > > property. > > The way I understand (or maybe rather feel) the DT bindings philosophy is that > hardware information for on-SoC IP cores that depend on the IP core version > should be conveyed through the compatible property. I might be wrong though. That is also my understanding. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width 2013-04-08 8:08 [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width Guennadi Liakhovetski 2013-04-08 9:56 ` Laurent Pinchart @ 2013-04-09 22:23 ` Magnus Damm 2013-04-10 0:48 ` Simon Horman 1 sibling, 1 reply; 9+ messages in thread From: Magnus Damm @ 2013-04-09 22:23 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Simon Horman, SH-Linux, devicetree-discuss@lists.ozlabs.org, Thomas Gleixner, linux-arm-kernel@lists.infradead.org Hi Guennadi, On Mon, Apr 8, 2013 at 5:08 PM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > Most Renesas irqpin controllers have 4-bit sense fields, however, some > have different widths. This patch adds a DT binding to optionally > specify such non-standard values. > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > --- > > v3: move the code to a common location, where device configuration > parameters are retrieved Thanks for rearranging the code, this looks good to me. Acked-by: Magnus Damm <damm@opensource.se> To be clear, I prefer your approach over a per-SoC compatible string. In general I think a per-SoC compatible string is nice in theory, but I don't think it is correct to use it to describe a change in a IP block that just happens to included in the SoC. Instead the version of the IP block shall be used with the compatible value. In some cases it may not be easy to retrieve such a version. The per-SoC compatible string may look good but they come with at least two drawbacks. Either 1) the driver has to be updated for each new SoC even though the device IP the driver is handling hasn't changed which leads to 1.1) more need for pointless per-SoC compatible string patches to be merged and tracked and back ported and 1.2) less chance of running a standard distro lacking per-SoC compatible string but has actual code for support or 2) to ship soon the per-SoC DT will use SoC compatible strings matching other SoC names which works but is even more confusing. For the INTC irqpin case I believe this approach with a single property is the best. Thanks, / magnus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width 2013-04-09 22:23 ` Magnus Damm @ 2013-04-10 0:48 ` Simon Horman 0 siblings, 0 replies; 9+ messages in thread From: Simon Horman @ 2013-04-10 0:48 UTC (permalink / raw) To: Magnus Damm Cc: Guennadi Liakhovetski, SH-Linux, devicetree-discuss@lists.ozlabs.org, Thomas Gleixner, linux-arm-kernel@lists.infradead.org On Wed, Apr 10, 2013 at 07:23:06AM +0900, Magnus Damm wrote: > Hi Guennadi, > > On Mon, Apr 8, 2013 at 5:08 PM, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> wrote: > > Most Renesas irqpin controllers have 4-bit sense fields, however, some > > have different widths. This patch adds a DT binding to optionally > > specify such non-standard values. > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com> > > --- > > > > v3: move the code to a common location, where device configuration > > parameters are retrieved > > Thanks for rearranging the code, this looks good to me. > > Acked-by: Magnus Damm <damm@opensource.se> Thanks, queued-up in the renesas-intc-irqpin branch. > To be clear, I prefer your approach over a per-SoC compatible string. > > In general I think a per-SoC compatible string is nice in theory, but > I don't think it is correct to use it to describe a change in a IP > block that just happens to included in the SoC. Instead the version of > the IP block shall be used with the compatible value. In some cases it > may not be easy to retrieve such a version. > > The per-SoC compatible string may look good but they come with at > least two drawbacks. Either > 1) the driver has to be updated for each new SoC even though the > device IP the driver is handling hasn't changed which leads to > 1.1) more need for pointless per-SoC compatible string patches to be > merged and tracked and back ported > and > 1.2) less chance of running a standard distro lacking per-SoC > compatible string but has actual code for support > or > 2) to ship soon the per-SoC DT will use SoC compatible strings > matching other SoC names which works but is even more confusing. > > For the INTC irqpin case I believe this approach with a single > property is the best. > > Thanks, > > / magnus > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-04-10 0:48 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-08 8:08 [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width Guennadi Liakhovetski 2013-04-08 9:56 ` Laurent Pinchart 2013-04-08 11:25 ` Guennadi Liakhovetski [not found] ` <Pine.LNX.4.64.1304081324370.29945-0199iw4Nj15frtckUFj5Ag@public.gmane.org> 2013-04-08 11:37 ` Laurent Pinchart 2013-04-08 12:15 ` Guennadi Liakhovetski 2013-04-08 23:22 ` Laurent Pinchart 2013-04-09 8:44 ` Simon Horman 2013-04-09 22:23 ` Magnus Damm 2013-04-10 0:48 ` Simon Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).