[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMHSBOV1ioE6hNtgTmvJKQ1mGBarbLsY_PmQhg1hAdqT8bdbZg@mail.gmail.com>
Date:   Tue, 20 Mar 2018 11:53:50 -0700
From:   Gwendal Grignou <gwendal@...omium.org>
To:     Enric Balletbo i Serra <enric.balletbo@...labora.com>
Cc:     Lee Jones <lee.jones@...aro.org>,
        Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        kernel@...labora.com, Linux Kernel <linux-kernel@...r.kernel.org>,
        Olof Johansson <olof@...om.net>
Subject: Re: [PATCH v3 6/6] platform/chrome: mfd/cros_ec_dev: Add sysfs entry
 to set keyboard wake lid angle
On Tue, Mar 20, 2018 at 8:51 AM, Enric Balletbo i Serra
<enric.balletbo@...labora.com> wrote:
> From: Gwendal Grignou <gwendal@...omium.org>
>
> This adds a sysfs attribute (/sys/class/chromeos/cros_ec/kb_wake_angle)
> used to set and get the keyboard wake lid angle. This attribute is
> present only if 2 accelerometers are controlled by the EC.
>
> This patch also moves the cros_ec features check before the device is
> added so the features map obtained from the EC is ready on time.
>
> Signed-off-by: Gwendal Grignou <gwendal@...omium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
> ---
>
> Changes in v3:
> - [6/6] Add Reviewed-by Andy Shevchenko
> - [6/6] Fix the code that has_kb_wake_angle in cros_ec_sensors_register().
>
> Changes in v2:
> - [6/6] Use DEVICE_ATTR_RW variant.
> - [6/6] Use one line when fits in 80 characters.
> - [6/6] Use the previous defined to_cros_ec_dev
>
>  drivers/mfd/cros_ec_dev.c               | 29 +++++-------
>  drivers/platform/chrome/cros_ec_sysfs.c | 81 +++++++++++++++++++++++++++++++++
>  include/linux/mfd/cros_ec.h             |  2 +
>  3 files changed, 95 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index e4fafdd96e5e..ceaff1cf2f65 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -305,7 +305,7 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>
>         resp = (struct ec_response_motion_sense *)msg->data;
>         sensor_num = resp->dump.sensor_count;
> -       /* Allocate 2 extra sensors in case lid angle or FIFO are needed */
> +       /* Allocate 1 extra sensors in FIFO are needed */
>         sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2),
Should be +1 instead of +2 to match the comment.
>                                GFP_KERNEL);
>         if (sensor_cells == NULL)
> @@ -362,16 +362,10 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>                 sensor_type[resp->info.type]++;
>                 id++;
>         }
> -       if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
> -               sensor_platforms[id].sensor_num = sensor_num;
>
> -               sensor_cells[id].name = "cros-ec-angle";
> -               sensor_cells[id].id = 0;
> -               sensor_cells[id].platform_data = &sensor_platforms[id];
> -               sensor_cells[id].pdata_size =
> -                       sizeof(struct cros_ec_sensor_platform);
> -               id++;
> -       }
> +       if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2)
> +               ec->has_kb_wake_angle = true;
> +
>         if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
>                 sensor_cells[id].name = "cros-ec-ring";
>                 id++;
> @@ -424,6 +418,14 @@ static int ec_device_probe(struct platform_device *pdev)
>                 goto failed;
>         }
>
> +       /* check whether this EC is a sensor hub. */
> +       if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
> +               cros_ec_sensors_register(ec);
> +
> +       /* Take control of the lightbar from the EC. */
> +       lb_manual_suspend_ctrl(ec, 1);
> +
> +       /* We can now add the sysfs class, we know which parameter to show */
>         retval = cdev_device_add(&ec->cdev, &ec->class_dev);
>         if (retval) {
>                 dev_err(dev, "cdev_device_add failed => %d\n", retval);
> @@ -433,13 +435,6 @@ static int ec_device_probe(struct platform_device *pdev)
>         if (cros_ec_debugfs_init(ec))
>                 dev_warn(dev, "failed to create debugfs directory\n");
>
> -       /* check whether this EC is a sensor hub. */
> -       if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
> -               cros_ec_sensors_register(ec);
> -
> -       /* Take control of the lightbar from the EC. */
> -       lb_manual_suspend_ctrl(ec, 1);
> -
>         return 0;
>
>  failed:
> diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
> index 78ae0d3760e4..5a6db3fe213a 100644
> --- a/drivers/platform/chrome/cros_ec_sysfs.c
> +++ b/drivers/platform/chrome/cros_ec_sysfs.c
> @@ -258,21 +258,102 @@ static ssize_t flashinfo_show(struct device *dev,
>         return ret;
>  }
>
> +/* Keyboard wake angle control */
> +static ssize_t kb_wake_angle_show(struct device *dev,
> +                                 struct device_attribute *attr, char *buf)
> +{
> +       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;
> +       int ret;
> +
> +       msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
> +       if (!msg)
> +               return -ENOMEM;
> +
> +       param = (struct ec_params_motion_sense *)msg->data;
> +       msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
> +       msg->version = 2;
> +       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);
> +       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);
> +       return ret;
> +}
> +
> +static ssize_t kb_wake_angle_store(struct device *dev,
> +                                  struct device_attribute *attr,
> +                                  const char *buf, size_t count)
> +{
> +       struct cros_ec_dev *ec = to_cros_ec_dev(dev);
> +       struct ec_params_motion_sense *param;
> +       struct cros_ec_command *msg;
> +       u16 angle;
> +       int ret;
> +
> +       ret = kstrtou16(buf, 0, &angle);
> +       if (ret)
> +               return ret;
> +
> +       msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
> +       if (!msg)
> +               return -ENOMEM;
> +
> +       param = (struct ec_params_motion_sense *)msg->data;
> +       msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
> +       msg->version = 2;
> +       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);
> +       if (ret < 0)
> +               return ret;
> +       return count;
> +}
> +
>  /* Module initialization */
>
>  static DEVICE_ATTR_RW(reboot);
>  static DEVICE_ATTR_RO(version);
>  static DEVICE_ATTR_RO(flashinfo);
> +static DEVICE_ATTR_RW(kb_wake_angle);
>
>  static struct attribute *__ec_attrs[] = {
> +       &dev_attr_kb_wake_angle.attr,
>         &dev_attr_reboot.attr,
>         &dev_attr_version.attr,
>         &dev_attr_flashinfo.attr,
>         NULL,
>  };
>
> +static umode_t cros_ec_ctrl_visible(struct kobject *kobj,
> +                                   struct attribute *a, int n)
> +{
> +       struct device *dev = container_of(kobj, struct device, kobj);
> +       struct cros_ec_dev *ec = to_cros_ec_dev(dev);
> +
> +       if (a == &dev_attr_kb_wake_angle.attr && !ec->has_kb_wake_angle)
> +               return 0;
> +
> +       return a->mode;
> +}
> +
>  struct attribute_group cros_ec_attr_group = {
>         .attrs = __ec_attrs,
> +       .is_visible = cros_ec_ctrl_visible,
>  };
>  EXPORT_SYMBOL(cros_ec_attr_group);
>
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index c61535979b8f..2d4e23c9ea0a 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -183,6 +183,7 @@ struct cros_ec_debugfs;
>   * @ec_dev: cros_ec_device structure to talk to the physical device
>   * @dev: pointer to the platform device
>   * @debug_info: cros_ec_debugfs structure for debugging information
> + * @has_kb_wake_angle: true if at least 2 accelerometer are connected to the EC.
>   * @cmd_offset: offset to apply for each command.
>   */
>  struct cros_ec_dev {
> @@ -191,6 +192,7 @@ struct cros_ec_dev {
>         struct cros_ec_device *ec_dev;
>         struct device *dev;
>         struct cros_ec_debugfs *debug_info;
> +       bool has_kb_wake_angle;
>         u16 cmd_offset;
>         u32 features[2];
>  };
> --
> 2.16.2
>
<div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 20,
2018 at 8:51 AM, Enric Balletbo i Serra <span dir="ltr"><<a
href="mailto:enric.balletbo@...labora.com"
target="_blank">enric.balletbo@...labora.com</a>></span>
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">From: Gwendal
Grignou <<a href="mailto:gwendal@...omium.org">gwendal@...omium.org</a>><br>
<br>
This adds a sysfs attribute (/sys/class/chromeos/cros_ec/<wbr>kb_wake_angle)<br>
used to set and get the keyboard wake lid angle. This attribute is<br>
present only if 2 accelerometers are controlled by the EC.<br>
<br>
This patch also moves the cros_ec features check before the device is<br>
added so the features map obtained from the EC is ready on time.<br>
<br>
Signed-off-by: Gwendal Grignou <<a
href="mailto:gwendal@...omium.org">gwendal@...omium.org</a>><br>
Signed-off-by: Enric Balletbo i Serra <<a
href="mailto:enric.balletbo@...labora.com">enric.balletbo@...labora.com</a>><br>
Reviewed-by: Andy Shevchenko <<a
href="mailto:andy.shevchenko@...il.com">andy.shevchenko@...il.com</a>><br>
---<br>
<br>
Changes in v3:<br>
- [6/6] Add Reviewed-by Andy Shevchenko<br>
- [6/6] Fix the code that has_kb_wake_angle in cros_ec_sensors_register().<br>
<br>
Changes in v2:<br>
- [6/6] Use DEVICE_ATTR_RW variant.<br>
- [6/6] Use one line when fits in 80 characters.<br>
- [6/6] Use the previous defined to_cros_ec_dev<br>
<br>
 drivers/mfd/cros_ec_dev.c         
     | 29 +++++-------<br>
 drivers/platform/chrome/cros_<wbr>ec_sysfs.c | 81
++++++++++++++++++++++++++++++<wbr>+++<br>
 include/linux/mfd/cros_ec.h         
   |  2 +<br>
 3 files changed, 95 insertions(+), 17 deletions(-)<br>
<br>
diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c<br>
index e4fafdd96e5e..ceaff1cf2f65 100644<br>
--- a/drivers/mfd/cros_ec_dev.c<br>
+++ b/drivers/mfd/cros_ec_dev.c<br>
@@ -305,7 +305,7 @@ static void cros_ec_sensors_register(<wbr>struct
cros_ec_dev *ec)<br>
<br>
        resp = (struct ec_response_motion_sense
*)msg->data;<br>
        sensor_num = resp->dump.sensor_count;<br>
-       /* Allocate 2 extra sensors in case lid
angle or FIFO are needed */<br>
+       /* Allocate 1 extra sensors in FIFO are
needed */<br>
        sensor_cells = kzalloc(sizeof(struct
mfd_cell) * (sensor_num + 2),<br>
                   
           GFP_KERNEL);<br>
        if (sensor_cells == NULL)<br>
@@ -362,16 +362,10 @@ static void cros_ec_sensors_register(<wbr>struct
cros_ec_dev *ec)<br>
               
sensor_type[resp->info.type]++<wbr>;<br>
                id++;<br>
        }<br>
-       if
(sensor_type[MOTIONSENSE_TYPE_<wbr>ACCEL] >= 2) {<br>
-             
 sensor_platforms[id].sensor_<wbr>num = sensor_num;<br>
<br>
-             
 sensor_cells[id].name = "cros-ec-angle";<br>
-             
 sensor_cells[id].id = 0;<br>
-             
 sensor_cells[id].platform_data = &sensor_platforms[id];<br>
-             
 sensor_cells[id].pdata_size =<br>
-                   
   sizeof(struct cros_ec_sensor_platform);<br>
-               id++;<br>
-       }<br>
+       if
(sensor_type[MOTIONSENSE_TYPE_<wbr>ACCEL] >= 2)<br>
+             
 ec->has_kb_wake_angle = true;<br>
+<br>
        if (cros_ec_check_features(ec,
EC_FEATURE_MOTION_SENSE_FIFO)) {<br>
               
sensor_cells[id].name = "cros-ec-ring";<br>
                id++;<br>
@@ -424,6 +418,14 @@ static int ec_device_probe(struct platform_device
*pdev)<br>
                goto failed;<br>
        }<br>
<br>
+       /* check whether this EC is a sensor hub. */<br>
+       if (cros_ec_check_features(ec,
EC_FEATURE_MOTION_SENSE))<br>
+             
 cros_ec_sensors_register(ec);<br>
