[PATCH] sst25l.c: simplify reading the device ManID/DevID

[PATCH] sst25l.c: simplify reading the device ManID/DevID

[H Hartley Sweeten]

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>

---

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)

Re: [PATCH] sst25l.c: simplify reading the device ManID/DevID

[Artem Bityutskiy]

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.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

RE: [PATCH] sst25l.c: simplify reading the device ManID/DevID

[H Hartley Sweeten]

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

RE: [PATCH] sst25l.c: simplify reading the device ManID/DevID

[Artem Bityutskiy]

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.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)