From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Wed, 16 Aug 2017 09:59:46 +0800 Subject: [U-Boot] [PATCH 2/2] sf: Preserve QE bit when clearing BP# bits for Macronix flash In-Reply-To: References: <1500821077-28325-1-git-send-email-bmeng.cn@gmail.com> <1500821077-28325-2-git-send-email-bmeng.cn@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Jagan, On Mon, Aug 14, 2017 at 1:35 PM, Bin Meng wrote: > Hi Jagan, > > On Mon, Aug 14, 2017 at 1:17 PM, Jagan Teki wrote: >> On Mon, Aug 14, 2017 at 10:34 AM, Bin Meng wrote: >>> Hi Jagan, >>> >>> On Mon, Aug 14, 2017 at 12:58 PM, Jagan Teki wrote: >>>> Hi Bin, >>>> >>>> On Mon, Aug 14, 2017 at 8:07 AM, Bin Meng wrote: >>>>> Hi Jagan, >>>>> >>>>> On Mon, Aug 14, 2017 at 1:22 AM, Jagan Teki wrote: >>>>>> Hi Bin, >>>>>> >>>>>> On Wed, Aug 2, 2017 at 3:56 AM, Bin Meng wrote: >>>>>>> Hi Jagan, >>>>>>> >>>>>>> On Wed, Aug 2, 2017 at 12:01 AM, Jagan Teki wrote: >>>>>>>> On Sun, Jul 23, 2017 at 8:14 PM, Bin Meng 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 >>>>>>>>> --- >>>>>>>>> >>>>>>>>> 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