+<br>
+       /* Take control of the lightbar from the EC. */<br>
+       lb_manual_suspend_ctrl(ec, 1);<br>
+<br>
+       /* We can now add the sysfs class, we know
which parameter to show */<br>
        retval = cdev_device_add(&ec->cdev,
&ec->class_dev);<br>
        if (retval) {<br>
                dev_err(dev,
"cdev_device_add failed => %d\n", retval);<br>
@@ -433,13 +435,6 @@ static int ec_device_probe(struct platform_device
*pdev)<br>
        if (cros_ec_debugfs_init(ec))<br>
                dev_warn(dev,
"failed to create debugfs directory\n");<br>
<br>
-       /* check whether this EC is a sensor hub. */<br>
-       if (cros_ec_check_features(ec,
EC_FEATURE_MOTION_SENSE))<br>
-             
 cros_ec_sensors_register(ec);<br>
-<br>
-       /* Take control of the lightbar from the EC. */<br>
-       lb_manual_suspend_ctrl(ec, 1);<br>
-<br>
        return 0;<br>
<br>
 failed:<br>
diff --git a/drivers/platform/chrome/<wbr>cros_ec_sysfs.c
b/drivers/platform/chrome/<wbr>cros_ec_sysfs.c<br>
index 78ae0d3760e4..5a6db3fe213a 100644<br>
--- a/drivers/platform/chrome/<wbr>cros_ec_sysfs.c<br>
+++ b/drivers/platform/chrome/<wbr>cros_ec_sysfs.c<br>
@@ -258,21 +258,102 @@ static ssize_t flashinfo_show(struct device *dev,<br>
        return ret;<br>
 }<br>
<br>
+/* Keyboard wake angle control */<br>
+static ssize_t kb_wake_angle_show(struct device *dev,<br>
+                   
             struct
device_attribute *attr, char *buf)<br>
+{<br>
+       struct cros_ec_dev *ec = to_cros_ec_dev(dev);<br>
+       struct ec_response_motion_sense *resp;<br>
+       struct ec_params_motion_sense *param;<br>
+       struct cros_ec_command *msg;<br>
+       int ret;<br>
+<br>
+       msg = kmalloc(sizeof(*msg) +
EC_HOST_PARAM_SIZE, GFP_KERNEL);<br>
+       if (!msg)<br>
+               return -ENOMEM;<br>
+<br>
+       param = (struct ec_params_motion_sense
*)msg->data;<br>
+       msg->command = EC_CMD_MOTION_SENSE_CMD
+ ec->cmd_offset;<br>
+       msg->version = 2;<br>
+       param->cmd = MOTIONSENSE_CMD_KB_WAKE_ANGLE;<br>
+       param->kb_wake_angle.data =
EC_MOTION_SENSE_NO_VALUE;<br>
+       msg->outsize = sizeof(*param);<br>
+       msg->insize = sizeof(*resp);<br>
+<br>
+       ret =
cros_ec_cmd_xfer_status(ec-><wbr>ec_dev, msg);<br>
+       if (ret < 0)<br>
+               goto exit;<br>
+<br>
+       resp = (struct ec_response_motion_sense
*)msg->data;<br>
+       ret = scnprintf(buf, PAGE_SIZE, "%d\n",
resp->kb_wake_angle.ret);<br>
+exit:<br>
+       kfree(msg);<br>
+       return ret;<br>
+}<br>
+<br>
+static ssize_t kb_wake_angle_store(struct device *dev,<br>
+                   
              struct
device_attribute *attr,<br>
+                   
              const char *buf,
size_t count)<br>
+{<br>
+       struct cros_ec_dev *ec = to_cros_ec_dev(dev);<br>
+       struct ec_params_motion_sense *param;<br>
+       struct cros_ec_command *msg;<br>
+       u16 angle;<br>
+       int ret;<br>
+<br>
+       ret = kstrtou16(buf, 0, &angle);<br>
+       if (ret)<br>
+               return ret;<br>
+<br>
+       msg = kmalloc(sizeof(*msg) +
EC_HOST_PARAM_SIZE, GFP_KERNEL);<br>
+       if (!msg)<br>
+               return -ENOMEM;<br>
+<br>
+       param = (struct ec_params_motion_sense
*)msg->data;<br>
+       msg->command = EC_CMD_MOTION_SENSE_CMD
+ ec->cmd_offset;<br>
+       msg->version = 2;<br>
+       param->cmd = MOTIONSENSE_CMD_KB_WAKE_ANGLE;<br>
+       param->kb_wake_angle.data = angle;<br>
+       msg->outsize = sizeof(*param);<br>
+       msg->insize = sizeof(struct
ec_response_motion_sense);<br>
+<br>
+       ret =
cros_ec_cmd_xfer_status(ec-><wbr>ec_dev, msg);<br>
+       kfree(msg);<br>
+       if (ret < 0)<br>
+               return ret;<br>
+       return count;<br>
+}<br>
+<br>
 /* Module initialization */<br>
<br>
 static DEVICE_ATTR_RW(reboot);<br>
 static DEVICE_ATTR_RO(version);<br>
 static DEVICE_ATTR_RO(flashinfo);<br>
+static DEVICE_ATTR_RW(kb_wake_angle);<br>
<br>
 static struct attribute *__ec_attrs[] = {<br>
+       &dev_attr_kb_wake_angle.attr,<br>
        &dev_attr_reboot.attr,<br>
        &dev_attr_version.attr,<br>
        &dev_attr_flashinfo.attr,<br>
        NULL,<br>
 };<br>
<br>
+static umode_t cros_ec_ctrl_visible(struct kobject *kobj,<br>
+                   
               struct
attribute *a, int n)<br>
+{<br>
+       struct device *dev = container_of(kobj,
struct device, kobj);<br>
+       struct cros_ec_dev *ec = to_cros_ec_dev(dev);<br>
+<br>
+       if (a == &dev_attr_kb_wake_angle.attr
&& !ec->has_kb_wake_angle)<br>
+               return 0;<br>
+<br>
+       return a->mode;<br>
+}<br>
+<br>
 struct attribute_group cros_ec_attr_group = {<br>
        .attrs = __ec_attrs,<br>
+       .is_visible = cros_ec_ctrl_visible,<br>
 };<br>
 EXPORT_SYMBOL(cros_ec_attr_<wbr>group);<br>
<br>
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h<br>
index c61535979b8f..2d4e23c9ea0a 100644<br>
--- a/include/linux/mfd/cros_ec.h<br>
+++ b/include/linux/mfd/cros_ec.h<br>
@@ -183,6 +183,7 @@ struct cros_ec_debugfs;<br>
  * @ec_dev: cros_ec_device structure to talk to the physical device<br>
  * @dev: pointer to the platform device<br>
  * @debug_info: cros_ec_debugfs structure for debugging information<br>
+ * @has_kb_wake_angle: true if at least 2 accelerometer are connected
to the EC.<br>
  * @cmd_offset: offset to apply for each command.<br>
  */<br>
 struct cros_ec_dev {<br>
@@ -191,6 +192,7 @@ struct cros_ec_dev {<br>
        struct cros_ec_device *ec_dev;<br>
        struct device *dev;<br>
        struct cros_ec_debugfs *debug_info;<br>
+       bool has_kb_wake_angle;<br>
        u16 cmd_offset;<br>
        u32 features[2];<br>
 };<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.16.2<br>
<br>
</font></span></blockquote></div><br></div>
Powered by blists - more mailing lists
 
