[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 at gmail.com> wrote:
> 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.
>

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

Regards,
Bin