From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bin Meng <bmeng.cn@gmail.com>
Date: Fri, 4 Aug 2017 12:21:49 +0800
Subject: [U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits
 for Macronix flash
In-Reply-To: <CAEUhbmXNXctGYZXFj_pXSVgMoap3EwvWKoLY4R=Fmz+fAVJCPA@mail.gmail.com>
References: <1500821077-28325-1-git-send-email-bmeng.cn@gmail.com>
 <1500821077-28325-2-git-send-email-bmeng.cn@gmail.com>
 <CAD6G_RSLCY8qcCsrk2b7xV5xbfXJ2G0gocMNJaO7kk90m6DCsg@mail.gmail.com>
 <CAEUhbmXNXctGYZXFj_pXSVgMoap3EwvWKoLY4R=Fmz+fAVJCPA@mail.gmail.com>
Message-ID: <CAEUhbmWNTF=bb7duYE-4RwvYxLzck3wsEgudrfir8nNEuYdnMA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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