2014-06-23 21:20:19

by Doug Anderson

[permalink] [raw]
Subject: [PATCH] i2c: cros_ec: Remove EC_I2C_FLAG_10BIT

In <https://lkml.org/lkml/2014/6/10/265> pointed out that the 10-bit
flag in the cros_ec_tunnel was useless. It went into a 16-bit flags
field but was defined at (1 << 16).

Since we have no 10-bit i2c devices on the other side of the tunnel on
any known devices this was never a problem. Until we do it makes
sense to remove this code. On the EC side the code to handle this
flag was removed in <https://chromium-review.googlesource.com/204162>.

Reported-by: Dave Jones <[email protected]>
Signed-off-by: Doug Anderson <[email protected]>
---
Note that this patch is based atop my current series of posts to
cleanup cros_ec. It wouldn't be hard to apply it to the current ToT
if someone wants to land this before the others.

drivers/i2c/busses/i2c-cros-ec-tunnel.c | 6 ++++--
include/linux/mfd/cros_ec_commands.h | 3 ---
2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 05e033c..6d7d009 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -94,7 +94,7 @@ static int ec_i2c_construct_message(u8 *buf, const struct i2c_msg i2c_msgs[],
msg->addr_flags = i2c_msg->addr;

if (i2c_msg->flags & I2C_M_TEN)
- msg->addr_flags |= EC_I2C_FLAG_10BIT;
+ return -EINVAL;

if (i2c_msg->flags & I2C_M_RD) {
msg->addr_flags |= EC_I2C_FLAG_READ;
@@ -218,7 +218,9 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
}
}

- ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
+ result = ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
+ if (result)
+ goto exit;

msg.version = 0;
msg.command = EC_CMD_I2C_PASSTHRU;
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 7853a64..a49cd41 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -1928,9 +1928,6 @@ struct ec_response_power_info {

#define EC_CMD_I2C_PASSTHRU 0x9e

-/* Slave address is 10 (not 7) bit */
-#define EC_I2C_FLAG_10BIT (1 << 16)
-
/* Read data; if not present, message is a write */
#define EC_I2C_FLAG_READ (1 << 15)

--
2.0.0.526.g5318336


2014-06-24 08:59:34

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH] i2c: cros_ec: Remove EC_I2C_FLAG_10BIT

On Mon, 23 Jun 2014, Doug Anderson wrote:

> In <https://lkml.org/lkml/2014/6/10/265> pointed out that the 10-bit
> flag in the cros_ec_tunnel was useless. It went into a 16-bit flags
> field but was defined at (1 << 16).
>
> Since we have no 10-bit i2c devices on the other side of the tunnel on
> any known devices this was never a problem. Until we do it makes
> sense to remove this code. On the EC side the code to handle this
> flag was removed in <https://chromium-review.googlesource.com/204162>.
>
> Reported-by: Dave Jones <[email protected]>
> Signed-off-by: Doug Anderson <[email protected]>
> ---
> Note that this patch is based atop my current series of posts to
> cleanup cros_ec. It wouldn't be hard to apply it to the current ToT
> if someone wants to land this before the others.
>
> drivers/i2c/busses/i2c-cros-ec-tunnel.c | 6 ++++--
> include/linux/mfd/cros_ec_commands.h | 3 ---

For the MFD part:
Acked-by: Lee Jones <[email protected]>

> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index 05e033c..6d7d009 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -94,7 +94,7 @@ static int ec_i2c_construct_message(u8 *buf, const struct i2c_msg i2c_msgs[],
> msg->addr_flags = i2c_msg->addr;
>
> if (i2c_msg->flags & I2C_M_TEN)
> - msg->addr_flags |= EC_I2C_FLAG_10BIT;
> + return -EINVAL;
>
> if (i2c_msg->flags & I2C_M_RD) {
> msg->addr_flags |= EC_I2C_FLAG_READ;
> @@ -218,7 +218,9 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
> }
> }
>
> - ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
> + result = ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
> + if (result)
> + goto exit;
>
> msg.version = 0;
> msg.command = EC_CMD_I2C_PASSTHRU;
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index 7853a64..a49cd41 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -1928,9 +1928,6 @@ struct ec_response_power_info {
>
> #define EC_CMD_I2C_PASSTHRU 0x9e
>
> -/* Slave address is 10 (not 7) bit */
> -#define EC_I2C_FLAG_10BIT (1 << 16)
> -
> /* Read data; if not present, message is a write */
> #define EC_I2C_FLAG_READ (1 << 15)
>

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

2014-06-24 15:53:27

by Simon Glass

[permalink] [raw]
Subject: Re: [PATCH] i2c: cros_ec: Remove EC_I2C_FLAG_10BIT

On 23 June 2014 15:20, Doug Anderson <[email protected]> wrote:
> In <https://lkml.org/lkml/2014/6/10/265> pointed out that the 10-bit
> flag in the cros_ec_tunnel was useless. It went into a 16-bit flags
> field but was defined at (1 << 16).
>
> Since we have no 10-bit i2c devices on the other side of the tunnel on
> any known devices this was never a problem. Until we do it makes
> sense to remove this code. On the EC side the code to handle this
> flag was removed in <https://chromium-review.googlesource.com/204162>.
>
> Reported-by: Dave Jones <[email protected]>
> Signed-off-by: Doug Anderson <[email protected]>

Funny. We certainly don't use it.

Reviewed-by: Simon Glass <[email protected]>

Regards,
Simon

2014-06-27 12:34:53

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH] i2c: cros_ec: Remove EC_I2C_FLAG_10BIT

On Mon, Jun 23, 2014 at 02:20:06PM -0700, Doug Anderson wrote:
> In <https://lkml.org/lkml/2014/6/10/265> pointed out that the 10-bit
> flag in the cros_ec_tunnel was useless. It went into a 16-bit flags
> field but was defined at (1 << 16).
>
> Since we have no 10-bit i2c devices on the other side of the tunnel on
> any known devices this was never a problem. Until we do it makes
> sense to remove this code. On the EC side the code to handle this
> flag was removed in <https://chromium-review.googlesource.com/204162>.
>
> Reported-by: Dave Jones <[email protected]>
> Signed-off-by: Doug Anderson <[email protected]>
> ---
> Note that this patch is based atop my current series of posts to
> cleanup cros_ec. It wouldn't be hard to apply it to the current ToT
> if someone wants to land this before the others.

I could take it into my tree, but I think it makes more sense if we
simply append it to your cleanup series?

Reviewed-by: Wolfram Sang <[email protected]>


Attachments:
(No filename) (985.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-06-27 15:49:19

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH] i2c: cros_ec: Remove EC_I2C_FLAG_10BIT

Wolfram,

On Fri, Jun 27, 2014 at 5:34 AM, Wolfram Sang <[email protected]> wrote:
> On Mon, Jun 23, 2014 at 02:20:06PM -0700, Doug Anderson wrote:
>> In <https://lkml.org/lkml/2014/6/10/265> pointed out that the 10-bit
>> flag in the cros_ec_tunnel was useless. It went into a 16-bit flags
>> field but was defined at (1 << 16).
>>
>> Since we have no 10-bit i2c devices on the other side of the tunnel on
>> any known devices this was never a problem. Until we do it makes
>> sense to remove this code. On the EC side the code to handle this
>> flag was removed in <https://chromium-review.googlesource.com/204162>.
>>
>> Reported-by: Dave Jones <[email protected]>
>> Signed-off-by: Doug Anderson <[email protected]>
>> ---
>> Note that this patch is based atop my current series of posts to
>> cleanup cros_ec. It wouldn't be hard to apply it to the current ToT
>> if someone wants to land this before the others.
>
> I could take it into my tree, but I think it makes more sense if we
> simply append it to your cleanup series?

Thanks, that sounds reasonable to me. Lee has Acked the whole series
and Simon Glass has reviewed it too, so I'd imagine that we're just
waiting on the input subsystem's Ack on a few of the patches.

If I happen to send out the other series for some reason I'll tack
this onto the end of it.

-Doug

2014-06-27 15:59:29

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH] i2c: cros_ec: Remove EC_I2C_FLAG_10BIT


> Thanks, that sounds reasonable to me. Lee has Acked the whole series
> and Simon Glass has reviewed it too, so I'd imagine that we're just
> waiting on the input subsystem's Ack on a few of the patches.

OK. I assume it will go in via the same tree as the cleanup series to
simplify dependencies; if it shall go in via I2C ping me again.


Attachments:
(No filename) (343.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments