lkml.org 
[lkml]   [2018]   [Mar]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] gpio: dwapb: Add support for 32 interrupts
On Wed, Mar 28, 2018 at 5:22 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> The DesignWare GPIO IP can be configured for either 1 or 32 interrupts,

1 to 32, or just a choice between two?

> but the driver currently only supports 1 interrupt. See the DesignWare
> DW_apb_gpio Databook description of the 'GPIO_INTR_IO' parameter.

Will see after holiday and perhaps make more comments. Here is just a
brief review.

> +- interrupts : The interrupts to the parent controller raised when GPIOs
> + generate the interrupts. If the controller provides one combined interrupt
> + for all GPIOs, specify a single interrupt. If the controller provides one
> + interrupt for each GPIO, provide a list of interrupts that correspond to each
> + of the GPIO pins. When specifying multiple interrupts, if any of the GPIOs are
> + not connected to an interrupt, use the interrupt-mask property.
> +- interrupt-mask : a 32-bit bit mask that specifies which interrupts in the list
> + of interrupts is valid, bit is 1 for a valid irq.

So, but why one will need that in practice? GPIO driver usually
provides a pin based IRQ chip which maps each pin to the corresponding
offset inside specific IRQ domain.

> + struct device_node *np = to_of_node(fwnode);
> + u32 irq_mask = 0xFFFFFFFF;

Why? Shouldn't it be dependent to the amount of actual pins / ports?
Intel Quark has only 8 AFAIR.

> + int j;
> +
> + /* Optional irq mask */
> + fwnode_property_read_u32(fwnode, "interrupt-mask", &irq_mask);
> +
> + /*
> + * The IP has configuration options to allow a single
> + * combined interrupt or one per gpio. If one per gpio,
> + * some might not be used.
> + */

> + for (j = 0; j < pp->ngpio; j++) {
> + if (irq_mask & BIT(j)) {

for_each_set_bit() is in kernel for ages!

> + pp->irq[j] = irq_of_parse_and_map(np, j);
> + if (pp->irq[j])
> + pp->has_irq = true;
> + }
> + }


So, on the first glance the patch looks either superfluous or taking
wrong approach. Please, elaborate more why it's done in this way and
what the case for all this in practice.

--
With Best Regards,
Andy Shevchenko

\
 
 \ /
  Last update: 2018-03-30 23:27    [W:0.231 / U:0.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site