Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 8 Jun 2023 18:12:46 +0200
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Peter De Schrijver <pdeschrijver@nvidia.com>
Cc:     jonathanh@nvidia.com, mperttunen@nvidia.com, sudeep.holla@arm.com,
        talho@nvidia.com, robh@kernel.org, linux-tegra@vger.kernel.org,
        linux-kernel@vger.kernel.org, stefank@nvidia.com,
        krzysztof.kozlowski@linaro.org
Subject: Re: [PATCH v4 6/6] firmware: tegra: bpmp: Add support for DRAM MRQ
 GSCs
Message-ID: <ZIH9_hFsWz2kKQJy@orome>
References: <20230511132048.1122075-1-pdeschrijver@nvidia.com>
 <20230511132048.1122075-7-pdeschrijver@nvidia.com>
 <ZGNOXO3rRtFx_12R@orome>
 <ZGNS9w+i9Y9gpz6R@44189d9-lcedt>
 <ZICo8wYqM8tmCEob@orome>
 <ZIG571modfPCnl2p@44189d9-lcedt>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="6AOjTCZJVyiG+eT9"
Content-Disposition: inline
In-Reply-To: <ZIG571modfPCnl2p@44189d9-lcedt>
User-Agent: Mutt/2.2.10 (2023-03-25)
Precedence: bulk


--6AOjTCZJVyiG+eT9
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jun 08, 2023 at 02:22:23PM +0300, Peter De Schrijver wrote:
> On Wed, Jun 07, 2023 at 05:57:39PM +0200, Thierry Reding wrote:
> > > No, on the contrary, now it's clear you can either have void __iomem *
> > > and struct gen_pool * or void *virt but not both.
> >=20
> > No, it's not clear. You can have one part of your driver write the
> > sram.virt field and another read dram.virt and they'll end up pointing
> > at the same memory location but with different meaning. That's why you
> > need to introduce the enumeration in order to specify which one of the
> > two you want to pick.
> >=20
> > And that's exactly where you start introducing the potential for
> > inconsistency: now you need to be extra careful that the enumeration and
> > the unions are set correctly. You effectively have two sources of truth
> > and they don't necessarily match. You can also end up (at least
> > theoretically) with the invalid value, so you need an extra check for
> > that too.
> >=20
> > You can avoid all of those inconsistencies if you reduce this to one
> > source of truth, namely the pointers that you're going to use.
> >=20
>=20
> There are 4 possible states for these pointers:
> both NULL
> both non-NULL
> sram pointer NULL, dram pointer non-NULL
> dram pointer NULL, sram pointer non-NULL
>=20
> So how is this one source of truth?

If you add a tristate enum you turn this into 6 possible states, how is
that any better?

My point is that the pointer contents are enough to determine which mode
we use. In either case we have to make sure that the state is consistent
so we can't end up with both non-NULL. The difference is that without
the enum we only have to make sure that the pointers are correct. With
the additional enum we also need to make sure that that value is
consistent with the values that we store in the pointers.

Anyway, time is running out, so I'll just apply the series and "fix"
this up myself.

Thierry

--6AOjTCZJVyiG+eT9
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmSB/f4ACgkQ3SOs138+
s6FYIg//bIAsnX/x/dT+okl1hS6XmkCxBgUmpv1UQ8zI4WHEw5zLyv8zr8rCWJ6c
YZ5jP+8tf7Z10rLfo5/fgMdlkblJDDHAnve5mMCfHwIpmZiBpEDGYVIsSTsBxIfi
TZ3zAHyyAUQX2EHXGkFk3qH+wIH6hv6XpKP7zB6r/E79yLSh8tVMiqevdQWVgp6t
5mb7q27n0aPLYVO2EVhFKydzYi3G/4t0zKGzZkW1Yccuksk04h9PfkLiMlJ9r24e
SYfLZL2c0Sv3tNR5pCg5lJWg9VhbJPQMIXzeDjsjAISQxyGCU67nq58x8bhQ90af
xHQfY01JiYF8g63olr8L3wkUnWIvtUsosxtTWoa5pJHGm1I5//FjJB6Z1bBD8u2w
b9ZbmIG4cQqUM9wMSiQ6+RLM083x0ZMhAcuhjPKExAO4IIxcmdr6QHj8G96ildia
kOW5Olgjg4yzYau9V59V1cGRdt5skkyrGk45Z7YE2K/LwvZKG6jCuqMHCJOqefcI
Kh8Ly+M18PA+CtXkcqkHYQtJVv5qNLe87JdMD9OKJ6sEGdAZ7wCslexGxPkHEd3Z
q9LftXQLjkAuzItlZaQedUJ5XvsJIwP/4kw0roU4NZe+5g6p8uzwzbPdHiUsQoco
oqd1mhIyo0zJHf9oMmIBRTWJ1Z4AzxNvrr8lx/iVArc1jBeQZi4=
=Frik
-----END PGP SIGNATURE-----

--6AOjTCZJVyiG+eT9--