| 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
| ||
|
Message-ID: <20200720163802.veql43cmal7sunit@yadavpratyush.com>
Date: Mon, 20 Jul 2020 22:08:02 +0530
From: Pratyush Yadav <me@...avpratyush.com>
To: Tudor.Ambarus@...rochip.com
Cc: p.yadav@...com, miquel.raynal@...tlin.com, richard@....at,
vigneshr@...com, broonie@...nel.org, Nicolas.Ferre@...rochip.com,
alexandre.belloni@...tlin.com, Ludovic.Desroches@...rochip.com,
matthias.bgg@...il.com, michal.simek@...inx.com,
linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-spi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, boris.brezillon@...labora.com,
nsekhar@...com
Subject: Re: [PATCH v10 07/17] mtd: spi-nor: sfdp: parse xSPI Profile 1.0
table
Hi Tudor,
On 08/07/20 04:01PM, Tudor.Ambarus@...rochip.com wrote:
> On 6/23/20 9:30 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > This table is indication that the flash is xSPI compliant and hence
> > supports octal DTR mode. Extract information like the fast read opcode,
> > dummy cycles, the number of dummy cycles needed for a Read Status
> > Register command, and the number of address bytes needed for a Read
> > Status Register command.
> >
> > We don't know what speed the controller is running at. Find the fast
> > read dummy cycles for the fastest frequency the flash can run at to be
> > sure we are never short of dummy cycles. If nothing is available,
> > default to 20. Flashes that use a different value should update it in
> > their fixup hooks.
> >
> > Since we want to set read settings, expose spi_nor_set_read_settings()
> > in core.h.
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@...com>
> > ---
> > drivers/mtd/spi-nor/core.c | 2 +-
> > drivers/mtd/spi-nor/core.h | 10 ++++
> > drivers/mtd/spi-nor/sfdp.c | 98 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 109 insertions(+), 1 deletion(-)
> >
[...]
> > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> > index 3f709de5ea67..d5a24e61813c 100644
> > --- a/drivers/mtd/spi-nor/sfdp.c
> > +++ b/drivers/mtd/spi-nor/sfdp.c
[...]
> > @@ -66,6 +70,16 @@ struct sfdp_bfpt_erase {
> > u32 shift;
> > };
> >
> > +/* xSPI Profile 1.0 table (from JESD216D.01). */
> > +#define PROFILE1_DWORD1_RD_FAST_CMD GENMASK(15, 8)
> > +#define PROFILE1_DWORD1_RDSR_DUMMY BIT(28)
> > +#define PROFILE1_DWORD1_RDSR_ADDR_BYTES BIT(29)
> > +#define PROFILE1_DWORD4_DUMMY_200MHZ GENMASK(11, 7)
> > +#define PROFILE1_DWORD5_DUMMY_166MHZ GENMASK(31, 27)
> > +#define PROFILE1_DWORD5_DUMMY_133MHZ GENMASK(21, 17)
> > +#define PROFILE1_DWORD5_DUMMY_100MHZ GENMASK(11, 7)
>
> we should order these macros in a consistent way. I see that previous macros
> are declared in order starting from MSB to LSB.
>
> > +#define PROFILE1_DUMMY_DEFAULT 20
>
> we need to explain why the default dummy value is 20.
No reason other than the fact that it is the default for the first flash
that uses Profile 1.0 parsing (S28HS512T). AFAIK a similar reasoning is
followed for the default being 8 for 1-1-4 or 1-1-8 modes.
I can't think of any reasonable way of deciding on a default value since
it varies from flash to flash.
> How about declaring all these macros immediately above of spi_nor_parse_profile1()?
>
> > +
> > #define SMPT_CMD_ADDRESS_LEN_MASK GENMASK(23, 22)
> > #define SMPT_CMD_ADDRESS_LEN_0 (0x0UL << 22)
> > #define SMPT_CMD_ADDRESS_LEN_3 (0x1UL << 22)
> > @@ -1106,6 +1120,86 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
> > return ret;
> > }
> >
> > +/**
> > + * spi_nor_parse_profile1() - parse the xSPI Profile 1.0 table
> > + * @nor: pointer to a 'struct spi_nor'
> > + * @param_header: pointer to the 'struct sfdp_parameter_header' describing
> > + * the 4-Byte Address Instruction Table length and version.
> > + * @params: pointer to the 'struct spi_nor_flash_parameter' to be.
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
> > +static int spi_nor_parse_profile1(struct spi_nor *nor,
> > + const struct sfdp_parameter_header *profile1_header,
> > + struct spi_nor_flash_parameter *params)
> > +{
> > + u32 *table, opcode, addr;
>
> s/table/dwords?
>
> u8 opcode?
>
> > + size_t len;
> > + int ret, i;
> > + u8 dummy;
> > +
> > + len = profile1_header->length * sizeof(*table);
> > + table = kmalloc(len, GFP_KERNEL);
> > + if (!table)
> > + return -ENOMEM;
> > +
> > + addr = SFDP_PARAM_HEADER_PTP(profile1_header);
> > + ret = spi_nor_read_sfdp(nor, addr, len, table);
> > + if (ret)
> > + goto out;
> > +
> > + /* Fix endianness of the table DWORDs. */
> > + for (i = 0; i < profile1_header->length; i++)
> > + table[i] = le32_to_cpu(table[i]);
>
> le32_to_cpu_array(table, profile1_header->length);
>
> > +
> > + /* Get 8D-8D-8D fast read opcode and dummy cycles. */
> > + opcode = FIELD_GET(PROFILE1_DWORD1_RD_FAST_CMD, table[0]);
> > +
> > + /*
> > + * We don't know what speed the controller is running at. Find the
> > + * dummy cycles for the fastest frequency the flash can run at to be
> > + * sure we are never short of dummy cycles. A value of 0 means the
> > + * frequency is not supported.
> > + *
> > + * Default to PROFILE1_DUMMY_DEFAULT if we don't find anything, and let
> > + * flashes set the correct value if needed in their fixup hooks.
> > + */
> > + dummy = FIELD_GET(PROFILE1_DWORD4_DUMMY_200MHZ, table[3]);
> > + if (!dummy)
> > + dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_166MHZ, table[4]);
> > + if (!dummy)
> > + dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_133MHZ, table[4]);
> > + if (!dummy)
> > + dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_100MHZ, table[4]);
> > + if (!dummy)
> > + dummy = PROFILE1_DUMMY_DEFAULT;
> > +
> > + /* Round up to an even value to avoid tripping controllers up. */
> > + dummy = ROUND_UP_TO(dummy, 2);
> > +
[...]
--
Regards,
Pratyush Yadav
Powered by blists - more mailing lists