Re: [PATCH] mtd: spi-nor: core: Set mtd->eraseregions for non-uniform erase map

From: Michael Walle <michael@walle.cc>
To: Tudor Ambarus <tudor.ambarus@linaro.org>
Cc: tkuw584924@gmail.com, linux-mtd@lists.infradead.org,
	pratyush@kernel.org, miquel.raynal@bootlin.com, richard@nod.at,
	vigneshr@ti.com, Bacem.Daassi@infineon.com,
	Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Subject: Re: [PATCH] mtd: spi-nor: core: Set mtd->eraseregions for non-uniform erase map
Date: Fri, 12 Jan 2024 13:01:06 +0100	[thread overview]
Message-ID: <2e781c2efd663fea6d4a75fac36f144a@walle.cc> (raw)
In-Reply-To: <bb98a601-5c76-4273-820e-2b1c9b449820@linaro.org>

>> +	struct mtd_erase_region_info *mtd_region;
>> +	u32 erase_size;
>> +	u8 erase_mask;
> 
> put the u8 last to avoid stack padding

I don't think that is a thing. Even if it were, it might clash
with the RCT.

I couldn't find anything about how automatic variables are placed
in memory. I'd say its not specified in the standard and the compiler
is free to do optimizations here or just keep the contents in
registers (?!).

Any stytle recommendations for spi-nor? I prefer RCT, but if we want
to say declaration order doesn't matter, I'm fine with that too.

>> +
>> +	for (i = 0; !spi_nor_region_is_last(&region[i]); i++)
>> +		;
>> +
>> +	n_regions = i + 1;
> 
> all this just to get the number of regions? how about saving the number
> of regions somewhere and use it everywhere?

This whole region thing should be rewritten to not store these magic
bits in the offset.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/