Skip to contentSkip to conversation

SPI: cmd_spi.c: add option to specify bus and mode

PATCH New Reinhard Meyer 2 participants 8 messages Aug 9, 2010 – Aug 26, 2010

lore
Share & tools
lore

Reinhard Meyer

author
#0 lore raw

PATCH 1/1 SPI: cmd_spi.c: add option to specify bus and mode

To facilitate better testing of SPI peripherals, add the possibility to specify bus and SPI mode to the "sspi" command

Signed-off-by Reinhard Meyer <[email protected]>
---
 common/cmd_spi.c |   33 +++++++++++++++++++++++----------
 1 files changed, 23 insertions(+), 10 deletions(-)
common/cmd_spi.c +23 -10
index bafa217..fd1b1f5 100644
@@ -47,7 +47,9 @@
 /*
  * Values from last command.
  */
-static unsigned int	device;
+static unsigned int	bus;
+static unsigned int	cs;
+static unsigned int	mode;
 static int   		bitlen;
 static uchar 		dout[MAX_SPI_BYTES];
 static uchar 		din[MAX_SPI_BYTES];

@@ -78,8 +80,18 @@ int do_spi (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	if ((flag & CMD_FLAG_REPEAT) == 0)
 	{
-		if (argc >= 2)
-			device = simple_strtoul(argv[1], NULL, 10);
+		if (argc >= 2) {
+			mode = CONFIG_DEFAULT_SPI_MODE;
+			bus = simple_strtoul(argv[1], &cp, 10);
+			if (*cp == ':') {
+				cs = simple_strtoul(cp+1, &cp, 10);
+				if (*cp == '.');
+					mode = simple_strtoul(cp+1, NULL, 10);
+			} else {
+				cs = bus;
+				bus = CONFIG_DEFAULT_SPI_BUS;
+			}
+		}
 		if (argc >= 3)
 			bitlen = simple_strtoul(argv[2], NULL, 10);
 		if (argc >= 4) {

@@ -107,15 +119,13 @@ int do_spi (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return 1;
 	}
 
-	/* FIXME: Make these parameters run-time configurable */
-	slave = spi_setup_slave(CONFIG_DEFAULT_SPI_BUS, device, 1000000,
-			CONFIG_DEFAULT_SPI_MODE);
+	slave = spi_setup_slave(bus, cs, 1000000, mode);
 	if (!slave) {
-		printf("Invalid device %d, giving up.\n", device);
+		printf("Invalid bus %d cs %d, giving up.\n", bus, cs);
 		return 1;
 	}
 
-	debug ("spi chipsel = %08X\n", device);
+	debug ("spi bus=%02x cs=%02x\n", bus, cs);
 
 	spi_claim_bus(slave);
 	if(spi_xfer(slave, bitlen, dout, din,

@@ -139,8 +149,11 @@ int do_spi (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 U_BOOT_CMD(
 	sspi,	5,	1,	do_spi,
 	"SPI utility commands",
-	"<device> <bit_len> <dout> - Send <bit_len> bits from <dout> out the SPI\n"
-	"<device>  - Identifies the chip select of the device\n"
+	"<cs> <bit_len> <dout> - Send <bit_len> bits from <dout> out the SPI\n"
+	"<bus>:<cs>[.<mode>] - extended form\n"
+	"<bus>     - Identifies the SPI bus of the device\n"
+	"<cs>      - Identifies the chip select of the device\n"
+	"<mode>    - Identifies the SPI mode to use for the transfer\n"
 	"<bit_len> - Number of bits to send (base 10)\n"
 	"<dout>    - Hexadecimal string that gets sent"
 );
Signature · 1 line
1.5.6.5
#1 lore raw

PATCH 1/1 SPI: cmd_spi.c: add option to specify bus and mode

Review int do_spi (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 124

Quoted 11 lines · Reinhard Meyer
(quoted diff) +2 -4
@@ -107,15 +119,13 @@ int do_spi (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 ? ? ? ? ? ? ? ?return 1;
 ? ? ? ?}
 
- ? ? ? /* FIXME: Make these parameters run-time configurable */
- ? ? ? slave = spi_setup_slave(CONFIG_DEFAULT_SPI_BUS, device, 1000000,
- ? ? ? ? ? ? ? ? ? ? ? CONFIG_DEFAULT_SPI_MODE);
+ ? ? ? slave = spi_setup_slave(bus, cs, 1000000, mode);
 ? ? ? ?if (!slave) {
- ? ? ? ? ? ? ? printf("Invalid device %d, giving up.\n", device);
+ ? ? ? ? ? ? ? printf("Invalid bus %d cs %d, giving up.\n", bus, cs);

i would use the simple naming convention like so:

    printf("Invalid device %d:%d\n", bus, cs);
Quoted 4 lines

?U_BOOT_CMD(
? ? ? ?sspi, ? 5, ? ? ?1, ? ? ?do_spi,
? ? ? ?"SPI utility commands",
- ? ? ? "<device> <bit_len> <dout> - Send <bit_len> bits from <dout> out the SPI\n"

Quoted 8 lines

- ? ? ? "<device> ?- Identifies the chip select of the device\n"
+ ? ? ? "<cs> <bit_len> <dout> - Send <bit_len> bits from <dout> out the SPI\n"
+ ? ? ? "<bus>:<cs>[.<mode>] - extended form\n"
+ ? ? ? "<bus> ? ? - Identifies the SPI bus of the device\n"
+ ? ? ? "<cs> ? ? ?- Identifies the chip select of the device\n"
+ ? ? ? "<mode> ? ?- Identifies the SPI mode to use for the transfer\n" ? ? ? ?"<bit_len> - Number of bits to send (base 10)\n"
? ? ? ?"<dout> ? ?- Hexadecimal string that gets sent"

this usage string no longer makes sense. how about:
"[<bus>:]<cs>[.<mode>] <bit_len> <dout> - Send <bit_len> bits from <dout> out the SPI\n"
-mike

author
#3 lore raw

PATCH 1/1 SPI: cmd_spi.c: add option to specify bus and mode

Dear Mike Frysinger,

Quoted 1 line

were you going to send an updated version ?

Soon. Currently I have other issues and a rather
"dirty" working tree :)

Reinhard

#4 lore raw

PATCH 1/1 SPI: cmd_spi.c: add option to specify bus and mode

Quoted 5 lines · On Tuesday, August 24, 2010 04:05:19 Reinhard Meyer wrote:

Dear Mike Frysinger,

Quoted 1 line

were you going to send an updated version ?

Soon. Currently I have other issues and a rather
"dirty" working tree :)

dont sweat it. just wanted to make sure it wasnt lost/forgotten/ignored. -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/20100824/1a45a508/attachment.pgp

author
#5 lore raw

PATCH 1/1 SPI: cmd_spi.c: add option to specify bus and mode

In reply to Mike Frysinger

Dear Mike Frysinger,

Quoted 3 lines

this usage string no longer makes sense. how about:
"[<bus>:]<cs>[.<mode>] <bit_len> <dout> - Send <bit_len> bits from <dout> out the SPI\n"

The problem is, that without blowing up the parser even more, only 2 forms of the command are valid:

the classic form

<cs> <bit_len> <dout>

or

<bus>:<cs>[.<mode>] <bit_len> <dout>

Since this command is mainly used for testing/debugging hardware, I saw no need to allow the form

<cs>.<mode> <bit_len> <dout>

If thats ok, I can make a patch with the help fixed.

Best Regards
Reinhard

#6 lore raw

PATCH 1/1 SPI: cmd_spi.c: add option to specify bus and mode

Quoted 7 lines · On Tuesday, August 24, 2010 06:53:49 Reinhard Meyer wrote:
Quoted 3 lines

this usage string no longer makes sense. how about:
"[<bus>:]<cs>[.<mode>] <bit_len> <dout> - Send <bit_len> bits from <dout> out the SPI\n"

The problem is, that without blowing up the parser even more, only 2 forms of the command are valid:

Quoted 7 lines

the classic form

<cs> <bit_len> <dout>

or

<bus>:<cs>[.<mode>] <bit_len> <dout>

how so ? the bus:cs.mode field is always going to be one argv, and your patch has it so that argv[1] is always parsed against this form. i dont see where this limitation is that you're seeing ...
-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/20100825/0c095ac0/attachment.pgp

author
#7 lore raw

PATCH 1/1 SPI: cmd_spi.c: add option to specify bus and mode

Dear Mike Frysinger,

Quoted 6 lines
Quoted 1 line

<bus>:<cs>[.<mode>]<bit_len> <dout>

how so ? the bus:cs.mode field is always going to be one argv, and your patch has it so that argv[1] is always parsed against this form. i dont see where this limitation is that you're seeing ...
-mike

Currently, without a ":", the "." part won't be handled. But I see that a simple rearrangement would solve this. I'll fix and test that.

Reinhard