All of lore.kernel.org
 help / color / mirror / Atom feed
From: jbrunet@baylibre.com (Jerome Brunet)
To: linus-amlogic@lists.infradead.org
Subject: [PATCH 4/9] pinctrl: meson: allow gpio to request irq
Date: Fri, 21 Oct 2016 11:06:38 +0200	[thread overview]
Message-ID: <1477040798.15560.96.camel@baylibre.com> (raw)
In-Reply-To: <CACRpkdaCZPS45t3eTcjJExOu=YT_YkjNkdpg09jQqvLQOJH=Qw@mail.gmail.com>

On Thu, 2016-10-20 at 21:21 +0200, Linus Walleij wrote:
> On Wed, Oct 19, 2016 at 12:08 PM, Jerome Brunet <jbrunet@baylibre.com
> > wrote:
> 
> > 
> > Add the ability for gpio to request irq from the gpio interrupt
> > controller
> > if present. We have to specificaly that the parent interrupt
> > controller is
> > the gpio interrupt controller because gpio on meson SoCs can't
> > generate
> > interrupt directly on the GIC.
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> (...)
> > 
> > +???????select IRQ_DOMAIN
> > ????????select OF_GPIO
> > +???????select OF_IRQ
> (...)
> > 
> > +static int meson_gpio_to_hwirq(struct meson_bank *bank, unsigned
> > int offset)
> > +{
> > +???????unsigned int hwirq;
> > +
> > +???????if (bank->irq_first < 0)
> > +???????????????/* this bank cannot generate irqs */
> > +???????????????return -1;
> > +
> > +???????hwirq = offset - bank->first + bank->irq_first;
> > +
> > +???????if (hwirq > bank->irq_last)
> > +???????????????/* this pin cannot generate irqs */
> > +???????????????return -1;
> > +
> > +???????return hwirq;
> > +}
> 
> This is reimplementing irqdomain.
> 
> > 
> > +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned int
> > offset)
> > +{
> (...)
> > 
> > +???????hwirq = meson_gpio_to_hwirq(bank, offset);
> > +???????if (hwirq < 0) {
> > +???????????????dev_dbg(pc->dev, "no interrupt for pin %u\n",
> > offset);
> > +???????????????return 0;
> > +???????}
> 
> Isn't this usecase (also as described in the cover letter) a textbook
> example of when you should be using hierarchical irqdomain?
> 
> Please check with Marc et al on hierarchical irqdomains.

Linus,
Do you mean I should create a new hierarchical?irqdomains in each of
the two pinctrl instances we have in these SoC, these domains being
stacked on the one I just added for controller in irqchip ?

I did not understand this is what you meant when I asked you the
question at ELCE.

> 
> Yours,
> Linus Walleij

WARNING: multiple messages have this Message-ID (diff)
From: Jerome Brunet <jbrunet@baylibre.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Carlo Caione <carlo@caione.org>,
	Kevin Hilman <khilman@baylibre.com>,
	"open list:ARM/Amlogic Meson..."
	<linux-amlogic@lists.infradead.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Russell King <linux@armlinux.org.uk>
Subject: Re: [PATCH 4/9] pinctrl: meson: allow gpio to request irq
Date: Fri, 21 Oct 2016 11:06:38 +0200	[thread overview]
Message-ID: <1477040798.15560.96.camel@baylibre.com> (raw)
In-Reply-To: <CACRpkdaCZPS45t3eTcjJExOu=YT_YkjNkdpg09jQqvLQOJH=Qw@mail.gmail.com>

On Thu, 2016-10-20 at 21:21 +0200, Linus Walleij wrote:
> On Wed, Oct 19, 2016 at 12:08 PM, Jerome Brunet <jbrunet@baylibre.com
> > wrote:
> 
> > 
> > Add the ability for gpio to request irq from the gpio interrupt
> > controller
> > if present. We have to specificaly that the parent interrupt
> > controller is
> > the gpio interrupt controller because gpio on meson SoCs can't
> > generate
> > interrupt directly on the GIC.
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> (...)
> > 
> > +       select IRQ_DOMAIN
> >         select OF_GPIO
> > +       select OF_IRQ
> (...)
> > 
> > +static int meson_gpio_to_hwirq(struct meson_bank *bank, unsigned
> > int offset)
> > +{
> > +       unsigned int hwirq;
> > +
> > +       if (bank->irq_first < 0)
> > +               /* this bank cannot generate irqs */
> > +               return -1;
> > +
> > +       hwirq = offset - bank->first + bank->irq_first;
> > +
> > +       if (hwirq > bank->irq_last)
> > +               /* this pin cannot generate irqs */
> > +               return -1;
> > +
> > +       return hwirq;
> > +}
> 
> This is reimplementing irqdomain.
> 
> > 
> > +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned int
> > offset)
> > +{
> (...)
> > 
> > +       hwirq = meson_gpio_to_hwirq(bank, offset);
> > +       if (hwirq < 0) {
> > +               dev_dbg(pc->dev, "no interrupt for pin %u\n",
> > offset);
> > +               return 0;
> > +       }
> 
> Isn't this usecase (also as described in the cover letter) a textbook
> example of when you should be using hierarchical irqdomain?
> 
> Please check with Marc et al on hierarchical irqdomains.

Linus,
Do you mean I should create a new hierarchical irqdomains in each of
the two pinctrl instances we have in these SoC, these domains being
stacked on the one I just added for controller in irqchip ?

I did not understand this is what you meant when I asked you the
question at ELCE.

> 
> Yours,
> Linus Walleij

WARNING: multiple messages have this Message-ID (diff)
From: jbrunet@baylibre.com (Jerome Brunet)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 4/9] pinctrl: meson: allow gpio to request irq
Date: Fri, 21 Oct 2016 11:06:38 +0200	[thread overview]
Message-ID: <1477040798.15560.96.camel@baylibre.com> (raw)
In-Reply-To: <CACRpkdaCZPS45t3eTcjJExOu=YT_YkjNkdpg09jQqvLQOJH=Qw@mail.gmail.com>

