Date:   Mon, 27 Feb 2023 21:40:47 -0500
From:   William Breathitt Gray <william.gray@linaro.org>
To:     Mark Brown <broonie@kernel.org>
Cc:     linus.walleij@linaro.org, brgl@bgdev.pl,
        linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        michael@walle.cc, quarium@gmail.com, jhentges@accesio.com,
        jay.dolan@accesio.com
Subject: Re: [PATCH 0/3] Migrate PCIe-IDIO-24 GPIO driver to the regmap API
Message-ID: <Y/1pr33s5IAE4QJS@fedora>
References: <cover.1677547393.git.william.gray@linaro.org>
 <Y/5RfjJCDdrZbHgJ@sirena.org.uk>
 <Y/1ksJHUENCwg/jy@fedora>
 <Y/5V7a4M+LhZQhsM@sirena.org.uk>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="1ZpSqVT8lVV/SEQV"
Content-Disposition: inline
In-Reply-To: <Y/5V7a4M+LhZQhsM@sirena.org.uk>
Precedence: bulk


--1ZpSqVT8lVV/SEQV
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 28, 2023 at 07:28:45PM +0000, Mark Brown wrote:
> On Mon, Feb 27, 2023 at 09:19:28PM -0500, William Breathitt Gray wrote:
> > On Tue, Feb 28, 2023 at 07:09:50PM +0000, Mark Brown wrote:
>=20
> > > The values from the config buffer are supposed to be written out in
> > > regmap_irq_sync_unlock() - why is something custom needed here?
>=20
> > The PCIe-IDIO-24 "COS Enable" serves a dual purpose of interrupt
> > enabling/disabling as well as configuring the interrupt types. Since
> > this register is used for masking, config buffer would clobber the
> > register if we use it in this particular case. Instead, we ignore the
> > config buffer and configure the type directly for the device (handling
> > the case where interrupts are masked and shouldn't be enabled).
>=20
> Could you be more concrete about what's going on here please?  In what
> way does this "COS Enable" serve these dual functions and why do they
> clobber each other?

An explanation of the device registers is provided in [PATCH 3/3];
here's the relevant portion:

The COS Enable register is used to enable/disable interrupts and
configure the interrupt levels; each bit maps to a group of eight inputs
as described below:

    Bit 0: IRQ EN Rising Edge IN0-7
    Bit 1: IRQ EN Rising Edge IN8-15
    Bit 2: IRQ EN Rising Edge IN16-23
    Bit 3: IRQ EN Rising Edge TTL0-7
    Bit 4: IRQ EN Falling Edge IN0-7
    Bit 5: IRQ EN Falling Edge IN8-15
    Bit 6: IRQ EN Falling Edge IN16-23
    Bit 7: IRQ EN Falling Edge TTL0-7

An interrupt is asserted when a change-of-state matching the interrupt
level configuration respective for a particular group of eight inputs
with enabled COS is detected.

So in order to mask lines, the respective bits need to be set to 0.
However, if we use the regmap-irq config buffer to set the type, this
mask will be cloberred and the disabled lines become enabled. To prevent
the clobber, we can save the type configuration to irq_drv_data for use
later in handle_mask_sync() and then update the type in this COS Enable
register only when the lines are unmasked.

William Breathitt Gray

--1ZpSqVT8lVV/SEQV
Content-Type: application/pgp-signature; name="signature.asc"

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

iHUEARYKAB0WIQSNN83d4NIlKPjon7a1SFbKvhIjKwUCY/1prwAKCRC1SFbKvhIj
K9s+AQCtKcJaTgx6IbX3HD2eUty2muCO/guMz/Cm1PjLitiLuQEAlSup7rwoNaBJ
Pa3mDIMAMQpO0Kugdp57+rXzMlVsvgU=
=nE8M
-----END PGP SIGNATURE-----

--1ZpSqVT8lVV/SEQV--