chrome-platform.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls
@ 2022-03-18 16:54 Guenter Roeck
  2022-03-18 17:59 ` Daisuke Nojiri
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Guenter Roeck @ 2022-03-18 16:54 UTC (permalink / raw)
  To: Benson Leung
  Cc: chrome-platform, linux-kernel, Guenter Roeck, Daisuke Nojiri,
	Rob Barnes, Rajat Jain, Brian Norris, Parth Malkan

Commit 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use
cros_ec_cmd_xfer_status helper") inadvertendly changed the userspace ABI.
Previously, cros_ec ioctls would only report errors if the EC communication
failed, and otherwise return success and the result of the EC
communication. An EC command execution failure was reported in the EC
response field. The above mentioned commit changed this behavior, and the
ioctl itself would fail. This breaks userspace commands trying to analyze
the EC command execution error since the actual EC command response is no
longer reported to userspace.

Fix the problem by re-introducing the cros_ec_cmd_xfer() helper, and use it
to handle ioctl messages.

Fixes: 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use cros_ec_cmd_xfer_status helper")
Cc: Daisuke Nojiri <dnojiri@chromium.org>
Cc: Rob Barnes <robbarnes@google.com>
Cc: Rajat Jain <rajatja@google.com>
Cc: Brian Norris <briannorris@chromium.org>
Cc: Parth Malkan <parthmalkan@google.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Updated comments / return value description. No functional change.

 drivers/platform/chrome/cros_ec_chardev.c   |  2 +-
 drivers/platform/chrome/cros_ec_proto.c     | 50 +++++++++++++++++----
 include/linux/platform_data/cros_ec_proto.h |  3 ++
 3 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index e0bce869c49a..fd33de546aee 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -301,7 +301,7 @@ static long cros_ec_chardev_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg)
 	}
 
 	s_cmd->command += ec->cmd_offset;
-	ret = cros_ec_cmd_xfer_status(ec->ec_dev, s_cmd);
+	ret = cros_ec_cmd_xfer(ec->ec_dev, s_cmd);
 	/* Only copy data to userland if data was received. */
 	if (ret < 0)
 		goto exit;
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index c4caf2e2de82..ac1419881ff3 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -560,22 +560,28 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
 EXPORT_SYMBOL(cros_ec_query_all);
 
 /**
- * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
+ * cros_ec_cmd_xfer() - Send a command to the ChromeOS EC.
  * @ec_dev: EC device.
  * @msg: Message to write.
  *
- * Call this to send a command to the ChromeOS EC. This should be used instead of calling the EC's
- * cmd_xfer() callback directly. It returns success status only if both the command was transmitted
- * successfully and the EC replied with success status.
+ * Call this to send a command to the ChromeOS EC. This should be used instead
+ * of calling the EC's cmd_xfer() callback directly. This function does not
+ * convert EC command execution error codes to Linux error codes. Most
+ * in-kernel users will want to use cros_ec_cmd_xfer_status() instead since
+ * that function implements the conversion.
  *
  * Return:
- * >=0 - The number of bytes transferred
- * <0 - Linux error code
+ * >0 - EC command was executed successfully. The return value is the number
+ *      of bytes returned by the EC (excluding the header).
+ * =0 - EC communication was successful. EC command execution results are
+ *      reported in msg->result. The result will be EC_RES_SUCCESS if the
+ *      command was executed successfully or report an EC command execution
+ *      error.
+ * <0 - EC communication error. Return value is the Linux error code.
  */
-int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
-			    struct cros_ec_command *msg)
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
 {
-	int ret, mapped;
+	int ret;
 
 	mutex_lock(&ec_dev->lock);
 	if (ec_dev->proto_version == EC_PROTO_VERSION_UNKNOWN) {
@@ -616,6 +622,32 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
 	ret = send_command(ec_dev, msg);
 	mutex_unlock(&ec_dev->lock);
 
+	return ret;
+}
+EXPORT_SYMBOL(cros_ec_cmd_xfer);
+
+/**
+ * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
+ * @ec_dev: EC device.
+ * @msg: Message to write.
+ *
+ * Call this to send a command to the ChromeOS EC. This should be used instead of calling the EC's
+ * cmd_xfer() callback directly. It returns success status only if both the command was transmitted
+ * successfully and the EC replied with success status.
+ *
+ * Return:
+ * >=0 - The number of bytes transferred.
+ * <0 - Linux error code
+ */
+int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
+			    struct cros_ec_command *msg)
+{
+	int ret, mapped;
+
+	ret = cros_ec_cmd_xfer(ec_dev, msg);
+	if (ret < 0)
+		return ret;
+
 	mapped = cros_ec_map_error(msg->result);
 	if (mapped) {
 		dev_dbg(ec_dev->dev, "Command result (err: %d [%d])\n",
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index df3c78c92ca2..16931569adce 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -216,6 +216,9 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 int cros_ec_check_result(struct cros_ec_device *ec_dev,
 			 struct cros_ec_command *msg);
 
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+		     struct cros_ec_command *msg);
+
 int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
 			    struct cros_ec_command *msg);
 
-- 
2.35.1


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

* Re: [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls
  2022-03-18 16:54 [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls Guenter Roeck
@ 2022-03-18 17:59 ` Daisuke Nojiri
       [not found] ` <CAC0y+AjJxRGE570hTAq4WEtFrWa71e9q_h4xyN_52o4b9G_r2g@mail.gmail.com>
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Daisuke Nojiri @ 2022-03-18 17:59 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Benson Leung, chrome-platform, linux-kernel, Daisuke Nojiri,
	Rob Barnes, Rajat Jain, Brian Norris, Parth Malkan

The comment looks good to me. The rest also looks good to me (based on
the FROMLIST patch on Gerrit).


On Fri, Mar 18, 2022 at 9:56 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> Commit 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use
> cros_ec_cmd_xfer_status helper") inadvertendly changed the userspace ABI.
> Previously, cros_ec ioctls would only report errors if the EC communication
> failed, and otherwise return success and the result of the EC
> communication. An EC command execution failure was reported in the EC
> response field. The above mentioned commit changed this behavior, and the
> ioctl itself would fail. This breaks userspace commands trying to analyze
> the EC command execution error since the actual EC command response is no
> longer reported to userspace.
>
> Fix the problem by re-introducing the cros_ec_cmd_xfer() helper, and use it
> to handle ioctl messages.
>
> Fixes: 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use cros_ec_cmd_xfer_status helper")
> Cc: Daisuke Nojiri <dnojiri@chromium.org>
> Cc: Rob Barnes <robbarnes@google.com>
> Cc: Rajat Jain <rajatja@google.com>
> Cc: Brian Norris <briannorris@chromium.org>
> Cc: Parth Malkan <parthmalkan@google.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Updated comments / return value description. No functional change.
>
>  drivers/platform/chrome/cros_ec_chardev.c   |  2 +-
>  drivers/platform/chrome/cros_ec_proto.c     | 50 +++++++++++++++++----
>  include/linux/platform_data/cros_ec_proto.h |  3 ++
>  3 files changed, 45 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
> index e0bce869c49a..fd33de546aee 100644
> --- a/drivers/platform/chrome/cros_ec_chardev.c
> +++ b/drivers/platform/chrome/cros_ec_chardev.c
> @@ -301,7 +301,7 @@ static long cros_ec_chardev_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg)
>         }
>
>         s_cmd->command += ec->cmd_offset;
> -       ret = cros_ec_cmd_xfer_status(ec->ec_dev, s_cmd);
> +       ret = cros_ec_cmd_xfer(ec->ec_dev, s_cmd);
>         /* Only copy data to userland if data was received. */
>         if (ret < 0)
>                 goto exit;
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index c4caf2e2de82..ac1419881ff3 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -560,22 +560,28 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
>  EXPORT_SYMBOL(cros_ec_query_all);
>
>  /**
> - * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
> + * cros_ec_cmd_xfer() - Send a command to the ChromeOS EC.
>   * @ec_dev: EC device.
>   * @msg: Message to write.
>   *
> - * Call this to send a command to the ChromeOS EC. This should be used instead of calling the EC's
> - * cmd_xfer() callback directly. It returns success status only if both the command was transmitted
> - * successfully and the EC replied with success status.
> + * Call this to send a command to the ChromeOS EC. This should be used instead
> + * of calling the EC's cmd_xfer() callback directly. This function does not
> + * convert EC command execution error codes to Linux error codes. Most
> + * in-kernel users will want to use cros_ec_cmd_xfer_status() instead since
> + * that function implements the conversion.
>   *
>   * Return:
> - * >=0 - The number of bytes transferred
> - * <0 - Linux error code
> + * >0 - EC command was executed successfully. The return value is the number
> + *      of bytes returned by the EC (excluding the header).
> + * =0 - EC communication was successful. EC command execution results are
> + *      reported in msg->result. The result will be EC_RES_SUCCESS if the
> + *      command was executed successfully or report an EC command execution
> + *      error.
> + * <0 - EC communication error. Return value is the Linux error code.
>   */
> -int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> -                           struct cros_ec_command *msg)
> +int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
>  {
> -       int ret, mapped;
> +       int ret;
>
>         mutex_lock(&ec_dev->lock);
>         if (ec_dev->proto_version == EC_PROTO_VERSION_UNKNOWN) {
> @@ -616,6 +622,32 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
>         ret = send_command(ec_dev, msg);
>         mutex_unlock(&ec_dev->lock);
>
> +       return ret;
> +}
> +EXPORT_SYMBOL(cros_ec_cmd_xfer);
> +
> +/**
> + * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
> + * @ec_dev: EC device.
> + * @msg: Message to write.
> + *
> + * Call this to send a command to the ChromeOS EC. This should be used instead of calling the EC's
> + * cmd_xfer() callback directly. It returns success status only if both the command was transmitted
> + * successfully and the EC replied with success status.
> + *
> + * Return:
> + * >=0 - The number of bytes transferred.
> + * <0 - Linux error code
> + */
> +int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> +                           struct cros_ec_command *msg)
> +{
> +       int ret, mapped;
> +
> +       ret = cros_ec_cmd_xfer(ec_dev, msg);
> +       if (ret < 0)
> +               return ret;
> +
>         mapped = cros_ec_map_error(msg->result);
>         if (mapped) {
>                 dev_dbg(ec_dev->dev, "Command result (err: %d [%d])\n",
> diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
> index df3c78c92ca2..16931569adce 100644
> --- a/include/linux/platform_data/cros_ec_proto.h
> +++ b/include/linux/platform_data/cros_ec_proto.h
> @@ -216,6 +216,9 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  int cros_ec_check_result(struct cros_ec_device *ec_dev,
>                          struct cros_ec_command *msg);
>
> +int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> +                    struct cros_ec_command *msg);
> +
>  int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
>                             struct cros_ec_command *msg);
>
> --
> 2.35.1
>

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

* Re: [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls
       [not found] ` <CAC0y+AjJxRGE570hTAq4WEtFrWa71e9q_h4xyN_52o4b9G_r2g@mail.gmail.com>
@ 2022-03-18 18:00   ` Daisuke Nojiri
  0 siblings, 0 replies; 6+ messages in thread
From: Daisuke Nojiri @ 2022-03-18 18:00 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Benson Leung, chrome-platform, linux-kernel, Daisuke Nojiri,
	Rob Barnes, Rajat Jain, Brian Norris, Parth Malkan

The comment looks good to me. The rest also looks good to me (based on
the FROMLIST patch on Gerrit).

Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
On Fri, Mar 18, 2022 at 10:50 AM Daisuke Nojiri <dnojiri@google.com> wrote:
>
> The comment looks good to me. The rest also looks good to me (based on the FROMLIST patch on Gerrit).
>
> On Fri, Mar 18, 2022 at 9:56 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> Commit 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use
>> cros_ec_cmd_xfer_status helper") inadvertendly changed the userspace ABI.
>> Previously, cros_ec ioctls would only report errors if the EC communication
>> failed, and otherwise return success and the result of the EC
>> communication. An EC command execution failure was reported in the EC
>> response field. The above mentioned commit changed this behavior, and the
>> ioctl itself would fail. This breaks userspace commands trying to analyze
>> the EC command execution error since the actual EC command response is no
>> longer reported to userspace.
>>
>> Fix the problem by re-introducing the cros_ec_cmd_xfer() helper, and use it
>> to handle ioctl messages.
>>
>> Fixes: 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use cros_ec_cmd_xfer_status helper")
>> Cc: Daisuke Nojiri <dnojiri@chromium.org>
>> Cc: Rob Barnes <robbarnes@google.com>
>> Cc: Rajat Jain <rajatja@google.com>
>> Cc: Brian Norris <briannorris@chromium.org>
>> Cc: Parth Malkan <parthmalkan@google.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> v2: Updated comments / return value description. No functional change.
>>
>>  drivers/platform/chrome/cros_ec_chardev.c   |  2 +-
>>  drivers/platform/chrome/cros_ec_proto.c     | 50 +++++++++++++++++----
>>  include/linux/platform_data/cros_ec_proto.h |  3 ++
>>  3 files changed, 45 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
>> index e0bce869c49a..fd33de546aee 100644
>> --- a/drivers/platform/chrome/cros_ec_chardev.c
>> +++ b/drivers/platform/chrome/cros_ec_chardev.c
>> @@ -301,7 +301,7 @@ static long cros_ec_chardev_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg)
>>         }
>>
>>         s_cmd->command += ec->cmd_offset;
>> -       ret = cros_ec_cmd_xfer_status(ec->ec_dev, s_cmd);
>> +       ret = cros_ec_cmd_xfer(ec->ec_dev, s_cmd);
>>         /* Only copy data to userland if data was received. */
>>         if (ret < 0)
>>                 goto exit;
>> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
>> index c4caf2e2de82..ac1419881ff3 100644
>> --- a/drivers/platform/chrome/cros_ec_proto.c
>> +++ b/drivers/platform/chrome/cros_ec_proto.c
>> @@ -560,22 +560,28 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
>>  EXPORT_SYMBOL(cros_ec_query_all);
>>
>>  /**
>> - * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
>> + * cros_ec_cmd_xfer() - Send a command to the ChromeOS EC.
>>   * @ec_dev: EC device.
>>   * @msg: Message to write.
>>   *
>> - * Call this to send a command to the ChromeOS EC. This should be used instead of calling the EC's
>> - * cmd_xfer() callback directly. It returns success status only if both the command was transmitted
>> - * successfully and the EC replied with success status.
>> + * Call this to send a command to the ChromeOS EC. This should be used instead
>> + * of calling the EC's cmd_xfer() callback directly. This function does not
>> + * convert EC command execution error codes to Linux error codes. Most
>> + * in-kernel users will want to use cros_ec_cmd_xfer_status() instead since
>> + * that function implements the conversion.
>>   *
>>   * Return:
>> - * >=0 - The number of bytes transferred
>> - * <0 - Linux error code
>> + * >0 - EC command was executed successfully. The return value is the number
>> + *      of bytes returned by the EC (excluding the header).
>> + * =0 - EC communication was successful. EC command execution results are
>> + *      reported in msg->result. The result will be EC_RES_SUCCESS if the
>> + *      command was executed successfully or report an EC command execution
>> + *      error.
>> + * <0 - EC communication error. Return value is the Linux error code.
>>   */
>> -int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
>> -                           struct cros_ec_command *msg)
>> +int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, struct cros_ec_command *msg)
>>  {
>> -       int ret, mapped;
>> +       int ret;
>>
>>         mutex_lock(&ec_dev->lock);
>>         if (ec_dev->proto_version == EC_PROTO_VERSION_UNKNOWN) {
>> @@ -616,6 +622,32 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
>>         ret = send_command(ec_dev, msg);
>>         mutex_unlock(&ec_dev->lock);
>>
>> +       return ret;
>> +}
>> +EXPORT_SYMBOL(cros_ec_cmd_xfer);
>> +
>> +/**
>> + * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
>> + * @ec_dev: EC device.
>> + * @msg: Message to write.
>> + *
>> + * Call this to send a command to the ChromeOS EC. This should be used instead of calling the EC's
>> + * cmd_xfer() callback directly. It returns success status only if both the command was transmitted
>> + * successfully and the EC replied with success status.
>> + *
>> + * Return:
>> + * >=0 - The number of bytes transferred.
>> + * <0 - Linux error code
>> + */
>> +int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
>> +                           struct cros_ec_command *msg)
>> +{
>> +       int ret, mapped;
>> +
>> +       ret = cros_ec_cmd_xfer(ec_dev, msg);
>> +       if (ret < 0)
>> +               return ret;
>> +
>>         mapped = cros_ec_map_error(msg->result);
>>         if (mapped) {
>>                 dev_dbg(ec_dev->dev, "Command result (err: %d [%d])\n",
>> diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
>> index df3c78c92ca2..16931569adce 100644
>> --- a/include/linux/platform_data/cros_ec_proto.h
>> +++ b/include/linux/platform_data/cros_ec_proto.h
>> @@ -216,6 +216,9 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>>  int cros_ec_check_result(struct cros_ec_device *ec_dev,
>>                          struct cros_ec_command *msg);
>>
>> +int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
>> +                    struct cros_ec_command *msg);
>> +
>>  int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
>>                             struct cros_ec_command *msg);
>>
>> --
>> 2.35.1
>>

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

* Re: [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls
  2022-03-18 16:54 [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls Guenter Roeck
  2022-03-18 17:59 ` Daisuke Nojiri
       [not found] ` <CAC0y+AjJxRGE570hTAq4WEtFrWa71e9q_h4xyN_52o4b9G_r2g@mail.gmail.com>
@ 2022-03-19  1:22 ` Brian Norris
  2022-04-19  3:30 ` patchwork-bot+chrome-platform
  2022-04-20  9:10 ` patchwork-bot+chrome-platform
  4 siblings, 0 replies; 6+ messages in thread
