linux-kernel - Re: [PATCH v3 10/41] mtd: spi-nor: make sector

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4ec7192b6bfaf956e1b8e747e5fd07e7@kernel.org>
Date:   Tue, 19 Sep 2023 13:31:10 +0200
From:   Michael Walle <mwalle@...nel.org>
To:     Tudor Ambarus <tudor.ambarus@...aro.org>
Cc:     Pratyush Yadav <pratyush@...nel.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org
Subject: Re: [PATCH v3 10/41] mtd: spi-nor: make sector_size optional

Am 2023-09-19 11:25, schrieb Tudor Ambarus:
> On 08.09.2023 13:16, Michael Walle wrote:
>> Most of the (old, non-SFDP) flashes use a sector size of 64k. Make 
>> that
>> a default value so it can be optional in the flash_info database.
>> 
>> As a preparation for conversion to the new database format, set the
>> sector size to zero if the default value is used. This way, the actual
>> change is happening with this patch ant not with a later conversion
>> patch.
>> 
>> Signed-off-by: Michael Walle <mwalle@...nel.org>
>> ---
>>  drivers/mtd/spi-nor/core.c | 6 ++++--
>>  drivers/mtd/spi-nor/core.h | 8 +++++---
>>  drivers/mtd/spi-nor/swp.c  | 6 +++++-
>>  3 files changed, 14 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index c84be791341e..368851ff9f40 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -2756,7 +2756,8 @@ static void spi_nor_no_sfdp_init_params(struct 
>> spi_nor *nor)
>>  {
>>  	struct spi_nor_flash_parameter *params = nor->params;
>>  	struct spi_nor_erase_map *map = &params->erase_map;
>> -	const u8 no_sfdp_flags = nor->info->no_sfdp_flags;
>> +	const struct flash_info *info = nor->info;
>> +	const u8 no_sfdp_flags = info->no_sfdp_flags;
>>  	u8 i, erase_mask;
>> 
>>  	if (no_sfdp_flags & SPI_NOR_DUAL_READ) {
>> @@ -2810,7 +2811,8 @@ static void spi_nor_no_sfdp_init_params(struct 
>> spi_nor *nor)
>>  		i++;
>>  	}
>>  	erase_mask |= BIT(i);
>> -	spi_nor_set_erase_type(&map->erase_type[i], nor->info->sector_size,
>> +	spi_nor_set_erase_type(&map->erase_type[i],
>> +			       info->sector_size ?: SPI_NOR_DEFAULT_SECTOR_SIZE,
>>  			       SPINOR_OP_SE);
> 
> all these info->sector_size checks can be removed if we use
> params->sector_size. I'll do it after applying the series.

See previous reply, there will be just this check. The one in swp.c
will go away. I don't have a strong opinion. I just didn't want to
have redundant information.

Speaking of.. there is a check missing in

--snip--
diff --git a/drivers/mtd/spi-nor/spansion.c 
b/drivers/mtd/spi-nor/spansion.c
index fd2652aa6c1e..e4c725000964 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -1053,7 +1053,8 @@ static int spansion_nor_late_init(struct spi_nor 
*nor)
  		nor->flags |= SNOR_F_4B_OPCODES;
  		/* No small sector erase for 4-byte command set */
  		nor->erase_opcode = SPINOR_OP_SE;
-		nor->mtd.erasesize = nor->info->sector_size;
+		nor->mtd.erasesize = nor->info->sector_size ?:
+				     SPI_NOR_DEFAULT_SECTOR_SIZE;
  	}

  	if (mfr_flags & (USE_CLSR | USE_CLPEF)) {
--snip--

Is it possible to amend this patch while you apply it? A later patch
should probably fixup the erase_map instead of setting the erase_opcode
and erasesize itself.

-michael