Linux GPIO subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH v1 1/1] pinctrl: stm32: Unshadow np variable in stm32_pctl_probe()
From: Andy Shevchenko @ 2022-05-09  9:27 UTC (permalink / raw)
  To: Fabien DESSENNE
  Cc: Linus Walleij, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel, Maxime Coquelin, Alexandre Torgue,
	kernel test robot
In-Reply-To: <30a2c669-693f-45ec-df0e-610e1f94bffd@foss.st.com>

On Mon, May 09, 2022 at 09:39:57AM +0200, Fabien DESSENNE wrote:
> Hi Andy
> 
> 
> Thank you for the patch.
> 
> Will this patch be applied in Linus pinctrl tree, or in the gpio-intel tree
> before being merged (linux-next) in the pinctrl tree?

I believe the pin control tree is a primary here, while applying to GPIO tree
won't harm. But I maintain neither of them :-)

> On 07/05/2022 12:22, Andy Shevchenko wrote:
> > The np variable is used globally for stm32_pctl_probe() and in one of
> > its code branches. cppcheck is not happy with that:
> > 
> >    pinctrl-stm32.c:1530:23: warning: Local variable 'np' shadows outer variable [shadowVariable]
> > 
> > Instead of simply renaming one of the variables convert some code to
> > use a device pointer directly.
> > 
> > Fixes: bb949ed9b16b ("pinctrl: stm32: Switch to use for_each_gpiochip_node() helper")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Reviewed-by: Fabien Dessenne <fabien.dessenne@foss.st.com>

Thanks!

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v6 00/23] Rust support
From: Wei Liu @ 2022-05-09  9:39 UTC (permalink / raw)
  To: Kees Cook
  Cc: Miguel Ojeda, Linus Torvalds, Greg Kroah-Hartman, rust-for-linux,
	linux-kernel, Jarkko Sakkinen, kunit-dev, linux-arm-kernel,
	linux-doc, linux-gpio, linux-kbuild, linux-kselftest,
	linux-perf-users, linuxppc-dev, linux-riscv, live-patching,
	Wei Liu
In-Reply-To: <202205070056.ACC3C3D@keescook>

On Sat, May 07, 2022 at 01:06:18AM -0700, Kees Cook wrote:
> On Sat, May 07, 2022 at 07:23:58AM +0200, Miguel Ojeda wrote:
> > ## Patch series status
> > 
> > The Rust support is still to be considered experimental. However,
> > support is good enough that kernel developers can start working on the
> > Rust abstractions for subsystems and write drivers and other modules.
> 
> I'd really like to see this landed for a few reasons:
> 
> - It's under active development, and I'd rather review the changes
>   "normally", incrementally, etc. Right now it can be hard to re-review
>   some of the "mostly the same each version" patches in the series.
> 
> - I'd like to break the catch-22 of "ask for a new driver to be
>   written in rust but the rust support isn't landed" vs "the rust
>   support isn't landed because there aren't enough drivers". It
>   really feels like "release early, release often" is needed here;
>   it's hard to develop against -next. :)

+1 to both points. :-)

^ permalink raw reply

* Re: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
From: Geert Uytterhoeven @ 2022-05-09  9:10 UTC (permalink / raw)
  To: Biju Das
  Cc: Prabhakar Mahadev Lad, Thomas Gleixner, Marc Zyngier, Rob Herring,
	Krzysztof Kozlowski, Linus Walleij, Bartosz Golaszewski,
	Geert Uytterhoeven, Philipp Zabel, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, Prabhakar
In-Reply-To: <OS0PR01MB5922B58BB70B92813041745786C69@OS0PR01MB5922.jpnprd01.prod.outlook.com>

On Mon, May 9, 2022 at 9:22 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller
> > driver
> >
> > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> >
> > This supports external pins being used as interrupts. It supports one line
> > for NMI, 8 external pins and 32 GPIO pins (out of 123) to be used as IRQ
> > lines.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> > --- /dev/null
> > +++ b/drivers/irqchip/irq-renesas-rzg2l.c

> > +static void rzg2l_irqc_irq_disable(struct irq_data *d) {
> > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > +
> > +     if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> > +             struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +             u32 offset = hw_irq - IRQC_TINT_START;
> > +             u32 tssr_offset = TSSR_OFFSET(offset);
> > +             u8 tssr_index = TSSR_INDEX(offset);
> > +             u32 reg;
> > +
> > +             raw_spin_lock(&priv->lock);
> > +             reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > +             reg &= ~(TSSEL_MASK << tssr_offset);
> > +             writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > +             raw_spin_unlock(&priv->lock);
> > +     }
> > +     irq_chip_disable_parent(d);
> > +}

> > +static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type) {
> > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +     unsigned int hwirq = irqd_to_hwirq(d);
> > +     u32 titseln = hwirq - IRQC_TINT_START;
> > +     u32 offset;
> > +     u8 sense;
> > +     u32 reg;
> > +
> > +     switch (type & IRQ_TYPE_SENSE_MASK) {
> > +     case IRQ_TYPE_EDGE_RISING:
> > +             sense = TITSR_TITSEL_EDGE_RISING;
> > +             break;
> > +
> > +     case IRQ_TYPE_EDGE_FALLING:
> > +             sense = TITSR_TITSEL_EDGE_FALLING;
> > +             break;
> > +
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
>
> > +     if (titseln < TITSR0_MAX_INT) {
> > +             offset = TITSR0;
> > +     } else {
> > +             titseln /= TITSEL_WIDTH;
> > +             offset  = TITSR1;
> > +     }
>
> as TITSR0 (0x24) and TITSR1(0x28) are contiguous address location
>
> May be like others, above declare it as
> u32 offset = TITSR0; ??
>
> and here
>  if ((titseln >= TITSR0_MAX_INT) {
>         titseln /= TITSEL_WIDTH;
>         offset  += 4;
>  }

Why "titseln /= TITSEL_WIDTH"?
Shouldn't that be "titseln -= TITSR0_MAX_INT"?
Do I need more coffee?

Can't you define TITSR_{OFFSET,INDEX}() helper macros, like for
TSSR above?

> > +
> > +     raw_spin_lock(&priv->lock);
> > +     reg = readl_relaxed(priv->base + offset);
> > +     reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
> > +     reg |= sense << (titseln * TITSEL_WIDTH);
> > +     writel_relaxed(reg, priv->base + offset);
> > +     raw_spin_unlock(&priv->lock);
> > +
> > +     return 0;
> > +}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 1/5] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller
From: Geert Uytterhoeven @ 2022-05-09  8:57 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Philipp Zabel, open list:GPIO SUBSYSTEM,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-Renesas, Prabhakar, Biju Das
In-Reply-To: <20220509050953.11005-2-prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi Prabhakar,

On Mon, May 9, 2022 at 7:10 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add DT bindings for the Renesas RZ/G2L Interrupt Controller.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
> @@ -0,0 +1,131 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
> +
> +maintainers:
> +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> +  - Geert Uytterhoeven <geert+renesas@glider.be>
> +
> +description: |
> +  IA55 performs various interrupt controls including synchronization for the external
> +  interrupts of NMI, IRQ, and GPIOINT and the interrupts of the built-in peripheral
> +  interrupts output by each IP. And it notifies the interrupt to the GIC
> +    - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
> +    - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
> +    - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
> +      stand-up edge detection interrupts)
> +
> +allOf:
> +  - $ref: /schemas/interrupt-controller.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - renesas,r9a07g044-irqc    # RZ/G2L
> +      - const: renesas,rzg2l-irqc
> +
> +  '#interrupt-cells':
> +    const: 2

Please document the meaning of the cells.

The rest LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [RFC v8 net-next 06/16] pinctrl: microchip-sgpio: add ability to be used in a non-mmio configuration
From: Andy Shevchenko @ 2022-05-09  8:44 UTC (permalink / raw)
  To: Colin Foster
  Cc: linux-arm-kernel, linux-gpio, netdev, Terry Bowman, Wolfram Sang,
	Steen Hegelund, Lars Povlsen, Linus Walleij, Russell King,
	Heiner Kallweit, Paolo Abeni, Jakub Kicinski, Eric Dumazet,
	David S. Miller, Florian Fainelli, Vivien Didelot, Andrew Lunn,
	UNGLinuxDriver, Alexandre Belloni, Claudiu Manoil,
	Vladimir Oltean, Lee Jones
In-Reply-To: <20220508185313.2222956-7-colin.foster@in-advantage.com>

On Sun, May 8, 2022 at 8:53 PM Colin Foster
<colin.foster@in-advantage.com> wrote:
>
> There are a few Ocelot chips that can contain SGPIO logic, but can be
> controlled externally. Specifically the VSC7511, 7512, 7513, and 7514. In
> the externally controlled configurations these registers are not
> memory-mapped.
>
> Add support for these non-memory-mapped configurations.

...

> -       regs = devm_platform_ioremap_resource(pdev, 0);
> -       if (IS_ERR(regs))
> -               return PTR_ERR(regs);
> +       regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
> +       if (IS_ERR(regs)) {
> +               /*
> +                * Fall back to using IORESOURCE_REG, which is possible in an
> +                * MFD configuration
> +                */
> +               res = platform_get_resource(pdev, IORESOURCE_REG, 0);
> +               if (!res) {
> +                       dev_err(dev, "Failed to get resource\n");
> +                       return -ENODEV;
> +               }
> +
> +               priv->regs = ocelot_init_regmap_from_resource(dev, res);
> +       } else {
> +               priv->regs = devm_regmap_init_mmio(dev, regs, &regmap_config);
> +       }
>
> -       priv->regs = devm_regmap_init_mmio(dev, regs, &regmap_config);
>         if (IS_ERR(priv->regs))
>                 return PTR_ERR(priv->regs);

This looks like repetition of something you have done in a few
previous patches. Can you avoid code duplication by introducing a
corresponding helper function?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
From: Marc Zyngier @ 2022-05-09  9:09 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Lukas Wunner, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	nickhu, palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will
In-Reply-To: <YnjWvbzn8ox+f2Y2@FVFF77S0Q05N>

On Mon, 09 May 2022 09:54:21 +0100,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> On Fri, May 06, 2022 at 10:32:42PM +0200, Lukas Wunner wrote:
> > On Tue, Oct 26, 2021 at 10:25:04AM +0100, Mark Rutland wrote:
> > > Now that entry code handles IRQ entry (including setting the IRQ regs)
> > > before calling irqchip code, irqchip code can safely call
> > > generic_handle_domain_irq(), and there's no functional reason for it to
> > > call handle_domain_irq().
> > > 
> > > Let's cement this split of responsibility and remove handle_domain_irq()
> > > entirely, updating irqchip drivers to call generic_handle_domain_irq().
> > > 
> > > For consistency, handle_domain_nmi() is similarly removed and replaced
> > > with a generic_handle_domain_nmi() function which also does not perform
> > > any entry logic.
> > > 
> > > Previously handle_domain_{irq,nmi}() had a WARN_ON() which would fire
> > > when they were called in an inappropriate context. So that we can
> > > identify similar issues going forward, similar WARN_ON_ONCE() logic is
> > > added to the generic_handle_*() functions, and comments are updated for
> > > clarity and consistency.
> > [...]
> > >  int generic_handle_domain_irq(struct irq_domain *domain, unsigned int hwirq)
> > >  {
> > > +	WARN_ON_ONCE(!in_irq());
> > >  	return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
> > >  }
> > >  EXPORT_SYMBOL_GPL(generic_handle_domain_irq);
> > 
> > Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
> > (See handle_irq_desc() and c16816acd086.)
> 
> I did this for consistency with the in_nmi() check in
> generic_handle_domain_nmi(); I was unaware of commit c16816acd086 and
> IRQD_HANDLE_ENFORCE_IRQCTX.
> 
> I'll have ot leave it to Marc and Thomas as to what we should do there.

My preference would be to not introduce things that result in
different behaviours for drivers, specially for things that are
evidently cross-architecture such as USB drivers (which seems to be
the case here).

I'd rather do something that allows these to be handled in the right
context such as a self-IPI. This would certainly work for the GIC. No
idea whether this is valid for x86, which is the other user.

Thanks,

	M.


-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply

* Re: [RFC v8 net-next 08/16] mfd: ocelot: add support for the vsc7512 chip via spi
From: Andy Shevchenko @ 2022-05-09  9:02 UTC (permalink / raw)
  To: Colin Foster
  Cc: linux-arm-kernel, linux-gpio, netdev, Terry Bowman, Wolfram Sang,
	Steen Hegelund, Lars Povlsen, Linus Walleij, Russell King,
	Heiner Kallweit, Paolo Abeni, Jakub Kicinski, Eric Dumazet,
	David S. Miller, Florian Fainelli, Vivien Didelot, Andrew Lunn,
	UNGLinuxDriver, Alexandre Belloni, Claudiu Manoil,
	Vladimir Oltean, Lee Jones
In-Reply-To: <20220508185313.2222956-9-colin.foster@in-advantage.com>

On Sun, May 8, 2022 at 8:53 PM Colin Foster
<colin.foster@in-advantage.com> wrote:
>
> The VSC7512 is a networking chip that contains several peripherals. Many of
> these peripherals are currently supported by the VSC7513 and VSC7514 chips,
> but those run on an internal CPU. The VSC7512 lacks this CPU, and must be
> controlled externally.
>
> Utilize the existing drivers by referencing the chip as an MFD. Add support
> for the two MDIO buses, the internal phys, pinctrl, and serial GPIO.

...

> +         If unsure, say N

Seems like an abrupt sentence.

...

> +EXPORT_SYMBOL(ocelot_chip_reset);

Please, switch to the namespace (_NS) variant of the exported-imported
symbols for these drivers.

...

> +int ocelot_core_init(struct device *dev)
> +{
> +       int ret;
> +
> +       ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, vsc7512_devs,
> +                                  ARRAY_SIZE(vsc7512_devs), NULL, 0, NULL);
> +       if (ret) {
> +               dev_err(dev, "Failed to add sub-devices: %d\n", ret);
> +               return ret;
> +       }
> +
> +       return 0;

Isn't it simple

  return devm_mfd_add_devices(...);

?

> +}

...

> +#include <linux/of.h>

Do you really use this? (See also below).

...

> +#define VSC7512_CPUORG_RES_START       0x71000000
> +#define VSC7512_CPUORG_RES_SIZE                0x2ff

Doesn't look right.
I'm expecting to see 0x300 here and -1 where it's needed in the code.

...

> +static const struct regmap_config ocelot_spi_regmap_config = {
> +       .reg_bits = 24,
> +       .reg_stride = 4,
> +       .reg_downshift = 2,
> +       .val_bits = 32,
> +
> +       .write_flag_mask = 0x80,

> +       .max_register = 0xffffffff,

Is it for real?! Have you considered what happens if someone actually
tries to read all these registers (and for the record it's not a
theoretical case, since the user may do it via debugfs)?

> +       .use_single_write = true,
> +       .can_multi_write = false,
> +
> +       .reg_format_endian = REGMAP_ENDIAN_BIG,
> +       .val_format_endian = REGMAP_ENDIAN_NATIVE,
> +};

...

> +       if (ddata->spi_padding_bytes > 0) {

' > 0' part is redundant.

> +               memset(&padding, 0, sizeof(padding));
> +
> +               padding.len = ddata->spi_padding_bytes;
> +               padding.tx_buf = dummy_buf;
> +               padding.dummy_data = 1;
> +
> +               spi_message_add_tail(&padding, &msg);
> +       }

...

> +       memcpy(&regmap_config, &ocelot_spi_regmap_config,
> +              sizeof(ocelot_spi_regmap_config));

sizeof(regmap_config) is a bit safer (in case somebody changes the
types of the src and dst).

...

> +       err = spi_setup(spi);
> +       if (err < 0) {
> +               dev_err(&spi->dev, "Error %d initializing SPI\n", err);
> +               return err;

return dev_err_probe(...);

> +       }
...

> +       ddata->cpuorg_regmap =
> +               ocelot_spi_devm_init_regmap(dev, dev,
> +                                           &vsc7512_dev_cpuorg_resource);

It's easier to read when it's a longer line. At least the last two can
be on one line.

...

> +       ddata->gcb_regmap = ocelot_spi_devm_init_regmap(dev, dev,
> +                                                       &vsc7512_gcb_resource);

Do you have different cases for two first parameters? If not, drop duplication.

...

> +       if (err) {
> +               dev_err(dev, "Error %d initializing Ocelot SPI bus\n", err);
> +               return err;

return dev_err_probe(...);

And everywhere else where it's appropriate.

> +       }

...

> +const struct of_device_id ocelot_spi_of_match[] = {
> +       { .compatible = "mscc,vsc7512_mfd_spi" },
> +       { },

No comma for terminator entry.

> +};

...

> +               .of_match_table = of_match_ptr(ocelot_spi_of_match),

of_match_ptr() is rather harmful than useful. We have a lot of
compiler warnings due to misuse of this. Besides that it prevents to
use driver in non-OF environments (if it is / will be the case).

...

> +/*
> + * Copyright 2021 Innovative Advantage Inc.
> + */

One line.

...

> +#include <linux/regmap.h>

I don't see the users of this. Use forward declarations (many of them
are actually missed).

Also, seems kconfig.h, err.h and errno.h missed.

> +#include <asm/byteorder.h>

> +struct ocelot_ddata {
> +       struct device *dev;
> +       struct regmap *gcb_regmap;
> +       struct regmap *cpuorg_regmap;
> +       int spi_padding_bytes;
> +       struct spi_device *spi;
> +};

...

> +#if IS_ENABLED(CONFIG_MFD_OCELOT)
> +struct regmap *ocelot_init_regmap_from_resource(struct device *child,
> +                                               const struct resource *res);
> +#else
>  static inline struct regmap *
>  ocelot_init_regmap_from_resource(struct device *child,
>                                  const struct resource *res)
>  {
>         return ERR_PTR(-EOPNOTSUPP);
>  }

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 17/17] irq: remove handle_domain_{irq,nmi}()
From: Mark Rutland @ 2022-05-09  8:54 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: maz, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	Octavian Purdila, linux-kernel, aou, catalin.marinas, deanbo422,
	green.hu, guoren, jonas, kernelfans, linux-arm-kernel, linux,
	nickhu, palmer, paul.walmsley, shorne, stefan.kristiansson, tglx,
	tsbogend, vgupta, vladimir.murzin, will
In-Reply-To: <20220506203242.GA1855@wunner.de>

On Fri, May 06, 2022 at 10:32:42PM +0200, Lukas Wunner wrote:
> On Tue, Oct 26, 2021 at 10:25:04AM +0100, Mark Rutland wrote:
> > Now that entry code handles IRQ entry (including setting the IRQ regs)
> > before calling irqchip code, irqchip code can safely call
> > generic_handle_domain_irq(), and there's no functional reason for it to
> > call handle_domain_irq().
> > 
> > Let's cement this split of responsibility and remove handle_domain_irq()
> > entirely, updating irqchip drivers to call generic_handle_domain_irq().
> > 
> > For consistency, handle_domain_nmi() is similarly removed and replaced
> > with a generic_handle_domain_nmi() function which also does not perform
> > any entry logic.
> > 
> > Previously handle_domain_{irq,nmi}() had a WARN_ON() which would fire
> > when they were called in an inappropriate context. So that we can
> > identify similar issues going forward, similar WARN_ON_ONCE() logic is
> > added to the generic_handle_*() functions, and comments are updated for
> > clarity and consistency.
> [...]
> >  int generic_handle_domain_irq(struct irq_domain *domain, unsigned int hwirq)
> >  {
> > +	WARN_ON_ONCE(!in_irq());
> >  	return handle_irq_desc(irq_resolve_mapping(domain, hwirq));
> >  }
> >  EXPORT_SYMBOL_GPL(generic_handle_domain_irq);
> 
> Why isn't the WARN_ON_ONCE() conditional on handle_enforce_irqctx()?
> (See handle_irq_desc() and c16816acd086.)

I did this for consistency with the in_nmi() check in
generic_handle_domain_nmi(); I was unaware of commit c16816acd086 and
IRQD_HANDLE_ENFORCE_IRQCTX.

I'll have ot leave it to Marc and Thomas as to what we should do there.

> I believe the above change causes a regression in drivers/gpio/gpio-dln2.c
> such that a gratuitous WARN splat is now emitted.

Do you mean you beleive that from inspection, or are you actually seeing this
go wrong in practice?

If it's the latter, are you able to give a copy of the dmesg splat?

Thanks,
Mark.

^ permalink raw reply

* Re: [RFC v8 net-next 05/16] pinctrl: ocelot: add ability to be used in a non-mmio configuration
From: Andy Shevchenko @ 2022-05-09  8:37 UTC (permalink / raw)
  To: Colin Foster
  Cc: linux-arm-kernel, linux-gpio, netdev, Terry Bowman, Wolfram Sang,
	Steen Hegelund, Lars Povlsen, Linus Walleij, Russell King,
	Heiner Kallweit, Paolo Abeni, Jakub Kicinski, Eric Dumazet,
	David S. Miller, Florian Fainelli, Vivien Didelot, Andrew Lunn,
	UNGLinuxDriver, Alexandre Belloni, Claudiu Manoil,
	Vladimir Oltean, Lee Jones
In-Reply-To: <20220508185313.2222956-6-colin.foster@in-advantage.com>

On Sun, May 8, 2022 at 8:53 PM Colin Foster
<colin.foster@in-advantage.com> wrote:
>
> There are a few Ocelot chips that contain pinctrl logic, but can be
> controlled externally. Specifically the VSC7511, 7512, 7513 and 7514. In
> the externally controlled configurations these registers are not
> memory-mapped.
>
> Add support for these non-memory-mapped configurations.

...

> +#if defined(REG)

Redundant.

> +#undef REG
> +#endif

...

> +               res = platform_get_resource(pdev, IORESOURCE_REG, 0);
> +               if (!res) {
> +                       dev_err(dev, "Failed to get resource\n");
> +                       return -ENODEV;

return dev_err_probe(...); ?

> +               }

...

>         if (IS_ERR(info->map)) {
>                 dev_err(dev, "Failed to create regmap\n");
>                 return PTR_ERR(info->map);

Ditto.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* RE: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
From: Biju Das @ 2022-05-09  7:56 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Prabhakar Mahadev Lad, Thomas Gleixner, Marc Zyngier, Rob Herring,
	Krzysztof Kozlowski, Linus Walleij, Bartosz Golaszewski,
	Geert Uytterhoeven, Philipp Zabel, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <CA+V-a8uM=98O_WdaS1L=U3Eiqf_8Eteu9mDPats0a44W8VgpXA@mail.gmail.com>

Hi Prabhakar,

> Subject: Re: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller
> driver
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Mon, May 9, 2022 at 8:22 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Prabhakar,
> >
> > Thanks for the patch.
> >
> > > Subject: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt
> > > Controller driver
> > >
> > > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> > >
> > > This supports external pins being used as interrupts. It supports
> > > one line for NMI, 8 external pins and 32 GPIO pins (out of 123) to
> > > be used as IRQ lines.
> > >
> > > Signed-off-by: Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  drivers/irqchip/Kconfig             |   8 +
> > >  drivers/irqchip/Makefile            |   1 +
> > >  drivers/irqchip/irq-renesas-rzg2l.c | 445
> > > ++++++++++++++++++++++++++++
> > >  3 files changed, 454 insertions(+)
> > >  create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
> > >
> > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index
> > > 15edb9a6fcae..f3d071422f3b 100644
> > > --- a/drivers/irqchip/Kconfig
> > > +++ b/drivers/irqchip/Kconfig
> > > @@ -242,6 +242,14 @@ config RENESAS_RZA1_IRQC
> > >         Enable support for the Renesas RZ/A1 Interrupt Controller,
> > > to use up
> > >         to 8 external interrupts with configurable sense select.
> > >
> > > +config RENESAS_RZG2L_IRQC
> > > +     bool "Renesas RZ/G2L (and alike SoC) IRQC support" if
> COMPILE_TEST
> > > +     select GENERIC_IRQ_CHIP
> > > +     select IRQ_DOMAIN_HIERARCHY
> > > +     help
> > > +       Enable support for the Renesas RZ/G2L (and alike SoC)
> > > +Interrupt
> > > Controller
> > > +       for external devices.
> > > +
> > >  config SL28CPLD_INTC
> > >       bool "Kontron sl28cpld IRQ controller"
> > >       depends on MFD_SL28CPLD=y || COMPILE_TEST diff --git
> > > a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index
> > > 160a1d8ceaa9..eaa56eec2b23 100644
> > > --- a/drivers/irqchip/Makefile
> > > +++ b/drivers/irqchip/Makefile
> > > @@ -51,6 +51,7 @@ obj-$(CONFIG_RDA_INTC)                      += irq-
> rda-intc.o
> > >  obj-$(CONFIG_RENESAS_INTC_IRQPIN)    += irq-renesas-intc-irqpin.o
> > >  obj-$(CONFIG_RENESAS_IRQC)           += irq-renesas-irqc.o
> > >  obj-$(CONFIG_RENESAS_RZA1_IRQC)              += irq-renesas-rza1.o
> > > +obj-$(CONFIG_RENESAS_RZG2L_IRQC)     += irq-renesas-rzg2l.o
> > >  obj-$(CONFIG_VERSATILE_FPGA_IRQ)     += irq-versatile-fpga.o
> > >  obj-$(CONFIG_ARCH_NSPIRE)            += irq-zevio.o
> > >  obj-$(CONFIG_ARCH_VT8500)            += irq-vt8500.o
> > > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c
> > > b/drivers/irqchip/irq- renesas-rzg2l.c new file mode 100644 index
> > > 000000000000..bd6e82100caf
> > > --- /dev/null
> > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > @@ -0,0 +1,445 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Renesas RZ/G2L IRQC Driver
> > > + *
> > > + * Copyright (C) 2022 Renesas Electronics Corporation.
> > > + *
> > > + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/err.h>
> > > +#include <linux/io.h>
> > > +#include <linux/irqchip.h>
> > > +#include <linux/irqdomain.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/spinlock.h>
> > > +
> > > +#define IRQC_IRQ_START                       1
> > > +#define IRQC_IRQ_COUNT                       8
> > > +#define IRQC_TINT_START                      (IRQC_IRQ_START +
> IRQC_IRQ_COUNT)
> > > +#define IRQC_TINT_COUNT                      32
> > > +#define IRQC_NUM_IRQ                 (IRQC_TINT_START +
> IRQC_TINT_COUNT)
> > > +
> > > +#define ISCR                         0x10
> > > +#define IITSR                                0x14
> > > +#define TSCR                         0x20
> > > +#define TITSR0                               0x24
> > > +#define TITSR1                               0x28
> > > +#define TITSR0_MAX_INT                       16
> > > +#define TITSEL_WIDTH                 0x2
> > > +#define TSSR(n)                              (0x30 + ((n) * 4))
> > > +#define TIEN                         BIT(7)
> > > +#define TSSEL_SHIFT(n)                       (8 * (n))
> > > +#define TSSEL_MASK                   GENMASK(7, 0)
> > > +#define IRQ_MASK                     0x3
> > > +
> > > +#define TSSR_OFFSET(n)                       ((n) % 4)
> > > +#define TSSR_INDEX(n)                        ((n) / 4)
> > > +
> > > +#define TITSR_TITSEL_EDGE_RISING     0
> > > +#define TITSR_TITSEL_EDGE_FALLING    1
> > > +#define TITSR_TITSEL_LEVEL_HIGH              2
> > > +#define TITSR_TITSEL_LEVEL_LOW               3
> > > +
> > > +#define IITSR_IITSEL(n, sense)               ((sense) << ((n) * 2))
> > > +#define IITSR_IITSEL_LEVEL_LOW               0
> > > +#define IITSR_IITSEL_EDGE_FALLING    1
> > > +#define IITSR_IITSEL_EDGE_RISING     2
> > > +#define IITSR_IITSEL_EDGE_BOTH               3
> > > +#define IITSR_IITSEL_MASK(n)         IITSR_IITSEL((n), 3)
> > > +
> > > +#define TINT_EXTRACT_HWIRQ(x)                ((x) & ~GENMASK(31, 16))
> > > +#define TINT_EXTRACT_GPIOINT(x)              ((x) >> 16)
> > > +
> > > +struct rzg2l_irqc_priv {
> > > +     void __iomem *base;
> > > +     struct of_phandle_args map[IRQC_NUM_IRQ];
> > > +     raw_spinlock_t lock;
> > > +};
> > > +
> > > +struct rzg2l_irqc_chip_data {
> > > +     int tint;
> > > +};
> > > +
> > > +static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data
> > > +*data) {
> > > +     return data->domain->host_data; }
> > > +
> > > +static void rzg2l_irq_eoi(struct irq_data *d) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     u32 bit = BIT(hw_irq);
> > > +     u32 reg;
> > > +
> > > +     reg = readl_relaxed(priv->base + ISCR);
> > > +     if (reg & bit)
> > > +             writel_relaxed(reg & ~bit, priv->base + ISCR); }
> > > +
> > > +static void rzg2l_tint_eoi(struct irq_data *d) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_TINT_START;
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     u32 bit = BIT(hw_irq);
> > > +     u32 reg;
> > > +
> > > +     reg = readl_relaxed(priv->base + TSCR);
> > > +     if (reg & bit)
> > > +             writel_relaxed(reg & ~bit, priv->base + TSCR); }
> > > +
> > > +static void rzg2l_irqc_eoi(struct irq_data *d) {
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > > +
> > > +     raw_spin_lock(&priv->lock);
> > > +     if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> > > +             rzg2l_irq_eoi(d);
> > > +     else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> > > +             rzg2l_tint_eoi(d);
> > > +     raw_spin_unlock(&priv->lock);
> > > +     irq_chip_eoi_parent(d);
> > > +}
> > > +
> > > +static void rzg2l_irqc_irq_disable(struct irq_data *d) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > > +
> > > +     if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> > > +             struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +             u32 offset = hw_irq - IRQC_TINT_START;
> > > +             u32 tssr_offset = TSSR_OFFSET(offset);
> > > +             u8 tssr_index = TSSR_INDEX(offset);
> > > +             u32 reg;
> > > +
> > > +             raw_spin_lock(&priv->lock);
> > > +             reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > > +             reg &= ~(TSSEL_MASK << tssr_offset);
> > > +             writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > > +             raw_spin_unlock(&priv->lock);
> > > +     }
> > > +     irq_chip_disable_parent(d);
> > > +}
> > > +
> > > +static void rzg2l_irqc_irq_enable(struct irq_data *d) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > > +
> > > +     if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> > > +             struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +             struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> > > +             u32 offset = hw_irq - IRQC_TINT_START;
> > > +             u32 tssr_offset = TSSR_OFFSET(offset);
> > > +             u8 tssr_index = TSSR_INDEX(offset);
> > > +             u32 reg;
> > > +
> > > +             raw_spin_lock(&priv->lock);
> > > +             reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > > +             reg |= (TIEN | chip_data->tint) <<
> TSSEL_SHIFT(tssr_offset);
> > > +             writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > > +             raw_spin_unlock(&priv->lock);
> > > +     }
> > > +     irq_chip_enable_parent(d);
> > > +}
> > > +
> > > +static int rzg2l_irq_set_type(struct irq_data *d, unsigned int type) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     u16 sense, tmp;
> > > +
> > > +     switch (type & IRQ_TYPE_SENSE_MASK) {
> > > +     case IRQ_TYPE_LEVEL_LOW:
> > > +             sense = IITSR_IITSEL_LEVEL_LOW;
> > > +             break;
> > > +
> > > +     case IRQ_TYPE_EDGE_FALLING:
> > > +             sense = IITSR_IITSEL_EDGE_FALLING;
> > > +             break;
> > > +
> > > +     case IRQ_TYPE_EDGE_RISING:
> > > +             sense = IITSR_IITSEL_EDGE_RISING;
> > > +             break;
> > > +
> > > +     case IRQ_TYPE_EDGE_BOTH:
> > > +             sense = IITSR_IITSEL_EDGE_BOTH;
> > > +             break;
> > > +
> > > +     default:
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     raw_spin_lock(&priv->lock);
> > > +     tmp = readl_relaxed(priv->base + IITSR);
> > > +     tmp &= ~IITSR_IITSEL_MASK(hw_irq);
> > > +     tmp |= IITSR_IITSEL(hw_irq, sense);
> > > +     writel_relaxed(tmp, priv->base + IITSR);
> > > +     raw_spin_unlock(&priv->lock);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type)
> {
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     unsigned int hwirq = irqd_to_hwirq(d);
> > > +     u32 titseln = hwirq - IRQC_TINT_START;
> > > +     u32 offset;
> > > +     u8 sense;
> > > +     u32 reg;
> > > +
> > > +     switch (type & IRQ_TYPE_SENSE_MASK) {
> > > +     case IRQ_TYPE_EDGE_RISING:
> > > +             sense = TITSR_TITSEL_EDGE_RISING;
> > > +             break;
> > > +
> > > +     case IRQ_TYPE_EDGE_FALLING:
> > > +             sense = TITSR_TITSEL_EDGE_FALLING;
> > > +             break;
> > > +
> > > +     default:
> > > +             return -EINVAL;
> > > +     }
> > > +
> >
> > > +     if (titseln < TITSR0_MAX_INT) {
> > > +             offset = TITSR0;
> > > +     } else {
> > > +             titseln /= TITSEL_WIDTH;
> > > +             offset  = TITSR1;
> > > +     }
> >
> > as TITSR0 (0x24) and TITSR1(0x28) are contiguous address location
> >
> > May be like others, above declare it as
> > u32 offset = TITSR0; ??
> >
> > and here
> >  if ((titseln >= TITSR0_MAX_INT) {
> >         titseln /= TITSEL_WIDTH;
> >         offset  += 4;
> >  }
> >
> The current implementation is suggested by the maintainer.
> 
> > > +
> > > +     raw_spin_lock(&priv->lock);
> > > +     reg = readl_relaxed(priv->base + offset);
> > > +     reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
> > > +     reg |= sense << (titseln * TITSEL_WIDTH);
> > > +     writel_relaxed(reg, priv->base + offset);
> > > +     raw_spin_unlock(&priv->lock);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
> {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > > +     int ret = -EINVAL;
> > > +
> > > +     if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> > > +             ret = rzg2l_irq_set_type(d, type);
> > > +     else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> > > +             ret = rzg2l_tint_set_edge(d, type);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     return irq_chip_set_type_parent(d, IRQ_TYPE_LEVEL_HIGH); }
> > > +
> > > +static struct irq_chip irqc_chip = {
> > > +     .name                   = "rzg2l-irqc",
> > > +     .irq_eoi                = rzg2l_irqc_eoi,
> > > +     .irq_mask               = irq_chip_mask_parent,
> > > +     .irq_unmask             = irq_chip_unmask_parent,
> > > +     .irq_disable            = rzg2l_irqc_irq_disable,
> > > +     .irq_enable             = rzg2l_irqc_irq_enable,
> > > +     .irq_get_irqchip_state  = irq_chip_get_parent_state,
> > > +     .irq_set_irqchip_state  = irq_chip_set_parent_state,
> > > +     .irq_retrigger          = irq_chip_retrigger_hierarchy,
> > > +     .irq_set_type           = rzg2l_irqc_set_type,
> > > +     .flags                  = IRQCHIP_MASK_ON_SUSPEND |
> > > +                               IRQCHIP_SET_TYPE_MASKED |
> > > +                               IRQCHIP_SKIP_SET_WAKE, };
> > > +
> > > +static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int
> virq,
> > > +                         unsigned int nr_irqs, void *arg) {
> > > +     struct rzg2l_irqc_priv *priv = domain->host_data;
> > > +     struct rzg2l_irqc_chip_data *chip_data = NULL;
> > > +     struct irq_fwspec spec;
> > > +     irq_hw_number_t hwirq;
> > > +     int tint = -EINVAL;
> > > +     unsigned int type;
> > > +     unsigned int i;
> > > +     int ret;
> > > +
> > > +     ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     /*
> > > +      * For TINIT interrupts ie where pinctrl driver is child of
> > > + irqc
> > > domain
> > > +      * the hwirq and TINT are encoded in fwspec->param[0].
> > > +      * hwirq for TINIT range from 9-40, hwirq is embedded 0-15
> > > + bits and
> > > TINT
> > > +      * from 16-31 bits. TINIT from the pinctrl driver needs to be
> > > programmed
> > > +      * in IRQC registers to enable a given gpio pin as interrupt.
> > > +      */
> > > +     if (hwirq > IRQC_IRQ_COUNT) {
> > > +             tint = TINT_EXTRACT_GPIOINT(hwirq);
> > > +             hwirq = TINT_EXTRACT_HWIRQ(hwirq);
> > > +     }
> > > +
> > > +     if (hwirq > (IRQC_NUM_IRQ - 1))
> > > +             return -EINVAL;
> >
> >
> > > +
> > > +     if (tint != -EINVAL && (hwirq < IRQC_TINT_START || hwirq >
> > > (IRQC_NUM_IRQ - 1)))
> > > +             return -EINVAL;
> >
> > hwirq > (IRQC_NUM_IRQ - 1) is redundant check, as it is checked above.
> >
> >
> Sure will drop it.

On further thinking,

I guess tint != -EINVAL is not required, if you add a check for
hwirq < IRQC_TINT_START on the if statement for tint. Ie,

     if (hwirq > IRQC_IRQ_COUNT) {
             tint = TINT_EXTRACT_GPIOINT(hwirq);
             hwirq = TINT_EXTRACT_HWIRQ(hwirq);
		 if (hwirq < IRQC_TINT_START)
			return -EINVAL;
     }

Then you only need

	if (hwirq > (IRQC_NUM_IRQ - 1))
		return -EINVAL;

Cheers,
Biju


> 
> Cheers,
> Prabhakar
> 
> > Cheers,
> > Biju
> >
> > > +
> > > +     chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> > > +     if (!chip_data)
> > > +             return -ENOMEM;
> > > +     chip_data->tint = tint;
> > > +
> > > +     ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> &irqc_chip,
> > > +                                         chip_data);
> > > +     if (ret) {
> > > +             kfree(chip_data);
> > > +             return ret;
> > > +     }
> > > +
> > > +     spec.fwnode = domain->parent->fwnode;
> > > +     spec.param_count = priv->map[hwirq].args_count;
> > > +     for (i = 0; i < spec.param_count; i++)
> > > +             spec.param[i] = priv->map[hwirq].args[i];
> > > +
> > > +     ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &spec);
> > > +     if (ret)
> > > +             kfree(chip_data);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static void rzg2l_irqc_domain_free(struct irq_domain *domain,
> > > +unsigned int
> > > virq,
> > > +                                unsigned int nr_irqs) {
> > > +     struct irq_data *d;
> > > +
> > > +     d = irq_domain_get_irq_data(domain, virq);
> > > +     if (d) {
> > > +             struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> > > +
> > > +             kfree(chip_data);
> > > +     }
> > > +     irq_domain_free_irqs_common(domain, virq, nr_irqs); }
> > > +
> > > +static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
> > > +     .alloc = rzg2l_irqc_alloc,
> > > +     .free = rzg2l_irqc_domain_free,
> > > +     .translate = irq_domain_translate_twocell, };
> > > +
> > > +static int rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
> > > +                             struct device_node *np) {
> > > +     unsigned int i;
> > > +     int ret;
> > > +
> > > +     for (i = 0; i < IRQC_NUM_IRQ; i++) {
> > > +             ret = of_irq_parse_one(np, i, &priv->map[i]);
> > > +             if (ret)
> > > +                     return ret;
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int rzg2l_irqc_init(struct device_node *node, struct
> > > +device_node
> > > +*parent) {
> > > +     struct irq_domain *irq_domain, *parent_domain;
> > > +     struct reset_control *resetn;
> > > +     struct rzg2l_irqc_priv *priv;
> > > +     struct clk *clk;
> > > +     struct clk *pclk;
> > > +     int ret;
> > > +
> > > +     priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > > +     if (!priv)
> > > +             return -ENOMEM;
> > > +
> > > +     priv->base = of_iomap(node, 0);
> > > +     if (!priv->base) {
> > > +             ret = -ENXIO;
> > > +             goto free_priv;
> > > +     }
> > > +
> > > +     clk = of_clk_get_by_name(node, "clk");
> > > +     if (IS_ERR(clk)) {
> > > +             ret = IS_ERR(clk);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     pclk = of_clk_get_by_name(node, "pclk");
> > > +     if (IS_ERR(pclk)) {
> > > +             ret = IS_ERR(pclk);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     resetn = of_reset_control_get_exclusive_by_index(node, 0);
> > > +     if (IS_ERR(resetn)) {
> > > +             ret = IS_ERR(resetn);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     parent_domain = irq_find_host(parent);
> > > +     if (!parent_domain) {
> > > +             pr_err("%pOF: cannot find parent domain\n", node);
> > > +             ret = -ENODEV;
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     ret = rzg2l_irqc_parse_map(priv, node);
> > > +     if (ret) {
> > > +             pr_err("%pOF: cannot parse interrupts: %d\n", node, ret);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     ret = reset_control_deassert(resetn);
> > > +     if (ret) {
> > > +             pr_err("%pOF: failed to deassert resetn pin, %d\n",
> > > + node,
> > > ret);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     raw_spin_lock_init(&priv->lock);
> > > +
> > > +     ret = clk_prepare_enable(clk);
> > > +     if (ret)
> > > +             goto assert_reset;
> > > +
> > > +     ret = clk_prepare_enable(pclk);
> > > +     if (ret)
> > > +             goto disable_clk;
> > > +
> > > +     irq_domain = irq_domain_add_hierarchy(parent_domain, 0,
> > > IRQC_NUM_IRQ,
> > > +                                           node,
> &rzg2l_irqc_domain_ops,
> > > +                                           priv);
> > > +     if (!irq_domain) {
> > > +             pr_err("%pOF: cannot initialize irq domain\n", node);
> > > +             ret = -ENOMEM;
> > > +             goto fail_irq_domain;
> > > +     }
> > > +
> > > +     return 0;
> > > +
> > > +fail_irq_domain:
> > > +     clk_disable_unprepare(pclk);
> > > +disable_clk:
> > > +     clk_disable_unprepare(clk);
> > > +assert_reset:
> > > +     reset_control_assert(resetn);
> > > +iounmap_base:
> > > +     iounmap(priv->base);
> > > +free_priv:
> > > +     kfree(priv);
> > > +     return ret;
> > > +}
> > > +
> > > +IRQCHIP_PLATFORM_DRIVER_BEGIN(rzg2l_irqc)
> > > +IRQCHIP_MATCH("renesas,rzg2l-irqc", rzg2l_irqc_init)
> > > +IRQCHIP_PLATFORM_DRIVER_END(rzg2l_irqc)
> > > +MODULE_AUTHOR("Lad Prabhakar
> > > +<prabhakar.mahadev-lad.rj@bp.renesas.com>");
> > > +MODULE_DESCRIPTION("Renesas RZ/G2L IRQC Driver");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.25.1
> >

^ permalink raw reply

* Re: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt
From: Lad, Prabhakar @ 2022-05-09  7:48 UTC (permalink / raw)
  To: Biju Das
  Cc: Prabhakar Mahadev Lad, Thomas Gleixner, Marc Zyngier, Rob Herring,
	Krzysztof Kozlowski, Linus Walleij, Bartosz Golaszewski,
	Geert Uytterhoeven, Philipp Zabel, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <OS0PR01MB59228AE4AF4EE53C382E8BA986C69@OS0PR01MB5922.jpnprd01.prod.outlook.com>

Hi Biju,

Thank you for the review.

On Mon, May 9, 2022 at 7:49 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Prabhakar,
>
> Thanks for the patch.
>
> > Subject: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to
> > handle GPIO interrupt
> >
> > Add IRQ domian to RZ/G2L pinctrl driver to handle GPIO interrupt.
> >
> > GPIO0-GPIO122 pins can be used as IRQ lines but only 32 pins can be used as
> > IRQ lines at given time. Selection of pins as IRQ lines is handled by IA55
> > (which is the IRQC block) which sits in between the GPIO and GIC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 205 ++++++++++++++++++++++++
> >  1 file changed, 205 insertions(+)
> >
> > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > index a48cac55152c..275dfec74329 100644
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -9,8 +9,10 @@
> >  #include <linux/clk.h>
> >  #include <linux/gpio/driver.h>
> >  #include <linux/io.h>
> > +#include <linux/interrupt.h>
> >  #include <linux/module.h>
> >  #include <linux/of_device.h>
> > +#include <linux/of_irq.h>
> >  #include <linux/pinctrl/pinconf-generic.h>  #include
> > <linux/pinctrl/pinconf.h>  #include <linux/pinctrl/pinctrl.h> @@ -89,6
> > +91,7 @@
> >  #define PIN(n)                       (0x0800 + 0x10 + (n))
> >  #define IOLH(n)                      (0x1000 + (n) * 8)
> >  #define IEN(n)                       (0x1800 + (n) * 8)
> > +#define ISEL(n)                      (0x2c80 + (n) * 8)
> >  #define PWPR                 (0x3014)
> >  #define SD_CH(n)             (0x3000 + (n) * 4)
> >  #define QSPI                 (0x3008)
> > @@ -112,6 +115,10 @@
> >  #define RZG2L_PIN_ID_TO_PORT_OFFSET(id)      (RZG2L_PIN_ID_TO_PORT(id) +
> > 0x10)
> >  #define RZG2L_PIN_ID_TO_PIN(id)              ((id) % RZG2L_PINS_PER_PORT)
> >
> > +#define RZG2L_TINT_MAX_INTERRUPT     32
> > +#define RZG2L_TINT_IRQ_START_INDEX   9
> > +#define RZG2L_PACK_HWIRQ(t, i)               (((t) << 16) | (i))
> > +
> >  struct rzg2l_dedicated_configs {
> >       const char *name;
> >       u32 config;
> > @@ -137,6 +144,9 @@ struct rzg2l_pinctrl {
> >
> >       struct gpio_chip                gpio_chip;
> >       struct pinctrl_gpio_range       gpio_range;
> > +     DECLARE_BITMAP(tint_slot, RZG2L_TINT_MAX_INTERRUPT);
> > +     spinlock_t                      bitmap_lock;
> > +     unsigned int                    hwirq[RZG2L_TINT_MAX_INTERRUPT];
> >
> >       spinlock_t                      lock;
> >  };
> > @@ -883,6 +893,8 @@ static int rzg2l_gpio_get(struct gpio_chip *chip,
> > unsigned int offset)
> >
> >  static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
> > {
> > +     unsigned int virq;
> > +
> >       pinctrl_gpio_free(chip->base + offset);
> >
> >       /*
> > @@ -890,6 +902,10 @@ static void rzg2l_gpio_free(struct gpio_chip *chip,
> > unsigned int offset)
> >        * drive the GPIO pin as an output.
> >        */
> >       rzg2l_gpio_direction_input(chip, offset);
> > +
> > +     virq = irq_find_mapping(chip->irq.domain, offset);
> > +     if (virq)
> > +             irq_dispose_mapping(virq);
> >  }
> >
> >  static const char * const rzg2l_gpio_names[] = { @@ -1104,14 +1120,193 @@
> > static struct {
> >       }
> >  };
> >
> > +static int rzg2l_gpio_get_gpioint(unsigned int virq) {
> > +     unsigned int gpioint = 0;
> > +     unsigned int i = 0;
> > +     u32 port, bit;
> > +
> > +     port = virq / 8;
> > +     bit = virq % 8;
> > +
> > +     if (port >= ARRAY_SIZE(rzg2l_gpio_configs))
> > +             return -EINVAL;
> > +
> > +     for (i = 0; i < port; i++)
> > +             gpioint += RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]);
> > +
> > +     if (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]))
> > +             return -EINVAL;
>
> May be combine this statement to above with
>
> || (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[port]))
>                 return -EINVAL;
>
The reason I have kept it outside the loop is that I'll have to check
it only once at the end of the loop instead of repeating the check
every time in the loop.

Cheers,
Prabhakar

> Cheers,
> BIju
>
> > +
> > +     gpioint += bit;
> > +
> > +     return gpioint;
> > +}
> > +
> > +static void rzg2l_gpio_irq_domain_free(struct irq_domain *domain, unsigned
> > int virq,
> > +                                    unsigned int nr_irqs)
> > +{
> > +     struct irq_data *d;
> > +
> > +     d = irq_domain_get_irq_data(domain, virq);
> > +     if (d) {
> > +             struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > +             struct rzg2l_pinctrl *pctrl = container_of(gc, struct
> > rzg2l_pinctrl, gpio_chip);
> > +             irq_hw_number_t hwirq = irqd_to_hwirq(d);
> > +             unsigned long flags;
> > +             unsigned int i;
> > +
> > +             for (i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) {
> > +                     if (pctrl->hwirq[i] == hwirq) {
> > +                             spin_lock_irqsave(&pctrl->bitmap_lock, flags);
> > +                             bitmap_release_region(pctrl->tint_slot, i,
> > get_order(1));
> > +                             spin_unlock_irqrestore(&pctrl->bitmap_lock,
> > flags);
> > +                             pctrl->hwirq[i] = 0;
> > +                             break;
> > +                     }
> > +             }
> > +     }
> > +     irq_domain_free_irqs_common(domain, virq, nr_irqs); }
> > +
> > +static void rzg2l_gpio_irq_disable(struct irq_data *d) {
> > +     struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > +     struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl,
> > gpio_chip);
> > +     unsigned int hwirq = irqd_to_hwirq(d);
> > +     unsigned long flags;
> > +     void __iomem *addr;
> > +     u32 port;
> > +     u8 bit;
> > +
> > +     port = RZG2L_PIN_ID_TO_PORT(hwirq);
> > +     bit = RZG2L_PIN_ID_TO_PIN(hwirq);
> > +
> > +     addr = pctrl->base + ISEL(port);
> > +     if (bit >= 4) {
> > +             bit -= 4;
> > +             addr += 4;
> > +     }
> > +
> > +     spin_lock_irqsave(&pctrl->lock, flags);
> > +     writel(readl(addr) & ~BIT(bit * 8), addr);
> > +     spin_unlock_irqrestore(&pctrl->lock, flags);
> > +
> > +     irq_chip_disable_parent(d);
> > +}
> > +
> > +static void rzg2l_gpio_irq_enable(struct irq_data *d) {
> > +     struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > +     struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl,
> > gpio_chip);
> > +     unsigned int hwirq = irqd_to_hwirq(d);
> > +     unsigned long flags;
> > +     void __iomem *addr;
> > +     u32 port;
> > +     u8 bit;
> > +
> > +     port = RZG2L_PIN_ID_TO_PORT(hwirq);
> > +     bit = RZG2L_PIN_ID_TO_PIN(hwirq);
> > +
> > +     addr = pctrl->base + ISEL(port);
> > +     if (bit >= 4) {
> > +             bit -= 4;
> > +             addr += 4;
> > +     }
> > +
> > +     spin_lock_irqsave(&pctrl->lock, flags);
> > +     writel(readl(addr) | BIT(bit * 8), addr);
> > +     spin_unlock_irqrestore(&pctrl->lock, flags);
> > +
> > +     irq_chip_enable_parent(d);
> > +}
> > +
> > +static int rzg2l_gpio_irq_set_type(struct irq_data *d, unsigned int
> > +type) {
> > +     return irq_chip_set_type_parent(d, type); }
> > +
> > +static void rzg2l_gpio_irqc_eoi(struct irq_data *d) {
> > +     irq_chip_eoi_parent(d);
> > +}
> > +
> > +static struct irq_chip rzg2l_gpio_irqchip = {
> > +     .name = "rzg2l-gpio",
> > +     .irq_disable = rzg2l_gpio_irq_disable,
> > +     .irq_enable = rzg2l_gpio_irq_enable,
> > +     .irq_mask = irq_chip_mask_parent,
> > +     .irq_unmask = irq_chip_unmask_parent,
> > +     .irq_set_type = rzg2l_gpio_irq_set_type,
> > +     .irq_eoi = rzg2l_gpio_irqc_eoi,
> > +};
> > +
> > +static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
> > +                                         unsigned int child,
> > +                                         unsigned int child_type,
> > +                                         unsigned int *parent,
> > +                                         unsigned int *parent_type)
> > +{
> > +     struct rzg2l_pinctrl *pctrl = gpiochip_get_data(gc);
> > +     unsigned long flags;
> > +     int gpioint, irq;
> > +
> > +     gpioint = rzg2l_gpio_get_gpioint(child);
> > +     if (gpioint < 0)
> > +             return gpioint;
> > +
> > +     spin_lock_irqsave(&pctrl->bitmap_lock, flags);
> > +     irq = bitmap_find_free_region(pctrl->tint_slot,
> > RZG2L_TINT_MAX_INTERRUPT, get_order(1));
> > +     spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
> > +     if (irq < 0)
> > +             return -ENOSPC;
> > +     pctrl->hwirq[irq] = child;
> > +     irq += RZG2L_TINT_IRQ_START_INDEX;
> > +
> > +     /* All these interrupts are level high in the CPU */
> > +     *parent_type = IRQ_TYPE_LEVEL_HIGH;
> > +     *parent = RZG2L_PACK_HWIRQ(gpioint, irq);
> > +     return 0;
> > +}
> > +
> > +static void *rzg2l_gpio_populate_parent_fwspec(struct gpio_chip *chip,
> > +                                            unsigned int parent_hwirq,
> > +                                            unsigned int parent_type)
> > +{
> > +     struct irq_fwspec *fwspec;
> > +
> > +     fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
> > +     if (!fwspec)
> > +             return NULL;
> > +
> > +     fwspec->fwnode = chip->irq.parent_domain->fwnode;
> > +     fwspec->param_count = 2;
> > +     fwspec->param[0] = parent_hwirq;
> > +     fwspec->param[1] = parent_type;
> > +
> > +     return fwspec;
> > +}
> > +
> >  static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)  {
> >       struct device_node *np = pctrl->dev->of_node;
> >       struct gpio_chip *chip = &pctrl->gpio_chip;
> >       const char *name = dev_name(pctrl->dev);
> > +     struct irq_domain *parent_domain;
> >       struct of_phandle_args of_args;
> > +     struct device_node *parent_np;
> > +     struct gpio_irq_chip *girq;
> >       int ret;
> >
> > +     parent_np = of_irq_find_parent(np);
> > +     if (!parent_np)
> > +             return -ENXIO;
> > +
> > +     parent_domain = irq_find_host(parent_np);
> > +     of_node_put(parent_np);
> > +     if (!parent_domain)
> > +             return -EPROBE_DEFER;
> > +
> >       ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0,
> > &of_args);
> >       if (ret) {
> >               dev_err(pctrl->dev, "Unable to parse gpio-ranges\n"); @@ -
> > 1138,6 +1333,15 @@ static int rzg2l_gpio_register(struct rzg2l_pinctrl
> > *pctrl)
> >       chip->base = -1;
> >       chip->ngpio = of_args.args[2];
> >
> > +     girq = &chip->irq;
> > +     girq->chip = &rzg2l_gpio_irqchip;
> > +     girq->fwnode = of_node_to_fwnode(np);
> > +     girq->parent_domain = parent_domain;
> > +     girq->child_to_parent_hwirq = rzg2l_gpio_child_to_parent_hwirq;
> > +     girq->populate_parent_alloc_arg = rzg2l_gpio_populate_parent_fwspec;
> > +     girq->child_irq_domain_ops.free = rzg2l_gpio_irq_domain_free;
> > +     girq->ngirq = RZG2L_TINT_MAX_INTERRUPT;
> > +
> >       pctrl->gpio_range.id = 0;
> >       pctrl->gpio_range.pin_base = 0;
> >       pctrl->gpio_range.base = 0;
> > @@ -1253,6 +1457,7 @@ static int rzg2l_pinctrl_probe(struct platform_device
> > *pdev)
> >       }
> >
> >       spin_lock_init(&pctrl->lock);
> > +     spin_lock_init(&pctrl->bitmap_lock);
> >
> >       platform_set_drvdata(pdev, pctrl);
> >
> > --
> > 2.25.1
>

