Re: [PATCH v4] wilco_ec: Add Dell's USB PowerShare Policy control

[Nick Crews]

Many thanks Enric!

On Fri, Oct 11, 2019 at 9:08 AM Enric Balletbo i Serra
<enric.balletbo@xxxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> >> Signed-off-by: Nick Crews <ncrews@xxxxxxxxxxxx>
> >> ---
> >>
> >> 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
> >>