[PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Magnus Damm]

From: Magnus Damm <damm+renesas@opensource.se>

Add r8a7779 specific support for IRLM bit configuration
in the INTC-IRQPIN driver. Without this code we need
special workaround code in arch/arm/mach-shmobile.

The IRLM bit for the INTC hardware exists on various
older SH-based SoCs and is used to select between two
modes for the external interrupt pins IRQ0 to IRQ3:

IRLM = 0: (default from reset on r8a7779)
In this mode the pins IRQ0 to IRQ3 are used together
to give a value between 0 and 15 to the SoC. External
logic is required for masking. This mode is not
supported by the INTC-IRQPIN driver.

IRLM = 1: (needs this patch or configuration elsewhere)
In this mode IRQ0 to IRQ3 operate as 4 individual
external interrupt pins. In this mode the SMSC ethernet
chip can be used via IRQ1 on r8a7779 Marzen. This mode
is the only supported mode by the INTC-IRQPIN driver.

For this patch to work the r8a7779 DTS needs to pass
the ICR0 register as the last register bank.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Same version as sent on December 3rd.

 Written against renesas-devel-20141202-v3.18-rc7 which is
 basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
 
 Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt |    5 +
 drivers/irqchip/irq-renesas-intc-irqpin.c                                      |   50 ++++++++--
 2 files changed, 46 insertions(+), 9 deletions(-)

--- 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
+++ work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt	2014-12-03 20:25:13.000000000 +0900
@@ -9,6 +9,11 @@ Required properties:
     - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
     - "renesas,intc-irqpin-r8a7779" (R-Car H1)
     - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
+
+- 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-cells: has to be <2>: an interrupt index and flags, as defined in
   interrupts.txt in this directory
 
--- 0001/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ work/drivers/irqchip/irq-renesas-intc-irqpin.c	2014-12-03 20:32:59.000000000 +0900
@@ -30,6 +30,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_data/irq-renesas-intc-irqpin.h>
 #include <linux/pm_runtime.h>
 
@@ -40,7 +41,9 @@
 #define INTC_IRQPIN_REG_SOURCE 2 /* INTREQnn */
 #define INTC_IRQPIN_REG_MASK 3 /* INTMSKnn */
 #define INTC_IRQPIN_REG_CLEAR 4 /* INTMSKCLRnn */
-#define INTC_IRQPIN_REG_NR 5
+#define INTC_IRQPIN_REG_NR_MANDATORY 5
+#define INTC_IRQPIN_REG_IRLM 5 /* ICR0 with IRLM bit (optional) */
+#define INTC_IRQPIN_REG_NR 6
 
 /* INTC external IRQ PIN hardware register access:
  *
@@ -82,6 +85,10 @@ struct intc_irqpin_priv {
 	u8 shared_irq_mask;
 };
 
+struct intc_irqpin_irlm_config {
+	unsigned int irlm_bit;
+};
+
 static unsigned long intc_irqpin_read32(void __iomem *iomem)
 {
 	return ioread32(iomem);
@@ -345,10 +352,23 @@ static struct irq_domain_ops intc_irqpin
 	.xlate  = irq_domain_xlate_twocell,
 };
 
+static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a7779 = {
+	.irlm_bit = 23, /* ICR0.IRLM0 */
+};
+
+static const struct of_device_id intc_irqpin_dt_ids[] = {
+	{ .compatible = "renesas,intc-irqpin", },
+	{ .compatible = "renesas,intc-irqpin-r8a7779",
+	  .data = &intc_irqpin_irlm_r8a7779 },
+	{},
+};
+MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
+
 static int intc_irqpin_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct renesas_intc_irqpin_config *pdata = dev->platform_data;
+	const struct of_device_id *of_id;
 	struct intc_irqpin_priv *p;
 	struct intc_irqpin_iomem *i;
 	struct resource *io[INTC_IRQPIN_REG_NR];
@@ -391,10 +411,11 @@ static int intc_irqpin_probe(struct plat
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
-	/* get hold of manadatory IOMEM */
+	/* get hold of register banks */
+	memset(io, 0, sizeof(io));
 	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
 		io[k] = platform_get_resource(pdev, IORESOURCE_MEM, k);
-		if (!io[k]) {
+		if (!io[k] && k < INTC_IRQPIN_REG_NR_MANDATORY) {
 			dev_err(dev, "not enough IOMEM resources\n");
 			ret = -EINVAL;
 			goto err0;
@@ -422,6 +443,10 @@ static int intc_irqpin_probe(struct plat
 	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
 		i = &p->iomem[k];
 
+		/* handle optional registers */
+		if (!io[k])
+			continue;
+
 		switch (resource_size(io[k])) {
 		case 1:
 			i->width = 8;
@@ -448,6 +473,19 @@ static int intc_irqpin_probe(struct plat
 		}
 	}
 
+	/* configure "individual IRQ mode" where needed */
+	of_id = of_match_device(intc_irqpin_dt_ids, dev);
+	if (of_id && of_id->data) {
+		const struct intc_irqpin_irlm_config *irlm_config = of_id->data;
+
+		if (io[INTC_IRQPIN_REG_IRLM])
+			intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_IRLM,
+						      irlm_config->irlm_bit,
+						      1, 1);
+		else
+			dev_warn(dev, "unable to select IRLM mode\n");
+	}
+
 	/* mask all interrupts using priority */
 	for (k = 0; k < p->number_of_irqs; k++)
 		intc_irqpin_mask_unmask_prio(p, k, 1);
@@ -550,12 +588,6 @@ static int intc_irqpin_remove(struct pla
 	return 0;
 }
 
-static const struct of_device_id intc_irqpin_dt_ids[] = {
-	{ .compatible = "renesas,intc-irqpin", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
-
 static struct platform_driver intc_irqpin_device_driver = {
 	.probe		= intc_irqpin_probe,
 	.remove		= intc_irqpin_remove,

[PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Magnus Damm]

From: Magnus Damm <damm+renesas@opensource.se>

Add r8a7779 specific support for IRLM bit configuration
in the INTC-IRQPIN driver. Without this code we need
special workaround code in arch/arm/mach-shmobile.

The IRLM bit for the INTC hardware exists on various
older SH-based SoCs and is used to select between two
modes for the external interrupt pins IRQ0 to IRQ3:

IRLM = 0: (default from reset on r8a7779)
In this mode the pins IRQ0 to IRQ3 are used together
to give a value between 0 and 15 to the SoC. External
logic is required for masking. This mode is not
supported by the INTC-IRQPIN driver.

IRLM = 1: (needs this patch or configuration elsewhere)
In this mode IRQ0 to IRQ3 operate as 4 individual
external interrupt pins. In this mode the SMSC ethernet
chip can be used via IRQ1 on r8a7779 Marzen. This mode
is the only supported mode by the INTC-IRQPIN driver.

For this patch to work the r8a7779 DTS needs to pass
the ICR0 register as the last register bank.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Same version as sent on December 3rd.

 Written against renesas-devel-20141202-v3.18-rc7 which is
 basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
 
 Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt |    5 +
 drivers/irqchip/irq-renesas-intc-irqpin.c                                      |   50 ++++++++--
 2 files changed, 46 insertions(+), 9 deletions(-)

--- 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
+++ work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt	2014-12-03 20:25:13.000000000 +0900
@@ -9,6 +9,11 @@ Required properties:
     - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
     - "renesas,intc-irqpin-r8a7779" (R-Car H1)
     - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
+
+- 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-cells: has to be <2>: an interrupt index and flags, as defined in
   interrupts.txt in this directory
 
--- 0001/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ work/drivers/irqchip/irq-renesas-intc-irqpin.c	2014-12-03 20:32:59.000000000 +0900
@@ -30,6 +30,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_data/irq-renesas-intc-irqpin.h>
 #include <linux/pm_runtime.h>
 
@@ -40,7 +41,9 @@
 #define INTC_IRQPIN_REG_SOURCE 2 /* INTREQnn */
 #define INTC_IRQPIN_REG_MASK 3 /* INTMSKnn */
 #define INTC_IRQPIN_REG_CLEAR 4 /* INTMSKCLRnn */
-#define INTC_IRQPIN_REG_NR 5
+#define INTC_IRQPIN_REG_NR_MANDATORY 5
+#define INTC_IRQPIN_REG_IRLM 5 /* ICR0 with IRLM bit (optional) */
+#define INTC_IRQPIN_REG_NR 6
 
 /* INTC external IRQ PIN hardware register access:
  *
@@ -82,6 +85,10 @@ struct intc_irqpin_priv {
 	u8 shared_irq_mask;
 };
 
+struct intc_irqpin_irlm_config {
+	unsigned int irlm_bit;
+};
+
 static unsigned long intc_irqpin_read32(void __iomem *iomem)
 {
 	return ioread32(iomem);
@@ -345,10 +352,23 @@ static struct irq_domain_ops intc_irqpin
 	.xlate  = irq_domain_xlate_twocell,
 };
 
+static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a7779 = {
+	.irlm_bit = 23, /* ICR0.IRLM0 */
+};
+
+static const struct of_device_id intc_irqpin_dt_ids[] = {
+	{ .compatible = "renesas,intc-irqpin", },
+	{ .compatible = "renesas,intc-irqpin-r8a7779",
+	  .data = &intc_irqpin_irlm_r8a7779 },
+	{},
+};
+MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
+
 static int intc_irqpin_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct renesas_intc_irqpin_config *pdata = dev->platform_data;
+	const struct of_device_id *of_id;
 	struct intc_irqpin_priv *p;
 	struct intc_irqpin_iomem *i;
 	struct resource *io[INTC_IRQPIN_REG_NR];
@@ -391,10 +411,11 @@ static int intc_irqpin_probe(struct plat
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
-	/* get hold of manadatory IOMEM */
+	/* get hold of register banks */
+	memset(io, 0, sizeof(io));
 	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
 		io[k] = platform_get_resource(pdev, IORESOURCE_MEM, k);
-		if (!io[k]) {
+		if (!io[k] && k < INTC_IRQPIN_REG_NR_MANDATORY) {
 			dev_err(dev, "not enough IOMEM resources\n");
 			ret = -EINVAL;
 			goto err0;
@@ -422,6 +443,10 @@ static int intc_irqpin_probe(struct plat
 	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
 		i = &p->iomem[k];
 
+		/* handle optional registers */
+		if (!io[k])
+			continue;
+
 		switch (resource_size(io[k])) {
 		case 1:
 			i->width = 8;
@@ -448,6 +473,19 @@ static int intc_irqpin_probe(struct plat
 		}
 	}
 
+	/* configure "individual IRQ mode" where needed */
+	of_id = of_match_device(intc_irqpin_dt_ids, dev);
+	if (of_id && of_id->data) {
+		const struct intc_irqpin_irlm_config *irlm_config = of_id->data;
+
+		if (io[INTC_IRQPIN_REG_IRLM])
+			intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_IRLM,
+						      irlm_config->irlm_bit,
+						      1, 1);
+		else
+			dev_warn(dev, "unable to select IRLM mode\n");
+	}
+
 	/* mask all interrupts using priority */
 	for (k = 0; k < p->number_of_irqs; k++)
 		intc_irqpin_mask_unmask_prio(p, k, 1);
@@ -550,12 +588,6 @@ static int intc_irqpin_remove(struct pla
 	return 0;
 }
 
-static const struct of_device_id intc_irqpin_dt_ids[] = {
-	{ .compatible = "renesas,intc-irqpin", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
-
 static struct platform_driver intc_irqpin_device_driver = {
 	.probe		= intc_irqpin_probe,
 	.remove		= intc_irqpin_remove,

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Laurent Pinchart]

Hi Magnus,

Thank you for the patch.

On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Add r8a7779 specific support for IRLM bit configuration
> in the INTC-IRQPIN driver. Without this code we need
> special workaround code in arch/arm/mach-shmobile.
> 
> The IRLM bit for the INTC hardware exists on various
> older SH-based SoCs and is used to select between two
> modes for the external interrupt pins IRQ0 to IRQ3:
> 
> IRLM = 0: (default from reset on r8a7779)
> In this mode the pins IRQ0 to IRQ3 are used together
> to give a value between 0 and 15 to the SoC. External
> logic is required for masking. This mode is not
> supported by the INTC-IRQPIN driver.
> 
> IRLM = 1: (needs this patch or configuration elsewhere)
> In this mode IRQ0 to IRQ3 operate as 4 individual
> external interrupt pins. In this mode the SMSC ethernet
> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> is the only supported mode by the INTC-IRQPIN driver.
> 
> For this patch to work the r8a7779 DTS needs to pass
> the ICR0 register as the last register bank.
> 
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Same version as sent on December 3rd.
> 
>  Written against renesas-devel-20141202-v3.18-rc7 which is
>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
> 
>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.
> txt |    5 +
>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
>  2 files changed, 46 insertions(+), 9 deletions(-)
> 
> ---
> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-ir
> qpin.txt +++
> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-ir
> qpin.txt	2014-12-03 20:25:13.000000000 +0900 @@ -9,6 +9,11 @@ Required
> properties:
>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> +
> +- 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.

Ouch. Ouch. Multiple times. Very much :-/

I suppose it's too late to fix the DT bindings ?

>  - #interrupt-cells: has to be <2>: an interrupt index and flags, as defined
> in interrupts.txt in this directory
> 
> --- 0001/drivers/irqchip/irq-renesas-intc-irqpin.c
> +++ work/drivers/irqchip/irq-renesas-intc-irqpin.c	2014-12-03
> 20:32:59.000000000 +0900 @@ -30,6 +30,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_data/irq-renesas-intc-irqpin.h>
>  #include <linux/pm_runtime.h>
> 
> @@ -40,7 +41,9 @@
>  #define INTC_IRQPIN_REG_SOURCE 2 /* INTREQnn */
>  #define INTC_IRQPIN_REG_MASK 3 /* INTMSKnn */
>  #define INTC_IRQPIN_REG_CLEAR 4 /* INTMSKCLRnn */
> -#define INTC_IRQPIN_REG_NR 5
> +#define INTC_IRQPIN_REG_NR_MANDATORY 5
> +#define INTC_IRQPIN_REG_IRLM 5 /* ICR0 with IRLM bit (optional) */
> +#define INTC_IRQPIN_REG_NR 6
> 
>  /* INTC external IRQ PIN hardware register access:
>   *
> @@ -82,6 +85,10 @@ struct intc_irqpin_priv {
>  	u8 shared_irq_mask;
>  };
> 
> +struct intc_irqpin_irlm_config {
> +	unsigned int irlm_bit;
> +};
> +
>  static unsigned long intc_irqpin_read32(void __iomem *iomem)
>  {
>  	return ioread32(iomem);
> @@ -345,10 +352,23 @@ static struct irq_domain_ops intc_irqpin
>  	.xlate  = irq_domain_xlate_twocell,
>  };
> 
> +static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a7779 = {
> +	.irlm_bit = 23, /* ICR0.IRLM0 */
> +};
> +
> +static const struct of_device_id intc_irqpin_dt_ids[] = {
> +	{ .compatible = "renesas,intc-irqpin", },
> +	{ .compatible = "renesas,intc-irqpin-r8a7779",
> +	  .data = &intc_irqpin_irlm_r8a7779 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
> +
>  static int intc_irqpin_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct renesas_intc_irqpin_config *pdata = dev->platform_data;
> +	const struct of_device_id *of_id;
>  	struct intc_irqpin_priv *p;
>  	struct intc_irqpin_iomem *i;
>  	struct resource *io[INTC_IRQPIN_REG_NR];
> @@ -391,10 +411,11 @@ static int intc_irqpin_probe(struct plat
>  	pm_runtime_enable(dev);
>  	pm_runtime_get_sync(dev);
> 
> -	/* get hold of manadatory IOMEM */
> +	/* get hold of register banks */
> +	memset(io, 0, sizeof(io));
>  	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
>  		io[k] = platform_get_resource(pdev, IORESOURCE_MEM, k);
> -		if (!io[k]) {
> +		if (!io[k] && k < INTC_IRQPIN_REG_NR_MANDATORY) {
>  			dev_err(dev, "not enough IOMEM resources\n");
>  			ret = -EINVAL;
>  			goto err0;
> @@ -422,6 +443,10 @@ static int intc_irqpin_probe(struct plat
>  	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
>  		i = &p->iomem[k];
> 
> +		/* handle optional registers */
> +		if (!io[k])
> +			continue;
> +
>  		switch (resource_size(io[k])) {
>  		case 1:
>  			i->width = 8;
> @@ -448,6 +473,19 @@ static int intc_irqpin_probe(struct plat
>  		}
>  	}
> 
> +	/* configure "individual IRQ mode" where needed */
> +	of_id = of_match_device(intc_irqpin_dt_ids, dev);
> +	if (of_id && of_id->data) {
> +		const struct intc_irqpin_irlm_config *irlm_config = of_id->data;
> +
> +		if (io[INTC_IRQPIN_REG_IRLM])
> +			intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_IRLM,
> +						      irlm_config->irlm_bit,
> +						      1, 1);
> +		else
> +			dev_warn(dev, "unable to select IRLM mode\n");
> +	}
> +
>  	/* mask all interrupts using priority */
>  	for (k = 0; k < p->number_of_irqs; k++)
>  		intc_irqpin_mask_unmask_prio(p, k, 1);
> @@ -550,12 +588,6 @@ static int intc_irqpin_remove(struct pla
>  	return 0;
>  }
> 
> -static const struct of_device_id intc_irqpin_dt_ids[] = {
> -	{ .compatible = "renesas,intc-irqpin", },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
> -
>  static struct platform_driver intc_irqpin_device_driver = {
>  	.probe		= intc_irqpin_probe,
>  	.remove		= intc_irqpin_remove,

-- 
Regards,

Laurent Pinchart

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Laurent Pinchart]

Hi Magnus,

Thank you for the patch.

On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Add r8a7779 specific support for IRLM bit configuration
> in the INTC-IRQPIN driver. Without this code we need
> special workaround code in arch/arm/mach-shmobile.
> 
> The IRLM bit for the INTC hardware exists on various
> older SH-based SoCs and is used to select between two
> modes for the external interrupt pins IRQ0 to IRQ3:
> 
> IRLM = 0: (default from reset on r8a7779)
> In this mode the pins IRQ0 to IRQ3 are used together
> to give a value between 0 and 15 to the SoC. External
> logic is required for masking. This mode is not
> supported by the INTC-IRQPIN driver.
> 
> IRLM = 1: (needs this patch or configuration elsewhere)
> In this mode IRQ0 to IRQ3 operate as 4 individual
> external interrupt pins. In this mode the SMSC ethernet
> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> is the only supported mode by the INTC-IRQPIN driver.
> 
> For this patch to work the r8a7779 DTS needs to pass
> the ICR0 register as the last register bank.
> 
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Same version as sent on December 3rd.
> 
>  Written against renesas-devel-20141202-v3.18-rc7 which is
>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
> 
>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.
> txt |    5 +
>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
>  2 files changed, 46 insertions(+), 9 deletions(-)
> 
> ---
> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-ir
> qpin.txt +++
> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-ir
> qpin.txt	2014-12-03 20:25:13.000000000 +0900 @@ -9,6 +9,11 @@ Required
> properties:
>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> +
> +- 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.

Ouch. Ouch. Multiple times. Very much :-/

I suppose it's too late to fix the DT bindings ?

>  - #interrupt-cells: has to be <2>: an interrupt index and flags, as defined
> in interrupts.txt in this directory
> 
> --- 0001/drivers/irqchip/irq-renesas-intc-irqpin.c
> +++ work/drivers/irqchip/irq-renesas-intc-irqpin.c	2014-12-03
> 20:32:59.000000000 +0900 @@ -30,6 +30,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_data/irq-renesas-intc-irqpin.h>
>  #include <linux/pm_runtime.h>
> 
> @@ -40,7 +41,9 @@
>  #define INTC_IRQPIN_REG_SOURCE 2 /* INTREQnn */
>  #define INTC_IRQPIN_REG_MASK 3 /* INTMSKnn */
>  #define INTC_IRQPIN_REG_CLEAR 4 /* INTMSKCLRnn */
> -#define INTC_IRQPIN_REG_NR 5
> +#define INTC_IRQPIN_REG_NR_MANDATORY 5
> +#define INTC_IRQPIN_REG_IRLM 5 /* ICR0 with IRLM bit (optional) */
> +#define INTC_IRQPIN_REG_NR 6
> 
>  /* INTC external IRQ PIN hardware register access:
>   *
> @@ -82,6 +85,10 @@ struct intc_irqpin_priv {
>  	u8 shared_irq_mask;
>  };
> 
> +struct intc_irqpin_irlm_config {
> +	unsigned int irlm_bit;
> +};
> +
>  static unsigned long intc_irqpin_read32(void __iomem *iomem)
>  {
>  	return ioread32(iomem);
> @@ -345,10 +352,23 @@ static struct irq_domain_ops intc_irqpin
>  	.xlate  = irq_domain_xlate_twocell,
>  };
> 
> +static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a7779 = {
> +	.irlm_bit = 23, /* ICR0.IRLM0 */
> +};
> +
> +static const struct of_device_id intc_irqpin_dt_ids[] = {
> +	{ .compatible = "renesas,intc-irqpin", },
> +	{ .compatible = "renesas,intc-irqpin-r8a7779",
> +	  .data = &intc_irqpin_irlm_r8a7779 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
> +
>  static int intc_irqpin_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct renesas_intc_irqpin_config *pdata = dev->platform_data;
> +	const struct of_device_id *of_id;
>  	struct intc_irqpin_priv *p;
>  	struct intc_irqpin_iomem *i;
>  	struct resource *io[INTC_IRQPIN_REG_NR];
> @@ -391,10 +411,11 @@ static int intc_irqpin_probe(struct plat
>  	pm_runtime_enable(dev);
>  	pm_runtime_get_sync(dev);
> 
> -	/* get hold of manadatory IOMEM */
> +	/* get hold of register banks */
> +	memset(io, 0, sizeof(io));
>  	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
>  		io[k] = platform_get_resource(pdev, IORESOURCE_MEM, k);
> -		if (!io[k]) {
> +		if (!io[k] && k < INTC_IRQPIN_REG_NR_MANDATORY) {
>  			dev_err(dev, "not enough IOMEM resources\n");
>  			ret = -EINVAL;
>  			goto err0;
> @@ -422,6 +443,10 @@ static int intc_irqpin_probe(struct plat
>  	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
>  		i = &p->iomem[k];
> 
> +		/* handle optional registers */
> +		if (!io[k])
> +			continue;
> +
>  		switch (resource_size(io[k])) {
>  		case 1:
>  			i->width = 8;
> @@ -448,6 +473,19 @@ static int intc_irqpin_probe(struct plat
>  		}
>  	}
> 
> +	/* configure "individual IRQ mode" where needed */
> +	of_id = of_match_device(intc_irqpin_dt_ids, dev);
> +	if (of_id && of_id->data) {
> +		const struct intc_irqpin_irlm_config *irlm_config = of_id->data;
> +
> +		if (io[INTC_IRQPIN_REG_IRLM])
> +			intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_IRLM,
> +						      irlm_config->irlm_bit,
> +						      1, 1);
> +		else
> +			dev_warn(dev, "unable to select IRLM mode\n");
> +	}
> +
>  	/* mask all interrupts using priority */
>  	for (k = 0; k < p->number_of_irqs; k++)
>  		intc_irqpin_mask_unmask_prio(p, k, 1);
> @@ -550,12 +588,6 @@ static int intc_irqpin_remove(struct pla
>  	return 0;
>  }
> 
> -static const struct of_device_id intc_irqpin_dt_ids[] = {
> -	{ .compatible = "renesas,intc-irqpin", },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
> -
>  static struct platform_driver intc_irqpin_device_driver = {
>  	.probe		= intc_irqpin_probe,
>  	.remove		= intc_irqpin_remove,

-- 
Regards,

Laurent Pinchart

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Magnus Damm]

Hi Laurent,

On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
>> From: Magnus Damm <damm+renesas@opensource.se>
>>
>> Add r8a7779 specific support for IRLM bit configuration
>> in the INTC-IRQPIN driver. Without this code we need
>> special workaround code in arch/arm/mach-shmobile.
>>
>> The IRLM bit for the INTC hardware exists on various
>> older SH-based SoCs and is used to select between two
>> modes for the external interrupt pins IRQ0 to IRQ3:
>>
>> IRLM = 0: (default from reset on r8a7779)
>> In this mode the pins IRQ0 to IRQ3 are used together
>> to give a value between 0 and 15 to the SoC. External
>> logic is required for masking. This mode is not
>> supported by the INTC-IRQPIN driver.
>>
>> IRLM = 1: (needs this patch or configuration elsewhere)
>> In this mode IRQ0 to IRQ3 operate as 4 individual
>> external interrupt pins. In this mode the SMSC ethernet
>> chip can be used via IRQ1 on r8a7779 Marzen. This mode
>> is the only supported mode by the INTC-IRQPIN driver.
>>
>> For this patch to work the r8a7779 DTS needs to pass
>> the ICR0 register as the last register bank.
>>
>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> ---
>>
>>  Same version as sent on December 3rd.
>>
>>  Written against renesas-devel-20141202-v3.18-rc7 which is
>>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
>>
>>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.
>> txt |    5 +
>>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
>>  2 files changed, 46 insertions(+), 9 deletions(-)
>>
>> ---
>> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-ir
>> qpin.txt +++
>> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-ir
>> qpin.txt      2014-12-03 20:25:13.000000000 +0900 @@ -9,6 +9,11 @@ Required
>> properties:
>>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
>>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
>>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
>> +
>> +- 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.
>
> Ouch. Ouch. Multiple times. Very much :-/
>
> I suppose it's too late to fix the DT bindings ?

I don't think it is ever too late, but the question is just how urgent
it is to get rid of the Marzen C board code. With the code in this
patch queue up we can get rid of the board code. If someone decides to
rework the INTC IRQPIN bits then we will have to live with the Marzen
code for longer. And if we should do it right and describe the actual
hardware, then perhaps we should consider the non-IRQ-pin interrupts
included in INTC for some SoCs. So it rapidly gets more complex...

Cheers,

/ magnus

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Magnus Damm]

Hi Laurent,

On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> Thank you for the patch.
>
> On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
>> From: Magnus Damm <damm+renesas@opensource.se>
>>
>> Add r8a7779 specific support for IRLM bit configuration
>> in the INTC-IRQPIN driver. Without this code we need
>> special workaround code in arch/arm/mach-shmobile.
>>
>> The IRLM bit for the INTC hardware exists on various
>> older SH-based SoCs and is used to select between two
>> modes for the external interrupt pins IRQ0 to IRQ3:
>>
>> IRLM = 0: (default from reset on r8a7779)
>> In this mode the pins IRQ0 to IRQ3 are used together
>> to give a value between 0 and 15 to the SoC. External
>> logic is required for masking. This mode is not
>> supported by the INTC-IRQPIN driver.
>>
>> IRLM = 1: (needs this patch or configuration elsewhere)
>> In this mode IRQ0 to IRQ3 operate as 4 individual
>> external interrupt pins. In this mode the SMSC ethernet
>> chip can be used via IRQ1 on r8a7779 Marzen. This mode
>> is the only supported mode by the INTC-IRQPIN driver.
>>
>> For this patch to work the r8a7779 DTS needs to pass
>> the ICR0 register as the last register bank.
>>
>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> ---
>>
>>  Same version as sent on December 3rd.
>>
>>  Written against renesas-devel-20141202-v3.18-rc7 which is
>>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
>>
>>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.
>> txt |    5 +
>>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
>>  2 files changed, 46 insertions(+), 9 deletions(-)
>>
>> ---
>> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-ir
>> qpin.txt +++
>> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-ir
>> qpin.txt      2014-12-03 20:25:13.000000000 +0900 @@ -9,6 +9,11 @@ Required
>> properties:
>>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
>>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
>>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
>> +
>> +- 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.
>
> Ouch. Ouch. Multiple times. Very much :-/
>
> I suppose it's too late to fix the DT bindings ?

I don't think it is ever too late, but the question is just how urgent
it is to get rid of the Marzen C board code. With the code in this
patch queue up we can get rid of the board code. If someone decides to
rework the INTC IRQPIN bits then we will have to live with the Marzen
code for longer. And if we should do it right and describe the actual
hardware, then perhaps we should consider the non-IRQ-pin interrupts
included in INTC for some SoCs. So it rapidly gets more complex...

Cheers,

/ magnus

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Laurent Pinchart]

Hi Magnus,

On Thursday 18 December 2014 10:26:27 Magnus Damm wrote:
> On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart wrote:
> > On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
> >> From: Magnus Damm <damm+renesas@opensource.se>
> >> 
> >> Add r8a7779 specific support for IRLM bit configuration
> >> in the INTC-IRQPIN driver. Without this code we need
> >> special workaround code in arch/arm/mach-shmobile.
> >> 
> >> The IRLM bit for the INTC hardware exists on various
> >> older SH-based SoCs and is used to select between two
> >> modes for the external interrupt pins IRQ0 to IRQ3:
> >> 
> >> IRLM = 0: (default from reset on r8a7779)
> >> In this mode the pins IRQ0 to IRQ3 are used together
> >> to give a value between 0 and 15 to the SoC. External
> >> logic is required for masking. This mode is not
> >> supported by the INTC-IRQPIN driver.
> >> 
> >> IRLM = 1: (needs this patch or configuration elsewhere)
> >> In this mode IRQ0 to IRQ3 operate as 4 individual
> >> external interrupt pins. In this mode the SMSC ethernet
> >> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> >> is the only supported mode by the INTC-IRQPIN driver.
> >> 
> >> For this patch to work the r8a7779 DTS needs to pass
> >> the ICR0 register as the last register bank.
> >> 
> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >> ---
> >> 
> >>  Same version as sent on December 3rd.
> >>  
> >>  Written against renesas-devel-20141202-v3.18-rc7 which is
> >>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
> >>  
> >>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqp
> >>  in.txt |    5 +
> >>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
> >>  2 files changed, 46 insertions(+), 9 deletions(-)
> >> 
> >> ---
> >> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> >> ir qpin.txt +++
> >> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> >> irqpin.txt      2014-12-03 20:25:13.000000000 +0900
> >> @@ -9,6 +9,11 @@
> >> Required properties:
> >>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
> >>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
> >>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> >> +
> >> +- 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.
> > 
> > Ouch. Ouch. Multiple times. Very much :-/
> > 
> > I suppose it's too late to fix the DT bindings ?
> 
> I don't think it is ever too late, but the question is just how urgent
> it is to get rid of the Marzen C board code. With the code in this
> patch queue up we can get rid of the board code. If someone decides to
> rework the INTC IRQPIN bits then we will have to live with the Marzen
> code for longer. And if we should do it right and describe the actual
> hardware, then perhaps we should consider the non-IRQ-pin interrupts
> included in INTC for some SoCs. So it rapidly gets more complex...

I don't think there's a need to delay getting rid of Marzen legacy code, my 
comment wasn't an attempt to delay your efforts :-)

Looking at the hardware we should have a single INTC DT node, not four. What 
I'm wondering is whether we could still fix the DT bindings, or if the 
backward compatibility requirements would make it too painful, especially 
given that R-Car Gen2 doesn't use INTC.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Laurent Pinchart]

Hi Magnus,

On Thursday 18 December 2014 10:26:27 Magnus Damm wrote:
> On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart wrote:
> > On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
> >> From: Magnus Damm <damm+renesas@opensource.se>
> >> 
> >> Add r8a7779 specific support for IRLM bit configuration
> >> in the INTC-IRQPIN driver. Without this code we need
> >> special workaround code in arch/arm/mach-shmobile.
> >> 
> >> The IRLM bit for the INTC hardware exists on various
> >> older SH-based SoCs and is used to select between two
> >> modes for the external interrupt pins IRQ0 to IRQ3:
> >> 
> >> IRLM = 0: (default from reset on r8a7779)
> >> In this mode the pins IRQ0 to IRQ3 are used together
> >> to give a value between 0 and 15 to the SoC. External
> >> logic is required for masking. This mode is not
> >> supported by the INTC-IRQPIN driver.
> >> 
> >> IRLM = 1: (needs this patch or configuration elsewhere)
> >> In this mode IRQ0 to IRQ3 operate as 4 individual
> >> external interrupt pins. In this mode the SMSC ethernet
> >> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> >> is the only supported mode by the INTC-IRQPIN driver.
> >> 
> >> For this patch to work the r8a7779 DTS needs to pass
> >> the ICR0 register as the last register bank.
> >> 
> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >> ---
> >> 
> >>  Same version as sent on December 3rd.
> >>  
> >>  Written against renesas-devel-20141202-v3.18-rc7 which is
> >>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
> >>  
> >>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqp
> >>  in.txt |    5 +
> >>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
> >>  2 files changed, 46 insertions(+), 9 deletions(-)
> >> 
> >> ---
> >> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> >> ir qpin.txt +++
> >> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> >> irqpin.txt      2014-12-03 20:25:13.000000000 +0900
> >> @@ -9,6 +9,11 @@
> >> Required properties:
> >>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
> >>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
> >>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> >> +
> >> +- 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.
> > 
> > Ouch. Ouch. Multiple times. Very much :-/
> > 
> > I suppose it's too late to fix the DT bindings ?
> 
> I don't think it is ever too late, but the question is just how urgent
> it is to get rid of the Marzen C board code. With the code in this
> patch queue up we can get rid of the board code. If someone decides to
> rework the INTC IRQPIN bits then we will have to live with the Marzen
> code for longer. And if we should do it right and describe the actual
> hardware, then perhaps we should consider the non-IRQ-pin interrupts
> included in INTC for some SoCs. So it rapidly gets more complex...

I don't think there's a need to delay getting rid of Marzen legacy code, my 
comment wasn't an attempt to delay your efforts :-)

Looking at the hardware we should have a single INTC DT node, not four. What 
I'm wondering is whether we could still fix the DT bindings, or if the 
backward compatibility requirements would make it too painful, especially 
given that R-Car Gen2 doesn't use INTC.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Magnus Damm]

Hi Laurent,

On Thu, Dec 18, 2014 at 11:39 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> On Thursday 18 December 2014 10:26:27 Magnus Damm wrote:
>> On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart wrote:
>> > On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
>> >> From: Magnus Damm <damm+renesas@opensource.se>
>> >>
>> >> Add r8a7779 specific support for IRLM bit configuration
>> >> in the INTC-IRQPIN driver. Without this code we need
>> >> special workaround code in arch/arm/mach-shmobile.
>> >>
>> >> The IRLM bit for the INTC hardware exists on various
>> >> older SH-based SoCs and is used to select between two
>> >> modes for the external interrupt pins IRQ0 to IRQ3:
>> >>
>> >> IRLM = 0: (default from reset on r8a7779)
>> >> In this mode the pins IRQ0 to IRQ3 are used together
>> >> to give a value between 0 and 15 to the SoC. External
>> >> logic is required for masking. This mode is not
>> >> supported by the INTC-IRQPIN driver.
>> >>
>> >> IRLM = 1: (needs this patch or configuration elsewhere)
>> >> In this mode IRQ0 to IRQ3 operate as 4 individual
>> >> external interrupt pins. In this mode the SMSC ethernet
>> >> chip can be used via IRQ1 on r8a7779 Marzen. This mode
>> >> is the only supported mode by the INTC-IRQPIN driver.
>> >>
>> >> For this patch to work the r8a7779 DTS needs to pass
>> >> the ICR0 register as the last register bank.
>> >>
>> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> >> ---
>> >>
>> >>  Same version as sent on December 3rd.
>> >>
>> >>  Written against renesas-devel-20141202-v3.18-rc7 which is
>> >>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
>> >>
>> >>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqp
>> >>  in.txt |    5 +
>> >>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
>> >>  2 files changed, 46 insertions(+), 9 deletions(-)
>> >>
>> >> ---
>> >> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
>> >> ir qpin.txt +++
>> >> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
>> >> irqpin.txt      2014-12-03 20:25:13.000000000 +0900
>> >> @@ -9,6 +9,11 @@
>> >> Required properties:
>> >>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
>> >>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
>> >>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
>> >> +
>> >> +- 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.
>> >
>> > Ouch. Ouch. Multiple times. Very much :-/
>> >
>> > I suppose it's too late to fix the DT bindings ?
>>
>> I don't think it is ever too late, but the question is just how urgent
>> it is to get rid of the Marzen C board code. With the code in this
>> patch queue up we can get rid of the board code. If someone decides to
>> rework the INTC IRQPIN bits then we will have to live with the Marzen
>> code for longer. And if we should do it right and describe the actual
>> hardware, then perhaps we should consider the non-IRQ-pin interrupts
>> included in INTC for some SoCs. So it rapidly gets more complex...
>
> I don't think there's a need to delay getting rid of Marzen legacy code, my
> comment wasn't an attempt to delay your efforts :-)

Thanks. Just to be clear, I've never had any problems with board code
written in C. =)

> Looking at the hardware we should have a single INTC DT node, not four. What
> I'm wondering is whether we could still fix the DT bindings, or if the
> backward compatibility requirements would make it too painful, especially
> given that R-Car Gen2 doesn't use INTC.

My feeling is that it is a waste of time for existing SoCs. Only old
stuff is using it from the mobile line (r8a7740 and sh73a0) and R-Car
Gen1 (r8a7779 and r8a7778). If for some unknown reason new SoCs come
out that make use of this hardware then we can rework at that point.

If you want to poke around with interrupts on old systems then how
about looking into DT-enabled PINT support for sh73a0 and others?

Cheers,

/ magnus

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Magnus Damm]

Hi Laurent,

On Thu, Dec 18, 2014 at 11:39 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> On Thursday 18 December 2014 10:26:27 Magnus Damm wrote:
>> On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart wrote:
>> > On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
>> >> From: Magnus Damm <damm+renesas@opensource.se>
>> >>
>> >> Add r8a7779 specific support for IRLM bit configuration
>> >> in the INTC-IRQPIN driver. Without this code we need
>> >> special workaround code in arch/arm/mach-shmobile.
>> >>
>> >> The IRLM bit for the INTC hardware exists on various
>> >> older SH-based SoCs and is used to select between two
>> >> modes for the external interrupt pins IRQ0 to IRQ3:
>> >>
>> >> IRLM = 0: (default from reset on r8a7779)
>> >> In this mode the pins IRQ0 to IRQ3 are used together
>> >> to give a value between 0 and 15 to the SoC. External
>> >> logic is required for masking. This mode is not
>> >> supported by the INTC-IRQPIN driver.
>> >>
>> >> IRLM = 1: (needs this patch or configuration elsewhere)
>> >> In this mode IRQ0 to IRQ3 operate as 4 individual
>> >> external interrupt pins. In this mode the SMSC ethernet
>> >> chip can be used via IRQ1 on r8a7779 Marzen. This mode
>> >> is the only supported mode by the INTC-IRQPIN driver.
>> >>
>> >> For this patch to work the r8a7779 DTS needs to pass
>> >> the ICR0 register as the last register bank.
>> >>
>> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> >> ---
>> >>
>> >>  Same version as sent on December 3rd.
>> >>
>> >>  Written against renesas-devel-20141202-v3.18-rc7 which is
>> >>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
>> >>
>> >>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqp
>> >>  in.txt |    5 +
>> >>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
>> >>  2 files changed, 46 insertions(+), 9 deletions(-)
>> >>
>> >> ---
>> >> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
>> >> ir qpin.txt +++
>> >> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
>> >> irqpin.txt      2014-12-03 20:25:13.000000000 +0900
>> >> @@ -9,6 +9,11 @@
>> >> Required properties:
>> >>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
>> >>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
>> >>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
>> >> +
>> >> +- 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.
>> >
>> > Ouch. Ouch. Multiple times. Very much :-/
>> >
>> > I suppose it's too late to fix the DT bindings ?
>>
>> I don't think it is ever too late, but the question is just how urgent
>> it is to get rid of the Marzen C board code. With the code in this
>> patch queue up we can get rid of the board code. If someone decides to
>> rework the INTC IRQPIN bits then we will have to live with the Marzen
>> code for longer. And if we should do it right and describe the actual
>> hardware, then perhaps we should consider the non-IRQ-pin interrupts
>> included in INTC for some SoCs. So it rapidly gets more complex...
>
> I don't think there's a need to delay getting rid of Marzen legacy code, my
> comment wasn't an attempt to delay your efforts :-)

Thanks. Just to be clear, I've never had any problems with board code
written in C. =)

> Looking at the hardware we should have a single INTC DT node, not four. What
> I'm wondering is whether we could still fix the DT bindings, or if the
> backward compatibility requirements would make it too painful, especially
> given that R-Car Gen2 doesn't use INTC.

My feeling is that it is a waste of time for existing SoCs. Only old
stuff is using it from the mobile line (r8a7740 and sh73a0) and R-Car
Gen1 (r8a7779 and r8a7778). If for some unknown reason new SoCs come
out that make use of this hardware then we can rework at that point.

If you want to poke around with interrupts on old systems then how
about looking into DT-enabled PINT support for sh73a0 and others?

Cheers,

/ magnus

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Jason Cooper]

On Thu, Dec 18, 2014 at 04:39:51AM +0200, Laurent Pinchart wrote:
> Hi Magnus,
> 
> On Thursday 18 December 2014 10:26:27 Magnus Damm wrote:
> > On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart wrote:
> > > On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
> > >> From: Magnus Damm <damm+renesas@opensource.se>
> > >> 
> > >> Add r8a7779 specific support for IRLM bit configuration
> > >> in the INTC-IRQPIN driver. Without this code we need
> > >> special workaround code in arch/arm/mach-shmobile.
> > >> 
> > >> The IRLM bit for the INTC hardware exists on various
> > >> older SH-based SoCs and is used to select between two
> > >> modes for the external interrupt pins IRQ0 to IRQ3:
> > >> 
> > >> IRLM = 0: (default from reset on r8a7779)
> > >> In this mode the pins IRQ0 to IRQ3 are used together
> > >> to give a value between 0 and 15 to the SoC. External
> > >> logic is required for masking. This mode is not
> > >> supported by the INTC-IRQPIN driver.
> > >> 
> > >> IRLM = 1: (needs this patch or configuration elsewhere)
> > >> In this mode IRQ0 to IRQ3 operate as 4 individual
> > >> external interrupt pins. In this mode the SMSC ethernet
> > >> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> > >> is the only supported mode by the INTC-IRQPIN driver.
> > >> 
> > >> For this patch to work the r8a7779 DTS needs to pass
> > >> the ICR0 register as the last register bank.
> > >> 
> > >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> > >> ---
> > >> 
> > >>  Same version as sent on December 3rd.
> > >>  
> > >>  Written against renesas-devel-20141202-v3.18-rc7 which is
> > >>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
> > >>  
> > >>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqp
> > >>  in.txt |    5 +
> > >>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
> > >>  2 files changed, 46 insertions(+), 9 deletions(-)
> > >> 
> > >> ---
> > >> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> > >> ir qpin.txt +++
> > >> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> > >> irqpin.txt      2014-12-03 20:25:13.000000000 +0900
> > >> @@ -9,6 +9,11 @@
> > >> Required properties:
> > >>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
> > >>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
> > >>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> > >> +
> > >> +- 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.
> > > 
> > > Ouch. Ouch. Multiple times. Very much :-/
> > > 
> > > I suppose it's too late to fix the DT bindings ?

Fix?  Is this a change to the reg property (from an undocumented
version)?  Or just adding the reg property?  If the later, then as long
as the driver preserves the old behavior when the reg property isn't
present, we should be good.

If the case was the former, then we need to keep in mind that these are
old SoCs and that this binding is mfgr-specific.  afair from the DT
maintainers, it's acceptable to have a little instability under these
conditions.

> > I don't think it is ever too late, but the question is just how urgent
> > it is to get rid of the Marzen C board code. With the code in this
> > patch queue up we can get rid of the board code. If someone decides to
> > rework the INTC IRQPIN bits then we will have to live with the Marzen
> > code for longer. And if we should do it right and describe the actual
> > hardware, then perhaps we should consider the non-IRQ-pin interrupts
> > included in INTC for some SoCs. So it rapidly gets more complex...
> 
> I don't think there's a need to delay getting rid of Marzen legacy code, my 
> comment wasn't an attempt to delay your efforts :-)
> 
> Looking at the hardware we should have a single INTC DT node, not four. What 
> I'm wondering is whether we could still fix the DT bindings, or if the 
> backward compatibility requirements would make it too painful, especially 
> given that R-Car Gen2 doesn't use INTC.

Please see above.  I'm sure the DT maintainers will chirp up if the
winds have shifted, but I'd rather we do it right if we're breaking
compatibility anyway.

thx,

Jason.

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Jason Cooper]

On Thu, Dec 18, 2014 at 04:39:51AM +0200, Laurent Pinchart wrote:
> Hi Magnus,
> 
> On Thursday 18 December 2014 10:26:27 Magnus Damm wrote:
> > On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart wrote:
> > > On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
> > >> From: Magnus Damm <damm+renesas@opensource.se>
> > >> 
> > >> Add r8a7779 specific support for IRLM bit configuration
> > >> in the INTC-IRQPIN driver. Without this code we need
> > >> special workaround code in arch/arm/mach-shmobile.
> > >> 
> > >> The IRLM bit for the INTC hardware exists on various
> > >> older SH-based SoCs and is used to select between two
> > >> modes for the external interrupt pins IRQ0 to IRQ3:
> > >> 
> > >> IRLM = 0: (default from reset on r8a7779)
> > >> In this mode the pins IRQ0 to IRQ3 are used together
> > >> to give a value between 0 and 15 to the SoC. External
> > >> logic is required for masking. This mode is not
> > >> supported by the INTC-IRQPIN driver.
> > >> 
> > >> IRLM = 1: (needs this patch or configuration elsewhere)
> > >> In this mode IRQ0 to IRQ3 operate as 4 individual
> > >> external interrupt pins. In this mode the SMSC ethernet
> > >> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> > >> is the only supported mode by the INTC-IRQPIN driver.
> > >> 
> > >> For this patch to work the r8a7779 DTS needs to pass
> > >> the ICR0 register as the last register bank.
> > >> 
> > >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> > >> ---
> > >> 
> > >>  Same version as sent on December 3rd.
> > >>  
> > >>  Written against renesas-devel-20141202-v3.18-rc7 which is
> > >>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
> > >>  
> > >>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqp
> > >>  in.txt |    5 +
> > >>  drivers/irqchip/irq-renesas-intc-irqpi                |   50 ++++++++--
> > >>  2 files changed, 46 insertions(+), 9 deletions(-)
> > >> 
> > >> ---
> > >> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> > >> ir qpin.txt +++
> > >> work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> > >> irqpin.txt      2014-12-03 20:25:13.000000000 +0900
> > >> @@ -9,6 +9,11 @@
> > >> Required properties:
> > >>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
> > >>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
> > >>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> > >> +
> > >> +- 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.
> > > 
> > > Ouch. Ouch. Multiple times. Very much :-/
> > > 
> > > I suppose it's too late to fix the DT bindings ?

Fix?  Is this a change to the reg property (from an undocumented
version)?  Or just adding the reg property?  If the later, then as long
as the driver preserves the old behavior when the reg property isn't
present, we should be good.

If the case was the former, then we need to keep in mind that these are
old SoCs and that this binding is mfgr-specific.  afair from the DT
maintainers, it's acceptable to have a little instability under these
conditions.

> > I don't think it is ever too late, but the question is just how urgent
> > it is to get rid of the Marzen C board code. With the code in this
> > patch queue up we can get rid of the board code. If someone decides to
> > rework the INTC IRQPIN bits then we will have to live with the Marzen
> > code for longer. And if we should do it right and describe the actual
> > hardware, then perhaps we should consider the non-IRQ-pin interrupts
> > included in INTC for some SoCs. So it rapidly gets more complex...
> 
> I don't think there's a need to delay getting rid of Marzen legacy code, my 
> comment wasn't an attempt to delay your efforts :-)
> 
> Looking at the hardware we should have a single INTC DT node, not four. What 
> I'm wondering is whether we could still fix the DT bindings, or if the 
> backward compatibility requirements would make it too painful, especially 
> given that R-Car Gen2 doesn't use INTC.

Please see above.  I'm sure the DT maintainers will chirp up if the
winds have shifted, but I'd rather we do it right if we're breaking
compatibility anyway.

thx,

Jason.

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Jason Cooper]

On Mon, Dec 15, 2014 at 02:09:20PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Add r8a7779 specific support for IRLM bit configuration
> in the INTC-IRQPIN driver. Without this code we need
> special workaround code in arch/arm/mach-shmobile.
> 
> The IRLM bit for the INTC hardware exists on various
> older SH-based SoCs and is used to select between two
> modes for the external interrupt pins IRQ0 to IRQ3:
> 
> IRLM = 0: (default from reset on r8a7779)
> In this mode the pins IRQ0 to IRQ3 are used together
> to give a value between 0 and 15 to the SoC. External
> logic is required for masking. This mode is not
> supported by the INTC-IRQPIN driver.
> 
> IRLM = 1: (needs this patch or configuration elsewhere)
> In this mode IRQ0 to IRQ3 operate as 4 individual
> external interrupt pins. In this mode the SMSC ethernet
> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> is the only supported mode by the INTC-IRQPIN driver.
> 
> For this patch to work the r8a7779 DTS needs to pass
> the ICR0 register as the last register bank.
> 
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Same version as sent on December 3rd.
> 
>  Written against renesas-devel-20141202-v3.18-rc7 which is
>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
>  
>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt |    5 +
>  drivers/irqchip/irq-renesas-intc-irqpin.c                                      |   50 ++++++++--
>  2 files changed, 46 insertions(+), 9 deletions(-)
> 
> --- 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
> +++ work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt	2014-12-03 20:25:13.000000000 +0900
> @@ -9,6 +9,11 @@ Required properties:
>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> +
> +- 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-cells: has to be <2>: an interrupt index and flags, as defined in
>    interrupts.txt in this directory
>  
> --- 0001/drivers/irqchip/irq-renesas-intc-irqpin.c
> +++ work/drivers/irqchip/irq-renesas-intc-irqpin.c	2014-12-03 20:32:59.000000000 +0900
> @@ -30,6 +30,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_data/irq-renesas-intc-irqpin.h>
>  #include <linux/pm_runtime.h>
>  
> @@ -40,7 +41,9 @@
>  #define INTC_IRQPIN_REG_SOURCE 2 /* INTREQnn */
>  #define INTC_IRQPIN_REG_MASK 3 /* INTMSKnn */
>  #define INTC_IRQPIN_REG_CLEAR 4 /* INTMSKCLRnn */
> -#define INTC_IRQPIN_REG_NR 5
> +#define INTC_IRQPIN_REG_NR_MANDATORY 5
> +#define INTC_IRQPIN_REG_IRLM 5 /* ICR0 with IRLM bit (optional) */
> +#define INTC_IRQPIN_REG_NR 6
>  
>  /* INTC external IRQ PIN hardware register access:
>   *
> @@ -82,6 +85,10 @@ struct intc_irqpin_priv {
>  	u8 shared_irq_mask;
>  };
>  
> +struct intc_irqpin_irlm_config {
> +	unsigned int irlm_bit;
> +};
> +
>  static unsigned long intc_irqpin_read32(void __iomem *iomem)
>  {
>  	return ioread32(iomem);
> @@ -345,10 +352,23 @@ static struct irq_domain_ops intc_irqpin
>  	.xlate  = irq_domain_xlate_twocell,
>  };
>  
> +static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a7779 = {
> +	.irlm_bit = 23, /* ICR0.IRLM0 */
> +};
> +
> +static const struct of_device_id intc_irqpin_dt_ids[] = {
> +	{ .compatible = "renesas,intc-irqpin", },
> +	{ .compatible = "renesas,intc-irqpin-r8a7779",
> +	  .data = &intc_irqpin_irlm_r8a7779 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
> +
>  static int intc_irqpin_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct renesas_intc_irqpin_config *pdata = dev->platform_data;
> +	const struct of_device_id *of_id;
>  	struct intc_irqpin_priv *p;
>  	struct intc_irqpin_iomem *i;
>  	struct resource *io[INTC_IRQPIN_REG_NR];
> @@ -391,10 +411,11 @@ static int intc_irqpin_probe(struct plat
>  	pm_runtime_enable(dev);
>  	pm_runtime_get_sync(dev);
>  
> -	/* get hold of manadatory IOMEM */
> +	/* get hold of register banks */
> +	memset(io, 0, sizeof(io));
>  	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
>  		io[k] = platform_get_resource(pdev, IORESOURCE_MEM, k);
> -		if (!io[k]) {
> +		if (!io[k] && k < INTC_IRQPIN_REG_NR_MANDATORY) {
>  			dev_err(dev, "not enough IOMEM resources\n");
>  			ret = -EINVAL;
>  			goto err0;
> @@ -422,6 +443,10 @@ static int intc_irqpin_probe(struct plat
>  	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
>  		i = &p->iomem[k];
>  
> +		/* handle optional registers */
> +		if (!io[k])
> +			continue;
> +
>  		switch (resource_size(io[k])) {
>  		case 1:
>  			i->width = 8;
> @@ -448,6 +473,19 @@ static int intc_irqpin_probe(struct plat
>  		}
>  	}
>  
> +	/* configure "individual IRQ mode" where needed */
> +	of_id = of_match_device(intc_irqpin_dt_ids, dev);
> +	if (of_id && of_id->data) {
> +		const struct intc_irqpin_irlm_config *irlm_config = of_id->data;
> +
> +		if (io[INTC_IRQPIN_REG_IRLM])
> +			intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_IRLM,
> +						      irlm_config->irlm_bit,
> +						      1, 1);
> +		else
> +			dev_warn(dev, "unable to select IRLM mode\n");

What was the default behavior of the arch code before the DT binding
change?  Should that action be performed here?

thx,

Jason.

> +	}
> +
>  	/* mask all interrupts using priority */
>  	for (k = 0; k < p->number_of_irqs; k++)
>  		intc_irqpin_mask_unmask_prio(p, k, 1);
> @@ -550,12 +588,6 @@ static int intc_irqpin_remove(struct pla
>  	return 0;
>  }
>  
> -static const struct of_device_id intc_irqpin_dt_ids[] = {
> -	{ .compatible = "renesas,intc-irqpin", },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
> -
>  static struct platform_driver intc_irqpin_device_driver = {
>  	.probe		= intc_irqpin_probe,
>  	.remove		= intc_irqpin_remove,

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Jason Cooper]

On Mon, Dec 15, 2014 at 02:09:20PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Add r8a7779 specific support for IRLM bit configuration
> in the INTC-IRQPIN driver. Without this code we need
> special workaround code in arch/arm/mach-shmobile.
> 
> The IRLM bit for the INTC hardware exists on various
> older SH-based SoCs and is used to select between two
> modes for the external interrupt pins IRQ0 to IRQ3:
> 
> IRLM = 0: (default from reset on r8a7779)
> In this mode the pins IRQ0 to IRQ3 are used together
> to give a value between 0 and 15 to the SoC. External
> logic is required for masking. This mode is not
> supported by the INTC-IRQPIN driver.
> 
> IRLM = 1: (needs this patch or configuration elsewhere)
> In this mode IRQ0 to IRQ3 operate as 4 individual
> external interrupt pins. In this mode the SMSC ethernet
> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> is the only supported mode by the INTC-IRQPIN driver.
> 
> For this patch to work the r8a7779 DTS needs to pass
> the ICR0 register as the last register bank.
> 
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Same version as sent on December 3rd.
> 
>  Written against renesas-devel-20141202-v3.18-rc7 which is
>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
>  
>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt |    5 +
>  drivers/irqchip/irq-renesas-intc-irqpin.c                                      |   50 ++++++++--
>  2 files changed, 46 insertions(+), 9 deletions(-)
> 
> --- 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
> +++ work/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt	2014-12-03 20:25:13.000000000 +0900
> @@ -9,6 +9,11 @@ Required properties:
>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> +
> +- 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-cells: has to be <2>: an interrupt index and flags, as defined in
>    interrupts.txt in this directory
>  
> --- 0001/drivers/irqchip/irq-renesas-intc-irqpin.c
> +++ work/drivers/irqchip/irq-renesas-intc-irqpin.c	2014-12-03 20:32:59.000000000 +0900
> @@ -30,6 +30,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_data/irq-renesas-intc-irqpin.h>
>  #include <linux/pm_runtime.h>
>  
> @@ -40,7 +41,9 @@
>  #define INTC_IRQPIN_REG_SOURCE 2 /* INTREQnn */
>  #define INTC_IRQPIN_REG_MASK 3 /* INTMSKnn */
>  #define INTC_IRQPIN_REG_CLEAR 4 /* INTMSKCLRnn */
> -#define INTC_IRQPIN_REG_NR 5
> +#define INTC_IRQPIN_REG_NR_MANDATORY 5
> +#define INTC_IRQPIN_REG_IRLM 5 /* ICR0 with IRLM bit (optional) */
> +#define INTC_IRQPIN_REG_NR 6
>  
>  /* INTC external IRQ PIN hardware register access:
>   *
> @@ -82,6 +85,10 @@ struct intc_irqpin_priv {
>  	u8 shared_irq_mask;
>  };
>  
> +struct intc_irqpin_irlm_config {
> +	unsigned int irlm_bit;
> +};
> +
>  static unsigned long intc_irqpin_read32(void __iomem *iomem)
>  {
>  	return ioread32(iomem);
> @@ -345,10 +352,23 @@ static struct irq_domain_ops intc_irqpin
>  	.xlate  = irq_domain_xlate_twocell,
>  };
>  
> +static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a7779 = {
> +	.irlm_bit = 23, /* ICR0.IRLM0 */
> +};
> +
> +static const struct of_device_id intc_irqpin_dt_ids[] = {
> +	{ .compatible = "renesas,intc-irqpin", },
> +	{ .compatible = "renesas,intc-irqpin-r8a7779",
> +	  .data = &intc_irqpin_irlm_r8a7779 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
> +
>  static int intc_irqpin_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct renesas_intc_irqpin_config *pdata = dev->platform_data;
> +	const struct of_device_id *of_id;
>  	struct intc_irqpin_priv *p;
>  	struct intc_irqpin_iomem *i;
>  	struct resource *io[INTC_IRQPIN_REG_NR];
> @@ -391,10 +411,11 @@ static int intc_irqpin_probe(struct plat
>  	pm_runtime_enable(dev);
>  	pm_runtime_get_sync(dev);
>  
> -	/* get hold of manadatory IOMEM */
> +	/* get hold of register banks */
> +	memset(io, 0, sizeof(io));
>  	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
>  		io[k] = platform_get_resource(pdev, IORESOURCE_MEM, k);
> -		if (!io[k]) {
> +		if (!io[k] && k < INTC_IRQPIN_REG_NR_MANDATORY) {
>  			dev_err(dev, "not enough IOMEM resources\n");
>  			ret = -EINVAL;
>  			goto err0;
> @@ -422,6 +443,10 @@ static int intc_irqpin_probe(struct plat
>  	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
>  		i = &p->iomem[k];
>  
> +		/* handle optional registers */
> +		if (!io[k])
> +			continue;
> +
>  		switch (resource_size(io[k])) {
>  		case 1:
>  			i->width = 8;
> @@ -448,6 +473,19 @@ static int intc_irqpin_probe(struct plat
>  		}
>  	}
>  
> +	/* configure "individual IRQ mode" where needed */
> +	of_id = of_match_device(intc_irqpin_dt_ids, dev);
> +	if (of_id && of_id->data) {
> +		const struct intc_irqpin_irlm_config *irlm_config = of_id->data;
> +
> +		if (io[INTC_IRQPIN_REG_IRLM])
> +			intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_IRLM,
> +						      irlm_config->irlm_bit,
> +						      1, 1);
> +		else
> +			dev_warn(dev, "unable to select IRLM mode\n");

What was the default behavior of the arch code before the DT binding
change?  Should that action be performed here?

thx,

Jason.

> +	}
> +
>  	/* mask all interrupts using priority */
>  	for (k = 0; k < p->number_of_irqs; k++)
>  		intc_irqpin_mask_unmask_prio(p, k, 1);
> @@ -550,12 +588,6 @@ static int intc_irqpin_remove(struct pla
>  	return 0;
>  }
>  
> -static const struct of_device_id intc_irqpin_dt_ids[] = {
> -	{ .compatible = "renesas,intc-irqpin", },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
> -
>  static struct platform_driver intc_irqpin_device_driver = {
>  	.probe		= intc_irqpin_probe,
>  	.remove		= intc_irqpin_remove,

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Laurent Pinchart]

Hi Jason,

On Wednesday 07 January 2015 12:15:45 Jason Cooper wrote:
> On Thu, Dec 18, 2014 at 04:39:51AM +0200, Laurent Pinchart wrote:
> > On Thursday 18 December 2014 10:26:27 Magnus Damm wrote:
> >> On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart wrote:
> >>> On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
> >>>> From: Magnus Damm <damm+renesas@opensource.se>
> >>>> 
> >>>> Add r8a7779 specific support for IRLM bit configuration
> >>>> in the INTC-IRQPIN driver. Without this code we need
> >>>> special workaround code in arch/arm/mach-shmobile.
> >>>> 
> >>>> The IRLM bit for the INTC hardware exists on various
> >>>> older SH-based SoCs and is used to select between two
> >>>> modes for the external interrupt pins IRQ0 to IRQ3:
> >>>> 
> >>>> IRLM = 0: (default from reset on r8a7779)
> >>>> In this mode the pins IRQ0 to IRQ3 are used together
> >>>> to give a value between 0 and 15 to the SoC. External
> >>>> logic is required for masking. This mode is not
> >>>> supported by the INTC-IRQPIN driver.
> >>>> 
> >>>> IRLM = 1: (needs this patch or configuration elsewhere)
> >>>> In this mode IRQ0 to IRQ3 operate as 4 individual
> >>>> external interrupt pins. In this mode the SMSC ethernet
> >>>> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> >>>> is the only supported mode by the INTC-IRQPIN driver.
> >>>> 
> >>>> For this patch to work the r8a7779 DTS needs to pass
> >>>> the ICR0 register as the last register bank.
> >>>> 
> >>>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >>>> ---
> >>>> 
> >>>>  Same version as sent on December 3rd.
> >>>>  
> >>>>  Written against renesas-devel-20141202-v3.18-rc7 which is
> >>>>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
> >>>>  
> >>>>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> >>>>  irqpin.txt |    5 +
> >>>>  drivers/irqchip/irq-renesas-intc-irqpi            |   50 ++++++++--
> >>>>  2 files changed, 46 insertions(+), 9 deletions(-)
> >>>> 
> >>>> ---
> >>>> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,i
> >>>> ntc-irqpin.txt +++
> >>>> work/Documentation/devicetree/bindings/interrupt-controller/renesas,i
> >>>> ntc-irqpin.txt      2014-12-03 20:25:13.000000000 +0900
> >>>> @@ -9,6 +9,11 @@
> >>>> Required properties:
> >>>>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
> >>>>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
> >>>>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> >>>> +
> >>>> +- 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.
> >>> 
> >>> Ouch. Ouch. Multiple times. Very much :-/
> >>> 
> >>> I suppose it's too late to fix the DT bindings ?
> 
> Fix?  Is this a change to the reg property (from an undocumented
> version)?  Or just adding the reg property?  If the later, then as long
> as the driver preserves the old behavior when the reg property isn't
> present, we should be good.
> 
> If the case was the former, then we need to keep in mind that these are
> old SoCs and that this binding is mfgr-specific.  afair from the DT
> maintainers, it's acceptable to have a little instability under these
> conditions.

The reg property is already used, and this patch documents it. My concern is 
that the property is (at least in my opinion) used in a pretty dirty way.

In a nutshell, the INTC IP core handles several banks of interrupts, with a 
set of registers per bank. The registers are interleaved instead of split per 
bank. However, the driver expects one device instance per bank, with one 
memory resource for each register. This predates DT support and has never been 
reworked. I would have liked to instantiate a single INTC device given that we 
have a single INTC IP core, but the change would likely be too disruptive.

> >> I don't think it is ever too late, but the question is just how urgent
> >> it is to get rid of the Marzen C board code. With the code in this
> >> patch queue up we can get rid of the board code. If someone decides to
> >> rework the INTC IRQPIN bits then we will have to live with the Marzen
> >> code for longer. And if we should do it right and describe the actual
> >> hardware, then perhaps we should consider the non-IRQ-pin interrupts
> >> included in INTC for some SoCs. So it rapidly gets more complex...
> > 
> > I don't think there's a need to delay getting rid of Marzen legacy code,
> > my comment wasn't an attempt to delay your efforts :-)
> > 
> > Looking at the hardware we should have a single INTC DT node, not four.
> > What I'm wondering is whether we could still fix the DT bindings, or if
> > the backward compatibility requirements would make it too painful,
> > especially given that R-Car Gen2 doesn't use INTC.
> 
> Please see above.  I'm sure the DT maintainers will chirp up if the
> winds have shifted, but I'd rather we do it right if we're breaking
> compatibility anyway.

We're not breaking compatibility with this patch. The question is whether we 
should break compatibility to do things right.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Laurent Pinchart]

Hi Jason,

On Wednesday 07 January 2015 12:15:45 Jason Cooper wrote:
> On Thu, Dec 18, 2014 at 04:39:51AM +0200, Laurent Pinchart wrote:
> > On Thursday 18 December 2014 10:26:27 Magnus Damm wrote:
> >> On Thu, Dec 18, 2014 at 6:41 AM, Laurent Pinchart wrote:
> >>> On Monday 15 December 2014 14:09:20 Magnus Damm wrote:
> >>>> From: Magnus Damm <damm+renesas@opensource.se>
> >>>> 
> >>>> Add r8a7779 specific support for IRLM bit configuration
> >>>> in the INTC-IRQPIN driver. Without this code we need
> >>>> special workaround code in arch/arm/mach-shmobile.
> >>>> 
> >>>> The IRLM bit for the INTC hardware exists on various
> >>>> older SH-based SoCs and is used to select between two
> >>>> modes for the external interrupt pins IRQ0 to IRQ3:
> >>>> 
> >>>> IRLM = 0: (default from reset on r8a7779)
> >>>> In this mode the pins IRQ0 to IRQ3 are used together
> >>>> to give a value between 0 and 15 to the SoC. External
> >>>> logic is required for masking. This mode is not
> >>>> supported by the INTC-IRQPIN driver.
> >>>> 
> >>>> IRLM = 1: (needs this patch or configuration elsewhere)
> >>>> In this mode IRQ0 to IRQ3 operate as 4 individual
> >>>> external interrupt pins. In this mode the SMSC ethernet
> >>>> chip can be used via IRQ1 on r8a7779 Marzen. This mode
> >>>> is the only supported mode by the INTC-IRQPIN driver.
> >>>> 
> >>>> For this patch to work the r8a7779 DTS needs to pass
> >>>> the ICR0 register as the last register bank.
> >>>> 
> >>>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >>>> ---
> >>>> 
> >>>>  Same version as sent on December 3rd.
> >>>>  
> >>>>  Written against renesas-devel-20141202-v3.18-rc7 which is
> >>>>  basically v3.18-rc7 plus latest arch/arm/mach-shmobile code.
> >>>>  
> >>>>  Documentation/devicetree/bindings/interrupt-controller/renesas,intc-
> >>>>  irqpin.txt |    5 +
> >>>>  drivers/irqchip/irq-renesas-intc-irqpi            |   50 ++++++++--
> >>>>  2 files changed, 46 insertions(+), 9 deletions(-)
> >>>> 
> >>>> ---
> >>>> 0001/Documentation/devicetree/bindings/interrupt-controller/renesas,i
> >>>> ntc-irqpin.txt +++
> >>>> work/Documentation/devicetree/bindings/interrupt-controller/renesas,i
> >>>> ntc-irqpin.txt      2014-12-03 20:25:13.000000000 +0900
> >>>> @@ -9,6 +9,11 @@
> >>>> Required properties:
> >>>>      - "renesas,intc-irqpin-r8a7778" (R-Car M1A)
> >>>>      - "renesas,intc-irqpin-r8a7779" (R-Car H1)
> >>>>      - "renesas,intc-irqpin-sh73a0" (SH-Mobile AG5)
> >>>> +
> >>>> +- 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.
> >>> 
> >>> Ouch. Ouch. Multiple times. Very much :-/
> >>> 
> >>> I suppose it's too late to fix the DT bindings ?
> 
> Fix?  Is this a change to the reg property (from an undocumented
> version)?  Or just adding the reg property?  If the later, then as long
> as the driver preserves the old behavior when the reg property isn't
> present, we should be good.
> 
> If the case was the former, then we need to keep in mind that these are
> old SoCs and that this binding is mfgr-specific.  afair from the DT
> maintainers, it's acceptable to have a little instability under these
> conditions.

The reg property is already used, and this patch documents it. My concern is 
that the property is (at least in my opinion) used in a pretty dirty way.

In a nutshell, the INTC IP core handles several banks of interrupts, with a 
set of registers per bank. The registers are interleaved instead of split per 
bank. However, the driver expects one device instance per bank, with one 
memory resource for each register. This predates DT support and has never been 
reworked. I would have liked to instantiate a single INTC device given that we 
have a single INTC IP core, but the change would likely be too disruptive.

> >> I don't think it is ever too late, but the question is just how urgent
> >> it is to get rid of the Marzen C board code. With the code in this
> >> patch queue up we can get rid of the board code. If someone decides to
> >> rework the INTC IRQPIN bits then we will have to live with the Marzen
> >> code for longer. And if we should do it right and describe the actual
> >> hardware, then perhaps we should consider the non-IRQ-pin interrupts
> >> included in INTC for some SoCs. So it rapidly gets more complex...
> > 
> > I don't think there's a need to delay getting rid of Marzen legacy code,
> > my comment wasn't an attempt to delay your efforts :-)
> > 
> > Looking at the hardware we should have a single INTC DT node, not four.
> > What I'm wondering is whether we could still fix the DT bindings, or if
> > the backward compatibility requirements would make it too painful,
> > especially given that R-Car Gen2 doesn't use INTC.
> 
> Please see above.  I'm sure the DT maintainers will chirp up if the
> winds have shifted, but I'd rather we do it right if we're breaking
> compatibility anyway.

We're not breaking compatibility with this patch. The question is whether we 
should break compatibility to do things right.

-- 
Regards,

Laurent Pinchart

[PATCH] irqchip: renesas-intc-irqpin: r8a7778 IRLM setup support

[Geert Uytterhoeven]

From: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Works the same as on r8a7779.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/irqchip/irq-renesas-intc-irqpin.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c
index 1b4e5cc9f7df9a0c..7f6cf19aa6ac00d7 100644
--- a/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
@@ -359,14 +359,16 @@ static const struct irq_domain_ops intc_irqpin_irq_domain_ops = {
 	.xlate  = irq_domain_xlate_twocell,
 };
 
-static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a7779 = {
+static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a777x = {
 	.irlm_bit = 23, /* ICR0.IRLM0 */
 };
 
 static const struct of_device_id intc_irqpin_dt_ids[] = {
 	{ .compatible = "renesas,intc-irqpin", },
+	{ .compatible = "renesas,intc-irqpin-r8a7778",
+	  .data = &intc_irqpin_irlm_r8a777x },
 	{ .compatible = "renesas,intc-irqpin-r8a7779",
-	  .data = &intc_irqpin_irlm_r8a7779 },
+	  .data = &intc_irqpin_irlm_r8a777x },
 	{},
 };
 MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
-- 
1.9.1

[PATCH] irqchip: renesas-intc-irqpin: r8a7778 IRLM setup support

[Geert Uytterhoeven]

From: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>

Works the same as on r8a7779.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/irqchip/irq-renesas-intc-irqpin.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c
index 1b4e5cc9f7df9a0c..7f6cf19aa6ac00d7 100644
--- a/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
@@ -359,14 +359,16 @@ static const struct irq_domain_ops intc_irqpin_irq_domain_ops = {
 	.xlate  = irq_domain_xlate_twocell,
 };
 
-static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a7779 = {
+static const struct intc_irqpin_irlm_config intc_irqpin_irlm_r8a777x = {
 	.irlm_bit = 23, /* ICR0.IRLM0 */
 };
 
 static const struct of_device_id intc_irqpin_dt_ids[] = {
 	{ .compatible = "renesas,intc-irqpin", },
+	{ .compatible = "renesas,intc-irqpin-r8a7778",
+	  .data = &intc_irqpin_irlm_r8a777x },
 	{ .compatible = "renesas,intc-irqpin-r8a7779",
-	  .data = &intc_irqpin_irlm_r8a7779 },
+	  .data = &intc_irqpin_irlm_r8a777x },
 	{},
 };
 MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
-- 
1.9.1

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7778 IRLM setup support

[Simon Horman]

On Wed, Sep 30, 2015 at 12:03:07PM +0200, Geert Uytterhoeven wrote:
> From: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> 
> Works the same as on r8a7779.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH] irqchip: renesas-intc-irqpin: r8a7778 IRLM setup support

[Simon Horman]

On Wed, Sep 30, 2015 at 12:03:07PM +0200, Geert Uytterhoeven wrote:
> From: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> 
> Works the same as on r8a7779.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>