Re: [U-Boot] [PATCH 1/2] Add driver for the ST M95xxx SPI EEPROM

[Jean-Christophe PLAGNIOL-VILLARD]

On 19:16 Wed 05 Aug     , Albin Tonnerre wrote:
> This chip is used in a number of boards manufactured by Calao-Systems
> which should be supported soon. This driver provides the necessary
> spi_read and spi_write functions necessary to communicate with the chip.
> 
> Signed-off-by: Albin Tonnerre <albin.tonne...@free-electrons.com>
> ---
>  drivers/spi/Makefile        |    1 +
>  drivers/spi/eeprom_m95xxx.c |  115 
> +++++++++++++++++++++++++++++++++++++++++++
not sure it's the right place
drivers mtd will better IMHO
as in drivers/spi it will more for spi host drivers
and not eeprom
>  2 files changed, 116 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/spi/eeprom_m95xxx.c
> 
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index a9f67a0..35c9e02 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -33,6 +33,7 @@ COBJS-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
>  COBJS-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
>  COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o
>  COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o
> +COBJS-$(CONFIG_M95XXX_SPI) += eeprom_m95xxx.o
>  
>  COBJS        := $(COBJS-y)
>  SRCS := $(COBJS:.o=.c)
> diff --git a/drivers/spi/eeprom_m95xxx.c b/drivers/spi/eeprom_m95xxx.c
> new file mode 100644
> index 0000000..9298c9f
> --- /dev/null
> +++ b/drivers/spi/eeprom_m95xxx.c
> @@ -0,0 +1,115 @@
> +/*
> + * Copyright (C) 2009
> + * Albin Tonnerre, Free Electrons <albin.tonne...@free-electrons.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * 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., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <spi.h>
> +
> +#define SPI_EEPROM_WREN              0x06
> +#define SPI_EEPROM_RDSR              0x05
> +#define SPI_EEPROM_READ              0x03
> +#define SPI_EEPROM_WRITE     0x02
> +
> +#ifndef CONFIG_DEFAULT_SPI_BUS
> +#define CONFIG_DEFAULT_SPI_BUS 0
> +#endif
> +
> +#ifndef CONFIG_DEFAULT_SPI_MODE
> +#define CONFIG_DEFAULT_SPI_MODE SPI_MODE_0
> +#endif
> +
> +ssize_t spi_read (uchar *addr, int alen, uchar *buffer, int len)
> +{
> +     struct spi_slave *slave;
> +     u8 cmd = SPI_EEPROM_READ;
> +
> +     slave = spi_setup_slave(CONFIG_DEFAULT_SPI_BUS, 1, 1000000,
> +                     CONFIG_DEFAULT_SPI_MODE);
> +     spi_claim_bus(slave);
> +
> +     /* command */
> +     if(spi_xfer(slave, 8, &cmd, NULL, SPI_XFER_BEGIN))
> +             return -1;
> +
> +     /* if alen == 3, addr[0] is the block number, we never use it here. All 
> we
> +      * need are the lower 16 bits*/
please use the syle of comment
/*
 *
 */
> +     if (alen == 3)
> +             addr++;
> +
> +     /* address, and data */
> +     if(spi_xfer(slave, 16, addr, NULL, 0))
> +             return -1;
> +     if(spi_xfer(slave, 8*len, NULL, buffer, SPI_XFER_END))
please add a space before and after the '*'
> +             return -1;
> +
> +     spi_release_bus(slave);
> +     spi_free_slave(slave);
> +     return len;
> +}
> +
> +ssize_t spi_write (uchar *addr, int alen, uchar *buffer, int len)
> +{
> +     struct spi_slave *slave;
> +     int i;
> +     char buf[3];
> +
> +     slave = spi_setup_slave(CONFIG_DEFAULT_SPI_BUS, 1, 1000000,
> +                     CONFIG_DEFAULT_SPI_MODE);
> +     spi_claim_bus(slave);
> +
> +     buf[0] = SPI_EEPROM_WREN;
> +     if(spi_xfer(slave, 8, buf, NULL, SPI_XFER_BEGIN | SPI_XFER_END))
> +             return -1;
> +
> +     buf[0] = SPI_EEPROM_WRITE;
> +
> +     /* As for reading, drop addr[0] if alen is 3 */
> +     if (alen == 3) {
> +             alen--;
> +             addr++;
> +     }
> +
> +     memcpy(buf+1, addr, alen);
please add a space before and after the '+'
> +     /* command + addr, then data */
> +     if(spi_xfer(slave, 24, buf, NULL, SPI_XFER_BEGIN))
> +             return -1;
> +     if(spi_xfer(slave, len*8, buffer, NULL, SPI_XFER_END))
please add a space before and after the '*'
> +             return -1;
> +
> +     for (i = 0; i < 1000; i++) {
why 1000?
> +             buf[0] = SPI_EEPROM_RDSR;
> +             buf[1] = 0;
> +             spi_xfer(slave, 16, buf, buf, SPI_XFER_BEGIN | SPI_XFER_END);
> +
Best Regards,
J.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot