From: Michael Walle <michael@walle.cc>
To: Tudor.Ambarus@microchip.com
Cc: linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
p.yadav@ti.com, miquel.raynal@bootlin.com, richard@nod.at,
vigneshr@ti.com
Subject: Re: [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data
Date: Mon, 05 Apr 2021 17:07:42 +0200 [thread overview]
Message-ID: <5359aa6ade80ff6d39e969c3be2957dd@walle.cc> (raw)
In-Reply-To: <e28e7cd3-9728-3428-1bae-9cda258174bc@microchip.com>
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 = ¶m_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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2021-04-05 15:08 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-18 9:24 [PATCH 0/2] mtd: spi-nor: support dumping sfdp tables Michael Walle
2021-03-18 9:24 ` [PATCH 1/2] mtd: spi-nor: sfdp: save a copy of the SFDP data Michael Walle
2021-03-22 14:21 ` Pratyush Yadav
2021-03-22 15:32 ` Michael Walle
2021-03-22 15:48 ` Michael Walle
2021-03-22 18:42 ` Pratyush Yadav
2021-03-22 22:31 ` Michael Walle
2021-03-23 9:37 ` Pratyush Yadav
2021-04-05 13:11 ` Tudor.Ambarus
2021-04-05 15:07 ` Michael Walle [this message]
2021-04-05 15:42 ` Tudor.Ambarus
2021-04-05 16:03 ` Michael Walle
2021-03-18 9:24 ` [PATCH 2/2] mtd: spi-nor: add initial sysfs support Michael Walle
2021-03-20 4:16 ` Yicong Yang
2021-03-20 19:17 ` Michael Walle
2021-03-22 14:43 ` Pratyush Yadav
2021-03-22 14:57 ` Michael Walle
2021-04-06 7:56 ` Vignesh Raghavendra
2021-04-06 8:47 ` Michael Walle
2021-04-06 11:43 ` Vignesh Raghavendra
2021-04-29 15:37 ` Alexander Williams
2021-04-29 15:46 ` Michael Walle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5359aa6ade80ff6d39e969c3be2957dd@walle.cc \
--to=michael@walle.cc \
--cc=Tudor.Ambarus@microchip.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=miquel.raynal@bootlin.com \
--cc=p.yadav@ti.com \
--cc=richard@nod.at \
--cc=vigneshr@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).