Patchwork sst25l.c: simplify reading the device ManID/DevID

login
register
mail settings
Submitter hartleys
Date April 20, 2010, 11:17 p.m.
Message ID <0D753D10438DA54287A00B0270842697636D85BD7C@AUSP01VMBX24.collaborationhost.net>
Download mbox | patch
Permalink /patch/50604/
State New
Headers show

Comments

hartleys - April 20, 2010, 11:17 p.m.
The Read-ID command will continuously output the Manufacture ID and Device ID
until the command is terminated by a low to high transition on the CE# pin.
We can take advantage of this in the sst25l_match_device routine by reading
both bytes in one spi_write_then_read transaction.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Andre Renaud <andre@bluewatersys.com>
Cc: Ryan Mallon <ryan@bluewatersys.com>

---
Artem Bityutskiy - April 29, 2010, 5:36 a.m.
On Tue, 2010-04-20 at 18:17 -0500, H Hartley Sweeten wrote:
> The Read-ID command will continuously output the Manufacture ID and Device ID
> until the command is terminated by a low to high transition on the CE# pin.
> We can take advantage of this in the sst25l_match_device routine by reading
> both bytes in one spi_write_then_read transaction.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Andre Renaud <andre@bluewatersys.com>
> Cc: Ryan Mallon <ryan@bluewatersys.com>

Pushed to l2-mtd-2.6.git / dunno.
hartleys - April 29, 2010, 5:11 p.m.
On Wednesday, April 28, 2010 10:37 PM, Artem Bityutskiy wrote:
> On Tue, 2010-04-20 at 18:17 -0500, H Hartley Sweeten wrote:
>> The Read-ID command will continuously output the Manufacture ID and Device ID
>> until the command is terminated by a low to high transition on the CE# pin.
>> We can take advantage of this in the sst25l_match_device routine by reading
>> both bytes in one spi_write_then_read transaction.
>> 
>> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>> Cc: David Woodhouse <dwmw2@infradead.org>
>> Cc: Andre Renaud <andre@bluewatersys.com>
>> Cc: Ryan Mallon <ryan@bluewatersys.com>
>
> Pushed to l2-mtd-2.6.git / dunno.

Artem,

I have discovered that the Read-Status-Register command has the same problem.
With the SST25L SPI flash chips, if the chip enable is deasserted after sending
a command that command will get aborted.

I ran across this while testing a new spi master driver for the ep93xx on an
EDB9307A dev board.  That board uses the processors SFRMOUT signal as part of
the chip select logic.  Unfortunately the ep93xx only asserts the SFRMOUT
signal as long as the spi transmit fifo contains data.  As soon as the last
bit is clocked into the receive fifo it gets deasserted.  Many of the other
ep93xx based boards have that same issue.

I have an updated patch that changes both of these into one synchronous message
which fixes the sst25l_status and sst25l_match_device functions.  These changes
should be transparent to any users of this driver.

Could you drop the current patch and I will submit the updated one for review?

Regards,
Hartley
Artem Bityutskiy - April 29, 2010, 6:07 p.m.
On Thu, 2010-04-29 at 12:11 -0500, H Hartley Sweeten wrote:
> On Wednesday, April 28, 2010 10:37 PM, Artem Bityutskiy wrote:
> > On Tue, 2010-04-20 at 18:17 -0500, H Hartley Sweeten wrote:
> >> The Read-ID command will continuously output the Manufacture ID and Device ID
> >> until the command is terminated by a low to high transition on the CE# pin.
> >> We can take advantage of this in the sst25l_match_device routine by reading
> >> both bytes in one spi_write_then_read transaction.
> >> 
> >> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> >> Cc: David Woodhouse <dwmw2@infradead.org>
> >> Cc: Andre Renaud <andre@bluewatersys.com>
> >> Cc: Ryan Mallon <ryan@bluewatersys.com>
> >
> > Pushed to l2-mtd-2.6.git / dunno.
> 
> Artem,
> 
> I have discovered that the Read-Status-Register command has the same problem.
> With the SST25L SPI flash chips, if the chip enable is deasserted after sending
> a command that command will get aborted.
> 
> I ran across this while testing a new spi master driver for the ep93xx on an
> EDB9307A dev board.  That board uses the processors SFRMOUT signal as part of
> the chip select logic.  Unfortunately the ep93xx only asserts the SFRMOUT
> signal as long as the spi transmit fifo contains data.  As soon as the last
> bit is clocked into the receive fifo it gets deasserted.  Many of the other
> ep93xx based boards have that same issue.
> 
> I have an updated patch that changes both of these into one synchronous message
> which fixes the sst25l_status and sst25l_match_device functions.  These changes
> should be transparent to any users of this driver.
> 
> Could you drop the current patch and I will submit the updated one for review?

Of course, just send it.

Patch

diff --git a/drivers/mtd/devices/sst25l.c b/drivers/mtd/devices/sst25l.c
index fe17054..b13ca75 100644
--- a/drivers/mtd/devices/sst25l.c
+++ b/drivers/mtd/devices/sst25l.c
@@ -328,7 +328,7 @@  out:
 static struct flash_info *__init sst25l_match_device(struct spi_device *spi)
 {
 	struct flash_info *flash_info = NULL;
-	unsigned char command[4], response;
+	unsigned char command[4], response[2];
 	int i, err;
 	uint16_t id;
 
@@ -336,25 +336,14 @@  static struct flash_info *__init sst25l_match_device(struct spi_device *spi)
 	command[1] = 0;
 	command[2] = 0;
 	command[3] = 0;
-	err = spi_write_then_read(spi, command, sizeof(command), &response, 1);
+	err = spi_write_then_read(spi, command, sizeof(command),
+				       response, sizeof(response));
 	if (err < 0) {
 		dev_err(&spi->dev, "error reading device id msb\n");
 		return NULL;
 	}
 
-	id = response << 8;
-
-	command[0] = SST25L_CMD_READ_ID;
-	command[1] = 0;
-	command[2] = 0;
-	command[3] = 1;
-	err = spi_write_then_read(spi, command, sizeof(command), &response, 1);
-	if (err < 0) {
-		dev_err(&spi->dev, "error reading device id lsb\n");
-		return NULL;
-	}
-
-	id |= response;
+	id = (response[0] << 8) | response[1];
 
 	for (i = 0; i < ARRAY_SIZE(sst25l_flash_info); i++)
 		if (sst25l_flash_info[i].device_id == id)