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: <20230715010407.1751715-1-fabrizio.castro.jz@renesas.com>
 <20230715010407.1751715-8-fabrizio.castro.jz@renesas.com> <CAMuHMdUzxQfi2w8dyF8gCitkSKkw_fRE074EUuB+tHKn8WM3=w@mail.gmail.com>
 <TYWPR01MB8775869576E4B3F0FEFCF3DCC23BA@TYWPR01MB8775.jpnprd01.prod.outlook.com>
In-Reply-To: <TYWPR01MB8775869576E4B3F0FEFCF3DCC23BA@TYWPR01MB8775.jpnprd01.prod.outlook.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Mon, 17 Jul 2023 14:18:22 +0300
Message-ID: <CAHp75VdNvWS04VN58r5VcSJskeVJF0g0_spSRb8f0_OP1P04QQ@mail.gmail.com>
Subject: Re: [PATCH 07/10] spi: rzv2m-csi: Switch to using {read,write}s{b,w}
To:     Fabrizio Castro <fabrizio.castro.jz@renesas.com>
Cc:     Geert Uytterhoeven <geert@linux-m68k.org>,
        Mark Brown <broonie@kernel.org>,
        "linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Chris Paterson <Chris.Paterson2@renesas.com>,
        Biju Das <biju.das@bp.renesas.com>,
        Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        "linux-renesas-soc@vger.kernel.org" 
        <linux-renesas-soc@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Jul 17, 2023 at 1:36=E2=80=AFPM Fabrizio Castro
<fabrizio.castro.jz@renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Subject: Re: [PATCH 07/10] spi: rzv2m-csi: Switch to using
> > {read,write}s{b,w}
> > On Sat, Jul 15, 2023 at 3:04=E2=80=AFAM Fabrizio Castro
> > <fabrizio.castro.jz@renesas.com> wrote:

...

> > According to the hardware documentation[1], the access size for both
> > the
> > CSI_OFIFO and CSI_IFIFO registers is 32 bits, so you must use writel()
> > resp. readl().  So please check with the hardware people first.
> >
> > [1] RZ/V2M User's Manual Hardware, Rev. 1.30.
>
> You are right, access is 32 bits (and although this patch works fine,
> we should avoid accessing those regs any other way). Now I remember
> why I decided to go for the bespoke loops in the first place, writesl
> and readsl provide the right register access, but the wrong pointer
> arithmetic for this use case.
> For this patch I ended up selecting writesw/writesb/readsw/readsb to
> get the right pointer arithmetic, but the register access is not as
> per manual.
>
> I can either fallback to using the bespoke loops (I can still
> remove the unnecessary u8* and u16* casting ;-) ), or I can add
> new APIs for this sort of access to io.h (e.g. writesbl, writeswl,
> readsbl, readswl, in order to get the pointer arithmetic right for
> the type of array handled, while keeping memory access as expected).
>
> What are your thoughts on that?

I think that you need to use readsl() / writesl() on the custom buffer
with something like

*_sparse() / *_condence() APIs added (perhaps locally to this driver)
as they may be reused by others in the future.
Having all flavours of read*()/write*() does not scale in my opinion.

--=20
With Best Regards,
Andy Shevchenko