How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hello,

I'm running into an issue on several Renesas SoC with IRQ domains and GPIOs.

On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are handled by 
two separate IP cores, namely the PFC (Pin Function Controller) and INTC 
(Interrupt Controller). The former is handled by the sh-pfc driver 
(drivers/pinctrl/sh-pfc) and the later by the irq-renesas-intc-irqpin driver 
(drivers/irqchip), referred to below as the irqpin driver.

The sh73a0, for instance, has 32 external interrupt lines that are multiplexed 
on pins usable as GPIOs. Both the GPIO and external interrupt functions are 
usable at the same time, which allows reading the state of the interrupt 
lines.

These external interrupts are for MMC/SD support, among other devices. In this 
specific case the MMC/SD Card Detect signal is wired to one of the external 
interrupt signals, and the corresponding GPIO is passed to the MMC/SD 
controller driver. Depending on other configuration parameters the driver can 
then either poll the Card Detect signal, or register an interrupt handler to 
detect changes in the signal state. This features is implemented by the MMC/SD 
core, which call gpio_to_irq() on the GPIO to retrieve the corresponding IRQ 
number.

On non-DT systems the external IRQs are statically mapped at a known offset. 
The sh-pfc driver, to implement the gpio_to_irq() function (through its 
gpiochip .to_irq() handler), simply searches a SoC-specific lookup table for 
the fixed IRQ number associated with a given GPIO.

However, on DT systems, IRQs are mapped dynamically on demand. The irqpin 
driver registers a simple IRQ domain, and the irq_create_mapping() function 
can then be used to map a given IRQ, specified as an offset in the domain. 
This is where the problem appears, as the irqchip .to_irq() function is 
implemented in the sh-pfc driver, which doesn't have access to the IRQ domain 
registered by the irqpin driver.

I could hack around this by exporting a function in the irqpin driver that 
would map an IRQ, and call that function from the sh-pfc driver. I'd rather 
avoid that solution as it would add a direct dependency between the two 
drivers.

Has anyone run into a similar issue ? My gut feeling is that the architecture 
isn't right somewhere, but I can't really pinpoint where. As the external IRQs 
are handled by an IP core separate from the PFC one could argue that the 
corresponding IRQs are not really GPIO IRQs, and that the PFC driver shouldn't 
implement the .to_irq() operation. However, this would push the problem down 
to all drivers that need a GPIO they can read and a (possibly optional) IRQ 
associated with the same signal, and that rely on gpio_to_irq() to retrieve 
the IRQ associated with the signal. If all those drivers were required to 
handle the GPIO and the IRQ separately, then we could as well completely 
remove gpio_to_irq() from the kernel, which doesn't sound like the right thing 
to do.

Advices and opinions will be welcome.

-- 
Regards,

Laurent Pinchart

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hello,

I'm running into an issue on several Renesas SoC with IRQ domains and GPIOs.

On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are handled by 
two separate IP cores, namely the PFC (Pin Function Controller) and INTC 
(Interrupt Controller). The former is handled by the sh-pfc driver 
(drivers/pinctrl/sh-pfc) and the later by the irq-renesas-intc-irqpin driver 
(drivers/irqchip), referred to below as the irqpin driver.

The sh73a0, for instance, has 32 external interrupt lines that are multiplexed 
on pins usable as GPIOs. Both the GPIO and external interrupt functions are 
usable at the same time, which allows reading the state of the interrupt 
lines.

These external interrupts are for MMC/SD support, among other devices. In this 
specific case the MMC/SD Card Detect signal is wired to one of the external 
interrupt signals, and the corresponding GPIO is passed to the MMC/SD 
controller driver. Depending on other configuration parameters the driver can 
then either poll the Card Detect signal, or register an interrupt handler to 
detect changes in the signal state. This features is implemented by the MMC/SD 
core, which call gpio_to_irq() on the GPIO to retrieve the corresponding IRQ 
number.

On non-DT systems the external IRQs are statically mapped at a known offset. 
The sh-pfc driver, to implement the gpio_to_irq() function (through its 
gpiochip .to_irq() handler), simply searches a SoC-specific lookup table for 
the fixed IRQ number associated with a given GPIO.

However, on DT systems, IRQs are mapped dynamically on demand. The irqpin 
driver registers a simple IRQ domain, and the irq_create_mapping() function 
can then be used to map a given IRQ, specified as an offset in the domain. 
This is where the problem appears, as the irqchip .to_irq() function is 
implemented in the sh-pfc driver, which doesn't have access to the IRQ domain 
registered by the irqpin driver.

I could hack around this by exporting a function in the irqpin driver that 
would map an IRQ, and call that function from the sh-pfc driver. I'd rather 
avoid that solution as it would add a direct dependency between the two 
drivers.

Has anyone run into a similar issue ? My gut feeling is that the architecture 
isn't right somewhere, but I can't really pinpoint where. As the external IRQs 
are handled by an IP core separate from the PFC one could argue that the 
corresponding IRQs are not really GPIO IRQs, and that the PFC driver shouldn't 
implement the .to_irq() operation. However, this would push the problem down 
to all drivers that need a GPIO they can read and a (possibly optional) IRQ 
associated with the same signal, and that rely on gpio_to_irq() to retrieve 
the IRQ associated with the signal. If all those drivers were required to 
handle the GPIO and the IRQ separately, then we could as well completely 
remove gpio_to_irq() from the kernel, which doesn't sound like the right thing 
to do.

Advices and opinions will be welcome.

-- 
Regards,

Laurent Pinchart

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Grant Likely]

On Wed, 24 Jul 2013 01:21:44 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> Hello,
> 
> I'm running into an issue on several Renesas SoC with IRQ domains and GPIOs.
> 
> On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are handled by 
> two separate IP cores, namely the PFC (Pin Function Controller) and INTC 
> (Interrupt Controller). The former is handled by the sh-pfc driver 
> (drivers/pinctrl/sh-pfc) and the later by the irq-renesas-intc-irqpin driver 
> (drivers/irqchip), referred to below as the irqpin driver.
> 
> The sh73a0, for instance, has 32 external interrupt lines that are multiplexed 
> on pins usable as GPIOs. Both the GPIO and external interrupt functions are 
> usable at the same time, which allows reading the state of the interrupt 
> lines.
> 
> These external interrupts are for MMC/SD support, among other devices. In this 
> specific case the MMC/SD Card Detect signal is wired to one of the external 
> interrupt signals, and the corresponding GPIO is passed to the MMC/SD 
> controller driver. Depending on other configuration parameters the driver can 
> then either poll the Card Detect signal, or register an interrupt handler to 
> detect changes in the signal state. This features is implemented by the MMC/SD 
> core, which call gpio_to_irq() on the GPIO to retrieve the corresponding IRQ 
> number.
> 
> On non-DT systems the external IRQs are statically mapped at a known offset. 
> The sh-pfc driver, to implement the gpio_to_irq() function (through its 
> gpiochip .to_irq() handler), simply searches a SoC-specific lookup table for 
> the fixed IRQ number associated with a given GPIO.
> 
> However, on DT systems, IRQs are mapped dynamically on demand. The irqpin 
> driver registers a simple IRQ domain, and the irq_create_mapping() function 
> can then be used to map a given IRQ, specified as an offset in the domain. 
> This is where the problem appears, as the irqchip .to_irq() function is 
> implemented in the sh-pfc driver, which doesn't have access to the IRQ domain 
> registered by the irqpin driver.
> 
> I could hack around this by exporting a function in the irqpin driver that 
> would map an IRQ, and call that function from the sh-pfc driver. I'd rather 
> avoid that solution as it would add a direct dependency between the two 
> drivers.

Well, the gpio controller really does need to know what irq is
associated with a GPIO line. If the gpio controller driver doesn't have
direct access to that information, then it needs to have a hook to set
it up.

In the past, I've seen gpio controllers have an interrupts property
specifying which interrupts it is connected to and can use for GPIO
events. That seems like a resonable scenario in this regard, but if
GPIOS are 1:1 mapped to irqs, then it means a lot of interrupt entries
need to appear in the GPIO node. The other option is to add a hook
directly to the gpio driver so that it knows to use a specific irq
controller; but that feels wrong. Thought it may be a bit verbose, the
addition of an interrupts property to the GPIO controller node is
probably what is wanted.

There has also been a trend to make gpio controllers also interrupt
controllers if they support being used directly as irq inputs, but that
doesn't really help your problem.  :-)

g.

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Grant Likely]

