All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Add support for limited i2c tunnel for exynos5250-spring
@ 2014-06-27 19:56 Doug Anderson
  2014-06-27 19:56 ` [PATCH 1/3] regmap: cache: regcache_hw_init() should use regmap_bulk_read() Doug Anderson
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Doug Anderson @ 2014-06-27 19:56 UTC (permalink / raw)
  To: Lee Jones, Wolfram Sang, Mark Brown
  Cc: Vincent Palatin, Bill Richardson, Randall Spangler, sjg,
	afaerber, stephan, olof, Doug Anderson, sameo, gregkh,
	linux-kernel, linux-i2c

This patches series possibly adds support for getting to the battery
and tps65090 device on exynos5250-spring.  I have simulated things on
exynos5420-peach-pit and found that this seems to work OK and I can
talk to both the battery and tps65090.  I have simulated this on
exynos5250-snow and found that it properly detects that the full I2C
passhtrough isn't available.  I don't have a Spring (or Skate) setup
for upstream testing right now but I figured possibly Andreas would be
interested in testing this.

As discussed in patch #3 this code path on the EC is not actually
exercised on production kernels but I think it should be solid and get
us up and running quickly.  I think this is also a cleaner solution
than what's in production kernels but possibly folks will be able to
come up with benefits of and arguments for using the specialized
regulator and battery commands to talk to the EC.

Someone interested in using this should add the cros ec tunnel to
their spring device tree and see what happens.  Note that this limited
tunnel doesn't actually export anything that's useful for i2cdetect to
use so to test you pretty much just need to add an sbs_battery and see
if it works.

I may not have tons of time for spinning this series so possibly
someone in the community who is interested in getting
exynos5250-spring supported would be interested in following up with
any needed review feedback.  I certainly wouldn't be offended.

As is probably obvious this patch series is based atop all of the
existing posted (and ACKed) cros_ec cleanups that I've sent up.


Doug Anderson (3):
  regmap: cache: regcache_hw_init() should use regmap_bulk_read()
  mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c
    result
  i2c: cros_ec: Support a limited i2c tunnel for exynos5250-spring

 drivers/base/regmap/regcache.c          |  2 +-
 drivers/i2c/busses/i2c-cros-ec-tunnel.c | 92 ++++++++++++++++++++++++++++++++-
 drivers/mfd/cros_ec_i2c.c               | 15 ++++--
 3 files changed, 104 insertions(+), 5 deletions(-)

-- 
2.0.0.526.g5318336


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] regmap: cache: regcache_hw_init() should use regmap_bulk_read()
  2014-06-27 19:56 [PATCH 0/3] Add support for limited i2c tunnel for exynos5250-spring Doug Anderson
@ 2014-06-27 19:56 ` Doug Anderson
  2014-07-03 14:55   ` Mark Brown
  2014-06-27 19:56 ` [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result Doug Anderson
  2014-06-27 19:56   ` Doug Anderson
  2 siblings, 1 reply; 11+ messages in thread
From: Doug Anderson @ 2014-06-27 19:56 UTC (permalink / raw)
  To: Lee Jones, Wolfram Sang, Mark Brown
  Cc: Vincent Palatin, Bill Richardson, Randall Spangler, sjg,
	afaerber, stephan, olof, Doug Anderson, gregkh, linux-kernel

We really should be using regmap_bulk_read() in regcache_hw_init().
The regmap_bulk_read() will translate into regmap_raw_read() when
appropriate.  Doing this fixes problems where regmap_smbus() will
crash because they don't implement .read and .write.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/base/regmap/regcache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 29b4128..6447486 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -45,7 +45,7 @@ static int regcache_hw_init(struct regmap *map)
 		tmp_buf = kmalloc(map->cache_size_raw, GFP_KERNEL);
 		if (!tmp_buf)
 			return -EINVAL;
-		ret = regmap_raw_read(map, 0, tmp_buf,
+		ret = regmap_bulk_read(map, 0, tmp_buf,
 				      map->num_reg_defaults_raw);
 		map->cache_bypass = cache_bypass;
 		if (ret < 0) {
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result
  2014-06-27 19:56 [PATCH 0/3] Add support for limited i2c tunnel for exynos5250-spring Doug Anderson
  2014-06-27 19:56 ` [PATCH 1/3] regmap: cache: regcache_hw_init() should use regmap_bulk_read() Doug Anderson
@ 2014-06-27 19:56 ` Doug Anderson
  2014-06-30 19:36   ` Simon Glass
  2014-07-02  7:23   ` Lee Jones
  2014-06-27 19:56   ` Doug Anderson
  2 siblings, 2 replies; 11+ messages in thread
From: Doug Anderson @ 2014-06-27 19:56 UTC (permalink / raw)
  To: Lee Jones, Wolfram Sang, Mark Brown
  Cc: Vincent Palatin, Bill Richardson, Randall Spangler, sjg,
	afaerber, stephan, olof, Doug Anderson, sameo, linux-kernel

We know how many bytes the EC should be sending us (which is also the
number of bytes transferred) and also how many bytes the EC actually
wanted to send to us.  When computing the checksum and copying back
data let's make sure we take the lesser of the two of those.  We'll
also complain if the EC tried to send us too many bytes.  The EC
sending us too few bytes is legit for when we send the EC an invalid
command.

This is based on similar code in cros_ec_spi.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/mfd/cros_ec_i2c.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index fd7a546..c0c30f4 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -35,6 +35,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
 	struct i2c_client *client = ec_dev->priv;
 	int ret = -ENOMEM;
 	int i;
+	int len;
 	int packet_len;
 	u8 *out_buf = NULL;
 	u8 *in_buf = NULL;
@@ -97,21 +98,29 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
 	if (ret)
 		goto done;
 
+	len = in_buf[1];
+	if (len > msg->insize) {
+		dev_err(ec_dev->dev, "packet too long (%d bytes, expected %d)",
+			len, msg->insize);
+		ret = -ENOSPC;
+		goto done;
+	}
+
 	/* copy response packet payload and compute checksum */
 	sum = in_buf[0] + in_buf[1];
-	for (i = 0; i < msg->insize; i++) {
+	for (i = 0; i < len; i++) {
 		msg->indata[i] = in_buf[2 + i];
 		sum += in_buf[2 + i];
 	}
 	dev_dbg(ec_dev->dev, "packet: %*ph, sum = %02x\n",
 		i2c_msg[1].len, in_buf, sum);
-	if (sum != in_buf[2 + msg->insize]) {
+	if (sum != in_buf[2 + len]) {
 		dev_err(ec_dev->dev, "bad packet checksum\n");
 		ret = -EBADMSG;
 		goto done;
 	}
 
-	ret = i2c_msg[1].buf[1];
+	ret = len;
  done:
 	kfree(in_buf);
 	kfree(out_buf);
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] i2c: cros_ec: Support a limited i2c tunnel for exynos5250-spring
@ 2014-06-27 19:56   ` Doug Anderson
  0 siblings, 0 replies; 11+ messages in thread
From: Doug Anderson @ 2014-06-27 19:56 UTC (permalink / raw)
  To: Lee Jones, Wolfram Sang, Mark Brown
  Cc: Vincent Palatin, Bill Richardson, Randall Spangler, sjg,
	afaerber, stephan, olof, Doug Anderson, linux-i2c, linux-kernel

On exynos5250-spring the battery and tps65090 regulator are sitting on
an i2c bus behind the EC (much like on exynos5420-peach-pit).  However
on spring we don't have the full EC_CMD_I2C_PASSTHRU command.

For the production kernel of spring we used a solution like this:
- Fork the tps65090 driver and make a version that talks straight to
  the cros_ec MFD driver and sends special EC commands for talking to
  the regulator.
- Add code into the i2c tunnel to look for i2c transfers and convert
  them into special EC commands for talking to the battery.

For reference:
* http://crosreview.com/66116
* https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/regulator/cros_ec-tps65090.c

The above solution works great but is a bunch of code.  It appears
that we can make do with the limited i2c passthrough commands that
actually happened to be present on the exynos5250-spring EC.  Doing
this we can present ourselves as supporting a small subset of smbus.
We might need to do a few extra transfers here and there but we don't
need any extra code.

Seriss-cc: linux-samsung-soc
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/i2c/busses/i2c-cros-ec-tunnel.c | 92 ++++++++++++++++++++++++++++++++-
 1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 6d7d009..c52c491 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -38,6 +38,76 @@ struct ec_i2c_device {
 	u8 response_buf[256];
 };
 
+static int ec_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+			   unsigned short flags, char read_write,
+			   u8 command, int size, union i2c_smbus_data *data)
+{
+	struct ec_i2c_device *bus = adap->algo_data;
+	struct cros_ec_command msg = {};
+	int result;
+
+	if (read_write == I2C_SMBUS_WRITE) {
+		struct ec_params_i2c_write params;
+
+		params.addr = addr << 1;
+		params.port = bus->remote_bus;
+		params.offset = command;
+
+		if (size == I2C_SMBUS_WORD_DATA) {
+			params.data = data->word;
+			params.write_size = 16;
+		} else if (size == I2C_SMBUS_BYTE_DATA) {
+			params.data = data->byte;
+			params.write_size = 8;
+		} else {
+			return -EINVAL;
+		}
+
+		msg.command = EC_CMD_I2C_WRITE;
+		msg.outdata = (uint8_t *)&params;
+		msg.outsize = sizeof(params);
+
+		result = bus->ec->cmd_xfer(bus->ec, &msg);
+		if (result < 0)
+			return -EIO;
+
+		return 0;
+	} else if (read_write == I2C_SMBUS_READ) {
+		struct ec_params_i2c_read params;
+		struct ec_response_i2c_read response;
+
+		params.addr = addr << 1;
+		params.port = bus->remote_bus;
+		params.offset = command;
+
+		if (size == I2C_SMBUS_WORD_DATA)
+			params.read_size = 16;
+		else if (size == I2C_SMBUS_BYTE_DATA)
+			params.read_size = 8;
+		else
+			return -EINVAL;
+
+		msg.command = EC_CMD_I2C_READ;
+		msg.outdata = (uint8_t *)&params;
+		msg.outsize = sizeof(params);
+		msg.indata = (uint8_t *)&response;
+		msg.insize = sizeof(response);
+
+		result = bus->ec->cmd_xfer(bus->ec, &msg);
+		if (result < 0)
+			return -EIO;
+
+		if (size == I2C_SMBUS_WORD_DATA)
+			data->word = response.data;
+		else
+			data->byte = response.data;
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 /**
  * ec_i2c_count_message - Count bytes needed for ec_i2c_construct_message
  *
@@ -233,6 +303,11 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
 	if (result < 0)
 		goto exit;
 
+	if (msg.result == EC_RES_INVALID_COMMAND) {
+		result = -EINVAL;
+		goto exit;
+	}
+
 	result = ec_i2c_parse_response(response, i2c_msgs, &num);
 	if (result < 0)
 		goto exit;
@@ -258,6 +333,16 @@ static const struct i2c_algorithm ec_i2c_algorithm = {
 	.functionality	= ec_i2c_functionality,
 };
 
+static u32 ec_smbus_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA;
+}
+
+static const struct i2c_algorithm ec_i2c_limited_algorithm = {
+	.smbus_xfer	= ec_smbus_xfer,
+	.functionality	= ec_smbus_functionality,
+};
+
 static int ec_i2c_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -288,11 +373,16 @@ static int ec_i2c_probe(struct platform_device *pdev)
 
 	bus->adap.owner = THIS_MODULE;
 	strlcpy(bus->adap.name, "cros-ec-i2c-tunnel", sizeof(bus->adap.name));
-	bus->adap.algo = &ec_i2c_algorithm;
 	bus->adap.algo_data = bus;
 	bus->adap.dev.parent = &pdev->dev;
 	bus->adap.dev.of_node = np;
 
+	/* Test if we can support full passthrough or just limited */
+	if (ec_i2c_xfer(&bus->adap, NULL, 0) == 0)
+		bus->adap.algo = &ec_i2c_algorithm;
+	else
+		bus->adap.algo = &ec_i2c_limited_algorithm;
+
 	err = i2c_add_adapter(&bus->adap);
 	if (err) {
 		dev_err(dev, "cannot register i2c adapter\n");
-- 
2.0.0.526.g5318336


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] i2c: cros_ec: Support a limited i2c tunnel for exynos5250-spring
@ 2014-06-27 19:56   ` Doug Anderson
  0 siblings, 0 replies; 11+ messages in thread
From: Doug Anderson @ 2014-06-27 19:56 UTC (permalink / raw)
  To: Lee Jones, Wolfram Sang, Mark Brown
  Cc: Vincent Palatin, Bill Richardson, Randall Spangler,
	sjg-F7+t8E8rja9g9hUCZPvPmw, afaerber-l3A5Bk7waGM,
	stephan-zGZ7W/ttz6xN8Ch2cx6nig, olof-nZhT3qVonbNeoWH0uzbU5w,
	Doug Anderson, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On exynos5250-spring the battery and tps65090 regulator are sitting on
an i2c bus behind the EC (much like on exynos5420-peach-pit).  However
on spring we don't have the full EC_CMD_I2C_PASSTHRU command.

For the production kernel of spring we used a solution like this:
- Fork the tps65090 driver and make a version that talks straight to
  the cros_ec MFD driver and sends special EC commands for talking to
  the regulator.
- Add code into the i2c tunnel to look for i2c transfers and convert
  them into special EC commands for talking to the battery.

For reference:
* http://crosreview.com/66116
* https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/regulator/cros_ec-tps65090.c

The above solution works great but is a bunch of code.  It appears
that we can make do with the limited i2c passthrough commands that
actually happened to be present on the exynos5250-spring EC.  Doing
this we can present ourselves as supporting a small subset of smbus.
We might need to do a few extra transfers here and there but we don't
need any extra code.

Seriss-cc: linux-samsung-soc
Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/i2c/busses/i2c-cros-ec-tunnel.c | 92 ++++++++++++++++++++++++++++++++-
 1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 6d7d009..c52c491 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -38,6 +38,76 @@ struct ec_i2c_device {
 	u8 response_buf[256];
 };
 
+static int ec_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+			   unsigned short flags, char read_write,
+			   u8 command, int size, union i2c_smbus_data *data)
+{
+	struct ec_i2c_device *bus = adap->algo_data;
+	struct cros_ec_command msg = {};
+	int result;
+
+	if (read_write == I2C_SMBUS_WRITE) {
+		struct ec_params_i2c_write params;
+
+		params.addr = addr << 1;
+		params.port = bus->remote_bus;
+		params.offset = command;
+
+		if (size == I2C_SMBUS_WORD_DATA) {
+			params.data = data->word;
+			params.write_size = 16;
+		} else if (size == I2C_SMBUS_BYTE_DATA) {
+			params.data = data->byte;
+			params.write_size = 8;
+		} else {
+			return -EINVAL;
+		}
+
+		msg.command = EC_CMD_I2C_WRITE;
+		msg.outdata = (uint8_t *)&params;
+		msg.outsize = sizeof(params);
+
+		result = bus->ec->cmd_xfer(bus->ec, &msg);
+		if (result < 0)
+			return -EIO;
+
+		return 0;
+	} else if (read_write == I2C_SMBUS_READ) {
+		struct ec_params_i2c_read params;
+		struct ec_response_i2c_read response;
+
+		params.addr = addr << 1;
+		params.port = bus->remote_bus;
+		params.offset = command;
+
+		if (size == I2C_SMBUS_WORD_DATA)
+			params.read_size = 16;
+		else if (size == I2C_SMBUS_BYTE_DATA)
+			params.read_size = 8;
+		else
+			return -EINVAL;
+
+		msg.command = EC_CMD_I2C_READ;
+		msg.outdata = (uint8_t *)&params;
+		msg.outsize = sizeof(params);
+		msg.indata = (uint8_t *)&response;
+		msg.insize = sizeof(response);
+
+		result = bus->ec->cmd_xfer(bus->ec, &msg);
+		if (result < 0)
+			return -EIO;
+
+		if (size == I2C_SMBUS_WORD_DATA)
+			data->word = response.data;
+		else
+			data->byte = response.data;
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 /**
  * ec_i2c_count_message - Count bytes needed for ec_i2c_construct_message
  *
@@ -233,6 +303,11 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
 	if (result < 0)
 		goto exit;
 
+	if (msg.result == EC_RES_INVALID_COMMAND) {
+		result = -EINVAL;
+		goto exit;
+	}
+
 	result = ec_i2c_parse_response(response, i2c_msgs, &num);
 	if (result < 0)
 		goto exit;
@@ -258,6 +333,16 @@ static const struct i2c_algorithm ec_i2c_algorithm = {
 	.functionality	= ec_i2c_functionality,
 };
 
+static u32 ec_smbus_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA;
+}
+
+static const struct i2c_algorithm ec_i2c_limited_algorithm = {
+	.smbus_xfer	= ec_smbus_xfer,
+	.functionality	= ec_smbus_functionality,
+};
+
 static int ec_i2c_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -288,11 +373,16 @@ static int ec_i2c_probe(struct platform_device *pdev)
 
 	bus->adap.owner = THIS_MODULE;
 	strlcpy(bus->adap.name, "cros-ec-i2c-tunnel", sizeof(bus->adap.name));
-	bus->adap.algo = &ec_i2c_algorithm;
 	bus->adap.algo_data = bus;
 	bus->adap.dev.parent = &pdev->dev;
 	bus->adap.dev.of_node = np;
 
+	/* Test if we can support full passthrough or just limited */
+	if (ec_i2c_xfer(&bus->adap, NULL, 0) == 0)
+		bus->adap.algo = &ec_i2c_algorithm;
+	else
+		bus->adap.algo = &ec_i2c_limited_algorithm;
+
 	err = i2c_add_adapter(&bus->adap);
 	if (err) {
 		dev_err(dev, "cannot register i2c adapter\n");
-- 
2.0.0.526.g5318336

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] i2c: cros_ec: Support a limited i2c tunnel for exynos5250-spring
  2014-06-27 19:56   ` Doug Anderson
  (?)
@ 2014-06-27 21:44   ` Doug Anderson
  -1 siblings, 0 replies; 11+ messages in thread
From: Doug Anderson @ 2014-06-27 21:44 UTC (permalink / raw)
  To: Lee Jones, Wolfram Sang, Mark Brown
  Cc: Vincent Palatin, Bill Richardson, Randall Spangler, Simon Glass,
	Andreas Färber, Stephan van Schaik, Olof Johansson,
	Doug Anderson, linux-i2c, linux-kernel

Hi,

On Fri, Jun 27, 2014 at 12:56 PM, Doug Anderson <dianders@chromium.org> wrote:
> On exynos5250-spring the battery and tps65090 regulator are sitting on
> an i2c bus behind the EC (much like on exynos5420-peach-pit).  However
> on spring we don't have the full EC_CMD_I2C_PASSTHRU command.
>
> For the production kernel of spring we used a solution like this:
> - Fork the tps65090 driver and make a version that talks straight to
>   the cros_ec MFD driver and sends special EC commands for talking to
>   the regulator.
> - Add code into the i2c tunnel to look for i2c transfers and convert
>   them into special EC commands for talking to the battery.
>
> For reference:
> * http://crosreview.com/66116
> * https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/regulator/cros_ec-tps65090.c
>
> The above solution works great but is a bunch of code.  It appears
> that we can make do with the limited i2c passthrough commands that
> actually happened to be present on the exynos5250-spring EC.  Doing
> this we can present ourselves as supporting a small subset of smbus.
> We might need to do a few extra transfers here and there but we don't
> need any extra code.
>
> Seriss-cc: linux-samsung-soc
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 92 ++++++++++++++++++++++++++++++++-
>  1 file changed, 91 insertions(+), 1 deletion(-)

I just got done talking to Randall and he suggested that this might
not actually work on Spring.  :(

It looks like in the Spring timeframe I2C EC commands were restricted
if the write protect screw was in place.  ...so someone will probably
want to take some of the code referenced above instead of this patch.

Patches #1 and #2 are probably still valid, though.

-Doug

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result
  2014-06-27 19:56 ` [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result Doug Anderson
@ 2014-06-30 19:36   ` Simon Glass
  2014-07-02  7:23   ` Lee Jones
  1 sibling, 0 replies; 11+ messages in thread
From: Simon Glass @ 2014-06-30 19:36 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Lee Jones, Wolfram Sang, Mark Brown, Vincent Palatin,
	Bill Richardson, Randall Spangler, Andreas Färber, stephan,
	Olof Johansson, Samuel Ortiz, lk

On 27 June 2014 12:56, Doug Anderson <dianders@chromium.org> wrote:
> We know how many bytes the EC should be sending us (which is also the
> number of bytes transferred) and also how many bytes the EC actually
> wanted to send to us.  When computing the checksum and copying back
> data let's make sure we take the lesser of the two of those.  We'll
> also complain if the EC tried to send us too many bytes.  The EC
> sending us too few bytes is legit for when we send the EC an invalid
> command.
>
> This is based on similar code in cros_ec_spi.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result
  2014-06-27 19:56 ` [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result Doug Anderson
  2014-06-30 19:36   ` Simon Glass
@ 2014-07-02  7:23   ` Lee Jones
  2014-07-02 15:51     ` Doug Anderson
  1 sibling, 1 reply; 11+ messages in thread
From: Lee Jones @ 2014-07-02  7:23 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Wolfram Sang, Mark Brown, Vincent Palatin, Bill Richardson,
	Randall Spangler, sjg, afaerber, stephan, olof, sameo,
	linux-kernel

On Fri, 27 Jun 2014, Doug Anderson wrote:

> We know how many bytes the EC should be sending us (which is also the
> number of bytes transferred) and also how many bytes the EC actually
> wanted to send to us.  When computing the checksum and copying back
> data let's make sure we take the lesser of the two of those.  We'll
> also complain if the EC tried to send us too many bytes.  The EC
> sending us too few bytes is legit for when we send the EC an invalid
> command.
> 
> This is based on similar code in cros_ec_spi.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/mfd/cros_ec_i2c.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Acked-by: Lee Jones <lee.jones@linaro.org>

Is this patch orthogonal i.e. can it be applied without the other two
patches?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result
  2014-07-02  7:23   ` Lee Jones
@ 2014-07-02 15:51     ` Doug Anderson
  2014-07-03  6:38       ` Lee Jones
  0 siblings, 1 reply; 11+ messages in thread
From: Doug Anderson @ 2014-07-02 15:51 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Mark Brown, Vincent Palatin, Bill Richardson,
	Randall Spangler, Simon Glass, Andreas Färber,
	Stephan van Schaik, Olof Johansson, Samuel Ortiz, linux-kernel

Lee,

On Wed, Jul 2, 2014 at 12:23 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 27 Jun 2014, Doug Anderson wrote:
>
>> We know how many bytes the EC should be sending us (which is also the
>> number of bytes transferred) and also how many bytes the EC actually
>> wanted to send to us.  When computing the checksum and copying back
>> data let's make sure we take the lesser of the two of those.  We'll
>> also complain if the EC tried to send us too many bytes.  The EC
>> sending us too few bytes is legit for when we send the EC an invalid
>> command.
>>
>> This is based on similar code in cros_ec_spi.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> ---
>>  drivers/mfd/cros_ec_i2c.c | 15 ++++++++++++---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> Acked-by: Lee Jones <lee.jones@linaro.org>
>
> Is this patch orthogonal i.e. can it be applied without the other two
> patches?

Yes.  If patch 3/3 had worked out then it would have required patch #1
for proper functioning and patch #2 (this patch) to avoid an ugly
error message in the log.  ...but patch #1 and this patch both can
stand on their own and can be applied.

-Doug

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result
  2014-07-02 15:51     ` Doug Anderson
@ 2014-07-03  6:38       ` Lee Jones
  0 siblings, 0 replies; 11+ messages in thread
From: Lee Jones @ 2014-07-03  6:38 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Wolfram Sang, Mark Brown, Vincent Palatin, Bill Richardson,
	Randall Spangler, Simon Glass, Andreas Färber,
	Stephan van Schaik, Olof Johansson, Samuel Ortiz, linux-kernel

On Wed, 02 Jul 2014, Doug Anderson wrote:
> On Wed, Jul 2, 2014 at 12:23 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Fri, 27 Jun 2014, Doug Anderson wrote:
> >
> >> We know how many bytes the EC should be sending us (which is also the
> >> number of bytes transferred) and also how many bytes the EC actually
> >> wanted to send to us.  When computing the checksum and copying back
> >> data let's make sure we take the lesser of the two of those.  We'll
> >> also complain if the EC tried to send us too many bytes.  The EC
> >> sending us too few bytes is legit for when we send the EC an invalid
> >> command.
> >>
> >> This is based on similar code in cros_ec_spi.
> >>
> >> Signed-off-by: Doug Anderson <dianders@chromium.org>
> >> ---
> >>  drivers/mfd/cros_ec_i2c.c | 15 ++++++++++++---
> >>  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > Acked-by: Lee Jones <lee.jones@linaro.org>
> >
> > Is this patch orthogonal i.e. can it be applied without the other two
> > patches?
> 
> Yes.  If patch 3/3 had worked out then it would have required patch #1
> for proper functioning and patch #2 (this patch) to avoid an ugly
> error message in the log.  ...but patch #1 and this patch both can
> stand on their own and can be applied.

Very well, patch applied than.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] regmap: cache: regcache_hw_init() should use regmap_bulk_read()
  2014-06-27 19:56 ` [PATCH 1/3] regmap: cache: regcache_hw_init() should use regmap_bulk_read() Doug Anderson
@ 2014-07-03 14:55   ` Mark Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Brown @ 2014-07-03 14:55 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Lee Jones, Wolfram Sang, Vincent Palatin, Bill Richardson,
	Randall Spangler, sjg, afaerber, stephan, olof, gregkh,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 681 bytes --]

On Fri, Jun 27, 2014 at 12:56:11PM -0700, Doug Anderson wrote:

> We really should be using regmap_bulk_read() in regcache_hw_init().
> The regmap_bulk_read() will translate into regmap_raw_read() when
> appropriate.  Doing this fixes problems where regmap_smbus() will
> crash because they don't implement .read and .write.

Not quite - _bulk_read() does do byte swapping so...

> -		ret = regmap_raw_read(map, 0, tmp_buf,
> +		ret = regmap_bulk_read(map, 0, tmp_buf,
>  				      map->num_reg_defaults_raw);
>  		map->cache_bypass = cache_bypass;
>  		if (ret < 0) {

...a direct replacement shouldn't quite work.  We need an actual
fallback to word at a time operation I think.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-07-03 14:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-27 19:56 [PATCH 0/3] Add support for limited i2c tunnel for exynos5250-spring Doug Anderson
2014-06-27 19:56 ` [PATCH 1/3] regmap: cache: regcache_hw_init() should use regmap_bulk_read() Doug Anderson
2014-07-03 14:55   ` Mark Brown
2014-06-27 19:56 ` [PATCH 2/3] mfd: cros_ec: Use the proper size when looking at the cros_ec_i2c result Doug Anderson
2014-06-30 19:36   ` Simon Glass
2014-07-02  7:23   ` Lee Jones
2014-07-02 15:51     ` Doug Anderson
2014-07-03  6:38       ` Lee Jones
2014-06-27 19:56 ` [PATCH 3/3] i2c: cros_ec: Support a limited i2c tunnel for exynos5250-spring Doug Anderson
2014-06-27 19:56   ` Doug Anderson
2014-06-27 21:44   ` Doug Anderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.