| Message ID | 1401090486-4414-3-git-send-email-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 26, 2014 at 09:47:57AM +0200, Hans de Goede wrote: > From: Chen-Yu Tsai <wens@csie.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@csie.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > 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. Maxime
Hi, On 05/27/2014 10:07 AM, Maxime Ripard wrote: > On Mon, May 26, 2014 at 09:47:57AM +0200, Hans de Goede wrote: >> From: Chen-Yu Tsai <wens@csie.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@csie.org> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> 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. That would require moving the irq_chip struct to become a member of the sunxi_pinctrl struct. Not undoable, but that seems like something which should be done in a separate patch. So shall I just drop the .name part of this patch for now ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 27, 2014 at 5:09 PM, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 05/27/2014 10:07 AM, Maxime Ripard wrote: >> On Mon, May 26, 2014 at 09:47:57AM +0200, Hans de Goede wrote: >>> From: Chen-Yu Tsai <wens@csie.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@csie.org> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> --- >>> 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. > > That would require moving the irq_chip struct to become a member of the > sunxi_pinctrl struct. Not undoable, but that seems like something which > should be done in a separate patch. So shall I just drop the .name part > of this patch for now ? Please do. AFAIK, .name is only used for the debugfs representation. This change was only meant for cosmetic purposes. ChenYu -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@csie.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@csie.org> Signed-off-by: Hans >> de Goede <hdegoede@redhat.com> --- >> 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. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@csie.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@csie.org> Signed-off-by: Hans > >> de Goede <hdegoede@redhat.com> --- > >> 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.. Maxime
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@csie.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@csie.org> Signed-off-by: >>>> Hans de Goede <hdegoede@redhat.com> --- >>>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, }; static void sunxi_pinctrl_irq_handler(unsigned irq, struct irq_desc *desc)