linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/3] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle
@ 2018-02-21 14:50 Enric Balletbo i Serra
  2018-02-21 15:16 ` Andy Shevchenko
  0 siblings, 1 reply; 3+ messages in thread
From: Enric Balletbo i Serra @ 2018-02-21 14:50 UTC (permalink / raw)
  To: Lee Jones, Benson Leung; +Cc: linux-kernel, kernel, groeck, gwendal

From: Gwendal Grignou <gwendal@chromium.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@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/mfd/cros_ec_dev.c               | 19 ++++----
 drivers/platform/chrome/cros_ec_sysfs.c | 83 +++++++++++++++++++++++++++++++++
 include/linux/mfd/cros_ec.h             |  1 +
 3 files changed, 94 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 9d4b74404f49..48eac38a7d62 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -485,15 +485,6 @@ static int ec_device_probe(struct platform_device *pdev)
 		goto failed;
 	}
 
-	retval = cdev_device_add(&ec->cdev, &ec->class_dev);
-	if (retval) {
-		dev_err(dev, "cdev_device_add failed => %d\n", retval);
-		goto failed;
-	}
-
-	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);
@@ -514,6 +505,16 @@ static int ec_device_probe(struct platform_device *pdev)
 	/* 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);
+		goto failed;
+	}
+
+	if (cros_ec_debugfs_init(ec))
+		dev_warn(dev, "failed to create debugfs directory\n");
+
 	return 0;
 
 failed:
diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index c03621e523a3..ae50a5342d58 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -259,21 +259,104 @@ static ssize_t show_ec_flashinfo(struct device *dev,
 	return ret;
 }
 
+/* Keyboard wake angle control */
+static ssize_t show_kb_wake_angle(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct ec_response_motion_sense *resp;
+	struct ec_params_motion_sense *param;
+	struct cros_ec_command *msg;
+	int ret;
+	struct cros_ec_dev *ec = container_of(
+			dev, struct cros_ec_dev, class_dev);
+
+	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 store_kb_wake_angle(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct ec_params_motion_sense *param;
+	struct cros_ec_command *msg;
+	int ret;
+	struct cros_ec_dev *ec = container_of(
+			dev, struct cros_ec_dev, class_dev);
+	u16 angle;
+
+	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(reboot, S_IWUSR | S_IRUGO, show_ec_reboot, store_ec_reboot);
 static DEVICE_ATTR(version, S_IRUGO, show_ec_version, NULL);
 static DEVICE_ATTR(flashinfo, S_IRUGO, show_ec_flashinfo, NULL);
+static DEVICE_ATTR(kb_wake_angle, S_IWUSR | S_IRUGO, show_kb_wake_angle,
+		   store_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 = container_of(dev, struct cros_ec_dev,
+					      class_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 804b3ddbf819..951c27333d46 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -191,6 +191,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.1

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

* Re: [PATCH 3/3] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle
  2018-02-21 14:50 [PATCH 3/3] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle Enric Balletbo i Serra
@ 2018-02-21 15:16 ` Andy Shevchenko
  2018-02-21 16:38   ` Enric Balletbo i Serra
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2018-02-21 15:16 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Lee Jones, Benson Leung, Linux Kernel Mailing List, kernel,
	groeck, gwendal

On Wed, Feb 21, 2018 at 4:50 PM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> From: Gwendal Grignou <gwendal@chromium.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.

> +/* Keyboard wake angle control */
> +static ssize_t show_kb_wake_angle(struct device *dev,
> +                                 struct device_attribute *attr, char *buf)
> +{
> +       struct ec_response_motion_sense *resp;
> +       struct ec_params_motion_sense *param;
> +       struct cros_ec_command *msg;
> +       int ret;

> +       struct cros_ec_dev *ec = container_of(
> +                       dev, struct cros_ec_dev, class_dev);

First of all, do not split lines like this.
Second, consider just to add a preparatory patch which introduces

#define to_cros_ec_dev(dev)  container_of(dev, struct cros_ec_dev, class_dev)

and use it here.

> +
> +       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);

Looks like one line.

> +exit:
> +       kfree(msg);
> +       return ret;
> +}
> +
> +static ssize_t store_kb_wake_angle(struct device *dev,
> +                                  struct device_attribute *attr,
> +                                  const char *buf, size_t count)
> +{
> +       struct ec_params_motion_sense *param;
> +       struct cros_ec_command *msg;
> +       int ret;
> +       struct cros_ec_dev *ec = container_of(
> +                       dev, struct cros_ec_dev, class_dev);
> +       u16 angle;
> +
> +       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(reboot, S_IWUSR | S_IRUGO, show_ec_reboot, store_ec_reboot);
>  static DEVICE_ATTR(version, S_IRUGO, show_ec_version, NULL);
>  static DEVICE_ATTR(flashinfo, S_IRUGO, show_ec_flashinfo, NULL);
> +static DEVICE_ATTR(kb_wake_angle, S_IWUSR | S_IRUGO, show_kb_wake_angle,
> +                  store_kb_wake_angle);

Consider to switch to

DEVICE_ATTR_RO()
DEVICE_ATTR_RW()

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH 3/3] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle
  2018-02-21 15:16 ` Andy Shevchenko
@ 2018-02-21 16:38   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 3+ messages in thread
From: Enric Balletbo i Serra @ 2018-02-21 16:38 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Lee Jones, Benson Leung, Linux Kernel Mailing List, kernel,
	groeck, gwendal

Hi Andy,

On 21/02/18 16:16, Andy Shevchenko wrote:
> On Wed, Feb 21, 2018 at 4:50 PM, Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
>> From: Gwendal Grignou <gwendal@chromium.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.
> 
>> +/* Keyboard wake angle control */
>> +static ssize_t show_kb_wake_angle(struct device *dev,
>> +                                 struct device_attribute *attr, char *buf)
>> +{
>> +       struct ec_response_motion_sense *resp;
>> +       struct ec_params_motion_sense *param;
>> +       struct cros_ec_command *msg;
>> +       int ret;
> 
>> +       struct cros_ec_dev *ec = container_of(
>> +                       dev, struct cros_ec_dev, class_dev);
> 
> First of all, do not split lines like this.
> Second, consider just to add a preparatory patch which introduces
> 
> #define to_cros_ec_dev(dev)  container_of(dev, struct cros_ec_dev, class_dev)
> 
> and use it here.
> 
>> +
>> +       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);
> 
> Looks like one line.
> 
>> +exit:
>> +       kfree(msg);
>> +       return ret;
>> +}
>> +
>> +static ssize_t store_kb_wake_angle(struct device *dev,
>> +                                  struct device_attribute *attr,
>> +                                  const char *buf, size_t count)
>> +{
>> +       struct ec_params_motion_sense *param;
>> +       struct cros_ec_command *msg;
>> +       int ret;
>> +       struct cros_ec_dev *ec = container_of(
>> +                       dev, struct cros_ec_dev, class_dev);
>> +       u16 angle;
>> +
>> +       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(reboot, S_IWUSR | S_IRUGO, show_ec_reboot, store_ec_reboot);
>>  static DEVICE_ATTR(version, S_IRUGO, show_ec_version, NULL);
>>  static DEVICE_ATTR(flashinfo, S_IRUGO, show_ec_flashinfo, NULL);
>> +static DEVICE_ATTR(kb_wake_angle, S_IWUSR | S_IRUGO, show_kb_wake_angle,
>> +                  store_kb_wake_angle);
> 
> Consider to switch to
> 
> DEVICE_ATTR_RO()
> DEVICE_ATTR_RW()
> 

I'll send a second version soon, thanks for the review.

Regards,
 Enric

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

end of thread, other threads:[~2018-02-21 16:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-21 14:50 [PATCH 3/3] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle Enric Balletbo i Serra
2018-02-21 15:16 ` Andy Shevchenko
2018-02-21 16:38   ` Enric Balletbo i Serra

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