RE: [PATCH 2/2] pinctrl: renesas: rzg2l: Enable noise filter for GPIO interrupt input

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	Marc Zyngier <maz@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-renesas-soc@vger.kernel.org" 
	<linux-renesas-soc@vger.kernel.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	Biju Das <biju.das.au@gmail.com>
Subject: RE: [PATCH 2/2] pinctrl: renesas: rzg2l: Enable noise filter for GPIO interrupt input
Date: Mon, 18 Sep 2023 13:18:50 +0000	[thread overview]
Message-ID: <OS0PR01MB5922356278D492FC32F0583586FBA@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAMuHMdVcy82eQNKtqufk8Nb6PKYTgmZw_b_mC+qgmGMHt1gyiw@mail.gmail.com>

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 2/2] pinctrl: renesas: rzg2l: Enable noise filter for
> GPIO interrupt input
> 
> Hi Biju,
> 
> On Mon, Sep 18, 2023 at 2:34 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > As per RZ/G2L hardware manual Rev.1.30 section 8.7.3 GPIO Interrupt
> > (TINT) and 41.4.1 Operation for GPIO function, we need to set digital
> > noise filter for GPIO interrupt.
> >
> > This patch enables noise filter for GPIO interrupt in
> > rzg2l_gpio_irq_enable() and disable it in rzg2l_gpio_irq_disable().
> >
> > Fixes: db2e5f21a48e ("pinctrl: renesas: pinctrl-rzg2l: Add IRQ domain
> > to handle GPIO interrupt")
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -96,6 +96,7 @@
> >  #define PIN(n)                 (0x0800 + 0x10 + (n))
> >  #define IOLH(n)                        (0x1000 + (n) * 8)
> >  #define IEN(n)                 (0x1800 + (n) * 8)
> > +#define FILONOFF(n)            (0x2080 + (n) * 8)
> >  #define ISEL(n)                        (0x2c80 + (n) * 8)
> >  #define PWPR                   (0x3014)
> >  #define SD_CH(n)               (0x3000 + (n) * 4)
> 
> LGTM, but shouldn't you configure the Digital Noise Filter Number
> (FILNUM) and Clock Selection (FILCLKSEL) registers, too?

Currently it uses reset values.

00b: 4-stage filter (41.666 ns x 4 = 166.666 ns) (initial value) for FILNUM and

00b: Not divided (initial value) for FILCLKSEL

Do you mean we should provide these settings to DT, so that
it is customised based on the PCB design and the environment
the board is used in? I guess this will make it easier for
customers to make the required changes for their application.

Cheers,
Biju