[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 at gmail.com> wrote:
> Hi Bin,
>
> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Jagan,
>>
>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki <jagannadh.teki at gmail.com> wrote:
>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng <bmeng.cn at 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 at 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