The INFO() macro always set the page_size to 256 bytes. Make that an
optinal parameter. This default is a sane one for all older flashes,
newer ones will set the page size by its SFDP tables anyway.
Signed-off-by: Michael Walle <[email protected]>
---
drivers/mtd/spi-nor/core.c | 7 +------
drivers/mtd/spi-nor/core.h | 8 ++++++--
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index c504a5af4032..138bc1e0a67c 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2017,11 +2017,6 @@ static const struct spi_nor_manufacturer *manufacturers[] = {
static const struct flash_info spi_nor_generic_flash = {
.name = "spi-nor-generic",
.n_banks = 1,
- /*
- * JESD216 rev A doesn't specify the page size, therefore we need a
- * sane default.
- */
- .page_size = 256,
.parse_sfdp = true,
};
@@ -3000,7 +2995,7 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
params->writesize = 1;
params->size = info->size;
params->bank_size = params->size;
- params->page_size = info->page_size;
+ params->page_size = info->page_size ?: SPI_NOR_DEFAULT_PAGE_SIZE;
if (!(info->flags & SPI_NOR_NO_FR)) {
/* Default to Fast Read for DT and non-DT platform devices. */
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 12c35409493b..25bc18197614 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -10,6 +10,11 @@
#include "sfdp.h"
#define SPI_NOR_MAX_ID_LEN 6
+/*
+ * 256 bytes is a sane default for most older flashes. Newer flashes will
+ * have the page size defined within their SFDP tables.
+ */
+#define SPI_NOR_DEFAULT_PAGE_SIZE 256
/* Standard SPI NOR flash operations. */
#define SPI_NOR_READID_OP(naddr, ndummy, buf, len) \
@@ -447,7 +452,7 @@ struct spi_nor_fixups {
* @sector_size: the size listed here is what works with SPINOR_OP_SE, which
* isn't necessarily called a "sector" by the vendor.
* @n_banks: the number of banks.
- * @page_size: the flash's page size.
+ * @page_size: (optional) the flash's page size. Defaults to 256.
* @addr_nbytes: number of address bytes to send.
*
* @parse_sfdp: true when flash supports SFDP tables. The false value has no
@@ -558,7 +563,6 @@ struct flash_info {
#define SPI_NOR_GEOMETRY(_sector_size, _n_sectors, _n_banks) \
.size = (_sector_size) * (_n_sectors), \
.sector_size = (_sector_size), \
- .page_size = 256, \
.n_banks = (_n_banks)
/* Used when the "_ext_id" is two bytes at most */
--
2.39.2
Hi Michael,
[email protected] wrote on Mon, 07 Aug 2023 15:21:00 +0200:
> The INFO() macro always set the page_size to 256 bytes. Make that an
> optinal parameter. This default is a sane one for all older flashes,
optional
> newer ones will set the page size by its SFDP tables anyway.
>
> Signed-off-by: Michael Walle <[email protected]>
> ---
> drivers/mtd/spi-nor/core.c | 7 +------
> drivers/mtd/spi-nor/core.h | 8 ++++++--
> 2 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index c504a5af4032..138bc1e0a67c 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2017,11 +2017,6 @@ static const struct spi_nor_manufacturer *manufacturers[] = {
> static const struct flash_info spi_nor_generic_flash = {
> .name = "spi-nor-generic",
> .n_banks = 1,
> - /*
> - * JESD216 rev A doesn't specify the page size, therefore we need a
> - * sane default.
> - */
> - .page_size = 256,
> .parse_sfdp = true,
> };
>
> @@ -3000,7 +2995,7 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
> params->writesize = 1;
> params->size = info->size;
> params->bank_size = params->size;
> - params->page_size = info->page_size;
> + params->page_size = info->page_size ?: SPI_NOR_DEFAULT_PAGE_SIZE;
Would you mind clarifying this? It does not look right, while
perhaps it is. But TBH, I have no idea what params->page_size will be
after this assignment.
>
> if (!(info->flags & SPI_NOR_NO_FR)) {
> /* Default to Fast Read for DT and non-DT platform devices. */
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 12c35409493b..25bc18197614 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -10,6 +10,11 @@
> #include "sfdp.h"
>
> #define SPI_NOR_MAX_ID_LEN 6
> +/*
> + * 256 bytes is a sane default for most older flashes. Newer flashes will
> + * have the page size defined within their SFDP tables.
> + */
> +#define SPI_NOR_DEFAULT_PAGE_SIZE 256
>
> /* Standard SPI NOR flash operations. */
> #define SPI_NOR_READID_OP(naddr, ndummy, buf, len) \
> @@ -447,7 +452,7 @@ struct spi_nor_fixups {
> * @sector_size: the size listed here is what works with SPINOR_OP_SE, which
> * isn't necessarily called a "sector" by the vendor.
> * @n_banks: the number of banks.
> - * @page_size: the flash's page size.
> + * @page_size: (optional) the flash's page size. Defaults to 256.
> * @addr_nbytes: number of address bytes to send.
> *
> * @parse_sfdp: true when flash supports SFDP tables. The false value has no
> @@ -558,7 +563,6 @@ struct flash_info {
> #define SPI_NOR_GEOMETRY(_sector_size, _n_sectors, _n_banks) \
> .size = (_sector_size) * (_n_sectors), \
> .sector_size = (_sector_size), \
> - .page_size = 256, \
> .n_banks = (_n_banks)
>
> /* Used when the "_ext_id" is two bytes at most */
>
Otherwise, lgtm.
Reviewed-by: Miquel Raynal <[email protected]>
Thanks,
Miquèl
Hi Miquel,
>> + params->page_size = info->page_size ?: SPI_NOR_DEFAULT_PAGE_SIZE;
>
> Would you mind clarifying this? It does not look right, while
> perhaps it is. But TBH, I have no idea what params->page_size will be
> after this assignment.
It's info->page_size, it's a shortcut [1].
I'm not the first one using it (in fact I was explicitly asked to use
it,
e.g. in gpio subsys):
$ grep -r "?:" **/*.c |wc -l
1268
-michael
[1] https://en.wikipedia.org/wiki/Elvis_operator
Hi Michael,
[email protected] wrote on Mon, 07 Aug 2023 16:29:38 +0200:
> Hi Miquel,
>
> >> + params->page_size = info->page_size ?: SPI_NOR_DEFAULT_PAGE_SIZE;
> >
> > Would you mind clarifying this? It does not look right, while
> > perhaps it is. But TBH, I have no idea what params->page_size will be
> > after this assignment.
>
> It's info->page_size, it's a shortcut [1].
Elvis operator :')
> I'm not the first one using it (in fact I was explicitly asked to use it,
> e.g. in gpio subsys):
>
> $ grep -r "?:" **/*.c |wc -l
> 1268
Okay then... I didn't know this was encouraged.
> -michael
>
> [1] https://en.wikipedia.org/wiki/Elvis_operator
Thanks,
Miquèl