From: <Tudor.Ambarus@microchip.com> To: <michael@walle.cc>, <p.yadav@ti.com>, <miquel.raynal@bootlin.com>, <richard@nod.at>, <vigneshr@ti.com>, <quic_c_sbhanu@quicinc.com> Cc: <linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/2] mtd: spi-nor: introduce SNOR_ID3() Date: Thu, 28 Jul 2022 03:24:36 +0000 [thread overview] Message-ID: <41e6b74d-6973-60da-1556-82a1e879ffd4@microchip.com> (raw) In-Reply-To: <160ddac1-6d0a-c922-1380-09e7823a285e@microchip.com> On 7/19/22 08:57, Tudor.Ambarus@microchip.com wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 5/10/22 17:02, Michael Walle wrote: > > Hi! > >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> Up until now, flashes were defined by specifying the JEDEC ID, the >> sector size and the number of sectors. This can be read by parsing the >> SFDP, we don't have to specify it. Thus provide a new macro SNOR_ID3() >> which just takes the JEDEC ID and implicitly set .parse_sfdp = true. All >> new flashes which have SFDP should use this macro. > > I like the idea, but you need to refine it a bit. > Your assumptions are true only for flashes that are compliant with SFDP revB or > later because params->page_size is initialized by querying BFPT DWORD 11. I think > it would be good to specify this in the comment section. Also, I think you introduce > a bug in spi_nor_select_erase() when CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not > selected. wanted_size will be zero, will you select an invalid erase type? > Would you please resubmit? > > Thanks, > ta > >> >> Signed-off-by: Michael Walle <michael@walle.cc> >> --- >> drivers/mtd/spi-nor/core.c | 7 +++++-- >> drivers/mtd/spi-nor/core.h | 9 +++++++++ >> 2 files changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c >> index 402b37cdbcea..29329ed0a934 100644 >> --- a/drivers/mtd/spi-nor/core.c >> +++ b/drivers/mtd/spi-nor/core.c >> @@ -2104,8 +2104,11 @@ static int spi_nor_select_pp(struct spi_nor *nor, >> * spi_nor_select_uniform_erase() - select optimum uniform erase type >> * @map: the erase map of the SPI NOR >> * @wanted_size: the erase type size to search for. Contains the value of >> - * info->sector_size or of the "small sector" size in case >> - * CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is defined. >> + * info->sector_size, the "small sector" size in case >> + * CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is defined or 0 if >> + * there is no information about the sector size. The >> + * latter is the case if the flash parameters are parsed >> + * solely by SFDP. >> * >> * Once the optimum uniform sector erase command is found, disable all the >> * other. >> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h >> index 61886868cd02..fab3038c4f4a 100644 >> --- a/drivers/mtd/spi-nor/core.h >> +++ b/drivers/mtd/spi-nor/core.h >> @@ -563,6 +563,15 @@ struct flash_info { >> .n_regions = (_n_regions), \ >> }, >> >> +#define SNOR_ID3(_jedec_id) How about SFDP_ID3 and SFDP_ID6 instead? \ >> + .id = { \ >> + ((_jedec_id) >> 16) & 0xff, \ >> + ((_jedec_id) >> 8) & 0xff, \ >> + (_jedec_id) & 0xff, \ >> + }, \ >> + .id_len = 3, \ >> + .parse_sfdp = true, \ >> + >> #define PARSE_SFDP \ >> .parse_sfdp = true, \ >> >> -- >> 2.30.2 >> > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ -- Cheers, ta
WARNING: multiple messages have this Message-ID (diff)
From: <Tudor.Ambarus@microchip.com> To: <michael@walle.cc>, <p.yadav@ti.com>, <miquel.raynal@bootlin.com>, <richard@nod.at>, <vigneshr@ti.com>, <quic_c_sbhanu@quicinc.com> Cc: <linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/2] mtd: spi-nor: introduce SNOR_ID3() Date: Thu, 28 Jul 2022 03:24:36 +0000 [thread overview] Message-ID: <41e6b74d-6973-60da-1556-82a1e879ffd4@microchip.com> (raw) In-Reply-To: <160ddac1-6d0a-c922-1380-09e7823a285e@microchip.com> On 7/19/22 08:57, Tudor.Ambarus@microchip.com wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 5/10/22 17:02, Michael Walle wrote: > > Hi! > >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> Up until now, flashes were defined by specifying the JEDEC ID, the >> sector size and the number of sectors. This can be read by parsing the >> SFDP, we don't have to specify it. Thus provide a new macro SNOR_ID3() >> which just takes the JEDEC ID and implicitly set .parse_sfdp = true. All >> new flashes which have SFDP should use this macro. > > I like the idea, but you need to refine it a bit. > Your assumptions are true only for flashes that are compliant with SFDP revB or > later because params->page_size is initialized by querying BFPT DWORD 11. I think > it would be good to specify this in the comment section. Also, I think you introduce > a bug in spi_nor_select_erase() when CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not > selected. wanted_size will be zero, will you select an invalid erase type? > Would you please resubmit? > > Thanks, > ta > >> >> Signed-off-by: Michael Walle <michael@walle.cc> >> --- >> drivers/mtd/spi-nor/core.c | 7 +++++-- >> drivers/mtd/spi-nor/core.h | 9 +++++++++ >> 2 files changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c >> index 402b37cdbcea..29329ed0a934 100644 >> --- a/drivers/mtd/spi-nor/core.c >> +++ b/drivers/mtd/spi-nor/core.c >> @@ -2104,8 +2104,11 @@ static int spi_nor_select_pp(struct spi_nor *nor, >> * spi_nor_select_uniform_erase() - select optimum uniform erase type >> * @map: the erase map of the SPI NOR >> * @wanted_size: the erase type size to search for. Contains the value of >> - * info->sector_size or of the "small sector" size in case >> - * CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is defined. >> + * info->sector_size, the "small sector" size in case >> + * CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is defined or 0 if >> + * there is no information about the sector size. The >> + * latter is the case if the flash parameters are parsed >> + * solely by SFDP. >> * >> * Once the optimum uniform sector erase command is found, disable all the >> * other. >> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h >> index 61886868cd02..fab3038c4f4a 100644 >> --- a/drivers/mtd/spi-nor/core.h >> +++ b/drivers/mtd/spi-nor/core.h >> @@ -563,6 +563,15 @@ struct flash_info { >> .n_regions = (_n_regions), \ >> }, >> >> +#define SNOR_ID3(_jedec_id) How about SFDP_ID3 and SFDP_ID6 instead? \ >> + .id = { \ >> + ((_jedec_id) >> 16) & 0xff, \ >> + ((_jedec_id) >> 8) & 0xff, \ >> + (_jedec_id) & 0xff, \ >> + }, \ >> + .id_len = 3, \ >> + .parse_sfdp = true, \ >> + >> #define PARSE_SFDP \ >> .parse_sfdp = true, \ >> >> -- >> 2.30.2 >> > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ -- Cheers, ta ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2022-07-28 3:24 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-10 14:02 [PATCH 0/2] introduce SNOR_ID3() Michael Walle 2022-05-10 14:02 ` Michael Walle 2022-05-10 14:02 ` [PATCH 1/2] mtd: spi-nor: " Michael Walle 2022-05-10 14:02 ` Michael Walle 2022-06-05 15:00 ` Tom Fitzhenry 2022-06-05 15:00 ` Tom Fitzhenry 2022-07-12 7:23 ` Pratyush Yadav 2022-07-15 12:19 ` Biju Das 2022-07-15 12:19 ` Biju Das 2022-07-12 7:23 ` Pratyush Yadav 2022-07-19 5:57 ` Tudor.Ambarus 2022-07-19 5:57 ` Tudor.Ambarus 2022-07-19 7:07 ` Michael Walle 2022-07-19 7:07 ` Michael Walle 2022-07-19 7:33 ` Tudor.Ambarus 2022-07-19 7:33 ` Tudor.Ambarus 2022-07-19 7:57 ` Michael Walle 2022-07-19 7:57 ` Michael Walle 2022-07-19 8:30 ` Tudor.Ambarus 2022-07-19 8:30 ` Tudor.Ambarus 2022-07-28 3:24 ` Tudor.Ambarus [this message] 2022-07-28 3:24 ` Tudor.Ambarus 2022-07-28 13:12 ` Michael Walle 2022-07-28 13:12 ` Michael Walle 2022-07-28 13:31 ` Tudor.Ambarus 2022-07-28 13:31 ` Tudor.Ambarus 2022-07-28 13:56 ` Michael Walle 2022-07-28 13:56 ` Michael Walle 2022-05-10 14:02 ` [PATCH 2/2] mtd: spi-nor: winbond: use SNOR_ID3() for w25q512nwm Michael Walle 2022-05-10 14:02 ` Michael Walle 2022-07-12 8:40 ` Tudor.Ambarus 2022-07-12 8:40 ` Tudor.Ambarus 2022-07-18 7:21 ` Michael Walle 2022-07-18 7:21 ` Michael Walle 2022-07-18 7:25 ` Michael Walle 2022-07-18 7:25 ` Michael Walle 2022-07-19 6:00 ` Tudor.Ambarus 2022-07-19 6:00 ` Tudor.Ambarus 2022-07-19 7:02 ` Michael Walle 2022-07-19 7:02 ` Michael Walle 2022-07-19 7:24 ` Tudor.Ambarus 2022-07-19 7:24 ` Tudor.Ambarus 2022-05-10 14:03 ` [PATCH 0/2] mtd: spi-nor: introduce SNOR_ID3() Michael Walle 2022-05-10 14:03 ` 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=41e6b74d-6973-60da-1556-82a1e879ffd4@microchip.com \ --to=tudor.ambarus@microchip.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ --cc=michael@walle.cc \ --cc=miquel.raynal@bootlin.com \ --cc=p.yadav@ti.com \ --cc=quic_c_sbhanu@quicinc.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.