| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFqH_53_rTSH2Qt=fJdLfC7bzij3E-6DTTwEh2G=i=J7nUFN4w@mail.gmail.com>
Date: Thu, 9 May 2019 23:46:22 +0200
From: Enric Balletbo Serra <eballetbo@...il.com>
To: Nick Crews <ncrews@...omium.org>
Cc: Enric Balletbo i Serra <enric.balletbo@...labora.com>,
Benson Leung <bleung@...omium.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
dlaurie@...omium.org, Daniel Erat <derat@...gle.com>,
Dmitry Torokhov <dtor@...gle.com>,
Simon Glass <sjg@...omium.org>, bartfab@...omium.org,
lamzin@...gle.com, jchwong@...gle.com
Subject: Re: [PATCH v3 2/2] platform/chrome: wilco_ec: Add USB PowerShare
Policy control
Hi Nick,
Thanks for the patch, some few comments below...
Missatge de Nick Crews <ncrews@...omium.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@...omium.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
Powered by blists - more mailing lists