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

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.