On Wed, 24 Jul 2013 01:21:44 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> Hello,
> 
> I'm running into an issue on several Renesas SoC with IRQ domains and GPIOs.
> 
> On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are handled by 
> two separate IP cores, namely the PFC (Pin Function Controller) and INTC 
> (Interrupt Controller). The former is handled by the sh-pfc driver 
> (drivers/pinctrl/sh-pfc) and the later by the irq-renesas-intc-irqpin driver 
> (drivers/irqchip), referred to below as the irqpin driver.
> 
> The sh73a0, for instance, has 32 external interrupt lines that are multiplexed 
> on pins usable as GPIOs. Both the GPIO and external interrupt functions are 
> usable at the same time, which allows reading the state of the interrupt 
> lines.
> 
> These external interrupts are for MMC/SD support, among other devices. In this 
> specific case the MMC/SD Card Detect signal is wired to one of the external 
> interrupt signals, and the corresponding GPIO is passed to the MMC/SD 
> controller driver. Depending on other configuration parameters the driver can 
> then either poll the Card Detect signal, or register an interrupt handler to 
> detect changes in the signal state. This features is implemented by the MMC/SD 
> core, which call gpio_to_irq() on the GPIO to retrieve the corresponding IRQ 
> number.
> 
> On non-DT systems the external IRQs are statically mapped at a known offset. 
> The sh-pfc driver, to implement the gpio_to_irq() function (through its 
> gpiochip .to_irq() handler), simply searches a SoC-specific lookup table for 
> the fixed IRQ number associated with a given GPIO.
> 
> However, on DT systems, IRQs are mapped dynamically on demand. The irqpin 
> driver registers a simple IRQ domain, and the irq_create_mapping() function 
> can then be used to map a given IRQ, specified as an offset in the domain. 
> This is where the problem appears, as the irqchip .to_irq() function is 
> implemented in the sh-pfc driver, which doesn't have access to the IRQ domain 
> registered by the irqpin driver.
> 
> I could hack around this by exporting a function in the irqpin driver that 
> would map an IRQ, and call that function from the sh-pfc driver. I'd rather 
> avoid that solution as it would add a direct dependency between the two 
> drivers.

Well, the gpio controller really does need to know what irq is
associated with a GPIO line. If the gpio controller driver doesn't have
direct access to that information, then it needs to have a hook to set
it up.

In the past, I've seen gpio controllers have an interrupts property
specifying which interrupts it is connected to and can use for GPIO
events. That seems like a resonable scenario in this regard, but if
GPIOS are 1:1 mapped to irqs, then it means a lot of interrupt entries
need to appear in the GPIO node. The other option is to add a hook
directly to the gpio driver so that it knows to use a specific irq
controller; but that feels wrong. Thought it may be a bit verbose, the
addition of an interrupts property to the GPIO controller node is
probably what is wanted.

There has also been a trend to make gpio controllers also interrupt
controllers if they support being used directly as irq inputs, but that
doesn't really help your problem.  :-)

g.

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Linus Walleij]

On Wed, Jul 24, 2013 at 1:21 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> Has anyone run into a similar issue ? My gut feeling is that the architecture
> isn't right somewhere, but I can't really pinpoint where.

We had a similar situation with the MFDs, where Mark, Lee and Sam
came up with the solution to include an irqdomain in the MFD cell
spawn function:

extern int mfd_add_devices(struct device *parent, int id,
                           struct mfd_cell *cells, int n_devs,
                           struct resource *mem_base,
                           int irq_base, struct irq_domain *irq_domain);

When each cell (i.e. a platform device) is created, the irq for that
cell will be translated with irq_create_mapping() so the cell/platform
device just get a Linux IRQ it can use and do not need to worry
about translating it.

Prior to this we had all sorts of exported translator functions
for the IRQs exported from each hub driver ---what a mess.

Can you think about a parent/child relationship making it possible
to pass the irqs readily translated in this case?

Yours,
Linus Walleij

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Linus Walleij]

On Wed, Jul 24, 2013 at 1:21 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> Has anyone run into a similar issue ? My gut feeling is that the architecture
> isn't right somewhere, but I can't really pinpoint where.

We had a similar situation with the MFDs, where Mark, Lee and Sam
came up with the solution to include an irqdomain in the MFD cell
spawn function:

extern int mfd_add_devices(struct device *parent, int id,
                           struct mfd_cell *cells, int n_devs,
                           struct resource *mem_base,
                           int irq_base, struct irq_domain *irq_domain);

When each cell (i.e. a platform device) is created, the irq for that
cell will be translated with irq_create_mapping() so the cell/platform
device just get a Linux IRQ it can use and do not need to worry
about translating it.

Prior to this we had all sorts of exported translator functions
for the IRQs exported from each hub driver ---what a mess.

Can you think about a parent/child relationship making it possible
to pass the irqs readily translated in this case?

Yours,
Linus Walleij

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hi Grant,

Thank you for your answer.

On Wednesday 24 July 2013 21:24:46 Grant Likely wrote:
> On Wed, 24 Jul 2013 01:21:44 +0200, Laurent Pinchart wrote:
> > Hello,
> > 
> > I'm running into an issue on several Renesas SoC with IRQ domains and
> > GPIOs.
> > 
> > On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are handled
> > by two separate IP cores, namely the PFC (Pin Function Controller) and
> > INTC (Interrupt Controller). The former is handled by the sh-pfc driver
> > (drivers/pinctrl/sh-pfc) and the later by the irq-renesas-intc-irqpin
> > driver (drivers/irqchip), referred to below as the irqpin driver.
> > 
> > The sh73a0, for instance, has 32 external interrupt lines that are
> > multiplexed on pins usable as GPIOs. Both the GPIO and external interrupt
> > functions are usable at the same time, which allows reading the state of
> > the interrupt lines.
> > 
> > These external interrupts are for MMC/SD support, among other devices. In
> > this specific case the MMC/SD Card Detect signal is wired to one of the
> > external interrupt signals, and the corresponding GPIO is passed to the
> > MMC/SD controller driver. Depending on other configuration parameters the
> > driver can then either poll the Card Detect signal, or register an
> > interrupt handler to detect changes in the signal state. This features is
> > implemented by the MMC/SD core, which call gpio_to_irq() on the GPIO to
> > retrieve the corresponding IRQ number.
> > 
> > On non-DT systems the external IRQs are statically mapped at a known
> > offset. The sh-pfc driver, to implement the gpio_to_irq() function
> > (through its gpiochip .to_irq() handler), simply searches a SoC-specific
> > lookup table for the fixed IRQ number associated with a given GPIO.
> > 
> > However, on DT systems, IRQs are mapped dynamically on demand. The irqpin
> > driver registers a simple IRQ domain, and the irq_create_mapping()
> > function can then be used to map a given IRQ, specified as an offset in
> > the domain. This is where the problem appears, as the irqchip .to_irq()
> > function is implemented in the sh-pfc driver, which doesn't have access to
> > the IRQ domain registered by the irqpin driver.
> > 
> > I could hack around this by exporting a function in the irqpin driver that
> > would map an IRQ, and call that function from the sh-pfc driver. I'd
> > rather avoid that solution as it would add a direct dependency between the
> > two drivers.
> 
> Well, the gpio controller really does need to know what irq is associated
> with a GPIO line. If the gpio controller driver doesn't have direct access
> to that information, then it needs to have a hook to set it up.

The GPIO controller knows what hardware IRQ is associated with a GPIO line. It 
can't however translate on its own that hardware IRQ number to a global IRQ 
number, as the global IRQ numbers are allocated dynamically through the IRQ 
mapping domain, which is local to the IRQ chip driver.

To summarize the issue, the GPIO API exports a GPIO to IRQ translation 
function, which requires two steps:

- translating from a GPIO number to an IRQ chip and an IRQ chip local hardware 
IRQ number

- translation from the hardware IRQ number to a global IRQ number

The second step currently requires information known to the IRQ chip driver 
only.

> In the past, I've seen gpio controllers have an interrupts property
> specifying which interrupts it is connected to and can use for GPIO events.
> That seems like a resonable scenario in this regard, but if GPIOS are 1:1
> mapped to irqs, then it means a lot of interrupt entries need to appear in
> the GPIO node. The other option is to add a hook directly to the gpio driver
> so that it knows to use a specific irq controller; but that feels wrong.
> Thought it may be a bit verbose, the addition of an interrupts property to
> the GPIO controller node is probably what is wanted.

That's a good idea. I don't actually need to specify all the interrupts 
associated with the GPIOs (there are 58 of them in the worst case), as the 
GPIO driver knows the hardware IRQ number of all interrupts associated with a 
GPIO. I could just specify the interrupt-parent in the GPIO DT node, and call 
irq_create_of_mapping() directly in my .to_irq() handler to translate the IRQ 
number using the interrupt-parent phandle as the controller argument. Would 
that be considered as an acceptable hack, or would it be preferred to specify 
the interrupts individually in the GPIO controller DT node ?

> There has also been a trend to make gpio controllers also interrupt
> controllers if they support being used directly as irq inputs, but that
> doesn't really help your problem.  :-)

Newer generations of the Renesas SoCs have a GPIO controller than also acts as 
an IRQ controller, but older generations unfortunately have two separate IP 
cores.

-- 
Regards,

Laurent Pinchart

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hi Grant,

Thank you for your answer.

On Wednesday 24 July 2013 21:24:46 Grant Likely wrote:
> On Wed, 24 Jul 2013 01:21:44 +0200, Laurent Pinchart wrote:
> > Hello,
> > 
> > I'm running into an issue on several Renesas SoC with IRQ domains and
> > GPIOs.
> > 
> > On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are handled
> > by two separate IP cores, namely the PFC (Pin Function Controller) and
> > INTC (Interrupt Controller). The former is handled by the sh-pfc driver
> > (drivers/pinctrl/sh-pfc) and the later by the irq-renesas-intc-irqpin
> > driver (drivers/irqchip), referred to below as the irqpin driver.
> > 
> > The sh73a0, for instance, has 32 external interrupt lines that are
> > multiplexed on pins usable as GPIOs. Both the GPIO and external interrupt
> > functions are usable at the same time, which allows reading the state of
> > the interrupt lines.
> > 
> > These external interrupts are for MMC/SD support, among other devices. In
> > this specific case the MMC/SD Card Detect signal is wired to one of the
> > external interrupt signals, and the corresponding GPIO is passed to the
> > MMC/SD controller driver. Depending on other configuration parameters the
> > driver can then either poll the Card Detect signal, or register an
> > interrupt handler to detect changes in the signal state. This features is
> > implemented by the MMC/SD core, which call gpio_to_irq() on the GPIO to
> > retrieve the corresponding IRQ number.
> > 
> > On non-DT systems the external IRQs are statically mapped at a known
> > offset. The sh-pfc driver, to implement the gpio_to_irq() function
> > (through its gpiochip .to_irq() handler), simply searches a SoC-specific
> > lookup table for the fixed IRQ number associated with a given GPIO.
> > 
> > However, on DT systems, IRQs are mapped dynamically on demand. The irqpin
> > driver registers a simple IRQ domain, and the irq_create_mapping()
> > function can then be used to map a given IRQ, specified as an offset in
> > the domain. This is where the problem appears, as the irqchip .to_irq()
> > function is implemented in the sh-pfc driver, which doesn't have access to
> > the IRQ domain registered by the irqpin driver.
> > 
> > I could hack around this by exporting a function in the irqpin driver that
> > would map an IRQ, and call that function from the sh-pfc driver. I'd
> > rather avoid that solution as it would add a direct dependency between the
> > two drivers.
> 
> Well, the gpio controller really does need to know what irq is associated
> with a GPIO line. If the gpio controller driver doesn't have direct access
> to that information, then it needs to have a hook to set it up.

The GPIO controller knows what hardware IRQ is associated with a GPIO line. It 
can't however translate on its own that hardware IRQ number to a global IRQ 
number, as the global IRQ numbers are allocated dynamically through the IRQ 
mapping domain, which is local to the IRQ chip driver.

To summarize the issue, the GPIO API exports a GPIO to IRQ translation 
function, which requires two steps:

- translating from a GPIO number to an IRQ chip and an IRQ chip local hardware 
IRQ number

- translation from the hardware IRQ number to a global IRQ number

The second step currently requires information known to the IRQ chip driver 
only.

> In the past, I've seen gpio controllers have an interrupts property
> specifying which interrupts it is connected to and can use for GPIO events.
> That seems like a resonable scenario in this regard, but if GPIOS are 1:1
> mapped to irqs, then it means a lot of interrupt entries need to appear in
> the GPIO node. The other option is to add a hook directly to the gpio driver
> so that it knows to use a specific irq controller; but that feels wrong.
> Thought it may be a bit verbose, the addition of an interrupts property to
> the GPIO controller node is probably what is wanted.

That's a good idea. I don't actually need to specify all the interrupts 
associated with the GPIOs (there are 58 of them in the worst case), as the 
GPIO driver knows the hardware IRQ number of all interrupts associated with a 
GPIO. I could just specify the interrupt-parent in the GPIO DT node, and call 
irq_create_of_mapping() directly in my .to_irq() handler to translate the IRQ 
number using the interrupt-parent phandle as the controller argument. Would 
that be considered as an acceptable hack, or would it be preferred to specify 
the interrupts individually in the GPIO controller DT node ?

> There has also been a trend to make gpio controllers also interrupt
> controllers if they support being used directly as irq inputs, but that
> doesn't really help your problem.  :-)

Newer generations of the Renesas SoCs have a GPIO controller than also acts as 
an IRQ controller, but older generations unfortunately have two separate IP 
cores.

-- 
Regards,

Laurent Pinchart

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hi Linus,

Thank you for your answer.

On Thursday 25 July 2013 11:20:54 Linus Walleij wrote:
> On Wed, Jul 24, 2013 at 1:21 AM, Laurent Pinchart wrote:
> > Has anyone run into a similar issue ? My gut feeling is that the
> > architecture isn't right somewhere, but I can't really pinpoint where.
> 
> We had a similar situation with the MFDs, where Mark, Lee and Sam came up
> with the solution to include an irqdomain in the MFD cell spawn function:
> 
> extern int mfd_add_devices(struct device *parent, int id,
>                            struct mfd_cell *cells, int n_devs,
>                            struct resource *mem_base,
>                            int irq_base, struct irq_domain *irq_domain);
> 
> When each cell (i.e. a platform device) is created, the irq for that cell
> will be translated with irq_create_mapping() so the cell/platform device
> just get a Linux IRQ it can use and do not need to worry about translating
> it.
> 
> Prior to this we had all sorts of exported translator functions for the IRQs
> exported from each hub driver ---what a mess.
> 
> Can you think about a parent/child relationship making it possible to pass
> the irqs readily translated in this case?

The two devices are independent, so there's no real parent/child relationship. 
However, as Grant proposed, I could list all the interrupts associated with 
GPIOs in the GPIO controller DT node. I would then just call 
irq_of_parse_and_map() in the .to_irq() handler to magically translate the 
GPIO number to a mapped IRQ number.

The number of interrupts can be pretty high (up to 58 in the worst case so 
far), so an alternative would be to specify the interrupt-parent only, and 
call irq_create_of_mapping() directly. What solution would you prefer ?

-- 
Regards,

Laurent Pinchart

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hi Linus,

Thank you for your answer.

On Thursday 25 July 2013 11:20:54 Linus Walleij wrote:
> On Wed, Jul 24, 2013 at 1:21 AM, Laurent Pinchart wrote:
> > Has anyone run into a similar issue ? My gut feeling is that the
> > architecture isn't right somewhere, but I can't really pinpoint where.
> 
> We had a similar situation with the MFDs, where Mark, Lee and Sam came up
> with the solution to include an irqdomain in the MFD cell spawn function:
> 
> extern int mfd_add_devices(struct device *parent, int id,
>                            struct mfd_cell *cells, int n_devs,
>                            struct resource *mem_base,
>                            int irq_base, struct irq_domain *irq_domain);
> 
> When each cell (i.e. a platform device) is created, the irq for that cell
> will be translated with irq_create_mapping() so the cell/platform device
> just get a Linux IRQ it can use and do not need to worry about translating
> it.
> 
> Prior to this we had all sorts of exported translator functions for the IRQs
> exported from each hub driver ---what a mess.
> 
> Can you think about a parent/child relationship making it possible to pass
> the irqs readily translated in this case?

The two devices are independent, so there's no real parent/child relationship. 
However, as Grant proposed, I could list all the interrupts associated with 
GPIOs in the GPIO controller DT node. I would then just call 
irq_of_parse_and_map() in the .to_irq() handler to magically translate the 
GPIO number to a mapped IRQ number.

The number of interrupts can be pretty high (up to 58 in the worst case so 
far), so an alternative would be to specify the interrupt-parent only, and 
call irq_create_of_mapping() directly. What solution would you prefer ?

-- 
Regards,

Laurent Pinchart

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 775 bytes --]

On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:

> The two devices are independent, so there's no real parent/child relationship. 
> However, as Grant proposed, I could list all the interrupts associated with 
> GPIOs in the GPIO controller DT node. I would then just call 
> irq_of_parse_and_map() in the .to_irq() handler to magically translate the 
> GPIO number to a mapped IRQ number.

> The number of interrupts can be pretty high (up to 58 in the worst case so 
> far), so an alternative would be to specify the interrupt-parent only, and 
> call irq_create_of_mapping() directly. What solution would you prefer ?

Are the interrupts in a contiguous block in the controller so you can
just pass around the controller and a base number?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Mark Brown]

On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:

> The two devices are independent, so there's no real parent/child relationship. 
> However, as Grant proposed, I could list all the interrupts associated with 
> GPIOs in the GPIO controller DT node. I would then just call 
> irq_of_parse_and_map() in the .to_irq() handler to magically translate the 
> GPIO number to a mapped IRQ number.

> The number of interrupts can be pretty high (up to 58 in the worst case so 
> far), so an alternative would be to specify the interrupt-parent only, and 
> call irq_create_of_mapping() directly. What solution would you prefer ?

Are the interrupts in a contiguous block in the controller so you can
just pass around the controller and a base number?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130725/7689d8e3/attachment.sig>

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Linus Walleij]

On Thu, Jul 25, 2013 at 11:45 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> The two devices are independent, so there's no real parent/child relationship.
> However, as Grant proposed, I could list all the interrupts associated with
> GPIOs in the GPIO controller DT node. I would then just call
> irq_of_parse_and_map() in the .to_irq() handler to magically translate the
> GPIO number to a mapped IRQ number.

That works even if the device tree will need a comment or two
to explain what is going on.

I suggested a similar solution for the OMAP crossbar mux
that is discussed in another thread, so I'm happy with this
approach.

Yours,
Linus Walleij

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Linus Walleij]

On Thu, Jul 25, 2013 at 11:45 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> The two devices are independent, so there's no real parent/child relationship.
> However, as Grant proposed, I could list all the interrupts associated with
> GPIOs in the GPIO controller DT node. I would then just call
> irq_of_parse_and_map() in the .to_irq() handler to magically translate the
> GPIO number to a mapped IRQ number.

That works even if the device tree will need a comment or two
to explain what is going on.

I suggested a similar solution for the OMAP crossbar mux
that is discussed in another thread, so I'm happy with this
approach.

Yours,
Linus Walleij

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Linus Walleij]

On Thu, Jul 25, 2013 at 3:15 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:
>
>> The number of interrupts can be pretty high (up to 58 in the worst case so
>> far), so an alternative would be to specify the interrupt-parent only, and
>> call irq_create_of_mapping() directly. What solution would you prefer ?
>
> Are the interrupts in a contiguous block in the controller so you can
> just pass around the controller and a base number?

That works with platform data and in-kernel structures, but AFAICT
device tree has no such "bulk" concept but expects you to list
each and every line individually in cases like this.

Yours,
Linus Walleij

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Linus Walleij]

On Thu, Jul 25, 2013 at 3:15 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:
>
>> The number of interrupts can be pretty high (up to 58 in the worst case so
>> far), so an alternative would be to specify the interrupt-parent only, and
>> call irq_create_of_mapping() directly. What solution would you prefer ?
>
> Are the interrupts in a contiguous block in the controller so you can
> just pass around the controller and a base number?

That works with platform data and in-kernel structures, but AFAICT
device tree has no such "bulk" concept but expects you to list
each and every line individually in cases like this.

Yours,
Linus Walleij

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]

Hi Mark,

On Thursday 25 July 2013 14:15:56 Mark Brown wrote:
> On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:
> > The two devices are independent, so there's no real parent/child
> > relationship. However, as Grant proposed, I could list all the interrupts
> > associated with GPIOs in the GPIO controller DT node. I would then just
> > call irq_of_parse_and_map() in the .to_irq() handler to magically
> > translate the GPIO number to a mapped IRQ number.
> > 
> > The number of interrupts can be pretty high (up to 58 in the worst case so
> > far), so an alternative would be to specify the interrupt-parent only, and
> > call irq_create_of_mapping() directly. What solution would you prefer ?
> 
> Are the interrupts in a contiguous block in the controller so you can just
> pass around the controller and a base number?

In two of the three SoCs I need to fix they are. I've just realized that in 
the last one the interrupts are in two contiguous blocks in two different 
parents. I will thus need at least a list of <parent-phandle base count>. Our 
standard interrupt bindings don't seem to support multiple parents, is that 
something that we want to fix or should I go for custom bindings ?

-- 
Regards,

Laurent Pinchart

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hi Mark,

On Thursday 25 July 2013 14:15:56 Mark Brown wrote:
> On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:
> > The two devices are independent, so there's no real parent/child
> > relationship. However, as Grant proposed, I could list all the interrupts
> > associated with GPIOs in the GPIO controller DT node. I would then just
> > call irq_of_parse_and_map() in the .to_irq() handler to magically
> > translate the GPIO number to a mapped IRQ number.
> > 
> > The number of interrupts can be pretty high (up to 58 in the worst case so
> > far), so an alternative would be to specify the interrupt-parent only, and
> > call irq_create_of_mapping() directly. What solution would you prefer ?
> 
> Are the interrupts in a contiguous block in the controller so you can just
> pass around the controller and a base number?

In two of the three SoCs I need to fix they are. I've just realized that in 
the last one the interrupts are in two contiguous blocks in two different 
parents. I will thus need at least a list of <parent-phandle base count>. Our 
standard interrupt bindings don't seem to support multiple parents, is that 
something that we want to fix or should I go for custom bindings ?

-- 
Regards,

Laurent Pinchart
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130725/be41a24f/attachment.sig>

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

On Thu, Jul 25, 2013 at 03:21:35PM +0200, Linus Walleij wrote:
> On Thu, Jul 25, 2013 at 3:15 PM, Mark Brown <broonie@kernel.org> wrote:

> > Are the interrupts in a contiguous block in the controller so you can
> > just pass around the controller and a base number?

> That works with platform data and in-kernel structures, but AFAICT
> device tree has no such "bulk" concept but expects you to list
> each and every line individually in cases like this.

It works fine with domains as well - the domains all have a hwirq number
which is local to the domain context and doesn't correspond to a Linux
interrupt number.  If you can say "the X interrupts in this domain
starting at Y correspond to these X GPIOs" then you should be able to
cope.  Hopefully.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Mark Brown]

On Thu, Jul 25, 2013 at 03:21:35PM +0200, Linus Walleij wrote:
> On Thu, Jul 25, 2013 at 3:15 PM, Mark Brown <broonie@kernel.org> wrote:

> > Are the interrupts in a contiguous block in the controller so you can
> > just pass around the controller and a base number?

> That works with platform data and in-kernel structures, but AFAICT
> device tree has no such "bulk" concept but expects you to list
> each and every line individually in cases like this.

It works fine with domains as well - the domains all have a hwirq number
which is local to the domain context and doesn't correspond to a Linux
interrupt number.  If you can say "the X interrupts in this domain
starting at Y correspond to these X GPIOs" then you should be able to
cope.  Hopefully.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130725/4d30ee7c/attachment.sig>

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

On Thu, Jul 25, 2013 at 03:22:29PM +0200, Laurent Pinchart wrote:
> On Thursday 25 July 2013 14:15:56 Mark Brown wrote:

> > Are the interrupts in a contiguous block in the controller so you can just
> > pass around the controller and a base number?

> In two of the three SoCs I need to fix they are. I've just realized that in 
> the last one the interrupts are in two contiguous blocks in two different 
> parents. I will thus need at least a list of <parent-phandle base count>. Our 
> standard interrupt bindings don't seem to support multiple parents, is that 
> something that we want to fix or should I go for custom bindings ?

It seems reasonable to define the bindings in a generic way in case
other people have the same problem but it's possible I may be missing a
trick regarding how to do this in DT so don't take my word for it.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Mark Brown]

On Thu, Jul 25, 2013 at 03:22:29PM +0200, Laurent Pinchart wrote:
> On Thursday 25 July 2013 14:15:56 Mark Brown wrote:

> > Are the interrupts in a contiguous block in the controller so you can just
> > pass around the controller and a base number?

> In two of the three SoCs I need to fix they are. I've just realized that in 
> the last one the interrupts are in two contiguous blocks in two different 
> parents. I will thus need at least a list of <parent-phandle base count>. Our 
> standard interrupt bindings don't seem to support multiple parents, is that 
> something that we want to fix or should I go for custom bindings ?

It seems reasonable to define the bindings in a generic way in case
other people have the same problem but it's possible I may be missing a
trick regarding how to do this in DT so don't take my word for it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130725/1c948990/attachment.sig>

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Grant Likely]

On Thu, 25 Jul 2013 15:22:29 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> Hi Mark,
> 
> On Thursday 25 July 2013 14:15:56 Mark Brown wrote:
> > On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:
> > > The two devices are independent, so there's no real parent/child
> > > relationship. However, as Grant proposed, I could list all the interrupts
> > > associated with GPIOs in the GPIO controller DT node. I would then just
> > > call irq_of_parse_and_map() in the .to_irq() handler to magically
> > > translate the GPIO number to a mapped IRQ number.
> > > 
> > > The number of interrupts can be pretty high (up to 58 in the worst case so
> > > far), so an alternative would be to specify the interrupt-parent only, and
> > > call irq_create_of_mapping() directly. What solution would you prefer ?
> > 
> > Are the interrupts in a contiguous block in the controller so you can just
> > pass around the controller and a base number?
> 
> In two of the three SoCs I need to fix they are. I've just realized that in 
> the last one the interrupts are in two contiguous blocks in two different 
> parents. I will thus need at least a list of <parent-phandle base count>. Our 
> standard interrupt bindings don't seem to support multiple parents,

You can actually do it by using a dummy node with interrupt-map and
interrupt-map-mask properties, but it is a pretty ugly solution in my
opinion.

> is that 
> something that we want to fix or should I go for custom bindings ?

Yes, I think it is something that we want to fix. Jean-Christophe was
going to propose an alternative to the interrupts property which allows
an array of <phandle interrupt-specifier> tuples, but I've not seen
anything yet. Go ahead and make a proposal.

You could try to encode a base+count variant, but honestly I don't think
it would be a good idea because it only would work with a very narrow
set of use cases. Consider if #interrupt-cells was set to 2. Which cell
gets incremented in the range of interrupts specified? Better I think to
merely have an array of fully specified irqs. Support for that property
could be transparently baked into the core interrupt parsing functions.

g.

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Grant Likely]

On Thu, 25 Jul 2013 15:22:29 +0200, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> Hi Mark,
> 
> On Thursday 25 July 2013 14:15:56 Mark Brown wrote:
> > On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:
> > > The two devices are independent, so there's no real parent/child
> > > relationship. However, as Grant proposed, I could list all the interrupts
> > > associated with GPIOs in the GPIO controller DT node. I would then just
> > > call irq_of_parse_and_map() in the .to_irq() handler to magically
> > > translate the GPIO number to a mapped IRQ number.
> > > 
> > > The number of interrupts can be pretty high (up to 58 in the worst case so
> > > far), so an alternative would be to specify the interrupt-parent only, and
> > > call irq_create_of_mapping() directly. What solution would you prefer ?
> > 
> > Are the interrupts in a contiguous block in the controller so you can just
> > pass around the controller and a base number?
> 
> In two of the three SoCs I need to fix they are. I've just realized that in 
> the last one the interrupts are in two contiguous blocks in two different 
> parents. I will thus need at least a list of <parent-phandle base count>. Our 
> standard interrupt bindings don't seem to support multiple parents,

You can actually do it by using a dummy node with interrupt-map and
interrupt-map-mask properties, but it is a pretty ugly solution in my
opinion.

> is that 
> something that we want to fix or should I go for custom bindings ?

Yes, I think it is something that we want to fix. Jean-Christophe was
going to propose an alternative to the interrupts property which allows
an array of <phandle interrupt-specifier> tuples, but I've not seen
anything yet. Go ahead and make a proposal.

You could try to encode a base+count variant, but honestly I don't think
it would be a good idea because it only would work with a very narrow
set of use cases. Consider if #interrupt-cells was set to 2. Which cell
gets incremented in the range of interrupts specified? Better I think to
merely have an array of fully specified irqs. Support for that property
could be transparently baked into the core interrupt parsing functions.

g.

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Tomasz Figa]

Hi Laurent,

On Wednesday 24 of July 2013 01:21:44 Laurent Pinchart wrote:
> Hello,
> 
> I'm running into an issue on several Renesas SoC with IRQ domains and
> GPIOs.
> 
> On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are
> handled by two separate IP cores, namely the PFC (Pin Function
> Controller) and INTC (Interrupt Controller). The former is handled by
> the sh-pfc driver (drivers/pinctrl/sh-pfc) and the later by the
> irq-renesas-intc-irqpin driver (drivers/irqchip), referred to below as
> the irqpin driver.

Is the INTC used for anything more than just external interrupts on GPIO 
lines?

> The sh73a0, for instance, has 32 external interrupt lines that are
> multiplexed on pins usable as GPIOs. Both the GPIO and external
> interrupt functions are usable at the same time, which allows reading
> the state of the interrupt lines.
> 
> These external interrupts are for MMC/SD support, among other devices.
> In this specific case the MMC/SD Card Detect signal is wired to one of
> the external interrupt signals, and the corresponding GPIO is passed to
> the MMC/SD controller driver. Depending on other configuration
> parameters the driver can then either poll the Card Detect signal, or
> register an interrupt handler to detect changes in the signal state.
> This features is implemented by the MMC/SD core, which call
> gpio_to_irq() on the GPIO to retrieve the corresponding IRQ number.
> 
> On non-DT systems the external IRQs are statically mapped at a known
> offset. The sh-pfc driver, to implement the gpio_to_irq() function
> (through its gpiochip .to_irq() handler), simply searches a
> SoC-specific lookup table for the fixed IRQ number associated with a
> given GPIO.
> 
> However, on DT systems, IRQs are mapped dynamically on demand. The
> irqpin driver registers a simple IRQ domain, and the
> irq_create_mapping() function can then be used to map a given IRQ,
> specified as an offset in the domain. This is where the problem
> appears, as the irqchip .to_irq() function is implemented in the sh-pfc

I assume it should be s/irqchip/gpiochip/ in the line above, shouldn't it?

> driver, which doesn't have access to the IRQ domain registered by the
> irqpin driver.
> 
> I could hack around this by exporting a function in the irqpin driver
> that would map an IRQ, and call that function from the sh-pfc driver.
> I'd rather avoid that solution as it would add a direct dependency
> between the two drivers.

If you could just get the IRQ domain registered by irqpin driver and use 
it in sh-pfc, then I guess it would solve your problem, as you could 
simply call irq_create_mapping() with the domain and hwirq as args in your 
gpiochip .to_irq() callback.

I'm not sure if it's not a hack, but you could add a property to the node 
of your pin controller that would contain a phandle to your interrupt 
controller. Then you could use of_parse_phandle() to get to device node of 
the INTC and then irq_find_host() to retrieve irq domain associated with 
it.

> Has anyone run into a similar issue ? My gut feeling is that the
> architecture isn't right somewhere, but I can't really pinpoint where.
> As the external IRQs are handled by an IP core separate from the PFC

Well, the fact that it's separate doesn't mean anything yet. Here my 
question whether it's used exclusively for GPIO interrupts or not becomes 
significant. If yes, maybe it could be simply moved to the pinctrl driver?

Best regards,
Tomasz

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Tomasz Figa]

Hi Laurent,

On Wednesday 24 of July 2013 01:21:44 Laurent Pinchart wrote:
> Hello,
> 
> I'm running into an issue on several Renesas SoC with IRQ domains and
> GPIOs.
> 
> On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are
> handled by two separate IP cores, namely the PFC (Pin Function
> Controller) and INTC (Interrupt Controller). The former is handled by
> the sh-pfc driver (drivers/pinctrl/sh-pfc) and the later by the
> irq-renesas-intc-irqpin driver (drivers/irqchip), referred to below as
> the irqpin driver.

Is the INTC used for anything more than just external interrupts on GPIO 
lines?