On Thu, 2016-10-20 at 21:21 +0200, Linus Walleij wrote:
> On Wed, Oct 19, 2016 at 12:08 PM, Jerome Brunet <jbrunet@baylibre.com
> > wrote:
> 
> > 
> > Add the ability for gpio to request irq from the gpio interrupt
> > controller
> > if present. We have to specificaly that the parent interrupt
> > controller is
> > the gpio interrupt controller because gpio on meson SoCs can't
> > generate
> > interrupt directly on the GIC.
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> (...)
> > 
> > +???????select IRQ_DOMAIN
> > ????????select OF_GPIO
> > +???????select OF_IRQ
> (...)
> > 
> > +static int meson_gpio_to_hwirq(struct meson_bank *bank, unsigned
> > int offset)
> > +{
> > +???????unsigned int hwirq;
> > +
> > +???????if (bank->irq_first < 0)
> > +???????????????/* this bank cannot generate irqs */
> > +???????????????return -1;
> > +
> > +???????hwirq = offset - bank->first + bank->irq_first;
> > +
> > +???????if (hwirq > bank->irq_last)
> > +???????????????/* this pin cannot generate irqs */
> > +???????????????return -1;
> > +
> > +???????return hwirq;
> > +}
> 
> This is reimplementing irqdomain.
> 
> > 
> > +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned int
> > offset)
> > +{
> (...)
> > 
> > +???????hwirq = meson_gpio_to_hwirq(bank, offset);
> > +???????if (hwirq < 0) {
> > +???????????????dev_dbg(pc->dev, "no interrupt for pin %u\n",
> > offset);
> > +???????????????return 0;
> > +???????}
> 
> Isn't this usecase (also as described in the cover letter) a textbook
> example of when you should be using hierarchical irqdomain?
> 
> Please check with Marc et al on hierarchical irqdomains.

Linus,
Do you mean I should create a new hierarchical?irqdomains in each of
the two pinctrl instances we have in these SoC, these domains being
stacked on the one I just added for controller in irqchip ?

I did not understand this is what you meant when I asked you the
question at ELCE.

> 
> Yours,
> Linus Walleij

  reply	other threads:[~2016-10-21  9:06 UTC|newest]

Thread overview: 109+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-19 10:08 [PATCH 0/9] irqchip: meson: add support for the gpio interrupt controller Jerome Brunet
2016-10-19 10:08 ` Jerome Brunet
2016-10-19 10:08 ` Jerome Brunet
2016-10-19 10:08 ` Jerome Brunet
2016-10-19 10:08 ` [PATCH 1/9] irqchip: meson: add support for " Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08 ` [PATCH 2/9] dt-bindings: interrupt-controller: add DT binding for meson GPIO " Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08 ` [PATCH 3/9] pinctrl: meson: update pinctrl data with gpio irq data Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08 ` [PATCH 4/9] pinctrl: meson: allow gpio to request irq Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-20 19:21   ` Linus Walleij
2016-10-20 19:21     ` Linus Walleij
2016-10-20 19:21     ` Linus Walleij
2016-10-21  9:06     ` Jerome Brunet [this message]
2016-10-21  9:06       ` Jerome Brunet
2016-10-21  9:06       ` Jerome Brunet
2016-10-25  9:14       ` Linus Walleij
2016-10-25  9:14         ` Linus Walleij
2016-10-25  9:14         ` Linus Walleij
2016-10-25 10:38         ` Marc Zyngier
2016-10-25 10:38           ` Marc Zyngier
2016-10-25 10:38           ` Marc Zyngier
2016-10-25 13:08           ` Jerome Brunet
2016-10-25 13:08             ` Jerome Brunet
2016-10-25 13:08             ` Jerome Brunet
2016-10-25 13:38             ` Marc Zyngier
2016-10-25 13:38               ` Marc Zyngier
2016-10-25 13:38               ` Marc Zyngier
2016-10-25 13:38               ` Marc Zyngier
2016-10-25 14:22               ` Jerome Brunet
2016-10-25 14:22                 ` Jerome Brunet
2016-10-25 14:22                 ` Jerome Brunet
2016-10-25 14:22                 ` Jerome Brunet
2016-10-25 14:47                 ` Marc Zyngier
2016-10-25 14:47                   ` Marc Zyngier
2016-10-25 14:47                   ` Marc Zyngier
2016-10-25 14:47                   ` Marc Zyngier
2016-10-25 15:31                   ` Jerome Brunet
2016-10-25 15:31                     ` Jerome Brunet
2016-10-25 15:31                     ` Jerome Brunet
2016-10-25 18:20                     ` Linus Walleij
2016-10-25 18:20                       ` Linus Walleij
2016-10-25 18:20                       ` Linus Walleij
2016-10-26 14:22                       ` Jerome Brunet
2016-10-26 14:22                         ` Jerome Brunet
2016-10-26 14:22                         ` Jerome Brunet
2016-10-26 14:22                         ` Jerome Brunet
2016-10-26 14:32                         ` Linus Walleij
2016-10-26 14:32                           ` Linus Walleij
2016-10-26 14:32                           ` Linus Walleij
2016-10-26 14:32                           ` Linus Walleij
2016-10-26 15:50                           ` Kevin Hilman
2016-10-26 15:50                             ` Kevin Hilman
2016-10-26 15:50                             ` Kevin Hilman
2016-10-26 15:50                             ` Kevin Hilman
2016-11-04 14:40                             ` Linus Walleij
2016-11-04 14:40                               ` Linus Walleij
2016-11-04 14:40                               ` Linus Walleij
2016-10-25 18:10                   ` Linus Walleij
2016-10-25 18:10                     ` Linus Walleij
2016-10-25 18:10                     ` Linus Walleij
2016-10-26 14:23                     ` Jerome Brunet
2016-10-26 14:23                       ` Jerome Brunet
2016-10-26 14:23                       ` Jerome Brunet
2016-10-26 14:44                       ` Linus Walleij
2016-10-26 14:44                         ` Linus Walleij
2016-10-26 14:44                         ` Linus Walleij
2016-10-26 14:44                         ` Linus Walleij
2016-10-27 10:42                         ` Jerome Brunet
2016-10-27 10:42                           ` Jerome Brunet
2016-10-27 10:42                           ` Jerome Brunet
2016-10-27 10:42                           ` Jerome Brunet
2016-11-04 15:03                           ` Linus Walleij
2016-11-04 15:03                             ` Linus Walleij
2016-11-04 15:03                             ` Linus Walleij
2016-10-25 10:39         ` Thomas Gleixner
2016-10-25 10:39           ` Thomas Gleixner
2016-10-25 10:39           ` Thomas Gleixner
2016-10-25 10:39           ` Thomas Gleixner
2016-10-19 10:08 ` [PATCH 5/9] dt-bindings: pinctrl: meson: update gpio dt-bindings Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08 ` [PATCH 6/9] ARM64: meson: enable MESON_IRQ_GPIO in Kconfig Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08 ` [PATCH 7/9] ARM: meson: enable MESON_IRQ_GPIO in Kconfig for meson8 Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08 ` [PATCH 8/9] ARM64: dts: amlogic: enable gpio interrupt controller on gxbb Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08 ` [PATCH 9/9] ARM: dts: amlogic: enable gpio interrupt controller on meson8 Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet
2016-10-19 10:08   ` Jerome Brunet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1477040798.15560.96.camel@baylibre.com \
    --to=jbrunet@baylibre.com \
    --cc=linus-amlogic@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.