diff mbox series

pinctrl: renesas: rzg2l: Configure interrupt input mode

Message ID 20231006121823.229193-1-biju.das.jz@bp.renesas.com
State Superseded
Headers show
Series pinctrl: renesas: rzg2l: Configure interrupt input mode | expand

Commit Message

Biju Das Oct. 6, 2023, 12:18 p.m. UTC
Configure GPIO interrupt as input mode. Also if the bootloader sets
gpio interrupt pin as function, override it as gpio.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Comments

Geert Uytterhoeven Oct. 12, 2023, 3:35 p.m. UTC | #1
Hi Biju,

On Fri, Oct 6, 2023 at 2:18 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Configure GPIO interrupt as input mode. Also if the bootloader sets
> gpio interrupt pin as function, override it as gpio.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -1580,6 +1580,26 @@ static const struct irq_chip rzg2l_gpio_irqchip = {
>         GPIOCHIP_IRQ_RESOURCE_HELPERS,
>  };
>
> +static int rzg2l_gpio_interrupt_input_mode(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip);
> +       const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[offset];
> +       u32 *pin_data = pin_desc->drv_data;
> +       u32 off = RZG2L_PIN_CFG_TO_PORT_OFFSET(*pin_data);
> +       u8 bit = RZG2L_PIN_ID_TO_PIN(offset);
> +       u8 reg8;
> +       int ret;
> +
> +       reg8 = readb(pctrl->base + PMC(off));
> +       if (reg8 & BIT(bit)) {
> +               ret = rzg2l_gpio_request(chip, offset);

Who is taking care of calling pinctrl_gpio_free() when the interrupt
has been freed?

> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return rzg2l_gpio_direction_input(chip, offset);
> +}
> +
>  static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
>                                             unsigned int child,
>                                             unsigned int child_type,
> @@ -1589,11 +1609,16 @@ static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
>         struct rzg2l_pinctrl *pctrl = gpiochip_get_data(gc);
>         unsigned long flags;
>         int gpioint, irq;
> +       int ret;
>
>         gpioint = rzg2l_gpio_get_gpioint(child, pctrl->data);
>         if (gpioint < 0)
>                 return gpioint;
>
> +       ret = rzg2l_gpio_interrupt_input_mode(gc, child);
> +       if (ret)
> +               return ret;
> +
>         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);

TBH, it's not very clear to me how this is used...
I assume this is called as girq->child_to_parent_hwirq() from
gpiochip_hierarchy_irq_domain_alloc()?
Is that done from request_irq(), or at interrupt controller
initialization time?

Thanks!

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
Biju Das Oct. 12, 2023, 4:24 p.m. UTC | #2
Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH] pinctrl: renesas: rzg2l: Configure interrupt input
> mode
> 
> Hi Biju,
> 
> On Fri, Oct 6, 2023 at 2:18 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Configure GPIO interrupt as input mode. Also if the bootloader sets
> > gpio interrupt pin as function, override it as gpio.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -1580,6 +1580,26 @@ static const struct irq_chip rzg2l_gpio_irqchip =
> {
> >         GPIOCHIP_IRQ_RESOURCE_HELPERS,  };
> >
> > +static int rzg2l_gpio_interrupt_input_mode(struct gpio_chip *chip,
> > +unsigned int offset) {
> > +       struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip);
> > +       const struct pinctrl_pin_desc *pin_desc = &pctrl-
> >desc.pins[offset];
> > +       u32 *pin_data = pin_desc->drv_data;
> > +       u32 off = RZG2L_PIN_CFG_TO_PORT_OFFSET(*pin_data);
> > +       u8 bit = RZG2L_PIN_ID_TO_PIN(offset);
> > +       u8 reg8;
> > +       int ret;
> > +
> > +       reg8 = readb(pctrl->base + PMC(off));
> > +       if (reg8 & BIT(bit)) {
> > +               ret = rzg2l_gpio_request(chip, offset);
> 
> Who is taking care of calling pinctrl_gpio_free() when the interrupt has
> been freed?

At the moment no, maybe we can use of rzg2l_gpio_irq_domain_free() to call pinctrl_gpio_free(),
Similar to freeing bitmap region??

> 
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> > +       return rzg2l_gpio_direction_input(chip, offset); }
> > +
> >  static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
> >                                             unsigned int child,
> >                                             unsigned int child_type,
> > @@ -1589,11 +1609,16 @@ static int
> rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
> >         struct rzg2l_pinctrl *pctrl = gpiochip_get_data(gc);
> >         unsigned long flags;
> >         int gpioint, irq;
> > +       int ret;
> >
> >         gpioint = rzg2l_gpio_get_gpioint(child, pctrl->data);
> >         if (gpioint < 0)
> >                 return gpioint;
> >
> > +       ret = rzg2l_gpio_interrupt_input_mode(gc, child);
> > +       if (ret)
> > +               return ret;
> > +
> >         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);
> 
> TBH, it's not very clear to me how this is used...
> I assume this is called as girq->child_to_parent_hwirq() from
> gpiochip_hierarchy_irq_domain_alloc()?
> Is that done from request_irq(), or at interrupt controller initialization
> time?

