| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20120329200150.GA27005@pengutronix.de>
Date: Thu, 29 Mar 2012 22:01:50 +0200
From: Wolfram Sang <w.sang@...gutronix.de>
To: Ivo Sieben <meltedpianoman@...il.com>
Cc: linux-kernel@...r.kernel.org, Chris Wright <chrisw@...s-sol.org>,
Jean Delvare <khali@...ux-fr.org>
Subject: Re: [PATCH-v2] Support M95040 SPI EEPROM
On Wed, Mar 28, 2012 at 03:50:02PM +0200, Ivo Sieben wrote:
> Updated the generic SPI EEPROM driver AT25 for support of an additional address
> bit in the instruction byte. Certain EEPROMS have a size that is larger than the
> number of address bytes would allow (e.g. like M95040 from ST that has 512 Byte
> size but uses only one address byte (A0 to A7) for addressing.) For the extra
> address bit (A8, A16 or A24) bit 3 of the instruction byte is used. This
> instruction bit is normally defined as don't care for other AT25 like chips.
>
> v2: Introduced a flag EE_INSTR_BIT3_IS_ADDR to support additional address bits
> in READ/WRITE instruction.
Yeah, this one looks promising. Thanks!
>
> Signed-off-by: Ivo Sieben <meltedpianoman@...il.com>
> ---
> drivers/misc/eeprom/at25.c | 24 +++++++++++++++++++++---
> include/linux/spi/eeprom.h | 10 ++++++++++
> 2 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
> index c627e41..5e0650f 100644
> --- a/drivers/misc/eeprom/at25.c
> +++ b/drivers/misc/eeprom/at25.c
> @@ -35,6 +35,7 @@ struct at25_data {
> struct spi_eeprom chip;
> struct bin_attribute bin;
> unsigned addrlen;
> + unsigned instr_bit3_address_offset;
I think we can live without this new member. See below
> };
>
> #define AT25_WREN 0x06 /* latch the write enable */
> @@ -50,6 +51,7 @@ struct at25_data {
> #define AT25_SR_BP1 0x08
> #define AT25_SR_WPEN 0x80 /* writeprotect enable */
>
> +#define AT25_INSTR_BIT3 0x08 /* Additional address bit in instr */
>
> #define EE_MAXADDRLEN 3 /* 24 bit addresses, up to 2 MBytes */
>
> @@ -75,6 +77,7 @@ at25_ee_read(
> ssize_t status;
> struct spi_transfer t[2];
> struct spi_message m;
> + u8 instr;
>
> if (unlikely(offset >= at25->bin.size))
> return 0;
> @@ -84,7 +87,12 @@ at25_ee_read(
> return count;
>
> cp = command;
> - *cp++ = AT25_READ;
> +
> + instr = AT25_READ;
> + if (at25->instr_bit3_address_offset)
> + if (offset >= at25->instr_bit3_address_offset)
> + instr |= AT25_INSTR_BIT3;
What about:
if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR && (offset >> (at25->addrlen * 8)))
instr |= AT25_INSTR_BIT3;
> + *cp++ = instr;
>
> /* 8/16/24-bit address is written MSB first */
> switch (at25->addrlen) {
> @@ -167,14 +175,14 @@ at25_ee_write(struct at25_data *at25, const char *buf, loff_t off,
> /* For write, rollover is within the page ... so we write at
> * most one page, then manually roll over to the next page.
> */
> - bounce[0] = AT25_WRITE;
> mutex_lock(&at25->lock);
> do {
> unsigned long timeout, retries;
> unsigned segment;
> unsigned offset = (unsigned) off;
> - u8 *cp = bounce + 1;
> + u8 *cp = bounce;
> int sr;
> + u8 instr;
>
> *cp = AT25_WREN;
> status = spi_write(at25->spi, cp, 1);
> @@ -184,6 +192,12 @@ at25_ee_write(struct at25_data *at25, const char *buf, loff_t off,
> break;
> }
>
> + instr = AT25_WRITE;
> + if (at25->instr_bit3_address_offset)
> + if (offset >= at25->instr_bit3_address_offset)
> + instr |= AT25_INSTR_BIT3;
Ditto.
> + *cp++ = instr;
> +
> /* 8/16/24-bit address is written MSB first */
> switch (at25->addrlen) {
> default: /* case 3 */
> @@ -296,6 +310,7 @@ static int at25_probe(struct spi_device *spi)
> int err;
> int sr;
> int addrlen;
> + unsigned instr_bit3_address_offset = 0;
>
> /* Chip description */
> chip = spi->dev.platform_data;
> @@ -317,6 +332,8 @@ static int at25_probe(struct spi_device *spi)
> err = -EINVAL;
> goto fail;
> }
> + if (chip->flags & EE_INSTR_BIT3_IS_ADDR)
> + instr_bit3_address_offset = 1U << (addrlen * 8);
>
> /* Ping the chip ... the status register is pretty portable,
> * unlike probing manufacturer IDs. We do expect that system
> @@ -339,6 +356,7 @@ static int at25_probe(struct spi_device *spi)
> at25->spi = spi_dev_get(spi);
> dev_set_drvdata(&spi->dev, at25);
> at25->addrlen = addrlen;
> + at25->instr_bit3_address_offset = instr_bit3_address_offset;
Then we could also skip all this in probe.
>
> /* Export the EEPROM bytes through sysfs, since that's convenient.
> * And maybe to other kernel code; it might hold a board's Ethernet
> diff --git a/include/linux/spi/eeprom.h b/include/linux/spi/eeprom.h
> index 306e7b1..403e007 100644
> --- a/include/linux/spi/eeprom.h
> +++ b/include/linux/spi/eeprom.h
> @@ -20,6 +20,16 @@ struct spi_eeprom {
> #define EE_ADDR3 0x0004 /* 24 bit addrs */
> #define EE_READONLY 0x0008 /* disallow writes */
>
> + /*
> + * Certain EEPROMS have a size that is larger than the number of address
> + * bytes would allow (e.g. like M95040 from ST that has 512 Byte size
> + * but uses only one address byte (A0 to A7) for addressing.) For
> + * the extra address bit (A8, A16 or A24) bit 3 of the instruction byte
> + * is used. This instruction bit is normally defined as don't care for
> + * other AT25 like chips.
> + */
> +#define EE_INSTR_BIT3_IS_ADDR 0x0010
> +
Good documentation!
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)
Powered by blists - more mailing lists