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

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

[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 <ncrews@chromium.org>
---
 .../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 000000000000..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 000000000000..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 <linux/platform_data/wilco-ec.h>
+#include <linux/sysfs.h>
+
+#define CMD_KB_CMOS			0x7C
+#define SUB_CMD_KB_CMOS_AUTO_ON		0x03
+
+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, &val);
+	if (ret < 0)
+		return ret;
+	if (val > 1)
+		return -EINVAL;
+
+	memset(&rq, 0, sizeof(rq));
+	rq.cmd = CMD_KB_CMOS;
+	rq.sub_cmd = SUB_CMD_KB_CMOS_AUTO_ON;
+	rq.val = val;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.type = WILCO_EC_MSG_LEGACY;
+	msg.request_data = &rq;
+	msg.request_size = sizeof(rq);
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_WO(boot_on_ac);
+
+static struct attribute *wilco_dev_attrs[] = {
+	&dev_attr_boot_on_ac.attr,
+	NULL,
+};
+
+static struct attribute_group wilco_dev_attr_group = {
+	.attrs = wilco_dev_attrs,
+};
+
+int wilco_ec_add_sysfs(struct wilco_ec_device *ec)
+{
+	return sysfs_create_group(&ec->dev->kobj, &wilco_dev_attr_group);
+}
+
+void wilco_ec_remove_sysfs(struct wilco_ec_device *ec)
+{
+	sysfs_create_group(&ec->dev->kobj, &wilco_dev_attr_group);
+}
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
index 1ff224793c99..7809b098ce27 100644
--- a/include/linux/platform_data/wilco-ec.h
+++ b/include/linux/platform_data/wilco-ec.h
@@ -123,4 +123,16 @@ struct wilco_ec_message {
  */
 int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
 
+/**
+ * wilco_ec_add_sysfs() - Create sysfs entries
+ * @ec: Wilco EC device
+ *
+ * wilco_ec_remove_sysfs() needs to be called afterwards
+ * to perform the necessary cleanup.
+ *
+ * Return: 0 on success or negative error code on failure.
+ */
+int wilco_ec_add_sysfs(struct wilco_ec_device *ec);
+void wilco_ec_remove_sysfs(struct wilco_ec_device *ec);
+
 #endif /* WILCO_EC_H */
-- 
2.20.1

[PATCH v3 2/2] platform/chrome: wilco_ec: Add USB PowerShare Policy control

[Nick Crews]

USB PowerShare is a policy which affects charging via the special
USB PowerShare port (marked with a small lightning bolt or battery icon)
when in low power states:
- In S0, the port will always provide power.
- In S0ix, if power_share is enabled, then power will be supplied to
  the port when on AC or if battery is > 50%. Else no power is supplied.
- In S5, if power_share is enabled, then power will be supplied to
  the port when on AC. Else no power is supplied.

v3 changes:
- Drop a silly blank line
- Use val > 1 instead of val != 0 && val != 1
v2 changes:
- Move documentation to Documentation/ABI/testing/sysfs-platform-wilco-ec
- Zero out reserved bytes in requests.

Signed-off-by: Nick Crews <ncrews@chromium.org>
---
 .../ABI/testing/sysfs-platform-wilco-ec       | 16 ++++
 drivers/platform/chrome/wilco_ec/sysfs.c      | 92 +++++++++++++++++++
 2 files changed, 108 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-wilco-ec b/Documentation/ABI/testing/sysfs-platform-wilco-ec
index e074c203cd32..0c07d5e0b737 100644
--- a/Documentation/ABI/testing/sysfs-platform-wilco-ec
+++ b/Documentation/ABI/testing/sysfs-platform-wilco-ec
@@ -9,3 +9,19 @@ Description:
 		Input should be parseable by kstrtou8() to 0 or 1.
 		Output will be either "0\n" or "1\n".
 
+What:		/sys/bus/platform/devices/GOOG000C\:00/usb_power_share
+Date:		April 2019
+KernelVersion:	5.2
+Description:
+		Control the USB PowerShare Policy. USB PowerShare is a policy
+		which affects charging via the special USB PowerShare port
+		(marked with a small lightning bolt or battery icon) when in
+		low power states:
+		- In S0, the port will always provide power.
+		- In S0ix, if power_share is enabled, then power will be
+		  supplied to the port when on AC or if battery is > 50%.
+		  Else no power is supplied.
+		- In S5, if power_share is enabled, then power will be supplied
+		  to the port when on AC. Else no power is supplied.
+
+		Input should be either "0" or "1".
diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
index 959b5da2eb16..14e1eee95d42 100644
--- a/drivers/platform/chrome/wilco_ec/sysfs.c
+++ b/drivers/platform/chrome/wilco_ec/sysfs.c
@@ -23,6 +23,26 @@ struct boot_on_ac_request {
 	u8 reserved7;
 } __packed;
 
+#define CMD_USB_POWER_SHARE 0x39
+
+enum usb_power_share_op {
+	POWER_SHARE_GET = 0,
+	POWER_SHARE_SET = 1,
+};
+
+struct usb_power_share_request {
+	u8 cmd;		/* Always CMD_USB_POWER_SHARE */
+	u8 reserved;
+	u8 op;		/* One of enum usb_power_share_op */
+	u8 val;		/* When setting, either 0 or 1 */
+} __packed;
+
+struct usb_power_share_response {
+	u8 reserved;
+	u8 status;	/* Set by EC to 0 on success, other value on failure */
+	u8 val;		/* When getting, set by EC to either 0 or 1 */
+} __packed;
+
 static ssize_t boot_on_ac_store(struct device *dev,
 				struct device_attribute *attr,
 				const char *buf, size_t count)
@@ -57,8 +77,80 @@ static ssize_t boot_on_ac_store(struct device *dev,
 
 static DEVICE_ATTR_WO(boot_on_ac);
 
+static int send_usb_power_share(struct wilco_ec_device *ec,
+				struct usb_power_share_request *rq,
+				struct usb_power_share_response *rs)
+{
+	struct wilco_ec_message msg;
+	int ret;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.type = WILCO_EC_MSG_LEGACY;
+	msg.request_data = rq;
+	msg.request_size = sizeof(*rq);
+	msg.response_data = rs;
+	msg.response_size = sizeof(*rs);
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0)
+		return ret;
+	if (rs->status)
+		return -EIO;
+
+	return 0;
+}
+
+static ssize_t usb_power_share_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(dev);
+	struct usb_power_share_request rq;
+	struct usb_power_share_response rs;
+	int ret;
+
+	memset(&rq, 0, sizeof(rq));
+	rq.cmd = CMD_USB_POWER_SHARE;
+	rq.op = POWER_SHARE_GET;
+
+	ret = send_usb_power_share(ec, &rq, &rs);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", rs.val);
+}
+
+static ssize_t usb_power_share_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(dev);
+	struct usb_power_share_request rq;
+	struct usb_power_share_response rs;
+	int ret;
+	u8 val;
+
+	ret = kstrtou8(buf, 10, &val);
+	if (ret < 0)
+		return ret;
+	if (val > 1)
+		return -EINVAL;
+
+	memset(&rq, 0, sizeof(rq));
+	rq.cmd = CMD_USB_POWER_SHARE;
+	rq.op = POWER_SHARE_SET;
+	rq.val = val;
+
+	ret = send_usb_power_share(ec, &rq, &rs);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(usb_power_share);
+
 static struct attribute *wilco_dev_attrs[] = {
 	&dev_attr_boot_on_ac.attr,
+	&dev_attr_usb_power_share.attr,
 	NULL,
 };
 
