From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Wed, 16 Aug 2017 20:26:10 +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 Wed, Aug 16, 2017 at 6:34 PM, Jagan Teki wrote: > Hi Bin, > > On Wed, Aug 16, 2017 at 7:29 AM, Bin Meng wrote: >> 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. > > 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/ > Looks good. Please apply. Thanks! Regards, Bin