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:   Tue, 25 Apr 2023 13:56:23 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Colin Foster <colin.foster@in-advantage.com>
Cc:     Maxime Chevallier <maxime.chevallier@bootlin.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Lee Jones <lee@kernel.org>, linux-kernel@vger.kernel.org,
        thomas.petazzoni@bootlin.com, alexis.lothore@bootlin.com
Subject: Re: [PATCH] regmap: don't check for alignment when using reg_shift
Message-ID: <bb836be3-456c-48fd-9b19-62279fee6b8d@sirena.org.uk>
References: <20230420150617.381922-1-maxime.chevallier@bootlin.com>
 <ZEKwxhJJNkuX7VTr@colin-ia-desktop>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="DiFt6ZB+CLaQPege"
Content-Disposition: inline
In-Reply-To: <ZEKwxhJJNkuX7VTr@colin-ia-desktop>
Precedence: bulk


--DiFt6ZB+CLaQPege
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Apr 21, 2023 at 08:50:30AM -0700, Colin Foster wrote:
> On Thu, Apr 20, 2023 at 05:06:17PM +0200, Maxime Chevallier wrote:

> > On regmap consumers that require address translation through
> > up/downshifting, the alignment check in the regmap core doesn't take the
> > translation into account. This doesn't matter when downshifting the
> > register address, as any address that fits a given alignment requirement
> > will still meet it when downshifted (a 4-byte aligned address will
> > always also be 2-bytes aligned for example).

> > However, when upshifting, this check causes spurious errors, as it
> > occurs before the upshifting.

> I don't follow why upshifting should make a difference to alignment.
> Assuming it does though, would it make sense to test

> map->format.reg_shift > 0

> instead of just !map->format.reg_shift?

Yeah, I think the question is more when we should run the alignment
check than if we should have one.  I think running the check after any
shifting makes sense, we'd be better off reorganising the checks if
needed than removing them.

>=20
> > -	if (!IS_ALIGNED(reg, map->reg_stride))
> > +	if (!map->format.reg_shift && !IS_ALIGNED(reg, map->reg_stride))
> >  		return -EINVAL;
>=20
> In the case of ocelot_spi, we'd want to flag an invalid access to a
> register like 0x71070003... Before this patch it would return -EINVAL,
> after this patch it would access 0x71070000.
>=20
> Colin Foster

--DiFt6ZB+CLaQPege
Content-Type: application/pgp-signature; name="signature.asc"

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

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmRHzfYACgkQJNaLcl1U
h9Cpggf/c7E3rKx9CqXXzBlq45k7DnQbPm0a1LBhR2l4e4Tt8fLdhpImflz2foAJ
rwiYt18SG3Lr8eXqXrPjItqjiHb79lpjJdB1rlze6rkVJohxVlpxTWZPYnCmhcXu
7NYMzlQpCiPZZzVqmIDF9/IuA/3pinFBVegyzJxdbjbhBGUZDvIL2hez0D2JfAlD
97OlNQPwEJ46d/AoWZjhHecb+7gWHZZRzvREkiqTYDAqrg5ZSXm3DJ+v6j0EfCV2
sWYYyjXdN0x5mLy7yZDiBnDP8FdQh5Fk+A/kQ1gR3Rqh69CJFEuv+O6+mkOgnVmf
VKTVmhsWvZwsYRp1jTkxeUbuv4CKmQ==
=+cEC
-----END PGP SIGNATURE-----

--DiFt6ZB+CLaQPege--