[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Zhiqiang Hou]

From: Hou Zhiqiang <B48286@freescale.com>

For more than 16MiB SPI flash chips, there are 3-Byte and 4-Byte address
mode, and only the 3-Byte address mode is supported in U-Boot so far.
So, reset the SPI flash to 3-Byte address mode in probe to ensure the SPI
flash work correctly, because it may has been set to 4-Byte address mode
after warm boot.

Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
---
Tested on T1042RDB board.
V6:
    Add the spi_release_bus.
V5:
    1. Removed #ifdef for STMICRO.
    2. Add support for Spansion chips (>16MiB) switch to 3-Byte address mode.
V4:
    Split the the patch to 2 patches for clear FSR and SPI flash address mode.
V3:
    Generate the patch based on the latest tree git://git.denx.de/u-boot.git.
V2:
    Add the operation of enter 3 Byte address mode in probe.
V1:
    Based on git://git.denx.de/u-boot.git.
 drivers/mtd/spi/sf_internal.h |  7 +++++++
 drivers/mtd/spi/sf_ops.c      | 40 ++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi/sf_probe.c    | 10 ++++++++++
 3 files changed, 57 insertions(+)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 1de1dac..9519bd8 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -75,6 +75,10 @@ enum {
 #define CMD_FLAG_STATUS			0x70
 #define CMD_CLEAR_FLAG_STATUS		0x50
 
+/* Used for Micron, Macronix and Winbond flashes */
+#define	CMD_ENTER_4B_ADDR		0xB7
+#define	CMD_EXIT_4B_ADDR		0xE9
+
 /* Read commands */
 #define CMD_READ_ARRAY_SLOW		0x03
 #define CMD_READ_ARRAY_FAST		0x0b
@@ -231,6 +235,9 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
 int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		size_t len, void *data);
 
+int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
+		int enable, u8 idcode0);
+
 #ifdef CONFIG_SPI_FLASH_MTD
 int spi_flash_mtd_register(struct spi_flash *flash);
 void spi_flash_mtd_unregister(void);
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index deebcab..de30c55 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -93,6 +93,46 @@ int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc)
 }
 #endif
 
+int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
+				int enable, u8 idcode0)
+{
+	int ret;
+	u8 cmd, bar;
+	bool need_wren = false;
+
+	ret = spi_claim_bus(flash->spi);
+	if (ret) {
+		debug("SF: unable to claim SPI bus\n");
+		return ret;
+	}
+
+	switch (idcode0) {
+	case SPI_FLASH_CFI_MFR_STMICRO:
+		/* Some Micron need WREN command; all will accept it */
+		need_wren = true;
+	case SPI_FLASH_CFI_MFR_MACRONIX:
+	case SPI_FLASH_CFI_MFR_WINBOND:
+		if (need_wren)
+			spi_flash_cmd_write_enable(flash);
+
+		cmd = enable ? CMD_ENTER_4B_ADDR : CMD_EXIT_4B_ADDR;
+		ret = spi_flash_cmd(flash->spi, cmd, NULL, 0);
+		if (need_wren)
+			spi_flash_cmd_write_disable(flash);
+
+		break;
+	default:
+		/* Spansion style */
+		bar = enable << 7;
+		cmd = CMD_BANKADDR_BRWR;
+		ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &bar, 1);
+	}
+
+	spi_release_bus(flash->spi);
+
+	return ret;
+}
+
 #ifdef CONFIG_SPI_FLASH_BAR
 static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8 bank_sel)
 {
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index e0283dc..3b204f8 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -170,6 +170,16 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 	flash->page_size <<= flash->shift;
 	flash->sector_size = params->sector_size << flash->shift;
 	flash->size = flash->sector_size * params->nr_sectors << flash->shift;
+
+	/*
+	 * So far, the 4-byte address mode haven't been supported in U-Boot,
+	 * and make sure the chip (> 16MiB) in default 3-byte address mode,
+	 * in case of warm bootup, the chip was set to 4-byte mode in kernel.
+	 */
+	if (flash->size > SPI_FLASH_16MB_BOUN) {
+		if (spi_flash_cmd_4B_addr_switch(flash, false, idcode[0]) < 0)
+			debug("SF: enter 3B address mode failed\n");
+	}
 #ifdef CONFIG_SF_DUAL_FLASH
 	if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
 		flash->size <<= 1;
-- 
2.1.0.27.g96db324

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Siva Durga Prasad Paladugu]

Hi Zhejiang/Jagan,

I think it would be good to extend this further to support 4-byte addressing in u-boot also.
This would be based on the driver, We can get the data that whether the controller supports 4-byte or not from the driver level(through slave struct)
and enable 4 byte addressing here based on that.

Let me know your thoughts on this.

Regards,
Siva

> -----Original Message-----
> From: Zhejiang Hou [mailto:B48286 at freescale.com]
> Sent: Wednesday, August 12, 2015 11:48 AM
> To: u-boot at lists.denx.de; jteki at openedev.com
> Cc: yorksun at freescale.com; Mingkai.Hu at freescale.com; Siva Durga Prasad
> Paladugu; Hou Zhiqiang
> Subject: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
>
> From: Hou Zhiqiang <B48286@freescale.com>
>
> For more than 16MiB SPI flash chips, there are 3-Byte and 4-Byte address
> mode, and only the 3-Byte address mode is supported in U-Boot so far.
> So, reset the SPI flash to 3-Byte address mode in probe to ensure the SPI
> flash work correctly, because it may has been set to 4-Byte address mode
> after warm boot.
>
> Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
> ---
> Tested on T1042RDB board.
> V6:
>     Add the spi_release_bus.
> V5:
>     1. Removed #ifdef for STMICRO.
>     2. Add support for Spansion chips (>16MiB) switch to 3-Byte address mode.
> V4:
>     Split the the patch to 2 patches for clear FSR and SPI flash address mode.
> V3:
>     Generate the patch based on the latest tree git://git.denx.de/u-boot.git.
> V2:
>     Add the operation of enter 3 Byte address mode in probe.
> V1:
>     Based on git://git.denx.de/u-boot.git.
>  drivers/mtd/spi/sf_internal.h |  7 +++++++
>  drivers/mtd/spi/sf_ops.c      | 40
> ++++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi/sf_probe.c    | 10 ++++++++++
>  3 files changed, 57 insertions(+)
>
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 1de1dac..9519bd8 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -75,6 +75,10 @@ enum {
>  #define CMD_FLAG_STATUS                      0x70
>  #define CMD_CLEAR_FLAG_STATUS                0x50
>
> +/* Used for Micron, Macronix and Winbond flashes */
> +#define      CMD_ENTER_4B_ADDR               0xB7
> +#define      CMD_EXIT_4B_ADDR                0xE9
> +
>  /* Read commands */
>  #define CMD_READ_ARRAY_SLOW          0x03
>  #define CMD_READ_ARRAY_FAST          0x0b
> @@ -231,6 +235,9 @@ int spi_flash_read_common(struct spi_flash *flash,
> const u8 *cmd,  int spi_flash_cmd_read_ops(struct spi_flash *flash, u32
> offset,
>               size_t len, void *data);
>
> +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> +             int enable, u8 idcode0);
> +
>  #ifdef CONFIG_SPI_FLASH_MTD
>  int spi_flash_mtd_register(struct spi_flash *flash);  void
> spi_flash_mtd_unregister(void); diff --git a/drivers/mtd/spi/sf_ops.c
> b/drivers/mtd/spi/sf_ops.c index deebcab..de30c55 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -93,6 +93,46 @@ int spi_flash_cmd_write_config(struct spi_flash *flash,
> u8 wc)  }  #endif
>
> +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> +                             int enable, u8 idcode0)
> +{
> +     int ret;
> +     u8 cmd, bar;
> +     bool need_wren = false;
> +
> +     ret = spi_claim_bus(flash->spi);
> +     if (ret) {
> +             debug("SF: unable to claim SPI bus\n");
> +             return ret;
> +     }
> +
> +     switch (idcode0) {
> +     case SPI_FLASH_CFI_MFR_STMICRO:
> +             /* Some Micron need WREN command; all will accept it */
> +             need_wren = true;
> +     case SPI_FLASH_CFI_MFR_MACRONIX:
> +     case SPI_FLASH_CFI_MFR_WINBOND:
> +             if (need_wren)
> +                     spi_flash_cmd_write_enable(flash);
> +
> +             cmd = enable ? CMD_ENTER_4B_ADDR :
> CMD_EXIT_4B_ADDR;
> +             ret = spi_flash_cmd(flash->spi, cmd, NULL, 0);
> +             if (need_wren)
> +                     spi_flash_cmd_write_disable(flash);
> +
> +             break;
> +     default:
> +             /* Spansion style */
> +             bar = enable << 7;
> +             cmd = CMD_BANKADDR_BRWR;
> +             ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &bar, 1);
> +     }
> +
> +     spi_release_bus(flash->spi);
> +
> +     return ret;
> +}
> +
>  #ifdef CONFIG_SPI_FLASH_BAR
>  static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8
> bank_sel)  { diff --git a/drivers/mtd/spi/sf_probe.c
> b/drivers/mtd/spi/sf_probe.c index e0283dc..3b204f8 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -170,6 +170,16 @@ static int spi_flash_validate_params(struct spi_slave
> *spi, u8 *idcode,
>       flash->page_size <<= flash->shift;
>       flash->sector_size = params->sector_size << flash->shift;
>       flash->size = flash->sector_size * params->nr_sectors << flash->shift;
> +
> +     /*
> +      * So far, the 4-byte address mode haven't been supported in U-
> Boot,
> +      * and make sure the chip (> 16MiB) in default 3-byte address mode,
> +      * in case of warm bootup, the chip was set to 4-byte mode in kernel.
> +      */
> +     if (flash->size > SPI_FLASH_16MB_BOUN) {
> +             if (spi_flash_cmd_4B_addr_switch(flash, false, idcode[0]) <
> 0)
> +                     debug("SF: enter 3B address mode failed\n");
> +     }
>  #ifdef CONFIG_SF_DUAL_FLASH
>       if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
>               flash->size <<= 1;
> --
> 2.1.0.27.g96db324



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Hou Zhiqiang]

> -----Original Message-----
> From: Siva Durga Prasad Paladugu [mailto:siva.durga.paladugu at xilinx.com]
> Sent: 2015?8?13? 17:18
> To: Hou Zhiqiang-B48286; u-boot at lists.denx.de; jteki at openedev.com
> Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
> Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
> 
> Hi Zhejiang/Jagan,
> 
> I think it would be good to extend this further to support 4-byte
> addressing in u-boot also.
> This would be based on the driver, We can get the data that whether the
> controller supports 4-byte or not from the driver level(through slave
> struct) and enable 4 byte addressing here based on that.
>

That is a long way, and I don't think it is necessary for U-Boot. The U-Boot
work well using BAR to address more than 16MiB flash.
This patch focus on the address mode issue when warm boot up base on the BAR
address mode.

> Let me know your thoughts on this.
> 
> Regards,
> Siva
> 
> > -----Original Message-----
> > From: Zhejiang Hou [mailto:B48286 at freescale.com]
> > Sent: Wednesday, August 12, 2015 11:48 AM
> > To: u-boot at lists.denx.de; jteki at openedev.com
> > Cc: yorksun at freescale.com; Mingkai.Hu at freescale.com; Siva Durga Prasad
> > Paladugu; Hou Zhiqiang
> > Subject: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
> >
> > From: Hou Zhiqiang <B48286@freescale.com>
> >
> > For more than 16MiB SPI flash chips, there are 3-Byte and 4-Byte
> > address mode, and only the 3-Byte address mode is supported in U-Boot
> so far.
> > So, reset the SPI flash to 3-Byte address mode in probe to ensure the
> > SPI flash work correctly, because it may has been set to 4-Byte
> > address mode after warm boot.
> >
> > Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
> > ---
> > Tested on T1042RDB board.
> > V6:
> >     Add the spi_release_bus.
> > V5:
> >     1. Removed #ifdef for STMICRO.
> >     2. Add support for Spansion chips (>16MiB) switch to 3-Byte address
> mode.
> > V4:
> >     Split the the patch to 2 patches for clear FSR and SPI flash
> address mode.
> > V3:
> >     Generate the patch based on the latest tree git://git.denx.de/u-
> boot.git.
> > V2:
> >     Add the operation of enter 3 Byte address mode in probe.
> > V1:
> >     Based on git://git.denx.de/u-boot.git.
> >  drivers/mtd/spi/sf_internal.h |  7 +++++++
> >  drivers/mtd/spi/sf_ops.c      | 40
> > ++++++++++++++++++++++++++++++++++++++++
> >  drivers/mtd/spi/sf_probe.c    | 10 ++++++++++
> >  3 files changed, 57 insertions(+)
> >
> > diff --git a/drivers/mtd/spi/sf_internal.h
> > b/drivers/mtd/spi/sf_internal.h index 1de1dac..9519bd8 100644
> > --- a/drivers/mtd/spi/sf_internal.h
> > +++ b/drivers/mtd/spi/sf_internal.h
> > @@ -75,6 +75,10 @@ enum {
> >  #define CMD_FLAG_STATUS                      0x70
> >  #define CMD_CLEAR_FLAG_STATUS                0x50
> >
> > +/* Used for Micron, Macronix and Winbond flashes */
> > +#define      CMD_ENTER_4B_ADDR               0xB7
> > +#define      CMD_EXIT_4B_ADDR                0xE9
> > +
> >  /* Read commands */
> >  #define CMD_READ_ARRAY_SLOW          0x03
> >  #define CMD_READ_ARRAY_FAST          0x0b
> > @@ -231,6 +235,9 @@ int spi_flash_read_common(struct spi_flash *flash,
> > const u8 *cmd,  int spi_flash_cmd_read_ops(struct spi_flash *flash,
> > u32 offset,
> >               size_t len, void *data);
> >
> > +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> > +             int enable, u8 idcode0);
> > +
> >  #ifdef CONFIG_SPI_FLASH_MTD
> >  int spi_flash_mtd_register(struct spi_flash *flash);  void
> > spi_flash_mtd_unregister(void); diff --git a/drivers/mtd/spi/sf_ops.c
> > b/drivers/mtd/spi/sf_ops.c index deebcab..de30c55 100644
> > --- a/drivers/mtd/spi/sf_ops.c
> > +++ b/drivers/mtd/spi/sf_ops.c
> > @@ -93,6 +93,46 @@ int spi_flash_cmd_write_config(struct spi_flash
> > *flash,
> > u8 wc)  }  #endif
> >
> > +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> > +                             int enable, u8 idcode0) {
> > +     int ret;
> > +     u8 cmd, bar;
> > +     bool need_wren = false;
> > +
> > +     ret = spi_claim_bus(flash->spi);
> > +     if (ret) {
> > +             debug("SF: unable to claim SPI bus\n");
> > +             return ret;
> > +     }
> > +
> > +     switch (idcode0) {
> > +     case SPI_FLASH_CFI_MFR_STMICRO:
> > +             /* Some Micron need WREN command; all will accept it */
> > +             need_wren = true;
> > +     case SPI_FLASH_CFI_MFR_MACRONIX:
> > +     case SPI_FLASH_CFI_MFR_WINBOND:
> > +             if (need_wren)
> > +                     spi_flash_cmd_write_enable(flash);
> > +
> > +             cmd = enable ? CMD_ENTER_4B_ADDR :
> > CMD_EXIT_4B_ADDR;
> > +             ret = spi_flash_cmd(flash->spi, cmd, NULL, 0);
> > +             if (need_wren)
> > +                     spi_flash_cmd_write_disable(flash);
> > +
> > +             break;
> > +     default:
> > +             /* Spansion style */
> > +             bar = enable << 7;
> > +             cmd = CMD_BANKADDR_BRWR;
> > +             ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &bar, 1);
> > +     }
> > +
> > +     spi_release_bus(flash->spi);
> > +
> > +     return ret;
> > +}
> > +
> >  #ifdef CONFIG_SPI_FLASH_BAR
> >  static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8
> > bank_sel)  { diff --git a/drivers/mtd/spi/sf_probe.c
> > b/drivers/mtd/spi/sf_probe.c index e0283dc..3b204f8 100644
> > --- a/drivers/mtd/spi/sf_probe.c
> > +++ b/drivers/mtd/spi/sf_probe.c
> > @@ -170,6 +170,16 @@ static int spi_flash_validate_params(struct
> > spi_slave *spi, u8 *idcode,
> >       flash->page_size <<= flash->shift;
> >       flash->sector_size = params->sector_size << flash->shift;
> >       flash->size = flash->sector_size * params->nr_sectors <<
> > flash->shift;
> > +
> > +     /*
> > +      * So far, the 4-byte address mode haven't been supported in U-
> > Boot,
> > +      * and make sure the chip (> 16MiB) in default 3-byte address
> mode,
> > +      * in case of warm bootup, the chip was set to 4-byte mode in
> kernel.
> > +      */
> > +     if (flash->size > SPI_FLASH_16MB_BOUN) {
> > +             if (spi_flash_cmd_4B_addr_switch(flash, false,
> > + idcode[0]) <
> > 0)
> > +                     debug("SF: enter 3B address mode failed\n");
> > +     }
> >  #ifdef CONFIG_SF_DUAL_FLASH
> >       if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> >               flash->size <<= 1;
> > --
> > 2.1.0.27.g96db324
> 
> 
> 
> This email and any attachments are intended for the sole use of the named
> recipient(s) and contain(s) confidential information that may be
> proprietary, privileged or copyrighted under applicable law. If you are
> not the intended recipient, do not read, copy, or forward this email
> message or any attachments. Delete this email message and any attachments
> immediately.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Stefan Roese]

On 13.08.2015 12:50, Hou Zhiqiang wrote:
>> -----Original Message-----
>> From: Siva Durga Prasad Paladugu [mailto:siva.durga.paladugu at xilinx.com]
>> Sent: 2015?8?13? 17:18
>> To: Hou Zhiqiang-B48286; u-boot at lists.denx.de; jteki at openedev.com
>> Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
>> Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
>>
>> Hi Zhejiang/Jagan,
>>
>> I think it would be good to extend this further to support 4-byte
>> addressing in u-boot also.
>> This would be based on the driver, We can get the data that whether the
>> controller supports 4-byte or not from the driver level(through slave
>> struct) and enable 4 byte addressing here based on that.
>>
>
> That is a long way, and I don't think it is necessary for U-Boot. The U-Boot
> work well using BAR to address more than 16MiB flash.
> This patch focus on the address mode issue when warm boot up base on the BAR
> address mode.

Please correct me if I'm wrong, but AFAIU this BAR thing 
(CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB SPI flash 
contiguously. Only 16MiB areas. So for example its not possible to put 
UBI/UBIFS in such a big partition.

I have to support Siva here. We really should try to support this 4-byte 
mode correctly. This can't be too hard.

Thanks,
Stefan

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Siva Durga Prasad Paladugu]

> -----Original Message-----
> From: Stefan Roese [mailto:sr at denx.de]
> Sent: Thursday, August 13, 2015 4:51 PM
> To: Hou Zhiqiang; Siva Durga Prasad Paladugu; u-boot at lists.denx.de;
> jteki at openedev.com
> Cc: nofooter; York Sun
> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> mode
>
> On 13.08.2015 12:50, Hou Zhiqiang wrote:
> >> -----Original Message-----
> >> From: Siva Durga Prasad Paladugu
> >> [mailto:siva.durga.paladugu at xilinx.com]
> >> Sent: 2015?8?13? 17:18
> >> To: Hou Zhiqiang-B48286; u-boot at lists.denx.de; jteki at openedev.com
> >> Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
> >> Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> >> mode
> >>
> >> Hi Zhejiang/Jagan,
> >>
> >> I think it would be good to extend this further to support 4-byte
> >> addressing in u-boot also.
> >> This would be based on the driver, We can get the data that whether
> >> the controller supports 4-byte or not from the driver level(through
> >> slave
> >> struct) and enable 4 byte addressing here based on that.
> >>
> >
> > That is a long way, and I don't think it is necessary for U-Boot. The
> > U-Boot work well using BAR to address more than 16MiB flash.
> > This patch focus on the address mode issue when warm boot up base on
> > the BAR address mode.
>
> Please correct me if I'm wrong, but AFAIU this BAR thing
> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB SPI flash
> contiguously. Only 16MiB areas. So for example its not possible to put
> UBI/UBIFS in such a big partition.
>
> I have to support Siva here. We really should try to support this 4-byte mode
> correctly. This can't be too hard.

Yeah, I think it wouldn't be too difficult to add support for 4-byte address.
we may just need to bypass the SPI_FLASH_BAR in read_ops , write_ops and erase_ops routines.

This support need not be part of this series. It can be a separate series. As we already added routine to disable /enable 4 byte as a part of this,
I just shared that it would be good to have 4 byte support also.


Regards,
Siva
>
> Thanks,
> Stefan



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Jagan Teki]

On 13 August 2015 at 16:57, Siva Durga Prasad Paladugu
<siva.durga.paladugu@xilinx.com> wrote:
>
>
>> -----Original Message-----
>> From: Stefan Roese [mailto:sr at denx.de]
>> Sent: Thursday, August 13, 2015 4:51 PM
>> To: Hou Zhiqiang; Siva Durga Prasad Paladugu; u-boot at lists.denx.de;
>> jteki at openedev.com
>> Cc: nofooter; York Sun
>> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
>> mode
>>
>> On 13.08.2015 12:50, Hou Zhiqiang wrote:
>> >> -----Original Message-----
>> >> From: Siva Durga Prasad Paladugu
>> >> [mailto:siva.durga.paladugu at xilinx.com]
>> >> Sent: 2015?8?13? 17:18
>> >> To: Hou Zhiqiang-B48286; u-boot at lists.denx.de; jteki at openedev.com
>> >> Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
>> >> Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
>> >> mode
>> >>
>> >> Hi Zhejiang/Jagan,
>> >>
>> >> I think it would be good to extend this further to support 4-byte
>> >> addressing in u-boot also.
>> >> This would be based on the driver, We can get the data that whether
>> >> the controller supports 4-byte or not from the driver level(through
>> >> slave
>> >> struct) and enable 4 byte addressing here based on that.
>> >>
>> >
>> > That is a long way, and I don't think it is necessary for U-Boot. The
>> > U-Boot work well using BAR to address more than 16MiB flash.
>> > This patch focus on the address mode issue when warm boot up base on
>> > the BAR address mode.
>>
>> Please correct me if I'm wrong, but AFAIU this BAR thing
>> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB SPI flash
>> contiguously. Only 16MiB areas. So for example its not possible to put
>> UBI/UBIFS in such a big partition.

Stefan,

No, BAR is accessing flash > 16MiB in 3-byte addressing mode, for your
example of
64MiB flash, it can grouped into 4 16MiB regions means 4 bank vales
bank0 to bank3

Based on the sf read/erase/write flash offsets, that particular bank
will enable an do
the relevant operations.

In-spite of having 4 byte addressing operations BAR should do exactly
same with existing
3-byte addressing.

>>
>> I have to support Siva here. We really should try to support this 4-byte mode
>> correctly. This can't be too hard.
>
> Yeah, I think it wouldn't be too difficult to add support for 4-byte address.
> we may just need to bypass the SPI_FLASH_BAR in read_ops , write_ops and erase_ops routines.
>
> This support need not be part of this series. It can be a separate series. As we already added routine to disable /enable 4 byte as a part of this,
> I just shared that it would be good to have 4 byte support also.

Siva,

Agree that adding 4-byte addressing is not too difficult, but by
passing BAR is not a good
idea, what if you have a flash with > 16 MiB and controller have
3-byte addressing command
support.

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Stefan Roese]

Jagan,

On 13.08.2015 13:45, Jagan Teki wrote:
>>> Please correct me if I'm wrong, but AFAIU this BAR thing
>>> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB SPI flash
>>> contiguously. Only 16MiB areas. So for example its not possible to put
>>> UBI/UBIFS in such a big partition.
>
> Stefan,
>
> No, BAR is accessing flash > 16MiB in 3-byte addressing mode, for your
> example of
> 64MiB flash, it can grouped into 4 16MiB regions means 4 bank vales
> bank0 to bank3
>
> Based on the sf read/erase/write flash offsets, that particular bank
> will enable an do
> the relevant operations.
>
> In-spite of having 4 byte addressing operations BAR should do exactly
> same with existing
> 3-byte addressing.

Okay, thanks for the explanation. If this really is the case, and this 
BAR support will seamlessly enable the access to the complete flash 
area, then the 4-byte mode should really not be necessary.

I'm wondering though about 2 things:

a) Do all SPI NOR flash chips support this BAR mode?

b) If yes, why isn't BAR enabled per default?

c) Why doesn't the Linux kernel use this BAR mode?

Thanks,
Stefan

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Siva Durga Prasad Paladugu]

Hi Jagan,


> -----Original Message-----
> From: Jagan Teki [mailto:jteki at openedev.com]
> Sent: Thursday, August 13, 2015 5:16 PM
> To: Siva Durga Prasad Paladugu
> Cc: Stefan Roese; Hou Zhiqiang; u-boot at lists.denx.de; nofooter; York Sun
> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> mode
>
> On 13 August 2015 at 16:57, Siva Durga Prasad Paladugu
> <siva.durga.paladugu@xilinx.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Stefan Roese [mailto:sr at denx.de]
> >> Sent: Thursday, August 13, 2015 4:51 PM
> >> To: Hou Zhiqiang; Siva Durga Prasad Paladugu; u-boot at lists.denx.de;
> >> jteki at openedev.com
> >> Cc: nofooter; York Sun
> >> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte
> >> address mode
> >>
> >> On 13.08.2015 12:50, Hou Zhiqiang wrote:
> >> >> -----Original Message-----
> >> >> From: Siva Durga Prasad Paladugu
> >> >> [mailto:siva.durga.paladugu at xilinx.com]
> >> >> Sent: 2015?8?13? 17:18
> >> >> To: Hou Zhiqiang-B48286; u-boot at lists.denx.de; jteki at openedev.com
> >> >> Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
> >> >> Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte
> >> >> address mode
> >> >>
> >> >> Hi Zhejiang/Jagan,
> >> >>
> >> >> I think it would be good to extend this further to support 4-byte
> >> >> addressing in u-boot also.
> >> >> This would be based on the driver, We can get the data that
> >> >> whether the controller supports 4-byte or not from the driver
> >> >> level(through slave
> >> >> struct) and enable 4 byte addressing here based on that.
> >> >>
> >> >
> >> > That is a long way, and I don't think it is necessary for U-Boot.
> >> > The U-Boot work well using BAR to address more than 16MiB flash.
> >> > This patch focus on the address mode issue when warm boot up base
> >> > on the BAR address mode.
> >>
> >> Please correct me if I'm wrong, but AFAIU this BAR thing
> >> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB SPI
> >> flash contiguously. Only 16MiB areas. So for example its not possible
> >> to put UBI/UBIFS in such a big partition.
>
> Stefan,
>
> No, BAR is accessing flash > 16MiB in 3-byte addressing mode, for your
> example of 64MiB flash, it can grouped into 4 16MiB regions means 4 bank
> vales
> bank0 to bank3
>
> Based on the sf read/erase/write flash offsets, that particular bank will
> enable an do the relevant operations.
>
> In-spite of having 4 byte addressing operations BAR should do exactly same
> with existing 3-byte addressing.
>
> >>
> >> I have to support Siva here. We really should try to support this
> >> 4-byte mode correctly. This can't be too hard.
> >
> > Yeah, I think it wouldn't be too difficult to add support for 4-byte address.
> > we may just need to bypass the SPI_FLASH_BAR in read_ops , write_ops
> and erase_ops routines.
> >
> > This support need not be part of this series. It can be a separate
> > series. As we already added routine to disable /enable 4 byte as a part of
> this, I just shared that it would be good to have 4 byte support also.
>
> Siva,
>
> Agree that adding 4-byte addressing is not too difficult, but by passing BAR is
> not a good idea, what if you have a flash with > 16 MiB and controller have 3-
> byte addressing command support.

Yes, if you look at my first mail on this, I mentioned, we should get that info somehow from the driver of
the respective controller, whether it supports four byte or not. For example from the spi_slave struct.
Here by-passing means that if controller supports four byte then only we should bypass that BAR otherwise proceed
with BAR.

Regards,
Siva
>
> thanks!
> --
> Jagan | openedev.


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Hou Zhiqiang]

Hello all,

As Siva said the 4-Byte support should be splited to another thread.

And this patch focus on the BAR addressing mode base on the current u-boot, 
the aim is make the spi flash work upon warm boot, specifically when boot 
from SPI flash, without correcting the address mode, it will crash when 
kernel set the spi flash to 4-Byte address mode. 

> -----Original Message-----
> From: Siva Durga Prasad Paladugu [mailto:siva.durga.paladugu at xilinx.com]
> Sent: 2015?8?13? 20:48
> To: Jagan Teki
> Cc: Stefan Roese; Hou Zhiqiang-B48286; u-boot at lists.denx.de; nofooter;
> Sun York-R58495
> Subject: RE: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte
> address mode
> 

B.R
Zhiqiang

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Siva Durga Prasad Paladugu]

> -----Original Message-----
> From: Hou Zhiqiang [mailto:B48286 at freescale.com]
> Sent: Friday, August 14, 2015 9:37 AM
> To: Siva Durga Prasad Paladugu; Jagan Teki
> Cc: Stefan Roese; u-boot at lists.denx.de; nofooter; York Sun
> Subject: RE: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> mode
>
> Hello all,
>
> As Siva said the 4-Byte support should be splited to another thread.
Yes, if we would like to add 4-byte then it can be a separate series and
we can go ahead with this patch for now.
I just got one review comment on the patch, sorry for my late finding.
I will reply to the mail with the patch to be more clear on the
comment where it is exactly meant for.


Regards,
Siva

>
> And this patch focus on the BAR addressing mode base on the current u-
> boot, the aim is make the spi flash work upon warm boot, specifically when
> boot from SPI flash, without correcting the address mode, it will crash when
> kernel set the spi flash to 4-Byte address mode.
>
> > -----Original Message-----
> > From: Siva Durga Prasad Paladugu
> > [mailto:siva.durga.paladugu at xilinx.com]
> > Sent: 2015?8?13? 20:48
> > To: Jagan Teki
> > Cc: Stefan Roese; Hou Zhiqiang-B48286; u-boot at lists.denx.de; nofooter;
> > Sun York-R58495
> > Subject: RE: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte
> > address mode
> >
>
> B.R
> Zhiqiang


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Siva Durga Prasad Paladugu]

Hi Zhiqiang,

> -----Original Message-----
> From: Zhiqiang Hou [mailto:B48286 at freescale.com]
> Sent: Wednesday, August 12, 2015 11:48 AM
> To: u-boot at lists.denx.de; jteki at openedev.com
> Cc: yorksun at freescale.com; Mingkai.Hu at freescale.com; Siva Durga Prasad
> Paladugu; Hou Zhiqiang
> Subject: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
>
> From: Hou Zhiqiang <B48286@freescale.com>
>
> For more than 16MiB SPI flash chips, there are 3-Byte and 4-Byte address
> mode, and only the 3-Byte address mode is supported in U-Boot so far.
> So, reset the SPI flash to 3-Byte address mode in probe to ensure the SPI
> flash work correctly, because it may has been set to 4-Byte address mode
> after warm boot.
>
> Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
> ---
> Tested on T1042RDB board.
> V6:
>     Add the spi_release_bus.
> V5:
>     1. Removed #ifdef for STMICRO.
>     2. Add support for Spansion chips (>16MiB) switch to 3-Byte address mode.
> V4:
>     Split the the patch to 2 patches for clear FSR and SPI flash address mode.
> V3:
>     Generate the patch based on the latest tree git://git.denx.de/u-boot.git.
> V2:
>     Add the operation of enter 3 Byte address mode in probe.
> V1:
>     Based on git://git.denx.de/u-boot.git.
>  drivers/mtd/spi/sf_internal.h |  7 +++++++
>  drivers/mtd/spi/sf_ops.c      | 40
> ++++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi/sf_probe.c    | 10 ++++++++++
>  3 files changed, 57 insertions(+)
>
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 1de1dac..9519bd8 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -75,6 +75,10 @@ enum {
>  #define CMD_FLAG_STATUS                      0x70
>  #define CMD_CLEAR_FLAG_STATUS                0x50
>
> +/* Used for Micron, Macronix and Winbond flashes */
> +#define      CMD_ENTER_4B_ADDR               0xB7
> +#define      CMD_EXIT_4B_ADDR                0xE9
> +
>  /* Read commands */
>  #define CMD_READ_ARRAY_SLOW          0x03
>  #define CMD_READ_ARRAY_FAST          0x0b
> @@ -231,6 +235,9 @@ int spi_flash_read_common(struct spi_flash *flash,
> const u8 *cmd,  int spi_flash_cmd_read_ops(struct spi_flash *flash, u32
> offset,
>               size_t len, void *data);
>
> +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> +             int enable, u8 idcode0);
> +
>  #ifdef CONFIG_SPI_FLASH_MTD
>  int spi_flash_mtd_register(struct spi_flash *flash);  void
> spi_flash_mtd_unregister(void); diff --git a/drivers/mtd/spi/sf_ops.c
> b/drivers/mtd/spi/sf_ops.c index deebcab..de30c55 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -93,6 +93,46 @@ int spi_flash_cmd_write_config(struct spi_flash *flash,
> u8 wc)  }  #endif
>
> +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> +                             int enable, u8 idcode0)
> +{
> +     int ret;
> +     u8 cmd, bar;
> +     bool need_wren = false;
> +
> +     ret = spi_claim_bus(flash->spi);
> +     if (ret) {
> +             debug("SF: unable to claim SPI bus\n");
> +             return ret;
> +     }
> +
> +     switch (idcode0) {
> +     case SPI_FLASH_CFI_MFR_STMICRO:
> +             /* Some Micron need WREN command; all will accept it */
> +             need_wren = true;
> +     case SPI_FLASH_CFI_MFR_MACRONIX:
> +     case SPI_FLASH_CFI_MFR_WINBOND:
> +             if (need_wren)
> +                     spi_flash_cmd_write_enable(flash);
> +
> +             cmd = enable ? CMD_ENTER_4B_ADDR :
> CMD_EXIT_4B_ADDR;
> +             ret = spi_flash_cmd(flash->spi, cmd, NULL, 0);
> +             if (need_wren)
> +                     spi_flash_cmd_write_disable(flash);
> +
> +             break;
> +     default:
> +             /* Spansion style */
> +             bar = enable << 7;
> +             cmd = CMD_BANKADDR_BRWR;
> +             ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &bar, 1);
> +     }
> +
> +     spi_release_bus(flash->spi);
> +
> +     return ret;
> +}
> +
>  #ifdef CONFIG_SPI_FLASH_BAR
>  static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8
> bank_sel)  { diff --git a/drivers/mtd/spi/sf_probe.c
> b/drivers/mtd/spi/sf_probe.c index e0283dc..3b204f8 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -170,6 +170,16 @@ static int spi_flash_validate_params(struct spi_slave
> *spi, u8 *idcode,
>       flash->page_size <<= flash->shift;
>       flash->sector_size = params->sector_size << flash->shift;
>       flash->size = flash->sector_size * params->nr_sectors << flash->shift;
> +
> +     /*
> +      * So far, the 4-byte address mode haven't been supported in U-
> Boot,
> +      * and make sure the chip (> 16MiB) in default 3-byte address mode,
> +      * in case of warm bootup, the chip was set to 4-byte mode in kernel.
> +      */
> +     if (flash->size > SPI_FLASH_16MB_BOUN) {
I think that this check should be as below
if (flash->size > (SPI_FLASH_16MB_BOUN << flash->shift)

Regards,
Siva

> +             if (spi_flash_cmd_4B_addr_switch(flash, false, idcode[0]) <
> 0)
> +                     debug("SF: enter 3B address mode failed\n");
> +     }
>  #ifdef CONFIG_SF_DUAL_FLASH
>       if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
>               flash->size <<= 1;
> --
> 2.1.0.27.g96db324



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Hou Zhiqiang]

Hi Jagannadha,

See Siva's comments inline and For the DUAL spi flash memories, do you have any comment?

> -----Original Message-----
> From: Siva Durga Prasad Paladugu [mailto:siva.durga.paladugu at xilinx.com]
> Sent: 2015?8?14? 12:55
> To: Hou Zhiqiang-B48286; u-boot at lists.denx.de; jteki at openedev.com
> Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
> Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
> 
> Hi Zhiqiang,
> 
> > -----Original Message-----
> > From: Zhiqiang Hou [mailto:B48286 at freescale.com]
> > Sent: Wednesday, August 12, 2015 11:48 AM
> > To: u-boot at lists.denx.de; jteki at openedev.com
> > Cc: yorksun at freescale.com; Mingkai.Hu at freescale.com; Siva Durga Prasad
> > Paladugu; Hou Zhiqiang
> > Subject: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
> >
> > From: Hou Zhiqiang <B48286@freescale.com>
> >
> > For more than 16MiB SPI flash chips, there are 3-Byte and 4-Byte
> > address mode, and only the 3-Byte address mode is supported in U-Boot
> so far.
> > So, reset the SPI flash to 3-Byte address mode in probe to ensure the
> > SPI flash work correctly, because it may has been set to 4-Byte
> > address mode after warm boot.
> >
> > Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
> > ---
> > Tested on T1042RDB board.
> > V6:
> >     Add the spi_release_bus.
> > V5:
> >     1. Removed #ifdef for STMICRO.
> >     2. Add support for Spansion chips (>16MiB) switch to 3-Byte address
> mode.
> > V4:
> >     Split the the patch to 2 patches for clear FSR and SPI flash
> address mode.
> > V3:
> >     Generate the patch based on the latest tree git://git.denx.de/u-
> boot.git.
> > V2:
> >     Add the operation of enter 3 Byte address mode in probe.
> > V1:
> >     Based on git://git.denx.de/u-boot.git.
> >  drivers/mtd/spi/sf_internal.h |  7 +++++++
> >  drivers/mtd/spi/sf_ops.c      | 40
> > ++++++++++++++++++++++++++++++++++++++++
> >  drivers/mtd/spi/sf_probe.c    | 10 ++++++++++
> >  3 files changed, 57 insertions(+)
> >
> > diff --git a/drivers/mtd/spi/sf_internal.h
> > b/drivers/mtd/spi/sf_internal.h index 1de1dac..9519bd8 100644
> > --- a/drivers/mtd/spi/sf_internal.h
> > +++ b/drivers/mtd/spi/sf_internal.h
> > @@ -75,6 +75,10 @@ enum {
> >  #define CMD_FLAG_STATUS                      0x70
> >  #define CMD_CLEAR_FLAG_STATUS                0x50
> >
> > +/* Used for Micron, Macronix and Winbond flashes */
> > +#define      CMD_ENTER_4B_ADDR               0xB7
> > +#define      CMD_EXIT_4B_ADDR                0xE9
> > +
> >  /* Read commands */
> >  #define CMD_READ_ARRAY_SLOW          0x03
> >  #define CMD_READ_ARRAY_FAST          0x0b
> > @@ -231,6 +235,9 @@ int spi_flash_read_common(struct spi_flash *flash,
> > const u8 *cmd,  int spi_flash_cmd_read_ops(struct spi_flash *flash,
> > u32 offset,
> >               size_t len, void *data);
> >
> > +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> > +             int enable, u8 idcode0);
> > +
> >  #ifdef CONFIG_SPI_FLASH_MTD
> >  int spi_flash_mtd_register(struct spi_flash *flash);  void
> > spi_flash_mtd_unregister(void); diff --git a/drivers/mtd/spi/sf_ops.c
> > b/drivers/mtd/spi/sf_ops.c index deebcab..de30c55 100644
> > --- a/drivers/mtd/spi/sf_ops.c
> > +++ b/drivers/mtd/spi/sf_ops.c
> > @@ -93,6 +93,46 @@ int spi_flash_cmd_write_config(struct spi_flash
> > *flash,
> > u8 wc)  }  #endif
> >
> > +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> > +                             int enable, u8 idcode0) {
> > +     int ret;
> > +     u8 cmd, bar;
> > +     bool need_wren = false;
> > +
> > +     ret = spi_claim_bus(flash->spi);
> > +     if (ret) {
> > +             debug("SF: unable to claim SPI bus\n");
> > +             return ret;
> > +     }
> > +
> > +     switch (idcode0) {
> > +     case SPI_FLASH_CFI_MFR_STMICRO:
> > +             /* Some Micron need WREN command; all will accept it */
> > +             need_wren = true;
> > +     case SPI_FLASH_CFI_MFR_MACRONIX:
> > +     case SPI_FLASH_CFI_MFR_WINBOND:
> > +             if (need_wren)
> > +                     spi_flash_cmd_write_enable(flash);
> > +
> > +             cmd = enable ? CMD_ENTER_4B_ADDR :
> > CMD_EXIT_4B_ADDR;
> > +             ret = spi_flash_cmd(flash->spi, cmd, NULL, 0);
> > +             if (need_wren)
> > +                     spi_flash_cmd_write_disable(flash);
> > +
> > +             break;
> > +     default:
> > +             /* Spansion style */
> > +             bar = enable << 7;
> > +             cmd = CMD_BANKADDR_BRWR;
> > +             ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &bar, 1);
> > +     }
> > +
> > +     spi_release_bus(flash->spi);
> > +
> > +     return ret;
> > +}
> > +
> >  #ifdef CONFIG_SPI_FLASH_BAR
> >  static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8
> > bank_sel)  { diff --git a/drivers/mtd/spi/sf_probe.c
> > b/drivers/mtd/spi/sf_probe.c index e0283dc..3b204f8 100644
> > --- a/drivers/mtd/spi/sf_probe.c
> > +++ b/drivers/mtd/spi/sf_probe.c
> > @@ -170,6 +170,16 @@ static int spi_flash_validate_params(struct
> > spi_slave *spi, u8 *idcode,
> >       flash->page_size <<= flash->shift;
> >       flash->sector_size = params->sector_size << flash->shift;
> >       flash->size = flash->sector_size * params->nr_sectors <<
> > flash->shift;
> > +
> > +     /*
> > +      * So far, the 4-byte address mode haven't been supported in U-
> > Boot,
> > +      * and make sure the chip (> 16MiB) in default 3-byte address
> mode,
> > +      * in case of warm bootup, the chip was set to 4-byte mode in
> kernel.
> > +      */
> > +     if (flash->size > SPI_FLASH_16MB_BOUN) {
> I think that this check should be as below if (flash->size >
> (SPI_FLASH_16MB_BOUN << flash->shift)
> 

Sorry, I don't know the DAUL memories, but I find the condition initializing
the BAR read/write commands to be checked without the shift.

Jagannadha, Can you help to confirm if the shift is needed here? 

> Regards,
> Siva
> 
> > +             if (spi_flash_cmd_4B_addr_switch(flash, false,
> > + idcode[0]) <
> > 0)
> > +                     debug("SF: enter 3B address mode failed\n");
> > +     }
> >  #ifdef CONFIG_SF_DUAL_FLASH
> >       if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> >               flash->size <<= 1;
> > --
> > 2.1.0.27.g96db324
> 
> 
> 
> This email and any attachments are intended for the sole use of the named
> recipient(s) and contain(s) confidential information that may be
> proprietary, privileged or copyrighted under applicable law. If you are
> not the intended recipient, do not read, copy, or forward this email
> message or any attachments. Delete this email message and any attachments
> immediately.

B.R
Zhiqiang

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Jagan Teki]

On 13 August 2015 at 17:23, Stefan Roese <sr@denx.de> wrote:
> Jagan,
>
> On 13.08.2015 13:45, Jagan Teki wrote:
>>>>
>>>> Please correct me if I'm wrong, but AFAIU this BAR thing
>>>> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB SPI flash
>>>> contiguously. Only 16MiB areas. So for example its not possible to put
>>>> UBI/UBIFS in such a big partition.
>>
>>
>> Stefan,
>>
>> No, BAR is accessing flash > 16MiB in 3-byte addressing mode, for your
>> example of
>> 64MiB flash, it can grouped into 4 16MiB regions means 4 bank vales
>> bank0 to bank3
>>
>> Based on the sf read/erase/write flash offsets, that particular bank
>> will enable an do
>> the relevant operations.
>>
>> In-spite of having 4 byte addressing operations BAR should do exactly
>> same with existing
>> 3-byte addressing.
>
>
> Okay, thanks for the explanation. If this really is the case, and this BAR
> support will seamlessly enable the access to the complete flash area, then
> the 4-byte mode should really not be necessary.
>
> I'm wondering though about 2 things:
>
> a) Do all SPI NOR flash chips support this BAR mode?

Based on my experience, Micron, Winbond, Spansion and Macronix support these.
Spansion call these as bank address register and rest are call it as
Extended address register.

> b) If yes, why isn't BAR enabled per default?

Bcz, of code size and the moment where I have added this support where
very few flashes which
are greater than 16MiB.

>
> c) Why doesn't the Linux kernel use this BAR mode?

The only overhead with BAR as per the coding is concerns was, We need write
the bar/extended address register based on the user input offset. that
means flash ops
like erase/read/write loop we need to check whether user is asking
which bar then write
that particular bar so-that flash region got changed for operations.

In-case of 4-byte addressing, probe will identify the addr_width and
update the flash operation
commands and flash ops will operate accordingly.

This may be the reason for Linux is not adding BAR and relied on
4-byte instead.
Probably I will add it on Linux in future, but if we have any choose
of using one-at-a-time.

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Jagan Teki]

Hi Siva,

On 13 August 2015 at 18:18, Siva Durga Prasad Paladugu
<siva.durga.paladugu@xilinx.com> wrote:
>> >> >> Hi Zhejiang/Jagan,
>> >> >>
>> >> >> I think it would be good to extend this further to support 4-byte
>> >> >> addressing in u-boot also.
>> >> >> This would be based on the driver, We can get the data that
>> >> >> whether the controller supports 4-byte or not from the driver
>> >> >> level(through slave
>> >> >> struct) and enable 4 byte addressing here based on that.
>> >> >>
>> >> >
>> >> > That is a long way, and I don't think it is necessary for U-Boot.
>> >> > The U-Boot work well using BAR to address more than 16MiB flash.
>> >> > This patch focus on the address mode issue when warm boot up base
>> >> > on the BAR address mode.
>> >>
>> >> Please correct me if I'm wrong, but AFAIU this BAR thing
>> >> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB SPI
>> >> flash contiguously. Only 16MiB areas. So for example its not possible
>> >> to put UBI/UBIFS in such a big partition.
>>
>> Stefan,
>>
>> No, BAR is accessing flash > 16MiB in 3-byte addressing mode, for your
>> example of 64MiB flash, it can grouped into 4 16MiB regions means 4 bank
>> vales
>> bank0 to bank3
>>
>> Based on the sf read/erase/write flash offsets, that particular bank will
>> enable an do the relevant operations.
>>
>> In-spite of having 4 byte addressing operations BAR should do exactly same
>> with existing 3-byte addressing.
>>
>> >>
>> >> I have to support Siva here. We really should try to support this
>> >> 4-byte mode correctly. This can't be too hard.
>> >
>> > Yeah, I think it wouldn't be too difficult to add support for 4-byte address.
>> > we may just need to bypass the SPI_FLASH_BAR in read_ops , write_ops
>> and erase_ops routines.
>> >
>> > This support need not be part of this series. It can be a separate
>> > series. As we already added routine to disable /enable 4 byte as a part of
>> this, I just shared that it would be good to have 4 byte support also.
>>
>> Siva,
>>
>> Agree that adding 4-byte addressing is not too difficult, but by passing BAR is
>> not a good idea, what if you have a flash with > 16 MiB and controller have 3-
>> byte addressing command support.
>
> Yes, if you look at my first mail on this, I mentioned, we should get that info somehow from the driver of
> the respective controller, whether it supports four byte or not. For example from the spi_slave struct.
> Here by-passing means that if controller supports four byte then only we should bypass that BAR otherwise proceed
> with BAR.

So from your points, you need both BAR and 4-byte addressing need to have on sf,
is that true? If yes some how controller will inform to sf which one can use?

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Siva Durga Prasad Paladugu]

Hi Jagan,

> -----Original Message-----
> From: Jagan Teki [mailto:jteki at openedev.com]
> Sent: Monday, August 17, 2015 1:43 PM
> To: Siva Durga Prasad Paladugu
> Cc: Hou Zhiqiang; Stefan Roese; nofooter; York Sun; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> mode
> 
> Hi Siva,
> 
> On 13 August 2015 at 18:18, Siva Durga Prasad Paladugu
> <siva.durga.paladugu@xilinx.com> wrote:
> >> >> >> Hi Zhejiang/Jagan,
> >> >> >>
> >> >> >> I think it would be good to extend this further to support
> >> >> >> 4-byte addressing in u-boot also.
> >> >> >> This would be based on the driver, We can get the data that
> >> >> >> whether the controller supports 4-byte or not from the driver
> >> >> >> level(through slave
> >> >> >> struct) and enable 4 byte addressing here based on that.
> >> >> >>
> >> >> >
> >> >> > That is a long way, and I don't think it is necessary for U-Boot.
> >> >> > The U-Boot work well using BAR to address more than 16MiB flash.
> >> >> > This patch focus on the address mode issue when warm boot up
> >> >> > base on the BAR address mode.
> >> >>
> >> >> Please correct me if I'm wrong, but AFAIU this BAR thing
> >> >> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB SPI
> >> >> flash contiguously. Only 16MiB areas. So for example its not
> >> >> possible to put UBI/UBIFS in such a big partition.
> >>
> >> Stefan,
> >>
> >> No, BAR is accessing flash > 16MiB in 3-byte addressing mode, for
> >> your example of 64MiB flash, it can grouped into 4 16MiB regions
> >> means 4 bank vales
> >> bank0 to bank3
> >>
> >> Based on the sf read/erase/write flash offsets, that particular bank
> >> will enable an do the relevant operations.
> >>
> >> In-spite of having 4 byte addressing operations BAR should do exactly
> >> same with existing 3-byte addressing.
> >>
> >> >>
> >> >> I have to support Siva here. We really should try to support this
> >> >> 4-byte mode correctly. This can't be too hard.
> >> >
> >> > Yeah, I think it wouldn't be too difficult to add support for 4-byte
> address.
> >> > we may just need to bypass the SPI_FLASH_BAR in read_ops ,
> >> > write_ops
> >> and erase_ops routines.
> >> >
> >> > This support need not be part of this series. It can be a separate
> >> > series. As we already added routine to disable /enable 4 byte as a
> >> > part of
> >> this, I just shared that it would be good to have 4 byte support also.
> >>
> >> Siva,
> >>
> >> Agree that adding 4-byte addressing is not too difficult, but by
> >> passing BAR is not a good idea, what if you have a flash with > 16
> >> MiB and controller have 3- byte addressing command support.
> >
> > Yes, if you look at my first mail on this, I mentioned, we should get
> > that info somehow from the driver of the respective controller, whether it
> supports four byte or not. For example from the spi_slave struct.
> > Here by-passing means that if controller supports four byte then only
> > we should bypass that BAR otherwise proceed with BAR.
> 
> So from your points, you need both BAR and 4-byte addressing need to have
> on sf, is that true? If yes some how controller will inform to sf which one can
> use?
Yes, we need both because some controllers won't support 4-byte addressing. 
This information we can get it from controller driver code or from device tree.
This would be same as slave->dual which we are getting now from driver code.

Regards,
Siva
> 
> thanks!
> --
> Jagan | openedev.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Jagan Teki]

On 17 August 2015 at 14:26, Siva Durga Prasad Paladugu
<siva.durga.paladugu@xilinx.com> wrote:
> Hi Jagan,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jteki at openedev.com]
>> Sent: Monday, August 17, 2015 1:43 PM
>> To: Siva Durga Prasad Paladugu
>> Cc: Hou Zhiqiang; Stefan Roese; nofooter; York Sun; u-boot at lists.denx.de
>> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
>> mode
>>
>> Hi Siva,
>>
>> On 13 August 2015 at 18:18, Siva Durga Prasad Paladugu
>> <siva.durga.paladugu@xilinx.com> wrote:
>> >> >> >> Hi Zhejiang/Jagan,
>> >> >> >>
>> >> >> >> I think it would be good to extend this further to support
>> >> >> >> 4-byte addressing in u-boot also.
>> >> >> >> This would be based on the driver, We can get the data that
>> >> >> >> whether the controller supports 4-byte or not from the driver
>> >> >> >> level(through slave
>> >> >> >> struct) and enable 4 byte addressing here based on that.
>> >> >> >>
>> >> >> >
>> >> >> > That is a long way, and I don't think it is necessary for U-Boot.
>> >> >> > The U-Boot work well using BAR to address more than 16MiB flash.
>> >> >> > This patch focus on the address mode issue when warm boot up
>> >> >> > base on the BAR address mode.
>> >> >>
>> >> >> Please correct me if I'm wrong, but AFAIU this BAR thing
>> >> >> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB SPI
>> >> >> flash contiguously. Only 16MiB areas. So for example its not
>> >> >> possible to put UBI/UBIFS in such a big partition.
>> >>
>> >> Stefan,
>> >>
>> >> No, BAR is accessing flash > 16MiB in 3-byte addressing mode, for
>> >> your example of 64MiB flash, it can grouped into 4 16MiB regions
>> >> means 4 bank vales
>> >> bank0 to bank3
>> >>
>> >> Based on the sf read/erase/write flash offsets, that particular bank
>> >> will enable an do the relevant operations.
>> >>
>> >> In-spite of having 4 byte addressing operations BAR should do exactly
>> >> same with existing 3-byte addressing.
>> >>
>> >> >>
>> >> >> I have to support Siva here. We really should try to support this
>> >> >> 4-byte mode correctly. This can't be too hard.
>> >> >
>> >> > Yeah, I think it wouldn't be too difficult to add support for 4-byte
>> address.
>> >> > we may just need to bypass the SPI_FLASH_BAR in read_ops ,
>> >> > write_ops
>> >> and erase_ops routines.
>> >> >
>> >> > This support need not be part of this series. It can be a separate
>> >> > series. As we already added routine to disable /enable 4 byte as a
>> >> > part of
>> >> this, I just shared that it would be good to have 4 byte support also.
>> >>
>> >> Siva,
>> >>
>> >> Agree that adding 4-byte addressing is not too difficult, but by
>> >> passing BAR is not a good idea, what if you have a flash with > 16
>> >> MiB and controller have 3- byte addressing command support.
>> >
>> > Yes, if you look at my first mail on this, I mentioned, we should get
>> > that info somehow from the driver of the respective controller, whether it
>> supports four byte or not. For example from the spi_slave struct.
>> > Here by-passing means that if controller supports four byte then only
>> > we should bypass that BAR otherwise proceed with BAR.
>>
>> So from your points, you need both BAR and 4-byte addressing need to have
>> on sf, is that true? If yes some how controller will inform to sf which one can
>> use?
> Yes, we need both because some controllers won't support 4-byte addressing.

Does a controller exist like that, means not supporting 4-byte
addressing but may
connect flash +16MiB?

> This information we can get it from controller driver code or from device tree.
> This would be same as slave->dual which we are getting now from driver code.

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Siva Durga Prasad Paladugu]

Hi Jagan,

> -----Original Message-----
> From: Jagan Teki [mailto:jteki at openedev.com]
> Sent: Monday, August 17, 2015 2:42 PM
> To: Siva Durga Prasad Paladugu
> Cc: Stefan Roese; u-boot at lists.denx.de; Hou Zhiqiang; York Sun
> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> mode
>
> On 17 August 2015 at 14:26, Siva Durga Prasad Paladugu
> <siva.durga.paladugu@xilinx.com> wrote:
> > Hi Jagan,
> >
> >> -----Original Message-----
> >> From: Jagan Teki [mailto:jteki at openedev.com]
> >> Sent: Monday, August 17, 2015 1:43 PM
> >> To: Siva Durga Prasad Paladugu
> >> Cc: Hou Zhiqiang; Stefan Roese; nofooter; York Sun;
> >> u-boot at lists.denx.de
> >> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte
> >> address mode
> >>
> >> Hi Siva,
> >>
> >> On 13 August 2015 at 18:18, Siva Durga Prasad Paladugu
> >> <siva.durga.paladugu@xilinx.com> wrote:
> >> >> >> >> Hi Zhejiang/Jagan,
> >> >> >> >>
> >> >> >> >> I think it would be good to extend this further to support
> >> >> >> >> 4-byte addressing in u-boot also.
> >> >> >> >> This would be based on the driver, We can get the data that
> >> >> >> >> whether the controller supports 4-byte or not from the
> >> >> >> >> driver level(through slave
> >> >> >> >> struct) and enable 4 byte addressing here based on that.
> >> >> >> >>
> >> >> >> >
> >> >> >> > That is a long way, and I don't think it is necessary for U-Boot.
> >> >> >> > The U-Boot work well using BAR to address more than 16MiB
> flash.
> >> >> >> > This patch focus on the address mode issue when warm boot up
> >> >> >> > base on the BAR address mode.
> >> >> >>
> >> >> >> Please correct me if I'm wrong, but AFAIU this BAR thing
> >> >> >> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB
> >> >> >> SPI flash contiguously. Only 16MiB areas. So for example its
> >> >> >> not possible to put UBI/UBIFS in such a big partition.
> >> >>
> >> >> Stefan,
> >> >>
> >> >> No, BAR is accessing flash > 16MiB in 3-byte addressing mode, for
> >> >> your example of 64MiB flash, it can grouped into 4 16MiB regions
> >> >> means 4 bank vales
> >> >> bank0 to bank3
> >> >>
> >> >> Based on the sf read/erase/write flash offsets, that particular
> >> >> bank will enable an do the relevant operations.
> >> >>
> >> >> In-spite of having 4 byte addressing operations BAR should do
> >> >> exactly same with existing 3-byte addressing.
> >> >>
> >> >> >>
> >> >> >> I have to support Siva here. We really should try to support
> >> >> >> this 4-byte mode correctly. This can't be too hard.
> >> >> >
> >> >> > Yeah, I think it wouldn't be too difficult to add support for
> >> >> > 4-byte
> >> address.
> >> >> > we may just need to bypass the SPI_FLASH_BAR in read_ops ,
> >> >> > write_ops
> >> >> and erase_ops routines.
> >> >> >
> >> >> > This support need not be part of this series. It can be a
> >> >> > separate series. As we already added routine to disable /enable
> >> >> > 4 byte as a part of
> >> >> this, I just shared that it would be good to have 4 byte support also.
> >> >>
> >> >> Siva,
> >> >>
> >> >> Agree that adding 4-byte addressing is not too difficult, but by
> >> >> passing BAR is not a good idea, what if you have a flash with > 16
> >> >> MiB and controller have 3- byte addressing command support.
> >> >
> >> > Yes, if you look at my first mail on this, I mentioned, we should
> >> > get that info somehow from the driver of the respective controller,
> >> > whether it
> >> supports four byte or not. For example from the spi_slave struct.
> >> > Here by-passing means that if controller supports four byte then
> >> > only we should bypass that BAR otherwise proceed with BAR.
> >>
> >> So from your points, you need both BAR and 4-byte addressing need to
> >> have on sf, is that true? If yes some how controller will inform to
> >> sf which one can use?
> > Yes, we need both because some controllers won't support 4-byte
> addressing.
>
> Does a controller exist like that, means not supporting 4-byte addressing but
> may connect flash +16MiB?

Yes Jagan It's can exist. Zynq would be an example of one such case.

Regards,
DP
>
> > This information we can get it from controller driver code or from device
> tree.
> > This would be same as slave->dual which we are getting now from driver
> code.
>
> thanks!
> --
> Jagan | openedev.


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Jagan Teki]

On 17 August 2015 at 14:44, Siva Durga Prasad Paladugu
<siva.durga.paladugu@xilinx.com> wrote:
> Hi Jagan,
>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jteki at openedev.com]
>> Sent: Monday, August 17, 2015 2:42 PM
>> To: Siva Durga Prasad Paladugu
>> Cc: Stefan Roese; u-boot at lists.denx.de; Hou Zhiqiang; York Sun
>> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
>> mode
>>
>> On 17 August 2015 at 14:26, Siva Durga Prasad Paladugu
>> <siva.durga.paladugu@xilinx.com> wrote:
>> > Hi Jagan,
>> >
>> >> -----Original Message-----
>> >> From: Jagan Teki [mailto:jteki at openedev.com]
>> >> Sent: Monday, August 17, 2015 1:43 PM
>> >> To: Siva Durga Prasad Paladugu
>> >> Cc: Hou Zhiqiang; Stefan Roese; nofooter; York Sun;
>> >> u-boot at lists.denx.de
>> >> Subject: Re: [U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte
>> >> address mode
>> >>
>> >> Hi Siva,
>> >>
>> >> On 13 August 2015 at 18:18, Siva Durga Prasad Paladugu
>> >> <siva.durga.paladugu@xilinx.com> wrote:
>> >> >> >> >> Hi Zhejiang/Jagan,
>> >> >> >> >>
>> >> >> >> >> I think it would be good to extend this further to support
>> >> >> >> >> 4-byte addressing in u-boot also.
>> >> >> >> >> This would be based on the driver, We can get the data that
>> >> >> >> >> whether the controller supports 4-byte or not from the
>> >> >> >> >> driver level(through slave
>> >> >> >> >> struct) and enable 4 byte addressing here based on that.
>> >> >> >> >>
>> >> >> >> >
>> >> >> >> > That is a long way, and I don't think it is necessary for U-Boot.
>> >> >> >> > The U-Boot work well using BAR to address more than 16MiB
>> flash.
>> >> >> >> > This patch focus on the address mode issue when warm boot up
>> >> >> >> > base on the BAR address mode.
>> >> >> >>
>> >> >> >> Please correct me if I'm wrong, but AFAIU this BAR thing
>> >> >> >> (CONFIG_SPI_FLASH_BAR) doesn't support to address e.g. a 64MiB
>> >> >> >> SPI flash contiguously. Only 16MiB areas. So for example its
>> >> >> >> not possible to put UBI/UBIFS in such a big partition.
>> >> >>
>> >> >> Stefan,
>> >> >>
>> >> >> No, BAR is accessing flash > 16MiB in 3-byte addressing mode, for
>> >> >> your example of 64MiB flash, it can grouped into 4 16MiB regions
>> >> >> means 4 bank vales
>> >> >> bank0 to bank3
>> >> >>
>> >> >> Based on the sf read/erase/write flash offsets, that particular
>> >> >> bank will enable an do the relevant operations.
>> >> >>
>> >> >> In-spite of having 4 byte addressing operations BAR should do
>> >> >> exactly same with existing 3-byte addressing.
>> >> >>
>> >> >> >>
>> >> >> >> I have to support Siva here. We really should try to support
>> >> >> >> this 4-byte mode correctly. This can't be too hard.
>> >> >> >
>> >> >> > Yeah, I think it wouldn't be too difficult to add support for
>> >> >> > 4-byte
>> >> address.
>> >> >> > we may just need to bypass the SPI_FLASH_BAR in read_ops ,
>> >> >> > write_ops
>> >> >> and erase_ops routines.
>> >> >> >
>> >> >> > This support need not be part of this series. It can be a
>> >> >> > separate series. As we already added routine to disable /enable
>> >> >> > 4 byte as a part of
>> >> >> this, I just shared that it would be good to have 4 byte support also.
>> >> >>
>> >> >> Siva,
>> >> >>
>> >> >> Agree that adding 4-byte addressing is not too difficult, but by
>> >> >> passing BAR is not a good idea, what if you have a flash with > 16
>> >> >> MiB and controller have 3- byte addressing command support.
>> >> >
>> >> > Yes, if you look at my first mail on this, I mentioned, we should
>> >> > get that info somehow from the driver of the respective controller,
>> >> > whether it
>> >> supports four byte or not. For example from the spi_slave struct.
>> >> > Here by-passing means that if controller supports four byte then
>> >> > only we should bypass that BAR otherwise proceed with BAR.
>> >>
>> >> So from your points, you need both BAR and 4-byte addressing need to
>> >> have on sf, is that true? If yes some how controller will inform to
>> >> sf which one can use?
>> > Yes, we need both because some controllers won't support 4-byte
>> addressing.
>>
>> Does a controller exist like that, means not supporting 4-byte addressing but
>> may connect flash +16MiB?
>
> Yes Jagan It's can exist. Zynq would be an example of one such case.

OK, I thought you may refer other than zynq-qspi yes I too knew this.

Let me back again for this.

>> > This information we can get it from controller driver code or from device
>> tree.
>> > This would be same as slave->dual which we are getting now from driver
>> code.

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Hou Zhiqiang]

Hello Jagannadha and Siva,

Do you have any idea for if there should be a shift upon the flash size?
If yes, why there isn't one when initializing the BAR read/write commands?

> -----Original Message-----
> From: Hou Zhiqiang-B48286
> Sent: 2015?8?14? 18:02
> To: 'Siva Durga Prasad Paladugu'; u-boot at lists.denx.de;
> jteki at openedev.com; 'jaganna at xilinx.com'
> Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
> Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
> 
> Hi Jagannadha,
> 
> See Siva's comments inline and For the DUAL spi flash memories, do you
> have any comment?
> 
> > -----Original Message-----
> > From: Siva Durga Prasad Paladugu
> > [mailto:siva.durga.paladugu at xilinx.com]
> > Sent: 2015?8?14? 12:55
> > To: Hou Zhiqiang-B48286; u-boot at lists.denx.de; jteki at openedev.com
> > Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
> > Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> > mode
> >
> > Hi Zhiqiang,
> >
> > > -----Original Message-----
> > > From: Zhiqiang Hou [mailto:B48286 at freescale.com]
> > > Sent: Wednesday, August 12, 2015 11:48 AM
> > > To: u-boot at lists.denx.de; jteki at openedev.com
> > > Cc: yorksun at freescale.com; Mingkai.Hu at freescale.com; Siva Durga
> > > Prasad Paladugu; Hou Zhiqiang
> > > Subject: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
> > >
> > > From: Hou Zhiqiang <B48286@freescale.com>
> > >
> > > For more than 16MiB SPI flash chips, there are 3-Byte and 4-Byte
> > > address mode, and only the 3-Byte address mode is supported in
> > > U-Boot
> > so far.
> > > So, reset the SPI flash to 3-Byte address mode in probe to ensure
> > > the SPI flash work correctly, because it may has been set to 4-Byte
> > > address mode after warm boot.
> > >
> > > Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
> > > ---
> > > Tested on T1042RDB board.
> > > V6:
> > >     Add the spi_release_bus.
> > > V5:
> > >     1. Removed #ifdef for STMICRO.
> > >     2. Add support for Spansion chips (>16MiB) switch to 3-Byte
> > > address
> > mode.
> > > V4:
> > >     Split the the patch to 2 patches for clear FSR and SPI flash
> > address mode.
> > > V3:
> > >     Generate the patch based on the latest tree git://git.denx.de/u-
> > boot.git.
> > > V2:
> > >     Add the operation of enter 3 Byte address mode in probe.
> > > V1:
> > >     Based on git://git.denx.de/u-boot.git.
> > >  drivers/mtd/spi/sf_internal.h |  7 +++++++
> > >  drivers/mtd/spi/sf_ops.c      | 40
> > > ++++++++++++++++++++++++++++++++++++++++
> > >  drivers/mtd/spi/sf_probe.c    | 10 ++++++++++
> > >  3 files changed, 57 insertions(+)
> > >
> > > diff --git a/drivers/mtd/spi/sf_internal.h
> > > b/drivers/mtd/spi/sf_internal.h index 1de1dac..9519bd8 100644
> > > --- a/drivers/mtd/spi/sf_internal.h
> > > +++ b/drivers/mtd/spi/sf_internal.h
> > > @@ -75,6 +75,10 @@ enum {
> > >  #define CMD_FLAG_STATUS                      0x70
> > >  #define CMD_CLEAR_FLAG_STATUS                0x50
> > >
> > > +/* Used for Micron, Macronix and Winbond flashes */
> > > +#define      CMD_ENTER_4B_ADDR               0xB7
> > > +#define      CMD_EXIT_4B_ADDR                0xE9
> > > +
> > >  /* Read commands */
> > >  #define CMD_READ_ARRAY_SLOW          0x03
> > >  #define CMD_READ_ARRAY_FAST          0x0b
> > > @@ -231,6 +235,9 @@ int spi_flash_read_common(struct spi_flash
> > > *flash, const u8 *cmd,  int spi_flash_cmd_read_ops(struct spi_flash
> > > *flash,
> > > u32 offset,
> > >               size_t len, void *data);
> > >
> > > +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> > > +             int enable, u8 idcode0);
> > > +
> > >  #ifdef CONFIG_SPI_FLASH_MTD
> > >  int spi_flash_mtd_register(struct spi_flash *flash);  void
> > > spi_flash_mtd_unregister(void); diff --git
> > > a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index
> > > deebcab..de30c55 100644
> > > --- a/drivers/mtd/spi/sf_ops.c
> > > +++ b/drivers/mtd/spi/sf_ops.c
> > > @@ -93,6 +93,46 @@ int spi_flash_cmd_write_config(struct spi_flash
> > > *flash,
> > > u8 wc)  }  #endif
> > >
> > > +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> > > +                             int enable, u8 idcode0) {
> > > +     int ret;
> > > +     u8 cmd, bar;
> > > +     bool need_wren = false;
> > > +
> > > +     ret = spi_claim_bus(flash->spi);
> > > +     if (ret) {
> > > +             debug("SF: unable to claim SPI bus\n");
> > > +             return ret;
> > > +     }
> > > +
> > > +     switch (idcode0) {
> > > +     case SPI_FLASH_CFI_MFR_STMICRO:
> > > +             /* Some Micron need WREN command; all will accept it */
> > > +             need_wren = true;
> > > +     case SPI_FLASH_CFI_MFR_MACRONIX:
> > > +     case SPI_FLASH_CFI_MFR_WINBOND:
> > > +             if (need_wren)
> > > +                     spi_flash_cmd_write_enable(flash);
> > > +
> > > +             cmd = enable ? CMD_ENTER_4B_ADDR :
> > > CMD_EXIT_4B_ADDR;
> > > +             ret = spi_flash_cmd(flash->spi, cmd, NULL, 0);
> > > +             if (need_wren)
> > > +                     spi_flash_cmd_write_disable(flash);
> > > +
> > > +             break;
> > > +     default:
> > > +             /* Spansion style */
> > > +             bar = enable << 7;
> > > +             cmd = CMD_BANKADDR_BRWR;
> > > +             ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &bar, 1);
> > > +     }
> > > +
> > > +     spi_release_bus(flash->spi);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > >  #ifdef CONFIG_SPI_FLASH_BAR
> > >  static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8
> > > bank_sel)  { diff --git a/drivers/mtd/spi/sf_probe.c
> > > b/drivers/mtd/spi/sf_probe.c index e0283dc..3b204f8 100644
> > > --- a/drivers/mtd/spi/sf_probe.c
> > > +++ b/drivers/mtd/spi/sf_probe.c
> > > @@ -170,6 +170,16 @@ static int spi_flash_validate_params(struct
> > > spi_slave *spi, u8 *idcode,
> > >       flash->page_size <<= flash->shift;
> > >       flash->sector_size = params->sector_size << flash->shift;
> > >       flash->size = flash->sector_size * params->nr_sectors <<
> > > flash->shift;
> > > +
> > > +     /*
> > > +      * So far, the 4-byte address mode haven't been supported in
> > > + U-
> > > Boot,
> > > +      * and make sure the chip (> 16MiB) in default 3-byte address
> > mode,
> > > +      * in case of warm bootup, the chip was set to 4-byte mode in
> > kernel.
> > > +      */
> > > +     if (flash->size > SPI_FLASH_16MB_BOUN) {
> > I think that this check should be as below if (flash->size >
> > (SPI_FLASH_16MB_BOUN << flash->shift)
> >
> 
> Sorry, I don't know the DAUL memories, but I find the condition
> initializing the BAR read/write commands to be checked without the shift.
> 
> Jagannadha, Can you help to confirm if the shift is needed here?
> 
> > Regards,
> > Siva
> >
> > > +             if (spi_flash_cmd_4B_addr_switch(flash, false,
> > > + idcode[0]) <
> > > 0)
> > > +                     debug("SF: enter 3B address mode failed\n");
> > > +     }
> > >  #ifdef CONFIG_SF_DUAL_FLASH
> > >       if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> > >               flash->size <<= 1;
> > > --
> > > 2.1.0.27.g96db324
> >
> >
> >
> > This email and any attachments are intended for the sole use of the
> > named
> > recipient(s) and contain(s) confidential information that may be
> > proprietary, privileged or copyrighted under applicable law. If you
> > are not the intended recipient, do not read, copy, or forward this
> > email message or any attachments. Delete this email message and any
> > attachments immediately.
 
B.R
Zhiqiang

[U-Boot] [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode

[Zhiqiang Hou]

Hi Jagan,

Up to now, I didn't find the 4-Byte addressing mode support.
Do you know if anybody have schedule to add it?
if no, could you please apply this patch, due to as I have said  when do the warm 
boot up, the SPI flash will be kept in 4-Byte addressing mode that kernel driver set, 
while the u-boot doesn't support 4-Byte mode and assume it is in 3-Byte mode.
This mismatch make the u-boot spi driver working incorrectly. What the worse is 
it will crash when booting from SPI flash. So please take it into account.

Thanks,
Zhiqiang

> -----Original Message-----
> From: Hou Zhiqiang-B48286 [mailto:B48286 at freescale.com]
> Sent: 2015?8?19? 20:27
> To: Hou Zhiqiang-B48286 <B48286@freescale.com>; Siva Durga Prasad Paladugu
> <siva.durga.paladugu@xilinx.com>; u-boot at lists.denx.de; jteki at openedev.com;
> jaganna at xilinx.com
> Cc: Yusong Sun <yorksun@freescale.com>; Hu Mingkai-B21284
> <Mingkai.Hu@freescale.com>; nofooter <nofooter@xilinx.com>
> Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address mode
> 
> Hello Jagannadha and Siva,
> 
> Do you have any idea for if there should be a shift upon the flash size?
> If yes, why there isn't one when initializing the BAR read/write commands?
> 
> > -----Original Message-----
> > From: Hou Zhiqiang-B48286
> > Sent: 2015?8?14? 18:02
> > To: 'Siva Durga Prasad Paladugu'; u-boot at lists.denx.de;
> > jteki at openedev.com; 'jaganna at xilinx.com'
> > Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
> > Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> > mode
> >
> > Hi Jagannadha,
> >
> > See Siva's comments inline and For the DUAL spi flash memories, do you
> > have any comment?
> >
> > > -----Original Message-----
> > > From: Siva Durga Prasad Paladugu
> > > [mailto:siva.durga.paladugu at xilinx.com]
> > > Sent: 2015?8?14? 12:55
> > > To: Hou Zhiqiang-B48286; u-boot at lists.denx.de; jteki at openedev.com
> > > Cc: Sun York-R58495; Hu Mingkai-B21284; nofooter
> > > Subject: RE: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> > > mode
> > >
> > > Hi Zhiqiang,
> > >
> > > > -----Original Message-----
> > > > From: Zhiqiang Hou [mailto:B48286 at freescale.com]
> > > > Sent: Wednesday, August 12, 2015 11:48 AM
> > > > To: u-boot at lists.denx.de; jteki at openedev.com
> > > > Cc: yorksun at freescale.com; Mingkai.Hu at freescale.com; Siva Durga
> > > > Prasad Paladugu; Hou Zhiqiang
> > > > Subject: [PATCH V6] sf: Turn SPI flash chip into 3-Byte address
> > > > mode
> > > >
> > > > From: Hou Zhiqiang <B48286@freescale.com>
> > > >
> > > > For more than 16MiB SPI flash chips, there are 3-Byte and 4-Byte
> > > > address mode, and only the 3-Byte address mode is supported in
> > > > U-Boot
> > > so far.
> > > > So, reset the SPI flash to 3-Byte address mode in probe to ensure
> > > > the SPI flash work correctly, because it may has been set to
> > > > 4-Byte address mode after warm boot.
> > > >
> > > > Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
> > > > ---
> > > > Tested on T1042RDB board.
> > > > V6:
> > > >     Add the spi_release_bus.
> > > > V5:
> > > >     1. Removed #ifdef for STMICRO.
> > > >     2. Add support for Spansion chips (>16MiB) switch to 3-Byte
> > > > address
> > > mode.
> > > > V4:
> > > >     Split the the patch to 2 patches for clear FSR and SPI flash
> > > address mode.
> > > > V3:
> > > >     Generate the patch based on the latest tree
> > > > git://git.denx.de/u-
> > > boot.git.
> > > > V2:
> > > >     Add the operation of enter 3 Byte address mode in probe.
> > > > V1:
> > > >     Based on git://git.denx.de/u-boot.git.
> > > >  drivers/mtd/spi/sf_internal.h |  7 +++++++
> > > >  drivers/mtd/spi/sf_ops.c      | 40
> > > > ++++++++++++++++++++++++++++++++++++++++
> > > >  drivers/mtd/spi/sf_probe.c    | 10 ++++++++++
> > > >  3 files changed, 57 insertions(+)
> > > >
> > > > diff --git a/drivers/mtd/spi/sf_internal.h
> > > > b/drivers/mtd/spi/sf_internal.h index 1de1dac..9519bd8 100644
> > > > --- a/drivers/mtd/spi/sf_internal.h
> > > > +++ b/drivers/mtd/spi/sf_internal.h
> > > > @@ -75,6 +75,10 @@ enum {
> > > >  #define CMD_FLAG_STATUS                      0x70
> > > >  #define CMD_CLEAR_FLAG_STATUS                0x50
> > > >
> > > > +/* Used for Micron, Macronix and Winbond flashes */
> > > > +#define      CMD_ENTER_4B_ADDR               0xB7
> > > > +#define      CMD_EXIT_4B_ADDR                0xE9
> > > > +
> > > >  /* Read commands */
> > > >  #define CMD_READ_ARRAY_SLOW          0x03
> > > >  #define CMD_READ_ARRAY_FAST          0x0b
> > > > @@ -231,6 +235,9 @@ int spi_flash_read_common(struct spi_flash
> > > > *flash, const u8 *cmd,  int spi_flash_cmd_read_ops(struct
> > > > spi_flash *flash,
> > > > u32 offset,
> > > >               size_t len, void *data);
> > > >
> > > > +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> > > > +             int enable, u8 idcode0);
> > > > +
> > > >  #ifdef CONFIG_SPI_FLASH_MTD
> > > >  int spi_flash_mtd_register(struct spi_flash *flash);  void
> > > > spi_flash_mtd_unregister(void); diff --git
> > > > a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index
> > > > deebcab..de30c55 100644
> > > > --- a/drivers/mtd/spi/sf_ops.c
> > > > +++ b/drivers/mtd/spi/sf_ops.c
> > > > @@ -93,6 +93,46 @@ int spi_flash_cmd_write_config(struct spi_flash
> > > > *flash,
> > > > u8 wc)  }  #endif
> > > >
> > > > +int spi_flash_cmd_4B_addr_switch(struct spi_flash *flash,
> > > > +                             int enable, u8 idcode0) {
> > > > +     int ret;
> > > > +     u8 cmd, bar;
> > > > +     bool need_wren = false;
> > > > +
> > > > +     ret = spi_claim_bus(flash->spi);
> > > > +     if (ret) {
> > > > +             debug("SF: unable to claim SPI bus\n");
> > > > +             return ret;
> > > > +     }
> > > > +
> > > > +     switch (idcode0) {
> > > > +     case SPI_FLASH_CFI_MFR_STMICRO:
> > > > +             /* Some Micron need WREN command; all will accept it */
> > > > +             need_wren = true;
> > > > +     case SPI_FLASH_CFI_MFR_MACRONIX:
> > > > +     case SPI_FLASH_CFI_MFR_WINBOND:
> > > > +             if (need_wren)
> > > > +                     spi_flash_cmd_write_enable(flash);
> > > > +
> > > > +             cmd = enable ? CMD_ENTER_4B_ADDR :
> > > > CMD_EXIT_4B_ADDR;
> > > > +             ret = spi_flash_cmd(flash->spi, cmd, NULL, 0);
> > > > +             if (need_wren)
> > > > +                     spi_flash_cmd_write_disable(flash);
> > > > +
> > > > +             break;
> > > > +     default:
> > > > +             /* Spansion style */
> > > > +             bar = enable << 7;
> > > > +             cmd = CMD_BANKADDR_BRWR;
> > > > +             ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &bar, 1);
> > > > +     }
> > > > +
> > > > +     spi_release_bus(flash->spi);
> > > > +
> > > > +     return ret;
> > > > +}
> > > > +
> > > >  #ifdef CONFIG_SPI_FLASH_BAR
> > > >  static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash,
> > > > u8
> > > > bank_sel)  { diff --git a/drivers/mtd/spi/sf_probe.c
> > > > b/drivers/mtd/spi/sf_probe.c index e0283dc..3b204f8 100644
> > > > --- a/drivers/mtd/spi/sf_probe.c
> > > > +++ b/drivers/mtd/spi/sf_probe.c
> > > > @@ -170,6 +170,16 @@ static int spi_flash_validate_params(struct
> > > > spi_slave *spi, u8 *idcode,
> > > >       flash->page_size <<= flash->shift;
> > > >       flash->sector_size = params->sector_size << flash->shift;
> > > >       flash->size = flash->sector_size * params->nr_sectors <<
> > > > flash->shift;
> > > > +
> > > > +     /*
> > > > +      * So far, the 4-byte address mode haven't been supported in
> > > > + U-
> > > > Boot,
> > > > +      * and make sure the chip (> 16MiB) in default 3-byte
> > > > + address
> > > mode,
> > > > +      * in case of warm bootup, the chip was set to 4-byte mode
> > > > + in
> > > kernel.
> > > > +      */
> > > > +     if (flash->size > SPI_FLASH_16MB_BOUN) {
> > > I think that this check should be as below if (flash->size >
> > > (SPI_FLASH_16MB_BOUN << flash->shift)
> > >
> >
> > Sorry, I don't know the DAUL memories, but I find the condition
> > initializing the BAR read/write commands to be checked without the shift.
> >
> > Jagannadha, Can you help to confirm if the shift is needed here?
> >
> > > Regards,
> > > Siva
> > >
> > > > +             if (spi_flash_cmd_4B_addr_switch(flash, false,
> > > > + idcode[0]) <
> > > > 0)
> > > > +                     debug("SF: enter 3B address mode failed\n");
> > > > +     }
> > > >  #ifdef CONFIG_SF_DUAL_FLASH
> > > >       if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> > > >               flash->size <<= 1;
> > > > --
> > > > 2.1.0.27.g96db324
> > >
> > >
> > >
> > > This email and any attachments are intended for the sole use of the
> > > named
> > > recipient(s) and contain(s) confidential information that may be
> > > proprietary, privileged or copyrighted under applicable law. If you
> > > are not the intended recipient, do not read, copy, or forward this
> > > email message or any attachments. Delete this email message and any
> > > attachments immediately.
> 
> B.R
> Zhiqiang