[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Guennadi Liakhovetski]

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

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Guennadi Liakhovetski]

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

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Guennadi Liakhovetski]

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

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Laurent Pinchart]

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

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Laurent Pinchart]

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

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Laurent Pinchart]

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

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Guennadi Liakhovetski]

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@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 ?

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/

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Guennadi Liakhovetski]

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@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 ?

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/

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Guennadi Liakhovetski]

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/

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Laurent Pinchart]

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.

> > > 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

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Laurent Pinchart]

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.

> > > 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

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Laurent Pinchart]

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

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Guennadi Liakhovetski]

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/

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Guennadi Liakhovetski]

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/

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Guennadi Liakhovetski]

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/

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Laurent Pinchart]

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

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Laurent Pinchart]

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

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Laurent Pinchart]

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

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Simon Horman]

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.

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Simon Horman]

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.

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Simon Horman]

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.

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Magnus Damm]

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

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Magnus Damm]

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

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Magnus Damm]

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

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Simon Horman]

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
> 

[PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Simon Horman]

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
> 

Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

[Simon Horman]

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
> 

[PATCH v3] irqchip: renesas-intc-irqpin: Improve binding documentation

[Geert Uytterhoeven]

Add missing documentation for required properties:
  - interrupt-controller,
  - parent interrupts (one entry per provided interrupt).

Add missing documentation for optional properties:
  - functional clock (managed since commit 705bc96c2c15313c ("irqchip:
    renesas-intc-irqpin: Add minimal runtime PM support")),
  - power-domains.

Add an example, taken from r8a7740.dtsi.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
---
v3:
  - Add Acked-by,

v2:
  - This is v2 of "Add optional functional clock to bindings",
  - Document all missing properties, not just the functional clock,
  - Use a generic name for the device node in the example.
---
 .../interrupt-controller/renesas,intc-irqpin.txt   | 39 ++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
index 4f7946ae8adcdc04..772c550d3b4bcfe2 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
@@ -13,9 +13,12 @@ Required properties:
 - reg: Base address and length of each register bank used by the external
   IRQ pins driven by the interrupt controller hardware module. The base
   addresses, length and number of required register banks varies with soctype.
-
+- interrupt-controller: Identifies the node as an interrupt controller.
 - #interrupt-cells: has to be <2>: an interrupt index and flags, as defined in
-  interrupts.txt in this directory
+  interrupts.txt in this directory.
+- interrupts: Must contain a list of interrupt specifiers. For each interrupt
+  provided by this irqpin controller instance, there must be one entry,
+  referring to the corresponding parent interrupt.
 
 Optional properties:
 
@@ -25,3 +28,35 @@ Optional properties:
   if different from the default 4 bits
 - control-parent: disable and enable interrupts on the parent interrupt
   controller, needed for some broken implementations
+- clocks: Must contain a reference to the functional clock.  This property is
+  mandatory if the hardware implements a controllable functional clock for
+  the irqpin controller instance.
+- power-domains: Must contain a reference to the power domain. This property is
+  mandatory if the irqpin controller instance is part of a controllable power
+  domain.
+
+
+Example
+-------
+
+	irqpin1: interrupt-controller@e6900004 {
+		compatible = "renesas,intc-irqpin-r8a7740",
+			     "renesas,intc-irqpin";
+		#interrupt-cells = <2>;
+		interrupt-controller;
+		reg = <0xe6900004 4>,
+			<0xe6900014 4>,
+			<0xe6900024 1>,
+			<0xe6900044 1>,
+			<0xe6900064 1>;
+		interrupts = <0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp2_clks R8A7740_CLK_INTCA>;
+		power-domains = <&pd_a4s>;
+	};
-- 
1.9.1

[PATCH v3] irqchip: renesas-intc-irqpin: Improve binding documentation

[Geert Uytterhoeven]

Add missing documentation for required properties:
  - interrupt-controller,
  - parent interrupts (one entry per provided interrupt).

Add missing documentation for optional properties:
  - functional clock (managed since commit 705bc96c2c15313c ("irqchip:
    renesas-intc-irqpin: Add minimal runtime PM support")),
  - power-domains.

Add an example, taken from r8a7740.dtsi.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
---
v3:
  - Add Acked-by,

v2:
  - This is v2 of "Add optional functional clock to bindings",
  - Document all missing properties, not just the functional clock,
  - Use a generic name for the device node in the example.
---
 .../interrupt-controller/renesas,intc-irqpin.txt   | 39 ++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
index 4f7946ae8adcdc04..772c550d3b4bcfe2 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
@@ -13,9 +13,12 @@ Required properties:
 - reg: Base address and length of each register bank used by the external
   IRQ pins driven by the interrupt controller hardware module. The base
   addresses, length and number of required register banks varies with soctype.
-
+- interrupt-controller: Identifies the node as an interrupt controller.
 - #interrupt-cells: has to be <2>: an interrupt index and flags, as defined in
-  interrupts.txt in this directory
+  interrupts.txt in this directory.
+- interrupts: Must contain a list of interrupt specifiers. For each interrupt
+  provided by this irqpin controller instance, there must be one entry,
+  referring to the corresponding parent interrupt.
 
 Optional properties:
 
@@ -25,3 +28,35 @@ Optional properties:
   if different from the default 4 bits
 - control-parent: disable and enable interrupts on the parent interrupt
   controller, needed for some broken implementations
+- clocks: Must contain a reference to the functional clock.  This property is
+  mandatory if the hardware implements a controllable functional clock for
+  the irqpin controller instance.
+- power-domains: Must contain a reference to the power domain. This property is
+  mandatory if the irqpin controller instance is part of a controllable power
+  domain.
+
+
+Example
+-------
+
+	irqpin1: interrupt-controller@e6900004 {
+		compatible = "renesas,intc-irqpin-r8a7740",
+			     "renesas,intc-irqpin";
+		#interrupt-cells = <2>;
+		interrupt-controller;
+		reg = <0xe6900004 4>,
+			<0xe6900014 4>,
+			<0xe6900024 1>,
+			<0xe6900044 1>,
+			<0xe6900064 1>;
+		interrupts = <0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp2_clks R8A7740_CLK_INTCA>;
+		power-domains = <&pd_a4s>;
+	};
-- 
1.9.1

[tip:irq/core] irqchip: renesas: intc-irqpin: Improve binding documentation

[tip-bot for Geert Uytterhoeven]

Commit-ID:  496c28b13eb9d4f9c751b672daad8b110084cfd8
Gitweb:     http://git.kernel.org/tip/496c28b13eb9d4f9c751b672daad8b110084cfd8
Author:     Geert Uytterhoeven <geert+renesas@glider.be>
AuthorDate: Fri, 29 May 2015 11:27:43 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 5 Jun 2015 22:26:33 +0200

irqchip: renesas: intc-irqpin: Improve binding documentation

Add missing documentation for required properties:
  - interrupt-controller,
  - parent interrupts (one entry per provided interrupt).

Add missing documentation for optional properties:
  - functional clock (managed since commit 705bc96c2c15313c ("irqchip:
    renesas-intc-irqpin: Add minimal runtime PM support")),
  - power-domains.

Add an example, taken from r8a7740.dtsi.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
Cc: Jason Cooper <jason@lakedaemon.net>
Link: http://lkml.kernel.org/r/1432891663-23641-1-git-send-email-geert%2Brenesas@glider.be
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 .../interrupt-controller/renesas,intc-irqpin.txt   | 39 ++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
index 4f7946a..772c550 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
@@ -13,9 +13,12 @@ Required properties:
 - reg: Base address and length of each register bank used by the external
   IRQ pins driven by the interrupt controller hardware module. The base
   addresses, length and number of required register banks varies with soctype.
-
+- interrupt-controller: Identifies the node as an interrupt controller.
 - #interrupt-cells: has to be <2>: an interrupt index and flags, as defined in
-  interrupts.txt in this directory
+  interrupts.txt in this directory.
+- interrupts: Must contain a list of interrupt specifiers. For each interrupt
+  provided by this irqpin controller instance, there must be one entry,
+  referring to the corresponding parent interrupt.
 
 Optional properties:
 
@@ -25,3 +28,35 @@ Optional properties:
   if different from the default 4 bits
 - control-parent: disable and enable interrupts on the parent interrupt
   controller, needed for some broken implementations
+- clocks: Must contain a reference to the functional clock.  This property is
+  mandatory if the hardware implements a controllable functional clock for
+  the irqpin controller instance.
+- power-domains: Must contain a reference to the power domain. This property is
+  mandatory if the irqpin controller instance is part of a controllable power
+  domain.
+
+
+Example
+-------
+
+	irqpin1: interrupt-controller@e6900004 {
+		compatible = "renesas,intc-irqpin-r8a7740",
+			     "renesas,intc-irqpin";
+		#interrupt-cells = <2>;
+		interrupt-controller;
+		reg = <0xe6900004 4>,
+			<0xe6900014 4>,
+			<0xe6900024 1>,
+			<0xe6900044 1>,
+			<0xe6900064 1>;
+		interrupts = <0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH
+			      0 149 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp2_clks R8A7740_CLK_INTCA>;
+		power-domains = <&pd_a4s>;
+	};