On 09/28/2017 04:56 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> Some GPIO controllers are subdivided into multiple logical blocks called
> banks (or ports). This is often caused by the design assigning separate
> resources, such as register regions or interrupts, to each bank, or some
> set of banks.
>
> This commit adds support for describing controllers that have such a
> banked design and provides common code for dealing with them.
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> drivers/gpio/gpiolib-of.c | 101 +++++++++++++++++++++++++++++++++++++++++
> drivers/gpio/gpiolib.c | 98 ++++++++++++++++++++++++++++++++++++++++
> include/linux/gpio/driver.h | 108 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of_gpio.h | 10 ++++
> 4 files changed, 317 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index bfcd20699ec8..9baabe00966d 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -309,6 +309,107 @@ int of_gpio_simple_xlate(struct gpio_chip *gc,
> }
> EXPORT_SYMBOL(of_gpio_simple_xlate);
>
> +/**
> + * gpio_banked_irq_domain_xlate - decode an IRQ specifier for banked chips
> + * @domain: IRQ domain
> + * @np: device tree node
> + * @spec: IRQ specifier
> + * @size: number of cells in IRQ specifier
> + * @hwirq: return location for the hardware IRQ number
> + * @type: return location for the IRQ type
> + *
> + * Translates the IRQ specifier found in device tree into a hardware IRQ
> + * number and an interrupt type.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int gpio_banked_irq_domain_xlate(struct irq_domain *domain,
> + struct device_node *np,
> + const u32 *spec, unsigned int size,
> + unsigned long *hwirq,
> + unsigned int *type)
> +{
> + struct gpio_chip *gc = domain->host_data;
> + unsigned int bank, line, i, offset = 0;
> +
> + if (size < 2)
> + return -EINVAL;
> +
> + bank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;
> + line = (spec[0] >> gc->of_gpio_line_mask) & gc->of_gpio_line_shift;
> +
> + if (bank >= gc->num_banks) {
> + dev_err(gc->parent, "invalid bank number: %u\n", bank);
> + return -EINVAL;
> + }
> +
> + if (line >= gc->banks[bank]->num_lines) {
> + dev_err(gc->parent, "invalid line number: %u\n", line);
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < bank; i++)
> + offset += gc->banks[i]->num_lines;
Just to clarify, why is above iteration required?
> +
> + *type = spec[1] & IRQ_TYPE_SENSE_MASK;
> + *hwirq = offset + line;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(gpio_banked_irq_domain_xlate);
> +
> +/**
> + * of_gpio_banked_xlate - translate GPIO specifier to a GPIO number and flags
> + * @gc: GPIO chip
> + * @gpiospec: GPIO specifier
> + * @flags: return location for flags parsed from the GPIO specifier
> + *
> + * This translation function takes into account multiple banks that can make
> + * up a single controller. Each bank can contain one or more pins. A single
> + * cell in the specifier is used to represent a (bank, pin) pair, with each
> + * encoded in different fields. The &gpio_chip.of_gpio_bank_shift and
> + * &gpio_chip.of_gpio_bank_mask fields, and &gpio_chip.of_gpio_line_shift and
> + * &gpio_chip.of_gpio_line_mask are used to specify the encoding.
> + *
> + * Returns:
> + * The chip-relative index of the pin given by the GPIO specifier.
> + */
> +int of_gpio_banked_xlate(struct gpio_chip *gc,
> + const struct of_phandle_args *gpiospec, u32 *flags)
> +{
> + unsigned int offset = 0, bank, line, i;
> + const u32 *spec = gpiospec->args;
> +
> + if (WARN_ON(gc->of_gpio_n_cells < 2))
> + return -EINVAL;
> +
> + if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
> + return -EINVAL;
> +
> + bank = (spec[0] >> gc->of_gpio_bank_shift) & gc->of_gpio_bank_mask;
> + line = (spec[0] >> gc->of_gpio_line_shift) & gc->of_gpio_line_mask;
> +
> + if (bank >= gc->num_banks) {
> + dev_err(gc->parent, "invalid bank number: %u\n", bank);
> + return -EINVAL;
> + }
> +
> + if (line >= gc->banks[bank]->num_lines) {
> + dev_err(gc->parent, "invalid line number: %u\n", line);
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < bank; i++)
> + offset += gc->banks[i]->num_lines;
> +
> + if (flags)
> + *flags = spec[1];
> +
> + return offset + line;
> +}
> +EXPORT_SYMBOL(of_gpio_banked_xlate);
Adding above two functions means adding new GPIO bindings (may be optional,
but common).
> +
> /**
> * of_mm_gpiochip_add_data - Add memory mapped GPIO chip (bank)
> * @np: device node of the GPIO chip
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index c15fb858848a..b3bd19b793d3 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1765,6 +1765,57 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip)
> gpiochip->to_irq = gpiochip_to_irq;
> gpiochip->irq.default_type = type;
>
> + if (gpiochip->num_banks > 0 && !gpiochip->irq.map) {
> + struct gpio_irq_chip *irq = &gpiochip->irq;
> + unsigned int i, j, offset = 0;
> +
> + if (!irq->parents) {
> + chip_err(gpiochip, "no parent interrupts defined\n");
> + return -EINVAL;
> + }
> +
> + irq->map = devm_kcalloc(gpiochip->parent, gpiochip->ngpio,
> + sizeof(*irq->map), GFP_KERNEL);
> + if (!irq->map)
> + return -ENOMEM;
> +
> + for (i = 0; i < gpiochip->num_banks; i++) {
> + struct gpio_bank *bank = gpiochip->banks[i];
> + unsigned int parent = bank->parent_irq;
> +
> + for (j = 0; j < bank->num_lines; j++) {
> + if (parent >= irq->num_parents) {
> + chip_err(gpiochip,
> + "invalid parent interrupt: %u\n",
> + parent);
> + return -EINVAL;
> + }
> +
> + irq->map[offset + j] = irq->parents[parent];
> + }
> +
> + offset += bank->num_lines;
Most of gpio drivers, you've listed in [1], have only one parent
(waste of memory). There should be way not to store it permanently.
> + }
> + }
> +
> + if (gpiochip->num_banks > 0) {
> + unsigned int i;
> +
> + for (i = 0; i < gpiochip->num_banks; i++) {
> + struct gpio_bank *bank = gpiochip->banks[i];
> + unsigned int num_lines = bank->num_lines;
> +
> + bank->pending = devm_kcalloc(gpiochip->parent,
> + BITS_TO_LONGS(num_lines),
> + sizeof(unsigned long),
> + GFP_KERNEL);
> + if (!bank->pending)
> + return -ENOMEM;
> +
> + bank->chip = gpiochip;
> + }
> + }
> +
> if (gpiochip->irq.domain_ops)
> ops = gpiochip->irq.domain_ops;
> else
> @@ -1973,6 +2024,53 @@ int gpiochip_irqchip_add_key(struct gpio_chip *gpiochip,
> }
> EXPORT_SYMBOL_GPL(gpiochip_irqchip_add_key);
>
> +/**
> + * gpio_irq_chip_banked_chained_handler - interrupt handler for banked IRQ chips
> + * @desc: IRQ descriptor
> + *
> + * Drivers can use this interrupt handler for banked GPIO controllers. This
> + * implementation iterates over all banks and handles pending interrupts of
> + * the pins associated with the bank.
> + *
> + * This function uses driver specific parts, split out into the
> + * &gpio_chip.update_bank() callback, to retrieves the interrupt pending
> + * state for each of the GPIOs exposed by the given bank.
> + */
> +void gpio_irq_chip_banked_chained_handler(struct irq_desc *desc)
> +{
> + struct gpio_chip *gpio = irq_desc_get_handler_data(desc);
As per Patch 1 - there are no restriction to use parent_handler_data with
this standard handler - in this case parent_handler_data might not be struct gpio_chip.
> + struct irq_chip *irq = irq_desc_get_chip(desc);
> + unsigned int parent = irq_desc_get_irq(desc);
> + struct gpio_irq_chip *chip = &gpio->irq;
> + unsigned int i, offset = 0;
> +
> + chained_irq_enter(irq, desc);
> +
> + for (i = 0; i < gpio->num_banks; i++) {
> + struct gpio_bank *bank = gpio->banks[i];
> + unsigned int line, virq;
> +
> + if (parent != chip->parents[bank->parent_irq])
> + goto skip;
You've used this handler in gpio-tegra.c.
So for compatible = "nvidia,tegra20-gpio":
- there are will be 7 parent irqs/banks.
- gpiochip will be used as chained_handler data.
So, how will it work:
- for bank0 it will take 1 iteration to get correct bank structure
- but for bank7 - 7 iteration always (in hot path?)
> +
> + chip->update_bank(bank);
Half of gpio drivers, you've listed in [1] required access to common
Interrupt status registers before proceeding to banks.
> +
> + for_each_set_bit(line, bank->pending, bank->num_lines) {
> + virq = irq_find_mapping(chip->domain, offset + line);
> + if (WARN_ON(virq == 0))
> + continue;
drivers might require to do additional action before/after generic_handle_irq()
(intel_mid_irq_handler())
> +
> + generic_handle_irq(virq);
> + }
> +
> +skip:
> + offset += bank->num_lines;
> + }
> +
> + chained_irq_exit(irq, desc);
> +}
> +EXPORT_SYMBOL_GPL(gpio_irq_chip_banked_chained_handler);
chained IRQ handler is not RT friendly (no control from User space)
> +
> #else /* CONFIG_GPIOLIB_IRQCHIP */
>
> static inline int gpiochip_add_irqchip(struct gpio_chip *gpiochip)
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index c453e0716228..3caa08b3d2b6 100644
[...]
>
> /**
>
[1] https://www.spinics.net/lists/linux-tegra/msg31105.html
--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