> The sh73a0, for instance, has 32 external interrupt lines that are
> multiplexed on pins usable as GPIOs. Both the GPIO and external
> interrupt functions are usable at the same time, which allows reading
> the state of the interrupt lines.
> 
> These external interrupts are for MMC/SD support, among other devices.
> In this specific case the MMC/SD Card Detect signal is wired to one of
> the external interrupt signals, and the corresponding GPIO is passed to
> the MMC/SD controller driver. Depending on other configuration
> parameters the driver can then either poll the Card Detect signal, or
> register an interrupt handler to detect changes in the signal state.
> This features is implemented by the MMC/SD core, which call
> gpio_to_irq() on the GPIO to retrieve the corresponding IRQ number.
> 
> On non-DT systems the external IRQs are statically mapped at a known
> offset. The sh-pfc driver, to implement the gpio_to_irq() function
> (through its gpiochip .to_irq() handler), simply searches a
> SoC-specific lookup table for the fixed IRQ number associated with a
> given GPIO.
> 
> However, on DT systems, IRQs are mapped dynamically on demand. The
> irqpin driver registers a simple IRQ domain, and the
> irq_create_mapping() function can then be used to map a given IRQ,
> specified as an offset in the domain. This is where the problem
> appears, as the irqchip .to_irq() function is implemented in the sh-pfc

I assume it should be s/irqchip/gpiochip/ in the line above, shouldn't it?

> driver, which doesn't have access to the IRQ domain registered by the
> irqpin driver.
> 
> I could hack around this by exporting a function in the irqpin driver
> that would map an IRQ, and call that function from the sh-pfc driver.
> I'd rather avoid that solution as it would add a direct dependency
> between the two drivers.

If you could just get the IRQ domain registered by irqpin driver and use 
it in sh-pfc, then I guess it would solve your problem, as you could 
simply call irq_create_mapping() with the domain and hwirq as args in your 
gpiochip .to_irq() callback.

I'm not sure if it's not a hack, but you could add a property to the node 
of your pin controller that would contain a phandle to your interrupt 
controller. Then you could use of_parse_phandle() to get to device node of 
the INTC and then irq_find_host() to retrieve irq domain associated with 
it.

> Has anyone run into a similar issue ? My gut feeling is that the
> architecture isn't right somewhere, but I can't really pinpoint where.
> As the external IRQs are handled by an IP core separate from the PFC

Well, the fact that it's separate doesn't mean anything yet. Here my 
question whether it's used exclusively for GPIO interrupts or not becomes 
significant. If yes, maybe it could be simply moved to the pinctrl driver?

Best regards,
Tomasz

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hi Tomasz,

On Sunday 28 July 2013 12:07:48 Tomasz Figa wrote:
> On Wednesday 24 of July 2013 01:21:44 Laurent Pinchart wrote:
> > Hello,
> > 
> > I'm running into an issue on several Renesas SoC with IRQ domains and
> > GPIOs.
> > 
> > On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are
> > handled by two separate IP cores, namely the PFC (Pin Function
> > Controller) and INTC (Interrupt Controller). The former is handled by
> > the sh-pfc driver (drivers/pinctrl/sh-pfc) and the later by the
> > irq-renesas-intc-irqpin driver (drivers/irqchip), referred to below as
> > the irqpin driver.
> 
> Is the INTC used for anything more than just external interrupts on GPIO
> lines?

Yes, it also handles other interrupt sources (NMI and peripherals), but those 
are not implemented now. The peripheral interrupts are also handled by the 
GIC, which is preferred over INTC.

> > The sh73a0, for instance, has 32 external interrupt lines that are
> > multiplexed on pins usable as GPIOs. Both the GPIO and external
> > interrupt functions are usable at the same time, which allows reading
> > the state of the interrupt lines.
> > 
> > These external interrupts are for MMC/SD support, among other devices.
> > In this specific case the MMC/SD Card Detect signal is wired to one of
> > the external interrupt signals, and the corresponding GPIO is passed to
> > the MMC/SD controller driver. Depending on other configuration
> > parameters the driver can then either poll the Card Detect signal, or
> > register an interrupt handler to detect changes in the signal state.
> > This features is implemented by the MMC/SD core, which call
> > gpio_to_irq() on the GPIO to retrieve the corresponding IRQ number.
> > 
> > On non-DT systems the external IRQs are statically mapped at a known
> > offset. The sh-pfc driver, to implement the gpio_to_irq() function
> > (through its gpiochip .to_irq() handler), simply searches a
> > SoC-specific lookup table for the fixed IRQ number associated with a
> > given GPIO.
> > 
> > However, on DT systems, IRQs are mapped dynamically on demand. The
> > irqpin driver registers a simple IRQ domain, and the
> > irq_create_mapping() function can then be used to map a given IRQ,
> > specified as an offset in the domain. This is where the problem
> > appears, as the irqchip .to_irq() function is implemented in the sh-pfc
> 
> I assume it should be s/irqchip/gpiochip/ in the line above, shouldn't it?

Yes, my bad.

> > driver, which doesn't have access to the IRQ domain registered by the
> > irqpin driver.
> > 
> > I could hack around this by exporting a function in the irqpin driver
> > that would map an IRQ, and call that function from the sh-pfc driver.
> > I'd rather avoid that solution as it would add a direct dependency
> > between the two drivers.
> 
> If you could just get the IRQ domain registered by irqpin driver and use
> it in sh-pfc, then I guess it would solve your problem, as you could
> simply call irq_create_mapping() with the domain and hwirq as args in your
> gpiochip .to_irq() callback.
>
> I'm not sure if it's not a hack, but you could add a property to the node
> of your pin controller that would contain a phandle to your interrupt
> controller. Then you could use of_parse_phandle() to get to device node of
> the INTC and then irq_find_host() to retrieve irq domain associated with
> it.

That was my initial idea. However, one on of the SoCs, the GPIO interrupts are 
divided in two separate blocks, handled by two different interrupt controller 
instances. I could thus have a list of phandle + range, but that becomes 
pretty hackish. Specifying the interrupts explicitly would be more extensible.

> > Has anyone run into a similar issue ? My gut feeling is that the
> > architecture isn't right somewhere, but I can't really pinpoint where.
> > As the external IRQs are handled by an IP core separate from the PFC
> 
> Well, the fact that it's separate doesn't mean anything yet. Here my
> question whether it's used exclusively for GPIO interrupts or not becomes
> significant. If yes, maybe it could be simply moved to the pinctrl driver?

Depending on the SoC, I have two different IRQ controllers used for GPIO 
interrupts. They're called INTC and IRQC. INTC has other purposes (although 
not implemented at the moment). The IRQC instances used for GPIO interrupts 
are (at the moment) dedicated to GPIO interrupts, but other instances of the 
same IP core are used for other interrupts, so a separate driver makes sense 
in my opinion.

-- 
Regards,

Laurent Pinchart

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hi Tomasz,

On Sunday 28 July 2013 12:07:48 Tomasz Figa wrote:
> On Wednesday 24 of July 2013 01:21:44 Laurent Pinchart wrote:
> > Hello,
> > 
> > I'm running into an issue on several Renesas SoC with IRQ domains and
> > GPIOs.
> > 
> > On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are
> > handled by two separate IP cores, namely the PFC (Pin Function
> > Controller) and INTC (Interrupt Controller). The former is handled by
> > the sh-pfc driver (drivers/pinctrl/sh-pfc) and the later by the
> > irq-renesas-intc-irqpin driver (drivers/irqchip), referred to below as
> > the irqpin driver.
> 
> Is the INTC used for anything more than just external interrupts on GPIO
> lines?

Yes, it also handles other interrupt sources (NMI and peripherals), but those 
are not implemented now. The peripheral interrupts are also handled by the 
GIC, which is preferred over INTC.

> > The sh73a0, for instance, has 32 external interrupt lines that are
> > multiplexed on pins usable as GPIOs. Both the GPIO and external
> > interrupt functions are usable at the same time, which allows reading
> > the state of the interrupt lines.
> > 
> > These external interrupts are for MMC/SD support, among other devices.
> > In this specific case the MMC/SD Card Detect signal is wired to one of
> > the external interrupt signals, and the corresponding GPIO is passed to
> > the MMC/SD controller driver. Depending on other configuration
> > parameters the driver can then either poll the Card Detect signal, or
> > register an interrupt handler to detect changes in the signal state.
> > This features is implemented by the MMC/SD core, which call
> > gpio_to_irq() on the GPIO to retrieve the corresponding IRQ number.
> > 
> > On non-DT systems the external IRQs are statically mapped at a known
> > offset. The sh-pfc driver, to implement the gpio_to_irq() function
> > (through its gpiochip .to_irq() handler), simply searches a
> > SoC-specific lookup table for the fixed IRQ number associated with a
> > given GPIO.
> > 
> > However, on DT systems, IRQs are mapped dynamically on demand. The
> > irqpin driver registers a simple IRQ domain, and the
> > irq_create_mapping() function can then be used to map a given IRQ,
> > specified as an offset in the domain. This is where the problem
> > appears, as the irqchip .to_irq() function is implemented in the sh-pfc
> 
> I assume it should be s/irqchip/gpiochip/ in the line above, shouldn't it?

