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: <20201001083737.iacunfb7l75elvex@ti.com>
Date:   Thu, 1 Oct 2020 14:07:39 +0530
From:   Pratyush Yadav <p.yadav@...com>
To:     <Tudor.Ambarus@...rochip.com>
CC:     <miquel.raynal@...tlin.com>, <richard@....at>, <vigneshr@...com>,
        <linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
        <nsekhar@...com>, <boris.brezillon@...labora.com>
Subject: Re: [PATCH v14 03/15] mtd: spi-nor: add support for DTR protocol

On 01/10/20 07:46AM, Tudor.Ambarus@...rochip.com wrote:
> On 9/30/20 9:57 PM, Pratyush Yadav wrote:
> > @@ -2387,12 +2496,16 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor 
> > *nor, u32 *hwcaps)
> >         struct spi_nor_flash_parameter *params = nor->params;
> >         unsigned int cap;
> > 
> > -       /* DTR modes are not supported yet, mask them all. */
> > -       *hwcaps &= ~SNOR_HWCAPS_DTR;
> > -
> >         /* X-X-X modes are not supported yet, mask them all. */
> >         *hwcaps &= ~SNOR_HWCAPS_X_X_X;
> > 
> > +       /*
> > +        * If the reset line is broken, we do not want to enter a stateful
> > +        * mode.
> > +        */
> > +       if (nor->flags & SNOR_F_BROKEN_RESET)
> > +               *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
> 
> SNOR_HWCAPS_X_X_X is already masked out above. Do we need to do it again?

That might change later and the person removing that line might not 
remember or even know to add it back here.
 
> > +
> >         for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
> >                 int rdidx, ppidx;
> > 
> > @@ -2967,7 +3098,9 @@ static int spi_nor_init(struct spi_nor *nor)
> >                 return err;
> >         }
> > 
> > -       if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES)) {
> > +       if (nor->addr_width == 4 &&
> > +           nor->read_proto != SNOR_PROTO_8_8_8_DTR &&
> > +           !(nor->flags & SNOR_F_4B_OPCODES)) {
> >                 /*
> >                  * If the RESET# pin isn't hooked up properly, or the system
> >                  * otherwise doesn't perform a reset command in the boot
> > @@ -3024,7 +3157,21 @@ static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
> > 
> >  static int spi_nor_set_addr_width(struct spi_nor *nor)
> >  {
> > -       if (nor->addr_width) {
> > +       if (nor->read_proto == SNOR_PROTO_8_8_8_DTR) {
> 
> This should come as an "else if". We need to let the posibility to retrieve
> addr_width from SFDP, the standard knows better than us.

Ok. Will fix.
 
> With these addressed, one can add:
> 
> Reviewed-by: Tudor Ambarus <tudor.ambarus@...rochip.com>
> 
> > +               /*
> > +                * In 8D-8D-8D mode, one byte takes half a cycle to transfer. So
> > +                * in this protocol an odd address width cannot be used because
> > +                * then the address phase would only span a cycle and a half.
> > +                * Half a cycle would be left over. We would then have to start
> > +                * the dummy phase in the middle of a cycle and so too the data
> > +                * phase, and we will end the transaction with half a cycle left
> > +                * over.
> > +                *
> > +                * Force all 8D-8D-8D flashes to use an address width of 4 to
> > +                * avoid this situation.
> > +                */
> > +               nor->addr_width = 4;
> > +       } else if (nor->addr_width) {
> >                 /* already configured from SFDP */
> >         } else if (nor->info->addr_width) {
> >                 nor->addr_width = nor->info->addr_width;

-- 
Regards,
Pratyush Yadav
Texas Instruments India

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