^ permalink raw reply

* RE: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt
From: Biju Das @ 2022-05-09  8:01 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Prabhakar Mahadev Lad, Thomas Gleixner, Marc Zyngier, Rob Herring,
	Krzysztof Kozlowski, Linus Walleij, Bartosz Golaszewski,
	Geert Uytterhoeven, Philipp Zabel, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <CA+V-a8syBvN7czRA7tq0TJSmUcBzmgjLrFmizHD6Ycp5kLXJWw@mail.gmail.com>

Hi Prabhakar,

> Subject: Re: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain
> to handle GPIO interrupt
> 
> Hi Biju,
> 
> Thank you for the review.
> 
> On Mon, May 9, 2022 at 7:49 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Prabhakar,
> >
> > Thanks for the patch.
> >
> > > Subject: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ
> > > domain to handle GPIO interrupt
> > >
> > > Add IRQ domian to RZ/G2L pinctrl driver to handle GPIO interrupt.
> > >
> > > GPIO0-GPIO122 pins can be used as IRQ lines but only 32 pins can be
> > > used as IRQ lines at given time. Selection of pins as IRQ lines is
> > > handled by IA55 (which is the IRQC block) which sits in between the
> GPIO and GIC.
> > >
> > > Signed-off-by: Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 205
> > > ++++++++++++++++++++++++
> > >  1 file changed, 205 insertions(+)
> > >
> > > diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > index a48cac55152c..275dfec74329 100644
> > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > @@ -9,8 +9,10 @@
> > >  #include <linux/clk.h>
> > >  #include <linux/gpio/driver.h>
> > >  #include <linux/io.h>
> > > +#include <linux/interrupt.h>
> > >  #include <linux/module.h>
> > >  #include <linux/of_device.h>
> > > +#include <linux/of_irq.h>
> > >  #include <linux/pinctrl/pinconf-generic.h>  #include
> > > <linux/pinctrl/pinconf.h>  #include <linux/pinctrl/pinctrl.h> @@
> > > -89,6
> > > +91,7 @@
> > >  #define PIN(n)                       (0x0800 + 0x10 + (n))
> > >  #define IOLH(n)                      (0x1000 + (n) * 8)
> > >  #define IEN(n)                       (0x1800 + (n) * 8)
> > > +#define ISEL(n)                      (0x2c80 + (n) * 8)
> > >  #define PWPR                 (0x3014)
> > >  #define SD_CH(n)             (0x3000 + (n) * 4)
> > >  #define QSPI                 (0x3008)
> > > @@ -112,6 +115,10 @@
> > >  #define RZG2L_PIN_ID_TO_PORT_OFFSET(id)      (RZG2L_PIN_ID_TO_PORT(id)
> +
> > > 0x10)
> > >  #define RZG2L_PIN_ID_TO_PIN(id)              ((id) %
> RZG2L_PINS_PER_PORT)
> > >
> > > +#define RZG2L_TINT_MAX_INTERRUPT     32
> > > +#define RZG2L_TINT_IRQ_START_INDEX   9
> > > +#define RZG2L_PACK_HWIRQ(t, i)               (((t) << 16) | (i))
> > > +
> > >  struct rzg2l_dedicated_configs {
> > >       const char *name;
> > >       u32 config;
> > > @@ -137,6 +144,9 @@ struct rzg2l_pinctrl {
> > >
> > >       struct gpio_chip                gpio_chip;
> > >       struct pinctrl_gpio_range       gpio_range;
> > > +     DECLARE_BITMAP(tint_slot, RZG2L_TINT_MAX_INTERRUPT);
> > > +     spinlock_t                      bitmap_lock;
> > > +     unsigned int                    hwirq[RZG2L_TINT_MAX_INTERRUPT];
> > >
> > >       spinlock_t                      lock;
> > >  };
> > > @@ -883,6 +893,8 @@ static int rzg2l_gpio_get(struct gpio_chip
> > > *chip, unsigned int offset)
> > >
> > >  static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int
> > > offset) {
> > > +     unsigned int virq;
> > > +
> > >       pinctrl_gpio_free(chip->base + offset);
> > >
> > >       /*
> > > @@ -890,6 +902,10 @@ static void rzg2l_gpio_free(struct gpio_chip
> > > *chip, unsigned int offset)
> > >        * drive the GPIO pin as an output.
> > >        */
> > >       rzg2l_gpio_direction_input(chip, offset);
> > > +
> > > +     virq = irq_find_mapping(chip->irq.domain, offset);
> > > +     if (virq)
> > > +             irq_dispose_mapping(virq);
> > >  }
> > >
> > >  static const char * const rzg2l_gpio_names[] = { @@ -1104,14
> > > +1120,193 @@ static struct {
> > >       }
> > >  };
> > >
> > > +static int rzg2l_gpio_get_gpioint(unsigned int virq) {
> > > +     unsigned int gpioint = 0;
> > > +     unsigned int i = 0;
> > > +     u32 port, bit;
> > > +
> > > +     port = virq / 8;
> > > +     bit = virq % 8;
> > > +
> > > +     if (port >= ARRAY_SIZE(rzg2l_gpio_configs))
> > > +             return -EINVAL;
> > > +
> > > +     for (i = 0; i < port; i++)
> > > +             gpioint +=
> > > + RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]);
> > > +
> > > +     if (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]))
> > > +             return -EINVAL;
> >
> > May be combine this statement to above with
> >
> > || (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[port]))
> >                 return -EINVAL;
> >
> The reason I have kept it outside the loop is that I'll have to check it
> only once at the end of the loop instead of repeating the check every time
> in the loop.

I meant above for loop, so that validation happens before the for loop??

if (port >= ARRAY_SIZE(rzg2l_gpio_configs)) || (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[port]))
	return -EINVAL;

Cheers,
Biju


> 
> Cheers,
> Prabhakar
> 
> > Cheers,
> > BIju
> >
> > > +
> > > +     gpioint += bit;
> > > +
> > > +     return gpioint;
> > > +}
> > > +
> > > +static void rzg2l_gpio_irq_domain_free(struct irq_domain *domain,
> > > +unsigned
> > > int virq,
> > > +                                    unsigned int nr_irqs) {
> > > +     struct irq_data *d;
> > > +
> > > +     d = irq_domain_get_irq_data(domain, virq);
> > > +     if (d) {
> > > +             struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > > +             struct rzg2l_pinctrl *pctrl = container_of(gc, struct
> > > rzg2l_pinctrl, gpio_chip);
> > > +             irq_hw_number_t hwirq = irqd_to_hwirq(d);
> > > +             unsigned long flags;
> > > +             unsigned int i;
> > > +
> > > +             for (i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) {
> > > +                     if (pctrl->hwirq[i] == hwirq) {
> > > +                             spin_lock_irqsave(&pctrl->bitmap_lock,
> flags);
> > > +
> > > + bitmap_release_region(pctrl->tint_slot, i,
> > > get_order(1));
> > > +
> > > + spin_unlock_irqrestore(&pctrl->bitmap_lock,
> > > flags);
> > > +                             pctrl->hwirq[i] = 0;
> > > +                             break;
> > > +                     }
> > > +             }
> > > +     }
> > > +     irq_domain_free_irqs_common(domain, virq, nr_irqs); }
> > > +
> > > +static void rzg2l_gpio_irq_disable(struct irq_data *d) {
> > > +     struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > > +     struct rzg2l_pinctrl *pctrl = container_of(gc, struct
> > > +rzg2l_pinctrl,
> > > gpio_chip);
> > > +     unsigned int hwirq = irqd_to_hwirq(d);
> > > +     unsigned long flags;
> > > +     void __iomem *addr;
> > > +     u32 port;
> > > +     u8 bit;
> > > +
> > > +     port = RZG2L_PIN_ID_TO_PORT(hwirq);
> > > +     bit = RZG2L_PIN_ID_TO_PIN(hwirq);
> > > +
> > > +     addr = pctrl->base + ISEL(port);
> > > +     if (bit >= 4) {
> > > +             bit -= 4;
> > > +             addr += 4;
> > > +     }
> > > +
> > > +     spin_lock_irqsave(&pctrl->lock, flags);
> > > +     writel(readl(addr) & ~BIT(bit * 8), addr);
> > > +     spin_unlock_irqrestore(&pctrl->lock, flags);
> > > +
> > > +     irq_chip_disable_parent(d);
> > > +}
> > > +
> > > +static void rzg2l_gpio_irq_enable(struct irq_data *d) {
> > > +     struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> > > +     struct rzg2l_pinctrl *pctrl = container_of(gc, struct
> > > +rzg2l_pinctrl,
> > > gpio_chip);
> > > +     unsigned int hwirq = irqd_to_hwirq(d);
> > > +     unsigned long flags;
> > > +     void __iomem *addr;
> > > +     u32 port;
> > > +     u8 bit;
> > > +
> > > +     port = RZG2L_PIN_ID_TO_PORT(hwirq);
> > > +     bit = RZG2L_PIN_ID_TO_PIN(hwirq);
> > > +
> > > +     addr = pctrl->base + ISEL(port);
> > > +     if (bit >= 4) {
> > > +             bit -= 4;
> > > +             addr += 4;
> > > +     }
> > > +
> > > +     spin_lock_irqsave(&pctrl->lock, flags);
> > > +     writel(readl(addr) | BIT(bit * 8), addr);
> > > +     spin_unlock_irqrestore(&pctrl->lock, flags);
> > > +
> > > +     irq_chip_enable_parent(d);
> > > +}
> > > +
> > > +static int rzg2l_gpio_irq_set_type(struct irq_data *d, unsigned int
> > > +type) {
> > > +     return irq_chip_set_type_parent(d, type); }
> > > +
> > > +static void rzg2l_gpio_irqc_eoi(struct irq_data *d) {
> > > +     irq_chip_eoi_parent(d);
> > > +}
> > > +
> > > +static struct irq_chip rzg2l_gpio_irqchip = {
> > > +     .name = "rzg2l-gpio",
> > > +     .irq_disable = rzg2l_gpio_irq_disable,
> > > +     .irq_enable = rzg2l_gpio_irq_enable,
> > > +     .irq_mask = irq_chip_mask_parent,
> > > +     .irq_unmask = irq_chip_unmask_parent,
> > > +     .irq_set_type = rzg2l_gpio_irq_set_type,
> > > +     .irq_eoi = rzg2l_gpio_irqc_eoi, };
> > > +
> > > +static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
> > > +                                         unsigned int child,
> > > +                                         unsigned int child_type,
> > > +                                         unsigned int *parent,
> > > +                                         unsigned int *parent_type)
> > > +{
> > > +     struct rzg2l_pinctrl *pctrl = gpiochip_get_data(gc);
> > > +     unsigned long flags;
> > > +     int gpioint, irq;
> > > +
> > > +     gpioint = rzg2l_gpio_get_gpioint(child);
> > > +     if (gpioint < 0)
> > > +             return gpioint;
> > > +
> > > +     spin_lock_irqsave(&pctrl->bitmap_lock, flags);
> > > +     irq = bitmap_find_free_region(pctrl->tint_slot,
> > > RZG2L_TINT_MAX_INTERRUPT, get_order(1));
> > > +     spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
> > > +     if (irq < 0)
> > > +             return -ENOSPC;
> > > +     pctrl->hwirq[irq] = child;
> > > +     irq += RZG2L_TINT_IRQ_START_INDEX;
> > > +
> > > +     /* All these interrupts are level high in the CPU */
> > > +     *parent_type = IRQ_TYPE_LEVEL_HIGH;
> > > +     *parent = RZG2L_PACK_HWIRQ(gpioint, irq);
> > > +     return 0;
> > > +}
> > > +
> > > +static void *rzg2l_gpio_populate_parent_fwspec(struct gpio_chip *chip,
> > > +                                            unsigned int parent_hwirq,
> > > +                                            unsigned int
> > > +parent_type) {
> > > +     struct irq_fwspec *fwspec;
> > > +
> > > +     fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
> > > +     if (!fwspec)
> > > +             return NULL;
> > > +
> > > +     fwspec->fwnode = chip->irq.parent_domain->fwnode;
> > > +     fwspec->param_count = 2;
> > > +     fwspec->param[0] = parent_hwirq;
> > > +     fwspec->param[1] = parent_type;
> > > +
> > > +     return fwspec;
> > > +}
> > > +
> > >  static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)  {
> > >       struct device_node *np = pctrl->dev->of_node;
> > >       struct gpio_chip *chip = &pctrl->gpio_chip;
> > >       const char *name = dev_name(pctrl->dev);
> > > +     struct irq_domain *parent_domain;
> > >       struct of_phandle_args of_args;
> > > +     struct device_node *parent_np;
> > > +     struct gpio_irq_chip *girq;
> > >       int ret;
> > >
> > > +     parent_np = of_irq_find_parent(np);
> > > +     if (!parent_np)
> > > +             return -ENXIO;
> > > +
> > > +     parent_domain = irq_find_host(parent_np);
> > > +     of_node_put(parent_np);
> > > +     if (!parent_domain)
> > > +             return -EPROBE_DEFER;
> > > +
> > >       ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3,
> > > 0, &of_args);
> > >       if (ret) {
> > >               dev_err(pctrl->dev, "Unable to parse gpio-ranges\n");
> > > @@ -
> > > 1138,6 +1333,15 @@ static int rzg2l_gpio_register(struct
> > > rzg2l_pinctrl
> > > *pctrl)
> > >       chip->base = -1;
> > >       chip->ngpio = of_args.args[2];
> > >
> > > +     girq = &chip->irq;
> > > +     girq->chip = &rzg2l_gpio_irqchip;
> > > +     girq->fwnode = of_node_to_fwnode(np);
> > > +     girq->parent_domain = parent_domain;
> > > +     girq->child_to_parent_hwirq = rzg2l_gpio_child_to_parent_hwirq;
> > > +     girq->populate_parent_alloc_arg =
> rzg2l_gpio_populate_parent_fwspec;
> > > +     girq->child_irq_domain_ops.free = rzg2l_gpio_irq_domain_free;
> > > +     girq->ngirq = RZG2L_TINT_MAX_INTERRUPT;
> > > +
> > >       pctrl->gpio_range.id = 0;
> > >       pctrl->gpio_range.pin_base = 0;
> > >       pctrl->gpio_range.base = 0;
> > > @@ -1253,6 +1457,7 @@ static int rzg2l_pinctrl_probe(struct
> > > platform_device
> > > *pdev)
> > >       }
> > >
> > >       spin_lock_init(&pctrl->lock);
> > > +     spin_lock_init(&pctrl->bitmap_lock);
> > >
> > >       platform_set_drvdata(pdev, pctrl);
> > >
> > > --
> > > 2.25.1
> >

^ permalink raw reply

* RE: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
From: Biju Das @ 2022-05-09  7:41 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Prabhakar Mahadev Lad, Thomas Gleixner, Marc Zyngier, Rob Herring,
	Krzysztof Kozlowski, Linus Walleij, Bartosz Golaszewski,
	Geert Uytterhoeven, Philipp Zabel, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <CA+V-a8uM=98O_WdaS1L=U3Eiqf_8Eteu9mDPats0a44W8VgpXA@mail.gmail.com>

Hi Prabhakar,

> Subject: Re: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller
> driver
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Mon, May 9, 2022 at 8:22 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Prabhakar,
> >
> > Thanks for the patch.
> >
> > > Subject: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt
> > > Controller driver
> > >
> > > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> > >
> > > This supports external pins being used as interrupts. It supports
> > > one line for NMI, 8 external pins and 32 GPIO pins (out of 123) to
> > > be used as IRQ lines.
> > >
> > > Signed-off-by: Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > >  drivers/irqchip/Kconfig             |   8 +
> > >  drivers/irqchip/Makefile            |   1 +
> > >  drivers/irqchip/irq-renesas-rzg2l.c | 445
> > > ++++++++++++++++++++++++++++
> > >  3 files changed, 454 insertions(+)
> > >  create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
> > >
> > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index
> > > 15edb9a6fcae..f3d071422f3b 100644
> > > --- a/drivers/irqchip/Kconfig
> > > +++ b/drivers/irqchip/Kconfig
> > > @@ -242,6 +242,14 @@ config RENESAS_RZA1_IRQC
> > >         Enable support for the Renesas RZ/A1 Interrupt Controller,
> > > to use up
> > >         to 8 external interrupts with configurable sense select.
> > >
> > > +config RENESAS_RZG2L_IRQC
> > > +     bool "Renesas RZ/G2L (and alike SoC) IRQC support" if
> COMPILE_TEST
> > > +     select GENERIC_IRQ_CHIP
> > > +     select IRQ_DOMAIN_HIERARCHY
> > > +     help
> > > +       Enable support for the Renesas RZ/G2L (and alike SoC)
> > > +Interrupt
> > > Controller
> > > +       for external devices.
> > > +
> > >  config SL28CPLD_INTC
> > >       bool "Kontron sl28cpld IRQ controller"
> > >       depends on MFD_SL28CPLD=y || COMPILE_TEST diff --git
> > > a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index
> > > 160a1d8ceaa9..eaa56eec2b23 100644
> > > --- a/drivers/irqchip/Makefile
> > > +++ b/drivers/irqchip/Makefile
> > > @@ -51,6 +51,7 @@ obj-$(CONFIG_RDA_INTC)                      += irq-
> rda-intc.o
> > >  obj-$(CONFIG_RENESAS_INTC_IRQPIN)    += irq-renesas-intc-irqpin.o
> > >  obj-$(CONFIG_RENESAS_IRQC)           += irq-renesas-irqc.o
> > >  obj-$(CONFIG_RENESAS_RZA1_IRQC)              += irq-renesas-rza1.o
> > > +obj-$(CONFIG_RENESAS_RZG2L_IRQC)     += irq-renesas-rzg2l.o
> > >  obj-$(CONFIG_VERSATILE_FPGA_IRQ)     += irq-versatile-fpga.o
> > >  obj-$(CONFIG_ARCH_NSPIRE)            += irq-zevio.o
> > >  obj-$(CONFIG_ARCH_VT8500)            += irq-vt8500.o
> > > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c
> > > b/drivers/irqchip/irq- renesas-rzg2l.c new file mode 100644 index
> > > 000000000000..bd6e82100caf
> > > --- /dev/null
> > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > @@ -0,0 +1,445 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Renesas RZ/G2L IRQC Driver
> > > + *
> > > + * Copyright (C) 2022 Renesas Electronics Corporation.
> > > + *
> > > + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/err.h>
> > > +#include <linux/io.h>
> > > +#include <linux/irqchip.h>
> > > +#include <linux/irqdomain.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/reset.h>
> > > +#include <linux/spinlock.h>
> > > +
> > > +#define IRQC_IRQ_START                       1
> > > +#define IRQC_IRQ_COUNT                       8
> > > +#define IRQC_TINT_START                      (IRQC_IRQ_START +
> IRQC_IRQ_COUNT)
> > > +#define IRQC_TINT_COUNT                      32
> > > +#define IRQC_NUM_IRQ                 (IRQC_TINT_START +
> IRQC_TINT_COUNT)
> > > +
> > > +#define ISCR                         0x10
> > > +#define IITSR                                0x14
> > > +#define TSCR                         0x20
> > > +#define TITSR0                               0x24
> > > +#define TITSR1                               0x28
> > > +#define TITSR0_MAX_INT                       16
> > > +#define TITSEL_WIDTH                 0x2
> > > +#define TSSR(n)                              (0x30 + ((n) * 4))
> > > +#define TIEN                         BIT(7)
> > > +#define TSSEL_SHIFT(n)                       (8 * (n))
> > > +#define TSSEL_MASK                   GENMASK(7, 0)
> > > +#define IRQ_MASK                     0x3
> > > +
> > > +#define TSSR_OFFSET(n)                       ((n) % 4)
> > > +#define TSSR_INDEX(n)                        ((n) / 4)
> > > +
> > > +#define TITSR_TITSEL_EDGE_RISING     0
> > > +#define TITSR_TITSEL_EDGE_FALLING    1
> > > +#define TITSR_TITSEL_LEVEL_HIGH              2
> > > +#define TITSR_TITSEL_LEVEL_LOW               3
> > > +
> > > +#define IITSR_IITSEL(n, sense)               ((sense) << ((n) * 2))
> > > +#define IITSR_IITSEL_LEVEL_LOW               0
> > > +#define IITSR_IITSEL_EDGE_FALLING    1
> > > +#define IITSR_IITSEL_EDGE_RISING     2
> > > +#define IITSR_IITSEL_EDGE_BOTH               3
> > > +#define IITSR_IITSEL_MASK(n)         IITSR_IITSEL((n), 3)
> > > +
> > > +#define TINT_EXTRACT_HWIRQ(x)                ((x) & ~GENMASK(31, 16))
> > > +#define TINT_EXTRACT_GPIOINT(x)              ((x) >> 16)
> > > +
> > > +struct rzg2l_irqc_priv {
> > > +     void __iomem *base;
> > > +     struct of_phandle_args map[IRQC_NUM_IRQ];
> > > +     raw_spinlock_t lock;
> > > +};
> > > +
> > > +struct rzg2l_irqc_chip_data {
> > > +     int tint;
> > > +};
> > > +
> > > +static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data
> > > +*data) {
> > > +     return data->domain->host_data; }
> > > +
> > > +static void rzg2l_irq_eoi(struct irq_data *d) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     u32 bit = BIT(hw_irq);
> > > +     u32 reg;
> > > +
> > > +     reg = readl_relaxed(priv->base + ISCR);
> > > +     if (reg & bit)
> > > +             writel_relaxed(reg & ~bit, priv->base + ISCR); }
> > > +
> > > +static void rzg2l_tint_eoi(struct irq_data *d) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_TINT_START;
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     u32 bit = BIT(hw_irq);
> > > +     u32 reg;
> > > +
> > > +     reg = readl_relaxed(priv->base + TSCR);
> > > +     if (reg & bit)
> > > +             writel_relaxed(reg & ~bit, priv->base + TSCR); }
> > > +
> > > +static void rzg2l_irqc_eoi(struct irq_data *d) {
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > > +
> > > +     raw_spin_lock(&priv->lock);
> > > +     if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> > > +             rzg2l_irq_eoi(d);
> > > +     else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> > > +             rzg2l_tint_eoi(d);
> > > +     raw_spin_unlock(&priv->lock);
> > > +     irq_chip_eoi_parent(d);
> > > +}
> > > +
> > > +static void rzg2l_irqc_irq_disable(struct irq_data *d) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > > +
> > > +     if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> > > +             struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +             u32 offset = hw_irq - IRQC_TINT_START;
> > > +             u32 tssr_offset = TSSR_OFFSET(offset);
> > > +             u8 tssr_index = TSSR_INDEX(offset);
> > > +             u32 reg;
> > > +
> > > +             raw_spin_lock(&priv->lock);
> > > +             reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > > +             reg &= ~(TSSEL_MASK << tssr_offset);
> > > +             writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > > +             raw_spin_unlock(&priv->lock);
> > > +     }
> > > +     irq_chip_disable_parent(d);
> > > +}
> > > +
> > > +static void rzg2l_irqc_irq_enable(struct irq_data *d) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > > +
> > > +     if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> > > +             struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +             struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> > > +             u32 offset = hw_irq - IRQC_TINT_START;
> > > +             u32 tssr_offset = TSSR_OFFSET(offset);
> > > +             u8 tssr_index = TSSR_INDEX(offset);
> > > +             u32 reg;
> > > +
> > > +             raw_spin_lock(&priv->lock);
> > > +             reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > > +             reg |= (TIEN | chip_data->tint) <<
> TSSEL_SHIFT(tssr_offset);
> > > +             writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > > +             raw_spin_unlock(&priv->lock);
> > > +     }
> > > +     irq_chip_enable_parent(d);
> > > +}
> > > +
> > > +static int rzg2l_irq_set_type(struct irq_data *d, unsigned int type) {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     u16 sense, tmp;
> > > +
> > > +     switch (type & IRQ_TYPE_SENSE_MASK) {
> > > +     case IRQ_TYPE_LEVEL_LOW:
> > > +             sense = IITSR_IITSEL_LEVEL_LOW;
> > > +             break;
> > > +
> > > +     case IRQ_TYPE_EDGE_FALLING:
> > > +             sense = IITSR_IITSEL_EDGE_FALLING;
> > > +             break;
> > > +
> > > +     case IRQ_TYPE_EDGE_RISING:
> > > +             sense = IITSR_IITSEL_EDGE_RISING;
> > > +             break;
> > > +
> > > +     case IRQ_TYPE_EDGE_BOTH:
> > > +             sense = IITSR_IITSEL_EDGE_BOTH;
> > > +             break;
> > > +
> > > +     default:
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     raw_spin_lock(&priv->lock);
> > > +     tmp = readl_relaxed(priv->base + IITSR);
> > > +     tmp &= ~IITSR_IITSEL_MASK(hw_irq);
> > > +     tmp |= IITSR_IITSEL(hw_irq, sense);
> > > +     writel_relaxed(tmp, priv->base + IITSR);
> > > +     raw_spin_unlock(&priv->lock);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type)
> {
> > > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > > +     unsigned int hwirq = irqd_to_hwirq(d);
> > > +     u32 titseln = hwirq - IRQC_TINT_START;
> > > +     u32 offset;
> > > +     u8 sense;
> > > +     u32 reg;
> > > +
> > > +     switch (type & IRQ_TYPE_SENSE_MASK) {
> > > +     case IRQ_TYPE_EDGE_RISING:
> > > +             sense = TITSR_TITSEL_EDGE_RISING;
> > > +             break;
> > > +
> > > +     case IRQ_TYPE_EDGE_FALLING:
> > > +             sense = TITSR_TITSEL_EDGE_FALLING;
> > > +             break;
> > > +
> > > +     default:
> > > +             return -EINVAL;
> > > +     }
> > > +
> >
> > > +     if (titseln < TITSR0_MAX_INT) {
> > > +             offset = TITSR0;
> > > +     } else {
> > > +             titseln /= TITSEL_WIDTH;
> > > +             offset  = TITSR1;
> > > +     }
> >
> > as TITSR0 (0x24) and TITSR1(0x28) are contiguous address location
> >
> > May be like others, above declare it as
> > u32 offset = TITSR0; ??
> >
> > and here
> >  if ((titseln >= TITSR0_MAX_INT) {
> >         titseln /= TITSEL_WIDTH;
> >         offset  += 4;
> >  }
> >
> The current implementation is suggested by the maintainer.

OK. I just suggested to optimize the code, since maintainer is
Ok with current implementation, I don't have any issue.

Cheers,
Biju

> 
> > > +
> > > +     raw_spin_lock(&priv->lock);
> > > +     reg = readl_relaxed(priv->base + offset);
> > > +     reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
> > > +     reg |= sense << (titseln * TITSEL_WIDTH);
> > > +     writel_relaxed(reg, priv->base + offset);
> > > +     raw_spin_unlock(&priv->lock);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
> {
> > > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > > +     int ret = -EINVAL;
> > > +
> > > +     if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> > > +             ret = rzg2l_irq_set_type(d, type);
> > > +     else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> > > +             ret = rzg2l_tint_set_edge(d, type);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     return irq_chip_set_type_parent(d, IRQ_TYPE_LEVEL_HIGH); }
> > > +
> > > +static struct irq_chip irqc_chip = {
> > > +     .name                   = "rzg2l-irqc",
> > > +     .irq_eoi                = rzg2l_irqc_eoi,
> > > +     .irq_mask               = irq_chip_mask_parent,
> > > +     .irq_unmask             = irq_chip_unmask_parent,
> > > +     .irq_disable            = rzg2l_irqc_irq_disable,
> > > +     .irq_enable             = rzg2l_irqc_irq_enable,
> > > +     .irq_get_irqchip_state  = irq_chip_get_parent_state,
> > > +     .irq_set_irqchip_state  = irq_chip_set_parent_state,
> > > +     .irq_retrigger          = irq_chip_retrigger_hierarchy,
> > > +     .irq_set_type           = rzg2l_irqc_set_type,
> > > +     .flags                  = IRQCHIP_MASK_ON_SUSPEND |
> > > +                               IRQCHIP_SET_TYPE_MASKED |
> > > +                               IRQCHIP_SKIP_SET_WAKE, };
> > > +
> > > +static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int
> virq,
> > > +                         unsigned int nr_irqs, void *arg) {
> > > +     struct rzg2l_irqc_priv *priv = domain->host_data;
> > > +     struct rzg2l_irqc_chip_data *chip_data = NULL;
> > > +     struct irq_fwspec spec;
> > > +     irq_hw_number_t hwirq;
> > > +     int tint = -EINVAL;
> > > +     unsigned int type;
> > > +     unsigned int i;
> > > +     int ret;
> > > +
> > > +     ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     /*
> > > +      * For TINIT interrupts ie where pinctrl driver is child of
> > > + irqc
> > > domain
> > > +      * the hwirq and TINT are encoded in fwspec->param[0].
> > > +      * hwirq for TINIT range from 9-40, hwirq is embedded 0-15
> > > + bits and
> > > TINT
> > > +      * from 16-31 bits. TINIT from the pinctrl driver needs to be
> > > programmed
> > > +      * in IRQC registers to enable a given gpio pin as interrupt.
> > > +      */
> > > +     if (hwirq > IRQC_IRQ_COUNT) {
> > > +             tint = TINT_EXTRACT_GPIOINT(hwirq);
> > > +             hwirq = TINT_EXTRACT_HWIRQ(hwirq);
> > > +     }
> > > +
> > > +     if (hwirq > (IRQC_NUM_IRQ - 1))
> > > +             return -EINVAL;
> >
> >
> > > +
> > > +     if (tint != -EINVAL && (hwirq < IRQC_TINT_START || hwirq >
> > > (IRQC_NUM_IRQ - 1)))
> > > +             return -EINVAL;
> >
> > hwirq > (IRQC_NUM_IRQ - 1) is redundant check, as it is checked above.
> >
> >
> Sure will drop it.
> 
> Cheers,
> Prabhakar
> 
> > Cheers,
> > Biju
> >
> > > +
> > > +     chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> > > +     if (!chip_data)
> > > +             return -ENOMEM;
> > > +     chip_data->tint = tint;
> > > +
> > > +     ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> &irqc_chip,
> > > +                                         chip_data);
> > > +     if (ret) {
> > > +             kfree(chip_data);
> > > +             return ret;
> > > +     }
> > > +
> > > +     spec.fwnode = domain->parent->fwnode;
> > > +     spec.param_count = priv->map[hwirq].args_count;
> > > +     for (i = 0; i < spec.param_count; i++)
> > > +             spec.param[i] = priv->map[hwirq].args[i];
> > > +
> > > +     ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &spec);
> > > +     if (ret)
> > > +             kfree(chip_data);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static void rzg2l_irqc_domain_free(struct irq_domain *domain,
> > > +unsigned int
> > > virq,
> > > +                                unsigned int nr_irqs) {
> > > +     struct irq_data *d;
> > > +
> > > +     d = irq_domain_get_irq_data(domain, virq);
> > > +     if (d) {
> > > +             struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> > > +
> > > +             kfree(chip_data);
> > > +     }
> > > +     irq_domain_free_irqs_common(domain, virq, nr_irqs); }
> > > +
> > > +static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
> > > +     .alloc = rzg2l_irqc_alloc,
> > > +     .free = rzg2l_irqc_domain_free,
> > > +     .translate = irq_domain_translate_twocell, };
> > > +
> > > +static int rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
> > > +                             struct device_node *np) {
> > > +     unsigned int i;
> > > +     int ret;
> > > +
> > > +     for (i = 0; i < IRQC_NUM_IRQ; i++) {
> > > +             ret = of_irq_parse_one(np, i, &priv->map[i]);
> > > +             if (ret)
> > > +                     return ret;
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int rzg2l_irqc_init(struct device_node *node, struct
> > > +device_node
> > > +*parent) {
> > > +     struct irq_domain *irq_domain, *parent_domain;
> > > +     struct reset_control *resetn;
> > > +     struct rzg2l_irqc_priv *priv;
> > > +     struct clk *clk;
> > > +     struct clk *pclk;
> > > +     int ret;
> > > +
> > > +     priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > > +     if (!priv)
> > > +             return -ENOMEM;
> > > +
> > > +     priv->base = of_iomap(node, 0);
> > > +     if (!priv->base) {
> > > +             ret = -ENXIO;
> > > +             goto free_priv;
> > > +     }
> > > +
> > > +     clk = of_clk_get_by_name(node, "clk");
> > > +     if (IS_ERR(clk)) {
> > > +             ret = IS_ERR(clk);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     pclk = of_clk_get_by_name(node, "pclk");
> > > +     if (IS_ERR(pclk)) {
> > > +             ret = IS_ERR(pclk);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     resetn = of_reset_control_get_exclusive_by_index(node, 0);
> > > +     if (IS_ERR(resetn)) {
> > > +             ret = IS_ERR(resetn);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     parent_domain = irq_find_host(parent);
> > > +     if (!parent_domain) {
> > > +             pr_err("%pOF: cannot find parent domain\n", node);
> > > +             ret = -ENODEV;
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     ret = rzg2l_irqc_parse_map(priv, node);
> > > +     if (ret) {
> > > +             pr_err("%pOF: cannot parse interrupts: %d\n", node, ret);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     ret = reset_control_deassert(resetn);
> > > +     if (ret) {
> > > +             pr_err("%pOF: failed to deassert resetn pin, %d\n",
> > > + node,
> > > ret);
> > > +             goto iounmap_base;
> > > +     }
> > > +
> > > +     raw_spin_lock_init(&priv->lock);
> > > +
> > > +     ret = clk_prepare_enable(clk);
> > > +     if (ret)
> > > +             goto assert_reset;
> > > +
> > > +     ret = clk_prepare_enable(pclk);
> > > +     if (ret)
> > > +             goto disable_clk;
> > > +
> > > +     irq_domain = irq_domain_add_hierarchy(parent_domain, 0,
> > > IRQC_NUM_IRQ,
> > > +                                           node,
> &rzg2l_irqc_domain_ops,
> > > +                                           priv);
> > > +     if (!irq_domain) {
> > > +             pr_err("%pOF: cannot initialize irq domain\n", node);
> > > +             ret = -ENOMEM;
> > > +             goto fail_irq_domain;
> > > +     }
> > > +
> > > +     return 0;
> > > +
> > > +fail_irq_domain:
> > > +     clk_disable_unprepare(pclk);
> > > +disable_clk:
> > > +     clk_disable_unprepare(clk);
> > > +assert_reset:
> > > +     reset_control_assert(resetn);
> > > +iounmap_base:
> > > +     iounmap(priv->base);
> > > +free_priv:
> > > +     kfree(priv);
> > > +     return ret;
> > > +}
> > > +
> > > +IRQCHIP_PLATFORM_DRIVER_BEGIN(rzg2l_irqc)
> > > +IRQCHIP_MATCH("renesas,rzg2l-irqc", rzg2l_irqc_init)
> > > +IRQCHIP_PLATFORM_DRIVER_END(rzg2l_irqc)
> > > +MODULE_AUTHOR("Lad Prabhakar
> > > +<prabhakar.mahadev-lad.rj@bp.renesas.com>");
> > > +MODULE_DESCRIPTION("Renesas RZ/G2L IRQC Driver");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.25.1
> >

^ permalink raw reply

* Re: [PATCH v1 1/1] pinctrl: stm32: Unshadow np variable in stm32_pctl_probe()
From: Fabien DESSENNE @ 2022-05-09  7:39 UTC (permalink / raw)
  To: Andy Shevchenko, Linus Walleij, linux-gpio, linux-stm32,
	linux-arm-kernel, linux-kernel
  Cc: Maxime Coquelin, Alexandre Torgue, kernel test robot
In-Reply-To: <20220507102257.26414-1-andriy.shevchenko@linux.intel.com>

Hi Andy


Thank you for the patch.

Will this patch be applied in Linus pinctrl tree, or in the gpio-intel 
tree before being merged (linux-next) in the pinctrl tree?

BR

Fabien



On 07/05/2022 12:22, Andy Shevchenko wrote:
> The np variable is used globally for stm32_pctl_probe() and in one of
> its code branches. cppcheck is not happy with that:
> 
>    pinctrl-stm32.c:1530:23: warning: Local variable 'np' shadows outer variable [shadowVariable]
> 
> Instead of simply renaming one of the variables convert some code to
> use a device pointer directly.
> 
> Fixes: bb949ed9b16b ("pinctrl: stm32: Switch to use for_each_gpiochip_node() helper")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Fabien Dessenne <fabien.dessenne@foss.st.com>



> ---
>   drivers/pinctrl/stm32/pinctrl-stm32.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> index e73f2d24611f..5d1d067b2247 100644
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -1364,8 +1364,9 @@ static int stm32_gpiolib_register_bank(struct stm32_pinctrl *pctl, struct fwnode
>   	return err;
>   }
>   
> -static struct irq_domain *stm32_pctrl_get_irq_domain(struct device_node *np)
> +static struct irq_domain *stm32_pctrl_get_irq_domain(struct platform_device *pdev)
>   {
> +	struct device_node *np = pdev->dev.of_node;
>   	struct device_node *parent;
>   	struct irq_domain *domain;
>   
> @@ -1482,23 +1483,19 @@ static int stm32_pctrl_create_pins_tab(struct stm32_pinctrl *pctl,
>   
>   int stm32_pctl_probe(struct platform_device *pdev)
>   {
> -	struct device_node *np = pdev->dev.of_node;
> +	const struct stm32_pinctrl_match_data *match_data;
>   	struct fwnode_handle *child;
> -	const struct of_device_id *match;
>   	struct device *dev = &pdev->dev;
>   	struct stm32_pinctrl *pctl;
>   	struct pinctrl_pin_desc *pins;
>   	int i, ret, hwlock_id;
>   	unsigned int banks;
>   
> -	if (!np)
> -		return -EINVAL;
> -
> -	match = of_match_device(dev->driver->of_match_table, dev);
> -	if (!match || !match->data)
> +	match_data = device_get_match_data(dev);
> +	if (!match_data)
>   		return -EINVAL;
>   
> -	if (!of_find_property(np, "pins-are-numbered", NULL)) {
> +	if (!device_property_present(dev, "pins-are-numbered")) {
>   		dev_err(dev, "only support pins-are-numbered format\n");
>   		return -EINVAL;
>   	}
> @@ -1510,7 +1507,7 @@ int stm32_pctl_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, pctl);
>   
>   	/* check for IRQ controller (may require deferred probe) */
> -	pctl->domain = stm32_pctrl_get_irq_domain(np);
> +	pctl->domain = stm32_pctrl_get_irq_domain(pdev);
>   	if (IS_ERR(pctl->domain))
>   		return PTR_ERR(pctl->domain);
>   
> @@ -1526,10 +1523,10 @@ int stm32_pctl_probe(struct platform_device *pdev)
>   	spin_lock_init(&pctl->irqmux_lock);
>   
>   	pctl->dev = dev;
> -	pctl->match_data = match->data;
> +	pctl->match_data = match_data;
>   
>   	/*  get optional package information */
> -	if (!of_property_read_u32(np, "st,package", &pctl->pkg))
> +	if (!device_property_read_u32(dev, "st,package", &pctl->pkg))
>   		dev_dbg(pctl->dev, "package detected: %x\n", pctl->pkg);
>   
>   	pctl->pins = devm_kcalloc(pctl->dev, pctl->match_data->npins,

^ permalink raw reply

* RE: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
From: Biju Das @ 2022-05-09  7:22 UTC (permalink / raw)
  To: Prabhakar Mahadev Lad, Thomas Gleixner, Marc Zyngier, Rob Herring,
	Krzysztof Kozlowski, Linus Walleij, Bartosz Golaszewski,
	Geert Uytterhoeven, Philipp Zabel, linux-gpio@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, Prabhakar,
	Prabhakar Mahadev Lad
In-Reply-To: <20220509050953.11005-3-prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi Prabhakar,

Thanks for the patch.

> Subject: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller
> driver
> 
> Add a driver for the Renesas RZ/G2L Interrupt Controller.
> 
> This supports external pins being used as interrupts. It supports one line
> for NMI, 8 external pins and 32 GPIO pins (out of 123) to be used as IRQ
> lines.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/irqchip/Kconfig             |   8 +
>  drivers/irqchip/Makefile            |   1 +
>  drivers/irqchip/irq-renesas-rzg2l.c | 445 ++++++++++++++++++++++++++++
>  3 files changed, 454 insertions(+)
>  create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index
> 15edb9a6fcae..f3d071422f3b 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -242,6 +242,14 @@ config RENESAS_RZA1_IRQC
>  	  Enable support for the Renesas RZ/A1 Interrupt Controller, to use
> up
>  	  to 8 external interrupts with configurable sense select.
> 
> +config RENESAS_RZG2L_IRQC
> +	bool "Renesas RZ/G2L (and alike SoC) IRQC support" if COMPILE_TEST
> +	select GENERIC_IRQ_CHIP
> +	select IRQ_DOMAIN_HIERARCHY
> +	help
> +	  Enable support for the Renesas RZ/G2L (and alike SoC) Interrupt
> Controller
> +	  for external devices.
> +
>  config SL28CPLD_INTC
>  	bool "Kontron sl28cpld IRQ controller"
>  	depends on MFD_SL28CPLD=y || COMPILE_TEST diff --git
> a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index
> 160a1d8ceaa9..eaa56eec2b23 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_RDA_INTC)			+= irq-rda-intc.o
>  obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
>  obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
>  obj-$(CONFIG_RENESAS_RZA1_IRQC)		+= irq-renesas-rza1.o
> +obj-$(CONFIG_RENESAS_RZG2L_IRQC)	+= irq-renesas-rzg2l.o
>  obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
>  obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
>  obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-
> renesas-rzg2l.c
> new file mode 100644
> index 000000000000..bd6e82100caf
> --- /dev/null
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -0,0 +1,445 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/G2L IRQC Driver
> + *
> + * Copyright (C) 2022 Renesas Electronics Corporation.
> + *
> + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of_address.h>
> +#include <linux/reset.h>
> +#include <linux/spinlock.h>
> +
> +#define IRQC_IRQ_START			1
> +#define IRQC_IRQ_COUNT			8
> +#define IRQC_TINT_START			(IRQC_IRQ_START + IRQC_IRQ_COUNT)
> +#define IRQC_TINT_COUNT			32
> +#define IRQC_NUM_IRQ			(IRQC_TINT_START + IRQC_TINT_COUNT)
> +
> +#define ISCR				0x10
> +#define IITSR				0x14
> +#define TSCR				0x20
> +#define TITSR0				0x24
> +#define TITSR1				0x28
> +#define TITSR0_MAX_INT			16
> +#define TITSEL_WIDTH			0x2
> +#define TSSR(n)				(0x30 + ((n) * 4))
> +#define TIEN				BIT(7)
> +#define TSSEL_SHIFT(n)			(8 * (n))
> +#define TSSEL_MASK			GENMASK(7, 0)
> +#define IRQ_MASK			0x3
> +
> +#define TSSR_OFFSET(n)			((n) % 4)
> +#define TSSR_INDEX(n)			((n) / 4)
> +
> +#define TITSR_TITSEL_EDGE_RISING	0
> +#define TITSR_TITSEL_EDGE_FALLING	1
> +#define TITSR_TITSEL_LEVEL_HIGH		2
> +#define TITSR_TITSEL_LEVEL_LOW		3
> +
> +#define IITSR_IITSEL(n, sense)		((sense) << ((n) * 2))
> +#define IITSR_IITSEL_LEVEL_LOW		0
> +#define IITSR_IITSEL_EDGE_FALLING	1
> +#define IITSR_IITSEL_EDGE_RISING	2
> +#define IITSR_IITSEL_EDGE_BOTH		3
> +#define IITSR_IITSEL_MASK(n)		IITSR_IITSEL((n), 3)
> +
> +#define TINT_EXTRACT_HWIRQ(x)		((x) & ~GENMASK(31, 16))
> +#define TINT_EXTRACT_GPIOINT(x)		((x) >> 16)
> +
> +struct rzg2l_irqc_priv {
> +	void __iomem *base;
> +	struct of_phandle_args map[IRQC_NUM_IRQ];
> +	raw_spinlock_t lock;
> +};
> +
> +struct rzg2l_irqc_chip_data {
> +	int tint;
> +};
> +
> +static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data *data)
> +{
> +	return data->domain->host_data;
> +}
> +
> +static void rzg2l_irq_eoi(struct irq_data *d) {
> +	unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> +	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> +	u32 bit = BIT(hw_irq);
> +	u32 reg;
> +
> +	reg = readl_relaxed(priv->base + ISCR);
> +	if (reg & bit)
> +		writel_relaxed(reg & ~bit, priv->base + ISCR); }
> +
> +static void rzg2l_tint_eoi(struct irq_data *d) {
> +	unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_TINT_START;
> +	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> +	u32 bit = BIT(hw_irq);
> +	u32 reg;
> +
> +	reg = readl_relaxed(priv->base + TSCR);
> +	if (reg & bit)
> +		writel_relaxed(reg & ~bit, priv->base + TSCR); }
> +
> +static void rzg2l_irqc_eoi(struct irq_data *d) {
> +	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> +	unsigned int hw_irq = irqd_to_hwirq(d);
> +
> +	raw_spin_lock(&priv->lock);
> +	if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> +		rzg2l_irq_eoi(d);
> +	else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> +		rzg2l_tint_eoi(d);
> +	raw_spin_unlock(&priv->lock);
> +	irq_chip_eoi_parent(d);
> +}
> +
> +static void rzg2l_irqc_irq_disable(struct irq_data *d) {
> +	unsigned int hw_irq = irqd_to_hwirq(d);
> +
> +	if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> +		struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> +		u32 offset = hw_irq - IRQC_TINT_START;
> +		u32 tssr_offset = TSSR_OFFSET(offset);
> +		u8 tssr_index = TSSR_INDEX(offset);
> +		u32 reg;
> +
> +		raw_spin_lock(&priv->lock);
> +		reg = readl_relaxed(priv->base + TSSR(tssr_index));
> +		reg &= ~(TSSEL_MASK << tssr_offset);
> +		writel_relaxed(reg, priv->base + TSSR(tssr_index));
> +		raw_spin_unlock(&priv->lock);
> +	}
> +	irq_chip_disable_parent(d);
> +}
> +
> +static void rzg2l_irqc_irq_enable(struct irq_data *d) {
> +	unsigned int hw_irq = irqd_to_hwirq(d);
> +
> +	if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> +		struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> +		struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> +		u32 offset = hw_irq - IRQC_TINT_START;
> +		u32 tssr_offset = TSSR_OFFSET(offset);
> +		u8 tssr_index = TSSR_INDEX(offset);
> +		u32 reg;
> +
> +		raw_spin_lock(&priv->lock);
> +		reg = readl_relaxed(priv->base + TSSR(tssr_index));
> +		reg |= (TIEN | chip_data->tint) << TSSEL_SHIFT(tssr_offset);
> +		writel_relaxed(reg, priv->base + TSSR(tssr_index));
> +		raw_spin_unlock(&priv->lock);
> +	}
> +	irq_chip_enable_parent(d);
> +}
> +
> +static int rzg2l_irq_set_type(struct irq_data *d, unsigned int type) {
> +	unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> +	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> +	u16 sense, tmp;
> +
> +	switch (type & IRQ_TYPE_SENSE_MASK) {
> +	case IRQ_TYPE_LEVEL_LOW:
> +		sense = IITSR_IITSEL_LEVEL_LOW;
> +		break;
> +
> +	case IRQ_TYPE_EDGE_FALLING:
> +		sense = IITSR_IITSEL_EDGE_FALLING;
> +		break;
> +
> +	case IRQ_TYPE_EDGE_RISING:
> +		sense = IITSR_IITSEL_EDGE_RISING;
> +		break;
> +
> +	case IRQ_TYPE_EDGE_BOTH:
> +		sense = IITSR_IITSEL_EDGE_BOTH;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	raw_spin_lock(&priv->lock);
> +	tmp = readl_relaxed(priv->base + IITSR);
> +	tmp &= ~IITSR_IITSEL_MASK(hw_irq);
> +	tmp |= IITSR_IITSEL(hw_irq, sense);
> +	writel_relaxed(tmp, priv->base + IITSR);
> +	raw_spin_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type) {
> +	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> +	unsigned int hwirq = irqd_to_hwirq(d);
> +	u32 titseln = hwirq - IRQC_TINT_START;
> +	u32 offset;
> +	u8 sense;
> +	u32 reg;
> +
> +	switch (type & IRQ_TYPE_SENSE_MASK) {
> +	case IRQ_TYPE_EDGE_RISING:
> +		sense = TITSR_TITSEL_EDGE_RISING;
> +		break;
> +
> +	case IRQ_TYPE_EDGE_FALLING:
> +		sense = TITSR_TITSEL_EDGE_FALLING;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +

> +	if (titseln < TITSR0_MAX_INT) {
> +		offset = TITSR0;
> +	} else {
> +		titseln /= TITSEL_WIDTH;
> +		offset  = TITSR1;
> +	}

as TITSR0 (0x24) and TITSR1(0x28) are contiguous address location

May be like others, above declare it as
u32 offset = TITSR0; ??

and here
 if ((titseln >= TITSR0_MAX_INT) {
	titseln /= TITSEL_WIDTH;
	offset  += 4;
 }

> +
> +	raw_spin_lock(&priv->lock);
> +	reg = readl_relaxed(priv->base + offset);
> +	reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
> +	reg |= sense << (titseln * TITSEL_WIDTH);
> +	writel_relaxed(reg, priv->base + offset);
> +	raw_spin_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type) {
> +	unsigned int hw_irq = irqd_to_hwirq(d);
> +	int ret = -EINVAL;
> +
> +	if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> +		ret = rzg2l_irq_set_type(d, type);
> +	else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> +		ret = rzg2l_tint_set_edge(d, type);
> +	if (ret)
> +		return ret;
> +
> +	return irq_chip_set_type_parent(d, IRQ_TYPE_LEVEL_HIGH); }
> +
> +static struct irq_chip irqc_chip = {
> +	.name			= "rzg2l-irqc",
> +	.irq_eoi		= rzg2l_irqc_eoi,
> +	.irq_mask		= irq_chip_mask_parent,
> +	.irq_unmask		= irq_chip_unmask_parent,
> +	.irq_disable		= rzg2l_irqc_irq_disable,
> +	.irq_enable		= rzg2l_irqc_irq_enable,
> +	.irq_get_irqchip_state	= irq_chip_get_parent_state,
> +	.irq_set_irqchip_state	= irq_chip_set_parent_state,
> +	.irq_retrigger		= irq_chip_retrigger_hierarchy,
> +	.irq_set_type		= rzg2l_irqc_set_type,
> +	.flags			= IRQCHIP_MASK_ON_SUSPEND |
> +				  IRQCHIP_SET_TYPE_MASKED |
> +				  IRQCHIP_SKIP_SET_WAKE,
> +};
> +
> +static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int virq,
> +			    unsigned int nr_irqs, void *arg) {
> +	struct rzg2l_irqc_priv *priv = domain->host_data;
> +	struct rzg2l_irqc_chip_data *chip_data = NULL;
> +	struct irq_fwspec spec;
> +	irq_hw_number_t hwirq;
> +	int tint = -EINVAL;
> +	unsigned int type;
> +	unsigned int i;
> +	int ret;
> +
> +	ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * For TINIT interrupts ie where pinctrl driver is child of irqc
> domain
> +	 * the hwirq and TINT are encoded in fwspec->param[0].
> +	 * hwirq for TINIT range from 9-40, hwirq is embedded 0-15 bits and
> TINT
> +	 * from 16-31 bits. TINIT from the pinctrl driver needs to be
> programmed
> +	 * in IRQC registers to enable a given gpio pin as interrupt.
> +	 */
> +	if (hwirq > IRQC_IRQ_COUNT) {
> +		tint = TINT_EXTRACT_GPIOINT(hwirq);
> +		hwirq = TINT_EXTRACT_HWIRQ(hwirq);
> +	}
> +
> +	if (hwirq > (IRQC_NUM_IRQ - 1))
> +		return -EINVAL;


> +
> +	if (tint != -EINVAL && (hwirq < IRQC_TINT_START || hwirq >
> (IRQC_NUM_IRQ - 1)))
> +		return -EINVAL;

hwirq > (IRQC_NUM_IRQ - 1) is redundant check, as it is checked above.


Cheers,
Biju

> +
> +	chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> +	if (!chip_data)
> +		return -ENOMEM;
> +	chip_data->tint = tint;
> +
> +	ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &irqc_chip,
> +					    chip_data);
> +	if (ret) {
> +		kfree(chip_data);
> +		return ret;
> +	}
> +
> +	spec.fwnode = domain->parent->fwnode;
> +	spec.param_count = priv->map[hwirq].args_count;
> +	for (i = 0; i < spec.param_count; i++)
> +		spec.param[i] = priv->map[hwirq].args[i];
> +
> +	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &spec);
> +	if (ret)
> +		kfree(chip_data);
> +
> +	return ret;
> +}
> +
> +static void rzg2l_irqc_domain_free(struct irq_domain *domain, unsigned int
> virq,
> +				   unsigned int nr_irqs)
> +{
> +	struct irq_data *d;
> +
> +	d = irq_domain_get_irq_data(domain, virq);
> +	if (d) {
> +		struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> +
> +		kfree(chip_data);
> +	}
> +	irq_domain_free_irqs_common(domain, virq, nr_irqs); }
> +
> +static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
> +	.alloc = rzg2l_irqc_alloc,
> +	.free = rzg2l_irqc_domain_free,
> +	.translate = irq_domain_translate_twocell, };
> +
> +static int rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
> +				struct device_node *np)
> +{
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < IRQC_NUM_IRQ; i++) {
> +		ret = of_irq_parse_one(np, i, &priv->map[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rzg2l_irqc_init(struct device_node *node, struct device_node
> +*parent) {
> +	struct irq_domain *irq_domain, *parent_domain;
> +	struct reset_control *resetn;
> +	struct rzg2l_irqc_priv *priv;
> +	struct clk *clk;
> +	struct clk *pclk;
> +	int ret;
> +
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->base = of_iomap(node, 0);
> +	if (!priv->base) {
> +		ret = -ENXIO;
> +		goto free_priv;
> +	}
> +
> +	clk = of_clk_get_by_name(node, "clk");
> +	if (IS_ERR(clk)) {
> +		ret = IS_ERR(clk);
> +		goto iounmap_base;
> +	}
> +
> +	pclk = of_clk_get_by_name(node, "pclk");
> +	if (IS_ERR(pclk)) {
> +		ret = IS_ERR(pclk);
> +		goto iounmap_base;
> +	}
> +
> +	resetn = of_reset_control_get_exclusive_by_index(node, 0);
> +	if (IS_ERR(resetn)) {
> +		ret = IS_ERR(resetn);
> +		goto iounmap_base;
> +	}
> +
> +	parent_domain = irq_find_host(parent);
> +	if (!parent_domain) {
> +		pr_err("%pOF: cannot find parent domain\n", node);
> +		ret = -ENODEV;
> +		goto iounmap_base;
> +	}
> +
> +	ret = rzg2l_irqc_parse_map(priv, node);
> +	if (ret) {
> +		pr_err("%pOF: cannot parse interrupts: %d\n", node, ret);
> +		goto iounmap_base;
> +	}
> +
> +	ret = reset_control_deassert(resetn);
> +	if (ret) {
> +		pr_err("%pOF: failed to deassert resetn pin, %d\n", node,
> ret);
> +		goto iounmap_base;
> +	}
> +
> +	raw_spin_lock_init(&priv->lock);
> +
> +	ret = clk_prepare_enable(clk);
> +	if (ret)
> +		goto assert_reset;
> +
> +	ret = clk_prepare_enable(pclk);
> +	if (ret)
> +		goto disable_clk;
> +
> +	irq_domain = irq_domain_add_hierarchy(parent_domain, 0,
> IRQC_NUM_IRQ,
> +					      node, &rzg2l_irqc_domain_ops,
> +					      priv);
> +	if (!irq_domain) {
> +		pr_err("%pOF: cannot initialize irq domain\n", node);
> +		ret = -ENOMEM;
> +		goto fail_irq_domain;
> +	}
> +
> +	return 0;
> +
> +fail_irq_domain:
> +	clk_disable_unprepare(pclk);
> +disable_clk:
> +	clk_disable_unprepare(clk);
> +assert_reset:
> +	reset_control_assert(resetn);
> +iounmap_base:
> +	iounmap(priv->base);
> +free_priv:
> +	kfree(priv);
> +	return ret;
> +}
> +
> +IRQCHIP_PLATFORM_DRIVER_BEGIN(rzg2l_irqc)
> +IRQCHIP_MATCH("renesas,rzg2l-irqc", rzg2l_irqc_init)
> +IRQCHIP_PLATFORM_DRIVER_END(rzg2l_irqc)
> +MODULE_AUTHOR("Lad Prabhakar
> +<prabhakar.mahadev-lad.rj@bp.renesas.com>");
> +MODULE_DESCRIPTION("Renesas RZ/G2L IRQC Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1


^ permalink raw reply

* Re: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
From: Lad, Prabhakar @ 2022-05-09  7:35 UTC (permalink / raw)
  To: Biju Das
  Cc: Prabhakar Mahadev Lad, Thomas Gleixner, Marc Zyngier, Rob Herring,
	Krzysztof Kozlowski, Linus Walleij, Bartosz Golaszewski,
	Geert Uytterhoeven, Philipp Zabel, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
In-Reply-To: <OS0PR01MB5922B58BB70B92813041745786C69@OS0PR01MB5922.jpnprd01.prod.outlook.com>

Hi Biju,

Thank you for the patch.

On Mon, May 9, 2022 at 8:22 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Prabhakar,
>
> Thanks for the patch.
>
> > Subject: [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller
> > driver
> >
> > Add a driver for the Renesas RZ/G2L Interrupt Controller.
> >
> > This supports external pins being used as interrupts. It supports one line
> > for NMI, 8 external pins and 32 GPIO pins (out of 123) to be used as IRQ
> > lines.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/irqchip/Kconfig             |   8 +
> >  drivers/irqchip/Makefile            |   1 +
> >  drivers/irqchip/irq-renesas-rzg2l.c | 445 ++++++++++++++++++++++++++++
> >  3 files changed, 454 insertions(+)
> >  create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c
> >
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index
> > 15edb9a6fcae..f3d071422f3b 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -242,6 +242,14 @@ config RENESAS_RZA1_IRQC
> >         Enable support for the Renesas RZ/A1 Interrupt Controller, to use
> > up
> >         to 8 external interrupts with configurable sense select.
> >
> > +config RENESAS_RZG2L_IRQC
> > +     bool "Renesas RZ/G2L (and alike SoC) IRQC support" if COMPILE_TEST
> > +     select GENERIC_IRQ_CHIP
> > +     select IRQ_DOMAIN_HIERARCHY
> > +     help
> > +       Enable support for the Renesas RZ/G2L (and alike SoC) Interrupt
> > Controller
> > +       for external devices.
> > +
> >  config SL28CPLD_INTC
> >       bool "Kontron sl28cpld IRQ controller"
> >       depends on MFD_SL28CPLD=y || COMPILE_TEST diff --git
> > a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index
> > 160a1d8ceaa9..eaa56eec2b23 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -51,6 +51,7 @@ obj-$(CONFIG_RDA_INTC)                      += irq-rda-intc.o
> >  obj-$(CONFIG_RENESAS_INTC_IRQPIN)    += irq-renesas-intc-irqpin.o
> >  obj-$(CONFIG_RENESAS_IRQC)           += irq-renesas-irqc.o
> >  obj-$(CONFIG_RENESAS_RZA1_IRQC)              += irq-renesas-rza1.o
> > +obj-$(CONFIG_RENESAS_RZG2L_IRQC)     += irq-renesas-rzg2l.o
> >  obj-$(CONFIG_VERSATILE_FPGA_IRQ)     += irq-versatile-fpga.o
> >  obj-$(CONFIG_ARCH_NSPIRE)            += irq-zevio.o
> >  obj-$(CONFIG_ARCH_VT8500)            += irq-vt8500.o
> > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-
> > renesas-rzg2l.c
> > new file mode 100644
> > index 000000000000..bd6e82100caf
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > @@ -0,0 +1,445 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas RZ/G2L IRQC Driver
> > + *
> > + * Copyright (C) 2022 Renesas Electronics Corporation.
> > + *
> > + * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/irqchip.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/of_address.h>
> > +#include <linux/reset.h>
> > +#include <linux/spinlock.h>
> > +
> > +#define IRQC_IRQ_START                       1
> > +#define IRQC_IRQ_COUNT                       8
> > +#define IRQC_TINT_START                      (IRQC_IRQ_START + IRQC_IRQ_COUNT)
> > +#define IRQC_TINT_COUNT                      32
> > +#define IRQC_NUM_IRQ                 (IRQC_TINT_START + IRQC_TINT_COUNT)
> > +
> > +#define ISCR                         0x10
> > +#define IITSR                                0x14
> > +#define TSCR                         0x20
> > +#define TITSR0                               0x24
> > +#define TITSR1                               0x28
> > +#define TITSR0_MAX_INT                       16
> > +#define TITSEL_WIDTH                 0x2
> > +#define TSSR(n)                              (0x30 + ((n) * 4))
> > +#define TIEN                         BIT(7)
> > +#define TSSEL_SHIFT(n)                       (8 * (n))
> > +#define TSSEL_MASK                   GENMASK(7, 0)
> > +#define IRQ_MASK                     0x3
> > +
> > +#define TSSR_OFFSET(n)                       ((n) % 4)
> > +#define TSSR_INDEX(n)                        ((n) / 4)
> > +
> > +#define TITSR_TITSEL_EDGE_RISING     0
> > +#define TITSR_TITSEL_EDGE_FALLING    1
> > +#define TITSR_TITSEL_LEVEL_HIGH              2
> > +#define TITSR_TITSEL_LEVEL_LOW               3
> > +
> > +#define IITSR_IITSEL(n, sense)               ((sense) << ((n) * 2))
> > +#define IITSR_IITSEL_LEVEL_LOW               0
> > +#define IITSR_IITSEL_EDGE_FALLING    1
> > +#define IITSR_IITSEL_EDGE_RISING     2
> > +#define IITSR_IITSEL_EDGE_BOTH               3
> > +#define IITSR_IITSEL_MASK(n)         IITSR_IITSEL((n), 3)
> > +
> > +#define TINT_EXTRACT_HWIRQ(x)                ((x) & ~GENMASK(31, 16))
> > +#define TINT_EXTRACT_GPIOINT(x)              ((x) >> 16)
> > +
> > +struct rzg2l_irqc_priv {
> > +     void __iomem *base;
> > +     struct of_phandle_args map[IRQC_NUM_IRQ];
> > +     raw_spinlock_t lock;
> > +};
> > +
> > +struct rzg2l_irqc_chip_data {
> > +     int tint;
> > +};
> > +
> > +static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data *data)
> > +{
> > +     return data->domain->host_data;
> > +}
> > +
> > +static void rzg2l_irq_eoi(struct irq_data *d) {
> > +     unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +     u32 bit = BIT(hw_irq);
> > +     u32 reg;
> > +
> > +     reg = readl_relaxed(priv->base + ISCR);
> > +     if (reg & bit)
> > +             writel_relaxed(reg & ~bit, priv->base + ISCR); }
> > +
> > +static void rzg2l_tint_eoi(struct irq_data *d) {
> > +     unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_TINT_START;
> > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +     u32 bit = BIT(hw_irq);
> > +     u32 reg;
> > +
> > +     reg = readl_relaxed(priv->base + TSCR);
> > +     if (reg & bit)
> > +             writel_relaxed(reg & ~bit, priv->base + TSCR); }
> > +
> > +static void rzg2l_irqc_eoi(struct irq_data *d) {
> > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > +
> > +     raw_spin_lock(&priv->lock);
> > +     if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> > +             rzg2l_irq_eoi(d);
> > +     else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> > +             rzg2l_tint_eoi(d);
> > +     raw_spin_unlock(&priv->lock);
> > +     irq_chip_eoi_parent(d);
> > +}
> > +
> > +static void rzg2l_irqc_irq_disable(struct irq_data *d) {
> > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > +
> > +     if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> > +             struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +             u32 offset = hw_irq - IRQC_TINT_START;
> > +             u32 tssr_offset = TSSR_OFFSET(offset);
> > +             u8 tssr_index = TSSR_INDEX(offset);
> > +             u32 reg;
> > +
> > +             raw_spin_lock(&priv->lock);
> > +             reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > +             reg &= ~(TSSEL_MASK << tssr_offset);
> > +             writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > +             raw_spin_unlock(&priv->lock);
> > +     }
> > +     irq_chip_disable_parent(d);
> > +}
> > +
> > +static void rzg2l_irqc_irq_enable(struct irq_data *d) {
> > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > +
> > +     if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
> > +             struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +             struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> > +             u32 offset = hw_irq - IRQC_TINT_START;
> > +             u32 tssr_offset = TSSR_OFFSET(offset);
> > +             u8 tssr_index = TSSR_INDEX(offset);
> > +             u32 reg;
> > +
> > +             raw_spin_lock(&priv->lock);
> > +             reg = readl_relaxed(priv->base + TSSR(tssr_index));
> > +             reg |= (TIEN | chip_data->tint) << TSSEL_SHIFT(tssr_offset);
> > +             writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > +             raw_spin_unlock(&priv->lock);
> > +     }
> > +     irq_chip_enable_parent(d);
> > +}
> > +
> > +static int rzg2l_irq_set_type(struct irq_data *d, unsigned int type) {
> > +     unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
> > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +     u16 sense, tmp;
> > +
> > +     switch (type & IRQ_TYPE_SENSE_MASK) {
> > +     case IRQ_TYPE_LEVEL_LOW:
> > +             sense = IITSR_IITSEL_LEVEL_LOW;
> > +             break;
> > +
> > +     case IRQ_TYPE_EDGE_FALLING:
> > +             sense = IITSR_IITSEL_EDGE_FALLING;
> > +             break;
> > +
> > +     case IRQ_TYPE_EDGE_RISING:
> > +             sense = IITSR_IITSEL_EDGE_RISING;
> > +             break;
> > +
> > +     case IRQ_TYPE_EDGE_BOTH:
> > +             sense = IITSR_IITSEL_EDGE_BOTH;
> > +             break;
> > +
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
> > +     raw_spin_lock(&priv->lock);
> > +     tmp = readl_relaxed(priv->base + IITSR);
> > +     tmp &= ~IITSR_IITSEL_MASK(hw_irq);
> > +     tmp |= IITSR_IITSEL(hw_irq, sense);
> > +     writel_relaxed(tmp, priv->base + IITSR);
> > +     raw_spin_unlock(&priv->lock);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type) {
> > +     struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> > +     unsigned int hwirq = irqd_to_hwirq(d);
> > +     u32 titseln = hwirq - IRQC_TINT_START;
> > +     u32 offset;
> > +     u8 sense;
> > +     u32 reg;
> > +
> > +     switch (type & IRQ_TYPE_SENSE_MASK) {
> > +     case IRQ_TYPE_EDGE_RISING:
> > +             sense = TITSR_TITSEL_EDGE_RISING;
> > +             break;
> > +
> > +     case IRQ_TYPE_EDGE_FALLING:
> > +             sense = TITSR_TITSEL_EDGE_FALLING;
> > +             break;
> > +
> > +     default:
> > +             return -EINVAL;
> > +     }
> > +
>
> > +     if (titseln < TITSR0_MAX_INT) {
> > +             offset = TITSR0;
> > +     } else {
> > +             titseln /= TITSEL_WIDTH;
> > +             offset  = TITSR1;
> > +     }
>
> as TITSR0 (0x24) and TITSR1(0x28) are contiguous address location
>
> May be like others, above declare it as
> u32 offset = TITSR0; ??
>
> and here
>  if ((titseln >= TITSR0_MAX_INT) {
>         titseln /= TITSEL_WIDTH;
>         offset  += 4;
>  }
>
The current implementation is suggested by the maintainer.

> > +
> > +     raw_spin_lock(&priv->lock);
> > +     reg = readl_relaxed(priv->base + offset);
> > +     reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
> > +     reg |= sense << (titseln * TITSEL_WIDTH);
> > +     writel_relaxed(reg, priv->base + offset);
> > +     raw_spin_unlock(&priv->lock);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type) {
> > +     unsigned int hw_irq = irqd_to_hwirq(d);
> > +     int ret = -EINVAL;
> > +
> > +     if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> > +             ret = rzg2l_irq_set_type(d, type);
> > +     else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
> > +             ret = rzg2l_tint_set_edge(d, type);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return irq_chip_set_type_parent(d, IRQ_TYPE_LEVEL_HIGH); }
> > +
> > +static struct irq_chip irqc_chip = {
> > +     .name                   = "rzg2l-irqc",
> > +     .irq_eoi                = rzg2l_irqc_eoi,
> > +     .irq_mask               = irq_chip_mask_parent,
> > +     .irq_unmask             = irq_chip_unmask_parent,
> > +     .irq_disable            = rzg2l_irqc_irq_disable,
> > +     .irq_enable             = rzg2l_irqc_irq_enable,
> > +     .irq_get_irqchip_state  = irq_chip_get_parent_state,
> > +     .irq_set_irqchip_state  = irq_chip_set_parent_state,
> > +     .irq_retrigger          = irq_chip_retrigger_hierarchy,
> > +     .irq_set_type           = rzg2l_irqc_set_type,
> > +     .flags                  = IRQCHIP_MASK_ON_SUSPEND |
> > +                               IRQCHIP_SET_TYPE_MASKED |
> > +                               IRQCHIP_SKIP_SET_WAKE,
> > +};
> > +
> > +static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int virq,
> > +                         unsigned int nr_irqs, void *arg) {
> > +     struct rzg2l_irqc_priv *priv = domain->host_data;
> > +     struct rzg2l_irqc_chip_data *chip_data = NULL;
> > +     struct irq_fwspec spec;
> > +     irq_hw_number_t hwirq;
> > +     int tint = -EINVAL;
> > +     unsigned int type;
> > +     unsigned int i;
> > +     int ret;
> > +
> > +     ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /*
> > +      * For TINIT interrupts ie where pinctrl driver is child of irqc
> > domain
> > +      * the hwirq and TINT are encoded in fwspec->param[0].
> > +      * hwirq for TINIT range from 9-40, hwirq is embedded 0-15 bits and
> > TINT
> > +      * from 16-31 bits. TINIT from the pinctrl driver needs to be
> > programmed
> > +      * in IRQC registers to enable a given gpio pin as interrupt.
> > +      */
> > +     if (hwirq > IRQC_IRQ_COUNT) {
> > +             tint = TINT_EXTRACT_GPIOINT(hwirq);
> > +             hwirq = TINT_EXTRACT_HWIRQ(hwirq);
> > +     }
> > +
> > +     if (hwirq > (IRQC_NUM_IRQ - 1))
> > +             return -EINVAL;
>
>
> > +
> > +     if (tint != -EINVAL && (hwirq < IRQC_TINT_START || hwirq >
> > (IRQC_NUM_IRQ - 1)))
> > +             return -EINVAL;
>
> hwirq > (IRQC_NUM_IRQ - 1) is redundant check, as it is checked above.
>
>
Sure will drop it.

Cheers,
Prabhakar

> Cheers,
> Biju
>
> > +
> > +     chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> > +     if (!chip_data)
> > +             return -ENOMEM;
> > +     chip_data->tint = tint;
> > +
> > +     ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &irqc_chip,
> > +                                         chip_data);
> > +     if (ret) {
> > +             kfree(chip_data);
> > +             return ret;
> > +     }
> > +
> > +     spec.fwnode = domain->parent->fwnode;
> > +     spec.param_count = priv->map[hwirq].args_count;
> > +     for (i = 0; i < spec.param_count; i++)
> > +             spec.param[i] = priv->map[hwirq].args[i];
> > +
> > +     ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &spec);
> > +     if (ret)
> > +             kfree(chip_data);
> > +
> > +     return ret;
> > +}
> > +
> > +static void rzg2l_irqc_domain_free(struct irq_domain *domain, unsigned int
> > virq,
> > +                                unsigned int nr_irqs)
> > +{
> > +     struct irq_data *d;
> > +
> > +     d = irq_domain_get_irq_data(domain, virq);
> > +     if (d) {
> > +             struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
> > +
> > +             kfree(chip_data);
> > +     }
> > +     irq_domain_free_irqs_common(domain, virq, nr_irqs); }
> > +
> > +static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
> > +     .alloc = rzg2l_irqc_alloc,
> > +     .free = rzg2l_irqc_domain_free,
> > +     .translate = irq_domain_translate_twocell, };
> > +
> > +static int rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
> > +                             struct device_node *np)
> > +{
> > +     unsigned int i;
> > +     int ret;
> > +
> > +     for (i = 0; i < IRQC_NUM_IRQ; i++) {
> > +             ret = of_irq_parse_one(np, i, &priv->map[i]);
> > +             if (ret)
> > +                     return ret;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int rzg2l_irqc_init(struct device_node *node, struct device_node
> > +*parent) {
> > +     struct irq_domain *irq_domain, *parent_domain;
> > +     struct reset_control *resetn;
> > +     struct rzg2l_irqc_priv *priv;
> > +     struct clk *clk;
> > +     struct clk *pclk;
> > +     int ret;
> > +
> > +     priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > +     if (!priv)
> > +             return -ENOMEM;
> > +
> > +     priv->base = of_iomap(node, 0);
> > +     if (!priv->base) {
> > +             ret = -ENXIO;
> > +             goto free_priv;
> > +     }
> > +
> > +     clk = of_clk_get_by_name(node, "clk");
> > +     if (IS_ERR(clk)) {
> > +             ret = IS_ERR(clk);
> > +             goto iounmap_base;
> > +     }
> > +
> > +     pclk = of_clk_get_by_name(node, "pclk");
> > +     if (IS_ERR(pclk)) {
> > +             ret = IS_ERR(pclk);
> > +             goto iounmap_base;
> > +     }
> > +
> > +     resetn = of_reset_control_get_exclusive_by_index(node, 0);
> > +     if (IS_ERR(resetn)) {
> > +             ret = IS_ERR(resetn);
> > +             goto iounmap_base;
> > +     }
> > +
> > +     parent_domain = irq_find_host(parent);
> > +     if (!parent_domain) {
> > +             pr_err("%pOF: cannot find parent domain\n", node);
> > +             ret = -ENODEV;
> > +             goto iounmap_base;
> > +     }
> > +
> > +     ret = rzg2l_irqc_parse_map(priv, node);
> > +     if (ret) {
> > +             pr_err("%pOF: cannot parse interrupts: %d\n", node, ret);
> > +             goto iounmap_base;
> > +     }
> > +
> > +     ret = reset_control_deassert(resetn);
> > +     if (ret) {
> > +             pr_err("%pOF: failed to deassert resetn pin, %d\n", node,
> > ret);
> > +             goto iounmap_base;
> > +     }
> > +
> > +     raw_spin_lock_init(&priv->lock);
> > +
> > +     ret = clk_prepare_enable(clk);
> > +     if (ret)
> > +             goto assert_reset;
> > +
> > +     ret = clk_prepare_enable(pclk);
> > +     if (ret)
> > +             goto disable_clk;
> > +
> > +     irq_domain = irq_domain_add_hierarchy(parent_domain, 0,
> > IRQC_NUM_IRQ,
> > +                                           node, &rzg2l_irqc_domain_ops,
> > +                                           priv);
> > +     if (!irq_domain) {
> > +             pr_err("%pOF: cannot initialize irq domain\n", node);
> > +             ret = -ENOMEM;
> > +             goto fail_irq_domain;
> > +     }
> > +
> > +     return 0;
> > +
> > +fail_irq_domain:
> > +     clk_disable_unprepare(pclk);
> > +disable_clk:
> > +     clk_disable_unprepare(clk);
> > +assert_reset:
> > +     reset_control_assert(resetn);
> > +iounmap_base:
> > +     iounmap(priv->base);
> > +free_priv:
> > +     kfree(priv);
> > +     return ret;
> > +}
> > +
> > +IRQCHIP_PLATFORM_DRIVER_BEGIN(rzg2l_irqc)
> > +IRQCHIP_MATCH("renesas,rzg2l-irqc", rzg2l_irqc_init)
> > +IRQCHIP_PLATFORM_DRIVER_END(rzg2l_irqc)
> > +MODULE_AUTHOR("Lad Prabhakar
> > +<prabhakar.mahadev-lad.rj@bp.renesas.com>");
> > +MODULE_DESCRIPTION("Renesas RZ/G2L IRQC Driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1
>

^ permalink raw reply

* RE: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt
From: Biju Das @ 2022-05-09  6:49 UTC (permalink / raw)
  To: Prabhakar Mahadev Lad, Thomas Gleixner, Marc Zyngier, Rob Herring,
	Krzysztof Kozlowski, Linus Walleij, Bartosz Golaszewski,
	Geert Uytterhoeven, Philipp Zabel, linux-gpio@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, Prabhakar,
	Prabhakar Mahadev Lad
In-Reply-To: <20220509050953.11005-6-prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi Prabhakar,

Thanks for the patch.

> Subject: [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to
> handle GPIO interrupt
> 
> Add IRQ domian to RZ/G2L pinctrl driver to handle GPIO interrupt.
> 
> GPIO0-GPIO122 pins can be used as IRQ lines but only 32 pins can be used as
> IRQ lines at given time. Selection of pins as IRQ lines is handled by IA55
> (which is the IRQC block) which sits in between the GPIO and GIC.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/pinctrl/renesas/pinctrl-rzg2l.c | 205 ++++++++++++++++++++++++
>  1 file changed, 205 insertions(+)
> 
> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> index a48cac55152c..275dfec74329 100644
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -9,8 +9,10 @@
>  #include <linux/clk.h>
>  #include <linux/gpio/driver.h>
>  #include <linux/io.h>
> +#include <linux/interrupt.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> +#include <linux/of_irq.h>
>  #include <linux/pinctrl/pinconf-generic.h>  #include
> <linux/pinctrl/pinconf.h>  #include <linux/pinctrl/pinctrl.h> @@ -89,6
> +91,7 @@
>  #define PIN(n)			(0x0800 + 0x10 + (n))
>  #define IOLH(n)			(0x1000 + (n) * 8)
>  #define IEN(n)			(0x1800 + (n) * 8)
> +#define ISEL(n)			(0x2c80 + (n) * 8)
>  #define PWPR			(0x3014)
>  #define SD_CH(n)		(0x3000 + (n) * 4)
>  #define QSPI			(0x3008)
> @@ -112,6 +115,10 @@
>  #define RZG2L_PIN_ID_TO_PORT_OFFSET(id)	(RZG2L_PIN_ID_TO_PORT(id) +
> 0x10)
>  #define RZG2L_PIN_ID_TO_PIN(id)		((id) % RZG2L_PINS_PER_PORT)
> 
> +#define RZG2L_TINT_MAX_INTERRUPT	32
> +#define RZG2L_TINT_IRQ_START_INDEX	9
> +#define RZG2L_PACK_HWIRQ(t, i)		(((t) << 16) | (i))
> +
>  struct rzg2l_dedicated_configs {
>  	const char *name;
>  	u32 config;
> @@ -137,6 +144,9 @@ struct rzg2l_pinctrl {
> 
>  	struct gpio_chip		gpio_chip;
>  	struct pinctrl_gpio_range	gpio_range;
> +	DECLARE_BITMAP(tint_slot, RZG2L_TINT_MAX_INTERRUPT);
> +	spinlock_t			bitmap_lock;
> +	unsigned int			hwirq[RZG2L_TINT_MAX_INTERRUPT];
> 
>  	spinlock_t			lock;
>  };
> @@ -883,6 +893,8 @@ static int rzg2l_gpio_get(struct gpio_chip *chip,
> unsigned int offset)
> 
>  static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
> {
> +	unsigned int virq;
> +
>  	pinctrl_gpio_free(chip->base + offset);
> 
>  	/*
> @@ -890,6 +902,10 @@ static void rzg2l_gpio_free(struct gpio_chip *chip,
> unsigned int offset)
>  	 * drive the GPIO pin as an output.
>  	 */
>  	rzg2l_gpio_direction_input(chip, offset);
> +
> +	virq = irq_find_mapping(chip->irq.domain, offset);
> +	if (virq)
> +		irq_dispose_mapping(virq);
>  }
> 
>  static const char * const rzg2l_gpio_names[] = { @@ -1104,14 +1120,193 @@
> static struct {
>  	}
>  };
> 
> +static int rzg2l_gpio_get_gpioint(unsigned int virq) {
> +	unsigned int gpioint = 0;
> +	unsigned int i = 0;
> +	u32 port, bit;
> +
> +	port = virq / 8;
> +	bit = virq % 8;
> +
> +	if (port >= ARRAY_SIZE(rzg2l_gpio_configs))
> +		return -EINVAL;
> +
> +	for (i = 0; i < port; i++)
> +		gpioint += RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]);
> +
> +	if (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]))
> +		return -EINVAL;

May be combine this statement to above with 

|| (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[port]))
		return -EINVAL;

Cheers,
BIju

> +
> +	gpioint += bit;
> +
> +	return gpioint;
> +}
> +
> +static void rzg2l_gpio_irq_domain_free(struct irq_domain *domain, unsigned
> int virq,
> +				       unsigned int nr_irqs)
> +{
> +	struct irq_data *d;
> +
> +	d = irq_domain_get_irq_data(domain, virq);
> +	if (d) {
> +		struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +		struct rzg2l_pinctrl *pctrl = container_of(gc, struct
> rzg2l_pinctrl, gpio_chip);
> +		irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +		unsigned long flags;
> +		unsigned int i;
> +
> +		for (i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) {
> +			if (pctrl->hwirq[i] == hwirq) {
> +				spin_lock_irqsave(&pctrl->bitmap_lock, flags);
> +				bitmap_release_region(pctrl->tint_slot, i,
> get_order(1));
> +				spin_unlock_irqrestore(&pctrl->bitmap_lock,
> flags);
> +				pctrl->hwirq[i] = 0;
> +				break;
> +			}
> +		}
> +	}
> +	irq_domain_free_irqs_common(domain, virq, nr_irqs); }
> +
> +static void rzg2l_gpio_irq_disable(struct irq_data *d) {
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl,
> gpio_chip);
> +	unsigned int hwirq = irqd_to_hwirq(d);
> +	unsigned long flags;
> +	void __iomem *addr;
> +	u32 port;
> +	u8 bit;
> +
> +	port = RZG2L_PIN_ID_TO_PORT(hwirq);
> +	bit = RZG2L_PIN_ID_TO_PIN(hwirq);
> +
> +	addr = pctrl->base + ISEL(port);
> +	if (bit >= 4) {
> +		bit -= 4;
> +		addr += 4;
> +	}
> +
> +	spin_lock_irqsave(&pctrl->lock, flags);
> +	writel(readl(addr) & ~BIT(bit * 8), addr);
> +	spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> +	irq_chip_disable_parent(d);
> +}
> +
> +static void rzg2l_gpio_irq_enable(struct irq_data *d) {
> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +	struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl,
> gpio_chip);
> +	unsigned int hwirq = irqd_to_hwirq(d);
> +	unsigned long flags;
> +	void __iomem *addr;
> +	u32 port;
> +	u8 bit;
> +
> +	port = RZG2L_PIN_ID_TO_PORT(hwirq);
> +	bit = RZG2L_PIN_ID_TO_PIN(hwirq);
> +
> +	addr = pctrl->base + ISEL(port);
> +	if (bit >= 4) {
> +		bit -= 4;
> +		addr += 4;
> +	}
> +
> +	spin_lock_irqsave(&pctrl->lock, flags);
> +	writel(readl(addr) | BIT(bit * 8), addr);
> +	spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> +	irq_chip_enable_parent(d);
> +}
> +
> +static int rzg2l_gpio_irq_set_type(struct irq_data *d, unsigned int
> +type) {
> +	return irq_chip_set_type_parent(d, type); }
> +
> +static void rzg2l_gpio_irqc_eoi(struct irq_data *d) {
> +	irq_chip_eoi_parent(d);
> +}
> +
> +static struct irq_chip rzg2l_gpio_irqchip = {
> +	.name = "rzg2l-gpio",
> +	.irq_disable = rzg2l_gpio_irq_disable,
> +	.irq_enable = rzg2l_gpio_irq_enable,
> +	.irq_mask = irq_chip_mask_parent,
> +	.irq_unmask = irq_chip_unmask_parent,
> +	.irq_set_type = rzg2l_gpio_irq_set_type,
> +	.irq_eoi = rzg2l_gpio_irqc_eoi,
> +};
> +
> +static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
> +					    unsigned int child,
> +					    unsigned int child_type,
> +					    unsigned int *parent,
> +					    unsigned int *parent_type)
> +{
> +	struct rzg2l_pinctrl *pctrl = gpiochip_get_data(gc);
> +	unsigned long flags;
> +	int gpioint, irq;
> +
> +	gpioint = rzg2l_gpio_get_gpioint(child);
> +	if (gpioint < 0)
> +		return gpioint;
> +
> +	spin_lock_irqsave(&pctrl->bitmap_lock, flags);
> +	irq = bitmap_find_free_region(pctrl->tint_slot,
> RZG2L_TINT_MAX_INTERRUPT, get_order(1));
> +	spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
> +	if (irq < 0)
> +		return -ENOSPC;
> +	pctrl->hwirq[irq] = child;
> +	irq += RZG2L_TINT_IRQ_START_INDEX;
> +
> +	/* All these interrupts are level high in the CPU */
> +	*parent_type = IRQ_TYPE_LEVEL_HIGH;
> +	*parent = RZG2L_PACK_HWIRQ(gpioint, irq);
> +	return 0;
> +}
> +
> +static void *rzg2l_gpio_populate_parent_fwspec(struct gpio_chip *chip,
> +					       unsigned int parent_hwirq,
> +					       unsigned int parent_type)
> +{
> +	struct irq_fwspec *fwspec;
> +
> +	fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
> +	if (!fwspec)
> +		return NULL;
> +
> +	fwspec->fwnode = chip->irq.parent_domain->fwnode;
> +	fwspec->param_count = 2;
> +	fwspec->param[0] = parent_hwirq;
> +	fwspec->param[1] = parent_type;
> +
> +	return fwspec;
> +}
> +
>  static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)  {
>  	struct device_node *np = pctrl->dev->of_node;
>  	struct gpio_chip *chip = &pctrl->gpio_chip;
>  	const char *name = dev_name(pctrl->dev);
> +	struct irq_domain *parent_domain;
>  	struct of_phandle_args of_args;
> +	struct device_node *parent_np;
> +	struct gpio_irq_chip *girq;
>  	int ret;
> 
> +	parent_np = of_irq_find_parent(np);
> +	if (!parent_np)
> +		return -ENXIO;
> +
> +	parent_domain = irq_find_host(parent_np);
> +	of_node_put(parent_np);
> +	if (!parent_domain)
> +		return -EPROBE_DEFER;
> +
>  	ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0,
> &of_args);
>  	if (ret) {
>  		dev_err(pctrl->dev, "Unable to parse gpio-ranges\n"); @@ -
> 1138,6 +1333,15 @@ static int rzg2l_gpio_register(struct rzg2l_pinctrl
> *pctrl)
>  	chip->base = -1;
>  	chip->ngpio = of_args.args[2];
> 
> +	girq = &chip->irq;
> +	girq->chip = &rzg2l_gpio_irqchip;
> +	girq->fwnode = of_node_to_fwnode(np);
> +	girq->parent_domain = parent_domain;
> +	girq->child_to_parent_hwirq = rzg2l_gpio_child_to_parent_hwirq;
> +	girq->populate_parent_alloc_arg = rzg2l_gpio_populate_parent_fwspec;
> +	girq->child_irq_domain_ops.free = rzg2l_gpio_irq_domain_free;
> +	girq->ngirq = RZG2L_TINT_MAX_INTERRUPT;
> +
>  	pctrl->gpio_range.id = 0;
>  	pctrl->gpio_range.pin_base = 0;
>  	pctrl->gpio_range.base = 0;
> @@ -1253,6 +1457,7 @@ static int rzg2l_pinctrl_probe(struct platform_device
> *pdev)
>  	}
> 
>  	spin_lock_init(&pctrl->lock);
> +	spin_lock_init(&pctrl->bitmap_lock);
> 
>  	platform_set_drvdata(pdev, pctrl);
> 
> --
> 2.25.1


^ permalink raw reply

* [next] gpio: gpio-sim.sh: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008
From: Naresh Kamboju @ 2022-05-09  5:40 UTC (permalink / raw)
  To: open list:KERNEL SELFTEST FRAMEWORK, open list:GPIO SUBSYSTEM,
	open list, lkft-triage
  Cc: Jon Hunter, Justin Chen, Shuah Khan, Mark Brown, brgl,
	Andy Shevchenko, andrei.lalaev, Linus Walleij, u.kleine-koenig,
	Raghuram Thammiraju, Florian Fainelli, Marc Zyngier,
	Nobuhiro Iwamatsu

Following kernel crash noticed while running kselftest gpio gpio-sim.sh on
qemu_arm64 with Linux next-20220506 kernel [1] & [2].

Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>

# selftests: gpio: gpio-sim.sh
# 1. chip_name and dev_name attributes
# 1.1. Chip name is communicated to user
# 1.2. chip_name returns 'none' if the chip is still pending
# 1.3. Device name is communicated to user
# 2. Creating and configuring simulated chips
# 2.1. Default number of lines is 1
# 2.2. Number of lines can be specified
# 2.3. Label can be set
# 2.4. Label can be left empty
# 2.5. Line names can be configured
# 2.6. Line config can remain unused if offset is greater than number of lines
# 2.7. Line configfs directory names are sanitized
# 2.8. Multiple chips can be created
# 2.9. Can't modify settings when chip is live
# 2.10. Can't create line items when chip is live
# 2.11. Probe errors are propagated to user-space
[  218.163457] gpio gpiochip1: (gpio-sim.0-node0): line cnt 34463 is
greater than fast path cnt 512
[  218.163739] gpiochip_find_base: cannot find free range
[  218.164216] gpiochip_add_data_with_key: GPIOs 0..34462
(gpio-sim.0-node0) failed to register, -28
[  218.164503] gpio-sim: probe of gpio-sim.0 failed with error -28
# 2.12. Cannot enable a chip without any GPIO banks
# 2.13. Duplicate chip labels are not allowed
# 2.14. Lines can be hogged
[  223.754983] gpio-2036 (?): hogged as input
[  224.231594] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000008
[  224.233378] Mem abort info:
[  224.233719]   ESR = 0x0000000096000006
[  224.234076]   EC = 0x25: DABT (current EL), IL = 32 bits
[  224.235432]   SET = 0, FnV = 0
[  224.235790]   EA = 0, S1PTW = 0
[  224.236146]   FSC = 0x06: level 2 translation fault
[  224.236592] Data abort info:
[  224.236926]   ISV = 0, ISS = 0x00000006
[  224.237300]   CM = 0, WnR = 0
[  224.237738] user pgtable: 4k pages, 48-bit VAs, pgdp=000000010ab2b000
[  224.238785] [0000000000000008] pgd=080000010b9c4003,
p4d=080000010b9c4003, pud=080000010b990003, pmd=0000000000000000
[  224.240838] Internal error: Oops: 96000006 [#1] PREEMPT SMP
[  224.241509] Modules linked in: gpio_sim rfkill crct10dif_ce sm3_ce
sm3 sha3_ce sha512_ce sha512_arm64 fuse [last unloaded: gpio_mockup]
[  224.242848] CPU: 2 PID: 1105 Comm: gpio-mockup-cde Not tainted
5.18.0-rc5-next-20220506 #1
[  224.243548] Hardware name: linux,dummy-virt (DT)
[  224.244109] pstate: 20400005 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  224.244643] pc : linereq_free+0xb0/0x120
[  224.245515] lr : linereq_create+0x548/0x640
[  224.245797] sp : ffff8000089eb870
[  224.246065] x29: ffff8000089eb870 x28: ffffd5cd009d5cd8 x27: ffff0000c32da0c0
[  224.246795] x26: ffffd5ccfeb521c8 x25: 0000000000000000 x24: ffff0000c60d1d20
[  224.247318] x23: ffff0000c60d1c00 x22: 0000000000000000 x21: 0000000000000118
[  224.247843] x20: 0000000000000001 x19: ffff0000c60d1c00 x18: 0000000000000000
[  224.248352] x17: ffffd5ccfcc2c288 x16: ffffd5ccfd019804 x15: ffffd5ccfd468a48
[  224.248875] x14: ffffd5ccfd4686c0 x13: ffffd5ccfcc11d48 x12: ffffd5ccfe224eec
[  224.249307] x11: ffffd5ccfe223b88 x10: ffffd5ccfcc2c4e8 x9 : ffffd5ccfd4688c8
[  224.249840] x8 : ffff0000c32e6108 x7 : bbbbbbbbbbbbbbbb x6 : 0000000000000001
[  224.250418] x5 : ffffd5ccff965000 x4 : ffffd5ccff9654f0 x3 : 0000000000000000
[  224.251050] x2 : ffff0000c33e5080 x1 : 0000000000000000 x0 : 0000000000000000
[  224.251741] Call trace:
[  224.251998]  linereq_free+0xb0/0x120
[  224.252315]  linereq_create+0x548/0x640
[  224.252571]  gpio_ioctl+0x88/0x754
[  224.252808]  __arm64_sys_ioctl+0xb4/0x100
[  224.253103]  invoke_syscall+0x78/0x100
[  224.253342]  el0_svc_common.constprop.0+0x104/0x124
[  224.253680]  do_el0_svc+0xb4/0xcc
[  224.253922]  el0_svc+0x68/0x160
[  224.254173]  el0t_64_sync_handler+0xbc/0x140
[  224.254504]  el0t_64_sync+0x18c/0x190
[  224.254929] Code: cb160273 8b130ef3 f9409261 b9413260 (f9400422)
[  224.255783] ---[ end trace 0000000000000000 ]---
# ./gpio-sim.sh: line 318:  1105 Segmentation fault
$BASE_DIR/gpio-mockup-cdev -s 1 /dev/`configfs_chip_name chip bank` 4
2> /dev/null

Broadcast message from systemd-journald@juno (Fri 2022-05-06 09:51:02 UTC):

kernel[304]: [  224.240838] Internal error: Oops: 96000006 [#1] PREEMPT SMP


Broadcast message from systemd-journald@juno (Fri 2022-05-06 09:51:02 UTC):

kernel[304]: [  224.254929] Code: cb160273 8b130ef3 f9409261 b9413260 (f9400422)

# 3. Controlling simulated chips
# 3.1. Pull can be set over sysfs
#
not ok 2 selftests: gpio: gpio-sim.sh # TIMEOUT 45 seconds

metadata:
  git_ref: master
  git_repo: ''
  git_sha: 38a288f5941ef03752887ad86f2d85442358c99a
  git_describe: next-20220506
  kernel_version: 5.18.0-rc5
  kernel-config: https://builds.tuxbuild.com/28mio5DFBEfnEtkiTLdPb9tTWVa/config
  build-url: https://gitlab.com/Linaro/lkft/mirrors/next/linux-next/-/pipelines/532821646
  artifact-location: https://builds.tuxbuild.com/28mio5DFBEfnEtkiTLdPb9tTWVa
  toolchain: gcc-11

Steps to reproduce:
# cd /opt/kselftests/default-in-kernel/gpio
# ./gpio-sim.sh

Full test logs.
[1] https://lkft.validation.linaro.org/scheduler/job/4994124#L1108
[2] https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20220506/testrun/9366936/suite/linux-log-parser/test/check-kernel-oops-4994124/log

--
Linaro LKFT
https://lkft.linaro.org

^ permalink raw reply

* [PATCH v2 4/5] gpio: gpiolib: Add ngirq member to struct gpio_irq_chip
From: Lad Prabhakar @ 2022-05-09  5:09 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Philipp Zabel, linux-gpio
  Cc: linux-kernel, devicetree, linux-renesas-soc, Prabhakar, Biju Das,
	Lad Prabhakar
In-Reply-To: <20220509050953.11005-1-prabhakar.mahadev-lad.rj@bp.renesas.com>

Supported GPIO IRQs by the chip is not always equal to the number of GPIO
pins. For example on Renesas RZ/G2L SoC where it has GPIO0-122 pins but at
a give point a maximum of only 32 GPIO pins can be used as IRQ lines in
the IRQC domain.

This patch adds ngirq member to struct gpio_irq_chip and passes this as a
size to irq_domain_create_hierarchy()/irq_domain_create_simple() if it is
being set in the driver otherwise fallbacks to using ngpio.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/gpio/gpiolib.c      | 4 ++--
 include/linux/gpio/driver.h | 8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 7be01c70ee4e..4b402141580e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1221,7 +1221,7 @@ static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
 	gc->irq.domain = irq_domain_create_hierarchy(
 		gc->irq.parent_domain,
 		0,
-		gc->ngpio,
+		gc->irq.ngirq ?: gc->ngpio,
 		gc->irq.fwnode,
 		&gc->irq.child_irq_domain_ops,
 		gc);
@@ -1574,7 +1574,7 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
 	} else {
 		/* Some drivers provide custom irqdomain ops */
 		gc->irq.domain = irq_domain_create_simple(fwnode,
-			gc->ngpio,
+			gc->irq.ngirq ?: gc->ngpio,
 			gc->irq.first,
 			gc->irq.domain_ops ?: &gpiochip_domain_ops,
 			gc);
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 874aabd270c9..ed6df186907d 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -51,6 +51,14 @@ struct gpio_irq_chip {
 	 */
 	const struct irq_domain_ops *domain_ops;
 
+	/**
+	 * @ngirq:
+	 *
+	 * The number of GPIO IRQ's handled by this IRQ domain; usually is
+	 * equal to ngpio.
+	 */
+	u16 ngirq;
+
 #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
 	/**
 	 * @fwnode:
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 2/5] irqchip: Add RZ/G2L IA55 Interrupt Controller driver
From: Lad Prabhakar @ 2022-05-09  5:09 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Philipp Zabel, linux-gpio
  Cc: linux-kernel, devicetree, linux-renesas-soc, Prabhakar, Biju Das,
	Lad Prabhakar
In-Reply-To: <20220509050953.11005-1-prabhakar.mahadev-lad.rj@bp.renesas.com>

Add a driver for the Renesas RZ/G2L Interrupt Controller.

This supports external pins being used as interrupts. It supports
one line for NMI, 8 external pins and 32 GPIO pins (out of 123)
to be used as IRQ lines.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/irqchip/Kconfig             |   8 +
 drivers/irqchip/Makefile            |   1 +
 drivers/irqchip/irq-renesas-rzg2l.c | 445 ++++++++++++++++++++++++++++
 3 files changed, 454 insertions(+)
 create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 15edb9a6fcae..f3d071422f3b 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -242,6 +242,14 @@ config RENESAS_RZA1_IRQC
 	  Enable support for the Renesas RZ/A1 Interrupt Controller, to use up
 	  to 8 external interrupts with configurable sense select.
 
+config RENESAS_RZG2L_IRQC
+	bool "Renesas RZ/G2L (and alike SoC) IRQC support" if COMPILE_TEST
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN_HIERARCHY
+	help
+	  Enable support for the Renesas RZ/G2L (and alike SoC) Interrupt Controller
+	  for external devices.
+
 config SL28CPLD_INTC
 	bool "Kontron sl28cpld IRQ controller"
 	depends on MFD_SL28CPLD=y || COMPILE_TEST
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 160a1d8ceaa9..eaa56eec2b23 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_RDA_INTC)			+= irq-rda-intc.o
 obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
 obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
 obj-$(CONFIG_RENESAS_RZA1_IRQC)		+= irq-renesas-rza1.o
+obj-$(CONFIG_RENESAS_RZG2L_IRQC)	+= irq-renesas-rzg2l.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
new file mode 100644
index 000000000000..bd6e82100caf
--- /dev/null
+++ b/drivers/irqchip/irq-renesas-rzg2l.c
@@ -0,0 +1,445 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L IRQC Driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation.
+ *
+ * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+#define IRQC_IRQ_START			1
+#define IRQC_IRQ_COUNT			8
+#define IRQC_TINT_START			(IRQC_IRQ_START + IRQC_IRQ_COUNT)
+#define IRQC_TINT_COUNT			32
+#define IRQC_NUM_IRQ			(IRQC_TINT_START + IRQC_TINT_COUNT)
+
+#define ISCR				0x10
+#define IITSR				0x14
+#define TSCR				0x20
+#define TITSR0				0x24
+#define TITSR1				0x28
+#define TITSR0_MAX_INT			16
+#define TITSEL_WIDTH			0x2
+#define TSSR(n)				(0x30 + ((n) * 4))
+#define TIEN				BIT(7)
+#define TSSEL_SHIFT(n)			(8 * (n))
+#define TSSEL_MASK			GENMASK(7, 0)
+#define IRQ_MASK			0x3
+
+#define TSSR_OFFSET(n)			((n) % 4)
+#define TSSR_INDEX(n)			((n) / 4)
+
+#define TITSR_TITSEL_EDGE_RISING	0
+#define TITSR_TITSEL_EDGE_FALLING	1
+#define TITSR_TITSEL_LEVEL_HIGH		2
+#define TITSR_TITSEL_LEVEL_LOW		3
+
+#define IITSR_IITSEL(n, sense)		((sense) << ((n) * 2))
+#define IITSR_IITSEL_LEVEL_LOW		0
+#define IITSR_IITSEL_EDGE_FALLING	1
+#define IITSR_IITSEL_EDGE_RISING	2
+#define IITSR_IITSEL_EDGE_BOTH		3
+#define IITSR_IITSEL_MASK(n)		IITSR_IITSEL((n), 3)
+
+#define TINT_EXTRACT_HWIRQ(x)		((x) & ~GENMASK(31, 16))
+#define TINT_EXTRACT_GPIOINT(x)		((x) >> 16)
+
+struct rzg2l_irqc_priv {
+	void __iomem *base;
+	struct of_phandle_args map[IRQC_NUM_IRQ];
+	raw_spinlock_t lock;
+};
+
+struct rzg2l_irqc_chip_data {
+	int tint;
+};
+
+static struct rzg2l_irqc_priv *irq_data_to_priv(struct irq_data *data)
+{
+	return data->domain->host_data;
+}
+
+static void rzg2l_irq_eoi(struct irq_data *d)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	u32 bit = BIT(hw_irq);
+	u32 reg;
+
+	reg = readl_relaxed(priv->base + ISCR);
+	if (reg & bit)
+		writel_relaxed(reg & ~bit, priv->base + ISCR);
+}
+
+static void rzg2l_tint_eoi(struct irq_data *d)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_TINT_START;
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	u32 bit = BIT(hw_irq);
+	u32 reg;
+
+	reg = readl_relaxed(priv->base + TSCR);
+	if (reg & bit)
+		writel_relaxed(reg & ~bit, priv->base + TSCR);
+}
+
+static void rzg2l_irqc_eoi(struct irq_data *d)
+{
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	unsigned int hw_irq = irqd_to_hwirq(d);
+
+	raw_spin_lock(&priv->lock);
+	if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
+		rzg2l_irq_eoi(d);
+	else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
+		rzg2l_tint_eoi(d);
+	raw_spin_unlock(&priv->lock);
+	irq_chip_eoi_parent(d);
+}
+
+static void rzg2l_irqc_irq_disable(struct irq_data *d)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d);
+
+	if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
+		struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+		u32 offset = hw_irq - IRQC_TINT_START;
+		u32 tssr_offset = TSSR_OFFSET(offset);
+		u8 tssr_index = TSSR_INDEX(offset);
+		u32 reg;
+
+		raw_spin_lock(&priv->lock);
+		reg = readl_relaxed(priv->base + TSSR(tssr_index));
+		reg &= ~(TSSEL_MASK << tssr_offset);
+		writel_relaxed(reg, priv->base + TSSR(tssr_index));
+		raw_spin_unlock(&priv->lock);
+	}
+	irq_chip_disable_parent(d);
+}
+
+static void rzg2l_irqc_irq_enable(struct irq_data *d)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d);
+
+	if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT) {
+		struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+		struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
+		u32 offset = hw_irq - IRQC_TINT_START;
+		u32 tssr_offset = TSSR_OFFSET(offset);
+		u8 tssr_index = TSSR_INDEX(offset);
+		u32 reg;
+
+		raw_spin_lock(&priv->lock);
+		reg = readl_relaxed(priv->base + TSSR(tssr_index));
+		reg |= (TIEN | chip_data->tint) << TSSEL_SHIFT(tssr_offset);
+		writel_relaxed(reg, priv->base + TSSR(tssr_index));
+		raw_spin_unlock(&priv->lock);
+	}
+	irq_chip_enable_parent(d);
+}
+
+static int rzg2l_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d) - IRQC_IRQ_START;
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	u16 sense, tmp;
+
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_LEVEL_LOW:
+		sense = IITSR_IITSEL_LEVEL_LOW;
+		break;
+
+	case IRQ_TYPE_EDGE_FALLING:
+		sense = IITSR_IITSEL_EDGE_FALLING;
+		break;
+
+	case IRQ_TYPE_EDGE_RISING:
+		sense = IITSR_IITSEL_EDGE_RISING;
+		break;
+
+	case IRQ_TYPE_EDGE_BOTH:
+		sense = IITSR_IITSEL_EDGE_BOTH;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	raw_spin_lock(&priv->lock);
+	tmp = readl_relaxed(priv->base + IITSR);
+	tmp &= ~IITSR_IITSEL_MASK(hw_irq);
+	tmp |= IITSR_IITSEL(hw_irq, sense);
+	writel_relaxed(tmp, priv->base + IITSR);
+	raw_spin_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int rzg2l_tint_set_edge(struct irq_data *d, unsigned int type)
+{
+	struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
+	unsigned int hwirq = irqd_to_hwirq(d);
+	u32 titseln = hwirq - IRQC_TINT_START;
+	u32 offset;
+	u8 sense;
+	u32 reg;
+
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_EDGE_RISING:
+		sense = TITSR_TITSEL_EDGE_RISING;
+		break;
+
+	case IRQ_TYPE_EDGE_FALLING:
+		sense = TITSR_TITSEL_EDGE_FALLING;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	if (titseln < TITSR0_MAX_INT) {
+		offset = TITSR0;
+	} else {
+		titseln /= TITSEL_WIDTH;
+		offset  = TITSR1;
+	}
+
+	raw_spin_lock(&priv->lock);
+	reg = readl_relaxed(priv->base + offset);
+	reg &= ~(IRQ_MASK << (titseln * TITSEL_WIDTH));
+	reg |= sense << (titseln * TITSEL_WIDTH);
+	writel_relaxed(reg, priv->base + offset);
+	raw_spin_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
+{
+	unsigned int hw_irq = irqd_to_hwirq(d);
+	int ret = -EINVAL;
+
+	if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
+		ret = rzg2l_irq_set_type(d, type);
+	else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_TINT_COUNT)
+		ret = rzg2l_tint_set_edge(d, type);
+	if (ret)
+		return ret;
+
+	return irq_chip_set_type_parent(d, IRQ_TYPE_LEVEL_HIGH);
+}
+
+static struct irq_chip irqc_chip = {
+	.name			= "rzg2l-irqc",
+	.irq_eoi		= rzg2l_irqc_eoi,
+	.irq_mask		= irq_chip_mask_parent,
+	.irq_unmask		= irq_chip_unmask_parent,
+	.irq_disable		= rzg2l_irqc_irq_disable,
+	.irq_enable		= rzg2l_irqc_irq_enable,
+	.irq_get_irqchip_state	= irq_chip_get_parent_state,
+	.irq_set_irqchip_state	= irq_chip_set_parent_state,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_type		= rzg2l_irqc_set_type,
+	.flags			= IRQCHIP_MASK_ON_SUSPEND |
+				  IRQCHIP_SET_TYPE_MASKED |
+				  IRQCHIP_SKIP_SET_WAKE,
+};
+
+static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int virq,
+			    unsigned int nr_irqs, void *arg)
+{
+	struct rzg2l_irqc_priv *priv = domain->host_data;
+	struct rzg2l_irqc_chip_data *chip_data = NULL;
+	struct irq_fwspec spec;
+	irq_hw_number_t hwirq;
+	int tint = -EINVAL;
+	unsigned int type;
+	unsigned int i;
+	int ret;
+
+	ret = irq_domain_translate_twocell(domain, arg, &hwirq, &type);
+	if (ret)
+		return ret;
+
+	/*
+	 * For TINIT interrupts ie where pinctrl driver is child of irqc domain
+	 * the hwirq and TINT are encoded in fwspec->param[0].
+	 * hwirq for TINIT range from 9-40, hwirq is embedded 0-15 bits and TINT
+	 * from 16-31 bits. TINIT from the pinctrl driver needs to be programmed
+	 * in IRQC registers to enable a given gpio pin as interrupt.
+	 */
+	if (hwirq > IRQC_IRQ_COUNT) {
+		tint = TINT_EXTRACT_GPIOINT(hwirq);
+		hwirq = TINT_EXTRACT_HWIRQ(hwirq);
+	}
+
+	if (hwirq > (IRQC_NUM_IRQ - 1))
+		return -EINVAL;
+
+	if (tint != -EINVAL && (hwirq < IRQC_TINT_START || hwirq > (IRQC_NUM_IRQ - 1)))
+		return -EINVAL;
+
+	chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
+	if (!chip_data)
+		return -ENOMEM;
+	chip_data->tint = tint;
+
+	ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &irqc_chip,
+					    chip_data);
+	if (ret) {
+		kfree(chip_data);
+		return ret;
+	}
+
+	spec.fwnode = domain->parent->fwnode;
+	spec.param_count = priv->map[hwirq].args_count;
+	for (i = 0; i < spec.param_count; i++)
+		spec.param[i] = priv->map[hwirq].args[i];
+
+	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &spec);
+	if (ret)
+		kfree(chip_data);
+
+	return ret;
+}
+
+static void rzg2l_irqc_domain_free(struct irq_domain *domain, unsigned int virq,
+				   unsigned int nr_irqs)
+{
+	struct irq_data *d;
+
+	d = irq_domain_get_irq_data(domain, virq);
+	if (d) {
+		struct rzg2l_irqc_chip_data *chip_data = d->chip_data;
+
+		kfree(chip_data);
+	}
+	irq_domain_free_irqs_common(domain, virq, nr_irqs);
+}
+
+static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
+	.alloc = rzg2l_irqc_alloc,
+	.free = rzg2l_irqc_domain_free,
+	.translate = irq_domain_translate_twocell,
+};
+
+static int rzg2l_irqc_parse_map(struct rzg2l_irqc_priv *priv,
+				struct device_node *np)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < IRQC_NUM_IRQ; i++) {
+		ret = of_irq_parse_one(np, i, &priv->map[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int rzg2l_irqc_init(struct device_node *node, struct device_node *parent)
+{
+	struct irq_domain *irq_domain, *parent_domain;
+	struct reset_control *resetn;
+	struct rzg2l_irqc_priv *priv;
+	struct clk *clk;
+	struct clk *pclk;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->base = of_iomap(node, 0);
+	if (!priv->base) {
+		ret = -ENXIO;
+		goto free_priv;
+	}
+
+	clk = of_clk_get_by_name(node, "clk");
+	if (IS_ERR(clk)) {
+		ret = IS_ERR(clk);
+		goto iounmap_base;
+	}
+
+	pclk = of_clk_get_by_name(node, "pclk");
+	if (IS_ERR(pclk)) {
+		ret = IS_ERR(pclk);
+		goto iounmap_base;
+	}
+
+	resetn = of_reset_control_get_exclusive_by_index(node, 0);
+	if (IS_ERR(resetn)) {
+		ret = IS_ERR(resetn);
+		goto iounmap_base;
+	}
+
+	parent_domain = irq_find_host(parent);
+	if (!parent_domain) {
+		pr_err("%pOF: cannot find parent domain\n", node);
+		ret = -ENODEV;
+		goto iounmap_base;
+	}
+
+	ret = rzg2l_irqc_parse_map(priv, node);
+	if (ret) {
+		pr_err("%pOF: cannot parse interrupts: %d\n", node, ret);
+		goto iounmap_base;
+	}
+
+	ret = reset_control_deassert(resetn);
+	if (ret) {
+		pr_err("%pOF: failed to deassert resetn pin, %d\n", node, ret);
+		goto iounmap_base;
+	}
+
+	raw_spin_lock_init(&priv->lock);
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		goto assert_reset;
+
+	ret = clk_prepare_enable(pclk);
+	if (ret)
+		goto disable_clk;
+
+	irq_domain = irq_domain_add_hierarchy(parent_domain, 0, IRQC_NUM_IRQ,
+					      node, &rzg2l_irqc_domain_ops,
+					      priv);
+	if (!irq_domain) {
+		pr_err("%pOF: cannot initialize irq domain\n", node);
+		ret = -ENOMEM;
+		goto fail_irq_domain;
+	}
+
+	return 0;
+
+fail_irq_domain:
+	clk_disable_unprepare(pclk);
+disable_clk:
+	clk_disable_unprepare(clk);
+assert_reset:
+	reset_control_assert(resetn);
+iounmap_base:
+	iounmap(priv->base);
+free_priv:
+	kfree(priv);
+	return ret;
+}
+
+IRQCHIP_PLATFORM_DRIVER_BEGIN(rzg2l_irqc)
+IRQCHIP_MATCH("renesas,rzg2l-irqc", rzg2l_irqc_init)
+IRQCHIP_PLATFORM_DRIVER_END(rzg2l_irqc)
+MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L IRQC Driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 5/5] pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO interrupt
From: Lad Prabhakar @ 2022-05-09  5:09 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Philipp Zabel, linux-gpio
  Cc: linux-kernel, devicetree, linux-renesas-soc, Prabhakar, Biju Das,
	Lad Prabhakar
In-Reply-To: <20220509050953.11005-1-prabhakar.mahadev-lad.rj@bp.renesas.com>

Add IRQ domian to RZ/G2L pinctrl driver to handle GPIO interrupt.

GPIO0-GPIO122 pins can be used as IRQ lines but only 32 pins can be
used as IRQ lines at given time. Selection of pins as IRQ lines
is handled by IA55 (which is the IRQC block) which sits in between the
GPIO and GIC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 205 ++++++++++++++++++++++++
 1 file changed, 205 insertions(+)

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index a48cac55152c..275dfec74329 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -9,8 +9,10 @@
 #include <linux/clk.h>
 #include <linux/gpio/driver.h>
 #include <linux/io.h>
+#include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/of_irq.h>
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinctrl.h>
@@ -89,6 +91,7 @@
 #define PIN(n)			(0x0800 + 0x10 + (n))
 #define IOLH(n)			(0x1000 + (n) * 8)
 #define IEN(n)			(0x1800 + (n) * 8)
+#define ISEL(n)			(0x2c80 + (n) * 8)
 #define PWPR			(0x3014)
 #define SD_CH(n)		(0x3000 + (n) * 4)
 #define QSPI			(0x3008)
@@ -112,6 +115,10 @@
 #define RZG2L_PIN_ID_TO_PORT_OFFSET(id)	(RZG2L_PIN_ID_TO_PORT(id) + 0x10)
 #define RZG2L_PIN_ID_TO_PIN(id)		((id) % RZG2L_PINS_PER_PORT)
 
+#define RZG2L_TINT_MAX_INTERRUPT	32
+#define RZG2L_TINT_IRQ_START_INDEX	9
+#define RZG2L_PACK_HWIRQ(t, i)		(((t) << 16) | (i))
+
 struct rzg2l_dedicated_configs {
 	const char *name;
 	u32 config;
@@ -137,6 +144,9 @@ struct rzg2l_pinctrl {
 
 	struct gpio_chip		gpio_chip;
 	struct pinctrl_gpio_range	gpio_range;
+	DECLARE_BITMAP(tint_slot, RZG2L_TINT_MAX_INTERRUPT);
+	spinlock_t			bitmap_lock;
+	unsigned int			hwirq[RZG2L_TINT_MAX_INTERRUPT];
 
 	spinlock_t			lock;
 };
@@ -883,6 +893,8 @@ static int rzg2l_gpio_get(struct gpio_chip *chip, unsigned int offset)
 
 static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
 {
+	unsigned int virq;
+
 	pinctrl_gpio_free(chip->base + offset);
 
 	/*
@@ -890,6 +902,10 @@ static void rzg2l_gpio_free(struct gpio_chip *chip, unsigned int offset)
 	 * drive the GPIO pin as an output.
 	 */
 	rzg2l_gpio_direction_input(chip, offset);
+
+	virq = irq_find_mapping(chip->irq.domain, offset);
+	if (virq)
+		irq_dispose_mapping(virq);
 }
 
 static const char * const rzg2l_gpio_names[] = {
@@ -1104,14 +1120,193 @@ static struct {
 	}
 };
 
+static int rzg2l_gpio_get_gpioint(unsigned int virq)
+{
+	unsigned int gpioint = 0;
+	unsigned int i = 0;
+	u32 port, bit;
+
+	port = virq / 8;
+	bit = virq % 8;
+
+	if (port >= ARRAY_SIZE(rzg2l_gpio_configs))
+		return -EINVAL;
+
+	for (i = 0; i < port; i++)
+		gpioint += RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]);
+
+	if (bit >= RZG2L_GPIO_PORT_GET_PINCNT(rzg2l_gpio_configs[i]))
+		return -EINVAL;
+
+	gpioint += bit;
+
+	return gpioint;
+}
+
+static void rzg2l_gpio_irq_domain_free(struct irq_domain *domain, unsigned int virq,
+				       unsigned int nr_irqs)
+{
+	struct irq_data *d;
+
+	d = irq_domain_get_irq_data(domain, virq);
+	if (d) {
+		struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+		struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip);
+		irq_hw_number_t hwirq = irqd_to_hwirq(d);
+		unsigned long flags;
+		unsigned int i;
+
+		for (i = 0; i < RZG2L_TINT_MAX_INTERRUPT; i++) {
+			if (pctrl->hwirq[i] == hwirq) {
+				spin_lock_irqsave(&pctrl->bitmap_lock, flags);
+				bitmap_release_region(pctrl->tint_slot, i, get_order(1));
+				spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
+				pctrl->hwirq[i] = 0;
+				break;
+			}
+		}
+	}
+	irq_domain_free_irqs_common(domain, virq, nr_irqs);
+}
+
+static void rzg2l_gpio_irq_disable(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip);
+	unsigned int hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	void __iomem *addr;
+	u32 port;
+	u8 bit;
+
+	port = RZG2L_PIN_ID_TO_PORT(hwirq);
+	bit = RZG2L_PIN_ID_TO_PIN(hwirq);
+
+	addr = pctrl->base + ISEL(port);
+	if (bit >= 4) {
+		bit -= 4;
+		addr += 4;
+	}
+
+	spin_lock_irqsave(&pctrl->lock, flags);
+	writel(readl(addr) & ~BIT(bit * 8), addr);
+	spin_unlock_irqrestore(&pctrl->lock, flags);
+
+	irq_chip_disable_parent(d);
+}
+
+static void rzg2l_gpio_irq_enable(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct rzg2l_pinctrl *pctrl = container_of(gc, struct rzg2l_pinctrl, gpio_chip);
+	unsigned int hwirq = irqd_to_hwirq(d);
+	unsigned long flags;
+	void __iomem *addr;
+	u32 port;
+	u8 bit;
+
+	port = RZG2L_PIN_ID_TO_PORT(hwirq);
+	bit = RZG2L_PIN_ID_TO_PIN(hwirq);
+
+	addr = pctrl->base + ISEL(port);
+	if (bit >= 4) {
+		bit -= 4;
+		addr += 4;
+	}
+
+	spin_lock_irqsave(&pctrl->lock, flags);
+	writel(readl(addr) | BIT(bit * 8), addr);
+	spin_unlock_irqrestore(&pctrl->lock, flags);
+
+	irq_chip_enable_parent(d);
+}
+
+static int rzg2l_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	return irq_chip_set_type_parent(d, type);
+}
+
+static void rzg2l_gpio_irqc_eoi(struct irq_data *d)
+{
+	irq_chip_eoi_parent(d);
+}
+
+static struct irq_chip rzg2l_gpio_irqchip = {
+	.name = "rzg2l-gpio",
+	.irq_disable = rzg2l_gpio_irq_disable,
+	.irq_enable = rzg2l_gpio_irq_enable,
+	.irq_mask = irq_chip_mask_parent,
+	.irq_unmask = irq_chip_unmask_parent,
+	.irq_set_type = rzg2l_gpio_irq_set_type,
+	.irq_eoi = rzg2l_gpio_irqc_eoi,
+};
+
+static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
+					    unsigned int child,
+					    unsigned int child_type,
+					    unsigned int *parent,
+					    unsigned int *parent_type)
+{
+	struct rzg2l_pinctrl *pctrl = gpiochip_get_data(gc);
+	unsigned long flags;
+	int gpioint, irq;
+
+	gpioint = rzg2l_gpio_get_gpioint(child);
+	if (gpioint < 0)
+		return gpioint;
+
+	spin_lock_irqsave(&pctrl->bitmap_lock, flags);
+	irq = bitmap_find_free_region(pctrl->tint_slot, RZG2L_TINT_MAX_INTERRUPT, get_order(1));
+	spin_unlock_irqrestore(&pctrl->bitmap_lock, flags);
+	if (irq < 0)
+		return -ENOSPC;
+	pctrl->hwirq[irq] = child;
+	irq += RZG2L_TINT_IRQ_START_INDEX;
+
+	/* All these interrupts are level high in the CPU */
+	*parent_type = IRQ_TYPE_LEVEL_HIGH;
+	*parent = RZG2L_PACK_HWIRQ(gpioint, irq);
+	return 0;
+}
+
+static void *rzg2l_gpio_populate_parent_fwspec(struct gpio_chip *chip,
+					       unsigned int parent_hwirq,
+					       unsigned int parent_type)
+{
+	struct irq_fwspec *fwspec;
+
+	fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL);
+	if (!fwspec)
+		return NULL;
+
+	fwspec->fwnode = chip->irq.parent_domain->fwnode;
+	fwspec->param_count = 2;
+	fwspec->param[0] = parent_hwirq;
+	fwspec->param[1] = parent_type;
+
+	return fwspec;
+}
+
 static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
 {
 	struct device_node *np = pctrl->dev->of_node;
 	struct gpio_chip *chip = &pctrl->gpio_chip;
 	const char *name = dev_name(pctrl->dev);
+	struct irq_domain *parent_domain;
 	struct of_phandle_args of_args;
+	struct device_node *parent_np;
+	struct gpio_irq_chip *girq;
 	int ret;
 
+	parent_np = of_irq_find_parent(np);
+	if (!parent_np)
+		return -ENXIO;
+
+	parent_domain = irq_find_host(parent_np);
+	of_node_put(parent_np);
+	if (!parent_domain)
+		return -EPROBE_DEFER;
+
 	ret = of_parse_phandle_with_fixed_args(np, "gpio-ranges", 3, 0, &of_args);
 	if (ret) {
 		dev_err(pctrl->dev, "Unable to parse gpio-ranges\n");
@@ -1138,6 +1333,15 @@ static int rzg2l_gpio_register(struct rzg2l_pinctrl *pctrl)
 	chip->base = -1;
 	chip->ngpio = of_args.args[2];
 
+	girq = &chip->irq;
+	girq->chip = &rzg2l_gpio_irqchip;
+	girq->fwnode = of_node_to_fwnode(np);
+	girq->parent_domain = parent_domain;
+	girq->child_to_parent_hwirq = rzg2l_gpio_child_to_parent_hwirq;
+	girq->populate_parent_alloc_arg = rzg2l_gpio_populate_parent_fwspec;
+	girq->child_irq_domain_ops.free = rzg2l_gpio_irq_domain_free;
+	girq->ngirq = RZG2L_TINT_MAX_INTERRUPT;
+
 	pctrl->gpio_range.id = 0;
 	pctrl->gpio_range.pin_base = 0;
 	pctrl->gpio_range.base = 0;
@@ -1253,6 +1457,7 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev)
 	}
 
 	spin_lock_init(&pctrl->lock);
