diff mbox series

[net-next] net: dsa: xrs700x: Correctly address device over I2C

Message ID 20210202191645.439-1-tobias@waldekranz.com
State New
Headers show
Series [net-next] net: dsa: xrs700x: Correctly address device over I2C | expand

Commit Message

Tobias Waldekranz Feb. 2, 2021, 7:16 p.m. UTC 
On read, master should send 31 MSB of the register (only even values
are ever used), followed by a 1 to indicate read. Then, reading two
bytes, the device will output the register's value.

On write, master sends 31 MSB of the register, followed by a 0 to
indicate write, followed by two bytes containing the register value.

Flexibilis' documentation (version 1.3) specifies the opposite
polarity (#read/write), but the scope indicates that it is, in fact,
read/#write.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---

George, have you used the chip in I2C mode with the code that is on
net-next now? I was not able to get the driver to even read the ID
register correctly.

 drivers/net/dsa/xrs700x/xrs700x_i2c.c | 31 ++++++++++++---------------
 1 file changed, 14 insertions(+), 17 deletions(-)

Comments

Jakub Kicinski Feb. 5, 2021, 3:11 a.m. UTC | #1 
On Tue, 2 Feb 2021 14:05:20 -0600 George McCollister wrote:
> On Tue, Feb 2, 2021 at 1:17 PM Tobias Waldekranz <tobias@waldekranz.com> wrote:

> [snip]

> >

> > George, have you used the chip in I2C mode with the code that is on

> > net-next now? I was not able to get the driver to even read the ID

> > register correctly.  

> 

> I wrote the i2c driver before I had any hardware in hand thinking I

> was going to get a board with the switch connected via i2c. When the

> board arrived it turned out it was connected via mdio so I wrote that

> driver as well. I looked it over quite carefully but I guess the

> documentation was wrong and I had the register addresses shifted off

> by one. I never ended up with hardware to test the i2c.


> Reviewed-by: George McCollister <george.mccollister@gmail.com>


Applied, thanks!
diff mbox series

Patch

diff --git a/drivers/net/dsa/xrs700x/xrs700x_i2c.c b/drivers/net/dsa/xrs700x/xrs700x_i2c.c
index a5f8883af829..16a46a78a037 100644
--- a/drivers/net/dsa/xrs700x/xrs700x_i2c.c
+++ b/drivers/net/dsa/xrs700x/xrs700x_i2c.c
@@ -10,33 +10,34 @@ 
 #include "xrs700x.h"
 #include "xrs700x_reg.h"
 
+struct xrs700x_i2c_cmd {
+	__be32 reg;
+	__be16 val;
+} __packed;
+
 static int xrs700x_i2c_reg_read(void *context, unsigned int reg,
 				unsigned int *val)
 {
 	struct device *dev = context;
 	struct i2c_client *i2c = to_i2c_client(dev);
-	unsigned char buf[4];
+	struct xrs700x_i2c_cmd cmd;
 	int ret;
 
-	buf[0] = reg >> 23 & 0xff;
-	buf[1] = reg >> 15 & 0xff;
-	buf[2] = reg >> 7 & 0xff;
-	buf[3] = (reg & 0x7f) << 1;
+	cmd.reg = cpu_to_be32(reg | 1);
 
-	ret = i2c_master_send(i2c, buf, sizeof(buf));
+	ret = i2c_master_send(i2c, (char *)&cmd.reg, sizeof(cmd.reg));
 	if (ret < 0) {
 		dev_err(dev, "xrs i2c_master_send returned %d\n", ret);
 		return ret;
 	}
 
-	ret = i2c_master_recv(i2c, buf, 2);
+	ret = i2c_master_recv(i2c, (char *)&cmd.val, sizeof(cmd.val));
 	if (ret < 0) {
 		dev_err(dev, "xrs i2c_master_recv returned %d\n", ret);
 		return ret;
 	}
 
-	*val = buf[0] << 8 | buf[1];
-
+	*val = be16_to_cpu(cmd.val);
 	return 0;
 }
 
@@ -45,17 +46,13 @@  static int xrs700x_i2c_reg_write(void *context, unsigned int reg,
 {
 	struct device *dev = context;
 	struct i2c_client *i2c = to_i2c_client(dev);
-	unsigned char buf[6];
+	struct xrs700x_i2c_cmd cmd;
 	int ret;
 
-	buf[0] = reg >> 23 & 0xff;
-	buf[1] = reg >> 15 & 0xff;
-	buf[2] = reg >> 7 & 0xff;
-	buf[3] = (reg & 0x7f) << 1 | 1;
-	buf[4] = val >> 8 & 0xff;
-	buf[5] = val & 0xff;
+	cmd.reg = cpu_to_be32(reg);
+	cmd.val = cpu_to_be16(val);
 
-	ret = i2c_master_send(i2c, buf, sizeof(buf));
+	ret = i2c_master_send(i2c, (char *)&cmd, sizeof(cmd));
 	if (ret < 0) {
 		dev_err(dev, "xrs i2c_master_send returned %d\n", ret);
 		return ret;