Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 3 Sep 2019 13:12:10 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Ben Whitten <ben.whitten@gmail.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Andreas =?iso-8859-1?Q?F=E4rber?= <afaerber@suse.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>, nandor.han@vaisala.com
Subject: Re: [PATCH] regmap: fix writes to non incrementing registers
Message-ID: <20190903121210.GA7916@sirena.co.uk>
References: <20190813212251.12316-1-ben.whitten@gmail.com>
 <20190814100115.GF4640@sirena.co.uk>
 <CAF3==iuZvCnmAg9hqs8ivHw0wHaUQEf8k9U8=KTekMMjdyyEKg@mail.gmail.com>
 <20190814161938.GI4640@sirena.co.uk>
 <CAF3==ivJycGpSpSy3Y9cL7kNf27=kZmETpb-v0=L7BNd1ZPJtg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="envbJBWh7q8WU6mo"
Content-Disposition: inline
In-Reply-To: <CAF3==ivJycGpSpSy3Y9cL7kNf27=kZmETpb-v0=L7BNd1ZPJtg@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--envbJBWh7q8WU6mo
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Sep 03, 2019 at 10:42:59AM +0100, Ben Whitten wrote:

> You mentioned that we likely have breakage elsewhere, I believe that
> regmap_noinc_write probably shouldn't ever have been calling
> _regmap_raw_write.

> Whilst regmap_noinc_read calls _regmap_raw_read, this function doesn't
> do any massage and just calls map->bus->read after selecting a page.
> regmap_raw_write on the other hand is meant for writing blocks to
> register ranges as these added checks show, and does split work based
> on page length etc.
> This splitting up is something that shouldn't apply to the FIFO as it's a
> deep register.

This just means that we need to take care of nonincrementing registers
in there, we don't want to end up with too many different places where
we might have to handle formatting since that leads to duplication.
It's about marshalling for physical I/O, it's a bit misnamed at this
point but nonincrementing registers definitely fit in there for me.

> I modified regmap_noinc_write to be much more like the raw_read
> internals, just select page then attempt a map->bus->gather_write,
> falling back to linearising if that's not supported.
> This approach worked at getting writing working into the FIFO.

Modify raw_write() to handle this instead, it is a mirror of the read
side it's just that writes are more complicated since there's more
things that could happen as you're discovering here.

> So I would propose that there are two 'Fixes:' patches here, one
> enhancing the checking when writing a register range to ensure you
> don't stumble into a noinc register.

Yes.

> And one to fix noinc_writes to send data directly to the bus once the
> page is selected with no splitting up as you would a range.

Push that into raw_write(), it just needs to special case when the first
register in a block is non-incrementing in the page selection logic (add
a !nonincrmenting in the while loop I think).

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

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

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl1uWJoACgkQJNaLcl1U
h9DPtgf/QDJZEpN/oh11BWf2OipytBVamOF1yMTPyALBAHdYTihwHFTrO7gWAbg4
V+3wRiHEpEAjudpyqt+YnTxKEt3MJm2mqfMRcVuZah/FfXFOa3SEuJeBhi8fjWdE
568d9++vG2mRlrCgqcPFZBI3VhMpifwmfiHC/r3auNIDGWhSBr1gG1T31NemVsd7
XCvOjkHGRU7SowtC5VQ75w/LKmCYzDHzGo52sCh+RCPb5BlAJF9PCHeQ6np0SDvn
0ptOHJVJZsLu1MxcR8BP+bNKDIsO5N/Fnbop9ed95wW9tIYP2M0XAA3T75UXkVpR
gy2roGuCXQgZIs4QtdZ/vfJO3aQOSw==
=YiAn
-----END PGP SIGNATURE-----

--envbJBWh7q8WU6mo--