Feedback-ID: i8771445c:Fastmail
Date:   Fri, 10 Feb 2023 09:29:36 +0100
From:   Maxime Ripard <maxime@cerno.tech>
To:     Andre Przywara <andre.przywara@arm.com>
Cc:     Andreas Feldner <andreas@feldner-bv.de>,
        Andreas Feldner <pelzi@flying-snail.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Chen-Yu Tsai <wens@csie.org>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Samuel Holland <samuel@sholland.org>,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ARM: dts: allwinner: minimize irq debounce filter per
 default
Message-ID: <20230210082936.qefzz4fsp3jpalvp@houat>
References: <Y+FaVorMl37F5Dve@debian-qemu.internal.flying-snail.de>
 <20230207011608.2ce24d17@slackpad.lan>
 <d0534762-3785-ec2d-8d1e-aba0e39f701b@feldner-bv.de>
 <20230209202952.673d5a60@slackpad.lan>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="3nragrb2jbk2zyfl"
Content-Disposition: inline
In-Reply-To: <20230209202952.673d5a60@slackpad.lan>
Precedence: bulk


--3nragrb2jbk2zyfl
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 09, 2023 at 08:29:52PM +0000, Andre Przywara wrote:
> > >>   &pio {
> > >> +	/* 1=EF=BF=BDs debounce filter on both IRQ banks */
> > > Is that supposed to be <micro> in UTF-8? It seems to have got lost in
> > > translation, or is that just me?
> > O yes, the Greek character slipped into the comment.
> > >> +	input-debounce =3D <1 1>;
> > > As mentioned above, I am not so sure this is generic enough to put it
> > > here for PA. And what is the significance of "1 us", in particular? Is
> > > that just the smallest value? =20
> >=20
> > Yes indeed it's a bit more complicated than I feel it needs to be. The
> > configuration is taken as microseconds and translated into the best
> > matching clock and divider by the driver. However, 0 is not translated
> > to the lowest divider of the high speed clock as would be logical if
> > you ask for zero microseconds, but to "leave at default". The default
> > of the board is 0 in the register, translating to lowest divider on the
> > _low_ speed clock.
>=20
> I'd say the "if (!debounce) continue;" code is just to defend against
> the division by zero, which would be the next statement to execute.
>=20
> We might want to change that to interpret 0 as "lowest possible", which
> would be 24MHz/1. Please feel free to send a patch in this regard, and
> CC: Maxime, to get some input on that idea.

I never had any complaint on that part either, so the default looks sane
to me.

If some board needs a higher debouncing rate, then we should obviously
set it up in the device tree of that board, but changing it for every
user also introduces the risk of breaking other boards that actually
require a lower debouncing frequency.

And any default is always going to be debated, there's one, it seems to
create little controversy, it seems to work in most case, I'd just stick
with it.

> > To me this is mindboggling.
> >=20
> > If you want to keep IRQ bank A as it is today and switch off the
> > definitely unnecessary (and _potentially_ IRQ eating) debounce off
> > for bank G only, I'd suggest the following setting:
> >=20
> >  =A0=A0=A0 input-debounce =3D <31 1>;
>=20
> It should be documented that the effective default is 31, I guess the
> binding is the right place.

Yeah, if the documentation is lacking, we should definitely improve it.

Maxime

--3nragrb2jbk2zyfl
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCY+YAawAKCRDj7w1vZxhR
xYg7AP0ckcuX2nOcAZ6OY/Rg9KQEpnr6I19NX9UOSX+nlSREVgD+JSHvch+fVoZk
53OrNbFnFVJH7IBNYSyVW1yqOhvuoA4=
=eFLj
-----END PGP SIGNATURE-----

--3nragrb2jbk2zyfl--