[U-Boot] [PATCH v3 17/17] sf: Add Flag status register polling support

[Jagan Teki]

Hi Simon,

On Thu, Jun 13, 2013 at 11:23 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Jagan,
>
> On Thu, Jun 13, 2013 at 10:46 AM, Willis, Max <Max.Willis at spansion.com>
> wrote:
>>
>> Hi Jagan, Simon - Adding Bacem and Gernot from Spansion. Would you be able
>> to add them to the other open threads as well? --Max
>>
>> -------------------------------------------------
>> Max Willis, PhD
>> Division Marketing Manager
>> Software Marketing
>> Spansion Inc.
>> -------------------------------------------------
>>
>> -----Original Message-----
>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
>> Sent: Wednesday, June 12, 2013 10:32 PM
>> To: Simon Glass
>> Cc: Jagannadha Sutradharudu Teki; U-Boot Mailing List; Michal Simek; Tom
>> Rini; Rajeshwari Shinde; Syed Hussain; Willis, Max; Todd Legler (tlegler);
>> Jagannadha Sutradharudu Teki
>> Subject: Re: [PATCH v3 17/17] sf: Add Flag status register polling support
>>
>> Hi Simon,
>>
>> On Thu, Jun 13, 2013 at 4:12 AM, Simon Glass <sjg at chromium.org> wrote:
>> > Hi Jagan,
>> >
>> > On Tue, Jun 11, 2013 at 12:23 PM, Jagannadha Sutradharudu Teki
>> > <jagannadha.sutradharudu-teki at xilinx.com> wrote:
>> >>
>> >> Flag status register polling is required for micron 512Mb flash
>> >> devices onwards, for performing erase/program operations.
>> >>
>> >> Like polling for WIP(Write-In-Progress) bit in read status register,
>> >> spi_flash_cmd_wait_ready will poll for PEC(Program-Erase-Control) bit
>> >> in flag status register.
>> >>
>> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna at xilinx.com>
>> >>
>> >> ---
>> >> Changes for v3:
>> >>         - define the flag status code on CONFIG_SPI_FLASH_STMICRO
>> >> Changes for v2:
>> >>         - none
>> >>
>> >>  drivers/mtd/spi/spi_flash.c          | 16 +++++++++++++---
>> >>  drivers/mtd/spi/spi_flash_internal.h |  2 ++
>> >>  2 files changed, 15 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/drivers/mtd/spi/spi_flash.c
>> >> b/drivers/mtd/spi/spi_flash.c index bb37f54..4f97fab0 100644
>> >> --- a/drivers/mtd/spi/spi_flash.c
>> >> +++ b/drivers/mtd/spi/spi_flash.c
>> >> @@ -200,12 +200,22 @@ int spi_flash_cmd_wait_ready(struct spi_flash
>> >> *flash, unsigned long timeout)
>> >>         unsigned long timebase;
>> >>         int ret;
>> >>         u8 status;
>> >> +       u8 check_status = 0x0;
>> >>         u8 poll_bit = STATUS_WIP;
>> >>         u8 cmd = CMD_READ_STATUS;
>> >>
>> >> +#ifdef CONFIG_SPI_FLASH_STMICRO
>> >> +       if (flash->size >= 0x4000000) {
>> >> +               poll_bit = STATUS_PEC;
>> >> +               check_status = poll_bit;
>> >> +               cmd = CMD_FLAG_STATUS;
>> >> +       }
>> >> +#endif
>> >> +
>> >
>> >
>> > Similarly to the bank addressing I think this might be better done as
>> > a 'poll method' rather than a hack for a particular flash type.
>> >
>>
>> You mean to say read_status cmd and flag_status cmd need to find it on
>> probe it self.?
>> Can you please explain "better done as a 'poll method' '
>
>
> What I mean is that spi_flash_cmd_wait_ready() is in generic code.
>
> CONFIG_SPI_FLASH_STMICRO means that U-Boot will support that flash, but it
> does not mean that it is the only flash. You have added a device-specific
> thing into generic code. If someone compiles a U-Boot with ST and Winbond
> flash, then both types of flash will use the code you added (the ST code
> above).

Understand.

Yes, initially i did this in runtime manner like

        if ((flash->idcode0 == 0x20) &&
			(flash->size >= 0x4000000)) {
		poll_bit = STATUS_PEC;
		check_status = poll_bit;
		cmd = CMD_FLAG_STATUS;
	}

I think even if we assign the read_status and flasg_status at probe
time the poll_bit and
check_status are different in these read and flag status reg'.

Any comments.

--
Thanks,
Jagan.