[U-Boot] [PATCH 0/6] sandbox: spi/sf emulation
Finished this up while on a jet plane. Ignoring the getopt patch, this should be good to go.
Mike Frysinger (6): sandbox: add lseek helper sandbox: add getopt support sandbox: SPI emulation bus sandbox: new SPI flash driver sandbox: enable new spi/sf layers sandbox: add getenv support
arch/sandbox/cpu/os.c | 31 ++++ arch/sandbox/cpu/start.c | 24 +++ arch/sandbox/include/asm/spi.h | 33 ++++ drivers/mtd/spi/Makefile | 1 + drivers/mtd/spi/sandbox.c | 318 ++++++++++++++++++++++++++++++++++++++++ drivers/spi/Makefile | 1 + drivers/spi/sandbox_spi.c | 188 ++++++++++++++++++++++++ include/configs/sandbox.h | 7 + include/os.h | 13 ++ 9 files changed, 616 insertions(+), 0 deletions(-) create mode 100644 arch/sandbox/include/asm/spi.h create mode 100644 drivers/mtd/spi/sandbox.c create mode 100644 drivers/spi/sandbox_spi.c
Follow up patches want to be able to seek fd's.
Signed-off-by: Mike Frysinger vapier@gentoo.org --- arch/sandbox/cpu/os.c | 5 +++++ include/os.h | 10 ++++++++++ 2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 093e7dc..6e257ff 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -45,6 +45,11 @@ ssize_t os_write(int fd, const void *buf, size_t count) return write(fd, buf, count); }
+off_t os_lseek(int fd, off_t offset, int whence) +{ + return lseek(fd, offset, whence); +} + int os_open(const char *pathname, int flags) { return open(pathname, flags); diff --git a/include/os.h b/include/os.h index c17a8a5..d182f40 100644 --- a/include/os.h +++ b/include/os.h @@ -49,6 +49,16 @@ ssize_t os_read(int fd, void *buf, size_t count); ssize_t os_write(int fd, const void *buf, size_t count);
/** + * Access to the OS lseek() system call + * + * \param fd File descriptor as returned by os_open() + * \param offset File offset (based on whence) + * \param whence Position offset is relative to + * \return new file offset + */ +off_t os_lseek(int fd, off_t offset, int whence); + +/** * Access to the OS open() system call * * \param pathname Pathname of file to open
Hi Mike,
On Sun, Jan 22, 2012 at 10:30 PM, Mike Frysinger vapier@gentoo.org wrote:
Follow up patches want to be able to seek fd's.
Signed-off-by: Mike Frysinger vapier@gentoo.org
arch/sandbox/cpu/os.c | 5 +++++ include/os.h | 10 ++++++++++ 2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 093e7dc..6e257ff 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -45,6 +45,11 @@ ssize_t os_write(int fd, const void *buf, size_t count) return write(fd, buf, count); }
+off_t os_lseek(int fd, off_t offset, int whence) +{
- return lseek(fd, offset, whence);
+}
I think we need some sort of #define setup for the whence parameter in the header file, like I did with open().
int os_open(const char *pathname, int flags) { return open(pathname, flags); diff --git a/include/os.h b/include/os.h index c17a8a5..d182f40 100644 --- a/include/os.h +++ b/include/os.h @@ -49,6 +49,16 @@ ssize_t os_read(int fd, void *buf, size_t count); ssize_t os_write(int fd, const void *buf, size_t count);
/**
- Access to the OS lseek() system call
- \param fd File descriptor as returned by os_open()
- \param offset File offset (based on whence)
- \param whence Position offset is relative to
Document the values of this param, since people can't really assume the mirror lseek() unless we tell them.
- \return new file offset
- */
+off_t os_lseek(int fd, off_t offset, int whence);
+/** * Access to the OS open() system call * * \param pathname Pathname of file to open -- 1.7.8.3
Regards, Simon
Note: just a PoC that the current SPI flash code is based on. Not meant to reject the getopt code Simon posted.
Signed-off-by: Mike Frysinger vapier@gentoo.org --- arch/sandbox/cpu/os.c | 13 +++++++++++++ arch/sandbox/cpu/start.c | 24 ++++++++++++++++++++++++ include/os.h | 2 ++ 3 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 6e257ff..1d3e54f 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -22,6 +22,7 @@ #include <errno.h> #include <fcntl.h> #include <stdlib.h> +#include <string.h> #include <termios.h> #include <time.h> #include <unistd.h> @@ -127,3 +128,15 @@ u64 os_get_nsec(void) return tv.tv_sec * 1000000000ULL + tv.tv_usec * 1000; #endif } + +extern char **sb_argv; +const char *os_getopt(const char *name, int has_arg) +{ + size_t i; + + for (i = 0; sb_argv[i]; ++i) + if (!strcmp(sb_argv[i], name)) + return sb_argv[i + !!has_arg]; + + return NULL; +} diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index a429e29..3508a35 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -20,9 +20,33 @@ */
#include <common.h> +#include <os.h> + +char **sb_argv; + +static const char usage[] = + "Usage: u-boot [options]\n" + "\n" + "Options: (note: not all options may be available)\n" + " -h, --help this message (imagine that)\n" +#ifdef CONFIG_SANDBOX_SPI + " --spi-<bus>-<cs> <spec> connect client to spi <bus> on <cs>\n" +# ifdef CONFIG_SPI_FLASH + " spec: sf:<file> treat <file> as spi flash\n" +# endif +#endif +;
int main(int argc, char *argv[]) { + /* Save the argv for people to access */ + sb_argv = argv; + + if (os_getopt("-h", 0) || os_getopt("--help", 0)) { + serial_puts(usage); + return 0; + } + /* * Do pre- and post-relocation init, then start up U-Boot. This will * never return. diff --git a/include/os.h b/include/os.h index d182f40..cb88509 100644 --- a/include/os.h +++ b/include/os.h @@ -112,4 +112,6 @@ void os_usleep(unsigned long usec); */ u64 os_get_nsec(void);
+const char *os_getopt(const char *name, int has_arg); + #endif
This adds a SPI framework for people to hook up simulated SPI clients.
Signed-off-by: Mike Frysinger vapier@gentoo.org --- arch/sandbox/include/asm/spi.h | 33 +++++++ drivers/spi/Makefile | 1 + drivers/spi/sandbox_spi.c | 183 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 0 deletions(-) create mode 100644 arch/sandbox/include/asm/spi.h create mode 100644 drivers/spi/sandbox_spi.c
diff --git a/arch/sandbox/include/asm/spi.h b/arch/sandbox/include/asm/spi.h new file mode 100644 index 0000000..082df37 --- /dev/null +++ b/arch/sandbox/include/asm/spi.h @@ -0,0 +1,33 @@ +/* + * Simulate a SPI port and clients + * + * Copyright (c) 2011-2012 The Chromium OS Authors. + * See file CREDITS for list of people who contributed to this + * project. + * + * Licensed under the GPL-2 or later. + */ + +#ifndef __ASM_SPI_H__ +#define __ASM_SPI_H__ + +#include <linux/types.h> + +struct sb_spi_emu_ops { + int (*setup)(void **priv, const char *spec); + void (*free)(void *priv); + void (*cs_activate)(void *priv); + void (*cs_deactivate)(void *priv); + int (*xfer)(void *priv, const u8 *tx, u8 *rx, uint bytes); +}; + +static inline void sb_spi_tristate(u8 *buf, uint len) +{ + /* + * XXX: make this into a user config option ? let them pick + * whether this is pulled low, or high, or tristates. + */ + memset(buf, 0xff, len); +} + +#endif diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index c967d87..d80ff8b 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -40,6 +40,7 @@ COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o COBJS-$(CONFIG_MXS_SPI) += mxs_spi.o COBJS-$(CONFIG_OC_TINY_SPI) += oc_tiny_spi.o COBJS-$(CONFIG_OMAP3_SPI) += omap3_spi.o +COBJS-$(CONFIG_SANDBOX_SPI) += sandbox_spi.o COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o COBJS-$(CONFIG_SH_SPI) += sh_spi.o COBJS-$(CONFIG_FSL_ESPI) += fsl_espi.o diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c new file mode 100644 index 0000000..eda0f3e --- /dev/null +++ b/drivers/spi/sandbox_spi.c @@ -0,0 +1,183 @@ +/* + * Simulate a SPI port + * + * Copyright (c) 2011-2012 The Chromium OS Authors. + * See file CREDITS for list of people who contributed to this + * project. + * + * Licensed under the GPL-2 or later. + */ + +#include <common.h> +#include <malloc.h> +#include <spi.h> +#include <os.h> + +#include <asm/spi.h> + +#ifndef CONFIG_SPI_IDLE_VAL +# define CONFIG_SPI_IDLE_VAL 0xFF +#endif + +struct sb_spi_slave { + struct spi_slave slave; + const char *spec; + const struct sb_spi_emu_ops *ops; + void *priv; +}; + +#define to_sb_spi_slave(s) container_of(s, struct sb_spi_slave, slave) + +static const char *sb_lookup_arg(unsigned int bus, unsigned int cs) +{ + char sf_arg[20]; + sprintf(sf_arg, "--spi-%u-%u", bus, cs); + return os_getopt(sf_arg, 1); +} + +static const struct { + const char *spec; + const struct sb_spi_emu_ops *ops; +} sb_emu_map[] = { +}; + +static int sb_parse_type(struct sb_spi_slave *sss) +{ + size_t i; + + for (i = 0; i < ARRAY_SIZE(sb_emu_map); ++i) { + size_t len = strlen(sb_emu_map[i].spec); + const char *sub_spec = sss->spec + len; + + sss->ops = sb_emu_map[i].ops; + if (!memcmp(sss->spec, sb_emu_map[i].spec, len) && + sss->spec[len] == ':') + return sss->ops->setup(&sss->priv, sub_spec + 1); + } + + return 1; +} + +int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{ + return sb_lookup_arg(bus, cs) ? 1 : 0; +} + +void spi_cs_activate(struct spi_slave *slave) +{ + struct sb_spi_slave *sss = to_sb_spi_slave(slave); + + if (sss->ops->cs_activate) + sss->ops->cs_activate(sss->priv); +} + +void spi_cs_deactivate(struct spi_slave *slave) +{ + struct sb_spi_slave *sss = to_sb_spi_slave(slave); + + if (sss->ops->cs_deactivate) + sss->ops->cs_deactivate(sss->priv); +} + +void spi_init(void) +{ +} + +void spi_set_speed(struct spi_slave *slave, uint hz) +{ +} + +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, + unsigned int max_hz, unsigned int mode) +{ + struct sb_spi_slave *sss; + + if (!spi_cs_is_valid(bus, cs)) + return NULL; + + sss = malloc(sizeof(*sss)); + if (!sss) + return NULL; + + sss->spec = sb_lookup_arg(bus, cs); + if (sb_parse_type(sss)) { + free(sss); + printf("sandbox_spi: unable to locate a slave client\n"); + return NULL; + } + + return &sss->slave; +} + +void spi_free_slave(struct spi_slave *slave) +{ + struct sb_spi_slave *sss = to_sb_spi_slave(slave); + + if (sss->ops->free) + sss->ops->free(sss->priv); + + free(sss); +} + +int spi_claim_bus(struct spi_slave *slave) +{ + return 0; +} + +void spi_release_bus(struct spi_slave *slave) +{ +} + +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, + void *din, unsigned long flags) +{ + struct sb_spi_slave *sss = to_sb_spi_slave(slave); + uint bytes = bitlen / 8, i; + int ret = 0; + u8 *tx = (void *)dout, *rx = din; + + if (bitlen == 0) + goto done; + + /* we can only do 8 bit transfers */ + if (bitlen % 8) { + flags |= SPI_XFER_END; + goto done; + } + + if (flags & SPI_XFER_BEGIN) + spi_cs_activate(slave); + + /* make sure rx/tx buffers are full so clients can assume */ + if (!tx) { + tx = malloc(bytes); + if (tx) + memset(tx, CONFIG_SPI_IDLE_VAL, bytes); + } + if (!rx) + rx = malloc(bytes); + if (tx && rx) { + debug("sb_xfer: bytes = %u\n tx:", bytes); + for (i = 0; i < bytes; ++i) + debug(" %u:%02x", i, tx[i]); + debug("\n"); + + ret = sss->ops->xfer(sss->priv, tx, rx, bytes); + + debug("sb_xfer: got back %i (that's %s)\n rx:", + ret, ret ? "bad" : "good"); + for (i = 0; i < bytes; ++i) + debug(" %u:%02x", i, rx[i]); + debug("\n"); + } + if (tx != dout) + free(tx); + if (rx != din) + free(rx); + + done: + if (flags & SPI_XFER_END) + spi_cs_deactivate(slave); + + return ret; +}
Hi Mike,
On Sun, Jan 22, 2012 at 10:30 PM, Mike Frysinger vapier@gentoo.org wrote:
This adds a SPI framework for people to hook up simulated SPI clients.
Signed-off-by: Mike Frysinger vapier@gentoo.org
arch/sandbox/include/asm/spi.h | 33 +++++++ drivers/spi/Makefile | 1 + drivers/spi/sandbox_spi.c | 183 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 0 deletions(-) create mode 100644 arch/sandbox/include/asm/spi.h create mode 100644 drivers/spi/sandbox_spi.c
I don't know the SPI interface as well as use - might be useful to have a few comments and debug() things when things go wrong I think. Please see below for some ideas.
diff --git a/arch/sandbox/include/asm/spi.h b/arch/sandbox/include/asm/spi.h new file mode 100644 index 0000000..082df37 --- /dev/null +++ b/arch/sandbox/include/asm/spi.h @@ -0,0 +1,33 @@ +/*
- Simulate a SPI port and clients
- Copyright (c) 2011-2012 The Chromium OS Authors.
- See file CREDITS for list of people who contributed to this
- project.
- Licensed under the GPL-2 or later.
- */
+#ifndef __ASM_SPI_H__ +#define __ASM_SPI_H__
+#include <linux/types.h>
+struct sb_spi_emu_ops {
- int (*setup)(void **priv, const char *spec);
- void (*free)(void *priv);
- void (*cs_activate)(void *priv);
- void (*cs_deactivate)(void *priv);
- int (*xfer)(void *priv, const u8 *tx, u8 *rx, uint bytes);
+};
I think this could do with some comments.
+static inline void sb_spi_tristate(u8 *buf, uint len)
What does this do - comments?
+{
- /*
- * XXX: make this into a user config option ? let them pick
- * whether this is pulled low, or high, or tristates.
- */
- memset(buf, 0xff, len);
+}
+#endif diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index c967d87..d80ff8b 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -40,6 +40,7 @@ COBJS-$(CONFIG_MXC_SPI) += mxc_spi.o COBJS-$(CONFIG_MXS_SPI) += mxs_spi.o COBJS-$(CONFIG_OC_TINY_SPI) += oc_tiny_spi.o COBJS-$(CONFIG_OMAP3_SPI) += omap3_spi.o +COBJS-$(CONFIG_SANDBOX_SPI) += sandbox_spi.o COBJS-$(CONFIG_SOFT_SPI) += soft_spi.o COBJS-$(CONFIG_SH_SPI) += sh_spi.o COBJS-$(CONFIG_FSL_ESPI) += fsl_espi.o diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c new file mode 100644 index 0000000..eda0f3e --- /dev/null +++ b/drivers/spi/sandbox_spi.c @@ -0,0 +1,183 @@ +/*
- Simulate a SPI port
- Copyright (c) 2011-2012 The Chromium OS Authors.
- See file CREDITS for list of people who contributed to this
- project.
- Licensed under the GPL-2 or later.
- */
+#include <common.h> +#include <malloc.h> +#include <spi.h> +#include <os.h>
+#include <asm/spi.h>
+#ifndef CONFIG_SPI_IDLE_VAL +# define CONFIG_SPI_IDLE_VAL 0xFF +#endif
+struct sb_spi_slave {
- struct spi_slave slave;
- const char *spec;
- const struct sb_spi_emu_ops *ops;
- void *priv;
+};
+#define to_sb_spi_slave(s) container_of(s, struct sb_spi_slave, slave)
+static const char *sb_lookup_arg(unsigned int bus, unsigned int cs) +{
- char sf_arg[20];
blank line?
- sprintf(sf_arg, "--spi-%u-%u", bus, cs);
- return os_getopt(sf_arg, 1);
+}
+static const struct {
- const char *spec;
- const struct sb_spi_emu_ops *ops;
+} sb_emu_map[] = { +};
+static int sb_parse_type(struct sb_spi_slave *sss) +{
- size_t i;
- for (i = 0; i < ARRAY_SIZE(sb_emu_map); ++i) {
- size_t len = strlen(sb_emu_map[i].spec);
- const char *sub_spec = sss->spec + len;
- sss->ops = sb_emu_map[i].ops;
- if (!memcmp(sss->spec, sb_emu_map[i].spec, len) &&
- sss->spec[len] == ':')
- return sss->ops->setup(&sss->priv, sub_spec + 1);
- }
- return 1;
+}
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
- return sb_lookup_arg(bus, cs) ? 1 : 0;
+}
+void spi_cs_activate(struct spi_slave *slave) +{
- struct sb_spi_slave *sss = to_sb_spi_slave(slave);
- if (sss->ops->cs_activate)
- sss->ops->cs_activate(sss->priv);
+}
+void spi_cs_deactivate(struct spi_slave *slave) +{
- struct sb_spi_slave *sss = to_sb_spi_slave(slave);
- if (sss->ops->cs_deactivate)
- sss->ops->cs_deactivate(sss->priv);
+}
+void spi_init(void) +{ +}
+void spi_set_speed(struct spi_slave *slave, uint hz) +{
Should this store the value somewhere?
+}
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
- unsigned int max_hz, unsigned int mode)
+{
- struct sb_spi_slave *sss;
- if (!spi_cs_is_valid(bus, cs))
- return NULL;
- sss = malloc(sizeof(*sss));
- if (!sss)
- return NULL;
Do we want a debug() message for this?
- sss->spec = sb_lookup_arg(bus, cs);
- if (sb_parse_type(sss)) {
- free(sss);
- printf("sandbox_spi: unable to locate a slave client\n");
- return NULL;
- }
- return &sss->slave;
+}
+void spi_free_slave(struct spi_slave *slave) +{
- struct sb_spi_slave *sss = to_sb_spi_slave(slave);
- if (sss->ops->free)
- sss->ops->free(sss->priv);
- free(sss);
+}
+int spi_claim_bus(struct spi_slave *slave) +{
Should we track claim/release for debugging purposes?
- return 0;
+}
+void spi_release_bus(struct spi_slave *slave) +{ +}
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
- void *din, unsigned long flags)
+{
- struct sb_spi_slave *sss = to_sb_spi_slave(slave);
- uint bytes = bitlen / 8, i;
- int ret = 0;
- u8 *tx = (void *)dout, *rx = din;
- if (bitlen == 0)
- goto done;
- /* we can only do 8 bit transfers */
- if (bitlen % 8) {
- flags |= SPI_XFER_END;
- goto done;
- }
debug() on these perhaps?
- if (flags & SPI_XFER_BEGIN)
- spi_cs_activate(slave);
- /* make sure rx/tx buffers are full so clients can assume */
- if (!tx) {
- tx = malloc(bytes);
- if (tx)
- memset(tx, CONFIG_SPI_IDLE_VAL, bytes);
- }
- if (!rx)
- rx = malloc(bytes);
- if (tx && rx) {
- debug("sb_xfer: bytes = %u\n tx:", bytes);
- for (i = 0; i < bytes; ++i)
- debug(" %u:%02x", i, tx[i]);
- debug("\n");
- ret = sss->ops->xfer(sss->priv, tx, rx, bytes);
- debug("sb_xfer: got back %i (that's %s)\n rx:",
- ret, ret ? "bad" : "good");
- for (i = 0; i < bytes; ++i)
- debug(" %u:%02x", i, rx[i]);
- debug("\n");
- }
debug() message if rx / tx is NULL?
- if (tx != dout)
- free(tx);
- if (rx != din)
- free(rx);
- done:
- if (flags & SPI_XFER_END)
- spi_cs_deactivate(slave);
- return ret;
+}
1.7.8.3
On Monday 23 January 2012 19:31:23 Simon Glass wrote:
On Sun, Jan 22, 2012 at 10:30 PM, Mike Frysinger wrote:
This adds a SPI framework for people to hook up simulated SPI clients.
I don't know the SPI interface as well as use - might be useful to have a few comments and debug() things when things go wrong I think. Please see below for some ideas.
i can add some more
+void spi_set_speed(struct spi_slave *slave, uint hz) +{
Should this store the value somewhere?
it could, but i'm not sure where it'd be used ... i don't want to get into simulating the actual line transitions :). the SID project is for that level of craziness: http://sourceware.org/sid/
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
unsigned int max_hz, unsigned int mode)+{
struct sb_spi_slave *sss;if (!spi_cs_is_valid(bus, cs))return NULL;sss = malloc(sizeof(*sss));if (!sss)return NULL;Do we want a debug() message for this?
for memory failures, i don't generally think so. for other probe type failures, sure.
+int spi_claim_bus(struct spi_slave *slave) +{
Should we track claim/release for debugging purposes?
ah, that'd be good -mike
This adds a SPI flash driver which simulates SPI flash clients. Currently supports the bare min that U-Boot requires: you can probe, read, erase, and write. Should be easy to extend to make it behave more exactly like a real SPI flash, but this is good enough to merge now.
Signed-off-by: Mike Frysinger vapier@gentoo.org --- drivers/mtd/spi/Makefile | 1 + drivers/mtd/spi/sandbox.c | 318 +++++++++++++++++++++++++++++++++++++++++++++ drivers/spi/sandbox_spi.c | 5 + 3 files changed, 324 insertions(+), 0 deletions(-) create mode 100644 drivers/mtd/spi/sandbox.c
diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile index 90f8392..fb37807 100644 --- a/drivers/mtd/spi/Makefile +++ b/drivers/mtd/spi/Makefile @@ -33,6 +33,7 @@ COBJS-$(CONFIG_SPI_FLASH) += spi_flash.o COBJS-$(CONFIG_SPI_FLASH_ATMEL) += atmel.o COBJS-$(CONFIG_SPI_FLASH_EON) += eon.o COBJS-$(CONFIG_SPI_FLASH_MACRONIX) += macronix.o +COBJS-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o COBJS-$(CONFIG_SPI_FLASH_SPANSION) += spansion.o COBJS-$(CONFIG_SPI_FLASH_SST) += sst.o COBJS-$(CONFIG_SPI_FLASH_STMICRO) += stmicro.o diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c new file mode 100644 index 0000000..a1bb641 --- /dev/null +++ b/drivers/mtd/spi/sandbox.c @@ -0,0 +1,318 @@ +/* + * Simulate a SPI flash + * + * Copyright (c) 2011-2012 The Chromium OS Authors. + * See file CREDITS for list of people who contributed to this + * project. + * + * Licensed under the GPL-2 or later. + */ + +#include <common.h> +#include <malloc.h> +#include <spi.h> +#include <os.h> + +#include <spi_flash.h> +#include "spi_flash_internal.h" + +#include <asm/spi.h> + +typedef enum { + SF_CMD, SF_ID, SF_READ, SF_WRITE, SF_ERASE, SF_READ_STATUS, SF_WREN, SF_WRDI, +} sb_sf_state; + +#define STAT_WIP (1 << 0) +#define STAT_WEL (1 << 1) + +struct sb_spi_flash_erase_commands { + u8 cmd; + u32 size; +}; +#define IDCODE_LEN 5 +#define MAX_ERASE_CMDS 2 +struct sb_spi_flash_data { + const char *name; + u8 idcode[IDCODE_LEN]; + u32 size; + const struct sb_spi_flash_erase_commands erase_cmds[MAX_ERASE_CMDS]; +}; + +struct sb_spi_flash { + sb_sf_state state; + uint off; + const struct sb_spi_flash_data *data; + int fd; + u8 status; +}; + +static const char *sb_sf_state_name(sb_sf_state state) +{ + static const char * const states[] = { + "CMD", "ID", "READ", "WRITE", "ERASE", "READ_STATUS", "WREN", "WRDI", + }; + return states[state]; +} + +static const struct sb_spi_flash_data sb_sf_flashes[] = { + { + "M25P16", { 0x20, 0x20, 0x15 }, (2 * 1024 * 1024), + { /* erase commands */ + { 0xd8, (64 * 1024), }, /* sector */ + { 0xc7, (2 * 1024 * 1024), }, /* bulk */ + }, + }, +}; + +static u8 sb_sf_0xff[0x10000]; + +static int sb_sf_setup(void **priv, const char *spec) +{ + /* spec = idcode:file */ + struct sb_spi_flash *sbsf; + const char *file; + size_t i, len, idname_len; + const struct sb_spi_flash_data *data; + + file = strchr(spec, ':'); + if (!file) + goto error; + idname_len = file - spec; + ++file; + + for (i = 0; i < ARRAY_SIZE(sb_sf_flashes); ++i) { + data = &sb_sf_flashes[i]; + len = strlen(data->name); + if (idname_len != len) + continue; + if (!memcmp(spec, data->name, len)) + break; + } + if (i == ARRAY_SIZE(sb_sf_flashes)) { + printf("sandbox_spi_flash: unknown flash '%*s'\n", + (int)idname_len, file); + goto error; + } + + if (sb_sf_0xff[0] == 0x00) + memset(sb_sf_0xff, 0xff, sizeof(sb_sf_0xff)); + + sbsf = malloc(sizeof(*sbsf)); + if (!sbsf) + goto error; + + sbsf->fd = os_open(file, 02); + if (sbsf->fd == -1) { + free(sbsf); + goto error; + } + + sbsf->state = SF_CMD; + sbsf->data = data; + + *priv = sbsf; + return 0; + + error: + printf("sandbox_spi_flash: unable to parse client spec\n"); + return 1; +} + +static void sb_sf_free(void *priv) +{ + struct sb_spi_flash *sbsf = priv; + + os_close(sbsf->fd); + free(sbsf); +} + +static void sb_sf_cs_deactivate(void *priv) +{ + struct sb_spi_flash *sbsf = priv; + + /* CS is no longer being asserted, so reset state */ + sbsf->state = SF_CMD; +} + +static int sb_sf_xfer(void *priv, const u8 *tx, u8 *rx, + uint bytes) +{ + struct sb_spi_flash *sbsf = priv; + uint i, written = 0; + + debug("sb_sf: state:%x(%s) bytes:%u\n", sbsf->state, + sb_sf_state_name(sbsf->state), bytes); + + if (sbsf->state == SF_CMD) { + sb_sf_state oldstate = sbsf->state; + + rx[written++] = 0; + sb_spi_tristate(rx, 1); + + /* + * XXX: we assume cmds that need addresses get them in one + * transfer rather than accumulating ... but u-boot + * doesn't care at all. + */ + + switch (tx[0]) { + case CMD_READ_ID: + sbsf->off = 0; + sbsf->state = SF_ID; + break; + case CMD_PAGE_PROGRAM: + /* XXX: check WEL in status */ + + /* + * XXX: need to handle exotic behavior: + * - unaligned addresses + * - too big of an address + */ + + sbsf->off = (tx[1] << 16) | (tx[2] << 8) | tx[3]; + os_lseek(sbsf->fd, sbsf->off, 0); + + written += 3; + sb_spi_tristate(&rx[1], 3); + + sbsf->state = SF_WRITE; + sbsf->status &= ~STAT_WEL; + break; + case CMD_READ_ARRAY_SLOW: + case CMD_READ_ARRAY_FAST: { + uint alen = 3; + + sbsf->off = (tx[1] << 16) | (tx[2] << 8) | tx[3]; + os_lseek(sbsf->fd, sbsf->off, 0); + debug(" read addr: %u\n", sbsf->off); + + if (tx[0] == CMD_READ_ARRAY_FAST) + /* read fast needs a dummy byte */ + ++alen; + + written += alen; + sb_spi_tristate(&rx[1], alen); + + sbsf->state = SF_READ; + break; + } + case CMD_WRITE_DISABLE: + debug(" write disabled\n"); + sbsf->status &= ~STAT_WEL; + break; + case CMD_READ_STATUS: + sbsf->state = SF_READ_STATUS; + break; + case CMD_WRITE_ENABLE: + debug(" write enabled\n"); + sbsf->status |= STAT_WEL; + break; + default: + /* handle erase commands first */ + for (i = 0; i < MAX_ERASE_CMDS; ++i) { + const struct sb_spi_flash_erase_commands *erase_cmd = + &sbsf->data->erase_cmds[i]; + + if (erase_cmd->cmd == 0x00) + continue; + if (tx[0] != erase_cmd->cmd) + continue; + + /* XXX: check WEL in status */ + + /* verify address is aligned */ + sbsf->off = (tx[1] << 16) | (tx[2] << 8) | tx[3]; + if (sbsf->off & ~(erase_cmd->size - 1)) { + debug(" sector erase: cmd:%#x needs align:%#x, but we got %#x\n", + erase_cmd->cmd, erase_cmd->size, sbsf->off); + sbsf->status &= ~STAT_WEL; + return 1; + } + + os_lseek(sbsf->fd, sbsf->off, 0); + debug(" sector erase addr: %u\n", sbsf->off); + + written += 3; + sb_spi_tristate(&rx[1], 3); + + /* XXX: latch WIP in status, and delay before clearing it ? */ + os_write(sbsf->fd, sb_sf_0xff, erase_cmd->size); + sbsf->status &= ~STAT_WEL; + goto cmd_done; + } + + debug(" cmd unknown: %#x\n", tx[0]); + return 1; + } + + cmd_done: + if (oldstate != sbsf->state) + debug(" cmd: transition to %s state\n", + sb_sf_state_name(sbsf->state)); + } + + if (written == bytes) + return 0; + + switch (sbsf->state) { + case SF_ID: { + const u8 *idcode = sbsf->data->idcode; + + debug(" id: off:%u\n tx:", sbsf->off); + for (i = sbsf->off; i < IDCODE_LEN; ++i) { + if (written < bytes) { + rx[written++] = idcode[i]; + debug(" %02x", idcode[i]); + } else + break; + } + if (written < bytes) { + i = bytes - written; + debug(" <memset remaining %u bytes>", i); + memset(rx + written, 0, i); + written += i; + } + debug("\n"); + break; + } + case SF_READ: + /* + * XXX: need to handle exotic behavior: + * - reading past end of device + */ + i = bytes - written; + debug(" tx: read(%u)\n", i); + if (i) + written += os_read(sbsf->fd, rx + written, i); + break; + case SF_READ_STATUS: + debug(" read status: %#x\n", sbsf->status); + i = bytes - written; + memset(rx + written, sbsf->status, i); + written += i; + break; + case SF_WRITE: + /* + * XXX: need to handle exotic behavior: + * - more than a page (256) worth of data + * - reading past end of device + */ + i = bytes - written; + debug(" rx: write(%u)\n", i); + if (i) + written += os_write(sbsf->fd, tx + written, i); + break; + default: + debug(" ??? no idea what to do ???\n"); + break; + } + + return written == bytes ? 0 : 1; +} + +const struct sb_spi_emu_ops sb_sf_ops = { + .setup = sb_sf_setup, + .free = sb_sf_free, + .cs_deactivate = sb_sf_cs_deactivate, + .xfer = sb_sf_xfer, +}; diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c index eda0f3e..ae15946 100644 --- a/drivers/spi/sandbox_spi.c +++ b/drivers/spi/sandbox_spi.c @@ -35,10 +35,15 @@ static const char *sb_lookup_arg(unsigned int bus, unsigned int cs) return os_getopt(sf_arg, 1); }
+extern const struct sb_spi_emu_ops sb_sf_ops; + static const struct { const char *spec; const struct sb_spi_emu_ops *ops; } sb_emu_map[] = { +#ifdef CONFIG_SPI_FLASH_SANDBOX + { "sf", &sb_sf_ops, }, +#endif };
static int sb_parse_type(struct sb_spi_slave *sss)
Hi Mike,
On Sun, Jan 22, 2012 at 10:30 PM, Mike Frysinger vapier@gentoo.org wrote:
This adds a SPI flash driver which simulates SPI flash clients. Currently supports the bare min that U-Boot requires: you can probe, read, erase, and write. Should be easy to extend to make it behave more exactly like a real SPI flash, but this is good enough to merge now.
Signed-off-by: Mike Frysinger vapier@gentoo.org
drivers/mtd/spi/Makefile | 1 + drivers/mtd/spi/sandbox.c | 318 +++++++++++++++++++++++++++++++++++++++++++++ drivers/spi/sandbox_spi.c | 5 + 3 files changed, 324 insertions(+), 0 deletions(-) create mode 100644 drivers/mtd/spi/sandbox.c
Again would like a few comments / docs. I'm pleased you have done this - it is much more comprehensive than my noddy implementation.
diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile index 90f8392..fb37807 100644 --- a/drivers/mtd/spi/Makefile +++ b/drivers/mtd/spi/Makefile @@ -33,6 +33,7 @@ COBJS-$(CONFIG_SPI_FLASH) += spi_flash.o COBJS-$(CONFIG_SPI_FLASH_ATMEL) += atmel.o COBJS-$(CONFIG_SPI_FLASH_EON) += eon.o COBJS-$(CONFIG_SPI_FLASH_MACRONIX) += macronix.o +COBJS-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o COBJS-$(CONFIG_SPI_FLASH_SPANSION) += spansion.o COBJS-$(CONFIG_SPI_FLASH_SST) += sst.o COBJS-$(CONFIG_SPI_FLASH_STMICRO) += stmicro.o diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c new file mode 100644 index 0000000..a1bb641 --- /dev/null +++ b/drivers/mtd/spi/sandbox.c @@ -0,0 +1,318 @@ +/*
- Simulate a SPI flash
- Copyright (c) 2011-2012 The Chromium OS Authors.
- See file CREDITS for list of people who contributed to this
- project.
- Licensed under the GPL-2 or later.
- */
+#include <common.h> +#include <malloc.h> +#include <spi.h> +#include <os.h>
+#include <spi_flash.h> +#include "spi_flash_internal.h"
+#include <asm/spi.h>
These are the different commands, right?
+typedef enum {
- SF_CMD, SF_ID, SF_READ, SF_WRITE, SF_ERASE, SF_READ_STATUS, SF_WREN, SF_WRDI,
+} sb_sf_state;
+#define STAT_WIP (1 << 0) +#define STAT_WEL (1 << 1)
What are these? Status bus? If so, perhaps a comment for each?
+struct sb_spi_flash_erase_commands {
- u8 cmd;
- u32 size;
+}; +#define IDCODE_LEN 5 +#define MAX_ERASE_CMDS 2 +struct sb_spi_flash_data {
- const char *name;
- u8 idcode[IDCODE_LEN];
- u32 size;
- const struct sb_spi_flash_erase_commands erase_cmds[MAX_ERASE_CMDS];
+};
+struct sb_spi_flash {
- sb_sf_state state;
- uint off;
- const struct sb_spi_flash_data *data;
- int fd;
- u8 status;
+};
All of the above could do with some comments IMO...
+static const char *sb_sf_state_name(sb_sf_state state) +{
- static const char * const states[] = {
- "CMD", "ID", "READ", "WRITE", "ERASE", "READ_STATUS", "WREN", "WRDI",
- };
I wonder if it would be better to put this array up next to the enum?
- return states[state];
+}
These are the emulated devices I think.
+static const struct sb_spi_flash_data sb_sf_flashes[] = {
- {
- "M25P16", { 0x20, 0x20, 0x15 }, (2 * 1024 * 1024),
- { /* erase commands */
- { 0xd8, (64 * 1024), }, /* sector */
- { 0xc7, (2 * 1024 * 1024), }, /* bulk */
Is <<10, <<20 better? Not sure.
- },
- },
+};
+static u8 sb_sf_0xff[0x10000];
Perhaps we should add an os_fputc() instead? Or perhaps write in smaller chunks?
+static int sb_sf_setup(void **priv, const char *spec) +{
- /* spec = idcode:file */
- struct sb_spi_flash *sbsf;
- const char *file;
- size_t i, len, idname_len;
- const struct sb_spi_flash_data *data;
- file = strchr(spec, ':');
- if (!file)
- goto error;
- idname_len = file - spec;
- ++file;
- for (i = 0; i < ARRAY_SIZE(sb_sf_flashes); ++i) {
- data = &sb_sf_flashes[i];
- len = strlen(data->name);
- if (idname_len != len)
- continue;
- if (!memcmp(spec, data->name, len))
- break;
- }
- if (i == ARRAY_SIZE(sb_sf_flashes)) {
- printf("sandbox_spi_flash: unknown flash '%*s'\n",
- (int)idname_len, file);
- goto error;
- }
- if (sb_sf_0xff[0] == 0x00)
- memset(sb_sf_0xff, 0xff, sizeof(sb_sf_0xff));
- sbsf = malloc(sizeof(*sbsf));
- if (!sbsf)
- goto error;
- sbsf->fd = os_open(file, 02);
- if (sbsf->fd == -1) {
- free(sbsf);
- goto error;
- }
- sbsf->state = SF_CMD;
- sbsf->data = data;
- *priv = sbsf;
- return 0;
- error:
- printf("sandbox_spi_flash: unable to parse client spec\n");
or out of memory or file would not open
- return 1;
+}
+static void sb_sf_free(void *priv) +{
- struct sb_spi_flash *sbsf = priv;
- os_close(sbsf->fd);
- free(sbsf);
+}
+static void sb_sf_cs_deactivate(void *priv) +{
- struct sb_spi_flash *sbsf = priv;
- /* CS is no longer being asserted, so reset state */
- sbsf->state = SF_CMD;
+}
+static int sb_sf_xfer(void *priv, const u8 *tx, u8 *rx,
- uint bytes)
+{
- struct sb_spi_flash *sbsf = priv;
- uint i, written = 0;
- debug("sb_sf: state:%x(%s) bytes:%u\n", sbsf->state,
- sb_sf_state_name(sbsf->state), bytes);
- if (sbsf->state == SF_CMD) {
I wonder if this if() could be split out as it makes the function very long. Might be hard if too many parameters though.
- sb_sf_state oldstate = sbsf->state;
- rx[written++] = 0;
- sb_spi_tristate(rx, 1);
- /*
- * XXX: we assume cmds that need addresses get them in one
- * transfer rather than accumulating ... but u-boot
- * doesn't care at all.
- */
- switch (tx[0]) {
- case CMD_READ_ID:
- sbsf->off = 0;
- sbsf->state = SF_ID;
- break;
- case CMD_PAGE_PROGRAM:
- /* XXX: check WEL in status */
- /*
- * XXX: need to handle exotic behavior:
- * - unaligned addresses
- * - too big of an address
- */
- sbsf->off = (tx[1] << 16) | (tx[2] << 8) | tx[3];
- os_lseek(sbsf->fd, sbsf->off, 0);
- written += 3;
- sb_spi_tristate(&rx[1], 3);
- sbsf->state = SF_WRITE;
- sbsf->status &= ~STAT_WEL;
- break;
- case CMD_READ_ARRAY_SLOW:
- case CMD_READ_ARRAY_FAST: {
- uint alen = 3;
- sbsf->off = (tx[1] << 16) | (tx[2] << 8) | tx[3];
- os_lseek(sbsf->fd, sbsf->off, 0);
- debug(" read addr: %u\n", sbsf->off);
- if (tx[0] == CMD_READ_ARRAY_FAST)
- /* read fast needs a dummy byte */
- ++alen;
- written += alen;
- sb_spi_tristate(&rx[1], alen);
- sbsf->state = SF_READ;
- break;
- }
Are you allowed to not indent the {} in this case? The } ends up at the same level as the switch().
- case CMD_WRITE_DISABLE:
- debug(" write disabled\n");
- sbsf->status &= ~STAT_WEL;
- break;
- case CMD_READ_STATUS:
- sbsf->state = SF_READ_STATUS;
- break;
- case CMD_WRITE_ENABLE:
- debug(" write enabled\n");
- sbsf->status |= STAT_WEL;
- break;
- default:
- /* handle erase commands first */
- for (i = 0; i < MAX_ERASE_CMDS; ++i) {
- const struct sb_spi_flash_erase_commands *erase_cmd =
- &sbsf->data->erase_cmds[i];
- if (erase_cmd->cmd == 0x00)
- continue;
- if (tx[0] != erase_cmd->cmd)
- continue;
- /* XXX: check WEL in status */
- /* verify address is aligned */
- sbsf->off = (tx[1] << 16) | (tx[2] << 8) | tx[3];
- if (sbsf->off & ~(erase_cmd->size - 1)) {
- debug(" sector erase: cmd:%#x needs align:%#x, but we got %#x\n",
- erase_cmd->cmd, erase_cmd->size, sbsf->off);
- sbsf->status &= ~STAT_WEL;
- return 1;
- }
- os_lseek(sbsf->fd, sbsf->off, 0);
- debug(" sector erase addr: %u\n", sbsf->off);
- written += 3;
- sb_spi_tristate(&rx[1], 3);
- /* XXX: latch WIP in status, and delay before clearing it ? */
- os_write(sbsf->fd, sb_sf_0xff, erase_cmd->size);
- sbsf->status &= ~STAT_WEL;
- goto cmd_done;
- }
- debug(" cmd unknown: %#x\n", tx[0]);
- return 1;
- }
- cmd_done:
- if (oldstate != sbsf->state)
- debug(" cmd: transition to %s state\n",
- sb_sf_state_name(sbsf->state));
- }
- if (written == bytes)
- return 0;
- switch (sbsf->state) {
- case SF_ID: {
- const u8 *idcode = sbsf->data->idcode;
- debug(" id: off:%u\n tx:", sbsf->off);
- for (i = sbsf->off; i < IDCODE_LEN; ++i) {
- if (written < bytes) {
- rx[written++] = idcode[i];
- debug(" %02x", idcode[i]);
- } else
- break;
- }
- if (written < bytes) {
- i = bytes - written;
Do you think you should use a different variable than i in this case and below? I doubt it would affect the code output.
- debug(" <memset remaining %u bytes>", i);
- memset(rx + written, 0, i);
- written += i;
- }
- debug("\n");
- break;
- }
- case SF_READ:
- /*
- * XXX: need to handle exotic behavior:
- * - reading past end of device
- */
- i = bytes - written;
- debug(" tx: read(%u)\n", i);
- if (i)
- written += os_read(sbsf->fd, rx + written, i);
- break;
- case SF_READ_STATUS:
- debug(" read status: %#x\n", sbsf->status);
- i = bytes - written;
- memset(rx + written, sbsf->status, i);
- written += i;
- break;
- case SF_WRITE:
- /*
- * XXX: need to handle exotic behavior:
- * - more than a page (256) worth of data
- * - reading past end of device
- */
- i = bytes - written;
- debug(" rx: write(%u)\n", i);
- if (i)
- written += os_write(sbsf->fd, tx + written, i);
- break;
- default:
- debug(" ??? no idea what to do ???\n");
- break;
- }
- return written == bytes ? 0 : 1;
+}
+const struct sb_spi_emu_ops sb_sf_ops = {
- .setup = sb_sf_setup,
- .free = sb_sf_free,
- .cs_deactivate = sb_sf_cs_deactivate,
- .xfer = sb_sf_xfer,
+}; diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c index eda0f3e..ae15946 100644 --- a/drivers/spi/sandbox_spi.c +++ b/drivers/spi/sandbox_spi.c @@ -35,10 +35,15 @@ static const char *sb_lookup_arg(unsigned int bus, unsigned int cs) return os_getopt(sf_arg, 1); }
+extern const struct sb_spi_emu_ops sb_sf_ops;
static const struct { const char *spec; const struct sb_spi_emu_ops *ops; } sb_emu_map[] = { +#ifdef CONFIG_SPI_FLASH_SANDBOX
- { "sf", &sb_sf_ops, },
+#endif };
static int sb_parse_type(struct sb_spi_slave *sss)
1.7.8.3
Regards, Simon
On Monday 23 January 2012 19:41:42 Simon Glass wrote:
On Sun, Jan 22, 2012 at 10:30 PM, Mike Frysinger wrote:
--- /dev/null +++ b/drivers/mtd/spi/sandbox.c
+#include <common.h> +#include <malloc.h> +#include <spi.h> +#include <os.h>
+#include <spi_flash.h> +#include "spi_flash_internal.h"
+#include <asm/spi.h>
These are the different commands, right?
are you referring to the enum after your quote ?
+typedef enum {
SF_CMD, SF_ID, SF_READ, SF_WRITE, SF_ERASE, SF_READ_STATUS,SF_WREN, SF_WRDI, +} sb_sf_state;
+#define STAT_WIP (1 << 0) +#define STAT_WEL (1 << 1)
What are these? Status bus? If so, perhaps a comment for each?
the STAT defines are for the status register (RDSR). these low bits are about the only ones that SPI flashes have in common. the others tend to diverge real quick (for things like locking different portions of the flash).
+static const char *sb_sf_state_name(sb_sf_state state) +{
static const char * const states[] = {"CMD", "ID", "READ", "WRITE", "ERASE", "READ_STATUS","WREN", "WRDI", + };
I wonder if it would be better to put this array up next to the enum?
my only reason for putting it here was to scope it. now only this func has access to the array and so it's the code enforces that.
+static const struct sb_spi_flash_data sb_sf_flashes[] = {
{"M25P16", { 0x20, 0x20, 0x15 }, (2 * 1024 * 1024),{ /* erase commands */{ 0xd8, (64 * 1024), }, /* sector */{ 0xc7, (2 * 1024 * 1024), }, /* bulk */Is <<10, <<20 better? Not sure.
personally i find (xxx * 1024 * 1024) easier to understand than bitshifts when it comes to sizes. but that could just be that i'm not used to reading things.
+static u8 sb_sf_0xff[0x10000];
Perhaps we should add an os_fputc() instead?
fputc() operates on a FILE*, not a fd. i don't think we want that. it's easy to emulate putc() with write().
however, we don't want to do that here. calling putc() 65 thousand times to avoid an array of 65 thousand bytes is the wrong trade off imo ;).
Or perhaps write in smaller chunks?
i picked 0x10000 because it was the largest erase block size and be lazy with a single write(). i could shrink it down to 0x1000 and then loop over in 4k chunks. but i'm not too worried about buffers that are 65kb when we talk about code that only runs on our 64bit dev platform ;).
+static int sb_sf_xfer(void *priv, const u8 *tx, u8 *rx,
uint bytes)+{
struct sb_spi_flash *sbsf = priv;uint i, written = 0;debug("sb_sf: state:%x(%s) bytes:%u\n", sbsf->state,sb_sf_state_name(sbsf->state), bytes);if (sbsf->state == SF_CMD) {I wonder if this if() could be split out as it makes the function very long. Might be hard if too many parameters though.
yeah, it is a long func. partly due to it starting small and me slowly expanding the state machine, and partly due to the state matching liking to be in one func. i might be able to make it work though ...
case CMD_READ_ARRAY_FAST: {uint alen = 3;sbsf->off = (tx[1] << 16) | (tx[2] << 8) | tx[3];os_lseek(sbsf->fd, sbsf->off, 0);debug(" read addr: %u\n", sbsf->off);if (tx[0] == CMD_READ_ARRAY_FAST)/* read fast needs a dummy byte */++alen;written += alen;sb_spi_tristate(&rx[1], alen);sbsf->state = SF_READ;break;}Are you allowed to not indent the {} in this case? The } ends up at the same level as the switch().
yes, it's ugly, but it's what i've seen in the kernel/u-boot. i just swallow hard and try not to look too hard.
switch (sbsf->state) {case SF_ID: {const u8 *idcode = sbsf->data->idcode;debug(" id: off:%u\n tx:", sbsf->off);for (i = sbsf->off; i < IDCODE_LEN; ++i) {if (written < bytes) {rx[written++] = idcode[i];debug(" %02x", idcode[i]);} elsebreak;}if (written < bytes) {i = bytes - written;Do you think you should use a different variable than i in this case and below? I doubt it would affect the code output.
i'm sure there's no code difference ;). i used "i" since it's available for scratch in this whole func. should be easy to tweak. -mike
We want to test SPI flash code in the sandbox, so enable the new drivers.
Signed-off-by: Mike Frysinger vapier@gentoo.org --- include/configs/sandbox.h | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 10565e6..c1962cd 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -51,6 +51,13 @@ #define CONFIG_ENV_SIZE 8192 #define CONFIG_ENV_IS_NOWHERE
+/* SPI */ +#define CONFIG_SANDBOX_SPI +#define CONFIG_CMD_SF +#define CONFIG_SPI_FLASH +#define CONFIG_SPI_FLASH_SANDBOX +#define CONFIG_SPI_FLASH_STMICRO + #define CONFIG_SYS_HZ 1000
/* Memory things - we don't really want a memory test */
Signed-off-by: Mike Frysinger vapier@gentoo.org --- arch/sandbox/cpu/os.c | 13 +++++++++++++ include/os.h | 1 + 2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 1d3e54f..4428f57 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -129,6 +129,19 @@ u64 os_get_nsec(void) #endif }
+const char *os_getenv(const char *name) +{ + /* We can't use getenv() as u-boot provides it own */ + extern char **environ; + size_t i, len = strlen(name); + + for (i = 0; environ[i]; ++i) + if (!strncmp(name, environ[i], len) && environ[i][len] == '=') + return &environ[i][len + 1]; + + return NULL; +} + extern char **sb_argv; const char *os_getopt(const char *name, int has_arg) { diff --git a/include/os.h b/include/os.h index cb88509..095a2ce 100644 --- a/include/os.h +++ b/include/os.h @@ -112,6 +112,7 @@ void os_usleep(unsigned long usec); */ u64 os_get_nsec(void);
+const char *os_getenv(const char *name); const char *os_getopt(const char *name, int has_arg);
#endif
Hi Mike,
On Sun, Jan 22, 2012 at 10:30 PM, Mike Frysinger vapier@gentoo.org wrote:
Signed-off-by: Mike Frysinger vapier@gentoo.org
Acked-by: Simon Glass sjg@chromium.org
arch/sandbox/cpu/os.c | 13 +++++++++++++ include/os.h | 1 + 2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 1d3e54f..4428f57 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -129,6 +129,19 @@ u64 os_get_nsec(void) #endif }
+const char *os_getenv(const char *name) +{
- /* We can't use getenv() as u-boot provides it own */
- extern char **environ;
- size_t i, len = strlen(name);
- for (i = 0; environ[i]; ++i)
- if (!strncmp(name, environ[i], len) && environ[i][len] == '=')
- return &environ[i][len + 1];
- return NULL;
+}
extern char **sb_argv; const char *os_getopt(const char *name, int has_arg) { diff --git a/include/os.h b/include/os.h index cb88509..095a2ce 100644 --- a/include/os.h +++ b/include/os.h @@ -112,6 +112,7 @@ void os_usleep(unsigned long usec); */ u64 os_get_nsec(void);
+const char *os_getenv(const char *name); const char *os_getopt(const char *name, int has_arg);
#endif
1.7.8.3
Is this used by earlier patches (iwc it probably should go earlier) or is it new?
I suppose we can use linker magic to make these sorts of functions available to os.c, but your way seems nicer.
Regards, Simon
On Monday 23 January 2012 19:44:15 Simon Glass wrote:
Is this used by earlier patches (iwc it probably should go earlier) or is it new?
it is not (anymore). i meant to only send out the first five, but this doesn't hurt. i'll probably leave it in my local branch until someone needs it.
I suppose we can use linker magic to make these sorts of functions available to os.c, but your way seems nicer.
it probably would be possible, but i think very fragile too ... -mike
participants (2)
-
Mike Frysinger -
Simon Glass