linux-kernel - Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver

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>] [day] [month] [year] [list]

Date:   Fri, 9 Aug 2019 17:25:57 +0200
From:   Boris Brezillon <boris.brezillon@...labora.com>
To:     Tomer Maimon <tmaimon77@...il.com>
Cc:     Mark Brown <broonie@...nel.org>, Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Boris Brezillon <bbrezillon@...nel.org>,
        Avi Fishman <avifishman70@...il.com>,
        Tali Perry <tali.perry1@...il.com>,
        Patrick Venture <venture@...gle.com>,
        Nancy Yuen <yuenn@...gle.com>,
        Benjamin Fair <benjaminfair@...gle.com>,
        linux-spi@...r.kernel.org, devicetree <devicetree@...r.kernel.org>,
        OpenBMC Maillist <openbmc@...ts.ozlabs.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver

On Fri, 9 Aug 2019 18:26:23 +0300
Tomer Maimon <tmaimon77@...il.com> wrote:

> Hi Boris,
> 
> Thanks a lot for your comment.
> 
> On Thu, 8 Aug 2019 at 18:32, Boris Brezillon <boris.brezillon@...labora.com>
> wrote:
> 
> > On Thu,  8 Aug 2019 16:14:48 +0300
> > Tomer Maimon <tmaimon77@...il.com> wrote:
> >
> >  
> > > +
> > > +static const struct spi_controller_mem_ops npcm_fiu_mem_ops = {
> > > +     .exec_op = npcm_fiu_exec_op,  
> >
> > No npcm_supports_op()? That's suspicious, especially after looking at
> > the npcm_fiu_exec_op() (and the functions called from there) where the
> > requested ->buswidth seems to be completely ignored...
> >
> > Sorry but I do not fully understand it, do you mean a support for the  
> buswidth?
> If yes it been done in the UMA functions as follow:
> 
>                 uma_cfg |= ilog2(op->cmd.buswidth);
>                 uma_cfg |= ilog2(op->addr.buswidth) <<
>                         NPCM_FIU_UMA_CFG_ADBPCK_SHIFT;
>                 uma_cfg |= ilog2(op->data.buswidth) <<
>                         NPCM_FIU_UMA_CFG_WDBPCK_SHIFT;
>                 uma_cfg |= op->addr.nbytes << NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;
>                 regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR, op->addr.val);
>

Hm, the default supports_op() implementation might be just fine for
your use case. But there's one thing you still need to check: the
number of addr cycles (or address size as you call it in this driver).
Looks like your IP is limited to 4 address cycles, if I'm right, you
should reject any operation that have op->addr.nbytes > 4. I also
wonder if there's a limitation on the data size you can have on a
single transfer. If there's one you should implement ->adjust_op() too.