| Message ID | 20231006121823.229193-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | pinctrl: renesas: rzg2l: Configure interrupt input mode | expand |
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
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
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 --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);
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(+)