lkml.org 
[lkml]   [2021]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Hi,

Am 2021-04-05 15:11, schrieb Tudor.Ambarus@microchip.com:
> On 3/18/21 11:24 AM, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> Due to possible mode switching to 8D-8D-8D, it might not be possible
>> to
>> read the SFDP after the initial probe. To be able to dump the SFDP via
>> sysfs afterwards, make a complete copy of it.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> drivers/mtd/spi-nor/core.h | 10 ++++++++
>> drivers/mtd/spi-nor/sfdp.c | 49
>> +++++++++++++++++++++++++++++++++++++
>> include/linux/mtd/spi-nor.h | 3 +++
>> 3 files changed, 62 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index 4a3f7f150b5d..668f22011b1d 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -407,6 +407,16 @@ struct spi_nor_manufacturer {
>> const struct spi_nor_fixups *fixups;
>> };
>>
>> +/**
>> + * struct sfdp - SFDP data
>> + * @num_dwords: number of entries in the dwords array
>> + * @dwords: array of double words of the SFDP data
>> + */
>> +struct sfdp {
>> + size_t num_dwords;
>> + u32 *dwords;
>> +};
>> +
>> /* Manufacturer drivers. */
>> extern const struct spi_nor_manufacturer spi_nor_atmel;
>> extern const struct spi_nor_manufacturer spi_nor_catalyst;
>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>> index 25142ec4737b..2b6c96e02532 100644
>> --- a/drivers/mtd/spi-nor/sfdp.c
>> +++ b/drivers/mtd/spi-nor/sfdp.c
>> @@ -16,6 +16,7 @@
>> (((p)->parameter_table_pointer[2] << 16) | \
>> ((p)->parameter_table_pointer[1] << 8) | \
>> ((p)->parameter_table_pointer[0] << 0))
>> +#define SFDP_PARAM_HEADER_PARAM_LEN(p) ((p)->length * 4)
>>
>> #define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table
>> */
>> #define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
>> @@ -1263,6 +1264,8 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
>> struct sfdp_parameter_header *param_headers = NULL;
>> struct sfdp_header header;
>> struct device *dev = nor->dev;
>> + struct sfdp *sfdp;
>> + size_t sfdp_size;
>> size_t psize;
>> int i, err;
>>
>> @@ -1285,6 +1288,9 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
>> bfpt_header->major != SFDP_JESD216_MAJOR)
>> return -EINVAL;
>>
>> + sfdp_size = SFDP_PARAM_HEADER_PTP(bfpt_header) +
>> + SFDP_PARAM_HEADER_PARAM_LEN(bfpt_header);
>> +
>> /*
>> * Allocate memory then read all parameter headers with a
>> single
>> * Read SFDP command. These parameter headers will actually be
>> parsed
>> @@ -1311,6 +1317,49 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
>> }
>> }
>>
>> + /*
>> + * Cache the complete SFDP data. It is not (easily) possible
>> to fetch
>> + * SFDP after probe time and we need it for the sysfs access.
>> + */
>> + for (i = 0; i < header.nph; i++) {
>> + param_header = &param_headers[i];
>> + sfdp_size = max_t(size_t, sfdp_size,
>> + SFDP_PARAM_HEADER_PTP(param_header)
>> +
>> +
>> SFDP_PARAM_HEADER_PARAM_LEN(param_header));
>> + }
>
> Michael, I like the idea of saving the SFDP data, but I think this can
> be
> improved a little. For example, it is not mandatory for the tables to
> be
> continuous in memory, there can be some gaps between BFPT and SMPT for
> example,
> thus we can improve the memory allocation logic.

I want to parse the SFDP as little as possible. Keep in mind, that this
should
help to debug SFDP (errors). Therefore, I don't want to rely on the SFDP
saying
"hey there is a hole, please skip it". Who knows if there is some useful
data?

> Also, we can make the saved sfdp
> data table-agnostic so that we don't duplicate the reads in
> parse_bfpt/smpt/4bait.

This falls into the same category as above. While it might be reused,
the
primary use case is to have the SFDP data available to a developer/user.
Eg.
what will you do with some holes in the sysfs read()? Return zeros?

FWIW I'm planning to refactor the read code so the cached values are
used.

-michael

\
 
 \ /
  Last update: 2021-04-05 17:08    [W:0.178 / U:1.432 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site