Basically during i2c_device_probe(). See the stack trace.

[    8.371896]  rzg2l_gpio_child_to_parent_hwirq+0xc4/0x1b8
[    8.377882]  gpiochip_hierarchy_irq_domain_alloc+0x90/0x1a4
[    8.384156]  irq_domain_alloc_irqs_locked+0xfc/0x354
[    8.389791]  irq_create_fwspec_mapping+0x208/0x350
[    8.395217]  irq_create_of_mapping+0x68/0x8c
[    8.400087]  of_irq_get+0x88/0xcc
[    8.403881]  i2c_device_probe+0x270/0x284

Cheers,
Biju
Geert Uytterhoeven Dec. 1, 2023, 2:50 p.m. UTC | #3
Hi Biju,

On Thu, Oct 12, 2023 at 6:24 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH] pinctrl: renesas: rzg2l: Configure interrupt input
> > mode
> > On Fri, Oct 6, 2023 at 2:18 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > Configure GPIO interrupt as input mode. Also if the bootloader sets
> > > gpio interrupt pin as function, override it as gpio.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > > @@ -1580,6 +1580,26 @@ static const struct irq_chip rzg2l_gpio_irqchip =
> > {
> > >         GPIOCHIP_IRQ_RESOURCE_HELPERS,  };
> > >
> > > +static int rzg2l_gpio_interrupt_input_mode(struct gpio_chip *chip,
> > > +unsigned int offset) {
> > > +       struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip);
> > > +       const struct pinctrl_pin_desc *pin_desc = &pctrl-
> > >desc.pins[offset];
> > > +       u32 *pin_data = pin_desc->drv_data;

Note to self: Prabhakar's patch[1] changes this to an array of u64s.

> > > +       u32 off = RZG2L_PIN_CFG_TO_PORT_OFFSET(*pin_data);
> > > +       u8 bit = RZG2L_PIN_ID_TO_PIN(offset);
> > > +       u8 reg8;
> > > +       int ret;
> > > +
> > > +       reg8 = readb(pctrl->base + PMC(off));
> > > +       if (reg8 & BIT(bit)) {
> > > +               ret = rzg2l_gpio_request(chip, offset);
> >
> > Who is taking care of calling pinctrl_gpio_free() when the interrupt has
> > been freed?
>
> At the moment no, maybe we can use of rzg2l_gpio_irq_domain_free() to call pinctrl_gpio_free(),
> Similar to freeing bitmap region??

Sounds good to me.
Thanks!

[1] "[PATCH v3 1/3] pinctrl: renesas: rzg2l: Include pinmap in
    RZG2L_GPIO_PORT_PACK() macro"
    https://lore.kernel.org/linux-renesas-soc/20231201131551.201503-2-prabhakar.mahadev-lad.rj@bp.renesas.com

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index c7c6d912a975..5233e4531d80 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -1580,6 +1580,26 @@  static const struct irq_chip rzg2l_gpio_irqchip = {
 	GPIOCHIP_IRQ_RESOURCE_HELPERS,
 };
 
+static int rzg2l_gpio_interrupt_input_mode(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rzg2l_pinctrl *pctrl = gpiochip_get_data(chip);
+	const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[offset];
+	u32 *pin_data = pin_desc->drv_data;
+	u32 off = RZG2L_PIN_CFG_TO_PORT_OFFSET(*pin_data);
+	u8 bit = RZG2L_PIN_ID_TO_PIN(offset);
+	u8 reg8;
+	int ret;
+
+	reg8 = readb(pctrl->base + PMC(off));
+	if (reg8 & BIT(bit)) {
+		ret = rzg2l_gpio_request(chip, offset);
+		if (ret)
+			return ret;
+	}
+
+	return rzg2l_gpio_direction_input(chip, offset);
+}
+
 static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
 					    unsigned int child,
 					    unsigned int child_type,
@@ -1589,11 +1609,16 @@  static int rzg2l_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
 	struct rzg2l_pinctrl *pctrl = gpiochip_get_data(gc);
 	unsigned long flags;
 	int gpioint, irq;
+	int ret;
 
 	gpioint = rzg2l_gpio_get_gpioint(child, pctrl->data);
 	if (gpioint < 0)
 		return gpioint;
 
+	ret = rzg2l_gpio_interrupt_input_mode(gc, child);
+	if (ret)
+		return ret;
+
 	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);