[U-Boot] [PATCH] Macronix MX25xx MTD SPI flash driver

[Mike Frysinger <vapier@gentoo.org>]

On Friday 03 April 2009 07:49:19 Prafulla Wadaskar wrote:
> --- a/drivers/mtd/spi/Makefile
> +++ b/drivers/mtd/spi/Makefile
> @@ -28,6 +28,7 @@ LIB	:= $(obj)libspi_flash.a
>  COBJS-$(CONFIG_SPI_FLASH)	+= spi_flash.o
>  COBJS-$(CONFIG_SPI_FLASH_ATMEL)	+= atmel.o
>  COBJS-$(CONFIG_SPI_FLASH_STMICRO)	+= stmicro.o
> +COBJS-$(CONFIG_SPI_FLASH_MACRONIX)	+= macronix.o

please keep the list sorted

> --- /dev/null
> +++ b/drivers/mtd/spi/macronix.c
> @@ -0,0 +1,319 @@
> + * Based on drivers/mtd/spi/stmicor.c

typo in file name

> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +#include <common.h>

please add a new line there

> +/* MX25Pxx-specific commands */
> +#define CMD_MX25PXX_WREN	0x06	/* Write Enable */
> +#define CMD_MX25PXX_WRDI	0x04	/* Write Disable */
> +#define CMD_MX25PXX_RDSR	0x05	/* Read Status Register */
> +#define CMD_MX25PXX_WRSR	0x01	/* Write Status Register */
> +#define CMD_MX25PXX_READ	0x03	/* Read Data Bytes */
> +#define CMD_MX25PXX_FAST_READ	0x0b	/* Read Data Bytes at Higher Speed */
> +#define CMD_MX25PXX_PP		0x02	/* Page Program */
> +#define CMD_MX25PXX_SE		0x20	/* Sector Erase */
> +#define CMD_MX25PXX_BE		0xD8	/* Block Erase */
> +#define CMD_MX25PXX_CE		0xc7	/* Chip Erase */
> +#define CMD_MX25PXX_DP		0xb9	/* Deep Power-down */
> +#define CMD_MX25PXX_RES		0xab	/* Release from DP, and Read Signature */

is it really MX25PXX ?  or should it be MX25XX ?  the P looks like a hold over 
from the stmicro driver.

> +struct macronix_spi_flash {
> +	const struct macronix_spi_flash_params *params;
> +	struct spi_flash flash;
> +};

the spi_flash struct needs to be the first member in order to prevent crashes 
when working with the spi flash layer

> +	u8 cmd[4] = { CMD_MX25PXX_RDSR, 0xff, 0xff, 0xff };
> +
> +	ret = spi_xfer(spi, 32, &cmd[0], NULL, SPI_XFER_BEGIN);

do you actually need to read/write 4 bytes here ?  shouldnt it be:
u8 cmd = CMD_MX25PXX_RDSR;
ret = spi_xfer(spi, 8, &cmd, NULL, SPI_XFER_BEGIN);

> +	if (ret) {
> +		debug("SF: Failed to send command %02x: %d\n", cmd[0], ret);

then you'll need to change to "cmd"

> +			debug("SF: STMicro Page Program failed\n");
> +	debug("SF: STMicro: Successfully programmed %u bytes @ 0x%x\n",
> +	      len, offset);
> +			debug("SF: STMicro page erase failed\n");
> +			debug("SF: STMicro page erase timed out\n");
> +	debug("SF: STMicro: Successfully erased %u bytes @ 0x%x\n",
> +	      len * sector_size, offset);
> +		debug("SF: Unsupported STMicro ID %02x\n", id[1]);

this isnt STMicro anymore ...

> +		/* Up to 2 seconds */
> +		ret = macronix_wait_ready(flash, 2 * CONFIG_SYS_HZ);

there's a common flash erase timeout define

> +*spi_flash_probe_macronix(struct spi_slave *spi, u8 * idcode)

no space between "*" and "idcode"

> +	u8 id[3];
> +
> +	ret = spi_flash_cmd(spi, CMD_READ_ID, id, sizeof(id));
> +	if (ret)
> +		return NULL;

no need to read id here ... use the idcode passed in to you

> +		if (params->idcode1 == idcode[2]) {
> +			break;
> +		}

drop the {} braces

> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -3,7 +3,6 @@
>   *
>   * Copyright (C) 2008 Atmel Corporation
>   */
> -#define DEBUG
>  #include <common.h>
>  #include <malloc.h>
>  #include <spi.h>

please drop this hunk ... it doesnt belong in this patch (and it's already 
been merged elsewhere)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090403/0c24ba2e/attachment.pgp