From: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
To: "Arnd Bergmann" <arnd@arndb.de>,
"Jonathan Neuschäfer" <j.ne@posteo.net>,
linux-kernel@vger.kernel.org
Cc: "Emmanuel Gil Peyrot" <linkmauve@linkmauve.fr>,
"Jonathan Neuschäfer" <j.neuschaefer@gmx.net>,
"Rob Herring" <robh+dt@kernel.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Aswath Govindraju" <a-govindraju@ti.com>,
"Vadym Kochan" <vadym.kochan@plvision.eu>,
devicetree@vger.kernel.org
Subject: [PATCH v3 1/3] misc: eeprom_93xx46: Remove hardcoded bit lengths
Date: Tue, 11 May 2021 23:07:24 +0200 [thread overview]
Message-ID: <20210511210727.24895-2-linkmauve@linkmauve.fr> (raw)
In-Reply-To: <20210511210727.24895-1-linkmauve@linkmauve.fr>
This avoids using magic numbers based on the length of an address or a
command, while we only want to differentiate between 8-bit and 16-bit.
The driver was previously wrapping around the offset in the write
operation, this now returns -EINVAL instead (but should never happen in
the first place).
If two pointer indirections are too many, we could move the flags to the
main struct instead, but I doubt it’s going to make any sensible
difference on any hardware.
Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
Reviewed-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
drivers/misc/eeprom/eeprom_93xx46.c | 57 ++++++++++++++++-------------
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index 80114f4c80ad..ffdb8e5a26e0 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -9,6 +9,7 @@
#include <linux/device.h>
#include <linux/gpio/consumer.h>
#include <linux/kernel.h>
+#include <linux/log2.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/of.h>
@@ -70,6 +71,7 @@ static int eeprom_93xx46_read(void *priv, unsigned int off,
struct eeprom_93xx46_dev *edev = priv;
char *buf = val;
int err = 0;
+ int bits;
if (unlikely(off >= edev->size))
return 0;
@@ -83,21 +85,21 @@ static int eeprom_93xx46_read(void *priv, unsigned int off,
if (edev->pdata->prepare)
edev->pdata->prepare(edev);
+ /* The opcode in front of the address is three bits. */
+ bits = edev->addrlen + 3;
+
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;
- if (edev->addrlen == 7) {
- cmd_addr |= off & 0x7f;
- bits = 10;
+ if (edev->pdata->flags & EE_ADDR8) {
+ cmd_addr |= off;
if (has_quirk_single_word_read(edev))
nbytes = 1;
} else {
- cmd_addr |= (off >> 1) & 0x3f;
- bits = 9;
+ cmd_addr |= (off >> 1);
if (has_quirk_single_word_read(edev))
nbytes = 2;
}
@@ -152,14 +154,14 @@ static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
int bits, ret;
u16 cmd_addr;
+ /* The opcode in front of the address is three bits. */
+ bits = edev->addrlen + 3;
+
cmd_addr = OP_START << edev->addrlen;
- if (edev->addrlen == 7) {
+ if (edev->pdata->flags & EE_ADDR8)
cmd_addr |= (is_on ? ADDR_EWEN : ADDR_EWDS) << 1;
- bits = 10;
- } else {
+ else
cmd_addr |= (is_on ? ADDR_EWEN : ADDR_EWDS);
- bits = 9;
- }
if (has_quirk_instruction_length(edev)) {
cmd_addr <<= 2;
@@ -205,15 +207,19 @@ eeprom_93xx46_write_word(struct eeprom_93xx46_dev *edev,
int bits, data_len, ret;
u16 cmd_addr;
+ if (unlikely(off >= edev->size))
+ return -EINVAL;
+
+ /* The opcode in front of the address is three bits. */
+ bits = edev->addrlen + 3;
+
cmd_addr = OP_WRITE << edev->addrlen;
- if (edev->addrlen == 7) {
- cmd_addr |= off & 0x7f;
- bits = 10;
+ if (edev->pdata->flags & EE_ADDR8) {
+ cmd_addr |= off;
data_len = 1;
} else {
- cmd_addr |= (off >> 1) & 0x3f;
- bits = 9;
+ cmd_addr |= (off >> 1);
data_len = 2;
}
@@ -253,7 +259,7 @@ static int eeprom_93xx46_write(void *priv, unsigned int off,
return count;
/* only write even number of bytes on 16-bit devices */
- if (edev->addrlen == 6) {
+ if (edev->pdata->flags & EE_ADDR16) {
step = 2;
count &= ~1;
}
@@ -295,14 +301,14 @@ static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
int bits, ret;
u16 cmd_addr;
+ /* The opcode in front of the address is three bits. */
+ bits = edev->addrlen + 3;
+
cmd_addr = OP_START << edev->addrlen;
- if (edev->addrlen == 7) {
+ if (edev->pdata->flags & EE_ADDR8)
cmd_addr |= ADDR_ERAL << 1;
- bits = 10;
- } else {
+ else
cmd_addr |= ADDR_ERAL;
- bits = 9;
- }
if (has_quirk_instruction_length(edev)) {
cmd_addr <<= 2;
@@ -455,10 +461,12 @@ static int eeprom_93xx46_probe(struct spi_device *spi)
if (!edev)
return -ENOMEM;
+ edev->size = 128;
+
if (pd->flags & EE_ADDR8)
- edev->addrlen = 7;
+ edev->addrlen = ilog2(edev->size);
else if (pd->flags & EE_ADDR16)
- edev->addrlen = 6;
+ edev->addrlen = ilog2(edev->size) - 1;
else {
dev_err(&spi->dev, "unspecified address type\n");
return -EINVAL;
@@ -469,7 +477,6 @@ static int eeprom_93xx46_probe(struct spi_device *spi)
edev->spi = spi;
edev->pdata = pd;
- edev->size = 128;
edev->nvmem_config.type = NVMEM_TYPE_EEPROM;
edev->nvmem_config.name = dev_name(&spi->dev);
edev->nvmem_config.dev = &spi->dev;
--
2.31.1
next prev parent reply other threads:[~2021-05-11 21:07 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-24 12:30 [PATCH 0/4] eeprom-93xx46: Add support for Atmel AT93C56 and AT93C66 Emmanuel Gil Peyrot
2021-04-24 12:30 ` [PATCH 1/4] misc: eeprom_93xx46: Add new at93c56 and at93c66 compatible strings Emmanuel Gil Peyrot
2021-04-24 13:02 ` Jonathan Neuschäfer
2021-04-24 12:30 ` [PATCH 2/4] misc: eeprom_93xx46: set size and addrlen according to the dts Emmanuel Gil Peyrot
2021-04-24 13:17 ` Jonathan Neuschäfer
2021-04-24 16:13 ` kernel test robot
2021-04-24 12:30 ` [PATCH 3/4] misc: eeprom_93xx46: Compute bits based on addrlen Emmanuel Gil Peyrot
2021-04-24 13:34 ` Jonathan Neuschäfer
2021-04-24 12:30 ` [PATCH 4/4] misc: eeprom_93xx46: Switch based on word size, not addrlen Emmanuel Gil Peyrot
2021-04-24 13:42 ` Jonathan Neuschäfer
2021-04-24 21:25 ` [PATCH v2 0/3] eeprom-93xx46: Add support for Atmel AT93C56 and AT93C66 Emmanuel Gil Peyrot
2021-04-24 21:25 ` [PATCH v2 1/3] misc: eeprom_93xx46: Remove hardcoded bit lengths Emmanuel Gil Peyrot
2021-04-26 7:46 ` Jonathan Neuschäfer
2021-04-24 21:25 ` [PATCH v2 2/3] misc: eeprom_93xx46: Add new 93c56 and 93c66 compatible strings Emmanuel Gil Peyrot
2021-04-26 7:56 ` Jonathan Neuschäfer
2021-04-24 21:25 ` [PATCH v2 3/3] dts: eeprom-93xx46: Add support for 93C46, 93C56 and 93C66 Emmanuel Gil Peyrot
2021-04-26 8:02 ` Jonathan Neuschäfer
2021-05-03 18:56 ` Rob Herring
2021-05-11 21:07 ` [PATCH v3 0/3] eeprom-93xx46: Add support for Atmel AT93C56 and AT93C66 Emmanuel Gil Peyrot
2021-05-11 21:07 ` Emmanuel Gil Peyrot [this message]
2021-05-11 21:07 ` [PATCH v3 2/3] misc: eeprom_93xx46: Add new 93c56 and 93c66 compatible strings Emmanuel Gil Peyrot
2021-05-11 21:07 ` [PATCH v3 3/3] dt-bindings: eeprom-93xx46: Add support for 93C46, 93C56 and 93C66 Emmanuel Gil Peyrot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210511210727.24895-2-linkmauve@linkmauve.fr \
--to=linkmauve@linkmauve.fr \
--cc=a-govindraju@ti.com \
--cc=arnd@arndb.de \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=j.ne@posteo.net \
--cc=j.neuschaefer@gmx.net \
--cc=linux-kernel@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=vadym.kochan@plvision.eu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).