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

2009-08-05 Thread 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 
> ---
>  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 000..9298c9f
> --- /dev/null
> +++ b/drivers/spi/eeprom_m95xxx.c
> @@ -0,0 +1,115 @@
> +/*
> + * Copyright (C) 2009
> + * Albin Tonnerre, Free Electrons 
> + *
> + * 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 
> +#include 
> +
> +#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, 100,
> + 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, 100,
> + 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


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

2009-08-07 Thread Albin Tonnerre
On Wed, Aug 05, 2009 at 09:24:43PM +0200, Jean-Christophe PLAGNIOL-VILLARD 
wrote :
> On 19:16 Wed 05 Aug , Albin Tonnerre wrote:
> > +   for (i = 0; i < 1000; i++) {
> why 1000?

Oh, true. I meant to fix that and replace it with something similar to what
atmel_dataflash does, ie using CONFIG_SYS_SPI_WRITE_TOUT. Updated in the patch I
re-submitted.

Regards,
-- 
Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2009-08-07 Thread Jean-Christophe PLAGNIOL-VILLARD
On 12:37 Fri 07 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 
> ---
>  drivers/mtd/spi/Makefile|1 +
>  drivers/mtd/spi/eeprom_m95xxx.c |  117 
> +++
>  2 files changed, 118 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/mtd/spi/eeprom_m95xxx.c
looks fine

Best Regards,
J.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot