* [PATCH 00/17] platform/chrome: Replace cros_ec_cmd_xfer_status
@ 2020-01-30 20:30 ` Prashant Malani
0 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Akshu Agrawal, Alexandre Belloni,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Benjamin Tissoires, Dmitry Torokhov, Enric Balletbo i Serra,
Evan Green, Fabien Lahoudere, Guenter Roeck, Gwendal Grignou,
Hans Verkuil, Hartmut Knaack, Jonathan Cameron,
Lars-Peter Clausen, Lee Jones, linux-i2c
Many callers of cros_ec_cmd_xfer_status() use similar setup and cleanup
code, including setting up the cros_ec_command message struct and
copying the received buffer.
This series introduces a replacement function cros_ec_send_cmd_msg() that
performs this setup and teardown, and then updates all call sites that
used xfer_status() to use the new function instead.
The final patch in the series drops cros_ec_cmd_xfer_status() altogether.
Prashant Malani (17):
platform/chrome: Add EC command msg wrapper
platform/chrome: chardev: Use send_cmd_msg()
platform/chrome: proto: Use send_cmd_msg
platform/chrome: usbpd_logger: Use cmd_send_msg()
platform/chrome: sensorhub: Use send_cmd_msg()
platform/chrome: debugfs: Use send_cmd_msg()
platform/chrome: sysfs: Use send_cmd_msg()
extcon: cros_ec: Use cros_ec_send_cmd_msg()
hid: google-hammer: Use cros_ec_send_cmd_msg()
iio: cros_ec: Use cros_ec_send_cmd_msg()
ASoC: cros_ec_codec: Use cros_ec_send_cmd_msg()
power: supply: cros: Use cros_ec_send_cmd_msg()
pwm: cros-ec: Remove cros_ec_cmd_xfer_status()
rtc: cros-ec: Use cros_ec_send_cmd_msg()
media: cros-ec-cec: Use cros_ec_send_cmd_msg()
i2c: cros-ec-tunnel: Use cros_ec_send_cmd_msg()
platform/chrome: Drop cros_ec_cmd_xfer_status()
drivers/extcon/extcon-usbc-cros-ec.c | 62 ++------
drivers/hid/hid-google-hammer.c | 23 +--
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 23 ++-
.../cros_ec_sensors/cros_ec_sensors_core.c | 43 +++---
.../media/platform/cros-ec-cec/cros-ec-cec.c | 39 ++---
drivers/platform/chrome/cros_ec_chardev.c | 18 +--
drivers/platform/chrome/cros_ec_debugfs.c | 135 ++++++------------
drivers/platform/chrome/cros_ec_proto.c | 75 ++++++----
drivers/platform/chrome/cros_ec_sensorhub.c | 29 ++--
drivers/platform/chrome/cros_ec_sysfs.c | 106 ++++++--------
drivers/platform/chrome/cros_usbpd_logger.c | 13 +-
drivers/power/supply/cros_usbpd-charger.c | 63 ++------
drivers/pwm/pwm-cros-ec.c | 27 ++--
drivers/rtc/rtc-cros-ec.c | 27 ++--
include/linux/platform_data/cros_ec_proto.h | 6 +-
sound/soc/codecs/cros_ec_codec.c | 71 +++------
16 files changed, 276 insertions(+), 484 deletions(-)
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 01/17] platform/chrome: Add EC command msg wrapper
2020-01-30 20:30 ` Prashant Malani
(?)
@ 2020-01-30 20:30 ` Prashant Malani
2020-02-03 15:27 ` Enric Balletbo i Serra
-1 siblings, 1 reply; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra,
Guenter Roeck, Lee Jones, Gwendal Grignou, Jonathan Cameron,
Evan Green
Many callers of cros_ec_cmd_xfer_status() use a similar set up of
allocating and filling a message buffer and then copying any received
data to a target buffer.
Create a utility function cros_ec_send_cmd_msg() that performs this
setup so that callers can use this function instead. Subsequent patches
will convert callers of cros_ec_cmd_xfer_status() to the new function
instead.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/platform/chrome/cros_ec_proto.c | 57 +++++++++++++++++++++
include/linux/platform_data/cros_ec_proto.h | 5 ++
2 files changed, 62 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index da1b1c45043333..53f3bfac71d90e 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -5,6 +5,7 @@
#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/mfd/cros_ec.h>
#include <linux/module.h>
#include <linux/platform_data/cros_ec_commands.h>
#include <linux/platform_data/cros_ec_proto.h>
@@ -570,6 +571,62 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
}
EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
+/**
+ * cros_ec_send_cmd_msg() - Utility function to send commands to ChromeOS EC.
+ * @ec: EC device struct.
+ * @version: Command version number (often 0).
+ * @command: Command ID including offset.
+ * @outdata: Data to be sent to the EC.
+ * @outsize: Size of the &outdata buffer.
+ * @indata: Data to be received from the EC.
+ * @insize: Size of the &indata buffer.
+ *
+ * This function is a wrapper around &cros_ec_cmd_xfer_status, and performs
+ * some of the common work involved with sending a command to the EC. This
+ * includes allocating and filling up a &struct cros_ec_command message buffer,
+ * and copying the received data to another buffer.
+ *
+ * Return: The number of bytes transferred on success or negative error code.
+ */
+int cros_ec_send_cmd_msg(struct cros_ec_device *ec, unsigned int version,
+ unsigned int command, void *outdata,
+ unsigned int outsize, void *indata,
+ unsigned int insize)
+{
+ struct cros_ec_command *msg;
+ int ret;
+
+ msg = kzalloc(sizeof(*msg) + max(outsize, insize), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = version;
+ msg->command = command;
+ msg->outsize = outsize;
+ msg->insize = insize;
+
+ if (outdata && outsize > 0)
+ memcpy(msg->data, outdata, outsize);
+
+ ret = cros_ec_cmd_xfer(ec, msg);
+ if (ret < 0) {
+ dev_err(ec->dev, "Command xfer error (err:%d)\n", ret);
+ goto cleanup;
+ } else if (msg->result != EC_RES_SUCCESS) {
+ dev_dbg(ec->dev, "Command result (err: %d)\n", msg->result);
+ ret = -EPROTO;
+ goto cleanup;
+ }
+
+ if (insize)
+ memcpy(indata, msg->data, insize);
+
+cleanup:
+ kfree(msg);
+ return ret;
+}
+EXPORT_SYMBOL(cros_ec_send_cmd_msg);
+
static int get_next_event_xfer(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg,
struct ec_response_get_next_event_v1 *event,
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 30098a5515231d..166ce26bdd79eb 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -201,6 +201,11 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg);
+int cros_ec_send_cmd_msg(struct cros_ec_device *ec_dev, unsigned int version,
+ unsigned int command, void *outdata,
+ unsigned int outsize, void *indata,
+ unsigned int insize);
+
int cros_ec_register(struct cros_ec_device *ec_dev);
int cros_ec_unregister(struct cros_ec_device *ec_dev);
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 01/17] platform/chrome: Add EC command msg wrapper
2020-01-30 20:30 ` [PATCH 01/17] platform/chrome: Add EC command msg wrapper Prashant Malani
@ 2020-02-03 15:27 ` Enric Balletbo i Serra
2020-02-03 18:24 ` Prashant Malani
0 siblings, 1 reply; 38+ messages in thread
From: Enric Balletbo i Serra @ 2020-02-03 15:27 UTC (permalink / raw)
To: Prashant Malani, linux-kernel
Cc: Benson Leung, Guenter Roeck, Lee Jones, Gwendal Grignou,
Jonathan Cameron, Evan Green
Hi Prashant,
Many thanks to work on this. Some comments below ...
On 30/1/20 21:30, Prashant Malani wrote:
> Many callers of cros_ec_cmd_xfer_status() use a similar set up of
> allocating and filling a message buffer and then copying any received
> data to a target buffer.
>
> Create a utility function cros_ec_send_cmd_msg() that performs this
> setup so that callers can use this function instead. Subsequent patches
> will convert callers of cros_ec_cmd_xfer_status() to the new function
> instead.
>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> ---
> drivers/platform/chrome/cros_ec_proto.c | 57 +++++++++++++++++++++
> include/linux/platform_data/cros_ec_proto.h | 5 ++
> 2 files changed, 62 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index da1b1c45043333..53f3bfac71d90e 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -5,6 +5,7 @@
>
> #include <linux/delay.h>
> #include <linux/device.h>
> +#include <linux/mfd/cros_ec.h>
> #include <linux/module.h>
> #include <linux/platform_data/cros_ec_commands.h>
> #include <linux/platform_data/cros_ec_proto.h>
> @@ -570,6 +571,62 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> }
> EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
>
> +/**
> + * cros_ec_send_cmd_msg() - Utility function to send commands to ChromeOS EC.
I'm wondering if just cros_ec_cmd() shouldn't be a better name. If it's a
replacement of current user usage of cros_ec_cmd_xfer and
cros_ec_cmd_xfer_status, this will be used a lot, and have a short name and
clear will help the users of this helper.
> + * @ec: EC device struct.
> + * @version: Command version number (often 0).
> + * @command: Command ID including offset.
> + * @outdata: Data to be sent to the EC.
> + * @outsize: Size of the &outdata buffer.
> + * @indata: Data to be received from the EC.
> + * @insize: Size of the &indata buffer.
> + *
> + * This function is a wrapper around &cros_ec_cmd_xfer_status, and performs
You say that is a wrapper around cros_ec_cmd_xfer_status but then you remove
that function, and rewrite the doc here. Just explain for what is this helper
without referencing cros_ec_cmd_xfer_status and cros_ec_cmd_xfer.
> + * some of the common work involved with sending a command to the EC. This
> + * includes allocating and filling up a &struct cros_ec_command message buffer,
> + * and copying the received data to another buffer.
> + *
> + * Return: The number of bytes transferred on success or negative error code.
> + */
> +int cros_ec_send_cmd_msg(struct cros_ec_device *ec, unsigned int version,
> + unsigned int command, void *outdata,
> + unsigned int outsize, void *indata,
> + unsigned int insize)
Should we change the parameter types from "unsigned int" to "u32" to match both
ec hardware and the storage type in struct cros_ec_command?
> +{
> + struct cros_ec_command *msg;
> + int ret;
> +
> + msg = kzalloc(sizeof(*msg) + max(outsize, insize), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + msg->version = version;
> + msg->command = command;
> + msg->outsize = outsize;
> + msg->insize = insize;
> +
> + if (outdata && outsize > 0)
> + memcpy(msg->data, outdata, outsize);
> +
> + ret = cros_ec_cmd_xfer(ec, msg);
> + if (ret < 0) {
> + dev_err(ec->dev, "Command xfer error (err:%d)\n", ret);
> + goto cleanup;
> + } else if (msg->result != EC_RES_SUCCESS) {
> + dev_dbg(ec->dev, "Command result (err: %d)\n", msg->result);
> + ret = -EPROTO;
> + goto cleanup;
> + }
> +
> + if (insize)
> + memcpy(indata, msg->data, insize);
> +
> +cleanup:
> + kfree(msg);
> + return ret;
> +}
> +EXPORT_SYMBOL(cros_ec_send_cmd_msg);
> +
> static int get_next_event_xfer(struct cros_ec_device *ec_dev,
> struct cros_ec_command *msg,
> struct ec_response_get_next_event_v1 *event,
> diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
> index 30098a5515231d..166ce26bdd79eb 100644
> --- a/include/linux/platform_data/cros_ec_proto.h
> +++ b/include/linux/platform_data/cros_ec_proto.h
> @@ -201,6 +201,11 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> struct cros_ec_command *msg);
>
> +int cros_ec_send_cmd_msg(struct cros_ec_device *ec_dev, unsigned int version,
> + unsigned int command, void *outdata,
> + unsigned int outsize, void *indata,
> + unsigned int insize);
> +
> int cros_ec_register(struct cros_ec_device *ec_dev);
>
> int cros_ec_unregister(struct cros_ec_device *ec_dev);
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 01/17] platform/chrome: Add EC command msg wrapper
2020-02-03 15:27 ` Enric Balletbo i Serra
@ 2020-02-03 18:24 ` Prashant Malani
0 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-02-03 18:24 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Linux Kernel Mailing List, Benson Leung, Guenter Roeck,
Lee Jones, Gwendal Grignou, Jonathan Cameron, Evan Green
On Mon, Feb 3, 2020 at 7:27 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Prashant,
>
> Many thanks to work on this. Some comments below ...
> > #include <linux/platform_data/cros_ec_commands.h>
> > #include <linux/platform_data/cros_ec_proto.h>
> > @@ -570,6 +571,62 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> > }
> > EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
> >
> > +/**
> > + * cros_ec_send_cmd_msg() - Utility function to send commands to ChromeOS EC.
>
> I'm wondering if just cros_ec_cmd() shouldn't be a better name. If it's a
> replacement of current user usage of cros_ec_cmd_xfer and
> cros_ec_cmd_xfer_status, this will be used a lot, and have a short name and
> clear will help the users of this helper.
Sounds good. I think in another patch (13/17) there is a point about
how it's tough to get rid of cros_ec_cmd_xfer() without non-trivial
rework to a few drivers, but cros_ec_cmd() is certainly more concise.
>
> > + * @ec: EC device struct.
> > + * @version: Command version number (often 0).
> > + * @command: Command ID including offset.
> > + * @outdata: Data to be sent to the EC.
> > + * @outsize: Size of the &outdata buffer.
> > + * @indata: Data to be received from the EC.
> > + * @insize: Size of the &indata buffer.
> > + *
> > + * This function is a wrapper around &cros_ec_cmd_xfer_status, and performs
>
> You say that is a wrapper around cros_ec_cmd_xfer_status but then you remove
> that function, and rewrite the doc here. Just explain for what is this helper
> without referencing cros_ec_cmd_xfer_status and cros_ec_cmd_xfer.
My apologies, I should have edited it to say cros_ec_cmd_xfer()
instead of _status().
I'll update the documentation to just avoid the two altogether.
>
> > + * some of the common work involved with sending a command to the EC. This
> > + * includes allocating and filling up a &struct cros_ec_command message buffer,
> > + * and copying the received data to another buffer.
> > + *
> > + * Return: The number of bytes transferred on success or negative error code.
> > + */
> > +int cros_ec_send_cmd_msg(struct cros_ec_device *ec, unsigned int version,
> > + unsigned int command, void *outdata,
> > + unsigned int outsize, void *indata,
> > + unsigned int insize)
>
> Should we change the parameter types from "unsigned int" to "u32" to match both
> ec hardware and the storage type in struct cros_ec_command?
Sounds good. I will make the change.
>
> > +{
> > + struct cros_ec_command *msg;
> > + int ret;
> > +
> > + msg = kzalloc(sizeof(*msg) + max(outsize, insize), GFP_KERNEL);
> > + if (!msg)
> > + return -ENOMEM;
> > +
> > + msg->version = version;
> > + msg->command = command;
> > + msg->outsize = outsize;
> > + msg->insize = insize;
> > +
> > + if (outdata && outsize > 0)
> > + memcpy(msg->data, outdata, outsize);
> > +
> > + ret = cros_ec_cmd_xfer(ec, msg);
> > + if (ret < 0) {
> > + dev_err(ec->dev, "Command xfer error (err:%d)\n", ret);
> > + goto cleanup;
> > + } else if (msg->result != EC_RES_SUCCESS) {
> > + dev_dbg(ec->dev, "Command result (err: %d)\n", msg->result);
> > + ret = -EPROTO;
> > + goto cleanup;
> > + }
> > +
> > + if (insize)
> > + memcpy(indata, msg->data, insize);
> > +
> > +cleanup:
> > + kfree(msg);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(cros_ec_send_cmd_msg);
> > +
> > static int get_next_event_xfer(struct cros_ec_device *ec_dev,
> > struct cros_ec_command *msg,
> > struct ec_response_get_next_event_v1 *event,
> > diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
> > index 30098a5515231d..166ce26bdd79eb 100644
> > --- a/include/linux/platform_data/cros_ec_proto.h
> > +++ b/include/linux/platform_data/cros_ec_proto.h
> > @@ -201,6 +201,11 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> > int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> > struct cros_ec_command *msg);
> >
> > +int cros_ec_send_cmd_msg(struct cros_ec_device *ec_dev, unsigned int version,
> > + unsigned int command, void *outdata,
> > + unsigned int outsize, void *indata,
> > + unsigned int insize);
> > +
> > int cros_ec_register(struct cros_ec_device *ec_dev);
> >
> > int cros_ec_unregister(struct cros_ec_device *ec_dev);
> >
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 02/17] platform/chrome: chardev: Use send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
(?)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra, Guenter Roeck
Update the Chrome OS character device driver to use the newly introduced
cros_ec_send_cmd_msg() function instead of cros_ec_cmd_xfer_status(),
thus avoiding message buffer set up work which is already done by the
new function.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/platform/chrome/cros_ec_chardev.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index 74ded441bb5006..f8136addb8b3f7 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -58,25 +58,21 @@ static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
"unknown", "read-only", "read-write", "invalid",
};
struct ec_response_get_version *resp;
- struct cros_ec_command *msg;
int ret;
- msg = kzalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
- if (!msg)
+ resp = kzalloc(sizeof(*resp), GFP_KERNEL);
+ if (!resp)
return -ENOMEM;
- msg->command = EC_CMD_GET_VERSION + ec->cmd_offset;
- msg->insize = sizeof(*resp);
-
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 0,
+ ec->cmd_offset + EC_CMD_GET_VERSION, NULL, 0,
+ resp, sizeof(*resp));
if (ret < 0) {
snprintf(str, maxlen,
- "Unknown EC version, returned error: %d\n",
- msg->result);
+ "Unknown EC version, returned error: %d\n", ret);
goto exit;
}
- resp = (struct ec_response_get_version *)msg->data;
if (resp->current_image >= ARRAY_SIZE(current_image_name))
resp->current_image = 3; /* invalid */
@@ -86,7 +82,7 @@ static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
ret = 0;
exit:
- kfree(msg);
+ kfree(resp);
return ret;
}
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 03/17] platform/chrome: proto: Use send_cmd_msg
2020-01-30 20:30 ` Prashant Malani
` (2 preceding siblings ...)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra, Guenter Roeck
Replace the use of cros_ec_cmd_xfer_status() with the new function
cros_ec_send_cmd_msg().
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/platform/chrome/cros_ec_proto.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 53f3bfac71d90e..efd1c0b6a830c8 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -808,31 +808,21 @@ EXPORT_SYMBOL(cros_ec_get_host_event);
*/
int cros_ec_check_features(struct cros_ec_dev *ec, int feature)
{
- struct cros_ec_command *msg;
int ret;
if (ec->features[0] == -1U && ec->features[1] == -1U) {
/* features bitmap not read yet */
- msg = kzalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
- msg->insize = sizeof(ec->features);
-
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 0,
+ ec->cmd_offset + EC_CMD_GET_FEATURES,
+ NULL, 0, ec->features,
+ sizeof(ec->features));
if (ret < 0) {
- dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
- ret, msg->result);
+ dev_warn(ec->dev, "cannot get EC features: %d\n", ret);
memset(ec->features, 0, sizeof(ec->features));
- } else {
- memcpy(ec->features, msg->data, sizeof(ec->features));
}
dev_dbg(ec->dev, "EC features %08x %08x\n",
ec->features[0], ec->features[1]);
-
- kfree(msg);
}
return ec->features[feature / 32] & EC_FEATURE_MASK_0(feature);
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 04/17] platform/chrome: usbpd_logger: Use cmd_send_msg()
2020-01-30 20:30 ` Prashant Malani
` (3 preceding siblings ...)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel; +Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra
Convert the earlier call of cros_ec_cmd_xfer_status() to
cros_ec_send_cmd_msg() which does the buffer setup and message
allocation.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/platform/chrome/cros_usbpd_logger.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/chrome/cros_usbpd_logger.c b/drivers/platform/chrome/cros_usbpd_logger.c
index 374cdd1e868ac1..356bc2fe068466 100644
--- a/drivers/platform/chrome/cros_usbpd_logger.c
+++ b/drivers/platform/chrome/cros_usbpd_logger.c
@@ -62,19 +62,16 @@ static int append_str(char *buf, int pos, const char *fmt, ...)
static struct ec_response_pd_log *ec_get_log_entry(struct logger_data *logger)
{
struct cros_ec_dev *ec_dev = logger->ec_dev;
- struct cros_ec_command *msg;
int ret;
- msg = (struct cros_ec_command *)logger->ec_buffer;
-
- msg->command = ec_dev->cmd_offset + EC_CMD_PD_GET_LOG_ENTRY;
- msg->insize = CROS_USBPD_LOG_RESP_SIZE;
-
- ret = cros_ec_cmd_xfer_status(ec_dev->ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec_dev->ec_dev, 0,
+ ec_dev->cmd_offset + EC_CMD_PD_GET_LOG_ENTRY,
+ NULL, 0,
+ logger->ec_buffer, CROS_USBPD_LOG_RESP_SIZE);
if (ret < 0)
return ERR_PTR(ret);
- return (struct ec_response_pd_log *)msg->data;
+ return (struct ec_response_pd_log *)logger->ec_buffer;
}
static void cros_usbpd_print_log_entry(struct ec_response_pd_log *r,
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 05/17] platform/chrome: sensorhub: Use send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
` (4 preceding siblings ...)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra, Guenter Roeck
Update the register() function to use the new
cros_ec_send_cmd_msg() function instead of cros_ec_cmd_xfer_status().
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/platform/chrome/cros_ec_sensorhub.c | 29 +++++++++------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c
index 04d8879689e985..8a05e7f5a71b2d 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub.c
@@ -54,7 +54,7 @@ static int cros_ec_sensorhub_register(struct device *dev,
struct cros_ec_dev *ec = sensorhub->ec;
struct ec_params_motion_sense *params;
struct ec_response_motion_sense *resp;
- struct cros_ec_command *msg;
+ void *ec_buf;
int ret, i, sensor_num;
char *name;
@@ -71,27 +71,23 @@ static int cros_ec_sensorhub_register(struct device *dev,
return -EINVAL;
}
- /* Prepare a message to send INFO command to each sensor. */
- msg = kzalloc(sizeof(*msg) + max(sizeof(*params), sizeof(*resp)),
- GFP_KERNEL);
- if (!msg)
+ ec_buf = kzalloc(max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
+ if (!ec_buf)
return -ENOMEM;
- msg->version = 1;
- msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
- msg->outsize = sizeof(*params);
- msg->insize = sizeof(*resp);
- params = (struct ec_params_motion_sense *)msg->data;
- resp = (struct ec_response_motion_sense *)msg->data;
+ params = ec_buf;
+ resp = ec_buf;
for (i = 0; i < sensor_num; i++) {
params->cmd = MOTIONSENSE_CMD_INFO;
params->info.sensor_num = i;
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 1,
+ EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset,
+ params, sizeof(*params), resp, sizeof(*resp));
if (ret < 0) {
- dev_warn(dev, "no info for EC sensor %d : %d/%d\n",
- i, ret, msg->result);
+ dev_warn(dev, "no info for EC sensor %d : %d\n",
+ i, ret);
continue;
}
@@ -141,11 +137,10 @@ static int cros_ec_sensorhub_register(struct device *dev,
goto error;
}
- kfree(msg);
- return 0;
+ ret = 0;
error:
- kfree(msg);
+ kfree(ec_buf);
return ret;
}
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 06/17] platform/chrome: debugfs: Use send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
` (5 preceding siblings ...)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra, Guenter Roeck
Replace cros_ec_cmd_xfer_status() calls to the new function
cros_ec_send_cmd_msg(), which is more readable and does the message
setup code.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/platform/chrome/cros_ec_debugfs.c | 135 ++++++++--------------
1 file changed, 46 insertions(+), 89 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 6ae484989d1f5f..58f2db1a4af032 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -32,7 +32,7 @@
* @ec: EC device this debugfs information belongs to
* @dir: dentry for debugfs files
* @log_buffer: circular buffer for console log information
- * @read_msg: preallocated EC command and buffer to read console log
+ * @ec_buf: preallocated EC buffer to send and receive log commands and output.
* @log_mutex: mutex to protect circular buffer
* @log_wq: waitqueue for log readers
* @log_poll_work: recurring task to poll EC for new console log data
@@ -43,7 +43,7 @@ struct cros_ec_debugfs {
struct dentry *dir;
/* EC log */
struct circ_buf log_buffer;
- struct cros_ec_command *read_msg;
+ void *ec_buf;
struct mutex log_mutex;
wait_queue_head_t log_wq;
struct delayed_work log_poll_work;
@@ -63,18 +63,17 @@ static void cros_ec_console_log_work(struct work_struct *__work)
log_poll_work);
struct cros_ec_dev *ec = debug_info->ec;
struct circ_buf *cb = &debug_info->log_buffer;
- struct cros_ec_command snapshot_msg = {
- .command = EC_CMD_CONSOLE_SNAPSHOT + ec->cmd_offset,
- };
struct ec_params_console_read_v1 *read_params =
- (struct ec_params_console_read_v1 *)debug_info->read_msg->data;
- uint8_t *ec_buffer = (uint8_t *)debug_info->read_msg->data;
+ (struct ec_params_console_read_v1 *)debug_info->ec_buf;
+ uint8_t *ec_buffer = (uint8_t *)debug_info->ec_buf;
int idx;
int buf_space;
int ret;
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, &snapshot_msg);
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 0,
+ EC_CMD_CONSOLE_SNAPSHOT + ec->cmd_offset,
+ NULL, 0, NULL, 0);
if (ret < 0)
goto resched;
@@ -91,8 +90,11 @@ static void cros_ec_console_log_work(struct work_struct *__work)
memset(read_params, '\0', sizeof(*read_params));
read_params->subcmd = CONSOLE_READ_RECENT;
- ret = cros_ec_cmd_xfer_status(ec->ec_dev,
- debug_info->read_msg);
+
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 1,
+ EC_CMD_CONSOLE_READ + ec->cmd_offset,
+ read_params, sizeof(*read_params),
+ ec_buffer, ec->ec_dev->max_response);
if (ret < 0)
break;
@@ -199,44 +201,29 @@ static ssize_t cros_ec_pdinfo_read(struct file *file,
char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
struct cros_ec_debugfs *debug_info = file->private_data;
struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
- struct {
- struct cros_ec_command msg;
- union {
- struct ec_response_usb_pd_control_v1 resp;
- struct ec_params_usb_pd_control params;
- };
- } __packed ec_buf;
- struct cros_ec_command *msg;
- struct ec_response_usb_pd_control_v1 *resp;
- struct ec_params_usb_pd_control *params;
+ struct ec_response_usb_pd_control_v1 resp = {0};
+ struct ec_params_usb_pd_control params = {0};
int i;
- msg = &ec_buf.msg;
- params = (struct ec_params_usb_pd_control *)msg->data;
- resp = (struct ec_response_usb_pd_control_v1 *)msg->data;
-
- msg->command = EC_CMD_USB_PD_CONTROL;
- msg->version = 1;
- msg->insize = sizeof(*resp);
- msg->outsize = sizeof(*params);
-
/*
* Read status from all PD ports until failure, typically caused
* by attempting to read status on a port that doesn't exist.
*/
for (i = 0; i < EC_USB_PD_MAX_PORTS; ++i) {
- params->port = i;
- params->role = 0;
- params->mux = 0;
- params->swap = 0;
-
- if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0)
+ params.port = i;
+ params.role = 0;
+ params.mux = 0;
+ params.swap = 0;
+
+ if (cros_ec_send_cmd_msg(ec_dev, 1, EC_CMD_USB_PD_CONTROL,
+ ¶ms, sizeof(params), &resp,
+ sizeof(resp)) < 0)
break;
p += scnprintf(p, sizeof(read_buf) + read_buf - p,
"p%d: %s en:%.2x role:%.2x pol:%.2x\n", i,
- resp->state, resp->enabled, resp->role,
- resp->polarity);
+ resp.state, resp.enabled, resp.role,
+ resp.polarity);
}
return simple_read_from_buffer(user_buf, count, ppos,
@@ -248,25 +235,17 @@ static ssize_t cros_ec_uptime_read(struct file *file, char __user *user_buf,
{
struct cros_ec_debugfs *debug_info = file->private_data;
struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
- struct {
- struct cros_ec_command cmd;
- struct ec_response_uptime_info resp;
- } __packed msg = {};
- struct ec_response_uptime_info *resp;
+ struct ec_response_uptime_info resp = {};
char read_buf[32];
int ret;
- resp = (struct ec_response_uptime_info *)&msg.resp;
-
- msg.cmd.command = EC_CMD_GET_UPTIME_INFO;
- msg.cmd.insize = sizeof(*resp);
-
- ret = cros_ec_cmd_xfer_status(ec_dev, &msg.cmd);
+ ret = cros_ec_send_cmd_msg(ec_dev, 0, EC_CMD_GET_UPTIME_INFO, NULL, 0,
+ &resp, sizeof(resp));
if (ret < 0)
return ret;
ret = scnprintf(read_buf, sizeof(read_buf), "%u\n",
- resp->time_since_ec_boot_ms);
+ resp.time_since_ec_boot_ms);
return simple_read_from_buffer(user_buf, count, ppos, read_buf, ret);
}
@@ -296,29 +275,16 @@ static const struct file_operations cros_ec_uptime_fops = {
static int ec_read_version_supported(struct cros_ec_dev *ec)
{
- struct ec_params_get_cmd_versions_v1 *params;
- struct ec_response_get_cmd_versions *response;
+ struct ec_params_get_cmd_versions_v1 params = {0};
+ struct ec_response_get_cmd_versions resp = {0};
int ret;
- struct cros_ec_command *msg;
-
- msg = kzalloc(sizeof(*msg) + max(sizeof(*params), sizeof(*response)),
- GFP_KERNEL);
- if (!msg)
- return 0;
-
- msg->command = EC_CMD_GET_CMD_VERSIONS + ec->cmd_offset;
- msg->outsize = sizeof(*params);
- msg->insize = sizeof(*response);
-
- params = (struct ec_params_get_cmd_versions_v1 *)msg->data;
- params->cmd = EC_CMD_CONSOLE_READ;
- response = (struct ec_response_get_cmd_versions *)msg->data;
-
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg) >= 0 &&
- response->version_mask & EC_VER_MASK(1);
-
- kfree(msg);
+ params.cmd = EC_CMD_CONSOLE_READ;
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 0,
+ EC_CMD_GET_CMD_VERSIONS + ec->cmd_offset,
+ ¶ms, sizeof(params),
+ &resp, sizeof(resp));
+ ret = ret >= 0 && resp.version_mask & EC_VER_MASK(1);
return ret;
}
@@ -343,17 +309,11 @@ static int cros_ec_create_console_log(struct cros_ec_debugfs *debug_info)
read_params_size = sizeof(struct ec_params_console_read_v1);
read_response_size = ec->ec_dev->max_response;
- debug_info->read_msg = devm_kzalloc(ec->dev,
- sizeof(*debug_info->read_msg) +
- max(read_params_size, read_response_size), GFP_KERNEL);
- if (!debug_info->read_msg)
+ debug_info->ec_buf = devm_kzalloc(ec->dev,
+ max(read_params_size, read_response_size), GFP_KERNEL);
+ if (!debug_info->ec_buf)
return -ENOMEM;
- debug_info->read_msg->version = 1;
- debug_info->read_msg->command = EC_CMD_CONSOLE_READ + ec->cmd_offset;
- debug_info->read_msg->outsize = read_params_size;
- debug_info->read_msg->insize = read_response_size;
-
debug_info->log_buffer.buf = buf;
debug_info->log_buffer.head = 0;
debug_info->log_buffer.tail = 0;
@@ -383,20 +343,17 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
{
struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
int ret;
- struct cros_ec_command *msg;
+ void *buf;
int insize;
insize = ec_dev->max_response;
- msg = devm_kzalloc(debug_info->ec->dev,
- sizeof(*msg) + insize, GFP_KERNEL);
- if (!msg)
+ buf = devm_kzalloc(debug_info->ec->dev, insize, GFP_KERNEL);
+ if (!buf)
return -ENOMEM;
- msg->command = EC_CMD_GET_PANIC_INFO;
- msg->insize = insize;
-
- ret = cros_ec_cmd_xfer_status(ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec_dev, 0, EC_CMD_GET_PANIC_INFO,
+ NULL, 0, buf, insize);
if (ret < 0) {
ret = 0;
goto free;
@@ -406,7 +363,7 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
if (ret == 0)
goto free;
- debug_info->panicinfo_blob.data = msg->data;
+ debug_info->panicinfo_blob.data = buf;
debug_info->panicinfo_blob.size = ret;
debugfs_create_blob("panicinfo", S_IFREG | 0444, debug_info->dir,
@@ -415,7 +372,7 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
return 0;
free:
- devm_kfree(debug_info->ec->dev, msg);
+ devm_kfree(debug_info->ec->dev, buf);
return ret;
}
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 07/17] platform/chrome: sysfs: Use send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
` (6 preceding siblings ...)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra, Guenter Roeck
Replace cros_ec_cmd_xfer_status() calls to the new function
cros_ec_send_cmd_msg() which is more readable and does the message setup code.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/platform/chrome/cros_ec_sysfs.c | 106 ++++++++++--------------
1 file changed, 42 insertions(+), 64 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index 74d36b8d4f46c7..87d5bfa1fcfa61 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -52,20 +52,13 @@ static ssize_t reboot_store(struct device *dev,
{"hibernate", EC_REBOOT_HIBERNATE, 0},
{"at-shutdown", -1, EC_REBOOT_FLAG_ON_AP_SHUTDOWN},
};
- struct cros_ec_command *msg;
- struct ec_params_reboot_ec *param;
+ struct ec_params_reboot_ec param = {0};
int got_cmd = 0, offset = 0;
int i;
int ret;
struct cros_ec_dev *ec = to_cros_ec_dev(dev);
- msg = kmalloc(sizeof(*msg) + sizeof(*param), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- param = (struct ec_params_reboot_ec *)msg->data;
-
- param->flags = 0;
+ param.flags = 0;
while (1) {
/* Find word to start scanning */
while (buf[offset] && isspace(buf[offset]))
@@ -77,9 +70,9 @@ static ssize_t reboot_store(struct device *dev,
if (!strncasecmp(words[i].str, buf+offset,
strlen(words[i].str))) {
if (words[i].flags) {
- param->flags |= words[i].flags;
+ param.flags |= words[i].flags;
} else {
- param->cmd = words[i].cmd;
+ param.cmd = words[i].cmd;
got_cmd = 1;
}
break;
@@ -96,15 +89,12 @@ static ssize_t reboot_store(struct device *dev,
goto exit;
}
- msg->version = 0;
- msg->command = EC_CMD_REBOOT_EC + ec->cmd_offset;
- msg->outsize = sizeof(*param);
- msg->insize = 0;
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 0,
+ EC_CMD_REBOOT_EC + ec->cmd_offset,
+ ¶m, sizeof(param), NULL, 0);
if (ret < 0)
count = ret;
exit:
- kfree(msg);
return count;
}
@@ -116,25 +106,24 @@ static ssize_t version_show(struct device *dev,
struct ec_response_get_chip_info *r_chip;
struct ec_response_board_version *r_board;
struct cros_ec_command *msg;
+ void *ec_buf;
int ret;
int count = 0;
struct cros_ec_dev *ec = to_cros_ec_dev(dev);
- msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
- if (!msg)
+ ec_buf = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
+ if (!ec_buf)
return -ENOMEM;
/* Get versions. RW may change. */
- msg->version = 0;
- msg->command = EC_CMD_GET_VERSION + ec->cmd_offset;
- msg->insize = sizeof(*r_ver);
- msg->outsize = 0;
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 0,
+ EC_CMD_GET_VERSION + ec->cmd_offset,
+ NULL, 0, ec_buf, sizeof(*r_ver));
if (ret < 0) {
count = ret;
goto exit;
}
- r_ver = (struct ec_response_get_version *)msg->data;
+ r_ver = (struct ec_response_get_version *)ec_buf;
/* Strings should be null-terminated, but let's be sure. */
r_ver->version_string_ro[sizeof(r_ver->version_string_ro) - 1] = '\0';
r_ver->version_string_rw[sizeof(r_ver->version_string_rw) - 1] = '\0';
@@ -146,8 +135,10 @@ static ssize_t version_show(struct device *dev,
"Firmware copy: %s\n",
(r_ver->current_image < ARRAY_SIZE(image_names) ?
image_names[r_ver->current_image] : "?"));
+ memset(ec_buf, 0, sizeof(*msg) + EC_HOST_PARAM_SIZE);
/* Get build info. */
+ msg = (struct cros_ec_command *)ec_buf;
msg->command = EC_CMD_GET_BUILD_INFO + ec->cmd_offset;
msg->insize = EC_HOST_PARAM_SIZE;
ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
@@ -206,40 +197,29 @@ static ssize_t version_show(struct device *dev,
}
exit:
- kfree(msg);
+ kfree(ec_buf);
return count;
}
static ssize_t flashinfo_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- struct ec_response_flash_info *resp;
- struct cros_ec_command *msg;
+ struct ec_response_flash_info resp = {0};
int ret;
struct cros_ec_dev *ec = to_cros_ec_dev(dev);
- msg = kmalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- /* The flash info shouldn't ever change, but ask each time anyway. */
- msg->version = 0;
- msg->command = EC_CMD_FLASH_INFO + ec->cmd_offset;
- msg->insize = sizeof(*resp);
- msg->outsize = 0;
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 0,
+ EC_CMD_FLASH_INFO + ec->cmd_offset,
+ NULL, 0, &resp, sizeof(resp));
if (ret < 0)
goto exit;
- resp = (struct ec_response_flash_info *)msg->data;
-
ret = scnprintf(buf, PAGE_SIZE,
"FlashSize %d\nWriteSize %d\n"
"EraseSize %d\nProtectSize %d\n",
- resp->flash_size, resp->write_block_size,
- resp->erase_block_size, resp->protect_block_size);
+ resp.flash_size, resp.write_block_size,
+ resp.erase_block_size, resp.protect_block_size);
exit:
- kfree(msg);
return ret;
}
@@ -250,29 +230,27 @@ static ssize_t kb_wake_angle_show(struct device *dev,
struct cros_ec_dev *ec = to_cros_ec_dev(dev);
struct ec_response_motion_sense *resp;
struct ec_params_motion_sense *param;
- struct cros_ec_command *msg;
+ void *ec_buf;
int ret;
- msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
- if (!msg)
+ ec_buf = kmalloc(EC_HOST_PARAM_SIZE, GFP_KERNEL);
+ if (!ec_buf)
return -ENOMEM;
- param = (struct ec_params_motion_sense *)msg->data;
- msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
- msg->version = 2;
+ param = (struct ec_params_motion_sense *)ec_buf;
+ resp = (struct ec_response_motion_sense *)ec_buf;
param->cmd = MOTIONSENSE_CMD_KB_WAKE_ANGLE;
param->kb_wake_angle.data = EC_MOTION_SENSE_NO_VALUE;
- msg->outsize = sizeof(*param);
- msg->insize = sizeof(*resp);
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 2,
+ EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset,
+ param, sizeof(*param), resp, sizeof(*resp));
if (ret < 0)
goto exit;
- resp = (struct ec_response_motion_sense *)msg->data;
ret = scnprintf(buf, PAGE_SIZE, "%d\n", resp->kb_wake_angle.ret);
exit:
- kfree(msg);
+ kfree(ec_buf);
return ret;
}
@@ -282,7 +260,8 @@ static ssize_t kb_wake_angle_store(struct device *dev,
{
struct cros_ec_dev *ec = to_cros_ec_dev(dev);
struct ec_params_motion_sense *param;
- struct cros_ec_command *msg;
+ struct ec_response_motion_sense *resp;
+ void *ec_buf;
u16 angle;
int ret;
@@ -290,20 +269,19 @@ static ssize_t kb_wake_angle_store(struct device *dev,
if (ret)
return ret;
- msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
- if (!msg)
+ ec_buf = kmalloc(EC_HOST_PARAM_SIZE, GFP_KERNEL);
+ if (!ec_buf)
return -ENOMEM;
- param = (struct ec_params_motion_sense *)msg->data;
- msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
- msg->version = 2;
+ param = (struct ec_params_motion_sense *)ec_buf;
+ resp = (struct ec_response_motion_sense *)ec_buf;
param->cmd = MOTIONSENSE_CMD_KB_WAKE_ANGLE;
param->kb_wake_angle.data = angle;
- msg->outsize = sizeof(*param);
- msg->insize = sizeof(struct ec_response_motion_sense);
- ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
- kfree(msg);
+ ret = cros_ec_send_cmd_msg(ec->ec_dev, 2,
+ EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset,
+ param, sizeof(*param), resp, sizeof(*resp));
+ kfree(ec_buf);
if (ret < 0)
return ret;
return count;
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 08/17] extcon: cros_ec: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
` (7 preceding siblings ...)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, MyungJoo Ham, Chanwoo Choi, Benson Leung,
Enric Balletbo i Serra, Guenter Roeck
Replace cros_ec_pd_command() with cros_ec_send_cmd_msg() which does the
same thing, but is defined in a common location in platform/chrome and
exposed for other modules to use.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/extcon/extcon-usbc-cros-ec.c | 62 ++++------------------------
1 file changed, 9 insertions(+), 53 deletions(-)
diff --git a/drivers/extcon/extcon-usbc-cros-ec.c b/drivers/extcon/extcon-usbc-cros-ec.c
index 5290cc2d19d953..e42d929d3a76da 100644
--- a/drivers/extcon/extcon-usbc-cros-ec.c
+++ b/drivers/extcon/extcon-usbc-cros-ec.c
@@ -45,49 +45,6 @@ enum usb_data_roles {
DR_DEVICE,
};
-/**
- * cros_ec_pd_command() - Send a command to the EC.
- * @info: pointer to struct cros_ec_extcon_info
- * @command: EC command
- * @version: EC command version
- * @outdata: EC command output data
- * @outsize: Size of outdata
- * @indata: EC command input data
- * @insize: Size of indata
- *
- * Return: 0 on success, <0 on failure.
- */
-static int cros_ec_pd_command(struct cros_ec_extcon_info *info,
- unsigned int command,
- unsigned int version,
- void *outdata,
- unsigned int outsize,
- void *indata,
- unsigned int insize)
-{
- struct cros_ec_command *msg;
- int ret;
-
- msg = kzalloc(sizeof(*msg) + max(outsize, insize), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- msg->version = version;
- msg->command = command;
- msg->outsize = outsize;
- msg->insize = insize;
-
- if (outsize)
- memcpy(msg->data, outdata, outsize);
-
- ret = cros_ec_cmd_xfer_status(info->ec, msg);
- if (ret >= 0 && insize)
- memcpy(indata, msg->data, insize);
-
- kfree(msg);
- return ret;
-}
-
/**
* cros_ec_usb_get_power_type() - Get power type info about PD device attached
* to given port.
@@ -102,8 +59,8 @@ static int cros_ec_usb_get_power_type(struct cros_ec_extcon_info *info)
int ret;
req.port = info->port_id;
- ret = cros_ec_pd_command(info, EC_CMD_USB_PD_POWER_INFO, 0,
- &req, sizeof(req), &resp, sizeof(resp));
+ ret = cros_ec_send_cmd_msg(info->ec, 0, EC_CMD_USB_PD_POWER_INFO,
+ &req, sizeof(req), &resp, sizeof(resp));
if (ret < 0)
return ret;
@@ -123,9 +80,8 @@ static int cros_ec_usb_get_pd_mux_state(struct cros_ec_extcon_info *info)
int ret;
req.port = info->port_id;
- ret = cros_ec_pd_command(info, EC_CMD_USB_PD_MUX_INFO, 0,
- &req, sizeof(req),
- &resp, sizeof(resp));
+ ret = cros_ec_send_cmd_msg(info->ec, 0, EC_CMD_USB_PD_MUX_INFO,
+ &req, sizeof(req), &resp, sizeof(resp));
if (ret < 0)
return ret;
@@ -152,9 +108,9 @@ static int cros_ec_usb_get_role(struct cros_ec_extcon_info *info,
pd_control.role = USB_PD_CTRL_ROLE_NO_CHANGE;
pd_control.mux = USB_PD_CTRL_MUX_NO_CHANGE;
pd_control.swap = USB_PD_CTRL_SWAP_NONE;
- ret = cros_ec_pd_command(info, EC_CMD_USB_PD_CONTROL, 1,
- &pd_control, sizeof(pd_control),
- &resp, sizeof(resp));
+ ret = cros_ec_send_cmd_msg(info->ec, 1, EC_CMD_USB_PD_CONTROL,
+ &pd_control, sizeof(pd_control),
+ &resp, sizeof(resp));
if (ret < 0)
return ret;
@@ -177,8 +133,8 @@ static int cros_ec_pd_get_num_ports(struct cros_ec_extcon_info *info)
struct ec_response_usb_pd_ports resp;
int ret;
- ret = cros_ec_pd_command(info, EC_CMD_USB_PD_PORTS,
- 0, NULL, 0, &resp, sizeof(resp));
+ ret = cros_ec_send_cmd_msg(info->ec, 0, EC_CMD_USB_PD_PORTS, NULL, 0,
+ &resp, sizeof(resp));
if (ret < 0)
return ret;
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 09/17] hid: google-hammer: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
` (8 preceding siblings ...)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Jiri Kosina, Benjamin Tissoires,
open list:HID CORE LAYER
Replace cros_ec_cmd_xfer_status() with cros_ec_send_cmd_msg() which does
the message buffer setup and cleanup.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/hid/hid-google-hammer.c | 23 +++++------------------
1 file changed, 5 insertions(+), 18 deletions(-)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 2aa4ed157aec87..428762a6a4993f 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -53,38 +53,25 @@ static bool cbas_parse_base_state(const void *data)
static int cbas_ec_query_base(struct cros_ec_device *ec_dev, bool get_state,
bool *state)
{
- struct ec_params_mkbp_info *params;
- struct cros_ec_command *msg;
+ struct ec_params_mkbp_info params = {0};
int ret;
- msg = kzalloc(sizeof(*msg) + max(sizeof(u32), sizeof(*params)),
- GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- msg->command = EC_CMD_MKBP_INFO;
- msg->version = 1;
- msg->outsize = sizeof(*params);
- msg->insize = sizeof(u32);
- params = (struct ec_params_mkbp_info *)msg->data;
- params->info_type = get_state ?
+ params.info_type = get_state ?
EC_MKBP_INFO_CURRENT : EC_MKBP_INFO_SUPPORTED;
- params->event_type = EC_MKBP_EVENT_SWITCH;
+ params.event_type = EC_MKBP_EVENT_SWITCH;
- ret = cros_ec_cmd_xfer_status(ec_dev, msg);
+ ret = cros_ec_send_cmd_msg(ec_dev, 1, EC_CMD_MKBP_INFO,
+ ¶ms, sizeof(params), state, sizeof(u32));
if (ret >= 0) {
if (ret != sizeof(u32)) {
dev_warn(ec_dev->dev, "wrong result size: %d != %zu\n",
ret, sizeof(u32));
ret = -EPROTO;
} else {
- *state = cbas_parse_base_state(msg->data);
ret = 0;
}
}
- kfree(msg);
-
return ret;
}
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 10/17] iio: cros_ec: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
` (9 preceding siblings ...)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
2020-02-02 9:43 ` Jonathan Cameron
-1 siblings, 1 reply; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Benson Leung,
Enric Balletbo i Serra, Guenter Roeck, Gwendal Grignou,
Fabien Lahoudere, Nick Vaccaro,
open list:IIO SUBSYSTEM AND DRIVERS
Replace cros_ec_cmd_xfer_status() with cros_ec_send_cmd_msg()
which does the message buffer setup and cleanup.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
.../cros_ec_sensors/cros_ec_sensors_core.c | 43 +++++++++----------
1 file changed, 21 insertions(+), 22 deletions(-)
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 81a7f692de2f37..f92032e97a84d7 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
@@ -31,24 +31,16 @@ 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 = {
- .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);
+ struct ec_params_get_cmd_versions params = {0};
+ struct ec_response_get_cmd_versions resp = {0};
+
+ params.cmd = cmd;
+ ret = cros_ec_send_cmd_msg(ec_dev, 0,
+ EC_CMD_GET_CMD_VERSIONS + cmd_offset,
+ ¶ms, sizeof(params),
+ &resp, sizeof(resp));
if (ret >= 0)
- *mask = buf.resp.version_mask;
+ *mask = resp.version_mask;
return ret;
}
@@ -164,15 +156,22 @@ int cros_ec_motion_send_host_cmd(struct cros_ec_sensors_core_state *state,
u16 opt_length)
{
int ret;
+ struct cros_ec_command *msg = state->msg;
if (opt_length)
- state->msg->insize = min(opt_length, state->ec->max_response);
+ msg->insize = min(opt_length, state->ec->max_response);
else
- state->msg->insize = state->ec->max_response;
+ msg->insize = state->ec->max_response;
- memcpy(state->msg->data, &state->param, sizeof(state->param));
-
- ret = cros_ec_cmd_xfer_status(state->ec, state->msg);
+ /*
+ * In order to not disrupt the usage of struct cros_ec_command *msg,
+ * which is defined higher up in the call stack, we pass in its
+ * members to cros_ec_send_cmd_msg, instead of removing it at all
+ * calling locations.
+ */
+ ret = cros_ec_send_cmd_msg(state->ec, msg->version, msg->command,
+ &state->param, sizeof(state->param),
+ msg->data, msg->insize);
if (ret < 0)
return ret;
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 10/17] iio: cros_ec: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` [PATCH 10/17] iio: cros_ec: " Prashant Malani
@ 2020-02-02 9:43 ` Jonathan Cameron
2020-02-03 18:31 ` Prashant Malani
0 siblings, 1 reply; 38+ messages in thread
From: Jonathan Cameron @ 2020-02-02 9:43 UTC (permalink / raw)
To: Prashant Malani
Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Benson Leung, Enric Balletbo i Serra,
Guenter Roeck, Gwendal Grignou, Fabien Lahoudere, Nick Vaccaro,
open list:IIO SUBSYSTEM AND DRIVERS
On Thu, 30 Jan 2020 12:30:54 -0800
Prashant Malani <pmalani@chromium.org> wrote:
> Replace cros_ec_cmd_xfer_status() with cros_ec_send_cmd_msg()
> which does the message buffer setup and cleanup.
>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>
In a series like this, make sure that patch 1 with the actual code being
pulled out is sent to everyone. Looking at what we have here, this
doesn't seem to fit well at all for one case, and I'm can't say the
other case shows much advantage either.
Jonathan
> ---
> .../cros_ec_sensors/cros_ec_sensors_core.c | 43 +++++++++----------
> 1 file changed, 21 insertions(+), 22 deletions(-)
>
> 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 81a7f692de2f37..f92032e97a84d7 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
> @@ -31,24 +31,16 @@ 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 = {
> - .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);
> + struct ec_params_get_cmd_versions params = {0};
> + struct ec_response_get_cmd_versions resp = {0};
> +
> + params.cmd = cmd;
Use c99 element setting to set this directly rather than zeroing
explicitly then setting the element.
Something like.
struct ec_params_get_cmde_versions params = {
.cmd = cmd;
};
> + ret = cros_ec_send_cmd_msg(ec_dev, 0,
> + EC_CMD_GET_CMD_VERSIONS + cmd_offset,
> + ¶ms, sizeof(params),
> + &resp, sizeof(resp));
> if (ret >= 0)
> - *mask = buf.resp.version_mask;
> + *mask = resp.version_mask;
> return ret;
> }
>
> @@ -164,15 +156,22 @@ int cros_ec_motion_send_host_cmd(struct cros_ec_sensors_core_state *state,
> u16 opt_length)
> {
> int ret;
> + struct cros_ec_command *msg = state->msg;
With this change the code becomes less readable and needs a comment to explain
why it is doing something odd. Either you need to figure out how to
make this fit properly such that the comment is not needed, or leave
the code alone.
>
> if (opt_length)
> - state->msg->insize = min(opt_length, state->ec->max_response);
> + msg->insize = min(opt_length, state->ec->max_response);
> else
> - state->msg->insize = state->ec->max_response;
> + msg->insize = state->ec->max_response;
>
> - memcpy(state->msg->data, &state->param, sizeof(state->param));
> -
> - ret = cros_ec_cmd_xfer_status(state->ec, state->msg);
> + /*
> + * In order to not disrupt the usage of struct cros_ec_command *msg,
> + * which is defined higher up in the call stack, we pass in its
> + * members to cros_ec_send_cmd_msg, instead of removing it at all
> + * calling locations.
> + */
> + ret = cros_ec_send_cmd_msg(state->ec, msg->version, msg->command,
> + &state->param, sizeof(state->param),
> + msg->data, msg->insize);
> if (ret < 0)
> return ret;
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 10/17] iio: cros_ec: Use cros_ec_send_cmd_msg()
2020-02-02 9:43 ` Jonathan Cameron
@ 2020-02-03 18:31 ` Prashant Malani
0 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-02-03 18:31 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Linux Kernel Mailing List, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Benson Leung, Enric Balletbo i Serra,
Guenter Roeck, Gwendal Grignou, Fabien Lahoudere, Nick Vaccaro,
open list:IIO SUBSYSTEM AND DRIVERS
Hi Jonathan, Thanks for the comments. Please see responses inline.
On Sun, Feb 2, 2020 at 1:43 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 30 Jan 2020 12:30:54 -0800
> Prashant Malani <pmalani@chromium.org> wrote:
>
> > Replace cros_ec_cmd_xfer_status() with cros_ec_send_cmd_msg()
> > which does the message buffer setup and cleanup.
> >
> > Signed-off-by: Prashant Malani <pmalani@chromium.org>
>
> In a series like this, make sure that patch 1 with the actual code being
> pulled out is sent to everyone. Looking at what we have here, this
> doesn't seem to fit well at all for one case, and I'm can't say the
> other case shows much advantage either.
Sorry about that. I'll be sure to add maintainers to Patch 1 in the
next versions.
I tried to follow : https://lwn.net/Articles/585782/ , but I think the
script might not be suited to this use case.
>
> Jonathan
>
> > ---
> > .../cros_ec_sensors/cros_ec_sensors_core.c | 43 +++++++++----------
> > 1 file changed, 21 insertions(+), 22 deletions(-)
> >
> > 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 81a7f692de2f37..f92032e97a84d7 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
> > @@ -31,24 +31,16 @@ 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 = {
> > - .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);
> > + struct ec_params_get_cmd_versions params = {0};
> > + struct ec_response_get_cmd_versions resp = {0};
> > +
> > + params.cmd = cmd;
> Use c99 element setting to set this directly rather than zeroing
> explicitly then setting the element.
>
> Something like.
>
> struct ec_params_get_cmde_versions params = {
> .cmd = cmd;
> };
Noted.
>
> > + ret = cros_ec_send_cmd_msg(ec_dev, 0,
> > + EC_CMD_GET_CMD_VERSIONS + cmd_offset,
> > + ¶ms, sizeof(params),
> > + &resp, sizeof(resp));
> > if (ret >= 0)
> > - *mask = buf.resp.version_mask;
> > + *mask = resp.version_mask;
> > return ret;
> > }
> >
> > @@ -164,15 +156,22 @@ int cros_ec_motion_send_host_cmd(struct cros_ec_sensors_core_state *state,
> > u16 opt_length)
> > {
> > int ret;
> > + struct cros_ec_command *msg = state->msg;
>
> With this change the code becomes less readable and needs a comment to explain
> why it is doing something odd. Either you need to figure out how to
> make this fit properly such that the comment is not needed, or leave
> the code alone.
Noted. Perhaps we can use cros_ec_cmd_xfer() instead of
cros_ec_cmd_xfer_status(). That I think will
keep readability the same and remove the need for comments.
>
> >
> > if (opt_length)
> > - state->msg->insize = min(opt_length, state->ec->max_response);
> > + msg->insize = min(opt_length, state->ec->max_response);
> > else
> > - state->msg->insize = state->ec->max_response;
> > + msg->insize = state->ec->max_response;
> >
> > - memcpy(state->msg->data, &state->param, sizeof(state->param));
> > -
> > - ret = cros_ec_cmd_xfer_status(state->ec, state->msg);
> > + /*
> > + * In order to not disrupt the usage of struct cros_ec_command *msg,
> > + * which is defined higher up in the call stack, we pass in its
> > + * members to cros_ec_send_cmd_msg, instead of removing it at all
> > + * calling locations.
> > + */
> > + ret = cros_ec_send_cmd_msg(state->ec, msg->version, msg->command,
> > + &state->param, sizeof(state->param),
> > + msg->data, msg->insize);
> > if (ret < 0)
> > return ret;
> >
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 11/17] ASoC: cros_ec_codec: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Cheng-Yi Chiang, Enric Balletbo i Serra,
Guenter Roeck, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Benson Leung,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...
Replace send_ec_host_command() with cros_ec_send_cmd_msg() which does
the same thing, but is defined in a common location in platform/chrome
and exposed for other modules to use. This allows us to remove
send_ec_host_command() entirely.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
sound/soc/codecs/cros_ec_codec.c | 71 ++++++++++----------------------
1 file changed, 21 insertions(+), 50 deletions(-)
diff --git a/sound/soc/codecs/cros_ec_codec.c b/sound/soc/codecs/cros_ec_codec.c
index 6a24f570c5e86f..49adb07d766963 100644
--- a/sound/soc/codecs/cros_ec_codec.c
+++ b/sound/soc/codecs/cros_ec_codec.c
@@ -69,38 +69,6 @@ static int ec_codec_capable(struct cros_ec_codec_priv *priv, uint8_t cap)
return priv->ec_capabilities & BIT(cap);
}
-static int send_ec_host_command(struct cros_ec_device *ec_dev, uint32_t cmd,
- uint8_t *out, size_t outsize,
- uint8_t *in, size_t insize)
-{
- int ret;
- struct cros_ec_command *msg;
-
- msg = kmalloc(sizeof(*msg) + max(outsize, insize), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- msg->version = 0;
- msg->command = cmd;
- msg->outsize = outsize;
- msg->insize = insize;
-
- if (outsize)
- memcpy(msg->data, out, outsize);
-
- ret = cros_ec_cmd_xfer_status(ec_dev, msg);
- if (ret < 0)
- goto error;
-
- if (insize)
- memcpy(in, msg->data, insize);
-
- ret = 0;
-error:
- kfree(msg);
- return ret;
-}
-
static int calculate_sha256(struct cros_ec_codec_priv *priv,
uint8_t *buf, uint32_t size, uint8_t *digest)
{
@@ -149,7 +117,7 @@ static int dmic_get_gain(struct snd_kcontrol *kcontrol,
p.cmd = EC_CODEC_DMIC_GET_GAIN_IDX;
p.get_gain_idx_param.channel = EC_CODEC_DMIC_CHANNEL_0;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret < 0)
@@ -158,7 +126,7 @@ static int dmic_get_gain(struct snd_kcontrol *kcontrol,
p.cmd = EC_CODEC_DMIC_GET_GAIN_IDX;
p.get_gain_idx_param.channel = EC_CODEC_DMIC_CHANNEL_1;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret < 0)
@@ -191,7 +159,7 @@ static int dmic_put_gain(struct snd_kcontrol *kcontrol,
p.cmd = EC_CODEC_DMIC_SET_GAIN_IDX;
p.set_gain_idx_param.channel = EC_CODEC_DMIC_CHANNEL_0;
p.set_gain_idx_param.gain = left;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret < 0)
return ret;
@@ -199,7 +167,7 @@ static int dmic_put_gain(struct snd_kcontrol *kcontrol,
p.cmd = EC_CODEC_DMIC_SET_GAIN_IDX;
p.set_gain_idx_param.channel = EC_CODEC_DMIC_CHANNEL_1;
p.set_gain_idx_param.gain = right;
- return send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ return cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p), NULL, 0);
}
@@ -231,7 +199,7 @@ static int dmic_probe(struct snd_soc_component *component)
p.cmd = EC_CODEC_DMIC_GET_MAX_GAIN;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret < 0) {
@@ -279,7 +247,7 @@ static int i2s_rx_hw_params(struct snd_pcm_substream *substream,
p.cmd = EC_CODEC_I2S_RX_SET_SAMPLE_DEPTH;
p.set_sample_depth_param.depth = depth;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_I2S_RX,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_I2S_RX,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret < 0)
return ret;
@@ -289,7 +257,7 @@ static int i2s_rx_hw_params(struct snd_pcm_substream *substream,
p.cmd = EC_CODEC_I2S_RX_SET_BCLK;
p.set_bclk_param.bclk = snd_soc_params_to_bclk(params);
- return send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_I2S_RX,
+ return cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_I2S_RX,
(uint8_t *)&p, sizeof(p), NULL, 0);
}
@@ -333,7 +301,7 @@ static int i2s_rx_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
p.cmd = EC_CODEC_I2S_RX_SET_DAIFMT;
p.set_daifmt_param.daifmt = daifmt;
- return send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_I2S_RX,
+ return cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_I2S_RX,
(uint8_t *)&p, sizeof(p), NULL, 0);
}
@@ -364,7 +332,7 @@ static int i2s_rx_event(struct snd_soc_dapm_widget *w,
return 0;
}
- return send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_I2S_RX,
+ return cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_I2S_RX,
(uint8_t *)&p, sizeof(p), NULL, 0);
}
@@ -415,7 +383,7 @@ static void *wov_map_shm(struct cros_ec_codec_priv *priv,
p.cmd = EC_CODEC_GET_SHM_ADDR;
p.get_shm_addr_param.shm_id = shm_id;
- if (send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC,
+ if (cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r)) < 0) {
dev_err(priv->dev, "failed to EC_CODEC_GET_SHM_ADDR\n");
@@ -453,7 +421,7 @@ static void *wov_map_shm(struct cros_ec_codec_priv *priv,
p.set_shm_addr_param.phys_addr = priv->ap_shm_last_alloc;
p.set_shm_addr_param.len = req;
p.set_shm_addr_param.shm_id = shm_id;
- if (send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC,
+ if (cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC,
(uint8_t *)&p, sizeof(p),
NULL, 0) < 0) {
dev_err(priv->dev, "failed to EC_CODEC_SET_SHM_ADDR\n");
@@ -571,7 +539,7 @@ static int wov_read_audio_shm(struct cros_ec_codec_priv *priv)
int ret;
p.cmd = EC_CODEC_WOV_READ_AUDIO_SHM;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret) {
@@ -596,7 +564,8 @@ static int wov_read_audio(struct cros_ec_codec_priv *priv)
while (remain >= 0) {
p.cmd = EC_CODEC_WOV_READ_AUDIO;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0,
+ EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret) {
@@ -669,7 +638,8 @@ static int wov_enable_put(struct snd_kcontrol *kcontrol,
else
p.cmd = EC_CODEC_WOV_DISABLE;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0,
+ EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret) {
dev_err(priv->dev, "failed to %s wov\n",
@@ -716,7 +686,7 @@ static int wov_set_lang_shm(struct cros_ec_codec_priv *priv,
p.cmd = EC_CODEC_WOV_SET_LANG_SHM;
memcpy(pp->hash, digest, SHA256_DIGEST_SIZE);
pp->total_len = size;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret) {
dev_err(priv->dev, "failed to EC_CODEC_WOV_SET_LANG_SHM\n");
@@ -743,7 +713,8 @@ static int wov_set_lang(struct cros_ec_codec_priv *priv,
pp->offset = i;
memcpy(pp->buf, buf + i, req);
pp->len = req;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0,
+ EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret) {
dev_err(priv->dev, "failed to EC_CODEC_WOV_SET_LANG\n");
@@ -782,7 +753,7 @@ static int wov_hotword_model_put(struct snd_kcontrol *kcontrol,
goto leave;
p.cmd = EC_CODEC_WOV_GET_LANG;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret)
@@ -1020,7 +991,7 @@ static int cros_ec_codec_platform_probe(struct platform_device *pdev)
atomic_set(&priv->dmic_probed, 0);
p.cmd = EC_CODEC_GET_CAPABILITIES;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret) {
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [alsa-devel] [PATCH 11/17] ASoC: cros_ec_codec: Use cros_ec_send_cmd_msg()
@ 2020-01-30 20:30 ` Prashant Malani
0 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Takashi Iwai, Liam Girdwood, Guenter Roeck, Mark Brown,
Prashant Malani, Enric Balletbo i Serra, Benson Leung,
Cheng-Yi Chiang
Replace send_ec_host_command() with cros_ec_send_cmd_msg() which does
the same thing, but is defined in a common location in platform/chrome
and exposed for other modules to use. This allows us to remove
send_ec_host_command() entirely.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
sound/soc/codecs/cros_ec_codec.c | 71 ++++++++++----------------------
1 file changed, 21 insertions(+), 50 deletions(-)
diff --git a/sound/soc/codecs/cros_ec_codec.c b/sound/soc/codecs/cros_ec_codec.c
index 6a24f570c5e86f..49adb07d766963 100644
--- a/sound/soc/codecs/cros_ec_codec.c
+++ b/sound/soc/codecs/cros_ec_codec.c
@@ -69,38 +69,6 @@ static int ec_codec_capable(struct cros_ec_codec_priv *priv, uint8_t cap)
return priv->ec_capabilities & BIT(cap);
}
-static int send_ec_host_command(struct cros_ec_device *ec_dev, uint32_t cmd,
- uint8_t *out, size_t outsize,
- uint8_t *in, size_t insize)
-{
- int ret;
- struct cros_ec_command *msg;
-
- msg = kmalloc(sizeof(*msg) + max(outsize, insize), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- msg->version = 0;
- msg->command = cmd;
- msg->outsize = outsize;
- msg->insize = insize;
-
- if (outsize)
- memcpy(msg->data, out, outsize);
-
- ret = cros_ec_cmd_xfer_status(ec_dev, msg);
- if (ret < 0)
- goto error;
-
- if (insize)
- memcpy(in, msg->data, insize);
-
- ret = 0;
-error:
- kfree(msg);
- return ret;
-}
-
static int calculate_sha256(struct cros_ec_codec_priv *priv,
uint8_t *buf, uint32_t size, uint8_t *digest)
{
@@ -149,7 +117,7 @@ static int dmic_get_gain(struct snd_kcontrol *kcontrol,
p.cmd = EC_CODEC_DMIC_GET_GAIN_IDX;
p.get_gain_idx_param.channel = EC_CODEC_DMIC_CHANNEL_0;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret < 0)
@@ -158,7 +126,7 @@ static int dmic_get_gain(struct snd_kcontrol *kcontrol,
p.cmd = EC_CODEC_DMIC_GET_GAIN_IDX;
p.get_gain_idx_param.channel = EC_CODEC_DMIC_CHANNEL_1;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret < 0)
@@ -191,7 +159,7 @@ static int dmic_put_gain(struct snd_kcontrol *kcontrol,
p.cmd = EC_CODEC_DMIC_SET_GAIN_IDX;
p.set_gain_idx_param.channel = EC_CODEC_DMIC_CHANNEL_0;
p.set_gain_idx_param.gain = left;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret < 0)
return ret;
@@ -199,7 +167,7 @@ static int dmic_put_gain(struct snd_kcontrol *kcontrol,
p.cmd = EC_CODEC_DMIC_SET_GAIN_IDX;
p.set_gain_idx_param.channel = EC_CODEC_DMIC_CHANNEL_1;
p.set_gain_idx_param.gain = right;
- return send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ return cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p), NULL, 0);
}
@@ -231,7 +199,7 @@ static int dmic_probe(struct snd_soc_component *component)
p.cmd = EC_CODEC_DMIC_GET_MAX_GAIN;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_DMIC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_DMIC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret < 0) {
@@ -279,7 +247,7 @@ static int i2s_rx_hw_params(struct snd_pcm_substream *substream,
p.cmd = EC_CODEC_I2S_RX_SET_SAMPLE_DEPTH;
p.set_sample_depth_param.depth = depth;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_I2S_RX,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_I2S_RX,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret < 0)
return ret;
@@ -289,7 +257,7 @@ static int i2s_rx_hw_params(struct snd_pcm_substream *substream,
p.cmd = EC_CODEC_I2S_RX_SET_BCLK;
p.set_bclk_param.bclk = snd_soc_params_to_bclk(params);
- return send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_I2S_RX,
+ return cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_I2S_RX,
(uint8_t *)&p, sizeof(p), NULL, 0);
}
@@ -333,7 +301,7 @@ static int i2s_rx_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
p.cmd = EC_CODEC_I2S_RX_SET_DAIFMT;
p.set_daifmt_param.daifmt = daifmt;
- return send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_I2S_RX,
+ return cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_I2S_RX,
(uint8_t *)&p, sizeof(p), NULL, 0);
}
@@ -364,7 +332,7 @@ static int i2s_rx_event(struct snd_soc_dapm_widget *w,
return 0;
}
- return send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_I2S_RX,
+ return cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_I2S_RX,
(uint8_t *)&p, sizeof(p), NULL, 0);
}
@@ -415,7 +383,7 @@ static void *wov_map_shm(struct cros_ec_codec_priv *priv,
p.cmd = EC_CODEC_GET_SHM_ADDR;
p.get_shm_addr_param.shm_id = shm_id;
- if (send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC,
+ if (cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r)) < 0) {
dev_err(priv->dev, "failed to EC_CODEC_GET_SHM_ADDR\n");
@@ -453,7 +421,7 @@ static void *wov_map_shm(struct cros_ec_codec_priv *priv,
p.set_shm_addr_param.phys_addr = priv->ap_shm_last_alloc;
p.set_shm_addr_param.len = req;
p.set_shm_addr_param.shm_id = shm_id;
- if (send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC,
+ if (cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC,
(uint8_t *)&p, sizeof(p),
NULL, 0) < 0) {
dev_err(priv->dev, "failed to EC_CODEC_SET_SHM_ADDR\n");
@@ -571,7 +539,7 @@ static int wov_read_audio_shm(struct cros_ec_codec_priv *priv)
int ret;
p.cmd = EC_CODEC_WOV_READ_AUDIO_SHM;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret) {
@@ -596,7 +564,8 @@ static int wov_read_audio(struct cros_ec_codec_priv *priv)
while (remain >= 0) {
p.cmd = EC_CODEC_WOV_READ_AUDIO;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0,
+ EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret) {
@@ -669,7 +638,8 @@ static int wov_enable_put(struct snd_kcontrol *kcontrol,
else
p.cmd = EC_CODEC_WOV_DISABLE;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0,
+ EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret) {
dev_err(priv->dev, "failed to %s wov\n",
@@ -716,7 +686,7 @@ static int wov_set_lang_shm(struct cros_ec_codec_priv *priv,
p.cmd = EC_CODEC_WOV_SET_LANG_SHM;
memcpy(pp->hash, digest, SHA256_DIGEST_SIZE);
pp->total_len = size;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret) {
dev_err(priv->dev, "failed to EC_CODEC_WOV_SET_LANG_SHM\n");
@@ -743,7 +713,8 @@ static int wov_set_lang(struct cros_ec_codec_priv *priv,
pp->offset = i;
memcpy(pp->buf, buf + i, req);
pp->len = req;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0,
+ EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p), NULL, 0);
if (ret) {
dev_err(priv->dev, "failed to EC_CODEC_WOV_SET_LANG\n");
@@ -782,7 +753,7 @@ static int wov_hotword_model_put(struct snd_kcontrol *kcontrol,
goto leave;
p.cmd = EC_CODEC_WOV_GET_LANG;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC_WOV,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC_WOV,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret)
@@ -1020,7 +991,7 @@ static int cros_ec_codec_platform_probe(struct platform_device *pdev)
atomic_set(&priv->dmic_probed, 0);
p.cmd = EC_CODEC_GET_CAPABILITIES;
- ret = send_ec_host_command(priv->ec_device, EC_CMD_EC_CODEC,
+ ret = cros_ec_send_cmd_msg(priv->ec_device, 0, EC_CMD_EC_CODEC,
(uint8_t *)&p, sizeof(p),
(uint8_t *)&r, sizeof(r));
if (ret) {
--
2.25.0.341.g760bfbb309-goog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 11/17] ASoC: cros_ec_codec: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` [alsa-devel] " Prashant Malani
@ 2020-02-01 11:03 ` Mark Brown
-1 siblings, 0 replies; 38+ messages in thread
From: Mark Brown @ 2020-02-01 11:03 UTC (permalink / raw)
To: Prashant Malani
Cc: linux-kernel, Cheng-Yi Chiang, Enric Balletbo i Serra,
Guenter Roeck, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Benson Leung,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...
[-- Attachment #1: Type: text/plain, Size: 435 bytes --]
On Thu, Jan 30, 2020 at 12:30:56PM -0800, Prashant Malani wrote:
> Replace send_ec_host_command() with cros_ec_send_cmd_msg() which does
> the same thing, but is defined in a common location in platform/chrome
> and exposed for other modules to use. This allows us to remove
> send_ec_host_command() entirely.
I only have this patch, I've nothing else from the series or a
cover letter. What's the story with dependencies and so on?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [alsa-devel] [PATCH 11/17] ASoC: cros_ec_codec: Use cros_ec_send_cmd_msg()
@ 2020-02-01 11:03 ` Mark Brown
0 siblings, 0 replies; 38+ messages in thread
From: Mark Brown @ 2020-02-01 11:03 UTC (permalink / raw)
To: Prashant Malani
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
linux-kernel, Takashi Iwai, Liam Girdwood, Guenter Roeck,
Enric Balletbo i Serra, Benson Leung, Cheng-Yi Chiang
[-- Attachment #1.1: Type: text/plain, Size: 435 bytes --]
On Thu, Jan 30, 2020 at 12:30:56PM -0800, Prashant Malani wrote:
> Replace send_ec_host_command() with cros_ec_send_cmd_msg() which does
> the same thing, but is defined in a common location in platform/chrome
> and exposed for other modules to use. This allows us to remove
> send_ec_host_command() entirely.
I only have this patch, I've nothing else from the series or a
cover letter. What's the story with dependencies and so on?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 11/17] ASoC: cros_ec_codec: Use cros_ec_send_cmd_msg()
2020-02-01 11:03 ` [alsa-devel] " Mark Brown
@ 2020-02-03 18:42 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-02-03 18:42 UTC (permalink / raw)
To: Mark Brown
Cc: Linux Kernel Mailing List, Cheng-Yi Chiang,
Enric Balletbo i Serra, Guenter Roeck, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Benson Leung,
moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...
Hi Mark, thanks for looking at the patch. Please see inline.
On Sun, Feb 2, 2020 at 4:00 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Jan 30, 2020 at 12:30:56PM -0800, Prashant Malani wrote:
> > Replace send_ec_host_command() with cros_ec_send_cmd_msg() which does
> > the same thing, but is defined in a common location in platform/chrome
> > and exposed for other modules to use. This allows us to remove
> > send_ec_host_command() entirely.
>
> I only have this patch, I've nothing else from the series or a
> cover letter. What's the story with dependencies and so on?
Sorry about that. I will add you to the cover letter for subsequent
versions (I followed https://lwn.net/Articles/585782/ but I think it
only adds the relevant mailing lists to the cover letter...)
Just for reference, the cover series LKML link is here:
https://lkml.org/lkml/2020/1/30/802
Best regards,
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [alsa-devel] [PATCH 11/17] ASoC: cros_ec_codec: Use cros_ec_send_cmd_msg()
@ 2020-02-03 18:42 ` Prashant Malani
0 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-02-03 18:42 UTC (permalink / raw)
To: Mark Brown
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
Linux Kernel Mailing List, Takashi Iwai, Liam Girdwood,
Guenter Roeck, Enric Balletbo i Serra, Benson Leung,
Cheng-Yi Chiang
Hi Mark, thanks for looking at the patch. Please see inline.
On Sun, Feb 2, 2020 at 4:00 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Jan 30, 2020 at 12:30:56PM -0800, Prashant Malani wrote:
> > Replace send_ec_host_command() with cros_ec_send_cmd_msg() which does
> > the same thing, but is defined in a common location in platform/chrome
> > and exposed for other modules to use. This allows us to remove
> > send_ec_host_command() entirely.
>
> I only have this patch, I've nothing else from the series or a
> cover letter. What's the story with dependencies and so on?
Sorry about that. I will add you to the cover letter for subsequent
versions (I followed https://lwn.net/Articles/585782/ but I think it
only adds the relevant mailing lists to the cover letter...)
Just for reference, the cover series LKML link is here:
https://lkml.org/lkml/2020/1/30/802
Best regards,
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 12/17] power: supply: cros: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
` (11 preceding siblings ...)
(?)
@ 2020-01-30 20:30 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:30 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra,
Guenter Roeck, Sebastian Reichel,
open list:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
Replace cros_usbpd_charger_ec_command() with cros_ec_send_cmd_msg()
which does the same thing, but is defined in a common location in
platform/chrome and exposed for other modules to use. This allows us to
remove cros_usbpd_charger_ec_command() entirely.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/power/supply/cros_usbpd-charger.c | 63 ++++++-----------------
1 file changed, 15 insertions(+), 48 deletions(-)
diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c
index ffad9ee03a6858..cacaca5737a6ee 100644
--- a/drivers/power/supply/cros_usbpd-charger.c
+++ b/drivers/power/supply/cros_usbpd-charger.c
@@ -92,46 +92,14 @@ static bool cros_usbpd_charger_port_is_dedicated(struct port_data *port)
return port->port_number >= port->charger->num_usbpd_ports;
}
-static int cros_usbpd_charger_ec_command(struct charger_data *charger,
- unsigned int version,
- unsigned int command,
- void *outdata,
- unsigned int outsize,
- void *indata,
- unsigned int insize)
-{
- struct cros_ec_dev *ec_dev = charger->ec_dev;
- struct cros_ec_command *msg;
- int ret;
-
- msg = kzalloc(sizeof(*msg) + max(outsize, insize), GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
-
- msg->version = version;
- msg->command = ec_dev->cmd_offset + command;
- msg->outsize = outsize;
- msg->insize = insize;
-
- if (outsize)
- memcpy(msg->data, outdata, outsize);
-
- ret = cros_ec_cmd_xfer_status(charger->ec_device, msg);
- if (ret >= 0 && insize)
- memcpy(indata, msg->data, insize);
-
- kfree(msg);
- return ret;
-}
-
static int cros_usbpd_charger_get_num_ports(struct charger_data *charger)
{
struct ec_response_charge_port_count resp;
int ret;
- ret = cros_usbpd_charger_ec_command(charger, 0,
- EC_CMD_CHARGE_PORT_COUNT,
- NULL, 0, &resp, sizeof(resp));
+ ret = cros_ec_send_cmd_msg(charger->ec_device, 0,
+ EC_CMD_CHARGE_PORT_COUNT,
+ NULL, 0, &resp, sizeof(resp));
if (ret < 0)
return ret;
@@ -143,8 +111,8 @@ static int cros_usbpd_charger_get_usbpd_num_ports(struct charger_data *charger)
struct ec_response_usb_pd_ports resp;
int ret;
- ret = cros_usbpd_charger_ec_command(charger, 0, EC_CMD_USB_PD_PORTS,
- NULL, 0, &resp, sizeof(resp));
+ ret = cros_ec_send_cmd_msg(charger->ec_device, 0, EC_CMD_USB_PD_PORTS,
+ NULL, 0, &resp, sizeof(resp));
if (ret < 0)
return ret;
@@ -160,10 +128,9 @@ static int cros_usbpd_charger_get_discovery_info(struct port_data *port)
req.port = port->port_number;
- ret = cros_usbpd_charger_ec_command(charger, 0,
- EC_CMD_USB_PD_DISCOVERY,
- &req, sizeof(req),
- &resp, sizeof(resp));
+ ret = cros_ec_send_cmd_msg(charger->ec_device, 0,
+ EC_CMD_USB_PD_DISCOVERY, &req, sizeof(req),
+ &resp, sizeof(resp));
if (ret < 0) {
dev_err(charger->dev,
"Unable to query discovery info (err:0x%x)\n", ret);
@@ -190,10 +157,10 @@ static int cros_usbpd_charger_get_power_info(struct port_data *port)
int ret;
req.port = port->port_number;
- ret = cros_usbpd_charger_ec_command(charger, 0,
- EC_CMD_USB_PD_POWER_INFO,
- &req, sizeof(req),
- &resp, sizeof(resp));
+ ret = cros_ec_send_cmd_msg(charger->ec_device, 0,
+ EC_CMD_USB_PD_POWER_INFO,
+ &req, sizeof(req),
+ &resp, sizeof(resp));
if (ret < 0) {
dev_err(dev, "Unable to query PD power info (err:0x%x)\n", ret);
return ret;
@@ -335,9 +302,9 @@ static int cros_usbpd_charger_set_ext_power_limit(struct charger_data *charger,
req.current_lim = current_lim;
req.voltage_lim = voltage_lim;
- ret = cros_usbpd_charger_ec_command(charger, 0,
- EC_CMD_EXTERNAL_POWER_LIMIT,
- &req, sizeof(req), NULL, 0);
+ ret = cros_ec_send_cmd_msg(charger->ec_device, 0,
+ EC_CMD_EXTERNAL_POWER_LIMIT,
+ &req, sizeof(req), NULL, 0);
if (ret < 0)
dev_err(charger->dev,
"Unable to set the 'External Power Limit': %d\n", ret);
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 13/17] pwm: cros-ec: Remove cros_ec_cmd_xfer_status()
2020-01-30 20:30 ` Prashant Malani
@ 2020-01-30 20:31 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:31 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Thierry Reding, Uwe Kleine-König,
Benson Leung, Enric Balletbo i Serra, Guenter Roeck,
open list:PWM SUBSYSTEM
Convert one existing usage of cros_ec_cmd_xfer_status() to
cros_ec_send_cmd_msg(), which accomplishes the same thing but also does
the EC message struct setup,and is defined in platform/chrome and is
accessible by other modules.
For the other usage, switch it to using cros_ec_cmd_xfer(), since the
calling functions rely on the result field of the struct cros_ec_command
struct that is used.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/pwm/pwm-cros-ec.c | 27 +++++++++------------------
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 89497448d21775..8bf610a6529e7e 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -32,25 +32,14 @@ static inline struct cros_ec_pwm_device *pwm_to_cros_ec_pwm(struct pwm_chip *c)
static int cros_ec_pwm_set_duty(struct cros_ec_device *ec, u8 index, u16 duty)
{
- struct {
- struct cros_ec_command msg;
- struct ec_params_pwm_set_duty params;
- } __packed buf;
- struct ec_params_pwm_set_duty *params = &buf.params;
- struct cros_ec_command *msg = &buf.msg;
-
- memset(&buf, 0, sizeof(buf));
+ struct ec_params_pwm_set_duty params = {0};
- msg->version = 0;
- msg->command = EC_CMD_PWM_SET_DUTY;
- msg->insize = 0;
- msg->outsize = sizeof(*params);
-
- params->duty = duty;
- params->pwm_type = EC_PWM_TYPE_GENERIC;
- params->index = index;
+ params.duty = duty;
+ params.pwm_type = EC_PWM_TYPE_GENERIC;
+ params.index = index;
- return cros_ec_cmd_xfer_status(ec, msg);
+ return cros_ec_send_cmd_msg(ec, 0, EC_CMD_PWM_SET_DUTY, ¶ms,
+ sizeof(params), NULL, 0);
}
static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
@@ -78,11 +67,13 @@ static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
params->pwm_type = EC_PWM_TYPE_GENERIC;
params->index = index;
- ret = cros_ec_cmd_xfer_status(ec, msg);
+ ret = cros_ec_cmd_xfer(ec, msg);
if (result)
*result = msg->result;
if (ret < 0)
return ret;
+ else if (msg->result != EC_RES_SUCCESS)
+ return -EPROTO;
return resp->duty;
}
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 13/17] pwm: cros-ec: Remove cros_ec_cmd_xfer_status()
@ 2020-01-30 20:31 ` Prashant Malani
0 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:31 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Thierry Reding, Uwe Kleine-König,
Benson Leung, Enric Balletbo i Serra, Guenter Roeck,
open list:PWM SUBSYSTEM
Convert one existing usage of cros_ec_cmd_xfer_status() to
cros_ec_send_cmd_msg(), which accomplishes the same thing but also does
the EC message struct setup,and is defined in platform/chrome and is
accessible by other modules.
For the other usage, switch it to using cros_ec_cmd_xfer(), since the
calling functions rely on the result field of the struct cros_ec_command
struct that is used.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/pwm/pwm-cros-ec.c | 27 +++++++++------------------
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 89497448d21775..8bf610a6529e7e 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -32,25 +32,14 @@ static inline struct cros_ec_pwm_device *pwm_to_cros_ec_pwm(struct pwm_chip *c)
static int cros_ec_pwm_set_duty(struct cros_ec_device *ec, u8 index, u16 duty)
{
- struct {
- struct cros_ec_command msg;
- struct ec_params_pwm_set_duty params;
- } __packed buf;
- struct ec_params_pwm_set_duty *params = &buf.params;
- struct cros_ec_command *msg = &buf.msg;
-
- memset(&buf, 0, sizeof(buf));
+ struct ec_params_pwm_set_duty params = {0};
- msg->version = 0;
- msg->command = EC_CMD_PWM_SET_DUTY;
- msg->insize = 0;
- msg->outsize = sizeof(*params);
-
- params->duty = duty;
- params->pwm_type = EC_PWM_TYPE_GENERIC;
- params->index = index;
+ params.duty = duty;
+ params.pwm_type = EC_PWM_TYPE_GENERIC;
+ params.index = index;
- return cros_ec_cmd_xfer_status(ec, msg);
+ return cros_ec_send_cmd_msg(ec, 0, EC_CMD_PWM_SET_DUTY, ¶ms,
+ sizeof(params), NULL, 0);
}
static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
@@ -78,11 +67,13 @@ static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
params->pwm_type = EC_PWM_TYPE_GENERIC;
params->index = index;
- ret = cros_ec_cmd_xfer_status(ec, msg);
+ ret = cros_ec_cmd_xfer(ec, msg);
if (result)
*result = msg->result;
if (ret < 0)
return ret;
+ else if (msg->result != EC_RES_SUCCESS)
+ return -EPROTO;
return resp->duty;
}
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 13/17] pwm: cros-ec: Remove cros_ec_cmd_xfer_status()
2020-01-30 20:31 ` Prashant Malani
(?)
@ 2020-02-03 15:33 ` Enric Balletbo i Serra
2020-02-03 18:26 ` Prashant Malani
-1 siblings, 1 reply; 38+ messages in thread
From: Enric Balletbo i Serra @ 2020-02-03 15:33 UTC (permalink / raw)
To: Prashant Malani, linux-kernel
Cc: Thierry Reding, Uwe Kleine-König, Benson Leung,
Guenter Roeck, open list:PWM SUBSYSTEM
On 30/1/20 21:31, Prashant Malani wrote:
> Convert one existing usage of cros_ec_cmd_xfer_status() to
> cros_ec_send_cmd_msg(), which accomplishes the same thing but also does
> the EC message struct setup,and is defined in platform/chrome and is
> accessible by other modules.
>
> For the other usage, switch it to using cros_ec_cmd_xfer(), since the
> calling functions rely on the result field of the struct cros_ec_command
> struct that is used.
>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> ---
> drivers/pwm/pwm-cros-ec.c | 27 +++++++++------------------
> 1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
> index 89497448d21775..8bf610a6529e7e 100644
> --- a/drivers/pwm/pwm-cros-ec.c
> +++ b/drivers/pwm/pwm-cros-ec.c
> @@ -32,25 +32,14 @@ static inline struct cros_ec_pwm_device *pwm_to_cros_ec_pwm(struct pwm_chip *c)
>
> static int cros_ec_pwm_set_duty(struct cros_ec_device *ec, u8 index, u16 duty)
> {
> - struct {
> - struct cros_ec_command msg;
> - struct ec_params_pwm_set_duty params;
> - } __packed buf;
> - struct ec_params_pwm_set_duty *params = &buf.params;
> - struct cros_ec_command *msg = &buf.msg;
> -
> - memset(&buf, 0, sizeof(buf));
> + struct ec_params_pwm_set_duty params = {0};
>
> - msg->version = 0;
> - msg->command = EC_CMD_PWM_SET_DUTY;
> - msg->insize = 0;
> - msg->outsize = sizeof(*params);
> -
> - params->duty = duty;
> - params->pwm_type = EC_PWM_TYPE_GENERIC;
> - params->index = index;
> + params.duty = duty;
> + params.pwm_type = EC_PWM_TYPE_GENERIC;
> + params.index = index;
>
> - return cros_ec_cmd_xfer_status(ec, msg);
> + return cros_ec_send_cmd_msg(ec, 0, EC_CMD_PWM_SET_DUTY, ¶ms,
> + sizeof(params), NULL, 0);
> }
>
> static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
> @@ -78,11 +67,13 @@ static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
> params->pwm_type = EC_PWM_TYPE_GENERIC;
> params->index = index;
>
> - ret = cros_ec_cmd_xfer_status(ec, msg);
> + ret = cros_ec_cmd_xfer(ec, msg);
Why? There is a good reason we introduced the cros_ec_cmd_xfer_status.
IMO the purpose of introduce the new wrapper only makes sense if we can cover
_all_ the cases, so we can remove cros_ec_cmd_xfer_status and make
cros_ec_cmd_xfer private to cros_ec_proto.
Is not possible to use the new wrapper here?
> if (result)
> *result = msg->result;
Hmm, I see, that's the problem ...
This driver will need a bit of rework but I think could be possible to use the
wrapper.
> if (ret < 0)
> return ret;
> + else if (msg->result != EC_RES_SUCCESS)
> + return -EPROTO;
>
> return resp->duty;
> }
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 13/17] pwm: cros-ec: Remove cros_ec_cmd_xfer_status()
2020-02-03 15:33 ` Enric Balletbo i Serra
@ 2020-02-03 18:26 ` Prashant Malani
2020-02-03 18:39 ` Prashant Malani
0 siblings, 1 reply; 38+ messages in thread
From: Prashant Malani @ 2020-02-03 18:26 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Linux Kernel Mailing List, Thierry Reding, Uwe Kleine-König,
Benson Leung, Guenter Roeck, open list:PWM SUBSYSTEM
On Mon, Feb 3, 2020 at 7:33 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
>
> On 30/1/20 21:31, Prashant Malani wrote:
> > Convert one existing usage of cros_ec_cmd_xfer_status() to
> > cros_ec_send_cmd_msg(), which accomplishes the same thing but also does
> > the EC message struct setup,and is defined in platform/chrome and is
> > accessible by other modules.
> >
> > For the other usage, switch it to using cros_ec_cmd_xfer(), since the
> > calling functions rely on the result field of the struct cros_ec_command
> > struct that is used.
> >
> > Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > ---
> > drivers/pwm/pwm-cros-ec.c | 27 +++++++++------------------
> > 1 file changed, 9 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
> > index 89497448d21775..8bf610a6529e7e 100644
> > --- a/drivers/pwm/pwm-cros-ec.c
> > +++ b/drivers/pwm/pwm-cros-ec.c
> > @@ -32,25 +32,14 @@ static inline struct cros_ec_pwm_device *pwm_to_cros_ec_pwm(struct pwm_chip *c)
> >
> > static int cros_ec_pwm_set_duty(struct cros_ec_device *ec, u8 index, u16 duty)
> > {
> > - struct {
> > - struct cros_ec_command msg;
> > - struct ec_params_pwm_set_duty params;
> > - } __packed buf;
> > - struct ec_params_pwm_set_duty *params = &buf.params;
> > - struct cros_ec_command *msg = &buf.msg;
> > -
> > - memset(&buf, 0, sizeof(buf));
> > + struct ec_params_pwm_set_duty params = {0};
> >
> > - msg->version = 0;
> > - msg->command = EC_CMD_PWM_SET_DUTY;
> > - msg->insize = 0;
> > - msg->outsize = sizeof(*params);
> > -
> > - params->duty = duty;
> > - params->pwm_type = EC_PWM_TYPE_GENERIC;
> > - params->index = index;
> > + params.duty = duty;
> > + params.pwm_type = EC_PWM_TYPE_GENERIC;
> > + params.index = index;
> >
> > - return cros_ec_cmd_xfer_status(ec, msg);
> > + return cros_ec_send_cmd_msg(ec, 0, EC_CMD_PWM_SET_DUTY, ¶ms,
> > + sizeof(params), NULL, 0);
> > }
> >
> > static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
> > @@ -78,11 +67,13 @@ static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
> > params->pwm_type = EC_PWM_TYPE_GENERIC;
> > params->index = index;
> >
> > - ret = cros_ec_cmd_xfer_status(ec, msg);
> > + ret = cros_ec_cmd_xfer(ec, msg);
>
> Why? There is a good reason we introduced the cros_ec_cmd_xfer_status.
>
> IMO the purpose of introduce the new wrapper only makes sense if we can cover
> _all_ the cases, so we can remove cros_ec_cmd_xfer_status and make
> cros_ec_cmd_xfer private to cros_ec_proto.
>
> Is not possible to use the new wrapper here?
>
> > if (result)
> > *result = msg->result;
>
> Hmm, I see, that's the problem ...
>
> This driver will need a bit of rework but I think could be possible to use the
> wrapper.
Yeah, I looked around, and it seems to use msg->result.
Perhaps we should work on reworking this driver before doing the large
patch series? I would be happy to work on it, unless you feel there is
someone else who'd be better suited. Kindly let me know.
>
> > if (ret < 0)
> > return ret;
> > + else if (msg->result != EC_RES_SUCCESS)
> > + return -EPROTO;
> >
> > return resp->duty;
> > }
> >
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 13/17] pwm: cros-ec: Remove cros_ec_cmd_xfer_status()
2020-02-03 18:26 ` Prashant Malani
@ 2020-02-03 18:39 ` Prashant Malani
0 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-02-03 18:39 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Linux Kernel Mailing List, Thierry Reding, Uwe Kleine-König,
Benson Leung, Guenter Roeck, open list:PWM SUBSYSTEM
Hi Enric,
On Mon, Feb 3, 2020 at 10:26 AM Prashant Malani <pmalani@chromium.org> wrote:
>
> On Mon, Feb 3, 2020 at 7:33 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
> >
> >
> > On 30/1/20 21:31, Prashant Malani wrote:
> > > Convert one existing usage of cros_ec_cmd_xfer_status() to
> > > cros_ec_send_cmd_msg(), which accomplishes the same thing but also does
> > > the EC message struct setup,and is defined in platform/chrome and is
> > > accessible by other modules.
> > >
> > > For the other usage, switch it to using cros_ec_cmd_xfer(), since the
> > > calling functions rely on the result field of the struct cros_ec_command
> > > struct that is used.
> > >
> > > Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > > ---
> > > drivers/pwm/pwm-cros-ec.c | 27 +++++++++------------------
> > > 1 file changed, 9 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
> > > index 89497448d21775..8bf610a6529e7e 100644
> > > --- a/drivers/pwm/pwm-cros-ec.c
> > > +++ b/drivers/pwm/pwm-cros-ec.c
> > > @@ -32,25 +32,14 @@ static inline struct cros_ec_pwm_device *pwm_to_cros_ec_pwm(struct pwm_chip *c)
> > >
> > > static int cros_ec_pwm_set_duty(struct cros_ec_device *ec, u8 index, u16 duty)
> > > {
> > > - struct {
> > > - struct cros_ec_command msg;
> > > - struct ec_params_pwm_set_duty params;
> > > - } __packed buf;
> > > - struct ec_params_pwm_set_duty *params = &buf.params;
> > > - struct cros_ec_command *msg = &buf.msg;
> > > -
> > > - memset(&buf, 0, sizeof(buf));
> > > + struct ec_params_pwm_set_duty params = {0};
> > >
> > > - msg->version = 0;
> > > - msg->command = EC_CMD_PWM_SET_DUTY;
> > > - msg->insize = 0;
> > > - msg->outsize = sizeof(*params);
> > > -
> > > - params->duty = duty;
> > > - params->pwm_type = EC_PWM_TYPE_GENERIC;
> > > - params->index = index;
> > > + params.duty = duty;
> > > + params.pwm_type = EC_PWM_TYPE_GENERIC;
> > > + params.index = index;
> > >
> > > - return cros_ec_cmd_xfer_status(ec, msg);
> > > + return cros_ec_send_cmd_msg(ec, 0, EC_CMD_PWM_SET_DUTY, ¶ms,
> > > + sizeof(params), NULL, 0);
> > > }
> > >
> > > static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
> > > @@ -78,11 +67,13 @@ static int __cros_ec_pwm_get_duty(struct cros_ec_device *ec, u8 index,
> > > params->pwm_type = EC_PWM_TYPE_GENERIC;
> > > params->index = index;
> > >
> > > - ret = cros_ec_cmd_xfer_status(ec, msg);
> > > + ret = cros_ec_cmd_xfer(ec, msg);
> >
> > Why? There is a good reason we introduced the cros_ec_cmd_xfer_status.
> >
> > IMO the purpose of introduce the new wrapper only makes sense if we can cover
> > _all_ the cases, so we can remove cros_ec_cmd_xfer_status and make
> > cros_ec_cmd_xfer private to cros_ec_proto.
I'm hoping for that too, but as we saw below (and some in some other
drivers), some callers of cros_ec_cmd_xfer() actually use the
msg->result.
Should we change the new wrapper to return the message via a pointer
(if not NULL), so something like this ? :
int cros_ec_send_cmd_msg(struct cros_ec_device *ec, unsigned int
version, uint32_t command, void *outdata, unsigned int outsize,
void *indata, unsigned int insize, uint32_t *result) ?
> >
> > Is not possible to use the new wrapper here?
> >
> > > if (result)
> > > *result = msg->result;
> >
> > Hmm, I see, that's the problem ...
> >
> > This driver will need a bit of rework but I think could be possible to use the
> > wrapper.
> Yeah, I looked around, and it seems to use msg->result.
> Perhaps we should work on reworking this driver before doing the large
> patch series? I would be happy to work on it, unless you feel there is
> someone else who'd be better suited. Kindly let me know.
> >
> > > if (ret < 0)
> > > return ret;
> > > + else if (msg->result != EC_RES_SUCCESS)
> > > + return -EPROTO;
> > >
> > > return resp->duty;
> > > }
> > >
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 14/17] rtc: cros-ec: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
` (13 preceding siblings ...)
(?)
@ 2020-01-30 20:31 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:31 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Alessandro Zummo, Alexandre Belloni,
Benson Leung, Enric Balletbo i Serra, Guenter Roeck,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM
Replace cros_ec_cmd_xfer_status() with cros_ec_send_cmd_msg() which does
the message buffer setup and cleanup.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/rtc/rtc-cros-ec.c | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)
diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
index d043d30f05bc1d..113638a82e2c0c 100644
--- a/drivers/rtc/rtc-cros-ec.c
+++ b/drivers/rtc/rtc-cros-ec.c
@@ -34,16 +34,11 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command,
u32 *response)
{
int ret;
- struct {
- struct cros_ec_command msg;
- struct ec_response_rtc data;
- } __packed msg;
- memset(&msg, 0, sizeof(msg));
- msg.msg.command = command;
- msg.msg.insize = sizeof(msg.data);
+ struct ec_response_rtc data = {0};
- ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+ ret = cros_ec_send_cmd_msg(cros_ec, 0, command, NULL, 0,
+ &data, sizeof(data));
if (ret < 0) {
dev_err(cros_ec->dev,
"error getting %s from EC: %d\n",
@@ -52,7 +47,7 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command,
return ret;
}
- *response = msg.data.time;
+ *response = data.time;
return 0;
}
@@ -61,17 +56,11 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command,
u32 param)
{
int ret = 0;
- struct {
- struct cros_ec_command msg;
- struct ec_response_rtc data;
- } __packed msg;
+ struct ec_response_rtc data;
- memset(&msg, 0, sizeof(msg));
- msg.msg.command = command;
- msg.msg.outsize = sizeof(msg.data);
- msg.data.time = param;
-
- ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+ data.time = param;
+ ret = cros_ec_send_cmd_msg(cros_ec, 0, command, &data, sizeof(data),
+ NULL, 0);
if (ret < 0) {
dev_err(cros_ec->dev, "error setting %s on EC: %d\n",
command == EC_CMD_RTC_SET_VALUE ? "time" : "alarm",
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 15/17] media: cros-ec-cec: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
` (14 preceding siblings ...)
(?)
@ 2020-01-30 20:31 ` Prashant Malani
2020-02-02 22:08 ` kbuild test robot
2020-02-03 5:35 ` kbuild test robot
-1 siblings, 2 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:31 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Mauro Carvalho Chehab, Benson Leung,
Enric Balletbo i Serra, Guenter Roeck, Hans Verkuil,
Neil Armstrong, Gwendal Grignou,
open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
Replace cros_ec_cmd_xfer_status() with cros_ec_send_cmd_msg() which does
the message buffer setup and cleanup, but is located in platform/chrome
and used by other drivers.
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
.../media/platform/cros-ec-cec/cros-ec-cec.c | 39 +++++++------------
1 file changed, 14 insertions(+), 25 deletions(-)
diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
index f048e89947850e..0ee7354dca9724 100644
--- a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
+++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
@@ -94,18 +94,14 @@ static int cros_ec_cec_set_log_addr(struct cec_adapter *adap, u8 logical_addr)
{
struct cros_ec_cec *cros_ec_cec = adap->priv;
struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
- struct {
- struct cros_ec_command msg;
- struct ec_params_cec_set data;
- } __packed msg = {};
+ struct ec_params_cec_set data;
int ret;
- msg.msg.command = EC_CMD_CEC_SET;
- msg.msg.outsize = sizeof(msg.data);
- msg.data.cmd = CEC_CMD_LOGICAL_ADDRESS;
- msg.data.val = logical_addr;
+ data.cmd = CEC_CMD_LOGICAL_ADDRESS;
+ data.val = logical_addr;
- ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+ ret = cros_ec_send_cmd_msg(cros_ec, 0, EC_CMD_CEC_SET, &data,
+ sizeof(data), NULL, 0);
if (ret < 0) {
dev_err(cros_ec->dev,
"error setting CEC logical address on EC: %d\n", ret);
@@ -120,17 +116,14 @@ static int cros_ec_cec_transmit(struct cec_adapter *adap, u8 attempts,
{
struct cros_ec_cec *cros_ec_cec = adap->priv;
struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
- struct {
- struct cros_ec_command msg;
- struct ec_params_cec_write data;
- } __packed msg = {};
+ struct ec_params_cec_write data = {};
int ret;
- msg.msg.command = EC_CMD_CEC_WRITE_MSG;
msg.msg.outsize = cec_msg->len;
- memcpy(msg.data.msg, cec_msg->msg, cec_msg->len);
+ memcpy(data.msg, cec_msg->msg, cec_msg->len);
- ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+ ret = cros_ec_send_cmd_msg(cros_ec, 0, EC_CMD_CEC_WRITE_MSG,
+ &data, sizeof(cec_msg->len), NULL, 0);
if (ret < 0) {
dev_err(cros_ec->dev,
"error writing CEC msg on EC: %d\n", ret);
@@ -144,18 +137,14 @@ static int cros_ec_cec_adap_enable(struct cec_adapter *adap, bool enable)
{
struct cros_ec_cec *cros_ec_cec = adap->priv;
struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
- struct {
- struct cros_ec_command msg;
- struct ec_params_cec_set data;
- } __packed msg = {};
+ struct ec_params_cec_set data = {0};
int ret;
- msg.msg.command = EC_CMD_CEC_SET;
- msg.msg.outsize = sizeof(msg.data);
- msg.data.cmd = CEC_CMD_ENABLE;
- msg.data.val = enable;
+ data.cmd = CEC_CMD_ENABLE;
+ data.val = enable;
- ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+ ret = cros_ec_send_cmd_msg(cros_ec, 0, EC_CMD_CEC_SET, &data,
+ sizeof(data), NULL, 0);
if (ret < 0) {
dev_err(cros_ec->dev,
"error %sabling CEC on EC: %d\n",
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 15/17] media: cros-ec-cec: Use cros_ec_send_cmd_msg()
2020-01-30 20:31 ` [PATCH 15/17] media: cros-ec-cec: " Prashant Malani
@ 2020-02-02 22:08 ` kbuild test robot
2020-02-03 5:35 ` kbuild test robot
1 sibling, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2020-02-02 22:08 UTC (permalink / raw)
To: Prashant Malani
Cc: kbuild-all, linux-kernel, Prashant Malani, Mauro Carvalho Chehab,
Benson Leung, Enric Balletbo i Serra, Guenter Roeck,
Hans Verkuil, Neil Armstrong, Gwendal Grignou,
open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
[-- Attachment #1: Type: text/plain, Size: 3602 bytes --]
Hi Prashant,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to chrome-platform-linux/for-next abelloni/rtc-next linux/master v5.5 next-20200130]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Prashant-Malani/platform-chrome-Replace-cros_ec_cmd_xfer_status/20200203-022935
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 94f2630b18975bb56eee5d1a36371db967643479
config: x86_64-randconfig-b002-20200202 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/media/platform/cros-ec-cec/cros-ec-cec.c: In function 'cros_ec_cec_transmit':
>> drivers/media/platform/cros-ec-cec/cros-ec-cec.c:122:2: error: 'msg' undeclared (first use in this function); did you mean 'msr'?
msg.msg.outsize = cec_msg->len;
^~~
msr
drivers/media/platform/cros-ec-cec/cros-ec-cec.c:122:2: note: each undeclared identifier is reported only once for each function it appears in
vim +122 drivers/media/platform/cros-ec-cec/cros-ec-cec.c
cd70de2d356ee6 Neil Armstrong 2018-07-04 113
cd70de2d356ee6 Neil Armstrong 2018-07-04 114 static int cros_ec_cec_transmit(struct cec_adapter *adap, u8 attempts,
cd70de2d356ee6 Neil Armstrong 2018-07-04 115 u32 signal_free_time, struct cec_msg *cec_msg)
cd70de2d356ee6 Neil Armstrong 2018-07-04 116 {
cd70de2d356ee6 Neil Armstrong 2018-07-04 117 struct cros_ec_cec *cros_ec_cec = adap->priv;
cd70de2d356ee6 Neil Armstrong 2018-07-04 118 struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
bd6054aba6ffe7 Prashant Malani 2020-01-30 119 struct ec_params_cec_write data = {};
cd70de2d356ee6 Neil Armstrong 2018-07-04 120 int ret;
cd70de2d356ee6 Neil Armstrong 2018-07-04 121
cd70de2d356ee6 Neil Armstrong 2018-07-04 @122 msg.msg.outsize = cec_msg->len;
bd6054aba6ffe7 Prashant Malani 2020-01-30 123 memcpy(data.msg, cec_msg->msg, cec_msg->len);
cd70de2d356ee6 Neil Armstrong 2018-07-04 124
bd6054aba6ffe7 Prashant Malani 2020-01-30 125 ret = cros_ec_send_cmd_msg(cros_ec, 0, EC_CMD_CEC_WRITE_MSG,
bd6054aba6ffe7 Prashant Malani 2020-01-30 126 &data, sizeof(cec_msg->len), NULL, 0);
cd70de2d356ee6 Neil Armstrong 2018-07-04 127 if (ret < 0) {
cd70de2d356ee6 Neil Armstrong 2018-07-04 128 dev_err(cros_ec->dev,
cd70de2d356ee6 Neil Armstrong 2018-07-04 129 "error writing CEC msg on EC: %d\n", ret);
cd70de2d356ee6 Neil Armstrong 2018-07-04 130 return ret;
cd70de2d356ee6 Neil Armstrong 2018-07-04 131 }
cd70de2d356ee6 Neil Armstrong 2018-07-04 132
cd70de2d356ee6 Neil Armstrong 2018-07-04 133 return 0;
cd70de2d356ee6 Neil Armstrong 2018-07-04 134 }
cd70de2d356ee6 Neil Armstrong 2018-07-04 135
:::::: The code at line 122 was first introduced by commit
:::::: cd70de2d356ee692477276bd5d6bc88c71a48733 media: platform: Add ChromeOS EC CEC driver
:::::: TO: Neil Armstrong <narmstrong@baylibre.com>
:::::: CC: Lee Jones <lee.jones@linaro.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35108 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 15/17] media: cros-ec-cec: Use cros_ec_send_cmd_msg()
@ 2020-02-02 22:08 ` kbuild test robot
0 siblings, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2020-02-02 22:08 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3669 bytes --]
Hi Prashant,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to chrome-platform-linux/for-next abelloni/rtc-next linux/master v5.5 next-20200130]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Prashant-Malani/platform-chrome-Replace-cros_ec_cmd_xfer_status/20200203-022935
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 94f2630b18975bb56eee5d1a36371db967643479
config: x86_64-randconfig-b002-20200202 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/media/platform/cros-ec-cec/cros-ec-cec.c: In function 'cros_ec_cec_transmit':
>> drivers/media/platform/cros-ec-cec/cros-ec-cec.c:122:2: error: 'msg' undeclared (first use in this function); did you mean 'msr'?
msg.msg.outsize = cec_msg->len;
^~~
msr
drivers/media/platform/cros-ec-cec/cros-ec-cec.c:122:2: note: each undeclared identifier is reported only once for each function it appears in
vim +122 drivers/media/platform/cros-ec-cec/cros-ec-cec.c
cd70de2d356ee6 Neil Armstrong 2018-07-04 113
cd70de2d356ee6 Neil Armstrong 2018-07-04 114 static int cros_ec_cec_transmit(struct cec_adapter *adap, u8 attempts,
cd70de2d356ee6 Neil Armstrong 2018-07-04 115 u32 signal_free_time, struct cec_msg *cec_msg)
cd70de2d356ee6 Neil Armstrong 2018-07-04 116 {
cd70de2d356ee6 Neil Armstrong 2018-07-04 117 struct cros_ec_cec *cros_ec_cec = adap->priv;
cd70de2d356ee6 Neil Armstrong 2018-07-04 118 struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
bd6054aba6ffe7 Prashant Malani 2020-01-30 119 struct ec_params_cec_write data = {};
cd70de2d356ee6 Neil Armstrong 2018-07-04 120 int ret;
cd70de2d356ee6 Neil Armstrong 2018-07-04 121
cd70de2d356ee6 Neil Armstrong 2018-07-04 @122 msg.msg.outsize = cec_msg->len;
bd6054aba6ffe7 Prashant Malani 2020-01-30 123 memcpy(data.msg, cec_msg->msg, cec_msg->len);
cd70de2d356ee6 Neil Armstrong 2018-07-04 124
bd6054aba6ffe7 Prashant Malani 2020-01-30 125 ret = cros_ec_send_cmd_msg(cros_ec, 0, EC_CMD_CEC_WRITE_MSG,
bd6054aba6ffe7 Prashant Malani 2020-01-30 126 &data, sizeof(cec_msg->len), NULL, 0);
cd70de2d356ee6 Neil Armstrong 2018-07-04 127 if (ret < 0) {
cd70de2d356ee6 Neil Armstrong 2018-07-04 128 dev_err(cros_ec->dev,
cd70de2d356ee6 Neil Armstrong 2018-07-04 129 "error writing CEC msg on EC: %d\n", ret);
cd70de2d356ee6 Neil Armstrong 2018-07-04 130 return ret;
cd70de2d356ee6 Neil Armstrong 2018-07-04 131 }
cd70de2d356ee6 Neil Armstrong 2018-07-04 132
cd70de2d356ee6 Neil Armstrong 2018-07-04 133 return 0;
cd70de2d356ee6 Neil Armstrong 2018-07-04 134 }
cd70de2d356ee6 Neil Armstrong 2018-07-04 135
:::::: The code at line 122 was first introduced by commit
:::::: cd70de2d356ee692477276bd5d6bc88c71a48733 media: platform: Add ChromeOS EC CEC driver
:::::: TO: Neil Armstrong <narmstrong@baylibre.com>
:::::: CC: Lee Jones <lee.jones@linaro.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35108 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 15/17] media: cros-ec-cec: Use cros_ec_send_cmd_msg()
2020-01-30 20:31 ` [PATCH 15/17] media: cros-ec-cec: " Prashant Malani
@ 2020-02-03 5:35 ` kbuild test robot
2020-02-03 5:35 ` kbuild test robot
1 sibling, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2020-02-03 5:35 UTC (permalink / raw)
To: Prashant Malani
Cc: kbuild-all, linux-kernel, Prashant Malani, Mauro Carvalho Chehab,
Benson Leung, Enric Balletbo i Serra, Guenter Roeck,
Hans Verkuil, Neil Armstrong, Gwendal Grignou,
open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)
[-- Attachment #1: Type: text/plain, Size: 3750 bytes --]
Hi Prashant,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to chrome-platform-linux/for-next abelloni/rtc-next linux/master v5.5 next-20200131]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Prashant-Malani/platform-chrome-Replace-cros_ec_cmd_xfer_status/20200203-022935
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 94f2630b18975bb56eee5d1a36371db967643479
config: s390-allmodconfig (attached as .config)
compiler: s390-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=s390
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/media/platform/cros-ec-cec/cros-ec-cec.c: In function 'cros_ec_cec_transmit':
>> drivers/media/platform/cros-ec-cec/cros-ec-cec.c:122:2: error: 'msg' undeclared (first use in this function); did you mean 'cspg'?
msg.msg.outsize = cec_msg->len;
^~~
cspg
drivers/media/platform/cros-ec-cec/cros-ec-cec.c:122:2: note: each undeclared identifier is reported only once for each function it appears in
vim +122 drivers/media/platform/cros-ec-cec/cros-ec-cec.c
cd70de2d356ee6 Neil Armstrong 2018-07-04 113
cd70de2d356ee6 Neil Armstrong 2018-07-04 114 static int cros_ec_cec_transmit(struct cec_adapter *adap, u8 attempts,
cd70de2d356ee6 Neil Armstrong 2018-07-04 115 u32 signal_free_time, struct cec_msg *cec_msg)
cd70de2d356ee6 Neil Armstrong 2018-07-04 116 {
cd70de2d356ee6 Neil Armstrong 2018-07-04 117 struct cros_ec_cec *cros_ec_cec = adap->priv;
cd70de2d356ee6 Neil Armstrong 2018-07-04 118 struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
bd6054aba6ffe7 Prashant Malani 2020-01-30 119 struct ec_params_cec_write data = {};
cd70de2d356ee6 Neil Armstrong 2018-07-04 120 int ret;
cd70de2d356ee6 Neil Armstrong 2018-07-04 121
cd70de2d356ee6 Neil Armstrong 2018-07-04 @122 msg.msg.outsize = cec_msg->len;
bd6054aba6ffe7 Prashant Malani 2020-01-30 123 memcpy(data.msg, cec_msg->msg, cec_msg->len);
cd70de2d356ee6 Neil Armstrong 2018-07-04 124
bd6054aba6ffe7 Prashant Malani 2020-01-30 125 ret = cros_ec_send_cmd_msg(cros_ec, 0, EC_CMD_CEC_WRITE_MSG,
bd6054aba6ffe7 Prashant Malani 2020-01-30 126 &data, sizeof(cec_msg->len), NULL, 0);
cd70de2d356ee6 Neil Armstrong 2018-07-04 127 if (ret < 0) {
cd70de2d356ee6 Neil Armstrong 2018-07-04 128 dev_err(cros_ec->dev,
cd70de2d356ee6 Neil Armstrong 2018-07-04 129 "error writing CEC msg on EC: %d\n", ret);
cd70de2d356ee6 Neil Armstrong 2018-07-04 130 return ret;
cd70de2d356ee6 Neil Armstrong 2018-07-04 131 }
cd70de2d356ee6 Neil Armstrong 2018-07-04 132
cd70de2d356ee6 Neil Armstrong 2018-07-04 133 return 0;
cd70de2d356ee6 Neil Armstrong 2018-07-04 134 }
cd70de2d356ee6 Neil Armstrong 2018-07-04 135
:::::: The code at line 122 was first introduced by commit
:::::: cd70de2d356ee692477276bd5d6bc88c71a48733 media: platform: Add ChromeOS EC CEC driver
:::::: TO: Neil Armstrong <narmstrong@baylibre.com>
:::::: CC: Lee Jones <lee.jones@linaro.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 57998 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 15/17] media: cros-ec-cec: Use cros_ec_send_cmd_msg()
@ 2020-02-03 5:35 ` kbuild test robot
0 siblings, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2020-02-03 5:35 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3819 bytes --]
Hi Prashant,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to chrome-platform-linux/for-next abelloni/rtc-next linux/master v5.5 next-20200131]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Prashant-Malani/platform-chrome-Replace-cros_ec_cmd_xfer_status/20200203-022935
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 94f2630b18975bb56eee5d1a36371db967643479
config: s390-allmodconfig (attached as .config)
compiler: s390-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=s390
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/media/platform/cros-ec-cec/cros-ec-cec.c: In function 'cros_ec_cec_transmit':
>> drivers/media/platform/cros-ec-cec/cros-ec-cec.c:122:2: error: 'msg' undeclared (first use in this function); did you mean 'cspg'?
msg.msg.outsize = cec_msg->len;
^~~
cspg
drivers/media/platform/cros-ec-cec/cros-ec-cec.c:122:2: note: each undeclared identifier is reported only once for each function it appears in
vim +122 drivers/media/platform/cros-ec-cec/cros-ec-cec.c
cd70de2d356ee6 Neil Armstrong 2018-07-04 113
cd70de2d356ee6 Neil Armstrong 2018-07-04 114 static int cros_ec_cec_transmit(struct cec_adapter *adap, u8 attempts,
cd70de2d356ee6 Neil Armstrong 2018-07-04 115 u32 signal_free_time, struct cec_msg *cec_msg)
cd70de2d356ee6 Neil Armstrong 2018-07-04 116 {
cd70de2d356ee6 Neil Armstrong 2018-07-04 117 struct cros_ec_cec *cros_ec_cec = adap->priv;
cd70de2d356ee6 Neil Armstrong 2018-07-04 118 struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
bd6054aba6ffe7 Prashant Malani 2020-01-30 119 struct ec_params_cec_write data = {};
cd70de2d356ee6 Neil Armstrong 2018-07-04 120 int ret;
cd70de2d356ee6 Neil Armstrong 2018-07-04 121
cd70de2d356ee6 Neil Armstrong 2018-07-04 @122 msg.msg.outsize = cec_msg->len;
bd6054aba6ffe7 Prashant Malani 2020-01-30 123 memcpy(data.msg, cec_msg->msg, cec_msg->len);
cd70de2d356ee6 Neil Armstrong 2018-07-04 124
bd6054aba6ffe7 Prashant Malani 2020-01-30 125 ret = cros_ec_send_cmd_msg(cros_ec, 0, EC_CMD_CEC_WRITE_MSG,
bd6054aba6ffe7 Prashant Malani 2020-01-30 126 &data, sizeof(cec_msg->len), NULL, 0);
cd70de2d356ee6 Neil Armstrong 2018-07-04 127 if (ret < 0) {
cd70de2d356ee6 Neil Armstrong 2018-07-04 128 dev_err(cros_ec->dev,
cd70de2d356ee6 Neil Armstrong 2018-07-04 129 "error writing CEC msg on EC: %d\n", ret);
cd70de2d356ee6 Neil Armstrong 2018-07-04 130 return ret;
cd70de2d356ee6 Neil Armstrong 2018-07-04 131 }
cd70de2d356ee6 Neil Armstrong 2018-07-04 132
cd70de2d356ee6 Neil Armstrong 2018-07-04 133 return 0;
cd70de2d356ee6 Neil Armstrong 2018-07-04 134 }
cd70de2d356ee6 Neil Armstrong 2018-07-04 135
:::::: The code at line 122 was first introduced by commit
:::::: cd70de2d356ee692477276bd5d6bc88c71a48733 media: platform: Add ChromeOS EC CEC driver
:::::: TO: Neil Armstrong <narmstrong@baylibre.com>
:::::: CC: Lee Jones <lee.jones@linaro.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 57998 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 16/17] i2c: cros-ec-tunnel: Use cros_ec_send_cmd_msg()
2020-01-30 20:30 ` Prashant Malani
@ 2020-01-30 20:31 ` Prashant Malani
-1 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:31 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra,
Guenter Roeck, Wolfram Sang, Jonathan Cameron, Andy Shevchenko,
Dmitry Torokhov, Akshu Agrawal,
open list:I2C SUBSYSTEM HOST DRIVERS
Replace cros_ec_cmd_xfer_status() calls with the new function
cros_ec_send_cmd_msg() which takes care of the EC message struct setup
and subsequent cleanup (which is a common pattern among users of
cros_ec_cmd_xfer_status).
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 958161c71985d9..50161dff912298 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -179,9 +179,8 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
const u16 bus_num = bus->remote_bus;
int request_len;
int response_len;
- int alloc_size;
int result;
- struct cros_ec_command *msg;
+ void *ec_buf;
request_len = ec_i2c_count_message(i2c_msgs, num);
if (request_len < 0) {
@@ -196,36 +195,32 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
return response_len;
}
- alloc_size = max(request_len, response_len);
- msg = kmalloc(sizeof(*msg) + alloc_size, GFP_KERNEL);
- if (!msg)
+ ec_buf = kmalloc(max(request_len, response_len), GFP_KERNEL);
+ if (!ec_buf)
return -ENOMEM;
- result = ec_i2c_construct_message(msg->data, i2c_msgs, num, bus_num);
+ result = ec_i2c_construct_message(ec_buf, i2c_msgs, num, bus_num);
if (result) {
dev_err(dev, "Error constructing EC i2c message %d\n", result);
goto exit;
}
- msg->version = 0;
- msg->command = EC_CMD_I2C_PASSTHRU;
- msg->outsize = request_len;
- msg->insize = response_len;
-
- result = cros_ec_cmd_xfer_status(bus->ec, msg);
+ result = cros_ec_send_cmd_msg(bus->ec, 0, EC_CMD_I2C_PASSTHRU,
+ ec_buf, request_len,
+ ec_buf, response_len);
if (result < 0) {
dev_err(dev, "Error transferring EC i2c message %d\n", result);
goto exit;
}
- result = ec_i2c_parse_response(msg->data, i2c_msgs, &num);
+ result = ec_i2c_parse_response(ec_buf, i2c_msgs, &num);
if (result < 0)
goto exit;
/* Indicate success by saying how many messages were sent */
result = num;
exit:
- kfree(msg);
+ kfree(ec_buf);
return result;
}
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 16/17] i2c: cros-ec-tunnel: Use cros_ec_send_cmd_msg()
@ 2020-01-30 20:31 ` Prashant Malani
0 siblings, 0 replies; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:31 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra,
Guenter Roeck, Wolfram Sang, Jonathan Cameron, Andy Shevchenko,
Dmitry Torokhov, Akshu Agrawal,
open list:I2C SUBSYSTEM HOST DRIVERS
Replace cros_ec_cmd_xfer_status() calls with the new function
cros_ec_send_cmd_msg() which takes care of the EC message struct setup
and subsequent cleanup (which is a common pattern among users of
cros_ec_cmd_xfer_status).
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 958161c71985d9..50161dff912298 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -179,9 +179,8 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
const u16 bus_num = bus->remote_bus;
int request_len;
int response_len;
- int alloc_size;
int result;
- struct cros_ec_command *msg;
+ void *ec_buf;
request_len = ec_i2c_count_message(i2c_msgs, num);
if (request_len < 0) {
@@ -196,36 +195,32 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
return response_len;
}
- alloc_size = max(request_len, response_len);
- msg = kmalloc(sizeof(*msg) + alloc_size, GFP_KERNEL);
- if (!msg)
+ ec_buf = kmalloc(max(request_len, response_len), GFP_KERNEL);
+ if (!ec_buf)
return -ENOMEM;
- result = ec_i2c_construct_message(msg->data, i2c_msgs, num, bus_num);
+ result = ec_i2c_construct_message(ec_buf, i2c_msgs, num, bus_num);
if (result) {
dev_err(dev, "Error constructing EC i2c message %d\n", result);
goto exit;
}
- msg->version = 0;
- msg->command = EC_CMD_I2C_PASSTHRU;
- msg->outsize = request_len;
- msg->insize = response_len;
-
- result = cros_ec_cmd_xfer_status(bus->ec, msg);
+ result = cros_ec_send_cmd_msg(bus->ec, 0, EC_CMD_I2C_PASSTHRU,
+ ec_buf, request_len,
+ ec_buf, response_len);
if (result < 0) {
dev_err(dev, "Error transferring EC i2c message %d\n", result);
goto exit;
}
- result = ec_i2c_parse_response(msg->data, i2c_msgs, &num);
+ result = ec_i2c_parse_response(ec_buf, i2c_msgs, &num);
if (result < 0)
goto exit;
/* Indicate success by saying how many messages were sent */
result = num;
exit:
- kfree(msg);
+ kfree(ec_buf);
return result;
}
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH 17/17] platform/chrome: Drop cros_ec_cmd_xfer_status()
2020-01-30 20:30 ` Prashant Malani
` (16 preceding siblings ...)
(?)
@ 2020-01-30 20:31 ` Prashant Malani
2020-02-03 15:35 ` Enric Balletbo i Serra
-1 siblings, 1 reply; 38+ messages in thread
From: Prashant Malani @ 2020-01-30 20:31 UTC (permalink / raw)
To: linux-kernel
Cc: Prashant Malani, Benson Leung, Enric Balletbo i Serra,
Guenter Roeck, Lee Jones, Gwendal Grignou, Jonathan Cameron,
Evan Green
Remove cros_ec_cmd_xfer_status() since all usages of that function
have been converted to cros_ec_send_cmd_msg().
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---
drivers/platform/chrome/cros_ec_proto.c | 42 +++++----------------
include/linux/platform_data/cros_ec_proto.h | 3 --
2 files changed, 9 insertions(+), 36 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index efd1c0b6a830c8..8b97702ba97393 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -542,35 +542,6 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
}
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.
- *
- * This function is identical to cros_ec_cmd_xfer, except it returns success
- * status only if both the command was transmitted successfully and the EC
- * replied with success status. It's not necessary to check msg->result when
- * using this function.
- *
- * Return: The number of bytes transferred on success or negative error code.
- */
-int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg)
-{
- int ret;
-
- ret = cros_ec_cmd_xfer(ec_dev, msg);
- if (ret < 0) {
- dev_err(ec_dev->dev, "Command xfer error (err:%d)\n", ret);
- } else if (msg->result != EC_RES_SUCCESS) {
- dev_dbg(ec_dev->dev, "Command result (err: %d)\n", msg->result);
- return -EPROTO;
- }
-
- return ret;
-}
-EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
-
/**
* cros_ec_send_cmd_msg() - Utility function to send commands to ChromeOS EC.
* @ec: EC device struct.
@@ -581,10 +552,15 @@ EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
* @indata: Data to be received from the EC.
* @insize: Size of the &indata buffer.
*
- * This function is a wrapper around &cros_ec_cmd_xfer_status, and performs
- * some of the common work involved with sending a command to the EC. This
- * includes allocating and filling up a &struct cros_ec_command message buffer,
- * and copying the received data to another buffer.
+ * This function is similar to cros_ec_cmd_xfer(), except it returns success
+ * status only if both the command was transmitted successfully and the EC
+ * replied with success status. It's not necessary to check msg->result when
+ * using this function.
+ *
+ * It also performs some of the common work involved with sending a command to
+ * the EC. This includes allocating and filling up a
+ * &struct cros_ec_command message buffer, and copying the received data to
+ * another buffer.
*
* Return: The number of bytes transferred on success or negative error code.
*/
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 166ce26bdd79eb..851bd9af582d94 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -198,9 +198,6 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
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);
-
int cros_ec_send_cmd_msg(struct cros_ec_device *ec_dev, unsigned int version,
unsigned int command, void *outdata,
unsigned int outsize, void *indata,
--
2.25.0.341.g760bfbb309-goog
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 17/17] platform/chrome: Drop cros_ec_cmd_xfer_status()
2020-01-30 20:31 ` [PATCH 17/17] platform/chrome: Drop cros_ec_cmd_xfer_status() Prashant Malani
@ 2020-02-03 15:35 ` Enric Balletbo i Serra
0 siblings, 0 replies; 38+ messages in thread
From: Enric Balletbo i Serra @ 2020-02-03 15:35 UTC (permalink / raw)
To: Prashant Malani, linux-kernel
Cc: Benson Leung, Guenter Roeck, Lee Jones, Gwendal Grignou,
Jonathan Cameron, Evan Green
On 30/1/20 21:31, Prashant Malani wrote:
> Remove cros_ec_cmd_xfer_status() since all usages of that function
> have been converted to cros_ec_send_cmd_msg().
>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> ---
> drivers/platform/chrome/cros_ec_proto.c | 42 +++++----------------
> include/linux/platform_data/cros_ec_proto.h | 3 --
> 2 files changed, 9 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index efd1c0b6a830c8..8b97702ba97393 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -542,35 +542,6 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> }
> 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.
> - *
> - * This function is identical to cros_ec_cmd_xfer, except it returns success
> - * status only if both the command was transmitted successfully and the EC
> - * replied with success status. It's not necessary to check msg->result when
> - * using this function.
> - *
> - * Return: The number of bytes transferred on success or negative error code.
> - */
> -int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> - struct cros_ec_command *msg)
> -{
> - int ret;
> -
> - ret = cros_ec_cmd_xfer(ec_dev, msg);
> - if (ret < 0) {
> - dev_err(ec_dev->dev, "Command xfer error (err:%d)\n", ret);
> - } else if (msg->result != EC_RES_SUCCESS) {
> - dev_dbg(ec_dev->dev, "Command result (err: %d)\n", msg->result);
> - return -EPROTO;
> - }
> -
> - return ret;
> -}
> -EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
> -
> /**
> * cros_ec_send_cmd_msg() - Utility function to send commands to ChromeOS EC.
> * @ec: EC device struct.
> @@ -581,10 +552,15 @@ EXPORT_SYMBOL(cros_ec_cmd_xfer_status);
> * @indata: Data to be received from the EC.
> * @insize: Size of the &indata buffer.
> *
> - * This function is a wrapper around &cros_ec_cmd_xfer_status, and performs
> - * some of the common work involved with sending a command to the EC. This
> - * includes allocating and filling up a &struct cros_ec_command message buffer,
> - * and copying the received data to another buffer.
> + * This function is similar to cros_ec_cmd_xfer(), except it returns success
> + * status only if both the command was transmitted successfully and the EC
> + * replied with success status. It's not necessary to check msg->result when
> + * using this function.
> + *
> + * It also performs some of the common work involved with sending a command to
> + * the EC. This includes allocating and filling up a
> + * &struct cros_ec_command message buffer, and copying the received data to
> + * another buffer.
As I said, explain at the first chance for what is this helper, so you don't
need to tweak the description here.
> *
> * Return: The number of bytes transferred on success or negative error code.
> */
> diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
> index 166ce26bdd79eb..851bd9af582d94 100644
> --- a/include/linux/platform_data/cros_ec_proto.h
> +++ b/include/linux/platform_data/cros_ec_proto.h
> @@ -198,9 +198,6 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
> 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);
> -
> int cros_ec_send_cmd_msg(struct cros_ec_device *ec_dev, unsigned int version,
> unsigned int command, void *outdata,
> unsigned int outsize, void *indata,
>
^ permalink raw reply [flat|nested] 38+ messages in thread