Messages in this thread Patch in this message | | | Date | Tue, 02 Feb 2021 20:08:43 +0000 | From | "John Thomson" <> | Subject | Re: [RFC PATCH] mtd: spi-nor: write support for minor aligned partitions |
| |
On Mon, 4 Jan 2021, at 12:28, John Thomson wrote: > --- a/drivers/mtd/mtdpart.c > +++ b/drivers/mtd/mtdpart.c > @@ -38,10 +38,11 @@ static struct mtd_info *allocate_partition(struct > mtd_info *parent, > struct mtd_info *master = mtd_get_master(parent); > int wr_alignment = (parent->flags & MTD_NO_ERASE) ? > master->writesize : master->erasesize; > + int wr_alignment_minor;
int wr_alignment_minor = 0;
> + if (!(child->flags & MTD_NO_ERASE)) { > wr_alignment = child->erasesize; > + if (child->erasesize_minor)
if (child->erasesize_minor && IS_ENABLED(CONFIG_MTD_SPI_NOR_USE_VARIABLE_ERASE)) { Config test wrap wr_alignment_minor being set, so that a partition on a minor boundary is only set writeable if the non-uniform erase path can be used.
> + wr_alignment_minor = child->erasesize_minor; > + }
> + if (wr_alignment_minor) {
smatch picked up a tested uninitialized symbol 'wr_alignment_minor' here, initialise as 0. Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> + if ((!wr_alignment_minor) || (wr_alignment_minor && remainder_minor != 0)) {
If it is safe to boolean test the ints and u32, I should use this consistently? if ((!wr_alignment_minor) || (wr_alignment_minor && remainder_minor)) { Or is it clearer to use the math tests? if ((wr_alignment_minor == 0) || (wr_alignment_minor > 0 && remainder_minor > 0)) {
> --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -1225,7 +1225,11 @@ static u8 spi_nor_convert_3to4_erase(u8 opcode) > > static bool spi_nor_has_uniform_erase(const struct spi_nor *nor) > { > +#ifdef CONFIG_MTD_SPI_NOR_USE_VARIABLE_ERASE
if (IS_ENABLED(CONFIG_MTD_SPI_NOR_USE_VARIABLE_ERASE)) { and the closing brace better than the #ifdef here?
> + return false; > +#else > return !!nor->params->erase_map.uniform_erase_type; > +#endif > } > > static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
Otherwise, is this approach valid, or is there a better method I can use?
Cheers, -- John Thomson
| |