Re: [PATCH v3 1/2] platform/chrome: wilco_ec: Add Boot on AC support

2019-05-15 Thread Enric Balletbo i Serra 
Hi,

On 10/5/19 18:09, Nick Crews wrote:
> Thanks for the review Enric!
> 
> I can resend the patch with the fixes, or if you think the fixes are
> simple enough, you could tweak them as you apply them. Let
> me know if you want me to resend a clean version.
> 

No need to resend, I tweaked them.

The patch is queued to the for-next branch for the autobuilders to play with, if
all goes well I'll add the patch for 5.3 when current merge window closes.

Thanks,
 Enric


>>> +
>>> +static DEVICE_ATTR_WO(boot_on_ac);
>>
>> Is not possible to read the flag? From the API description seems that it is.
> 
> It is not possible to read the flag. The API description is wrong,
> I'll fix remove
> the line about reading from the documentation.
> 
>>> +void wilco_ec_remove_sysfs(struct wilco_ec_device *ec)
>>> +{
>>> +   sysfs_create_group(>dev->kobj, _dev_attr_group);
>>
>> As Guenter pointed:  sysfs_remove_group()
> 
> Yes, exactly.
>

Re: [PATCH v3 1/2] platform/chrome: wilco_ec: Add Boot on AC support

2019-05-10 Thread Nick Crews 
Thanks for the review Enric!

I can resend the patch with the fixes, or if you think the fixes are
simple enough, you could tweak them as you apply them. Let
me know if you want me to resend a clean version.

> > +
> > +static DEVICE_ATTR_WO(boot_on_ac);
>
> Is not possible to read the flag? From the API description seems that it is.

It is not possible to read the flag. The API description is wrong,
I'll fix remove
the line about reading from the documentation.

> > +void wilco_ec_remove_sysfs(struct wilco_ec_device *ec)
> > +{
> > +   sysfs_create_group(>dev->kobj, _dev_attr_group);
>
> As Guenter pointed:  sysfs_remove_group()

Yes, exactly.

Re: [PATCH v3 1/2] platform/chrome: wilco_ec: Add Boot on AC support

2019-05-09 Thread Enric Balletbo Serra 
Hi Nick,

Thanks for the patch, some few comments below...

Missatge de Nick Crews  del dia dc., 17 d’abr.
2019 a les 3:22:
>
> Boot on AC is a policy which makes the device boot from S5 when AC
> power is connected. This is useful for users who want to run their
> device headless or with a dock.
>
> v3 changes:
> - Add docstring to wilco_ec_add_sysfs()
> - Tweak a comment
> - Use val > 1 instead of val != 0 && val != 1
> v2 changes:
> - Move documentation to Documentation/ABI/testing/sysfs-platform-wilco-ec
>
> Signed-off-by: Nick Crews 
> ---
>  .../ABI/testing/sysfs-platform-wilco-ec   | 11 +++
>  drivers/platform/chrome/wilco_ec/Makefile |  2 +-
>  drivers/platform/chrome/wilco_ec/core.c   |  9 +++
>  drivers/platform/chrome/wilco_ec/sysfs.c  | 77 +++
>  include/linux/platform_data/wilco-ec.h| 12 +++
>  5 files changed, 110 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-platform-wilco-ec
>  create mode 100644 drivers/platform/chrome/wilco_ec/sysfs.c
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-wilco-ec 
> b/Documentation/ABI/testing/sysfs-platform-wilco-ec
> new file mode 100644
> index ..e074c203cd32
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-wilco-ec
> @@ -0,0 +1,11 @@
> +What:  /sys/bus/platform/devices/GOOG000C\:00/boot_on_ac
> +Date:  April 2019
> +KernelVersion: 5.2
> +Description:
> +   Boot on AC is a policy which makes the device boot from S5
> +   when AC power is connected. This is useful for users who
> +   want to run their device headless or with a dock.
> +
> +   Input should be parseable by kstrtou8() to 0 or 1.
> +   Output will be either "0\n" or "1\n".
> +
> diff --git a/drivers/platform/chrome/wilco_ec/Makefile 
> b/drivers/platform/chrome/wilco_ec/Makefile
> index 063e7fb4ea17..1281dd7737c4 100644
> --- a/drivers/platform/chrome/wilco_ec/Makefile
> +++ b/drivers/platform/chrome/wilco_ec/Makefile
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>
> -wilco_ec-objs  := core.o mailbox.o
> +wilco_ec-objs  := core.o mailbox.o sysfs.o
>  obj-$(CONFIG_WILCO_EC) += wilco_ec.o
>  wilco_ec_debugfs-objs  := debugfs.o
>  obj-$(CONFIG_WILCO_EC_DEBUGFS) += wilco_ec_debugfs.o
> diff --git a/drivers/platform/chrome/wilco_ec/core.c 
> b/drivers/platform/chrome/wilco_ec/core.c
> index 05e1e2be1c91..abd15d04e57b 100644
> --- a/drivers/platform/chrome/wilco_ec/core.c
> +++ b/drivers/platform/chrome/wilco_ec/core.c
> @@ -89,8 +89,16 @@ static int wilco_ec_probe(struct platform_device *pdev)
> goto unregister_debugfs;
> }
>
> +   ret = wilco_ec_add_sysfs(ec);
> +   if (ret < 0) {
> +   dev_err(dev, "Failed to create sysfs entries: %d", ret);
> +   goto unregister_rtc;
> +   }
> +
> return 0;
>
> +unregister_rtc:
> +   platform_device_unregister(ec->rtc_pdev);
>  unregister_debugfs:
> if (ec->debugfs_pdev)
> platform_device_unregister(ec->debugfs_pdev);
> @@ -102,6 +110,7 @@ static int wilco_ec_remove(struct platform_device *pdev)
>  {
> struct wilco_ec_device *ec = platform_get_drvdata(pdev);
>
> +   wilco_ec_remove_sysfs(ec);
> platform_device_unregister(ec->rtc_pdev);
> if (ec->debugfs_pdev)
> platform_device_unregister(ec->debugfs_pdev);
> diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c 
> b/drivers/platform/chrome/wilco_ec/sysfs.c
> new file mode 100644
> index ..959b5da2eb16
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/sysfs.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 Google LLC
> + *
> + * Sysfs properties to view and modify EC-controlled features on Wilco 
> devices.
> + * The entries will appear under /sys/bus/platform/devices/GOOG000C:00/
> + *
> + * See Documentation/ABI/testing/sysfs-platform-wilco-ec for more 
> information.
> + */
> +
> +#include 
> +#include 
> +
> +#define CMD_KB_CMOS0x7C
> +#define SUB_CMD_KB_CMOS_AUTO_ON0x03
> +
> +struct boot_on_ac_request {
> +   u8 cmd; /* Always CMD_KB_CMOS */
> +   u8 reserved1;
> +   u8 sub_cmd; /* Always SUB_CMD_KB_CMOS_AUTO_ON */
> +   u8 reserved3to5[3];
> +   u8 val; /* Either 0 or 1 */
> +   u8 reserved7;
> +} __packed;
> +
> +static ssize_t boot_on_ac_store(struct device *dev,
> +   struct device_attribute *attr,
> +   const char *buf, size_t count)
> +{
> +   struct wilco_ec_device *ec = dev_get_drvdata(dev);
> +   struct boot_on_ac_request rq;
> +   struct wilco_ec_message msg;
> +   int ret;
> +   u8 val;
> +
> +   ret = kstrtou8(buf, 10, );
> +   if (ret <

Re: [PATCH v3 1/2] platform/chrome: wilco_ec: Add Boot on AC support

2019-05-01 Thread Nick Crews 
Hi Enric,

Are these two patches an acceptable use of sysfs? There were concerns
earlier about abusing sysfs, but I think that these two uses follow
other sysfs use-cases well.

Thanks,
Nick

Re: [PATCH v3 1/2] platform/chrome: wilco_ec: Add Boot on AC support

2019-04-18 Thread Nick Crews 
There's one error that Guenter just found...

> +
> +int wilco_ec_add_sysfs(struct wilco_ec_device *ec)
> +{
> +   return sysfs_create_group(>dev->kobj, _dev_attr_group);
> +}
> +
> +void wilco_ec_remove_sysfs(struct wilco_ec_device *ec)
> +{
> +   sysfs_create_group(>dev->kobj, _dev_attr_group);

Should be sysfs_remove_group()

[PATCH v3 1/2] platform/chrome: wilco_ec: Add Boot on AC support

2019-04-16 Thread Nick Crews 
Boot on AC is a policy which makes the device boot from S5 when AC
power is connected. This is useful for users who want to run their
device headless or with a dock.

v3 changes:
- Add docstring to wilco_ec_add_sysfs()
- Tweak a comment
- Use val > 1 instead of val != 0 && val != 1
v2 changes:
- Move documentation to Documentation/ABI/testing/sysfs-platform-wilco-ec

Signed-off-by: Nick Crews 
---
 .../ABI/testing/sysfs-platform-wilco-ec   | 11 +++
 drivers/platform/chrome/wilco_ec/Makefile |  2 +-
 drivers/platform/chrome/wilco_ec/core.c   |  9 +++
 drivers/platform/chrome/wilco_ec/sysfs.c  | 77 +++
 include/linux/platform_data/wilco-ec.h| 12 +++
 5 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-wilco-ec
 create mode 100644 drivers/platform/chrome/wilco_ec/sysfs.c

diff --git a/Documentation/ABI/testing/sysfs-platform-wilco-ec 
b/Documentation/ABI/testing/sysfs-platform-wilco-ec
new file mode 100644
index ..e074c203cd32
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-wilco-ec
@@ -0,0 +1,11 @@
+What:  /sys/bus/platform/devices/GOOG000C\:00/boot_on_ac
+Date:  April 2019
+KernelVersion: 5.2
+Description:
+   Boot on AC is a policy which makes the device boot from S5
+   when AC power is connected. This is useful for users who
+   want to run their device headless or with a dock.
+
+   Input should be parseable by kstrtou8() to 0 or 1.
+   Output will be either "0\n" or "1\n".
+
diff --git a/drivers/platform/chrome/wilco_ec/Makefile 
b/drivers/platform/chrome/wilco_ec/Makefile
index 063e7fb4ea17..1281dd7737c4 100644
--- a/drivers/platform/chrome/wilco_ec/Makefile
+++ b/drivers/platform/chrome/wilco_ec/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-wilco_ec-objs  := core.o mailbox.o
+wilco_ec-objs  := core.o mailbox.o sysfs.o
 obj-$(CONFIG_WILCO_EC) += wilco_ec.o
 wilco_ec_debugfs-objs  := debugfs.o
 obj-$(CONFIG_WILCO_EC_DEBUGFS) += wilco_ec_debugfs.o
diff --git a/drivers/platform/chrome/wilco_ec/core.c 
b/drivers/platform/chrome/wilco_ec/core.c
index 05e1e2be1c91..abd15d04e57b 100644
--- a/drivers/platform/chrome/wilco_ec/core.c
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -89,8 +89,16 @@ static int wilco_ec_probe(struct platform_device *pdev)
goto unregister_debugfs;
}
 
+   ret = wilco_ec_add_sysfs(ec);
+   if (ret < 0) {
+   dev_err(dev, "Failed to create sysfs entries: %d", ret);
+   goto unregister_rtc;
+   }
+
return 0;
 
+unregister_rtc:
+   platform_device_unregister(ec->rtc_pdev);
 unregister_debugfs:
if (ec->debugfs_pdev)
platform_device_unregister(ec->debugfs_pdev);
@@ -102,6 +110,7 @@ static int wilco_ec_remove(struct platform_device *pdev)
 {
struct wilco_ec_device *ec = platform_get_drvdata(pdev);
 
+   wilco_ec_remove_sysfs(ec);
platform_device_unregister(ec->rtc_pdev);
if (ec->debugfs_pdev)
platform_device_unregister(ec->debugfs_pdev);
diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c 
b/drivers/platform/chrome/wilco_ec/sysfs.c
new file mode 100644
index ..959b5da2eb16
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/sysfs.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Google LLC
+ *
+ * Sysfs properties to view and modify EC-controlled features on Wilco devices.
+ * The entries will appear under /sys/bus/platform/devices/GOOG000C:00/
+ *
+ * See Documentation/ABI/testing/sysfs-platform-wilco-ec for more information.
+ */
+
+#include 
+#include 
+
+#define CMD_KB_CMOS0x7C
+#define SUB_CMD_KB_CMOS_AUTO_ON0x03
+
+struct boot_on_ac_request {
+   u8 cmd; /* Always CMD_KB_CMOS */
+   u8 reserved1;
+   u8 sub_cmd; /* Always SUB_CMD_KB_CMOS_AUTO_ON */
+   u8 reserved3to5[3];
+   u8 val; /* Either 0 or 1 */
+   u8 reserved7;
+} __packed;
+
+static ssize_t boot_on_ac_store(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct wilco_ec_device *ec = dev_get_drvdata(dev);
+   struct boot_on_ac_request rq;
+   struct wilco_ec_message msg;
+   int ret;
+   u8 val;
+
+   ret = kstrtou8(buf, 10, );
+   if (ret < 0)
+   return ret;
+   if (val > 1)
+   return -EINVAL;
+
+   memset(, 0, sizeof(rq));
+   rq.cmd = CMD_KB_CMOS;
+   rq.sub_cmd = SUB_CMD_KB_CMOS_AUTO_ON;
+   rq.val = val;
+
+   memset(, 0, sizeof(msg));
+   msg.type = WILCO_EC_MSG_LEGACY;
+   msg.request_data = 
+   msg.request_size = sizeof(rq);
+   ret =