[PATCH v2 0/1] iio: common: cros_ec_sensors: Add protocol v3 support

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2 0/1] iio: common: cros_ec_sensors: Add protocol v3 support
@ 2019-06-28 14:40 Fabien Lahoudere
  2019-06-28 14:41 ` [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version Fabien Lahoudere
  0 siblings, 1 reply; 5+ messages in thread
From: Fabien Lahoudere @ 2019-06-28 14:40 UTC (permalink / raw)
  Cc: gwendal, egranata, kernel, Fabien Lahoudere, Jonathan Cameron,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Benson Leung, Enric Balletbo i Serra, Guenter Roeck,
	Allison Randal, Alexios Zavras, Thomas Gleixner,
	Greg Kroah-Hartman, linux-iio, linux-kernel

This patch is part of a split of the following patch:
https://lkml.org/lkml/2019/6/18/268
To fix Enric comments from https://lkml.org/lkml/2019/6/25/949
I extract it from the other serie to speed up acceptance because
other patches need it to be upstreamed.

Changes since v1:
- Drop second patch
- return ENODEV if version is 0

Fabien Lahoudere (1):
  iio: common: cros_ec_sensors: determine protocol version

 .../cros_ec_sensors/cros_ec_sensors_core.c    | 40 ++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

-- 
2.19.2


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

* [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version
  2019-06-28 14:40 [PATCH v2 0/1] iio: common: cros_ec_sensors: Add protocol v3 support Fabien Lahoudere
@ 2019-06-28 14:41 ` Fabien Lahoudere
  2019-06-28 15:13   ` Enric Balletbo i Serra
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Fabien Lahoudere @ 2019-06-28 14:41 UTC (permalink / raw)
  Cc: gwendal, egranata, kernel, Fabien Lahoudere, Nick Vaccaro,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Benson Leung, Enric Balletbo i Serra,
	Guenter Roeck, Alexios Zavras, Allison Randal, Thomas Gleixner,
	Greg Kroah-Hartman, linux-iio, linux-kernel

This patch adds a function to determine which version of the
protocol is used to communicate with EC.

Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.com>
Signed-off-by: Nick Vaccaro <nvaccaro@chromium.org>
---
 .../cros_ec_sensors/cros_ec_sensors_core.c    | 40 ++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 130362ca421b..75d9b617f6c8 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -25,6 +25,35 @@ static char *cros_ec_loc[] = {
 	[MOTIONSENSE_LOC_MAX] = "unknown",
 };
 
+static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev,
+					     u16 cmd_offset, u16 cmd, u32 *mask)
+{
+	int ret;
+	struct {
+		struct cros_ec_command msg;
+		union {
+			struct ec_params_get_cmd_versions params;
+			struct ec_response_get_cmd_versions resp;
+		};
+	} __packed buf = {
+		.msg = {
+			.version = 0,
+			.command = EC_CMD_GET_CMD_VERSIONS + cmd_offset,
+			.insize = sizeof(struct ec_response_get_cmd_versions),
+			.outsize = sizeof(struct ec_params_get_cmd_versions)
+			},
+		.params = {.cmd = cmd}
+	};
+
+	ret = cros_ec_cmd_xfer_status(ec_dev, &buf.msg);
+	if (ret < 0)
+		return ret;
+
+	*mask = buf.resp.version_mask;
+
+	return 0;
+}
+
 int cros_ec_sensors_core_init(struct platform_device *pdev,
 			      struct iio_dev *indio_dev,
 			      bool physical_device)
@@ -33,6 +62,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
 	struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
 	struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
 	struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
+	u32 ver_mask;
+	int ret;
 
 	platform_set_drvdata(pdev, indio_dev);
 
@@ -47,8 +78,15 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
 
 	mutex_init(&state->cmd_lock);
 
+	ret = cros_ec_get_host_cmd_version_mask(state->ec,
+						ec->cmd_offset,
+						EC_CMD_MOTION_SENSE_CMD,
+						&ver_mask);
+	if (ret < 0 || ver_mask == 0)
+		return -ENODEV;
+
 	/* Set up the host command structure. */
-	state->msg->version = 2;
+	state->msg->version = fls(ver_mask) - 1;;
 	state->msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
 	state->msg->outsize = sizeof(struct ec_params_motion_sense);
 
-- 
2.19.2


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

* Re: [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version
  2019-06-28 14:41 ` [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version Fabien Lahoudere
@ 2019-06-28 15:13   ` Enric Balletbo i Serra
  2019-07-01 20:53   ` [PATCH] iio: common: cros_ec_sensors: fix semicolon.cocci warnings kbuild test robot
  2019-07-01 20:53   ` [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version kbuild test robot
  2 siblings, 0 replies; 5+ messages in thread
From: Enric Balletbo i Serra @ 2019-06-28 15:13 UTC (permalink / raw)
  To: Fabien Lahoudere
  Cc: gwendal, egranata, kernel, Nick Vaccaro, Jonathan Cameron,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Benson Leung, Guenter Roeck, Alexios Zavras, Allison Randal,
	Thomas Gleixner, Greg Kroah-Hartman, linux-iio, linux-kernel

Hi,

Thanks for the quick respin, two few comments below

On 28/6/19 16:41, Fabien Lahoudere wrote:
> This patch adds a function to determine which version of the
> protocol is used to communicate with EC.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.com>
> Signed-off-by: Nick Vaccaro <nvaccaro@chromium.org>

The order must be the opposite, first Nick and then you.

> ---
>  .../cros_ec_sensors/cros_ec_sensors_core.c    | 40 ++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index 130362ca421b..75d9b617f6c8 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -25,6 +25,35 @@ static char *cros_ec_loc[] = {
>  	[MOTIONSENSE_LOC_MAX] = "unknown",
>  };
>  
> +static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev,
> +					     u16 cmd_offset, u16 cmd, u32 *mask)
> +{
> +	int ret;
> +	struct {
> +		struct cros_ec_command msg;
> +		union {
> +			struct ec_params_get_cmd_versions params;
> +			struct ec_response_get_cmd_versions resp;
> +		};
> +	} __packed buf = {
> +		.msg = {
> +			.version = 0,
> +			.command = EC_CMD_GET_CMD_VERSIONS + cmd_offset,
> +			.insize = sizeof(struct ec_response_get_cmd_versions),
> +			.outsize = sizeof(struct ec_params_get_cmd_versions)
> +			},
> +		.params = {.cmd = cmd}
> +	};
> +
> +	ret = cros_ec_cmd_xfer_status(ec_dev, &buf.msg);
> +	if (ret < 0)
> +		return ret;
> +
> +	*mask = buf.resp.version_mask;
> +
> +	return 0;
> +}
> +
>  int cros_ec_sensors_core_init(struct platform_device *pdev,
>  			      struct iio_dev *indio_dev,
>  			      bool physical_device)
> @@ -33,6 +62,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>  	struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
>  	struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
>  	struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
> +	u32 ver_mask;

If you follow the error path in the cros_ec_get_host_cmd_version_mask there is a
possible use of an uninitialized variable.

	u32 ver_mask = 0;

> +	int ret;
>  
>  	platform_set_drvdata(pdev, indio_dev);
>  
> @@ -47,8 +78,15 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>  
>  	mutex_init(&state->cmd_lock);
>  

What about adding a comment to explain the ver_mask thing for the record?

	/*
	 * If the EC is very old or misbehaving is it possible that the
	 * communication succeed and have the version mask set to an invalid
	 * value. So check that version mask is valid (!= 0), otherwise return
	 * an error.
	 */

At least this is what I understood from the previous discussion.

> +	ret = cros_ec_get_host_cmd_version_mask(state->ec,
> +						ec->cmd_offset,
> +						EC_CMD_MOTION_SENSE_CMD,
> +						&ver_mask);
> +	if (ret < 0 || ver_mask == 0)
> +		return -ENODEV;
> +
>  	/* Set up the host command structure. */
> -	state->msg->version = 2;
> +	state->msg->version = fls(ver_mask) - 1;;
>  	state->msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
>  	state->msg->outsize = sizeof(struct ec_params_motion_sense);
>  
> 

Thanks,
~ Enric

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

* [PATCH] iio: common: cros_ec_sensors: fix semicolon.cocci warnings
  2019-06-28 14:41 ` [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version Fabien Lahoudere
  2019-06-28 15:13   ` Enric Balletbo i Serra
@ 2019-07-01 20:53   ` kbuild test robot
  2019-07-01 20:53   ` [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version kbuild test robot
  2 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2019-07-01 20:53 UTC (permalink / raw)
  To: Fabien Lahoudere
  Cc: kbuild-all, gwendal, egranata, kernel, Fabien Lahoudere,
	Nick Vaccaro, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Benson Leung,
	Enric Balletbo i Serra, Guenter Roeck, Alexios Zavras,
	Allison Randal, Thomas Gleixner, Greg Kroah-Hartman, linux-iio,
	linux-kernel

From: kbuild test robot <lkp@intel.com>

drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c:89:41-42: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: a7862d65b738 ("iio: common: cros_ec_sensors: determine protocol version")
CC: Fabien Lahoudere <fabien.lahoudere@collabora.com>
Signed-off-by: kbuild test robot <lkp@intel.com>
---

url:    https://github.com/0day-ci/linux/commits/Fabien-Lahoudere/iio-common-cros_ec_sensors-determine-protocol-version/20190701-223411
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg

 cros_ec_sensors_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -86,7 +86,7 @@ int cros_ec_sensors_core_init(struct pla
 		return -ENODEV;
 
 	/* Set up the host command structure. */
-	state->msg->version = fls(ver_mask) - 1;;
+	state->msg->version = fls(ver_mask) - 1;
 	state->msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
 	state->msg->outsize = sizeof(struct ec_params_motion_sense);
 

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

* Re: [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version
  2019-06-28 14:41 ` [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version Fabien Lahoudere
  2019-06-28 15:13   ` Enric Balletbo i Serra
  2019-07-01 20:53   ` [PATCH] iio: common: cros_ec_sensors: fix semicolon.cocci warnings kbuild test robot
@ 2019-07-01 20:53   ` kbuild test robot
  2 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2019-07-01 20:53 UTC (permalink / raw)
  To: Fabien Lahoudere
  Cc: kbuild-all, gwendal, egranata, kernel, Fabien Lahoudere,
	Nick Vaccaro, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Benson Leung,
	Enric Balletbo i Serra, Guenter Roeck, Alexios Zavras,
	Allison Randal, Thomas Gleixner, Greg Kroah-Hartman, linux-iio,
	linux-kernel

Hi Fabien,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on next-20190625]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Fabien-Lahoudere/iio-common-cros_ec_sensors-determine-protocol-version/20190701-223411
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


coccinelle warnings: (new ones prefixed by >>)

>> drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c:89:41-42: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

end of thread, other threads:[~2019-07-01 20:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-28 14:40 [PATCH v2 0/1] iio: common: cros_ec_sensors: Add protocol v3 support Fabien Lahoudere
2019-06-28 14:41 ` [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version Fabien Lahoudere
2019-06-28 15:13   ` Enric Balletbo i Serra
2019-07-01 20:53   ` [PATCH] iio: common: cros_ec_sensors: fix semicolon.cocci warnings kbuild test robot
2019-07-01 20:53   ` [PATCH v2 1/1] iio: common: cros_ec_sensors: determine protocol version kbuild test robot

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.