| Message ID | 20211122095020.393346-2-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtd: spi-nor: Clean params init | expand |
Am 2021-11-22 10:50, schrieb Tudor Ambarus: > Use NOR parameters in the probe's sequence of calls, thus > nor->params->size instead of nor->mtd.size and let the mtd_info > fields be used by the mtd calls (mtd->_erase, mtd->_read, mtd->_write). > mtd_info fields should not be used during probe because we haven't > registered mtd yet. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Reviewed-by: Michael Walle <michael@walle.cc> -michael
On 22/11/21 11:50AM, Tudor Ambarus wrote: > Use NOR parameters in the probe's sequence of calls, thus > nor->params->size instead of nor->mtd.size and let the mtd_info > fields be used by the mtd calls (mtd->_erase, mtd->_read, mtd->_write). > mtd_info fields should not be used during probe because we haven't > registered mtd yet. These changes look good to me, but there is one thing this seems to be missing. In xlinx.c, I see that xilinx_setup() sets nor->mtd.size. I think it should be updated to set nor->params->size instead otherwise functions like spi_nor_set_addr_width() might end up using the wrong value. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > v4: new patch > > drivers/mtd/spi-nor/core.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 88dd0908d172..5b9c827d411c 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -2115,7 +2115,7 @@ static int spi_nor_spimem_check_op(struct spi_nor *nor, > */ > op->addr.nbytes = 4; > if (!spi_mem_supports_op(nor->spimem, op)) { > - if (nor->mtd.size > SZ_16M) > + if (nor->params->size > SZ_16M) > return -EOPNOTSUPP; > > /* If flash size <= 16MB, 3 address bytes are sufficient */ > @@ -3011,7 +3011,7 @@ static int spi_nor_set_addr_width(struct spi_nor *nor) > nor->addr_width = 3; > } > > - if (nor->addr_width == 3 && nor->mtd.size > 0x1000000) { > + if (nor->addr_width == 3 && nor->params->size > 0x1000000) { > /* enable 4-byte addressing if the device exceeds 16MiB */ > nor->addr_width = 4; > } > @@ -3245,7 +3245,7 @@ static int spi_nor_create_read_dirmap(struct spi_nor *nor) > SPI_MEM_OP_DUMMY(nor->read_dummy, 0), > SPI_MEM_OP_DATA_IN(0, NULL, 0)), > .offset = 0, > - .length = nor->mtd.size, > + .length = nor->params->size, > }; > struct spi_mem_op *op = &info.op_tmpl; > > @@ -3276,7 +3276,7 @@ static int spi_nor_create_write_dirmap(struct spi_nor *nor) > SPI_MEM_OP_NO_DUMMY, > SPI_MEM_OP_DATA_OUT(0, NULL, 0)), > .offset = 0, > - .length = nor->mtd.size, > + .length = nor->params->size, > }; > struct spi_mem_op *op = &info.op_tmpl; > > -- > 2.25.1 >
On 11/30/21 11:56 AM, Pratyush Yadav wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 22/11/21 11:50AM, Tudor Ambarus wrote: >> Use NOR parameters in the probe's sequence of calls, thus >> nor->params->size instead of nor->mtd.size and let the mtd_info >> fields be used by the mtd calls (mtd->_erase, mtd->_read, mtd->_write). >> mtd_info fields should not be used during probe because we haven't >> registered mtd yet. > > These changes look good to me, but there is one thing this seems to be > missing. In xlinx.c, I see that xilinx_setup() sets nor->mtd.size. I > think it should be updated to set nor->params->size instead otherwise thanks, you're correct. But I should better do it as a separate patch that fixes: Fixes: 641edddb4f43 ("mtd: spi-nor: Add s3an_post_sfdp_fixups()") because mtd->size is overwritten in that patch with mtd->size = params->size; while it should have kept its s3an specific value. Will respin the entire series with this change as the first patch. > functions like spi_nor_set_addr_width() might end up using the wrong > value. > >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > >> --- >> v4: new patch >> >> drivers/mtd/spi-nor/core.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c >> index 88dd0908d172..5b9c827d411c 100644 >> --- a/drivers/mtd/spi-nor/core.c >> +++ b/drivers/mtd/spi-nor/core.c >> @@ -2115,7 +2115,7 @@ static int spi_nor_spimem_check_op(struct spi_nor *nor, >> */ >> op->addr.nbytes = 4; >> if (!spi_mem_supports_op(nor->spimem, op)) { >> - if (nor->mtd.size > SZ_16M) >> + if (nor->params->size > SZ_16M) >> return -EOPNOTSUPP; >> >> /* If flash size <= 16MB, 3 address bytes are sufficient */ >> @@ -3011,7 +3011,7 @@ static int spi_nor_set_addr_width(struct spi_nor *nor) >> nor->addr_width = 3; >> } >> >> - if (nor->addr_width == 3 && nor->mtd.size > 0x1000000) { >> + if (nor->addr_width == 3 && nor->params->size > 0x1000000) { >> /* enable 4-byte addressing if the device exceeds 16MiB */ >> nor->addr_width = 4; >> } >> @@ -3245,7 +3245,7 @@ static int spi_nor_create_read_dirmap(struct spi_nor *nor) >> SPI_MEM_OP_DUMMY(nor->read_dummy, 0), >> SPI_MEM_OP_DATA_IN(0, NULL, 0)), >> .offset = 0, >> - .length = nor->mtd.size, >> + .length = nor->params->size, >> }; >> struct spi_mem_op *op = &info.op_tmpl; >> >> @@ -3276,7 +3276,7 @@ static int spi_nor_create_write_dirmap(struct spi_nor *nor) >> SPI_MEM_OP_NO_DUMMY, >> SPI_MEM_OP_DATA_OUT(0, NULL, 0)), >> .offset = 0, >> - .length = nor->mtd.size, >> + .length = nor->params->size, >> }; >> struct spi_mem_op *op = &info.op_tmpl; >> >> -- >> 2.25.1 >> > > -- > Regards, > Pratyush Yadav > Texas Instruments Inc. >
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 88dd0908d172..5b9c827d411c 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -2115,7 +2115,7 @@ static int spi_nor_spimem_check_op(struct spi_nor *nor, */ op->addr.nbytes = 4; if (!spi_mem_supports_op(nor->spimem, op)) { - if (nor->mtd.size > SZ_16M) + if (nor->params->size > SZ_16M) return -EOPNOTSUPP; /* If flash size <= 16MB, 3 address bytes are sufficient */ @@ -3011,7 +3011,7 @@ static int spi_nor_set_addr_width(struct spi_nor *nor) nor->addr_width = 3; } - if (nor->addr_width == 3 && nor->mtd.size > 0x1000000) { + if (nor->addr_width == 3 && nor->params->size > 0x1000000) { /* enable 4-byte addressing if the device exceeds 16MiB */ nor->addr_width = 4; } @@ -3245,7 +3245,7 @@ static int spi_nor_create_read_dirmap(struct spi_nor *nor) SPI_MEM_OP_DUMMY(nor->read_dummy, 0), SPI_MEM_OP_DATA_IN(0, NULL, 0)), .offset = 0, - .length = nor->mtd.size, + .length = nor->params->size, }; struct spi_mem_op *op = &info.op_tmpl; @@ -3276,7 +3276,7 @@ static int spi_nor_create_write_dirmap(struct spi_nor *nor) SPI_MEM_OP_NO_DUMMY, SPI_MEM_OP_DATA_OUT(0, NULL, 0)), .offset = 0, - .length = nor->mtd.size, + .length = nor->params->size, }; struct spi_mem_op *op = &info.op_tmpl;
Use NOR parameters in the probe's sequence of calls, thus nor->params->size instead of nor->mtd.size and let the mtd_info fields be used by the mtd calls (mtd->_erase, mtd->_read, mtd->_write). mtd_info fields should not be used during probe because we haven't registered mtd yet. Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- v4: new patch drivers/mtd/spi-nor/core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)