Re: [PATCH v2 4/5] misc: eeprom_93xx46: Add quirks to support Atmel AT93C46D device.
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 11/19/2015 12:59 AM, Vladimir Zapolskiy wrote:
> On 19.11.2015 05:29, Cory Tusar wrote:
>> Atmel devices in this family have some quirks not found in other similar
>> chips - they do not support a sequential read of the entire EEPROM
>> contents, and the control word sent at the start of each operation
>> varies in bit length.
>>
>> This commit adds quirk support to the driver and modifies the read
>> implementation to support non-sequential reads for consistency with
>> other misc/eeprom drivers.
>>
>> Tested on a custom Freescale VF610-based platform, with an AT93C46D
>> device attached via dspi2. The spi-gpio driver was used to allow the
>> necessary non-byte-sized transfers.
>>
>> Signed-off-by: Cory Tusar
>> ---
>> drivers/misc/eeprom/eeprom_93xx46.c | 128
>> ++--
>> include/linux/eeprom_93xx46.h | 6 ++
>> 2 files changed, 98 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c
>> b/drivers/misc/eeprom/eeprom_93xx46.c
>> index 1f29d9a..0386b03 100644
>> --- a/drivers/misc/eeprom/eeprom_93xx46.c
>> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
>> @@ -27,6 +27,15 @@
>> #define ADDR_ERAL 0x20
>> #define ADDR_EWEN 0x30
>>
>> +struct eeprom_93xx46_devtype_data {
>> +unsigned int quirks;
>> +};
>> +
>> +static const struct eeprom_93xx46_devtype_data atmel_at93c46d_data = {
>> +.quirks = EEPROM_93XX46_QUIRK_SINGLE_WORD_READ |
>> + EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH,
>> +};
>> +
>> struct eeprom_93xx46_dev {
>> struct spi_device *spi;
>> struct eeprom_93xx46_platform_data *pdata;
>> @@ -35,6 +44,16 @@ struct eeprom_93xx46_dev {
>> int addrlen;
>> };
>>
>> +static inline bool has_quirk_single_word_read(struct eeprom_93xx46_dev
>> *edev)
>> +{
>> +return !!(edev->pdata->quirks & EEPROM_93XX46_QUIRK_SINGLE_WORD_READ);
>
> bool return type will do the work, please remove !!.
Will fix.
>> +}
>> +
>> +static inline bool has_quirk_instruction_length(struct eeprom_93xx46_dev
>> *edev)
>> +{
>> +return !!(edev->pdata->quirks & EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH);
>
> Same here.
Will fix.
>> +}
>> +
>> static ssize_t
>> eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
>> struct bin_attribute *bin_attr,
>> @@ -42,58 +61,73 @@ eeprom_93xx46_bin_read(struct file *filp, struct kobject
>> *kobj,
>> {
>> struct eeprom_93xx46_dev *edev;
>> struct device *dev;
>> -struct spi_message m;
>> -struct spi_transfer t[2];
>> -int bits, ret;
>> -u16 cmd_addr;
>> +ssize_t ret = 0;
>>
>> dev = container_of(kobj, struct device, kobj);
>> edev = dev_get_drvdata(dev);
>>
>> -cmd_addr = OP_READ << edev->addrlen;
>> +mutex_lock(&edev->lock);
>>
>> -if (edev->addrlen == 7) {
>> -cmd_addr |= off & 0x7f;
>> -bits = 10;
>> -} else {
>> -cmd_addr |= (off >> 1) & 0x3f;
>> -bits = 9;
>> -}
>> +if (edev->pdata->prepare)
>> +edev->pdata->prepare(edev);
>>
>> -dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
>> -cmd_addr, edev->spi->max_speed_hz);
>> +while (count) {
>> +struct spi_message m;
>> +struct spi_transfer t[2] = { { 0 } };
>
> Just { 0 };
No...just '{ 0 }' may be functional, but results in a compiler
warning.
>> +u16 cmd_addr = OP_READ << edev->addrlen;
>> +size_t nbytes = count;
>> +int bits;
>> +int err;
>> +
>> +if (edev->addrlen == 7) {
>> +cmd_addr |= off & 0x7f;
>> +bits = 10;
>> +if (has_quirk_single_word_read(edev))
>> +nbytes = 1;
>> +} else {
>> +cmd_addr |= (off >> 1) & 0x3f;
>> +bits = 9;
>> +if (has_quirk_single_word_read(edev))
>> +nbytes = 2;
>> +}
>>
>> -spi_message_init(&m);
>> -memset(t, 0, sizeof(t));
>> +dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
>> +cmd_addr, edev->spi->max_speed_hz);
>>
>> -t[0].tx_buf = (char *)&cmd_addr;
>> -t[0].len = 2;
>> -t[0].bits_per_word = bits;
>> -spi_message_add_tail(&t[0], &m);
>> +spi_message_init(&m);
>>
>> -t[1].rx_buf = buf;
>> -t[1].len = count;
>> -t[1].bits_per_word = 8;
>> -spi_message_add_tail(&t[1], &m);
>> +t[0].tx_buf = (char *)&cmd_addr;
>> +t[0].len = 2;
>> +t[0].bits_per_word = bits;
>> +spi_message_add_tail(&t[0], &m);
>>
>> -mutex_lock(&edev->lock);
>> +t[1].rx_buf = buf;
>> +t[1].len = count;
>> +t[1].bits_per_word = 8;
>> +spi_message_add_tail(&t[1], &m);
>>
>> -if (edev->pdata->prepare)
>> -
Re: [PATCH v2 4/5] misc: eeprom_93xx46: Add quirks to support Atmel AT93C46D device.
On 19.11.2015 05:29, Cory Tusar wrote:
> Atmel devices in this family have some quirks not found in other similar
> chips - they do not support a sequential read of the entire EEPROM
> contents, and the control word sent at the start of each operation
> varies in bit length.
>
> This commit adds quirk support to the driver and modifies the read
> implementation to support non-sequential reads for consistency with
> other misc/eeprom drivers.
>
> Tested on a custom Freescale VF610-based platform, with an AT93C46D
> device attached via dspi2. The spi-gpio driver was used to allow the
> necessary non-byte-sized transfers.
>
> Signed-off-by: Cory Tusar
> ---
> drivers/misc/eeprom/eeprom_93xx46.c | 128
> ++--
> include/linux/eeprom_93xx46.h | 6 ++
> 2 files changed, 98 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c
> b/drivers/misc/eeprom/eeprom_93xx46.c
> index 1f29d9a..0386b03 100644
> --- a/drivers/misc/eeprom/eeprom_93xx46.c
> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
> @@ -27,6 +27,15 @@
> #define ADDR_ERAL0x20
> #define ADDR_EWEN0x30
>
> +struct eeprom_93xx46_devtype_data {
> + unsigned int quirks;
> +};
> +
> +static const struct eeprom_93xx46_devtype_data atmel_at93c46d_data = {
> + .quirks = EEPROM_93XX46_QUIRK_SINGLE_WORD_READ |
> + EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH,
> +};
> +
> struct eeprom_93xx46_dev {
> struct spi_device *spi;
> struct eeprom_93xx46_platform_data *pdata;
> @@ -35,6 +44,16 @@ struct eeprom_93xx46_dev {
> int addrlen;
> };
>
> +static inline bool has_quirk_single_word_read(struct eeprom_93xx46_dev *edev)
> +{
> + return !!(edev->pdata->quirks & EEPROM_93XX46_QUIRK_SINGLE_WORD_READ);
bool return type will do the work, please remove !!.
> +}
> +
> +static inline bool has_quirk_instruction_length(struct eeprom_93xx46_dev
> *edev)
> +{
> + return !!(edev->pdata->quirks & EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH);
Same here.
> +}
> +
> static ssize_t
> eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
> struct bin_attribute *bin_attr,
> @@ -42,58 +61,73 @@ eeprom_93xx46_bin_read(struct file *filp, struct kobject
> *kobj,
> {
> struct eeprom_93xx46_dev *edev;
> struct device *dev;
> - struct spi_message m;
> - struct spi_transfer t[2];
> - int bits, ret;
> - u16 cmd_addr;
> + ssize_t ret = 0;
>
> dev = container_of(kobj, struct device, kobj);
> edev = dev_get_drvdata(dev);
>
> - cmd_addr = OP_READ << edev->addrlen;
> + mutex_lock(&edev->lock);
>
> - if (edev->addrlen == 7) {
> - cmd_addr |= off & 0x7f;
> - bits = 10;
> - } else {
> - cmd_addr |= (off >> 1) & 0x3f;
> - bits = 9;
> - }
> + if (edev->pdata->prepare)
> + edev->pdata->prepare(edev);
>
> - dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
> - cmd_addr, edev->spi->max_speed_hz);
> + while (count) {
> + struct spi_message m;
> + struct spi_transfer t[2] = { { 0 } };
Just { 0 };
> + u16 cmd_addr = OP_READ << edev->addrlen;
> + size_t nbytes = count;
> + int bits;
> + int err;
> +
> + if (edev->addrlen == 7) {
> + cmd_addr |= off & 0x7f;
> + bits = 10;
> + if (has_quirk_single_word_read(edev))
> + nbytes = 1;
> + } else {
> + cmd_addr |= (off >> 1) & 0x3f;
> + bits = 9;
> + if (has_quirk_single_word_read(edev))
> + nbytes = 2;
> + }
>
> - spi_message_init(&m);
> - memset(t, 0, sizeof(t));
> + dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
> + cmd_addr, edev->spi->max_speed_hz);
>
> - t[0].tx_buf = (char *)&cmd_addr;
> - t[0].len = 2;
> - t[0].bits_per_word = bits;
> - spi_message_add_tail(&t[0], &m);
> + spi_message_init(&m);
>
> - t[1].rx_buf = buf;
> - t[1].len = count;
> - t[1].bits_per_word = 8;
> - spi_message_add_tail(&t[1], &m);
> + t[0].tx_buf = (char *)&cmd_addr;
> + t[0].len = 2;
> + t[0].bits_per_word = bits;
> + spi_message_add_tail(&t[0], &m);
>
> - mutex_lock(&edev->lock);
> + t[1].rx_buf = buf;
> + t[1].len = count;
> + t[1].bits_per_word = 8;
> + spi_message_add_tail(&t[1], &m);
>
> - if (edev->pdata->prepare)
> - edev->pdata->prepare(edev);
> + err = spi_sync(edev->spi, &m);
> + /* have to wait at least Tcsl ns */
> + ndelay(250);
>
> - ret = spi_sync(edev->spi, &m);
> - /* have to wait at least Tcsl ns */
> - ndelay(250);
> -
[PATCH v2 4/5] misc: eeprom_93xx46: Add quirks to support Atmel AT93C46D device.
Atmel devices in this family have some quirks not found in other similar
chips - they do not support a sequential read of the entire EEPROM
contents, and the control word sent at the start of each operation
varies in bit length.
This commit adds quirk support to the driver and modifies the read
implementation to support non-sequential reads for consistency with
other misc/eeprom drivers.
Tested on a custom Freescale VF610-based platform, with an AT93C46D
device attached via dspi2. The spi-gpio driver was used to allow the
necessary non-byte-sized transfers.
Signed-off-by: Cory Tusar
---
drivers/misc/eeprom/eeprom_93xx46.c | 128 ++--
include/linux/eeprom_93xx46.h | 6 ++
2 files changed, 98 insertions(+), 36 deletions(-)
diff --git a/drivers/misc/eeprom/eeprom_93xx46.c
b/drivers/misc/eeprom/eeprom_93xx46.c
index 1f29d9a..0386b03 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -27,6 +27,15 @@
#define ADDR_ERAL 0x20
#define ADDR_EWEN 0x30
+struct eeprom_93xx46_devtype_data {
+ unsigned int quirks;
+};
+
+static const struct eeprom_93xx46_devtype_data atmel_at93c46d_data = {
+ .quirks = EEPROM_93XX46_QUIRK_SINGLE_WORD_READ |
+ EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH,
+};
+
struct eeprom_93xx46_dev {
struct spi_device *spi;
struct eeprom_93xx46_platform_data *pdata;
@@ -35,6 +44,16 @@ struct eeprom_93xx46_dev {
int addrlen;
};
+static inline bool has_quirk_single_word_read(struct eeprom_93xx46_dev *edev)
+{
+ return !!(edev->pdata->quirks & EEPROM_93XX46_QUIRK_SINGLE_WORD_READ);
+}
+
+static inline bool has_quirk_instruction_length(struct eeprom_93xx46_dev *edev)
+{
+ return !!(edev->pdata->quirks & EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH);
+}
+
static ssize_t
eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr,
@@ -42,58 +61,73 @@ eeprom_93xx46_bin_read(struct file *filp, struct kobject
*kobj,
{
struct eeprom_93xx46_dev *edev;
struct device *dev;
- struct spi_message m;
- struct spi_transfer t[2];
- int bits, ret;
- u16 cmd_addr;
+ ssize_t ret = 0;
dev = container_of(kobj, struct device, kobj);
edev = dev_get_drvdata(dev);
- cmd_addr = OP_READ << edev->addrlen;
+ mutex_lock(&edev->lock);
- if (edev->addrlen == 7) {
- cmd_addr |= off & 0x7f;
- bits = 10;
- } else {
- cmd_addr |= (off >> 1) & 0x3f;
- bits = 9;
- }
+ if (edev->pdata->prepare)
+ edev->pdata->prepare(edev);
- dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
- cmd_addr, edev->spi->max_speed_hz);
+ while (count) {
+ struct spi_message m;
+ struct spi_transfer t[2] = { { 0 } };
+ u16 cmd_addr = OP_READ << edev->addrlen;
+ size_t nbytes = count;
+ int bits;
+ int err;
+
+ if (edev->addrlen == 7) {
+ cmd_addr |= off & 0x7f;
+ bits = 10;
+ if (has_quirk_single_word_read(edev))
+ nbytes = 1;
+ } else {
+ cmd_addr |= (off >> 1) & 0x3f;
+ bits = 9;
+ if (has_quirk_single_word_read(edev))
+ nbytes = 2;
+ }
- spi_message_init(&m);
- memset(t, 0, sizeof(t));
+ dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
+ cmd_addr, edev->spi->max_speed_hz);
- t[0].tx_buf = (char *)&cmd_addr;
- t[0].len = 2;
- t[0].bits_per_word = bits;
- spi_message_add_tail(&t[0], &m);
+ spi_message_init(&m);
- t[1].rx_buf = buf;
- t[1].len = count;
- t[1].bits_per_word = 8;
- spi_message_add_tail(&t[1], &m);
+ t[0].tx_buf = (char *)&cmd_addr;
+ t[0].len = 2;
+ t[0].bits_per_word = bits;
+ spi_message_add_tail(&t[0], &m);
- mutex_lock(&edev->lock);
+ t[1].rx_buf = buf;
+ t[1].len = count;
+ t[1].bits_per_word = 8;
+ spi_message_add_tail(&t[1], &m);
- if (edev->pdata->prepare)
- edev->pdata->prepare(edev);
+ err = spi_sync(edev->spi, &m);
+ /* have to wait at least Tcsl ns */
+ ndelay(250);
- ret = spi_sync(edev->spi, &m);
- /* have to wait at least Tcsl ns */
- ndelay(250);
- if (ret) {
- dev_err(&edev->spi->dev, "read %zu bytes at %d: err. %d\n",
- count, (int)off, ret);
+ if (err) {
+ dev_err(&edev->spi->dev, "read %zu bytes at %d: err.
%d\