From: Brian Norris @ 2022-03-19  1:22 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Benson Leung, chrome-platform, linux-kernel, Daisuke Nojiri,
	Rob Barnes, Rajat Jain, Parth Malkan

On Fri, Mar 18, 2022 at 09:54:22AM -0700, Guenter Roeck wrote:
> Commit 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use
> cros_ec_cmd_xfer_status helper") inadvertendly changed the userspace ABI.
> Previously, cros_ec ioctls would only report errors if the EC communication
> failed, and otherwise return success and the result of the EC
> communication. An EC command execution failure was reported in the EC
> response field. The above mentioned commit changed this behavior, and the
> ioctl itself would fail. This breaks userspace commands trying to analyze
> the EC command execution error since the actual EC command response is no
> longer reported to userspace.
> 
> Fix the problem by re-introducing the cros_ec_cmd_xfer() helper, and use it
> to handle ioctl messages.
> 
> Fixes: 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use cros_ec_cmd_xfer_status helper")

Probably could use a Cc: <stable@vger.kernel.org> in here, since this is
a user-space ABI regression. But these days, there's effectively no
difference vs. just a Fixes tag, because someone's bot will usually pick
it up.

> Cc: Daisuke Nojiri <dnojiri@chromium.org>
> Cc: Rob Barnes <robbarnes@google.com>
> Cc: Rajat Jain <rajatja@google.com>
> Cc: Brian Norris <briannorris@chromium.org>
> Cc: Parth Malkan <parthmalkan@google.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Updated comments / return value description. No functional change.

