Hi Nick, Thanks for the patch, some few comments below...
Missatge de Nick Crews <ncr...@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 <ncr...@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 >