Re: [PATCH 2/4] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts

2014-09-23 Thread Abhilash Kesavan 
Hi Tomasz,

On Mon, Sep 22, 2014 at 1:25 PM, Tomasz Figa  wrote:
> On 22.09.2014 08:17, Abhilash Kesavan wrote:
>> Hi Tomasz,
>>
>> On Sat, Sep 13, 2014 at 4:57 PM, Tomasz Figa  wrote:
>>> Hi Abhilash,
>>>
>>> Please see my comments inline.
>>>
>>> On 13.09.2014 10:50, Abhilash Kesavan wrote:
 Exynos7 uses different offsets for wakeup interrupt configuration 
 registers.
 So a new irq_chip instance for Exynos7 wakeup interrupts is added. The 
 irq_chip
 selection is now based on the wakeup interrupt controller compatible 
 string.
>>>
>>> [snip]
>>>
 @@ -328,9 +322,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data 
 *irqd, unsigned int on)
  /*
   * irq_chip for wakeup interrupts
   */
 -static struct exynos_irq_chip exynos_wkup_irq_chip = {
 +static struct exynos_irq_chip exynos_wkup_irq_chip;
 +
>>>
>>> Why do you still need this, if you have both variants below?
>>
>> After adding __initdata to the two variants, I will require to have a
>> copy of one of them.
>>
>>>
 +static struct exynos_irq_chip exynos4210_wkup_irq_chip = {
   .chip = {
 - .name = "exynos_wkup_irq_chip",
 + .name = "exynos4210_wkup_irq_chip",
   .irq_unmask = exynos_irq_unmask,
   .irq_mask = exynos_irq_mask,
   .irq_ack = exynos_irq_ack,
 @@ -342,6 +338,29 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = {
   .eint_pend = EXYNOS_WKUP_EPEND_OFFSET,
  };

 +static struct exynos_irq_chip exynos7_wkup_irq_chip = {
 + .chip = {
 + .name = "exynos7_wkup_irq_chip",
 + .irq_unmask = exynos_irq_unmask,
 + .irq_mask = exynos_irq_mask,
 + .irq_ack = exynos_irq_ack,
 + .irq_set_type = exynos_irq_set_type,
 + .irq_set_wake = exynos_wkup_irq_set_wake,
 + },
 + .eint_con = EXYNOS7_WKUP_ECON_OFFSET,
 + .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET,
 + .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET,
 +};
 +
 +/* list of external wakeup controllers supported */
 +static const struct of_device_id exynos_wkup_irq_ids[] = {
 + { .compatible = "samsung,exynos4210-wakeup-eint",
 + .data = &exynos4210_wkup_irq_chip },
 + { .compatible = "samsung,exynos7-wakeup-eint",
 + .data = &exynos7_wkup_irq_chip },
 + { }
 +};
 +
  /* interrupt handler for wakeup interrupts 0..15 */
  static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc)
  {
 @@ -434,7 +453,12 @@ static int exynos_eint_wkup_init(struct 
 samsung_pinctrl_drv_data *d)
   int idx, irq;

   for_each_child_of_node(dev->of_node, np) {
 - if (of_match_node(exynos_wkup_irq_ids, np)) {
 + const struct of_device_id *match;
 +
 + match = of_match_node(exynos_wkup_irq_ids, np);
 + if (match) {
 + memcpy(&exynos_wkup_irq_chip, match->data,
 + sizeof(struct exynos_irq_chip));
>>>
>>> Hmm, this doesn't look correct to me. You are modifying a static struct
>>> here. Why couldn't you simply use the exynos irq chip pointed by
>>> match->data in further registration code?
>>
>> That will not be available later once I use __initdata.
>>
>
> Then either __initdata shouldn't be necessary or kmemdup() should be
> used to allocate a copy.
Will fix this and send out a new version soon.

Regards,
Abhilash
>
>>>
   wkup_np = np;
   break;
   }
 diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h 
 b/drivers/pinctrl/samsung/pinctrl-exynos.h
 index e060722..0db1e52 100644
 --- a/drivers/pinctrl/samsung/pinctrl-exynos.h
 +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h
 @@ -25,6 +25,9 @@
  #define EXYNOS_WKUP_ECON_OFFSET  0xE00
  #define EXYNOS_WKUP_EMASK_OFFSET 0xF00
  #define EXYNOS_WKUP_EPEND_OFFSET 0xF40
 +#define EXYNOS7_WKUP_ECON_OFFSET 0x700
 +#define EXYNOS7_WKUP_EMASK_OFFSET0x900
 +#define EXYNOS7_WKUP_EPEND_OFFSET0xA00
>>>
>>> Interestingly enough, the offsets look just like the normal GPIO
>>> interrupt controller of previous Exynos SoCs. Are you sure those are
>>> correct? Also if somehow the controller now resembles the normal one,
>>> doesn't it have the SVC register making it possible to reuse the non
>>> wake-up code instead?
>>
>> The wakeup interrupt register offsets are the same as the GPIO
>> interrupt offsets in earlier Exynos SoCs. There is no SVC register for
>> the wakeup interrupt block.
>
> OK, your code is fine then.
>
> Best regards,
> Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordo

Re: [PATCH 2/4] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts

2014-09-22 Thread Tomasz Figa 
On 22.09.2014 08:17, Abhilash Kesavan wrote:
> Hi Tomasz,
> 
> On Sat, Sep 13, 2014 at 4:57 PM, Tomasz Figa  wrote:
>> Hi Abhilash,
>>
>> Please see my comments inline.
>>
>> On 13.09.2014 10:50, Abhilash Kesavan wrote:
>>> Exynos7 uses different offsets for wakeup interrupt configuration registers.
>>> So a new irq_chip instance for Exynos7 wakeup interrupts is added. The 
>>> irq_chip
>>> selection is now based on the wakeup interrupt controller compatible string.
>>
>> [snip]
>>
>>> @@ -328,9 +322,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data 
>>> *irqd, unsigned int on)
>>>  /*
>>>   * irq_chip for wakeup interrupts
>>>   */
>>> -static struct exynos_irq_chip exynos_wkup_irq_chip = {
>>> +static struct exynos_irq_chip exynos_wkup_irq_chip;
>>> +
>>
>> Why do you still need this, if you have both variants below?
> 
> After adding __initdata to the two variants, I will require to have a
> copy of one of them.
> 
>>
>>> +static struct exynos_irq_chip exynos4210_wkup_irq_chip = {
>>>   .chip = {
>>> - .name = "exynos_wkup_irq_chip",
>>> + .name = "exynos4210_wkup_irq_chip",
>>>   .irq_unmask = exynos_irq_unmask,
>>>   .irq_mask = exynos_irq_mask,
>>>   .irq_ack = exynos_irq_ack,
>>> @@ -342,6 +338,29 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = {
>>>   .eint_pend = EXYNOS_WKUP_EPEND_OFFSET,
>>>  };
>>>
>>> +static struct exynos_irq_chip exynos7_wkup_irq_chip = {
>>> + .chip = {
>>> + .name = "exynos7_wkup_irq_chip",
>>> + .irq_unmask = exynos_irq_unmask,
>>> + .irq_mask = exynos_irq_mask,
>>> + .irq_ack = exynos_irq_ack,
>>> + .irq_set_type = exynos_irq_set_type,
>>> + .irq_set_wake = exynos_wkup_irq_set_wake,
>>> + },
>>> + .eint_con = EXYNOS7_WKUP_ECON_OFFSET,
>>> + .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET,
>>> + .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET,
>>> +};
>>> +
>>> +/* list of external wakeup controllers supported */
>>> +static const struct of_device_id exynos_wkup_irq_ids[] = {
>>> + { .compatible = "samsung,exynos4210-wakeup-eint",
>>> + .data = &exynos4210_wkup_irq_chip },
>>> + { .compatible = "samsung,exynos7-wakeup-eint",
>>> + .data = &exynos7_wkup_irq_chip },
>>> + { }
>>> +};
>>> +
>>>  /* interrupt handler for wakeup interrupts 0..15 */
>>>  static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc)
>>>  {
>>> @@ -434,7 +453,12 @@ static int exynos_eint_wkup_init(struct 
>>> samsung_pinctrl_drv_data *d)
>>>   int idx, irq;
>>>
>>>   for_each_child_of_node(dev->of_node, np) {
>>> - if (of_match_node(exynos_wkup_irq_ids, np)) {
>>> + const struct of_device_id *match;
>>> +
>>> + match = of_match_node(exynos_wkup_irq_ids, np);
>>> + if (match) {
>>> + memcpy(&exynos_wkup_irq_chip, match->data,
>>> + sizeof(struct exynos_irq_chip));
>>
>> Hmm, this doesn't look correct to me. You are modifying a static struct
>> here. Why couldn't you simply use the exynos irq chip pointed by
>> match->data in further registration code?
> 
> That will not be available later once I use __initdata.
> 

Then either __initdata shouldn't be necessary or kmemdup() should be
used to allocate a copy.

>>
>>>   wkup_np = np;
>>>   break;
>>>   }
>>> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h 
>>> b/drivers/pinctrl/samsung/pinctrl-exynos.h
>>> index e060722..0db1e52 100644
>>> --- a/drivers/pinctrl/samsung/pinctrl-exynos.h
>>> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h
>>> @@ -25,6 +25,9 @@
>>>  #define EXYNOS_WKUP_ECON_OFFSET  0xE00
>>>  #define EXYNOS_WKUP_EMASK_OFFSET 0xF00
>>>  #define EXYNOS_WKUP_EPEND_OFFSET 0xF40
>>> +#define EXYNOS7_WKUP_ECON_OFFSET 0x700
>>> +#define EXYNOS7_WKUP_EMASK_OFFSET0x900
>>> +#define EXYNOS7_WKUP_EPEND_OFFSET0xA00
>>
>> Interestingly enough, the offsets look just like the normal GPIO
>> interrupt controller of previous Exynos SoCs. Are you sure those are
>> correct? Also if somehow the controller now resembles the normal one,
>> doesn't it have the SVC register making it possible to reuse the non
>> wake-up code instead?
> 
> The wakeup interrupt register offsets are the same as the GPIO
> interrupt offsets in earlier Exynos SoCs. There is no SVC register for
> the wakeup interrupt block.

OK, your code is fine then.

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts

2014-09-21 Thread Abhilash Kesavan 
Hi Tomasz,

On Sat, Sep 13, 2014 at 4:57 PM, Tomasz Figa  wrote:
> Hi Abhilash,
>
> Please see my comments inline.
>
> On 13.09.2014 10:50, Abhilash Kesavan wrote:
>> Exynos7 uses different offsets for wakeup interrupt configuration registers.
>> So a new irq_chip instance for Exynos7 wakeup interrupts is added. The 
>> irq_chip
>> selection is now based on the wakeup interrupt controller compatible string.
>
> [snip]
>
>> @@ -328,9 +322,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data 
>> *irqd, unsigned int on)
>>  /*
>>   * irq_chip for wakeup interrupts
>>   */
>> -static struct exynos_irq_chip exynos_wkup_irq_chip = {
>> +static struct exynos_irq_chip exynos_wkup_irq_chip;
>> +
>
> Why do you still need this, if you have both variants below?

After adding __initdata to the two variants, I will require to have a
copy of one of them.

>
>> +static struct exynos_irq_chip exynos4210_wkup_irq_chip = {
>>   .chip = {
>> - .name = "exynos_wkup_irq_chip",
>> + .name = "exynos4210_wkup_irq_chip",
>>   .irq_unmask = exynos_irq_unmask,
>>   .irq_mask = exynos_irq_mask,
>>   .irq_ack = exynos_irq_ack,
>> @@ -342,6 +338,29 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = {
>>   .eint_pend = EXYNOS_WKUP_EPEND_OFFSET,
>>  };
>>
>> +static struct exynos_irq_chip exynos7_wkup_irq_chip = {
>> + .chip = {
>> + .name = "exynos7_wkup_irq_chip",
>> + .irq_unmask = exynos_irq_unmask,
>> + .irq_mask = exynos_irq_mask,
>> + .irq_ack = exynos_irq_ack,
>> + .irq_set_type = exynos_irq_set_type,
>> + .irq_set_wake = exynos_wkup_irq_set_wake,
>> + },
>> + .eint_con = EXYNOS7_WKUP_ECON_OFFSET,
>> + .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET,
>> + .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET,
>> +};
>> +
>> +/* list of external wakeup controllers supported */
>> +static const struct of_device_id exynos_wkup_irq_ids[] = {
>> + { .compatible = "samsung,exynos4210-wakeup-eint",
>> + .data = &exynos4210_wkup_irq_chip },
>> + { .compatible = "samsung,exynos7-wakeup-eint",
>> + .data = &exynos7_wkup_irq_chip },
>> + { }
>> +};
>> +
>>  /* interrupt handler for wakeup interrupts 0..15 */
>>  static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc)
>>  {
>> @@ -434,7 +453,12 @@ static int exynos_eint_wkup_init(struct 
>> samsung_pinctrl_drv_data *d)
>>   int idx, irq;
>>
>>   for_each_child_of_node(dev->of_node, np) {
>> - if (of_match_node(exynos_wkup_irq_ids, np)) {
>> + const struct of_device_id *match;
>> +
>> + match = of_match_node(exynos_wkup_irq_ids, np);
>> + if (match) {
>> + memcpy(&exynos_wkup_irq_chip, match->data,
>> + sizeof(struct exynos_irq_chip));
>
> Hmm, this doesn't look correct to me. You are modifying a static struct
> here. Why couldn't you simply use the exynos irq chip pointed by
> match->data in further registration code?

That will not be available later once I use __initdata.

>
>>   wkup_np = np;
>>   break;
>>   }
>> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h 
>> b/drivers/pinctrl/samsung/pinctrl-exynos.h
>> index e060722..0db1e52 100644
>> --- a/drivers/pinctrl/samsung/pinctrl-exynos.h
>> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h
>> @@ -25,6 +25,9 @@
>>  #define EXYNOS_WKUP_ECON_OFFSET  0xE00
>>  #define EXYNOS_WKUP_EMASK_OFFSET 0xF00
>>  #define EXYNOS_WKUP_EPEND_OFFSET 0xF40
>> +#define EXYNOS7_WKUP_ECON_OFFSET 0x700
>> +#define EXYNOS7_WKUP_EMASK_OFFSET0x900
>> +#define EXYNOS7_WKUP_EPEND_OFFSET0xA00
>
> Interestingly enough, the offsets look just like the normal GPIO
> interrupt controller of previous Exynos SoCs. Are you sure those are
> correct? Also if somehow the controller now resembles the normal one,
> doesn't it have the SVC register making it possible to reuse the non
> wake-up code instead?

The wakeup interrupt register offsets are the same as the GPIO
interrupt offsets in earlier Exynos SoCs. There is no SVC register for
the wakeup interrupt block.

Regards,
Abhilash

>
> Best regards,
> Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts

2014-09-13 Thread Tomasz Figa 
Hi Abhilash,

Please see my comments inline.

On 13.09.2014 10:50, Abhilash Kesavan wrote:
> Exynos7 uses different offsets for wakeup interrupt configuration registers.
> So a new irq_chip instance for Exynos7 wakeup interrupts is added. The 
> irq_chip
> selection is now based on the wakeup interrupt controller compatible string.

[snip]

> @@ -328,9 +322,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data 
> *irqd, unsigned int on)
>  /*
>   * irq_chip for wakeup interrupts
>   */
> -static struct exynos_irq_chip exynos_wkup_irq_chip = {
> +static struct exynos_irq_chip exynos_wkup_irq_chip;
> +

Why do you still need this, if you have both variants below?

> +static struct exynos_irq_chip exynos4210_wkup_irq_chip = {
>   .chip = {
> - .name = "exynos_wkup_irq_chip",
> + .name = "exynos4210_wkup_irq_chip",
>   .irq_unmask = exynos_irq_unmask,
>   .irq_mask = exynos_irq_mask,
>   .irq_ack = exynos_irq_ack,
> @@ -342,6 +338,29 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = {
>   .eint_pend = EXYNOS_WKUP_EPEND_OFFSET,
>  };
>  
> +static struct exynos_irq_chip exynos7_wkup_irq_chip = {
> + .chip = {
> + .name = "exynos7_wkup_irq_chip",
> + .irq_unmask = exynos_irq_unmask,
> + .irq_mask = exynos_irq_mask,
> + .irq_ack = exynos_irq_ack,
> + .irq_set_type = exynos_irq_set_type,
> + .irq_set_wake = exynos_wkup_irq_set_wake,
> + },
> + .eint_con = EXYNOS7_WKUP_ECON_OFFSET,
> + .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET,
> + .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET,
> +};
> +
> +/* list of external wakeup controllers supported */
> +static const struct of_device_id exynos_wkup_irq_ids[] = {
> + { .compatible = "samsung,exynos4210-wakeup-eint",
> + .data = &exynos4210_wkup_irq_chip },
> + { .compatible = "samsung,exynos7-wakeup-eint",
> + .data = &exynos7_wkup_irq_chip },
> + { }
> +};
> +
>  /* interrupt handler for wakeup interrupts 0..15 */
>  static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc)
>  {
> @@ -434,7 +453,12 @@ static int exynos_eint_wkup_init(struct 
> samsung_pinctrl_drv_data *d)
>   int idx, irq;
>  
>   for_each_child_of_node(dev->of_node, np) {
> - if (of_match_node(exynos_wkup_irq_ids, np)) {
> + const struct of_device_id *match;
> +
> + match = of_match_node(exynos_wkup_irq_ids, np);
> + if (match) {
> + memcpy(&exynos_wkup_irq_chip, match->data,
> + sizeof(struct exynos_irq_chip));

Hmm, this doesn't look correct to me. You are modifying a static struct
here. Why couldn't you simply use the exynos irq chip pointed by
match->data in further registration code?

>   wkup_np = np;
>   break;
>   }
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h 
> b/drivers/pinctrl/samsung/pinctrl-exynos.h
> index e060722..0db1e52 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.h
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.h
> @@ -25,6 +25,9 @@
>  #define EXYNOS_WKUP_ECON_OFFSET  0xE00
>  #define EXYNOS_WKUP_EMASK_OFFSET 0xF00
>  #define EXYNOS_WKUP_EPEND_OFFSET 0xF40
> +#define EXYNOS7_WKUP_ECON_OFFSET 0x700
> +#define EXYNOS7_WKUP_EMASK_OFFSET0x900
> +#define EXYNOS7_WKUP_EPEND_OFFSET0xA00

Interestingly enough, the offsets look just like the normal GPIO
interrupt controller of previous Exynos SoCs. Are you sure those are
correct? Also if somehow the controller now resembles the normal one,
doesn't it have the SVC register making it possible to reuse the non
wake-up code instead?

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts

2014-09-13 Thread Thomas Abraham 
Hi Abhilash,

On Sat, Sep 13, 2014 at 2:20 PM, Abhilash Kesavan  wrote:
> Exynos7 uses different offsets for wakeup interrupt configuration registers.
> So a new irq_chip instance for Exynos7 wakeup interrupts is added. The 
> irq_chip
> selection is now based on the wakeup interrupt controller compatible string.
>
> Signed-off-by: Abhilash Kesavan 
> Cc: Thomas Abraham 
> Cc: Tomasz Figa 
> Cc: Linus Walleij 
> ---
>  .../bindings/pinctrl/samsung-pinctrl.txt   |2 +
>  drivers/pinctrl/samsung/pinctrl-exynos.c   |   42 
> +++-
>  drivers/pinctrl/samsung/pinctrl-exynos.h   |3 ++
>  3 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt 
> b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> index e82aaf4..f80519a 100644
> --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> @@ -136,6 +136,8 @@ B. External Wakeup Interrupts: For supporting external 
> wakeup interrupts, a
> found on Samsung S3C64xx SoCs,
>   - samsung,exynos4210-wakeup-eint: represents wakeup interrupt controller
> found on Samsung Exynos4210 and S5PC110/S5PV210 SoCs.
> + - samsung,exynos7-wakeup-eint: represents wakeup interrupt controller
> +   found on Samsung Exynos7 SoC.
> - interrupt-parent: phandle of the interrupt parent to which the external
>   wakeup interrupts are forwarded to.
> - interrupts: interrupt used by multiplexed wakeup interrupts.
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c 
> b/drivers/pinctrl/samsung/pinctrl-exynos.c
> index 3133a1e..fe15ab8 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
> @@ -56,12 +56,6 @@ static struct samsung_pin_bank_type bank_type_alive = {
> .reg_offset = { 0x00, 0x04, 0x08, 0x0c, },
>  };
>
> -/* list of external wakeup controllers supported */
> -static const struct of_device_id exynos_wkup_irq_ids[] = {
> -   { .compatible = "samsung,exynos4210-wakeup-eint", },
> -   { }
> -};
> -
>  static void exynos_irq_mask(struct irq_data *irqd)
>  {
> struct irq_chip *chip = irq_data_get_irq_chip(irqd);
> @@ -328,9 +322,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data 
> *irqd, unsigned int on)
>  /*
>   * irq_chip for wakeup interrupts
>   */
> -static struct exynos_irq_chip exynos_wkup_irq_chip = {
> +static struct exynos_irq_chip exynos_wkup_irq_chip;
> +
> +static struct exynos_irq_chip exynos4210_wkup_irq_chip = {

This should be marked as initdata.

> .chip = {
> -   .name = "exynos_wkup_irq_chip",
> +   .name = "exynos4210_wkup_irq_chip",
> .irq_unmask = exynos_irq_unmask,
> .irq_mask = exynos_irq_mask,
> .irq_ack = exynos_irq_ack,
> @@ -342,6 +338,29 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = {
> .eint_pend = EXYNOS_WKUP_EPEND_OFFSET,
>  };
>
> +static struct exynos_irq_chip exynos7_wkup_irq_chip = {

this as well.

> +   .chip = {
> +   .name = "exynos7_wkup_irq_chip",
> +   .irq_unmask = exynos_irq_unmask,
> +   .irq_mask = exynos_irq_mask,
> +   .irq_ack = exynos_irq_ack,
> +   .irq_set_type = exynos_irq_set_type,
> +   .irq_set_wake = exynos_wkup_irq_set_wake,
> +   },
> +   .eint_con = EXYNOS7_WKUP_ECON_OFFSET,
> +   .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET,
> +   .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET,
> +};
> +
> +/* list of external wakeup controllers supported */
> +static const struct of_device_id exynos_wkup_irq_ids[] = {
> +   { .compatible = "samsung,exynos4210-wakeup-eint",
> +   .data = &exynos4210_wkup_irq_chip },
> +   { .compatible = "samsung,exynos7-wakeup-eint",
> +   .data = &exynos7_wkup_irq_chip },
> +   { }
> +};
> +
>  /* interrupt handler for wakeup interrupts 0..15 */
>  static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc)
>  {
> @@ -434,7 +453,12 @@ static int exynos_eint_wkup_init(struct 
> samsung_pinctrl_drv_data *d)
> int idx, irq;
>
> for_each_child_of_node(dev->of_node, np) {
> -   if (of_match_node(exynos_wkup_irq_ids, np)) {
> +   const struct of_device_id *match;
> +
> +   match = of_match_node(exynos_wkup_irq_ids, np);
> +   if (match) {
> +   memcpy(&exynos_wkup_irq_chip, match->data,
> +   sizeof(struct exynos_irq_chip));
> wkup_np = np;
> break;
> }
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h 
> b/drivers/pinctrl/samsung/pinctrl-exynos.h
> index e060722..0db1e52 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.h
> +++ b/drivers/pinctrl/samsung/pinct

[PATCH 2/4] pinctrl: exynos: Add irq_chip instance for Exynos7 wakeup interrupts

2014-09-13 Thread Abhilash Kesavan 
Exynos7 uses different offsets for wakeup interrupt configuration registers.
So a new irq_chip instance for Exynos7 wakeup interrupts is added. The irq_chip
selection is now based on the wakeup interrupt controller compatible string.

Signed-off-by: Abhilash Kesavan 
Cc: Thomas Abraham 
Cc: Tomasz Figa 
Cc: Linus Walleij 
---
 .../bindings/pinctrl/samsung-pinctrl.txt   |2 +
 drivers/pinctrl/samsung/pinctrl-exynos.c   |   42 +++-
 drivers/pinctrl/samsung/pinctrl-exynos.h   |3 ++
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt 
b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index e82aaf4..f80519a 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -136,6 +136,8 @@ B. External Wakeup Interrupts: For supporting external 
wakeup interrupts, a
found on Samsung S3C64xx SoCs,
  - samsung,exynos4210-wakeup-eint: represents wakeup interrupt controller
found on Samsung Exynos4210 and S5PC110/S5PV210 SoCs.
+ - samsung,exynos7-wakeup-eint: represents wakeup interrupt controller
+   found on Samsung Exynos7 SoC.
- interrupt-parent: phandle of the interrupt parent to which the external
  wakeup interrupts are forwarded to.
- interrupts: interrupt used by multiplexed wakeup interrupts.
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c 
b/drivers/pinctrl/samsung/pinctrl-exynos.c
index 3133a1e..fe15ab8 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
@@ -56,12 +56,6 @@ static struct samsung_pin_bank_type bank_type_alive = {
.reg_offset = { 0x00, 0x04, 0x08, 0x0c, },
 };
 
-/* list of external wakeup controllers supported */
-static const struct of_device_id exynos_wkup_irq_ids[] = {
-   { .compatible = "samsung,exynos4210-wakeup-eint", },
-   { }
-};
-
 static void exynos_irq_mask(struct irq_data *irqd)
 {
struct irq_chip *chip = irq_data_get_irq_chip(irqd);
@@ -328,9 +322,11 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, 
unsigned int on)
 /*
  * irq_chip for wakeup interrupts
  */
-static struct exynos_irq_chip exynos_wkup_irq_chip = {
+static struct exynos_irq_chip exynos_wkup_irq_chip;
+
+static struct exynos_irq_chip exynos4210_wkup_irq_chip = {
.chip = {
-   .name = "exynos_wkup_irq_chip",
+   .name = "exynos4210_wkup_irq_chip",
.irq_unmask = exynos_irq_unmask,
.irq_mask = exynos_irq_mask,
.irq_ack = exynos_irq_ack,
@@ -342,6 +338,29 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = {
.eint_pend = EXYNOS_WKUP_EPEND_OFFSET,
 };
 
+static struct exynos_irq_chip exynos7_wkup_irq_chip = {
+   .chip = {
+   .name = "exynos7_wkup_irq_chip",
+   .irq_unmask = exynos_irq_unmask,
+   .irq_mask = exynos_irq_mask,
+   .irq_ack = exynos_irq_ack,
+   .irq_set_type = exynos_irq_set_type,
+   .irq_set_wake = exynos_wkup_irq_set_wake,
+   },
+   .eint_con = EXYNOS7_WKUP_ECON_OFFSET,
+   .eint_mask = EXYNOS7_WKUP_EMASK_OFFSET,
+   .eint_pend = EXYNOS7_WKUP_EPEND_OFFSET,
+};
+
+/* list of external wakeup controllers supported */
+static const struct of_device_id exynos_wkup_irq_ids[] = {
+   { .compatible = "samsung,exynos4210-wakeup-eint",
+   .data = &exynos4210_wkup_irq_chip },
+   { .compatible = "samsung,exynos7-wakeup-eint",
+   .data = &exynos7_wkup_irq_chip },
+   { }
+};
+
 /* interrupt handler for wakeup interrupts 0..15 */
 static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc)
 {
@@ -434,7 +453,12 @@ static int exynos_eint_wkup_init(struct 
samsung_pinctrl_drv_data *d)
int idx, irq;
 
for_each_child_of_node(dev->of_node, np) {
-   if (of_match_node(exynos_wkup_irq_ids, np)) {
+   const struct of_device_id *match;
+
+   match = of_match_node(exynos_wkup_irq_ids, np);
+   if (match) {
+   memcpy(&exynos_wkup_irq_chip, match->data,
+   sizeof(struct exynos_irq_chip));
wkup_np = np;
break;
}
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h 
b/drivers/pinctrl/samsung/pinctrl-exynos.h
index e060722..0db1e52 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.h
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.h
@@ -25,6 +25,9 @@
 #define EXYNOS_WKUP_ECON_OFFSET0xE00
 #define EXYNOS_WKUP_EMASK_OFFSET   0xF00
 #define EXYNOS_WKUP_EPEND_OFFSET   0xF40
+#define EXYNOS7_WKUP_ECON_OFFSET   0x700
+#define EXYNOS7_WKUP_EMASK_OFFSET  0x900
+#define EXYNOS7_WKUP_EPEND_OFFSET  0xA00
 #define EXYNOS_SVC_OFFSET