+	spin_lock_init(&pctrl->bitmap_lock);
 
 	platform_set_drvdata(pdev, pctrl);
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 1/5] dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt Controller
From: Lad Prabhakar @ 2022-05-09  5:09 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Philipp Zabel, linux-gpio
  Cc: linux-kernel, devicetree, linux-renesas-soc, Prabhakar, Biju Das,
	Lad Prabhakar
In-Reply-To: <20220509050953.11005-1-prabhakar.mahadev-lad.rj@bp.renesas.com>

Add DT bindings for the Renesas RZ/G2L Interrupt Controller.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../renesas,rzg2l-irqc.yaml                   | 131 ++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
new file mode 100644
index 000000000000..5f2e1dd1d42a
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/renesas,rzg2l-irqc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L (and alike SoC's) Interrupt Controller (IA55)
+
+maintainers:
+  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+  - Geert Uytterhoeven <geert+renesas@glider.be>
+
+description: |
+  IA55 performs various interrupt controls including synchronization for the external
+  interrupts of NMI, IRQ, and GPIOINT and the interrupts of the built-in peripheral
+  interrupts output by each IP. And it notifies the interrupt to the GIC
+    - IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
+    - GPIO pins used as external interrupt input pins, mapped to 32 GIC SPI interrupts
+    - NMI edge select (NMI is not treated as NMI exception and supports fall edge and
+      stand-up edge detection interrupts)
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a07g044-irqc    # RZ/G2L
+      - const: renesas,rzg2l-irqc
+
+  '#interrupt-cells':
+    const: 2
+
+  '#address-cells':
+    const: 0
+
+  interrupt-controller: true
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 41
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: clk
+      - const: pclk
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - '#interrupt-cells'
+  - '#address-cells'
+  - interrupt-controller
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/r9a07g044-cpg.h>
+
+    irqc: interrupt-controller@110a0000 {
+            compatible = "renesas,r9a07g044-irqc", "renesas,rzg2l-irqc";
+            reg = <0x110a0000 0x10000>;
+            #interrupt-cells = <2>;
+            #address-cells = <0>;
+            interrupt-controller;
+            interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 446 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 447 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 448 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 450 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 451 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 452 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 453 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 454 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 455 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 457 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 459 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 460 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 461 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 462 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 463 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 464 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 465 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 466 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 467 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 468 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 469 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 470 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 472 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 473 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 474 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 475 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cpg CPG_MOD R9A07G044_IA55_CLK>,
+                     <&cpg CPG_MOD R9A07G044_IA55_PCLK>;
+            clock-names = "clk", "pclk";
+            power-domains = <&cpg>;
+            resets = <&cpg R9A07G044_IA55_RESETN>;
+    };
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 0/5] Renesas RZ/G2L IRQC support
From: Lad Prabhakar @ 2022-05-09  5:09 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Philipp Zabel, linux-gpio
  Cc: linux-kernel, devicetree, linux-renesas-soc, Prabhakar, Biju Das,
	Lad Prabhakar

