From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jagan Teki Date: Mon, 14 Aug 2017 10:28:20 +0530 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 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. >>>>> + } >>>> >>>> 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. OK, thanks. -- Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India.