On Fri, Apr 20, 2018 at 10:26:08AM -0700, Guenter Roeck wrote: > On Wed, Apr 18, 2018 at 03:34:09PM +0300, Heikki Krogerus wrote: > > 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") > > Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > > --- > > drivers/usb/typec/tps6598x.c | 42 ++++++++++++++++++++++++++++++------ > > 1 file changed, 35 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c > > index 8b8406867c02..82f09cd9792d 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,6 +81,23 @@ struct tps6598x { > > struct typec_capability typec_cap; > > }; > > > > +static int > > +tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, ssize_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; > > + > > Sanity check ? > if (data[0] != len) > return -Esomething; No. Then we would not even need the len parameter. The idea is to allow reading a number of bytes specified by caller, regardless of the maximum size of the block. Thanks, -- heikki