[PATCH 00/02] r8a7779 renesas-intc-irqpin IRLM configuration patches

[PATCH 00/02] r8a7779 renesas-intc-irqpin IRLM configuration patches

[Magnus Damm]

r8a7779 renesas-intc-irqpin IRLM configuration patches

[PATCH 01/02] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support
[PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround

These patches extend the INTC-IRQPIN driver with r8a7779 specific
register support code that configures "Individual IRQ mode" from
the driver instead of workaround in arch/arm/mach-shmobile.

With this in place we are one step closer to get rid of C board
code for the r8a7779 SoC.

There are no build dependencies between the two patches, however
for correct runtime operation patch 1 needs to be applied before
patch 2. I suggest merging patch 1 through the IRQCHIP tree and
adding patch 1 to mach-shmobile when the first patch hits -rc1.

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

 Built on top of renesas-devel-20141202-v3.18-rc7 and
 "[PATCH] ARM: shmobile: r8a7779 CCF DTS update"

 Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt |    5 +
 arch/arm/boot/dts/r8a7779.dtsi                                                 |    5 -
 arch/arm/mach-shmobile/board-marzen-reference.c                                |    7 -
 drivers/irqchip/irq-renesas-intc-irqpin.c                                      |   50 ++++++++--
 4 files changed, 49 insertions(+), 18 deletions(-)

[PATCH 01/02] 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>
---

 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 01/02] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[Simon Horman]

Hi Magnus,

I see you have been busy with the marzen board.

On Wed, Dec 03, 2014 at 09:18:03PM +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>
> ---
> 
>  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,

I am unclear about the relationship between this last hunk and the rest of
the patch. It seems to be removing the only compat string that is
recognised by the driver.

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

[Magnus Damm]

Hi Simon,

On Thu, Dec 4, 2014 at 4:18 PM, Simon Horman <horms@verge.net.au> wrote:
> Hi Magnus,
>
> I see you have been busy with the marzen board.

Yes! =)

> On Wed, Dec 03, 2014 at 09:18:03PM +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>
>> ---
>>
>>  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,
>
> I am unclear about the relationship between this last hunk and the rest of
> the patch. It seems to be removing the only compat string that is
> recognised by the driver.

The MODULE_DEVICE_TABLE() bits are moved up before the probe()
function so it can be used together with of_match_device() to
determine if the r8a7779 specific setting should be applied or not. So
the last hunk is intentional and needed. The original compat string is
still there.

Thanks,

/ magnus

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

[Simon Horman]

On Thu, Dec 04, 2014 at 04:29:49PM +0900, Magnus Damm wrote:
> Hi Simon,
> 
> On Thu, Dec 4, 2014 at 4:18 PM, Simon Horman <horms@verge.net.au> wrote:
> > Hi Magnus,
> >
> > I see you have been busy with the marzen board.
> 
> Yes! =)
> 
> > On Wed, Dec 03, 2014 at 09:18:03PM +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>
> >> ---
> >>
> >>  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,
> >
> > I am unclear about the relationship between this last hunk and the rest of
> > the patch. It seems to be removing the only compat string that is
> > recognised by the driver.
> 
> The MODULE_DEVICE_TABLE() bits are moved up before the probe()
> function so it can be used together with of_match_device() to
> determine if the r8a7779 specific setting should be applied or not. So
> the last hunk is intentional and needed. The original compat string is
> still there.

Thanks, I see it now.

[tip:irq/core] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

[tip-bot for Magnus Damm]

Commit-ID:  e03f9088e22ca7e2b0de826466540e2527518e52
Gitweb:     http://git.kernel.org/tip/e03f9088e22ca7e2b0de826466540e2527518e52
Author:     Magnus Damm <damm+renesas@opensource.se>
AuthorDate: Wed, 3 Dec 2014 21:18:03 +0900
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 26 Jan 2015 11:38:22 +0100

irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

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>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: horms@verge.net.au
Cc: jason@lakedaemon.net
Link: http://lkml.kernel.org/r/20141203121803.5936.35881.sendpatchset@w520
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 .../interrupt-controller/renesas,intc-irqpin.txt   |  5 +++
 drivers/irqchip/irq-renesas-intc-irqpin.c          | 50 ++++++++++++++++++----
 2 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
index c73acd0..4f7946a 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
@@ -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
 
diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c b/drivers/irqchip/irq-renesas-intc-irqpin.c
index 078cac5..9a0767b 100644
--- a/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
@@ -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_irq_domain_ops = {
 	.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 platform_device *pdev)
 	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 platform_device *pdev)
 	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 platform_device *pdev)
 		}
 	}
 
+	/* 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 platform_device *pdev)
 	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 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround

[Magnus Damm]

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

Adjust the r8a7779 SoC DTS and the Marzen Reference
C board code to use DTS only for INTC-IRQPIN IRLM setup.

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

 Written on top of renesas-devel-20141202-v3.18-rc7 and
 [PATCH] ARM: shmobile: r8a7779 CCF DTS update

 Has a runtime dependency on:
 [PATCH 01/02] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support

 arch/arm/boot/dts/r8a7779.dtsi                  |    5 +++--
 arch/arm/mach-shmobile/board-marzen-reference.c |    7 -------
 2 files changed, 3 insertions(+), 9 deletions(-)

--- 0002/arch/arm/boot/dts/r8a7779.dtsi
+++ work/arch/arm/boot/dts/r8a7779.dtsi	2014-12-03 20:27:49.000000000 +0900
@@ -139,7 +139,7 @@
 		interrupt-controller;
 	};
 
-	irqpin0: irqpin@fe780010 {
+	irqpin0: irqpin@fe780000 {
 		compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin";
 		#interrupt-cells = <2>;
 		status = "disabled";
@@ -148,7 +148,8 @@
 			<0xfe780010 4>,
 			<0xfe780024 4>,
 			<0xfe780044 4>,
-			<0xfe780064 4>;
+			<0xfe780064 4>,
+			<0xfe780000 4>;
 		interrupts = <0 27 IRQ_TYPE_LEVEL_HIGH
 			      0 28 IRQ_TYPE_LEVEL_HIGH
 			      0 29 IRQ_TYPE_LEVEL_HIGH
--- 0001/arch/arm/mach-shmobile/board-marzen-reference.c
+++ work/arch/arm/mach-shmobile/board-marzen-reference.c	2014-12-03 20:28:37.000000000 +0900
@@ -32,12 +32,6 @@ static void __init marzen_init_timer(voi
 	clocksource_of_init();
 }
 
-static void __init marzen_init(void)
-{
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
-	r8a7779_init_irq_extpin_dt(1); /* IRQ1 as individual interrupt */
-}
-
 static const char *marzen_boards_compat_dt[] __initdata = {
 	"renesas,marzen",
 	"renesas,marzen-reference",
@@ -50,7 +44,6 @@ DT_MACHINE_START(MARZEN, "marzen")
 	.init_early	= shmobile_init_delay,
 	.init_time	= marzen_init_timer,
 	.init_irq	= r8a7779_init_irq_dt,
-	.init_machine	= marzen_init,
 	.init_late	= shmobile_init_late,
 	.dt_compat	= marzen_boards_compat_dt,
 MACHINE_END

Re: [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround

[Simon Horman]

Hi Magnus,

On Wed, Dec 03, 2014 at 09:18:13PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Adjust the r8a7779 SoC DTS and the Marzen Reference
> C board code to use DTS only for INTC-IRQPIN IRLM setup.
> 
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Written on top of renesas-devel-20141202-v3.18-rc7 and
>  [PATCH] ARM: shmobile: r8a7779 CCF DTS update
> 
>  Has a runtime dependency on:
>  [PATCH 01/02] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support
> 
>  arch/arm/boot/dts/r8a7779.dtsi                  |    5 +++--
>  arch/arm/mach-shmobile/board-marzen-reference.c |    7 -------
>  2 files changed, 3 insertions(+), 9 deletions(-)
> 
> --- 0002/arch/arm/boot/dts/r8a7779.dtsi
> +++ work/arch/arm/boot/dts/r8a7779.dtsi	2014-12-03 20:27:49.000000000 +0900
> @@ -139,7 +139,7 @@
>  		interrupt-controller;
>  	};
>  
> -	irqpin0: irqpin@fe780010 {
> +	irqpin0: irqpin@fe780000 {
>  		compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin";
>  		#interrupt-cells = <2>;
>  		status = "disabled";
> @@ -148,7 +148,8 @@
>  			<0xfe780010 4>,
>  			<0xfe780024 4>,
>  			<0xfe780044 4>,
> -			<0xfe780064 4>;
> +			<0xfe780064 4>,
> +			<0xfe780000 4>;

Is there any order implied by the above list?
Naïvely I would expect it to be sorted numerically.

>  		interrupts = <0 27 IRQ_TYPE_LEVEL_HIGH
>  			      0 28 IRQ_TYPE_LEVEL_HIGH
>  			      0 29 IRQ_TYPE_LEVEL_HIGH
> --- 0001/arch/arm/mach-shmobile/board-marzen-reference.c
> +++ work/arch/arm/mach-shmobile/board-marzen-reference.c	2014-12-03 20:28:37.000000000 +0900
> @@ -32,12 +32,6 @@ static void __init marzen_init_timer(voi
>  	clocksource_of_init();
>  }
>  
> -static void __init marzen_init(void)
> -{
> -	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> -	r8a7779_init_irq_extpin_dt(1); /* IRQ1 as individual interrupt */
> -}
> -
>  static const char *marzen_boards_compat_dt[] __initdata = {
>  	"renesas,marzen",
>  	"renesas,marzen-reference",
> @@ -50,7 +44,6 @@ DT_MACHINE_START(MARZEN, "marzen")
>  	.init_early	= shmobile_init_delay,
>  	.init_time	= marzen_init_timer,
>  	.init_irq	= r8a7779_init_irq_dt,
> -	.init_machine	= marzen_init,
>  	.init_late	= shmobile_init_late,
>  	.dt_compat	= marzen_boards_compat_dt,
>  MACHINE_END
> 

Re: [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround

[Magnus Damm]

Hi Simon,

On Thu, Dec 4, 2014 at 4:21 PM, Simon Horman <horms@verge.net.au> wrote:
> Hi Magnus,
>
> On Wed, Dec 03, 2014 at 09:18:13PM +0900, Magnus Damm wrote:
>> From: Magnus Damm <damm+renesas@opensource.se>
>>
>> Adjust the r8a7779 SoC DTS and the Marzen Reference
>> C board code to use DTS only for INTC-IRQPIN IRLM setup.
>>
>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> ---
>>
>>  Written on top of renesas-devel-20141202-v3.18-rc7 and
>>  [PATCH] ARM: shmobile: r8a7779 CCF DTS update
>>
>>  Has a runtime dependency on:
>>  [PATCH 01/02] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support
>>
>>  arch/arm/boot/dts/r8a7779.dtsi                  |    5 +++--
>>  arch/arm/mach-shmobile/board-marzen-reference.c |    7 -------
>>  2 files changed, 3 insertions(+), 9 deletions(-)
>>
>> --- 0002/arch/arm/boot/dts/r8a7779.dtsi
>> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2014-12-03 20:27:49.000000000 +0900
>> @@ -139,7 +139,7 @@
>>               interrupt-controller;
>>       };
>>
>> -     irqpin0: irqpin@fe780010 {
>> +     irqpin0: irqpin@fe780000 {
>>               compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin";
>>               #interrupt-cells = <2>;
>>               status = "disabled";
>> @@ -148,7 +148,8 @@
>>                       <0xfe780010 4>,
>>                       <0xfe780024 4>,
>>                       <0xfe780044 4>,
>> -                     <0xfe780064 4>;
>> +                     <0xfe780064 4>,
>> +                     <0xfe780000 4>;
>
> Is there any order implied by the above list?
> Naïvely I would expect it to be sorted numerically.

Yes, the driver assumes the register banks to be passed in a certain
order. In the case of r8a7779 we add one more register bank at the end
for IRLM setup. Register detail (base address, access size, order and
bitfield width) varies with SoC version. So the IRLM register will be
at different addresses depending on SoC, but the driver wants it at
the end of the list.

Cheers,

/ magnus

Re: [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround

[Simon Horman]

On Thu, Dec 04, 2014 at 04:33:25PM +0900, Magnus Damm wrote:
> Hi Simon,
> 
> On Thu, Dec 4, 2014 at 4:21 PM, Simon Horman <horms@verge.net.au> wrote:
> > Hi Magnus,
> >
> > On Wed, Dec 03, 2014 at 09:18:13PM +0900, Magnus Damm wrote:
> >> From: Magnus Damm <damm+renesas@opensource.se>
> >>
> >> Adjust the r8a7779 SoC DTS and the Marzen Reference
> >> C board code to use DTS only for INTC-IRQPIN IRLM setup.
> >>
> >> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> >> ---
> >>
> >>  Written on top of renesas-devel-20141202-v3.18-rc7 and
> >>  [PATCH] ARM: shmobile: r8a7779 CCF DTS update
> >>
> >>  Has a runtime dependency on:
> >>  [PATCH 01/02] irqchip: renesas-intc-irqpin: r8a7779 IRLM setup support
> >>
> >>  arch/arm/boot/dts/r8a7779.dtsi                  |    5 +++--
> >>  arch/arm/mach-shmobile/board-marzen-reference.c |    7 -------
> >>  2 files changed, 3 insertions(+), 9 deletions(-)
> >>
> >> --- 0002/arch/arm/boot/dts/r8a7779.dtsi
> >> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2014-12-03 20:27:49.000000000 +0900
> >> @@ -139,7 +139,7 @@
> >>               interrupt-controller;
> >>       };
> >>
> >> -     irqpin0: irqpin@fe780010 {
> >> +     irqpin0: irqpin@fe780000 {
> >>               compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin";
> >>               #interrupt-cells = <2>;
> >>               status = "disabled";
> >> @@ -148,7 +148,8 @@
> >>                       <0xfe780010 4>,
> >>                       <0xfe780024 4>,
> >>                       <0xfe780044 4>,
> >> -                     <0xfe780064 4>;
> >> +                     <0xfe780064 4>,
> >> +                     <0xfe780000 4>;
> >
> > Is there any order implied by the above list?
> > Naïvely I would expect it to be sorted numerically.
> 
> Yes, the driver assumes the register banks to be passed in a certain
> order. In the case of r8a7779 we add one more register bank at the end
> for IRLM setup. Register detail (base address, access size, order and
> bitfield width) varies with SoC version. So the IRLM register will be
> at different addresses depending on SoC, but the driver wants it at
> the end of the list.

Thanks, if it is intentional then that is fine by me.

Re: [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround

[Geert Uytterhoeven]

Hi Magnus,

On Thu, Dec 4, 2014 at 8:33 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> --- 0002/arch/arm/boot/dts/r8a7779.dtsi
>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2014-12-03 20:27:49.000000000 +0900
>>> @@ -139,7 +139,7 @@
>>>               interrupt-controller;
>>>       };
>>>
>>> -     irqpin0: irqpin@fe780010 {
>>> +     irqpin0: irqpin@fe780000 {
>>>               compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin";
>>>               #interrupt-cells = <2>;
>>>               status = "disabled";
>>> @@ -148,7 +148,8 @@
>>>                       <0xfe780010 4>,
>>>                       <0xfe780024 4>,
>>>                       <0xfe780044 4>,
>>> -                     <0xfe780064 4>;
>>> +                     <0xfe780064 4>,
>>> +                     <0xfe780000 4>;
>>
>> Is there any order implied by the above list?
>> Naïvely I would expect it to be sorted numerically.
>
> Yes, the driver assumes the register banks to be passed in a certain
> order. In the case of r8a7779 we add one more register bank at the end
> for IRLM setup. Register detail (base address, access size, order and
> bitfield width) varies with SoC version. So the IRLM register will be
> at different addresses depending on SoC, but the driver wants it at
> the end of the list.

As these are all individual registers, and there are that many, I think
it's worth adding a reg-names property to identify the registers.
Of course the driver still has to support the old anonymous order
for backwards compatibility.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround

[Magnus Damm]

Hi Geert,

On Thu, Dec 4, 2014 at 6:19 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Thu, Dec 4, 2014 at 8:33 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>> --- 0002/arch/arm/boot/dts/r8a7779.dtsi
>>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2014-12-03 20:27:49.000000000 +0900
>>>> @@ -139,7 +139,7 @@
>>>>               interrupt-controller;
>>>>       };
>>>>
>>>> -     irqpin0: irqpin@fe780010 {
>>>> +     irqpin0: irqpin@fe780000 {
>>>>               compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin";
>>>>               #interrupt-cells = <2>;
>>>>               status = "disabled";
>>>> @@ -148,7 +148,8 @@
>>>>                       <0xfe780010 4>,
>>>>                       <0xfe780024 4>,
>>>>                       <0xfe780044 4>,
>>>> -                     <0xfe780064 4>;
>>>> +                     <0xfe780064 4>,
>>>> +                     <0xfe780000 4>;
>>>
>>> Is there any order implied by the above list?
>>> Naïvely I would expect it to be sorted numerically.
>>
>> Yes, the driver assumes the register banks to be passed in a certain
>> order. In the case of r8a7779 we add one more register bank at the end
>> for IRLM setup. Register detail (base address, access size, order and
>> bitfield width) varies with SoC version. So the IRLM register will be
>> at different addresses depending on SoC, but the driver wants it at
>> the end of the list.
>
> As these are all individual registers, and there are that many, I think
> it's worth adding a reg-names property to identify the registers.
> Of course the driver still has to support the old anonymous order
> for backwards compatibility.

If we should rework things, then I propose going the other way around.
=) Basically only passing a single base address with a certain SoC
specific compat string, and based on that letting the driver
internally figure out which register is at what offset and the access
size and bitfield size. Either way we have a limited number of SoCs
and they are all old.

Cheers,

/ magnus

Re: [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround

[Geert Uytterhoeven]

Hi Magnus,

On Thu, Dec 4, 2014 at 10:24 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On Thu, Dec 4, 2014 at 6:19 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Thu, Dec 4, 2014 at 8:33 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>>> --- 0002/arch/arm/boot/dts/r8a7779.dtsi
>>>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2014-12-03 20:27:49.000000000 +0900
>>>>> @@ -139,7 +139,7 @@
>>>>>               interrupt-controller;
>>>>>       };
>>>>>
>>>>> -     irqpin0: irqpin@fe780010 {
>>>>> +     irqpin0: irqpin@fe780000 {
>>>>>               compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin";
>>>>>               #interrupt-cells = <2>;
>>>>>               status = "disabled";
>>>>> @@ -148,7 +148,8 @@
>>>>>                       <0xfe780010 4>,
>>>>>                       <0xfe780024 4>,
>>>>>                       <0xfe780044 4>,
>>>>> -                     <0xfe780064 4>;
>>>>> +                     <0xfe780064 4>,
>>>>> +                     <0xfe780000 4>;
>>>>
>>>> Is there any order implied by the above list?
>>>> Naïvely I would expect it to be sorted numerically.
>>>
>>> Yes, the driver assumes the register banks to be passed in a certain
>>> order. In the case of r8a7779 we add one more register bank at the end
>>> for IRLM setup. Register detail (base address, access size, order and
>>> bitfield width) varies with SoC version. So the IRLM register will be
>>> at different addresses depending on SoC, but the driver wants it at
>>> the end of the list.
>>
>> As these are all individual registers, and there are that many, I think
>> it's worth adding a reg-names property to identify the registers.
>> Of course the driver still has to support the old anonymous order
>> for backwards compatibility.
>
> If we should rework things, then I propose going the other way around.
> =) Basically only passing a single base address with a certain SoC
> specific compat string, and based on that letting the driver
> internally figure out which register is at what offset and the access
> size and bitfield size.

That's gonna mean a complete new compatible value.
Seems like we shouldn't have added "renesas,intc-irqpin-r8a7779",
as the SoC-type was encoded in the reg properties...

> Either way we have a limited number of SoCs and they are all old.

So your current patch looks like the best option for now
(can you promise future R-Car SoCs won't have an intc-irqpin hardware
 block ;-)?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 02/02] ARM: shmobile: marzen-reference: Remove IRLM workaround

[Magnus Damm]

Hi Geert,

On Thu, Dec 4, 2014 at 6:31 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Thu, Dec 4, 2014 at 10:24 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> On Thu, Dec 4, 2014 at 6:19 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> On Thu, Dec 4, 2014 at 8:33 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>>>> --- 0002/arch/arm/boot/dts/r8a7779.dtsi
>>>>>> +++ work/arch/arm/boot/dts/r8a7779.dtsi       2014-12-03 20:27:49.000000000 +0900
>>>>>> @@ -139,7 +139,7 @@
>>>>>>               interrupt-controller;
>>>>>>       };
>>>>>>
>>>>>> -     irqpin0: irqpin@fe780010 {
>>>>>> +     irqpin0: irqpin@fe780000 {
>>>>>>               compatible = "renesas,intc-irqpin-r8a7779", "renesas,intc-irqpin";
>>>>>>               #interrupt-cells = <2>;
>>>>>>               status = "disabled";
>>>>>> @@ -148,7 +148,8 @@
>>>>>>                       <0xfe780010 4>,
>>>>>>                       <0xfe780024 4>,
>>>>>>                       <0xfe780044 4>,
>>>>>> -                     <0xfe780064 4>;
>>>>>> +                     <0xfe780064 4>,
>>>>>> +                     <0xfe780000 4>;
>>>>>
>>>>> Is there any order implied by the above list?
>>>>> Naïvely I would expect it to be sorted numerically.
>>>>
>>>> Yes, the driver assumes the register banks to be passed in a certain
>>>> order. In the case of r8a7779 we add one more register bank at the end
>>>> for IRLM setup. Register detail (base address, access size, order and
>>>> bitfield width) varies with SoC version. So the IRLM register will be
>>>> at different addresses depending on SoC, but the driver wants it at
>>>> the end of the list.
>>>
>>> As these are all individual registers, and there are that many, I think
>>> it's worth adding a reg-names property to identify the registers.
>>> Of course the driver still has to support the old anonymous order
>>> for backwards compatibility.
>>
>> If we should rework things, then I propose going the other way around.
>> =) Basically only passing a single base address with a certain SoC
>> specific compat string, and based on that letting the driver
>> internally figure out which register is at what offset and the access
>> size and bitfield size.
>
> That's gonna mean a complete new compatible value.
> Seems like we shouldn't have added "renesas,intc-irqpin-r8a7779",
> as the SoC-type was encoded in the reg properties...

Yeah, having that SoC specific compat string in the DTS does not
exactly help us that this point. We could however use a slightly
different SoC compat string or something else that is unused if we
wanted to change things around.

>> Either way we have a limited number of SoCs and they are all old.
>
> So your current patch looks like the best option for now
> (can you promise future R-Car SoCs won't have an intc-irqpin hardware
>  block ;-)?

Hehe, almost. =)

I think I can promise that R-Car hardware won't use that hardware -
the IRQC hardware block replaced INTC-IRQPIN in R-Car Gen2 and it will
most likely not make a comeback. However, other SoC product lines may
show up with old interrupt controllers. But if so we can rework things
at that point and make use of a fresh compatible string for a clean
start.

Cheers,

/ magnus