linux-kernel - Re: [PATCH 07/10] spi: rzv2m-csi: Switch to using {read,write}s{b,w}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75VdNvWS04VN58r5VcSJskeVJF0g0_spSRb8f0_OP1P04QQ@mail.gmail.com>
Date:   Mon, 17 Jul 2023 14:18:22 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Fabrizio Castro <fabrizio.castro.jz@...esas.com>
Cc:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Mark Brown <broonie@...nel.org>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Chris Paterson <Chris.Paterson2@...esas.com>,
        Biju Das <biju.das@...renesas.com>,
        Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH 07/10] spi: rzv2m-csi: Switch to using {read,write}s{b,w}

On Mon, Jul 17, 2023 at 1:36 PM Fabrizio Castro
<fabrizio.castro.jz@...esas.com> wrote:
> > From: Geert Uytterhoeven <geert@...ux-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 AM Fabrizio Castro
> > <fabrizio.castro.jz@...esas.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.

-- 
With Best Regards,
Andy Shevchenko