diff mbox

[PATCHv2] usb: typec: tps6598x: handle block reads separately with plain-I2C adapters

Message ID 20180425142209.30745-1-heikki.krogerus@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Heikki Krogerus April 25, 2018, 2:22 p.m. UTC
If the I2C adapter that the PD controller is attached to
does not support SMBus protocol, the driver needs to handle
block reads separately. The first byte returned in block
read protocol will show the total number of bytes. It needs
to be stripped away.

This is handled separately in the driver only because right
now we have no way of requesting the used protocol with
regmap-i2c. This is in practice a workaround for what is
really a problem in regmap-i2c. The other option would have
been to register custom regmap, or not use regmap at all,
however, since the solution is very simple, I choose to use
it in this case for convenience. It is easy to remove once
we figure out how to handle this kind of cases in
regmap-i2c.

Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
Changed since v1:
- Added sanity check
- Fixed also 2 byte reads
---
 drivers/usb/typec/tps6598x.c | 47 ++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 8 deletions(-)
diff mbox

Patch

diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index 8b8406867c02..4b4c8d271b27 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -73,6 +73,7 @@  struct tps6598x {
 	struct device *dev;
 	struct regmap *regmap;
 	struct mutex lock; /* device lock */
+	u8 i2c_protocol:1;
 
 	struct typec_port *port;
 	struct typec_partner *partner;
@@ -80,19 +81,39 @@  struct tps6598x {
 	struct typec_capability typec_cap;
 };
 
+static int
+tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
+{
+	u8 data[len + 1];
+	int ret;
+
+	if (!tps->i2c_protocol)
+		return regmap_raw_read(tps->regmap, reg, val, len);
+
+	ret = regmap_raw_read(tps->regmap, reg, data, sizeof(data));
+	if (ret)
+		return ret;
+
+	if (data[0] < len)
+		return -EIO;
+
+	memcpy(val, &data[1], len);
+	return 0;
+}
+
 static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
 {
-	return regmap_raw_read(tps->regmap, reg, val, sizeof(u16));
+	return tps6598x_block_read(tps, reg, val, sizeof(u16));
 }
 
 static inline int tps6598x_read32(struct tps6598x *tps, u8 reg, u32 *val)
 {
-	return regmap_raw_read(tps->regmap, reg, val, sizeof(u32));
+	return tps6598x_block_read(tps, reg, val, sizeof(u32));
 }
 
 static inline int tps6598x_read64(struct tps6598x *tps, u8 reg, u64 *val)
 {
-	return regmap_raw_read(tps->regmap, reg, val, sizeof(u64));
+	return tps6598x_block_read(tps, reg, val, sizeof(u64));
 }
 
 static inline int tps6598x_write16(struct tps6598x *tps, u8 reg, u16 val)
@@ -121,8 +142,8 @@  static int tps6598x_read_partner_identity(struct tps6598x *tps)
 	struct tps6598x_rx_identity_reg id;
 	int ret;
 
-	ret = regmap_raw_read(tps->regmap, TPS_REG_RX_IDENTITY_SOP,
-			      &id, sizeof(id));
+	ret = tps6598x_block_read(tps, TPS_REG_RX_IDENTITY_SOP,
+				  &id, sizeof(id));
 	if (ret)
 		return ret;
 
@@ -224,13 +245,13 @@  static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
 	} while (val);
 
 	if (out_len) {
-		ret = regmap_raw_read(tps->regmap, TPS_REG_DATA1,
-				      out_data, out_len);
+		ret = tps6598x_block_read(tps, TPS_REG_DATA1,
+					  out_data, out_len);
 		if (ret)
 			return ret;
 		val = out_data[0];
 	} else {
-		ret = regmap_read(tps->regmap, TPS_REG_DATA1, &val);
+		ret = tps6598x_block_read(tps, TPS_REG_DATA1, &val, sizeof(u8));
 		if (ret)
 			return ret;
 	}
@@ -385,6 +406,16 @@  static int tps6598x_probe(struct i2c_client *client)
 	if (!vid)
 		return -ENODEV;
 
+	/*
+	 * Checking can the adapter handle SMBus protocol. If it can not, the
+	 * driver needs to take care of block reads separately.
+	 *
+	 * FIXME: Testing with I2C_FUNC_I2C. regmap-i2c uses I2C protocol
+	 * unconditionally if the adapter has I2C_FUNC_I2C set.
+	 */
+	if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		tps->i2c_protocol = true;
+
 	ret = tps6598x_read32(tps, TPS_REG_STATUS, &status);
 	if (ret < 0)
 		return ret;