[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHX4x84LaoPBs1ZG-k+VMQvTNS+r=TGZv0NQokOgbvNjMk1=8w@mail.gmail.com>
Date: Fri, 11 Oct 2019 11:16:42 -0600
From: Nick Crews <ncrews@...omium.org>
To: Enric Balletbo i Serra <enric.balletbo@...labora.com>
Cc: Daniel Campello <campello@...omium.org>,
LKML <linux-kernel@...r.kernel.org>,
Benson Leung <bleung@...omium.org>,
Raul E Rangel <rrangel@...omium.org>
Subject: Re: [PATCH v4] wilco_ec: Add Dell's USB PowerShare Policy control
Many thanks Enric!
On Fri, Oct 11, 2019 at 9:08 AM Enric Balletbo i Serra
<enric.balletbo@...labora.com> wrote:
>
> Hi Daniel, Nick
>
> On 9/10/19 17:00, Nick Crews wrote:
> > On Tue, Oct 8, 2019 at 4:18 PM Daniel Campello <campello@...omium.org> wrote:
> >>
> >> 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 usb_charge 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 usb_charge is enabled, then power will be supplied to
> >> the port when on AC. Else no power is supplied.
> >>
> >> Signed-off-by: Daniel Campello <campello@...omium.org>
> >> Signed-off-by: Nick Crews <ncrews@...omium.org>
> >> ---
> >>
> >> v4 changes:
> >> - Renamed from usb_power_share to usb_charge to match existing feature
> >> in other platforms in the kernel (i.e., sony-laptop, samsung-laptop,
> >> lg-laptop)
> >
> > Daniel and I put in considerable effort trying to get this integrated
> > with the USB subsystem. However, it was becoming much too
> > complicated, so we hoped that if we made this more consistent
> > with the three existing examples it would be acceptable.
> >
>
> Agree, let's land as is for now. Prefixed the patch subject with
> "platform/chrome: ", replaced tabs for space in the documentation (to be
> coherent with the rest of the file) and queued for autobuilders to play with. If
> all goes well will be applied for 5.5.
>
> Thanks,
> Enric
>
>
> > Thanks for the thoughts,
> > Nick
> >
> >> 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.
> >>
> >> .../ABI/testing/sysfs-platform-wilco-ec | 17 ++++
> >> drivers/platform/chrome/wilco_ec/sysfs.c | 91 +++++++++++++++++++
> >> 2 files changed, 108 insertions(+)
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-platform-wilco-ec b/Documentation/ABI/testing/sysfs-platform-wilco-ec
> >> index 8827a734f933..bb7ba67cae97 100644
> >> --- a/Documentation/ABI/testing/sysfs-platform-wilco-ec
> >> +++ b/Documentation/ABI/testing/sysfs-platform-wilco-ec
> >> @@ -31,6 +31,23 @@ Description:
> >> Output will a version string be similar to the example below:
> >> 08B6
> >>
> >> +What: /sys/bus/platform/devices/GOOG000C\:00/usb_charge
> >> +Date: October 2019
> >> +KernelVersion: 5.5
> >> +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 usb_charge 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 usb_charge 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".
> >> +
> >> What: /sys/bus/platform/devices/GOOG000C\:00/version
> >> Date: May 2019
> >> KernelVersion: 5.3
> >> diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
> >> index 3b86a21005d3..f0d174b6bb21 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_CHARGE 0x39
> >> +
> >> +enum usb_charge_op {
> >> + USB_CHARGE_GET = 0,
> >> + USB_CHARGE_SET = 1,
> >> +};
> >> +
> >> +struct usb_charge_request {
> >> + u8 cmd; /* Always CMD_USB_CHARGE */
> >> + u8 reserved;
> >> + u8 op; /* One of enum usb_charge_op */
> >> + u8 val; /* When setting, either 0 or 1 */
> >> +} __packed;
> >> +
> >> +struct usb_charge_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;
> >> +
> >> #define CMD_EC_INFO 0x38
> >> enum get_ec_info_op {
> >> CMD_GET_EC_LABEL = 0,
> >> @@ -131,12 +151,83 @@ static ssize_t model_number_show(struct device *dev,
> >>
> >> static DEVICE_ATTR_RO(model_number);
> >>
> >> +static int send_usb_charge(struct wilco_ec_device *ec,
> >> + struct usb_charge_request *rq,
> >> + struct usb_charge_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_charge_show(struct device *dev,
> >> + struct device_attribute *attr, char *buf)
> >> +{
> >> + struct wilco_ec_device *ec = dev_get_drvdata(dev);
> >> + struct usb_charge_request rq;
> >> + struct usb_charge_response rs;
> >> + int ret;
> >> +
> >> + memset(&rq, 0, sizeof(rq));
> >> + rq.cmd = CMD_USB_CHARGE;
> >> + rq.op = USB_CHARGE_GET;
> >> +
> >> + ret = send_usb_charge(ec, &rq, &rs);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + return sprintf(buf, "%d\n", rs.val);
> >> +}
> >> +
> >> +static ssize_t usb_charge_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_charge_request rq;
> >> + struct usb_charge_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_CHARGE;
> >> + rq.op = USB_CHARGE_SET;
> >> + rq.val = val;
> >> +
> >> + ret = send_usb_charge(ec, &rq, &rs);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + return count;
> >> +}
> >> +
> >> +static DEVICE_ATTR_RW(usb_charge);
> >>
> >> static struct attribute *wilco_dev_attrs[] = {
> >> &dev_attr_boot_on_ac.attr,
> >> &dev_attr_build_date.attr,
> >> &dev_attr_build_revision.attr,
> >> &dev_attr_model_number.attr,
> >> + &dev_attr_usb_charge.attr,
> >> &dev_attr_version.attr,
> >> NULL,
> >> };
> >> --
> >> 2.23.0.581.g78d2f28ef7-goog
> >>
Powered by blists - more mailing lists