Hi All,

The RZ/G2L Interrupt Controller is a front-end for the GIC found on
Renesas RZ/G2L SoC's with below pins:
- IRQ sense select for 8 external interrupts, mapped to 8 GIC SPI interrupts
- GPIO pins used as external interrupt input pins out of GPIOINT0-122 a
  maximum of only 32 can be mapped to 32 GIC SPI interrupts,
- NMI edge select.

                                                                _____________
                                                                |    GIC     |
                                                                |  ________  |
                                         ____________           | |        | |
NMI ------------------------------------>|          |  SPI0-479 | | GIC-600| |
                _______                  |          |------------>|        | |
                |      |                 |          |  PPI16-31 | |        | |
                |      | IRQ0-IRQ7       |   IRQC   |------------>|        | |
P0_P48_4 ------>| GPIO |---------------->|          |           | |________| |
                |      |GPIOINT0-122     |          |           |            |
                |      |---------------->| TINT0-31 |           |            |
                |______|                 |__________|           |____________|

The proposed patches add hierarchical IRQ domain, one in IRQC driver and another in
pinctrl driver. Upon interrupt requests map the interrupt to GIC. Out of GPIOINT0-122
only 32 can be mapped to GIC SPI, this mapping is handled by the pinctrl and IRQC driver.

Cheers,
Prabhakar

Changes for v1->v2:
* Included RB tag from Rob
* Fixed review comments pointed by Geert
* included GPIO driver changes

Changes for RFCV4 -> V1:
* Used unevaluatedProperties.
* Altered the sequence of reg property
* Set the parent type
* Used raw_spin_lock() instead of raw_spin_lock_irqsave()
* Simplified parsing IRQ map.
* Will send the GPIO and pinctrl changes as part of separate series

Changes for RFC v4:
* Used locking while RMW
* Now using interrupts property instead of interrupt-map
* Patch series depends on [0]
* Updated binding doc
* Fixed comments pointed by Andy

[0] https://patchwork.kernel.org/project/linux-renesas-soc/patch/
20220316200633.28974-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Changes for RFC v3:
-> Re-structured the driver as a hierarchical irq domain instead of chained
-> made use of IRQCHIP_* macros
-> dropped locking
-> Added support for IRQ0-7 interrupts
-> Introduced 2 new patches for GPIOLIB
-> Switched to using GPIOLIB for irqdomains in pinctrl

RFC v2: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
20210921193028.13099-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

RFC v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/
20210803175109.1729-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Lad Prabhakar (5):
  dt-bindings: interrupt-controller: Add Renesas RZ/G2L Interrupt
    Controller
  irqchip: Add RZ/G2L IA55 Interrupt Controller driver
  gpio: gpiolib: Allow free() callback to be overridden
  gpio: gpiolib: Add ngirq member to struct gpio_irq_chip
  pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain to handle GPIO
    interrupt

 .../renesas,rzg2l-irqc.yaml                   | 131 ++++++
 drivers/gpio/gpiolib.c                        |  13 +-
 drivers/irqchip/Kconfig                       |   8 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-renesas-rzg2l.c           | 445 ++++++++++++++++++
 drivers/pinctrl/renesas/pinctrl-rzg2l.c       | 205 ++++++++
 include/linux/gpio/driver.h                   |   8 +
 7 files changed, 806 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,rzg2l-irqc.yaml
 create mode 100644 drivers/irqchip/irq-renesas-rzg2l.c