-- 
2.20.1

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

[Nick Crews]

There's one error that Guenter just found...

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

Should be sysfs_remove_group()

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

[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

[Enric Balletbo Serra]

Hi Nick,

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

Missatge de Nick Crews <ncrews@chromium.org> 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 <ncrews@chromium.org>
> ---
>  .../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 000000000000..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 000000000000..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 <linux/platform_data/wilco-ec.h>
> +#include <linux/sysfs.h>
> +
> +#define CMD_KB_CMOS                    0x7C
> +#define SUB_CMD_KB_CMOS_AUTO_ON                0x03
> +
> +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, &val);
> +       if (ret < 0)
> +               return ret;
> +       if (val > 1)
> +               return -EINVAL;
> +
> +       memset(&rq, 0, sizeof(rq));
> +       rq.cmd = CMD_KB_CMOS;
> +       rq.sub_cmd = SUB_CMD_KB_CMOS_AUTO_ON;
> +       rq.val = val;
> +
> +       memset(&msg, 0, sizeof(msg));
> +       msg.type = WILCO_EC_MSG_LEGACY;
> +       msg.request_data = &rq;
> +       msg.request_size = sizeof(rq);
> +       ret = wilco_ec_mailbox(ec, &msg);
> +       if (ret < 0)
> +               return ret;
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR_WO(boot_on_ac);

Is not possible to read the flag? From the API description seems that it is.

> +
> +static struct attribute *wilco_dev_attrs[] = {
> +       &dev_attr_boot_on_ac.attr,
> +       NULL,
> +};
> +
> +static struct attribute_group wilco_dev_attr_group = {
> +       .attrs = wilco_dev_attrs,
> +};
> +
> +int wilco_ec_add_sysfs(struct wilco_ec_device *ec)
> +{
> +       return sysfs_create_group(&ec->dev->kobj, &wilco_dev_attr_group);
> +}
> +
> +void wilco_ec_remove_sysfs(struct wilco_ec_device *ec)
> +{
> +       sysfs_create_group(&ec->dev->kobj, &wilco_dev_attr_group);

As Guenter pointed:  sysfs_remove_group()

> +}
> diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> index 1ff224793c99..7809b098ce27 100644
> --- a/include/linux/platform_data/wilco-ec.h
> +++ b/include/linux/platform_data/wilco-ec.h
> @@ -123,4 +123,16 @@ struct wilco_ec_message {
>   */
>  int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
>
> +/**
> + * wilco_ec_add_sysfs() - Create sysfs entries
> + * @ec: Wilco EC device
> + *
> + * wilco_ec_remove_sysfs() needs to be called afterwards
> + * to perform the necessary cleanup.
> + *
> + * Return: 0 on success or negative error code on failure.
> + */
> +int wilco_ec_add_sysfs(struct wilco_ec_device *ec);
> +void wilco_ec_remove_sysfs(struct wilco_ec_device *ec);
> +
>  #endif /* WILCO_EC_H */
> --
> 2.20.1
>

Re: [PATCH v3 2/2] platform/chrome: wilco_ec: Add USB PowerShare Policy control

[Enric Balletbo Serra]

Hi Nick,

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

Missatge de Nick Crews <ncrews@chromium.org> del dia dc., 17 d’abr.
2019 a les 3:21:
>
> USB PowerShare is a policy which affects charging via the special
> USB PowerShare port (marked with a small lightning bolt or battery icon)
> when in low power states:
> - In S0, the port will always provide power.
> - In S0ix, if power_share is enabled, then power will be supplied to
>   the port when on AC or if battery is > 50%. Else no power is supplied.
> - In S5, if power_share is enabled, then power will be supplied to
>   the port when on AC. Else no power is supplied.
>
> v3 changes:
> - Drop a silly blank line
> - Use val > 1 instead of val != 0 && val != 1
> v2 changes:
> - Move documentation to Documentation/ABI/testing/sysfs-platform-wilco-ec
> - Zero out reserved bytes in requests.
>
> Signed-off-by: Nick Crews <ncrews@chromium.org>
> ---
>  .../ABI/testing/sysfs-platform-wilco-ec       | 16 ++++
>  drivers/platform/chrome/wilco_ec/sysfs.c      | 92 +++++++++++++++++++
>  2 files changed, 108 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-wilco-ec b/Documentation/ABI/testing/sysfs-platform-wilco-ec
> index e074c203cd32..0c07d5e0b737 100644
> --- a/Documentation/ABI/testing/sysfs-platform-wilco-ec
> +++ b/Documentation/ABI/testing/sysfs-platform-wilco-ec
> @@ -9,3 +9,19 @@ Description:
>                 Input should be parseable by kstrtou8() to 0 or 1.
>                 Output will be either "0\n" or "1\n".
>
> +What:          /sys/bus/platform/devices/GOOG000C\:00/usb_power_share
> +Date:          April 2019
> +KernelVersion: 5.2
> +Description:
> +               Control the USB PowerShare Policy. USB PowerShare is a policy
> +               which affects charging via the special USB PowerShare port
> +               (marked with a small lightning bolt or battery icon) when in
> +               low power states:
> +               - In S0, the port will always provide power.
> +               - In S0ix, if power_share is enabled, then power will be
> +                 supplied to the port when on AC or if battery is > 50%.
> +                 Else no power is supplied.
> +               - In S5, if power_share is enabled, then power will be supplied
> +                 to the port when on AC. Else no power is supplied.
> +

So, basically, this is to enable/disable USB Powershare capability. I
think that this is not an uncommon capability can we give it a respin
and see if we can do this generic?

> +               Input should be either "0" or "1".
> diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
> index 959b5da2eb16..14e1eee95d42 100644
> --- a/drivers/platform/chrome/wilco_ec/sysfs.c
> +++ b/drivers/platform/chrome/wilco_ec/sysfs.c
> @@ -23,6 +23,26 @@ struct boot_on_ac_request {
>         u8 reserved7;
>  } __packed;
>
> +#define CMD_USB_POWER_SHARE 0x39
> +
> +enum usb_power_share_op {
> +       POWER_SHARE_GET = 0,
> +       POWER_SHARE_SET = 1,
> +};
> +
> +struct usb_power_share_request {
> +       u8 cmd;         /* Always CMD_USB_POWER_SHARE */
> +       u8 reserved;
> +       u8 op;          /* One of enum usb_power_share_op */
> +       u8 val;         /* When setting, either 0 or 1 */
> +} __packed;
> +
> +struct usb_power_share_response {
> +       u8 reserved;
> +       u8 status;      /* Set by EC to 0 on success, other value on failure */
> +       u8 val;         /* When getting, set by EC to either 0 or 1 */
> +} __packed;
> +
>  static ssize_t boot_on_ac_store(struct device *dev,
>                                 struct device_attribute *attr,
>                                 const char *buf, size_t count)
> @@ -57,8 +77,80 @@ static ssize_t boot_on_ac_store(struct device *dev,
>
>  static DEVICE_ATTR_WO(boot_on_ac);
>
> +static int send_usb_power_share(struct wilco_ec_device *ec,
> +                               struct usb_power_share_request *rq,
> +                               struct usb_power_share_response *rs)
> +{
> +       struct wilco_ec_message msg;
> +       int ret;
> +
> +       memset(&msg, 0, sizeof(msg));
> +       msg.type = WILCO_EC_MSG_LEGACY;
> +       msg.request_data = rq;
> +       msg.request_size = sizeof(*rq);
> +       msg.response_data = rs;
> +       msg.response_size = sizeof(*rs);
> +       ret = wilco_ec_mailbox(ec, &msg);
> +       if (ret < 0)
> +               return ret;
> +       if (rs->status)
> +               return -EIO;
> +
> +       return 0;
> +}
> +
> +static ssize_t usb_power_share_show(struct device *dev,
> +                                   struct device_attribute *attr, char *buf)
> +{
> +       struct wilco_ec_device *ec = dev_get_drvdata(dev);
> +       struct usb_power_share_request rq;
> +       struct usb_power_share_response rs;
> +       int ret;
> +
> +       memset(&rq, 0, sizeof(rq));
> +       rq.cmd = CMD_USB_POWER_SHARE;
> +       rq.op = POWER_SHARE_GET;
> +
> +       ret = send_usb_power_share(ec, &rq, &rs);
> +       if (ret < 0)
> +               return ret;
> +
> +       return sprintf(buf, "%d\n", rs.val);
> +}
> +
> +static ssize_t usb_power_share_store(struct device *dev,
> +                                    struct device_attribute *attr,
> +                                    const char *buf, size_t count)
> +{
> +       struct wilco_ec_device *ec = dev_get_drvdata(dev);
> +       struct usb_power_share_request rq;
> +       struct usb_power_share_response rs;
> +       int ret;
> +       u8 val;
> +
> +       ret = kstrtou8(buf, 10, &val);
> +       if (ret < 0)
> +               return ret;
> +       if (val > 1)
> +               return -EINVAL;
> +
> +       memset(&rq, 0, sizeof(rq));
> +       rq.cmd = CMD_USB_POWER_SHARE;
> +       rq.op = POWER_SHARE_SET;
> +       rq.val = val;
> +
> +       ret = send_usb_power_share(ec, &rq, &rs);
> +       if (ret < 0)
> +               return ret;
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR_RW(usb_power_share);
> +
>  static struct attribute *wilco_dev_attrs[] = {
>         &dev_attr_boot_on_ac.attr,
> +       &dev_attr_usb_power_share.attr,
>         NULL,
>  };
>
> --
> 2.20.1
>

Thanks,
 Enric

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

[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(&ec->dev->kobj, &wilco_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

[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(&ec->dev->kobj, &wilco_dev_attr_group);
>>
>> As Guenter pointed:  sysfs_remove_group()
> 
> Yes, exactly.
>