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;
MIME-Version: 1.0
References: <20230620202824.380313-1-russell.h.weight@intel.com>
 <ZJfB9zXhizbqmIrm@yilunxu-OptiPlex-7050> <c5c95368-eac3-4250-b50a-50f0a526b753@sirena.org.uk>
In-Reply-To: <c5c95368-eac3-4250-b50a-50f0a526b753@sirena.org.uk>
From:   Jim Wylder <jwylder@google.com>
Date:   Mon, 26 Jun 2023 15:23:45 -0500
Message-ID: <CAEP57O-EsrW0KgSDDnDDEYrvL-SEtO-Ji6oShNzvP3y0o9AqCw@mail.gmail.com>
Subject: Re: [PATCH 1/1] regmap: spi-avmm: Fix regmap_bus max_raw_write
To:     Mark Brown <broonie@kernel.org>
Cc:     Xu Yilun <yilun.xu@intel.com>,
        Russ Weight <russell.h.weight@intel.com>,
        gregkh@linuxfoundation.org, rafael@kernel.org,
        linux-kernel@vger.kernel.org, matthew.gerlach@linux.intel.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Jun 26, 2023 at 2:47=E2=80=AFPM Mark Brown <broonie@kernel.org> wro=
te:
>
> On Sun, Jun 25, 2023 at 12:26:31PM +0800, Xu Yilun wrote:
>
> > IIUC, max_raw_write/read is the max batch *DATA* size that could be
> > handled by the receiver. reg addr bytes are not counted in. I'm not 100=
%
> > sure this is obeyed by all drivers. But see several examples:
>
> There's clearly been some confusion in a bunch of drivers, those you've
> identified below need fixing too for the new code from the looks of it.
> I'm frankly unclear why some of the drivers you're pointing at are even
> implementing raw buses.
>
> > So I'm not sure if commit 3981514180c9 is actually necessary.
>
> That's "regmap: Account for register length when chunking".  It's
> certainly a bit unclear now I go do another survey, though it's also
> clear that things like the handling of padding are intermittent at best.
> We probably would be safe reverting that.
>
> Jim, where were you seeing the issue here?

Hope I am answering your question.

The issue I experienced is that if a bus (in my case a limited i2c controll=
er)
defines a quirk with max_raw_write, then the chunking algorithm would
divide the data into max_raw_write chunks.  The i2c bus would then
prepend the address values to the chunk which would
always get rejected because it was at least one byte too large.

My original fix, that I posted was to add a special flag (reg_in_write)
that a bus could set to choose the to have the register accounted
for in the chunking algorithm.  This was admittedly inelegant.

After reviews, we thought using the reg_bytes would be a better
solution and that padding should be accounted for.

I had not seen an issue with padding for this algorithm.  Only
the case specified above with i2c with prepending the address.

Would it be possible to reconsider adding a flag or argument to
regmap_bus to guard this chunking behavior?

>
> Please include human readable descriptions of things like commits and
> issues being discussed in e-mail in your mails, this makes them much
> easier for humans to read especially when they have no internet access.
> I do frequently catch up on my mail on flights or while otherwise
> travelling so this is even more pressing for me than just being about
> making things a bit easier to read.