-- 
2.25.1


^ permalink raw reply

* [PATCH v2 3/5] gpio: gpiolib: Allow free() callback to be overridden
From: Lad Prabhakar @ 2022-05-09  5:09 UTC (permalink / raw)
  To: Thomas Gleixner, Marc Zyngier, Rob Herring, Krzysztof Kozlowski,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Philipp Zabel, linux-gpio
  Cc: linux-kernel, devicetree, linux-renesas-soc, Prabhakar, Biju Das,
	Lad Prabhakar
In-Reply-To: <20220509050953.11005-1-prabhakar.mahadev-lad.rj@bp.renesas.com>

Allow free() callback to be overridden from irq_domain_ops for
hierarchical chips.

This allows drivers to free any resources which are allocated during
populate_parent_alloc_arg().

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/gpio/gpiolib.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b7694171655c..7be01c70ee4e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1187,15 +1187,18 @@ static void gpiochip_hierarchy_setup_domain_ops(struct irq_domain_ops *ops)
 	ops->activate = gpiochip_irq_domain_activate;
 	ops->deactivate = gpiochip_irq_domain_deactivate;
 	ops->alloc = gpiochip_hierarchy_irq_domain_alloc;
-	ops->free = irq_domain_free_irqs_common;
 
 	/*
-	 * We only allow overriding the translate() function for
+	 * We only allow overriding the translate() and free() function for
 	 * hierarchical chips, and this should only be done if the user
-	 * really need something other than 1:1 translation.
+	 * really need something other than 1:1 translation for translate()
+	 * callback and free if user wants to free up any resources which
+	 * were allocated during callbacks for example populate_parent_alloc_arg.
 	 */
 	if (!ops->translate)
 		ops->translate = gpiochip_hierarchy_irq_domain_translate;
+	if (!ops->free)
+		ops->free = irq_domain_free_irqs_common;
 }
 
 static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
-- 
2.25.1


^ permalink raw reply related

* Re: [RFC v8 net-next 04/16] net: mdio: mscc-miim: add ability to be used in a non-mmio configuration
From: Andy Shevchenko @ 2022-05-08 21:14 UTC (permalink / raw)
  To: Colin Foster
  Cc: linux-arm-kernel, linux-gpio, netdev, Terry Bowman, Wolfram Sang,
	Steen Hegelund, Lars Povlsen, Linus Walleij, Russell King,
	Heiner Kallweit, Paolo Abeni, Jakub Kicinski, Eric Dumazet,
	David S. Miller, Florian Fainelli, Vivien Didelot, Andrew Lunn,
	UNGLinuxDriver, Alexandre Belloni, Claudiu Manoil,
	Vladimir Oltean, Lee Jones
In-Reply-To: <20220508185313.2222956-5-colin.foster@in-advantage.com>

On Sun, May 8, 2022 at 8:53 PM Colin Foster
<colin.foster@in-advantage.com> wrote:
>
> There are a few Ocelot chips that contain the logic for this bus, but are
> controlled externally. Specifically the VSC7511, 7512, 7513, and 7514. In
> the externally controlled configurations these registers are not
> memory-mapped.
>
> Add support for these non-memory-mapped configurations.

...

> +               res = platform_get_resource(pdev, IORESOURCE_REG, 0);
> +               if (!res) {
> +                       dev_err(dev, "Unable to get MIIM resource\n");
> +                       return -ENODEV;

return dev_err_probe(...); ?

> +               }

...

> +       if (IS_ERR(phy_regmap)) {
> +               dev_err(dev, "Unable to create phy register regmap\n");
> +               return PTR_ERR(phy_regmap);

Ditto.

>         }

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox