[U-Boot] [PATCH 1/2] sf: Add Macronix MX25U6435F device parameters

[U-Boot] [PATCH 1/2] sf: Add Macronix MX25U6435F device parameters

[Bin Meng]

This adds support for Macronix flash MX25U6435F (device ID 0xc22537).

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/mtd/spi/spi_flash_ids.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/spi/spi_flash_ids.c b/drivers/mtd/spi/spi_flash_ids.c
index edca94e..c4ccf48 100644
--- a/drivers/mtd/spi/spi_flash_ids.c
+++ b/drivers/mtd/spi/spi_flash_ids.c
@@ -81,6 +81,7 @@ const struct spi_flash_info spi_flash_ids[] = {
 	{"mx25l12805",	   INFO(0xc22018, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
 	{"mx25l25635f",	   INFO(0xc22019, 0x0, 64 * 1024,   512, RD_FULL | WR_QPP) },
 	{"mx25l51235f",	   INFO(0xc2201a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
+	{"mx25u6435f",	   INFO(0xc22537, 0x0, 64 * 1024,   128, RD_FULL | WR_QPP) },
 	{"mx25l12855e",	   INFO(0xc22618, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
 	{"mx66u51235f",    INFO(0xc2253a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
 	{"mx66l1g45g",     INFO(0xc2201b, 0x0, 64 * 1024,  2048, RD_FULL | WR_QPP) },
-- 
2.9.2

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

On some flash (like Macronix), QE (quad enable) bit is in the same
status register as BP# bits, and we need preserve its original value
during a reboot cycle as this is required by some platforms (like
Intel ICH SPI controller working under descriptor mode).

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 0034a28..7d8c660 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
 	if (IS_ERR_OR_NULL(info))
 		return -ENOENT;
 
-	/* Flash powers up read-only, so clear BP# bits */
+	/*
+	 * Flash powers up read-only, so clear BP# bits.
+	 *
+	 * Note on some flash (like Macronix), QE (quad enable) bit is in the
+	 * same status register as BP# bits, and we need preserve its original
+	 * value during a reboot cycle as this is required by some platforms
+	 * (like Intel ICH SPI controller working under descriptor mode).
+	 */
 	if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
-	    JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
 	    JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
 		write_sr(flash, 0);
+	if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
+		u8 sr = 0;
+
+		read_sr(flash, &sr);
+		sr &= STATUS_QEB_MXIC;
+		write_sr(flash, sr);
+	}
 
 	flash->name = info->name;
 	flash->memory_map = spi->memory_map;
-- 
2.9.2

[U-Boot] [PATCH 1/2] sf: Add Macronix MX25U6435F device parameters

[Bin Meng]

Hi Jagan,

On Sun, Jul 23, 2017 at 10:44 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> This adds support for Macronix flash MX25U6435F (device ID 0xc22537).
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  drivers/mtd/spi/spi_flash_ids.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mtd/spi/spi_flash_ids.c b/drivers/mtd/spi/spi_flash_ids.c
> index edca94e..c4ccf48 100644
> --- a/drivers/mtd/spi/spi_flash_ids.c
> +++ b/drivers/mtd/spi/spi_flash_ids.c
> @@ -81,6 +81,7 @@ const struct spi_flash_info spi_flash_ids[] = {
>         {"mx25l12805",     INFO(0xc22018, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
>         {"mx25l25635f",    INFO(0xc22019, 0x0, 64 * 1024,   512, RD_FULL | WR_QPP) },
>         {"mx25l51235f",    INFO(0xc2201a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
> +       {"mx25u6435f",     INFO(0xc22537, 0x0, 64 * 1024,   128, RD_FULL | WR_QPP) },
>         {"mx25l12855e",    INFO(0xc22618, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
>         {"mx66u51235f",    INFO(0xc2253a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
>         {"mx66l1g45g",     INFO(0xc2201b, 0x0, 64 * 1024,  2048, RD_FULL | WR_QPP) },
> --

Ping.

Regards,
Bin

[U-Boot] [PATCH 1/2] sf: Add Macronix MX25U6435F device parameters

[Jagan Teki]

On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> This adds support for Macronix flash MX25U6435F (device ID 0xc22537).
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  drivers/mtd/spi/spi_flash_ids.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mtd/spi/spi_flash_ids.c b/drivers/mtd/spi/spi_flash_ids.c
> index edca94e..c4ccf48 100644
> --- a/drivers/mtd/spi/spi_flash_ids.c
> +++ b/drivers/mtd/spi/spi_flash_ids.c
> @@ -81,6 +81,7 @@ const struct spi_flash_info spi_flash_ids[] = {
>         {"mx25l12805",     INFO(0xc22018, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
>         {"mx25l25635f",    INFO(0xc22019, 0x0, 64 * 1024,   512, RD_FULL | WR_QPP) },
>         {"mx25l51235f",    INFO(0xc2201a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
> +       {"mx25u6435f",     INFO(0xc22537, 0x0, 64 * 1024,   128, RD_FULL | WR_QPP) },

Applied to u-boot-spi/master

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 1/2] sf: Add Macronix MX25U6435F device parameters

[Bin Meng]

Hi Jagan,

On Mon, Jul 31, 2017 at 5:33 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> This adds support for Macronix flash MX25U6435F (device ID 0xc22537).
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  drivers/mtd/spi/spi_flash_ids.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mtd/spi/spi_flash_ids.c b/drivers/mtd/spi/spi_flash_ids.c
>> index edca94e..c4ccf48 100644
>> --- a/drivers/mtd/spi/spi_flash_ids.c
>> +++ b/drivers/mtd/spi/spi_flash_ids.c
>> @@ -81,6 +81,7 @@ const struct spi_flash_info spi_flash_ids[] = {
>>         {"mx25l12805",     INFO(0xc22018, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
>>         {"mx25l25635f",    INFO(0xc22019, 0x0, 64 * 1024,   512, RD_FULL | WR_QPP) },
>>         {"mx25l51235f",    INFO(0xc2201a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
>> +       {"mx25u6435f",     INFO(0xc22537, 0x0, 64 * 1024,   128, RD_FULL | WR_QPP) },
>
> Applied to u-boot-spi/master

What about the 2nd patch [1] in this series?

And I don't see this patch showing in
http://git.denx.de/?p=u-boot/u-boot-spi.git;a=summary. Am I missing
anything?

[1] http://patchwork.ozlabs.org/patch/792565/

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Simon Glass]

On 23 July 2017 at 08:44, Bin Meng <bmeng.cn@gmail.com> wrote:
> On some flash (like Macronix), QE (quad enable) bit is in the same
> status register as BP# bits, and we need preserve its original value
> during a reboot cycle as this is required by some platforms (like
> Intel ICH SPI controller working under descriptor mode).
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 1/2] sf: Add Macronix MX25U6435F device parameters

[Simon Glass]

On 23 July 2017 at 08:44, Bin Meng <bmeng.cn@gmail.com> wrote:
> This adds support for Macronix flash MX25U6435F (device ID 0xc22537).
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  drivers/mtd/spi/spi_flash_ids.c | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Jagan Teki]

On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On some flash (like Macronix), QE (quad enable) bit is in the same
> status register as BP# bits, and we need preserve its original value
> during a reboot cycle as this is required by some platforms (like
> Intel ICH SPI controller working under descriptor mode).
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 0034a28..7d8c660 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>         if (IS_ERR_OR_NULL(info))
>                 return -ENOENT;
>
> -       /* Flash powers up read-only, so clear BP# bits */
> +       /*
> +        * Flash powers up read-only, so clear BP# bits.
> +        *
> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
> +        * same status register as BP# bits, and we need preserve its original
> +        * value during a reboot cycle as this is required by some platforms
> +        * (like Intel ICH SPI controller working under descriptor mode).
> +        */
>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>                 write_sr(flash, 0);
> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
> +               u8 sr = 0;
> +
> +               read_sr(flash, &sr);
> +               sr &= STATUS_QEB_MXIC;
> +               write_sr(flash, sr);
> +       }

It doesn't make sense to have one(specific) controller fix to be
generic to all macronix chips, think about alternative.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

Hi Jagan,

On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> On some flash (like Macronix), QE (quad enable) bit is in the same
>> status register as BP# bits, and we need preserve its original value
>> during a reboot cycle as this is required by some platforms (like
>> Intel ICH SPI controller working under descriptor mode).
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>> index 0034a28..7d8c660 100644
>> --- a/drivers/mtd/spi/spi_flash.c
>> +++ b/drivers/mtd/spi/spi_flash.c
>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>         if (IS_ERR_OR_NULL(info))
>>                 return -ENOENT;
>>
>> -       /* Flash powers up read-only, so clear BP# bits */
>> +       /*
>> +        * Flash powers up read-only, so clear BP# bits.
>> +        *
>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>> +        * same status register as BP# bits, and we need preserve its original
>> +        * value during a reboot cycle as this is required by some platforms
>> +        * (like Intel ICH SPI controller working under descriptor mode).
>> +        */
>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>                 write_sr(flash, 0);
>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>> +               u8 sr = 0;
>> +
>> +               read_sr(flash, &sr);
>> +               sr &= STATUS_QEB_MXIC;
>> +               write_sr(flash, sr);
>> +       }
>
> It doesn't make sense to have one(specific) controller fix to be
> generic to all macronix chips, think about alternative.
>

This is no way to fix at the controller level. Actually this is
nothing related to controller level. It's just the bootstrap settings
(QE bit in this case) that cannot be overwritten by someone else
(although it's seen on Intel, it might happen on some other
architecture). The logic in the codes are having issues. Its comment
says: clear BP# bits, but it clears QE bit for Macronix flash as well,
which is wrong. The update was just to make sure the codes do as what
its comment says.

If you have any other alternative, please suggest.

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

On Wed, Aug 2, 2017 at 6:26 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>> status register as BP# bits, and we need preserve its original value
>>> during a reboot cycle as this is required by some platforms (like
>>> Intel ICH SPI controller working under descriptor mode).
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>>
>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>> index 0034a28..7d8c660 100644
>>> --- a/drivers/mtd/spi/spi_flash.c
>>> +++ b/drivers/mtd/spi/spi_flash.c
>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>         if (IS_ERR_OR_NULL(info))
>>>                 return -ENOENT;
>>>
>>> -       /* Flash powers up read-only, so clear BP# bits */
>>> +       /*
>>> +        * Flash powers up read-only, so clear BP# bits.
>>> +        *
>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>> +        * same status register as BP# bits, and we need preserve its original
>>> +        * value during a reboot cycle as this is required by some platforms
>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>> +        */
>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>                 write_sr(flash, 0);
>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>> +               u8 sr = 0;
>>> +
>>> +               read_sr(flash, &sr);
>>> +               sr &= STATUS_QEB_MXIC;
>>> +               write_sr(flash, sr);
>>> +       }
>>
>> It doesn't make sense to have one(specific) controller fix to be
>> generic to all macronix chips, think about alternative.
>>
>
> This is no way to fix at the controller level. Actually this is
> nothing related to controller level. It's just the bootstrap settings
> (QE bit in this case) that cannot be overwritten by someone else
> (although it's seen on Intel, it might happen on some other
> architecture). The logic in the codes are having issues. Its comment
> says: clear BP# bits, but it clears QE bit for Macronix flash as well,
> which is wrong. The update was just to make sure the codes do as what
> its comment says.
>
> If you have any other alternative, please suggest.
>

Ping again..

Can you please comment on this? I would like this patch gets in the
upcoming release.

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

On Fri, Aug 4, 2017 at 12:21 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Wed, Aug 2, 2017 at 6:26 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Jagan,
>>
>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>> status register as BP# bits, and we need preserve its original value
>>>> during a reboot cycle as this is required by some platforms (like
>>>> Intel ICH SPI controller working under descriptor mode).
>>>>
>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>> ---
>>>>
>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>> index 0034a28..7d8c660 100644
>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>         if (IS_ERR_OR_NULL(info))
>>>>                 return -ENOENT;
>>>>
>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>> +       /*
>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>> +        *
>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>> +        * same status register as BP# bits, and we need preserve its original
>>>> +        * value during a reboot cycle as this is required by some platforms
>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>> +        */
>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>                 write_sr(flash, 0);
>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>> +               u8 sr = 0;
>>>> +
>>>> +               read_sr(flash, &sr);
>>>> +               sr &= STATUS_QEB_MXIC;
>>>> +               write_sr(flash, sr);
>>>> +       }
>>>
>>> It doesn't make sense to have one(specific) controller fix to be
>>> generic to all macronix chips, think about alternative.
>>>
>>
>> This is no way to fix at the controller level. Actually this is
>> nothing related to controller level. It's just the bootstrap settings
>> (QE bit in this case) that cannot be overwritten by someone else
>> (although it's seen on Intel, it might happen on some other
>> architecture). The logic in the codes are having issues. Its comment
>> says: clear BP# bits, but it clears QE bit for Macronix flash as well,
>> which is wrong. The update was just to make sure the codes do as what
>> its comment says.
>>
>> If you have any other alternative, please suggest.
>>
>
> Ping again..
>
> Can you please comment on this? I would like this patch gets in the
> upcoming release.
>

Ping ..

[U-Boot] [PATCH 1/2] sf: Add Macronix MX25U6435F device parameters

[Bin Meng]

Hi Jagan,

On Mon, Jul 31, 2017 at 7:33 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Jul 31, 2017 at 5:33 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> This adds support for Macronix flash MX25U6435F (device ID 0xc22537).
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>>
>>>  drivers/mtd/spi/spi_flash_ids.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mtd/spi/spi_flash_ids.c b/drivers/mtd/spi/spi_flash_ids.c
>>> index edca94e..c4ccf48 100644
>>> --- a/drivers/mtd/spi/spi_flash_ids.c
>>> +++ b/drivers/mtd/spi/spi_flash_ids.c
>>> @@ -81,6 +81,7 @@ const struct spi_flash_info spi_flash_ids[] = {
>>>         {"mx25l12805",     INFO(0xc22018, 0x0, 64 * 1024,   256, RD_FULL | WR_QPP) },
>>>         {"mx25l25635f",    INFO(0xc22019, 0x0, 64 * 1024,   512, RD_FULL | WR_QPP) },
>>>         {"mx25l51235f",    INFO(0xc2201a, 0x0, 64 * 1024,  1024, RD_FULL | WR_QPP) },
>>> +       {"mx25u6435f",     INFO(0xc22537, 0x0, 64 * 1024,   128, RD_FULL | WR_QPP) },
>>
>> Applied to u-boot-spi/master
>

I don't see this patch gets applied anywhere. Can you please make sure
the patch state is consistent when you reply the email?

> What about the 2nd patch [1] in this series?
>
> And I don't see this patch showing in
> http://git.denx.de/?p=u-boot/u-boot-spi.git;a=summary. Am I missing
> anything?
>
> [1] http://patchwork.ozlabs.org/patch/792565/

Also I don't see your response for the 2nd patch for some days.

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Jagan Teki]

Hi Bing,

On Mon, Aug 7, 2017 at 1:09 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Fri, Aug 4, 2017 at 12:21 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> On Wed, Aug 2, 2017 at 6:26 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>> status register as BP# bits, and we need preserve its original value
>>>>> during a reboot cycle as this is required by some platforms (like
>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>
>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>> ---
>>>>>
>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>> index 0034a28..7d8c660 100644
>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>                 return -ENOENT;
>>>>>
>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>> +       /*
>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>> +        *
>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>> +        */
>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>                 write_sr(flash, 0);
>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>> +               u8 sr = 0;
>>>>> +
>>>>> +               read_sr(flash, &sr);
>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>> +               write_sr(flash, sr);
>>>>> +       }
>>>>
>>>> It doesn't make sense to have one(specific) controller fix to be
>>>> generic to all macronix chips, think about alternative.
>>>>
>>>
>>> This is no way to fix at the controller level. Actually this is
>>> nothing related to controller level. It's just the bootstrap settings
>>> (QE bit in this case) that cannot be overwritten by someone else
>>> (although it's seen on Intel, it might happen on some other
>>> architecture). The logic in the codes are having issues. Its comment
>>> says: clear BP# bits, but it clears QE bit for Macronix flash as well,
>>> which is wrong. The update was just to make sure the codes do as what
>>> its comment says.
>>>
>>> If you have any other alternative, please suggest.
>>>
>>
>> Ping again..

Wait for sometime, I've queue that I need to review-it and respond
accordingly patches with latest may take some time. And commenting
yes will respond soon.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

On Mon, Aug 7, 2017 at 3:54 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> Hi Bing,
>
> On Mon, Aug 7, 2017 at 1:09 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> On Fri, Aug 4, 2017 at 12:21 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> On Wed, Aug 2, 2017 at 6:26 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> Hi Jagan,
>>>>
>>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>>> status register as BP# bits, and we need preserve its original value
>>>>>> during a reboot cycle as this is required by some platforms (like
>>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>>
>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>> ---
>>>>>>
>>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>>> index 0034a28..7d8c660 100644
>>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>>                 return -ENOENT;
>>>>>>
>>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>>> +       /*
>>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>>> +        *
>>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>>> +        */
>>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>>                 write_sr(flash, 0);
>>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>>> +               u8 sr = 0;
>>>>>> +
>>>>>> +               read_sr(flash, &sr);
>>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>>> +               write_sr(flash, sr);
>>>>>> +       }
>>>>>
>>>>> It doesn't make sense to have one(specific) controller fix to be
>>>>> generic to all macronix chips, think about alternative.
>>>>>
>>>>
>>>> This is no way to fix at the controller level. Actually this is
>>>> nothing related to controller level. It's just the bootstrap settings
>>>> (QE bit in this case) that cannot be overwritten by someone else
>>>> (although it's seen on Intel, it might happen on some other
>>>> architecture). The logic in the codes are having issues. Its comment
>>>> says: clear BP# bits, but it clears QE bit for Macronix flash as well,
>>>> which is wrong. The update was just to make sure the codes do as what
>>>> its comment says.
>>>>
>>>> If you have any other alternative, please suggest.
>>>>
>>>
>>> Ping again..
>
> Wait for sometime, I've queue that I need to review-it and respond
> accordingly patches with latest may take some time. And commenting
> yes will respond soon.
>

Ping!

Can you please respond with a reasonable time frame (ie: when you will
have time to look at this)?

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Jagan Teki]

Hi Bin,

On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>> status register as BP# bits, and we need preserve its original value
>>> during a reboot cycle as this is required by some platforms (like
>>> Intel ICH SPI controller working under descriptor mode).
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>>
>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>> index 0034a28..7d8c660 100644
>>> --- a/drivers/mtd/spi/spi_flash.c
>>> +++ b/drivers/mtd/spi/spi_flash.c
>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>         if (IS_ERR_OR_NULL(info))
>>>                 return -ENOENT;
>>>
>>> -       /* Flash powers up read-only, so clear BP# bits */
>>> +       /*
>>> +        * Flash powers up read-only, so clear BP# bits.
>>> +        *
>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>> +        * same status register as BP# bits, and we need preserve its original
>>> +        * value during a reboot cycle as this is required by some platforms
>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>> +        */
>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>                 write_sr(flash, 0);
>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>> +               u8 sr = 0;
>>> +
>>> +               read_sr(flash, &sr);
>>> +               sr &= STATUS_QEB_MXIC;
>>> +               write_sr(flash, sr);
>>> +       }
>>
>> It doesn't make sense to have one(specific) controller fix to be
>> generic to all macronix chips, think about alternative.
>>
>
> This is no way to fix at the controller level. Actually this is
> nothing related to controller level. It's just the bootstrap settings
> (QE bit in this case) that cannot be overwritten by someone else
> (although it's seen on Intel, it might happen on some other
> architecture). The logic in the codes are having issues. Its comment
> says: clear BP# bits, but it clears QE bit for Macronix flash as well,
> which is wrong. The update was just to make sure the codes do as what
> its comment says.

I believe QEB is same position for all Macronix chips, checked few
parts true? what if the supported chips from id tables doesn't have
QEB at-all means specific chip support upto dual?

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

Hi Jagan,

On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> Hi Bin,
>
> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Jagan,
>>
>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>> status register as BP# bits, and we need preserve its original value
>>>> during a reboot cycle as this is required by some platforms (like
>>>> Intel ICH SPI controller working under descriptor mode).
>>>>
>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>> ---
>>>>
>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>> index 0034a28..7d8c660 100644
>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>         if (IS_ERR_OR_NULL(info))
>>>>                 return -ENOENT;
>>>>
>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>> +       /*
>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>> +        *
>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>> +        * same status register as BP# bits, and we need preserve its original
>>>> +        * value during a reboot cycle as this is required by some platforms
>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>> +        */
>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>                 write_sr(flash, 0);
>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>> +               u8 sr = 0;
>>>> +
>>>> +               read_sr(flash, &sr);
>>>> +               sr &= STATUS_QEB_MXIC;
>>>> +               write_sr(flash, sr);
>>>> +       }
>>>
>>> It doesn't make sense to have one(specific) controller fix to be
>>> generic to all macronix chips, think about alternative.
>>>
>>
>> This is no way to fix at the controller level. Actually this is
>> nothing related to controller level. It's just the bootstrap settings
>> (QE bit in this case) that cannot be overwritten by someone else
>> (although it's seen on Intel, it might happen on some other
>> architecture). The logic in the codes are having issues. Its comment
>> says: clear BP# bits, but it clears QE bit for Macronix flash as well,
>> which is wrong. The update was just to make sure the codes do as what
>> its comment says.
>
> I believe QEB is same position for all Macronix chips, checked few
> parts true? what if the supported chips from id tables doesn't have
> QEB at-all means specific chip support upto dual?
>

Correct, QEB is in the same position (bit6) for all Macronix chips. If
a chipset that does not support QEB, that bit (bit6) is reserved, and
current patch still works.

So current patch can correctly handle both situations. The issue here
is that what we do here for the status register does NOT conform the
comment. We only wanted to clear the #BP bits. We should NOT clear the
QEB bit at all.

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Jagan Teki]

Hi Bin,

On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> Hi Bin,
>>
>> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>> status register as BP# bits, and we need preserve its original value
>>>>> during a reboot cycle as this is required by some platforms (like
>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>
>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>> ---
>>>>>
>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>> index 0034a28..7d8c660 100644
>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>                 return -ENOENT;
>>>>>
>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>> +       /*
>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>> +        *
>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>> +        */
>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>                 write_sr(flash, 0);
>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>> +               u8 sr = 0;
>>>>> +
>>>>> +               read_sr(flash, &sr);
>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>> +               write_sr(flash, sr);

Better assign sr with QEB for macronix and call write_sr once.

>>>>> +       }
>>>>
>>>> It doesn't make sense to have one(specific) controller fix to be
>>>> generic to all macronix chips, think about alternative.
>>>>
>>>
>>> This is no way to fix at the controller level. Actually this is
>>> nothing related to controller level. It's just the bootstrap settings
>>> (QE bit in this case) that cannot be overwritten by someone else
>>> (although it's seen on Intel, it might happen on some other
>>> architecture). The logic in the codes are having issues. Its comment
>>> says: clear BP# bits, but it clears QE bit for Macronix flash as well,
>>> which is wrong. The update was just to make sure the codes do as what
>>> its comment says.
>>
>> I believe QEB is same position for all Macronix chips, checked few
>> parts true? what if the supported chips from id tables doesn't have
>> QEB at-all means specific chip support upto dual?
>>
>
> Correct, QEB is in the same position (bit6) for all Macronix chips. If
> a chipset that does not support QEB, that bit (bit6) is reserved, and
> current patch still works.
>
> So current patch can correctly handle both situations. The issue here
> is that what we do here for the status register does NOT conform the
> comment. We only wanted to clear the #BP bits. We should NOT clear the
> QEB bit at all.

OK, thanks.

-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

Hi Jagan,

On Mon, Aug 14, 2017 at 12:58 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> Hi Bin,
>
> On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Jagan,
>>
>> On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>> Hi Bin,
>>>
>>> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> Hi Jagan,
>>>>
>>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>>> status register as BP# bits, and we need preserve its original value
>>>>>> during a reboot cycle as this is required by some platforms (like
>>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>>
>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>> ---
>>>>>>
>>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>>> index 0034a28..7d8c660 100644
>>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>>                 return -ENOENT;
>>>>>>
>>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>>> +       /*
>>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>>> +        *
>>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>>> +        */
>>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>>                 write_sr(flash, 0);
>>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>>> +               u8 sr = 0;
>>>>>> +
>>>>>> +               read_sr(flash, &sr);
>>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>>> +               write_sr(flash, sr);
>
> Better assign sr with QEB for macronix and call write_sr once.

For these Macronix flashes that does not support quard RW, QEB bit is
reserved. Writing 1 to a reserved bit is not a good practice.

>
>>>>>> +       }
>>>>>
>>>>> It doesn't make sense to have one(specific) controller fix to be
>>>>> generic to all macronix chips, think about alternative.
>>>>>
>>>>
>>>> This is no way to fix at the controller level. Actually this is
>>>> nothing related to controller level. It's just the bootstrap settings
>>>> (QE bit in this case) that cannot be overwritten by someone else
>>>> (although it's seen on Intel, it might happen on some other
>>>> architecture). The logic in the codes are having issues. Its comment
>>>> says: clear BP# bits, but it clears QE bit for Macronix flash as well,
>>>> which is wrong. The update was just to make sure the codes do as what
>>>> its comment says.
>>>
>>> I believe QEB is same position for all Macronix chips, checked few
>>> parts true? what if the supported chips from id tables doesn't have
>>> QEB at-all means specific chip support upto dual?
>>>
>>
>> Correct, QEB is in the same position (bit6) for all Macronix chips. If
>> a chipset that does not support QEB, that bit (bit6) is reserved, and
>> current patch still works.
>>
>> So current patch can correctly handle both situations. The issue here
>> is that what we do here for the status register does NOT conform the
>> comment. We only wanted to clear the #BP bits. We should NOT clear the
>> QEB bit at all.
>
> OK, thanks.

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Jagan Teki]

On Mon, Aug 14, 2017 at 10:34 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Aug 14, 2017 at 12:58 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> Hi Bin,
>>
>> On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>> Hi Bin,
>>>>
>>>> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>> Hi Jagan,
>>>>>
>>>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>>>> status register as BP# bits, and we need preserve its original value
>>>>>>> during a reboot cycle as this is required by some platforms (like
>>>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>>>
>>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>>> ---
>>>>>>>
>>>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>>>> index 0034a28..7d8c660 100644
>>>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>>>                 return -ENOENT;
>>>>>>>
>>>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>>>> +       /*
>>>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>>>> +        *
>>>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>>>> +        */
>>>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>>>                 write_sr(flash, 0);
>>>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>>>> +               u8 sr = 0;
>>>>>>> +
>>>>>>> +               read_sr(flash, &sr);
>>>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>>>> +               write_sr(flash, sr);
>>
>> Better assign sr with QEB for macronix and call write_sr once.
>
> For these Macronix flashes that does not support quard RW, QEB bit is
> reserved. Writing 1 to a reserved bit is not a good practice.

Yeah, i.e what I'm concern here. (apart from fixing comment) this
issue came-up with your controller along with specific connected chip
which support RW WEB.

What if we couldn't preserve QEB? because if user need quad operation
anyway code will check QEB if not it will enable.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

Hi Jagan,

On Mon, Aug 14, 2017 at 1:17 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Mon, Aug 14, 2017 at 10:34 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Jagan,
>>
>> On Mon, Aug 14, 2017 at 12:58 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>> Hi Bin,
>>>
>>> On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> Hi Jagan,
>>>>
>>>> On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>> Hi Bin,
>>>>>
>>>>> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>> Hi Jagan,
>>>>>>
>>>>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>>>>> status register as BP# bits, and we need preserve its original value
>>>>>>>> during a reboot cycle as this is required by some platforms (like
>>>>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>>>>
>>>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>>>>> index 0034a28..7d8c660 100644
>>>>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>>>>                 return -ENOENT;
>>>>>>>>
>>>>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>>>>> +       /*
>>>>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>>>>> +        *
>>>>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>>>>> +        */
>>>>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>>>>                 write_sr(flash, 0);
>>>>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>>>>> +               u8 sr = 0;
>>>>>>>> +
>>>>>>>> +               read_sr(flash, &sr);
>>>>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>>>>> +               write_sr(flash, sr);
>>>
>>> Better assign sr with QEB for macronix and call write_sr once.
>>
>> For these Macronix flashes that does not support quard RW, QEB bit is
>> reserved. Writing 1 to a reserved bit is not a good practice.
>
> Yeah, i.e what I'm concern here. (apart from fixing comment) this
> issue came-up with your controller along with specific connected chip
> which support RW WEB.

As I said, this is nothing related to the SPI controller driver. It
can (technically) happen on other platforms. I don't understand what
your concerns here. Your suggestion of writing SR once does not work.

>
> What if we couldn't preserve QEB? because if user need quad operation
> anyway code will check QEB if not it will enable.

The board simply bricks after a successful boot once. Because the QE
bit is cleared by U-Boot during this successful boot, next time when
the board powers-up, the SoC won't get a valid bootstrap setting from
SPI flash. The bootstrap settings are stored in the SPI flash and
there is a QE bit enable in the bootstrap setting. When SoC finds out
the QE bit is turned on in the bootstrap setting but SPI flash's QE
bit is off, the SoC refuses to boot.

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

Hi Jagan,

On Mon, Aug 14, 2017 at 1:35 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Aug 14, 2017 at 1:17 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> On Mon, Aug 14, 2017 at 10:34 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Mon, Aug 14, 2017 at 12:58 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>> Hi Bin,
>>>>
>>>> On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>> Hi Jagan,
>>>>>
>>>>> On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>> Hi Bin,
>>>>>>
>>>>>> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>> Hi Jagan,
>>>>>>>
>>>>>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>>>>>> status register as BP# bits, and we need preserve its original value
>>>>>>>>> during a reboot cycle as this is required by some platforms (like
>>>>>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>>>>>
>>>>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>>>>>> index 0034a28..7d8c660 100644
>>>>>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>>>>>                 return -ENOENT;
>>>>>>>>>
>>>>>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>>>>>> +       /*
>>>>>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>>>>>> +        *
>>>>>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>>>>>> +        */
>>>>>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>>>>>                 write_sr(flash, 0);
>>>>>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>>>>>> +               u8 sr = 0;
>>>>>>>>> +
>>>>>>>>> +               read_sr(flash, &sr);
>>>>>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>>>>>> +               write_sr(flash, sr);
>>>>
>>>> Better assign sr with QEB for macronix and call write_sr once.
>>>
>>> For these Macronix flashes that does not support quard RW, QEB bit is
>>> reserved. Writing 1 to a reserved bit is not a good practice.
>>
>> Yeah, i.e what I'm concern here. (apart from fixing comment) this
>> issue came-up with your controller along with specific connected chip
>> which support RW WEB.
>
> As I said, this is nothing related to the SPI controller driver. It
> can (technically) happen on other platforms. I don't understand what
> your concerns here. Your suggestion of writing SR once does not work.
>
>>
>> What if we couldn't preserve QEB? because if user need quad operation
>> anyway code will check QEB if not it will enable.
>
> The board simply bricks after a successful boot once. Because the QE
> bit is cleared by U-Boot during this successful boot, next time when
> the board powers-up, the SoC won't get a valid bootstrap setting from
> SPI flash. The bootstrap settings are stored in the SPI flash and
> there is a QE bit enable in the bootstrap setting. When SoC finds out
> the QE bit is turned on in the bootstrap setting but SPI flash's QE
> bit is off, the SoC refuses to boot.
>

Sadly, this discussion ends to nowhere again. Can you please indicate
your clear opinion on how to proceed?

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Jagan Teki]

Hi Bin,

On Wed, Aug 16, 2017 at 7:29 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Aug 14, 2017 at 1:35 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Jagan,
>>
>> On Mon, Aug 14, 2017 at 1:17 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>> On Mon, Aug 14, 2017 at 10:34 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> Hi Jagan,
>>>>
>>>> On Mon, Aug 14, 2017 at 12:58 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>> Hi Bin,
>>>>>
>>>>> On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>> Hi Jagan,
>>>>>>
>>>>>> On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>>> Hi Bin,
>>>>>>>
>>>>>>> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>> Hi Jagan,
>>>>>>>>
>>>>>>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>>>>>>> status register as BP# bits, and we need preserve its original value
>>>>>>>>>> during a reboot cycle as this is required by some platforms (like
>>>>>>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>>>>>>> index 0034a28..7d8c660 100644
>>>>>>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>>>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>>>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>>>>>>                 return -ENOENT;
>>>>>>>>>>
>>>>>>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>>>>>>> +       /*
>>>>>>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>>>>>>> +        *
>>>>>>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>>>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>>>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>>>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>>>>>>> +        */
>>>>>>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>>>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>>>>>>                 write_sr(flash, 0);
>>>>>>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>>>>>>> +               u8 sr = 0;
>>>>>>>>>> +
>>>>>>>>>> +               read_sr(flash, &sr);
>>>>>>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>>>>>>> +               write_sr(flash, sr);
>>>>>
>>>>> Better assign sr with QEB for macronix and call write_sr once.
>>>>
>>>> For these Macronix flashes that does not support quard RW, QEB bit is
>>>> reserved. Writing 1 to a reserved bit is not a good practice.
>>>
>>> Yeah, i.e what I'm concern here. (apart from fixing comment) this
>>> issue came-up with your controller along with specific connected chip
>>> which support RW WEB.
>>
>> As I said, this is nothing related to the SPI controller driver. It
>> can (technically) happen on other platforms. I don't understand what
>> your concerns here. Your suggestion of writing SR once does not work.

OK, then lets proceed with this.

finally, I've made a change for easy readability [1], If you're OK I
will apply this otherwise suggest any?

[1] https://paste.ubuntu.com/25324972/

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Bin Meng]

Hi Jagan,

On Wed, Aug 16, 2017 at 6:34 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> Hi Bin,
>
> On Wed, Aug 16, 2017 at 7:29 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Jagan,
>>
>> On Mon, Aug 14, 2017 at 1:35 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Mon, Aug 14, 2017 at 1:17 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>> On Mon, Aug 14, 2017 at 10:34 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>> Hi Jagan,
>>>>>
>>>>> On Mon, Aug 14, 2017 at 12:58 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>> Hi Bin,
>>>>>>
>>>>>> On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>> Hi Jagan,
>>>>>>>
>>>>>>> On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>>>> Hi Bin,
>>>>>>>>
>>>>>>>> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>>> Hi Jagan,
>>>>>>>>>
>>>>>>>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>>>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>>>>>>>> status register as BP# bits, and we need preserve its original value
>>>>>>>>>>> during a reboot cycle as this is required by some platforms (like
>>>>>>>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>>>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>>>>>>>> index 0034a28..7d8c660 100644
>>>>>>>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>>>>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>>>>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>>>>>>>                 return -ENOENT;
>>>>>>>>>>>
>>>>>>>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>>>>>>>> +       /*
>>>>>>>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>>>>>>>> +        *
>>>>>>>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>>>>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>>>>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>>>>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>>>>>>>> +        */
>>>>>>>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>>>>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>>>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>>>>>>>                 write_sr(flash, 0);
>>>>>>>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>>>>>>>> +               u8 sr = 0;
>>>>>>>>>>> +
>>>>>>>>>>> +               read_sr(flash, &sr);
>>>>>>>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>>>>>>>> +               write_sr(flash, sr);
>>>>>>
>>>>>> Better assign sr with QEB for macronix and call write_sr once.
>>>>>
>>>>> For these Macronix flashes that does not support quard RW, QEB bit is
>>>>> reserved. Writing 1 to a reserved bit is not a good practice.
>>>>
>>>> Yeah, i.e what I'm concern here. (apart from fixing comment) this
>>>> issue came-up with your controller along with specific connected chip
>>>> which support RW WEB.
>>>
>>> As I said, this is nothing related to the SPI controller driver. It
>>> can (technically) happen on other platforms. I don't understand what
>>> your concerns here. Your suggestion of writing SR once does not work.
>
> OK, then lets proceed with this.
>
> finally, I've made a change for easy readability [1], If you're OK I
> will apply this otherwise suggest any?
>
> [1] https://paste.ubuntu.com/25324972/
>

Looks good. Please apply. Thanks!

Regards,
Bin

[U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash

[Jagan Teki]

On Wed, Aug 16, 2017 at 5:56 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Wed, Aug 16, 2017 at 6:34 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> Hi Bin,
>>
>> On Wed, Aug 16, 2017 at 7:29 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Jagan,
>>>
>>> On Mon, Aug 14, 2017 at 1:35 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> Hi Jagan,
>>>>
>>>> On Mon, Aug 14, 2017 at 1:17 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>> On Mon, Aug 14, 2017 at 10:34 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>> Hi Jagan,
>>>>>>
>>>>>> On Mon, Aug 14, 2017 at 12:58 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>>> Hi Bin,
>>>>>>>
>>>>>>> On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>> Hi Jagan,
>>>>>>>>
>>>>>>>> On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>>>>> Hi Bin,
>>>>>>>>>
>>>>>>>>> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>>>> Hi Jagan,
>>>>>>>>>>
>>>>>>>>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>>>>>>>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>>>>>>>>> On some flash (like Macronix), QE (quad enable) bit is in the same
>>>>>>>>>>>> status register as BP# bits, and we need preserve its original value
>>>>>>>>>>>> during a reboot cycle as this is required by some platforms (like
>>>>>>>>>>>> Intel ICH SPI controller working under descriptor mode).
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>>
>>>>>>>>>>>>  drivers/mtd/spi/spi_flash.c | 17 +++++++++++++++--
>>>>>>>>>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>>>>>>>>>>> index 0034a28..7d8c660 100644
>>>>>>>>>>>> --- a/drivers/mtd/spi/spi_flash.c
>>>>>>>>>>>> +++ b/drivers/mtd/spi/spi_flash.c
>>>>>>>>>>>> @@ -947,11 +947,24 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>>>>>>>         if (IS_ERR_OR_NULL(info))
>>>>>>>>>>>>                 return -ENOENT;
>>>>>>>>>>>>
>>>>>>>>>>>> -       /* Flash powers up read-only, so clear BP# bits */
>>>>>>>>>>>> +       /*
>>>>>>>>>>>> +        * Flash powers up read-only, so clear BP# bits.
>>>>>>>>>>>> +        *
>>>>>>>>>>>> +        * Note on some flash (like Macronix), QE (quad enable) bit is in the
>>>>>>>>>>>> +        * same status register as BP# bits, and we need preserve its original
>>>>>>>>>>>> +        * value during a reboot cycle as this is required by some platforms
>>>>>>>>>>>> +        * (like Intel ICH SPI controller working under descriptor mode).
>>>>>>>>>>>> +        */
>>>>>>>>>>>>         if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_ATMEL ||
>>>>>>>>>>>> -           JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX ||
>>>>>>>>>>>>             JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
>>>>>>>>>>>>                 write_sr(flash, 0);
>>>>>>>>>>>> +       if (JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_MACRONIX) {
>>>>>>>>>>>> +               u8 sr = 0;
>>>>>>>>>>>> +
>>>>>>>>>>>> +               read_sr(flash, &sr);
>>>>>>>>>>>> +               sr &= STATUS_QEB_MXIC;
>>>>>>>>>>>> +               write_sr(flash, sr);
>>>>>>>
>>>>>>> Better assign sr with QEB for macronix and call write_sr once.
>>>>>>
>>>>>> For these Macronix flashes that does not support quard RW, QEB bit is
>>>>>> reserved. Writing 1 to a reserved bit is not a good practice.
>>>>>
>>>>> Yeah, i.e what I'm concern here. (apart from fixing comment) this
>>>>> issue came-up with your controller along with specific connected chip
>>>>> which support RW WEB.
>>>>
>>>> As I said, this is nothing related to the SPI controller driver. It
>>>> can (technically) happen on other platforms. I don't understand what
>>>> your concerns here. Your suggestion of writing SR once does not work.
>>
>> OK, then lets proceed with this.
>>
>> finally, I've made a change for easy readability [1], If you're OK I
>> will apply this otherwise suggest any?
>>
>> [1] https://paste.ubuntu.com/25324972/

Applied to u-boot-spi/master

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.