| 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: <80641a1c-d922-22cc-edd4-76d475e1aac8@linaro.org>
Date: Tue, 19 Sep 2023 12:24:07 +0300
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Michael Walle <mwalle@...nel.org>,
Pratyush Yadav <pratyush@...nel.org>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Richard Weinberger <richard@....at>,
Vignesh Raghavendra <vigneshr@...com>
Cc: linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org
Subject: Re: [PATCH v3 05/41] mtd: spi-nor: convert .n_sectors to .size
On 08.09.2023 13:16, Michael Walle wrote:
> .n_sectors is rarely used. In fact it is only used in swp.c and to
> calculate the flash size in the core. The use in swp.c might be
> converted to use the (largest) flash erase size. For now, we just
> locally calculate the sector size.
>
> Simplify the flash_info database and set the size of the flash directly.
> This also let us use the SZ_x macros.
>
> Signed-off-by: Michael Walle <mwalle@...nel.org>
> ---
> drivers/mtd/spi-nor/core.c | 2 +-
> drivers/mtd/spi-nor/core.h | 8 ++++----
> drivers/mtd/spi-nor/swp.c | 9 +++++----
> drivers/mtd/spi-nor/xilinx.c | 4 ++--
> 4 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 286155002cdc..f4cc2eafcc5e 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2999,7 +2999,7 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
>
> /* Set SPI NOR sizes. */
> params->writesize = 1;
> - params->size = (u64)info->sector_size * info->n_sectors;
> + params->size = info->size;
> params->bank_size = params->size;
> params->page_size = info->page_size;
>
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index dfc20a3296fb..12c35409493b 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -443,9 +443,9 @@ struct spi_nor_fixups {
> * @id: the flash's ID bytes. The first three bytes are the
> * JEDIC ID. JEDEC ID zero means "no ID" (mostly older chips).
> * @id_len: the number of bytes of ID.
> + * @size: the size of the flash in bytes.
> * @sector_size: the size listed here is what works with SPINOR_OP_SE, which
> * isn't necessarily called a "sector" by the vendor.
> - * @n_sectors: the number of sectors.
> * @n_banks: the number of banks.
> * @page_size: the flash's page size.
> * @addr_nbytes: number of address bytes to send.
> @@ -505,8 +505,8 @@ struct flash_info {
> char *name;
> u8 id[SPI_NOR_MAX_ID_LEN];
> u8 id_len;
> + size_t size;
> unsigned sector_size;
> - u16 n_sectors;
> u16 page_size;
> u8 n_banks;
> u8 addr_nbytes;
> @@ -556,8 +556,8 @@ struct flash_info {
> .id_len = 6
>
> #define SPI_NOR_GEOMETRY(_sector_size, _n_sectors, _n_banks) \
> + .size = (_sector_size) * (_n_sectors), \
> .sector_size = (_sector_size), \
> - .n_sectors = (_n_sectors), \
> .page_size = 256, \
> .n_banks = (_n_banks)
>
> @@ -575,8 +575,8 @@ struct flash_info {
> SPI_NOR_GEOMETRY((_sector_size), (_n_sectors), 1),
>
> #define CAT25_INFO(_sector_size, _n_sectors, _page_size, _addr_nbytes) \
> + .size = (_sector_size) * (_n_sectors), \
> .sector_size = (_sector_size), \
> - .n_sectors = (_n_sectors), \
> .page_size = (_page_size), \
> .n_banks = 1, \
> .addr_nbytes = (_addr_nbytes), \
> diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c
> index 5ab9d5324860..40bf52867095 100644
> --- a/drivers/mtd/spi-nor/swp.c
> +++ b/drivers/mtd/spi-nor/swp.c
> @@ -34,17 +34,18 @@ static u8 spi_nor_get_sr_tb_mask(struct spi_nor *nor)
> static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor)
> {
> unsigned int bp_slots, bp_slots_needed;
> + unsigned int sector_size = nor->info->sector_size;
this should use nor->params instead, but we can do it later on. New
flash additions must specify the sector size, otherwise we get a
division by zero below. I see you introduce a default value in patch 10,
which will guard us. I assume you checked that there's no flash that
specifies BP and sector size of zero up to patch 9 inclusively,
otherwise we'll break bisect-ability. Let me know if yes, and I'll
update the commit message when applying.
> + u64 n_sectors = div_u64(nor->params->size, sector_size);
> u8 mask = spi_nor_get_sr_bp_mask(nor);
>
> /* Reserved one for "protect none" and one for "protect all". */
> bp_slots = (1 << hweight8(mask)) - 2;
> - bp_slots_needed = ilog2(nor->info->n_sectors);
> + bp_slots_needed = ilog2(n_sectors);
>
> if (bp_slots_needed > bp_slots)
> - return nor->info->sector_size <<
> - (bp_slots_needed - bp_slots);
> + return sector_size << (bp_slots_needed - bp_slots);
> else
> - return nor->info->sector_size;
> + return sector_size;
> }
>
> static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs,
> diff --git a/drivers/mtd/spi-nor/xilinx.c b/drivers/mtd/spi-nor/xilinx.c
> index 34267591282c..284e2e4970ab 100644
> --- a/drivers/mtd/spi-nor/xilinx.c
> +++ b/drivers/mtd/spi-nor/xilinx.c
> @@ -23,8 +23,8 @@
>
> #define S3AN_INFO(_jedec_id, _n_sectors, _page_size) \
> SPI_NOR_ID(_jedec_id, 0), \
> + .size = 8 * (_page_size) * (_n_sectors), \
> .sector_size = (8 * (_page_size)), \
> - .n_sectors = (_n_sectors), \
> .page_size = (_page_size), \
> .n_banks = 1, \
> .flags = SPI_NOR_NO_FR
> @@ -138,7 +138,7 @@ static int xilinx_nor_setup(struct spi_nor *nor,
> page_size = (nor->params->page_size == 264) ? 256 : 512;
> nor->params->page_size = page_size;
> nor->mtd.writebufsize = page_size;
> - nor->params->size = 8 * page_size * nor->info->n_sectors;
> + nor->params->size = nor->info->size;
> nor->mtd.erasesize = 8 * page_size;
> } else {
> /* Flash in Default addressing mode */
>
Powered by blists - more mailing lists