| Message ID | 1429868632-7014-1-git-send-email-jagannadh.teki@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi Bin, On 24 April 2015 at 15:13, Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> wrote: > Upto now flash sector_size is assigned from params which isn't > necessarily a sector size from vendor, so based on the SECT_* > flags from flash_params the erase_size will compute and it will > become the sector_size finally. > > Bug report (from Bin Meng): > => sf probe > SF: Detected SST25VF016B with page size 256 Bytes, erase size 4 KiB, > total 2 MiB, mapped at ffe00000 > > => sf erase 0 +100 > SF: 65536 bytes @ 0x0 Erased: OK Can you please test this. > > Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> > Reported-by: Bin Meng <bmeng.cn@gmail.com> > --- > Changes for v2: > - > drivers/mtd/spi/sf_internal.h | 3 ++- > drivers/mtd/spi/sf_probe.c | 3 +++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h > index bd834dc..bef8701 100644 > --- a/drivers/mtd/spi/sf_internal.h > +++ b/drivers/mtd/spi/sf_internal.h > @@ -119,7 +119,8 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len, > * @name: Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO]) > * @jedec: Device jedec ID (0x[1byte_manuf_id][2byte_dev_id]) > * @ext_jedec: Device ext_jedec ID > - * @sector_size: Sector size of this device > + * @sector_size: Isn't necessarily a sector size from vendor, > + * the size here is what works with Sector erase (64KB) > * @nr_sectors: No.of sectors on this device > * @e_rd_cmd: Enum list for read commands > * @flags: Important param, for flash specific behaviour > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c > index de8d0b7..3f6b882 100644 > --- a/drivers/mtd/spi/sf_probe.c > +++ b/drivers/mtd/spi/sf_probe.c > @@ -184,6 +184,9 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, > flash->erase_size = flash->sector_size; > } > > + /* Now erase size becomes valid sector size */ > + flash->sector_size = flash->erase_size; > + > /* Look for the fastest read cmd */ > cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx); > if (cmd) { > -- > 1.9.1 > thanks!
Hi Jagan, On Fri, Apr 24, 2015 at 5:43 PM, Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> wrote: > Upto now flash sector_size is assigned from params which isn't > necessarily a sector size from vendor, so based on the SECT_* > flags from flash_params the erase_size will compute and it will > become the sector_size finally. > > Bug report (from Bin Meng): > => sf probe > SF: Detected SST25VF016B with page size 256 Bytes, erase size 4 KiB, > total 2 MiB, mapped at ffe00000 > > => sf erase 0 +100 > SF: 65536 bytes @ 0x0 Erased: OK > > Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> > Reported-by: Bin Meng <bmeng.cn@gmail.com> Tested-by: Bin Meng <bmeng.cn@gmail.com> But please see my comments blow. > --- > Changes for v2: > - > drivers/mtd/spi/sf_internal.h | 3 ++- > drivers/mtd/spi/sf_probe.c | 3 +++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h > index bd834dc..bef8701 100644 > --- a/drivers/mtd/spi/sf_internal.h > +++ b/drivers/mtd/spi/sf_internal.h > @@ -119,7 +119,8 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len, > * @name: Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO]) > * @jedec: Device jedec ID (0x[1byte_manuf_id][2byte_dev_id]) > * @ext_jedec: Device ext_jedec ID > - * @sector_size: Sector size of this device > + * @sector_size: Isn't necessarily a sector size from vendor, > + * the size here is what works with Sector erase (64KB) Sector -> sector. Also I think we should remove (64KB) here as it is confusing. > * @nr_sectors: No.of sectors on this device > * @e_rd_cmd: Enum list for read commands > * @flags: Important param, for flash specific behaviour > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c > index de8d0b7..3f6b882 100644 > --- a/drivers/mtd/spi/sf_probe.c > +++ b/drivers/mtd/spi/sf_probe.c > @@ -184,6 +184,9 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, > flash->erase_size = flash->sector_size; > } > > + /* Now erase size becomes valid sector size */ > + flash->sector_size = flash->erase_size; > + > /* Look for the fastest read cmd */ > cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx); > if (cmd) { > -- Regards, Bin
On 27 April 2015 at 10:54, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Jagan, > > On Fri, Apr 24, 2015 at 5:43 PM, Jagannadha Sutradharudu Teki > <jagannadh.teki@gmail.com> wrote: >> Upto now flash sector_size is assigned from params which isn't >> necessarily a sector size from vendor, so based on the SECT_* >> flags from flash_params the erase_size will compute and it will >> become the sector_size finally. >> >> Bug report (from Bin Meng): >> => sf probe >> SF: Detected SST25VF016B with page size 256 Bytes, erase size 4 KiB, >> total 2 MiB, mapped at ffe00000 >> >> => sf erase 0 +100 >> SF: 65536 bytes @ 0x0 Erased: OK >> >> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> >> Reported-by: Bin Meng <bmeng.cn@gmail.com> > > Tested-by: Bin Meng <bmeng.cn@gmail.com> > > But please see my comments blow. > >> --- >> Changes for v2: >> - >> drivers/mtd/spi/sf_internal.h | 3 ++- >> drivers/mtd/spi/sf_probe.c | 3 +++ >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h >> index bd834dc..bef8701 100644 >> --- a/drivers/mtd/spi/sf_internal.h >> +++ b/drivers/mtd/spi/sf_internal.h >> @@ -119,7 +119,8 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len, >> * @name: Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO]) >> * @jedec: Device jedec ID (0x[1byte_manuf_id][2byte_dev_id]) >> * @ext_jedec: Device ext_jedec ID >> - * @sector_size: Sector size of this device >> + * @sector_size: Isn't necessarily a sector size from vendor, >> + * the size here is what works with Sector erase (64KB) Ok I will replace CMD_ERASE_64K instead of Sector erase (64KB) "the size listed here is what works with CMD_ERASE_64K" Any comments? > > Sector -> sector. Also I think we should remove (64KB) here as it is confusing. > >> * @nr_sectors: No.of sectors on this device >> * @e_rd_cmd: Enum list for read commands >> * @flags: Important param, for flash specific behaviour >> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c >> index de8d0b7..3f6b882 100644 >> --- a/drivers/mtd/spi/sf_probe.c >> +++ b/drivers/mtd/spi/sf_probe.c >> @@ -184,6 +184,9 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, >> flash->erase_size = flash->sector_size; >> } >> >> + /* Now erase size becomes valid sector size */ >> + flash->sector_size = flash->erase_size; >> + >> /* Look for the fastest read cmd */ >> cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx); >> if (cmd) { >> -- thanks!
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index bd834dc..bef8701 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -119,7 +119,8 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len, * @name: Device name ([MANUFLETTER][DEVTYPE][DENSITY][EXTRAINFO]) * @jedec: Device jedec ID (0x[1byte_manuf_id][2byte_dev_id]) * @ext_jedec: Device ext_jedec ID - * @sector_size: Sector size of this device + * @sector_size: Isn't necessarily a sector size from vendor, + * the size here is what works with Sector erase (64KB) * @nr_sectors: No.of sectors on this device * @e_rd_cmd: Enum list for read commands * @flags: Important param, for flash specific behaviour diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index de8d0b7..3f6b882 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -184,6 +184,9 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, flash->erase_size = flash->sector_size; } + /* Now erase size becomes valid sector size */ + flash->sector_size = flash->erase_size; + /* Look for the fastest read cmd */ cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx); if (cmd) {
Upto now flash sector_size is assigned from params which isn't necessarily a sector size from vendor, so based on the SECT_* flags from flash_params the erase_size will compute and it will become the sector_size finally. Bug report (from Bin Meng): => sf probe SF: Detected SST25VF016B with page size 256 Bytes, erase size 4 KiB, total 2 MiB, mapped at ffe00000 => sf erase 0 +100 SF: 65536 bytes @ 0x0 Erased: OK Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> Reported-by: Bin Meng <bmeng.cn@gmail.com> --- Changes for v2: - drivers/mtd/spi/sf_internal.h | 3 ++- drivers/mtd/spi/sf_probe.c | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-)