[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 at gmail.com> wrote:
> On Mon, Aug 14, 2017 at 10:34 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Jagan,
>>
>> On Mon, Aug 14, 2017 at 12:58 PM, Jagan Teki <jagannadh.teki at gmail.com> wrote:
>>> Hi Bin,
>>>
>>> On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>> 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);
>>>
>>> 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