Yes, my bad.

> > driver, which doesn't have access to the IRQ domain registered by the
> > irqpin driver.
> > 
> > I could hack around this by exporting a function in the irqpin driver
> > that would map an IRQ, and call that function from the sh-pfc driver.
> > I'd rather avoid that solution as it would add a direct dependency
> > between the two drivers.
> 
> If you could just get the IRQ domain registered by irqpin driver and use
> it in sh-pfc, then I guess it would solve your problem, as you could
> simply call irq_create_mapping() with the domain and hwirq as args in your
> gpiochip .to_irq() callback.
>
> I'm not sure if it's not a hack, but you could add a property to the node
> of your pin controller that would contain a phandle to your interrupt
> controller. Then you could use of_parse_phandle() to get to device node of
> the INTC and then irq_find_host() to retrieve irq domain associated with
> it.

That was my initial idea. However, one on of the SoCs, the GPIO interrupts are 
divided in two separate blocks, handled by two different interrupt controller 
instances. I could thus have a list of phandle + range, but that becomes 
pretty hackish. Specifying the interrupts explicitly would be more extensible.

> > Has anyone run into a similar issue ? My gut feeling is that the
> > architecture isn't right somewhere, but I can't really pinpoint where.
> > As the external IRQs are handled by an IP core separate from the PFC
> 
> Well, the fact that it's separate doesn't mean anything yet. Here my
> question whether it's used exclusively for GPIO interrupts or not becomes
> significant. If yes, maybe it could be simply moved to the pinctrl driver?

Depending on the SoC, I have two different IRQ controllers used for GPIO 
interrupts. They're called INTC and IRQC. INTC has other purposes (although 
not implemented at the moment). The IRQC instances used for GPIO interrupts 
are (at the moment) dedicated to GPIO interrupts, but other instances of the 
same IP core are used for other interrupts, so a separate driver makes sense 
in my opinion.

-- 
Regards,

Laurent Pinchart

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hi Grant,

On Saturday 27 July 2013 23:00:21 Grant Likely wrote:
> On Thu, 25 Jul 2013 15:22:29 +0200, Laurent Pinchart wrote:
> > On Thursday 25 July 2013 14:15:56 Mark Brown wrote:
> > > On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:
> > > > The two devices are independent, so there's no real parent/child
> > > > relationship. However, as Grant proposed, I could list all the
> > > > interrupts associated with GPIOs in the GPIO controller DT node. I
> > > > would then just call irq_of_parse_and_map() in the .to_irq() handler
> > > > to magically translate the GPIO number to a mapped IRQ number.
> > > > 
> > > > The number of interrupts can be pretty high (up to 58 in the worst
> > > > case so far), so an alternative would be to specify the interrupt-
> > > > parent only, and call irq_create_of_mapping() directly. What solution
> > > > would you prefer ?
> > > 
> > > Are the interrupts in a contiguous block in the controller so you can
> > > just pass around the controller and a base number?
> > 
> > In two of the three SoCs I need to fix they are. I've just realized that
> > in the last one the interrupts are in two contiguous blocks in two
> > different parents. I will thus need at least a list of <parent-phandle
> > base count>.
> >
> > Our standard interrupt bindings don't seem to support multiple parents,
>
> You can actually do it by using a dummy node with interrupt-map and
> interrupt-map-mask properties, but it is a pretty ugly solution in my
> opinion.
> 
> > is that something that we want to fix or should I go for custom bindings ?
> 
> Yes, I think it is something that we want to fix. Jean-Christophe was going
> to propose an alternative to the interrupts property which allows an array
> of <phandle interrupt-specifier> tuples, but I've not seen anything yet. Go
> ahead and make a proposal.

More work, great :-)

A bit of bikeshedding here, as the "interrupts" property is already used, how 
should I name the new property ?

> You could try to encode a base+count variant, but honestly I don't think it
> would be a good idea because it only would work with a very narrow set of
> use cases. Consider if #interrupt-cells was set to 2. Which cell gets
> incremented in the range of interrupts specified? Better I think to merely
> have an array of fully specified irqs. Support for that property could be
> transparently baked into the core interrupt parsing functions.

I agree, I'll try that.

-- 
Regards,

Laurent Pinchart

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Laurent Pinchart]

Hi Grant,

On Saturday 27 July 2013 23:00:21 Grant Likely wrote:
> On Thu, 25 Jul 2013 15:22:29 +0200, Laurent Pinchart wrote:
> > On Thursday 25 July 2013 14:15:56 Mark Brown wrote:
> > > On Thu, Jul 25, 2013 at 11:45:33AM +0200, Laurent Pinchart wrote:
> > > > The two devices are independent, so there's no real parent/child
> > > > relationship. However, as Grant proposed, I could list all the
> > > > interrupts associated with GPIOs in the GPIO controller DT node. I
> > > > would then just call irq_of_parse_and_map() in the .to_irq() handler
> > > > to magically translate the GPIO number to a mapped IRQ number.
> > > > 
> > > > The number of interrupts can be pretty high (up to 58 in the worst
> > > > case so far), so an alternative would be to specify the interrupt-
> > > > parent only, and call irq_create_of_mapping() directly. What solution
> > > > would you prefer ?
> > > 
> > > Are the interrupts in a contiguous block in the controller so you can
> > > just pass around the controller and a base number?
> > 
> > In two of the three SoCs I need to fix they are. I've just realized that
> > in the last one the interrupts are in two contiguous blocks in two
> > different parents. I will thus need at least a list of <parent-phandle
> > base count>.
> >
> > Our standard interrupt bindings don't seem to support multiple parents,
>
> You can actually do it by using a dummy node with interrupt-map and
> interrupt-map-mask properties, but it is a pretty ugly solution in my
> opinion.
> 
> > is that something that we want to fix or should I go for custom bindings ?
> 
> Yes, I think it is something that we want to fix. Jean-Christophe was going
> to propose an alternative to the interrupts property which allows an array
> of <phandle interrupt-specifier> tuples, but I've not seen anything yet. Go
> ahead and make a proposal.

More work, great :-)

A bit of bikeshedding here, as the "interrupts" property is already used, how 
should I name the new property ?

> You could try to encode a base+count variant, but honestly I don't think it
> would be a good idea because it only would work with a very narrow set of
> use cases. Consider if #interrupt-cells was set to 2. Which cell gets
> incremented in the range of interrupts specified? Better I think to merely
> have an array of fully specified irqs. Support for that property could be
> transparently baked into the core interrupt parsing functions.

I agree, I'll try that.

-- 
Regards,

Laurent Pinchart

Re: How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Tomasz Figa]

On Wednesday 31 of July 2013 13:11:33 Laurent Pinchart wrote:
> Hi Tomasz,
> 
> On Sunday 28 July 2013 12:07:48 Tomasz Figa wrote:
> > On Wednesday 24 of July 2013 01:21:44 Laurent Pinchart wrote:
> > > Hello,
> > > 
> > > I'm running into an issue on several Renesas SoC with IRQ domains and
> > > GPIOs.
> > > 
> > > On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are
> > > handled by two separate IP cores, namely the PFC (Pin Function
> > > Controller) and INTC (Interrupt Controller). The former is handled by
> > > the sh-pfc driver (drivers/pinctrl/sh-pfc) and the later by the
> > > irq-renesas-intc-irqpin driver (drivers/irqchip), referred to below as
> > > the irqpin driver.
> > 
> > Is the INTC used for anything more than just external interrupts on GPIO
> > lines?
> 
> Yes, it also handles other interrupt sources (NMI and peripherals), but
> those are not implemented now. The peripheral interrupts are also handled
> by the GIC, which is preferred over INTC.

OK, this is much more clear now.

> > > The sh73a0, for instance, has 32 external interrupt lines that are
> > > multiplexed on pins usable as GPIOs. Both the GPIO and external
> > > interrupt functions are usable at the same time, which allows reading
> > > the state of the interrupt lines.
> > > 
> > > These external interrupts are for MMC/SD support, among other devices.
> > > In this specific case the MMC/SD Card Detect signal is wired to one of
> > > the external interrupt signals, and the corresponding GPIO is passed to
> > > the MMC/SD controller driver. Depending on other configuration
> > > parameters the driver can then either poll the Card Detect signal, or
> > > register an interrupt handler to detect changes in the signal state.
> > > This features is implemented by the MMC/SD core, which call
> > > gpio_to_irq() on the GPIO to retrieve the corresponding IRQ number.
> > > 
> > > On non-DT systems the external IRQs are statically mapped at a known
> > > offset. The sh-pfc driver, to implement the gpio_to_irq() function
> > > (through its gpiochip .to_irq() handler), simply searches a
> > > SoC-specific lookup table for the fixed IRQ number associated with a
> > > given GPIO.
> > > 
> > > However, on DT systems, IRQs are mapped dynamically on demand. The
> > > irqpin driver registers a simple IRQ domain, and the
> > > irq_create_mapping() function can then be used to map a given IRQ,
> > > specified as an offset in the domain. This is where the problem
> > > appears, as the irqchip .to_irq() function is implemented in the sh-pfc
> > 
> > I assume it should be s/irqchip/gpiochip/ in the line above, shouldn't it?
> 
> Yes, my bad.

No problem.

> > > driver, which doesn't have access to the IRQ domain registered by the
> > > irqpin driver.
> > > 
> > > I could hack around this by exporting a function in the irqpin driver
> > > that would map an IRQ, and call that function from the sh-pfc driver.
> > > I'd rather avoid that solution as it would add a direct dependency
> > > between the two drivers.
> > 
> > If you could just get the IRQ domain registered by irqpin driver and use
> > it in sh-pfc, then I guess it would solve your problem, as you could
> > simply call irq_create_mapping() with the domain and hwirq as args in your
> > gpiochip .to_irq() callback.
> > 
> > I'm not sure if it's not a hack, but you could add a property to the node
> > of your pin controller that would contain a phandle to your interrupt
> > controller. Then you could use of_parse_phandle() to get to device node of
> > the INTC and then irq_find_host() to retrieve irq domain associated with
> > it.
> 
> That was my initial idea. However, one on of the SoCs, the GPIO interrupts
> are divided in two separate blocks, handled by two different interrupt
> controller instances. I could thus have a list of phandle + range, but that
> becomes pretty hackish. Specifying the interrupts explicitly would be more
> extensible.

Yes, in this case passing the domain alone makes little sense and domain + 
range would be a bit hackish indeed.

> > > Has anyone run into a similar issue ? My gut feeling is that the
> > > architecture isn't right somewhere, but I can't really pinpoint where.
> > > As the external IRQs are handled by an IP core separate from the PFC
> > 
> > Well, the fact that it's separate doesn't mean anything yet. Here my
> > question whether it's used exclusively for GPIO interrupts or not becomes
> > significant. If yes, maybe it could be simply moved to the pinctrl driver?
> 
> Depending on the SoC, I have two different IRQ controllers used for GPIO
> interrupts. They're called INTC and IRQC. INTC has other purposes (although
> not implemented at the moment). The IRQC instances used for GPIO interrupts
> are (at the moment) dedicated to GPIO interrupts, but other instances of the
> same IP core are used for other interrupts, so a separate driver makes
> sense in my opinion.

OK. So you need a bit smarter solution then.

Best regards,
Tomasz

How to create IRQ mappings in a GPIO driver that doesn't control its IRQ domain ?

[Tomasz Figa]

On Wednesday 31 of July 2013 13:11:33 Laurent Pinchart wrote:
> Hi Tomasz,
> 
> On Sunday 28 July 2013 12:07:48 Tomasz Figa wrote:
> > On Wednesday 24 of July 2013 01:21:44 Laurent Pinchart wrote:
> > > Hello,
> > > 
> > > I'm running into an issue on several Renesas SoC with IRQ domains and
> > > GPIOs.
> > > 
> > > On sh73a0, r8a73a4 and r8a7740, GPIOs and external interrupts are
> > > handled by two separate IP cores, namely the PFC (Pin Function
> > > Controller) and INTC (Interrupt Controller). The former is handled by
> > > the sh-pfc driver (drivers/pinctrl/sh-pfc) and the later by the
> > > irq-renesas-intc-irqpin driver (drivers/irqchip), referred to below as
> > > the irqpin driver.
> > 
> > Is the INTC used for anything more than just external interrupts on GPIO
> > lines?
> 
> Yes, it also handles other interrupt sources (NMI and peripherals), but
> those are not implemented now. The peripheral interrupts are also handled
> by the GIC, which is preferred over INTC.

OK, this is much more clear now.

> > > The sh73a0, for instance, has 32 external interrupt lines that are
> > > multiplexed on pins usable as GPIOs. Both the GPIO and external
> > > interrupt functions are usable at the same time, which allows reading
> > > the state of the interrupt lines.
> > > 
> > > These external interrupts are for MMC/SD support, among other devices.
> > > In this specific case the MMC/SD Card Detect signal is wired to one of
> > > the external interrupt signals, and the corresponding GPIO is passed to
> > > the MMC/SD controller driver. Depending on other configuration
> > > parameters the driver can then either poll the Card Detect signal, or
> > > register an interrupt handler to detect changes in the signal state.
> > > This features is implemented by the MMC/SD core, which call
> > > gpio_to_irq() on the GPIO to retrieve the corresponding IRQ number.
> > > 
> > > On non-DT systems the external IRQs are statically mapped at a known
> > > offset. The sh-pfc driver, to implement the gpio_to_irq() function
> > > (through its gpiochip .to_irq() handler), simply searches a
> > > SoC-specific lookup table for the fixed IRQ number associated with a
> > > given GPIO.
> > > 
> > > However, on DT systems, IRQs are mapped dynamically on demand. The
> > > irqpin driver registers a simple IRQ domain, and the
> > > irq_create_mapping() function can then be used to map a given IRQ,
> > > specified as an offset in the domain. This is where the problem
> > > appears, as the irqchip .to_irq() function is implemented in the sh-pfc
> > 
> > I assume it should be s/irqchip/gpiochip/ in the line above, shouldn't it?
> 
> Yes, my bad.

No problem.

> > > driver, which doesn't have access to the IRQ domain registered by the
> > > irqpin driver.
> > > 
> > > I could hack around this by exporting a function in the irqpin driver
> > > that would map an IRQ, and call that function from the sh-pfc driver.
> > > I'd rather avoid that solution as it would add a direct dependency
> > > between the two drivers.
> > 
> > If you could just get the IRQ domain registered by irqpin driver and use
> > it in sh-pfc, then I guess it would solve your problem, as you could
> > simply call irq_create_mapping() with the domain and hwirq as args in your
> > gpiochip .to_irq() callback.
> > 
> > I'm not sure if it's not a hack, but you could add a property to the node
> > of your pin controller that would contain a phandle to your interrupt
> > controller. Then you could use of_parse_phandle() to get to device node of
> > the INTC and then irq_find_host() to retrieve irq domain associated with
> > it.
> 
> That was my initial idea. However, one on of the SoCs, the GPIO interrupts
> are divided in two separate blocks, handled by two different interrupt
> controller instances. I could thus have a list of phandle + range, but that
> becomes pretty hackish. Specifying the interrupts explicitly would be more
> extensible.

Yes, in this case passing the domain alone makes little sense and domain + 
range would be a bit hackish indeed.

> > > Has anyone run into a similar issue ? My gut feeling is that the
> > > architecture isn't right somewhere, but I can't really pinpoint where.
> > > As the external IRQs are handled by an IP core separate from the PFC
> > 
> > Well, the fact that it's separate doesn't mean anything yet. Here my
> > question whether it's used exclusively for GPIO interrupts or not becomes
> > significant. If yes, maybe it could be simply moved to the pinctrl driver?
> 
> Depending on the SoC, I have two different IRQ controllers used for GPIO
> interrupts. They're called INTC and IRQC. INTC has other purposes (although
> not implemented at the moment). The IRQC instances used for GPIO interrupts
> are (at the moment) dedicated to GPIO interrupts, but other instances of the
> same IP core are used for other interrupts, so a separate driver makes
> sense in my opinion.

OK. So you need a bit smarter solution then.

Best regards,
Tomasz