Re: [PATCH 02/11] pinctrl: sunxi: add IRQCHIP_SKIP_SET_WAKE flag for pinctrl irq chip

[Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 28.05.2014 12:29, Maxime Ripard wrote:
> On Tue, May 27, 2014 at 06:14:31PM +0200, Tomasz Figa wrote:
>> On 27.05.2014 10:07, Maxime Ripard wrote:
>>> On Mon, May 26, 2014 at 09:47:57AM +0200, Hans de Goede wrote:
>>>> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>>>> 
>>>> The sunxi pinctrl irq chip driver does not support wakeup at
>>>> the moment. Adding IRQCHIP_SKIP_SET_WAKE lets the irqs work
>>>> with drivers using wakeup.
>>>> 
>>>> Also add a name to the irq chip.
>>>> 
>>>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> Signed-off-by:
>>>> Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> --- 
>>>> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 ++ 1 file changed,
>>>> 2 insertions(+)
>>>> 
>>>> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c 
>>>> b/drivers/pinctrl/sunxi/pinctrl-sunxi.c index
>>>> db9ccd6..ec60c2e 100644 ---
>>>> a/drivers/pinctrl/sunxi/pinctrl-sunxi.c +++ 
>>>> b/drivers/pinctrl/sunxi/pinctrl-sunxi.c @@ -646,6 +646,8 @@ 
>>>> static struct irq_chip sunxi_pinctrl_irq_chip = {
>>>> .irq_mask_ack	= sunxi_pinctrl_irq_mask_ack, .irq_unmask	= 
>>>> sunxi_pinctrl_irq_unmask, .irq_set_type	= 
>>>> sunxi_pinctrl_irq_set_type, +	.name		= "sunxi-pio", +	.flags
>>>> = IRQCHIP_SKIP_SET_WAKE,
>>> 
>>> I'd rather see the name set to dev_name() or something like
>>> that. This will not work that great with multiple pin
>>> controller supporting interrupts.
>> 
>> Correct me if I'm wrong, but I've always thought that struct
>> irq_chip describes a class of IRQ controllers, not particular
>> instances.
> 
> It's not said as such in <linux/irq.h>, but maybe..

It is not said the otherwise as well, so hopefully someone could
clarify this. Thomas?

My conclusion about the purpose of this struct is based on its
contents - name, flags and a number of callbacks, nothing that could
be per-instance of identical chips. Moreover there is per interrupt
chip_data, which is usually used to store per-instance data by irqchip
drivers.

Best regards,
Tomasz