[U-Boot] [PATCH v3 01/17] sf: Add bank address register writing support

[Jagan Teki]

On Thu, Jun 13, 2013 at 8:56 PM, Tom Rini <trini at ti.com> wrote:
> On Thu, Jun 13, 2013 at 08:23:13AM +0530, Jagan Teki wrote:
>> Hi Simon,
>>
>> On Thu, Jun 13, 2013 at 3:48 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:
>> >>
>> >> This patch provides support to program a flash bank address
>> >> register.
>> >>
>> >> extended/bank address register contains an information to access
>> >> the 4th byte addressing in 3-byte address mode.
>> >>
>> >> Currently added an bank address register writing support for
>> >> spansion flashes.
>> >>
>> >> reff' the spec for more details about bank addr register
>> >> in Page-63, Table 8.16
>> >> http://www.spansion.com/Support/Datasheets/S25FL128S_256S_00.pdf
>> >>
>> >> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna at xilinx.com>
>> >
>> >
>> > Reviewed-by: Simon Glass <sjg at chromium.org>
>> >
>> > See comment below, perhaps a follow-on patch so you can get this in first?
>> >
>> >>
>> >> ---
>> >> Changes for v3:
>> >>         - none
>> >> Changes for v2:
>> >>         - none
>> >>
>> >>  drivers/mtd/spi/spi_flash.c          | 37
>> >> ++++++++++++++++++++++++++++++++++++
>> >>  drivers/mtd/spi/spi_flash_internal.h |  6 ++++++
>> >>  include/spi_flash.h                  |  2 ++
>> >>  3 files changed, 45 insertions(+)
>> >>
>> >> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>> >> index 0e38f59..7aba520 100644
>> >> --- a/drivers/mtd/spi/spi_flash.c
>> >> +++ b/drivers/mtd/spi/spi_flash.c
>> >> @@ -278,6 +278,40 @@ int spi_flash_cmd_write_status(struct spi_flash
>> >> *flash, u8 sr)
>> >>         return 0;
>> >>  }
>> >>
>> >> +int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8 bank_sel)
>> >> +{
>> >> +       u8 cmd, idcode0;
>> >> +       int ret;
>> >> +
>> >> +       idcode0 = flash->idcode0;
>> >> +       if (idcode0 == 0x01) {
>> >
>> >
>> > I wonder why you need this to be hard-coded here? As a follow-on patch, I
>> > suggest you have something in the probe function which decides which bank
>> > addressing algorithm to use (e.g. winbond, ST, none). and then this code can
>> > be:
>> >
>> > switch (flash->bank_addr_algo) {
>> > case SF_ALGO_WINBOND:
>> >    ...
>> > case SF_ALGO_ST:
>> >   ...
>> > }
>>
>> Means in probe time we can assign particular bank commands something like
>>
>> switch (*idp) {
>> case SF_WINBOND_IDCODE0:
>>      flash->bank_wr_cmd = ;
>>      flash->bank_rd_cmd = ;
>> case SF_SPAN_IDCODE0:
>>     flash->bank_wr_cmd = ;
>>     flash->bank_rd_cmd = ;
>> }
>>
>> Please let me know your comments.
>
> Correct, and similar to how some of the filesystem code works.  Looks
> good to me.

Thank you, I am sending the respective changes on v4.

--
Thanks,
Jagan.