Reviewed-by: Brian Norris <briannorris@chromium.org>

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

* Re: [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls
  2022-03-18 16:54 [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls Guenter Roeck
                   ` (2 preceding siblings ...)
  2022-03-19  1:22 ` Brian Norris
@ 2022-04-19  3:30 ` patchwork-bot+chrome-platform
  2022-04-20  9:10 ` patchwork-bot+chrome-platform
  4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+chrome-platform @ 2022-04-19  3:30 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: bleung, chrome-platform, linux-kernel, dnojiri, robbarnes,
	rajatja, briannorris, parthmalkan

Hello:

This patch was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Fri, 18 Mar 2022 09:54:22 -0700 you wrote:
> Commit 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use
> cros_ec_cmd_xfer_status helper") inadvertendly changed the userspace ABI.
> Previously, cros_ec ioctls would only report errors if the EC communication
> failed, and otherwise return success and the result of the EC
> communication. An EC command execution failure was reported in the EC
> response field. The above mentioned commit changed this behavior, and the
> ioctl itself would fail. This breaks userspace commands trying to analyze
> the EC command execution error since the actual EC command response is no
> longer reported to userspace.
> 
> [...]

Here is the summary with links:
  - [v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls
    https://git.kernel.org/chrome-platform/c/57b888ca2541

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls
  2022-03-18 16:54 [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls Guenter Roeck
                   ` (3 preceding siblings ...)
  2022-04-19  3:30 ` patchwork-bot+chrome-platform
@ 2022-04-20  9:10 ` patchwork-bot+chrome-platform
  4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+chrome-platform @ 2022-04-20  9:10 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: bleung, chrome-platform, linux-kernel, dnojiri, robbarnes,
	rajatja, briannorris, parthmalkan

Hello:

This patch was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Fri, 18 Mar 2022 09:54:22 -0700 you wrote:
> Commit 413dda8f2c6f ("platform/chrome: cros_ec_chardev: Use
> cros_ec_cmd_xfer_status helper") inadvertendly changed the userspace ABI.
> Previously, cros_ec ioctls would only report errors if the EC communication
> failed, and otherwise return success and the result of the EC
> communication. An EC command execution failure was reported in the EC
> response field. The above mentioned commit changed this behavior, and the
> ioctl itself would fail. This breaks userspace commands trying to analyze
> the EC command execution error since the actual EC command response is no
> longer reported to userspace.
> 
> [...]

Here is the summary with links:
  - [v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls
    https://git.kernel.org/chrome-platform/c/57b888ca2541

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2022-04-20  9:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-18 16:54 [PATCH v2] platform/chrome: Re-introduce cros_ec_cmd_xfer and use it for ioctls Guenter Roeck
2022-03-18 17:59 ` Daisuke Nojiri
     [not found] ` <CAC0y+AjJxRGE570hTAq4WEtFrWa71e9q_h4xyN_52o4b9G_r2g@mail.gmail.com>
2022-03-18 18:00   ` Daisuke Nojiri
2022-03-19  1:22 ` Brian Norris
2022-04-19  3:30 ` patchwork-bot+chrome-platform
2022-04-20  9:10 ` patchwork-bot+chrome-platform

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).