All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Roese <sr@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Fix CFI flash driver for 8-bit bus support
Date: Mon, 4 Apr 2011 12:14:17 +0200	[thread overview]
Message-ID: <201104041214.17355.sr@denx.de> (raw)
In-Reply-To: <1301728621-27620-1-git-send-email-aaron.williams@caviumnetworks.com>

Hi Aaron,

On Saturday 02 April 2011 09:17:01 Aaron Williams wrote:
> This patch corrects the addresses used when working with Spansion/AMD FLASH
> chips. Addressing for 8 and 16 bits is almost identical except in the
> 16-bit case the LSB of the address is always 0.  The confusion arose
> because the addresses in the datasheet for 16-bit mode are word addresses
> but this code assumed it was byte addresses.
> 
> I have only been able to test this on our Octeon boards which use either an
> 8-bit or 16-bit bus.  I have not tested the case where there's an 8-bit
> part on a 16-bit bus.
> 
> This patch also adds some delays as suggested by Spansion.
> 
> If a part can be both 8 and 16-bits, it forces it to work in 8-bit mode if
> an 8-bit bus is detected.

Thanks.

Apart from the comments from Albert and Rogan, here some further notes:

- Please explain why the delay() calls are added. If really needed, please
  move them into a separate patch, as this seems unrelated to the 8/16
  bit issue.

- Please add a short description and rationale to the commit text (and
  perhaps to cfi_flash.h), why the offsets for the CFI data are now
  changed (e.g. 0x10 -> 0x20).

And one further comment below:
 
> @@ -1043,6 +1049,8 @@ int flash_erase (flash_info_t * info, int s_first,
> int s_last) int prot;
>  	flash_sect_t sect;
>  	int st;
> +
> +	debug("%s: erasing sectors %d to %d\n", __func__, s_first, s_last);
> 
>  	if (info->flash_id != FLASH_MAN_CFI) {
>  		puts ("Can't erase unknown flash type - aborted\n");
> @@ -1082,6 +1090,7 @@ int flash_erase (flash_info_t * info, int s_first,
> int s_last) break;
>  			case CFI_CMDSET_AMD_STANDARD:
>  			case CFI_CMDSET_AMD_EXTENDED:
> +				flash_write_cmd (info, 0, 0, AMD_CMD_RESET);

Is this correct? A reset command in every flash_erase() call?

Thanks.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

  parent reply	other threads:[~2011-04-04 10:14 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-02  7:17 [U-Boot] [PATCH] Fix CFI flash driver for 8-bit bus support Aaron Williams
2011-04-02 11:23 ` Albert ARIBAUD
2011-04-02 19:37   ` Rogan Dawes
2011-04-04 10:14 ` Stefan Roese [this message]
2011-04-12  7:46   ` Aaron Williams
2011-04-12  8:09     ` Stefan Roese
2011-04-12  8:33       ` Aaron Williams
2011-04-12  9:10         ` Stefan Roese

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201104041214.17355.sr@denx.de \
    --to=sr@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.