linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 1/2] platform/chrome: wilco_ec: Add property helper library
@ 2019-04-22 18:07 Nick Crews
  2019-04-22 18:07 ` [PATCH v7 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver Nick Crews
  2019-04-23 14:43 ` [PATCH v7 1/2] platform/chrome: wilco_ec: Add property helper library Enric Balletbo i Serra
  0 siblings, 2 replies; 5+ messages in thread
From: Nick Crews @ 2019-04-22 18:07 UTC (permalink / raw)
  To: enric.balletbo, bleung, sre, linux-pm
  Cc: linux-kernel, dlaurie, lamzin, bartfab, derat, dtor, sjg,
	jchwong, Nick Crews

A Property is typically a data item that is stored to NVRAM
by the EC. Each of these data items has an index associated
with it, known as the Property ID (PID). Properties may have
variable lengths, up to a max of WILCO_EC_PROPERTY_MAX_SIZE
bytes. Properties can be simple integers, or they may be more
complex binary data.

This patch adds support for getting and setting properties.
This will be useful for setting the charge algorithm and charge
schedules, which all use properties.

Signed-off-by: Nick Crews <ncrews@chromium.org>
---
v7 changes:
-Remove bogus gerrit FROMLIST tag in commit title
v6 changes:
-Add EC_* prefix to enum property_ops so they are more unique.
-Split up the commit so properties are added in a first commit
v5 changes:
-Remove OP_SYNC, it has no immediate use case.
-Merge properties.h into wilco-ec.h
-Remove enum get_set_sync_op from the public interface,
 since without OP_SYNC they are irrelevant.
-Fix Kconfigs and Makefiles so they actually work
 with the v4 changes
-Tweak some formatting, spacing, and comments
-Fix validation of charge_type so illegal values
 can't be set. Before negative error codes were
 accidentally getting casted to positive numbers
-Remove more unneeded parentheses.
v4 changes:
-Use put_unaligned_le32() to store PID in request.
-Move implementation from
 drivers/platform/chrome/wilco_ec/charge_config.c to
 drivers/power/supply/wilco_charger.c
-Move drivers/platform/chrome/wilco_ec/properties.h to
 include/linux/platform_data/wilco-ec-properties.h
-Remove parentheses in switch statement in psp_val_to_charge_mode()
-Check for any negatvie return code from psp_val_to_charge_mode()
 instead of just -EINVAL so its less brittle
-Tweak comments in wilco-ec-properties.h
v3 changes:
-Add this changelog
-Fix commit message tags
v2 changes:
-Update Documentation to say KernelVersion 5.2
-Update Documentation to explain Trickle mode better.
-rename things from using *PCC* to *CHARGE*
-Split up conversions between POWER_SUPPLY_PROP_CHARGE_TYPE values
and Wilco EC codes
-Use devm_ flavor of power_supply_register(), which simplifies things
-Add extra error checking on property messages received from the EC
-Fix bug in memcpy() calls in properties.c
-Refactor fill_property_id()
-Add valid input checks to charge_type
-Properly convert charge_type when get()ting

 drivers/platform/chrome/wilco_ec/Makefile     |   2 +-
 drivers/platform/chrome/wilco_ec/properties.c | 132 ++++++++++++++++++
 include/linux/platform_data/wilco-ec.h        |  71 ++++++++++
 3 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100644 drivers/platform/chrome/wilco_ec/properties.c

diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
index 063e7fb4ea17..29b734137786 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 properties.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/properties.c b/drivers/platform/chrome/wilco_ec/properties.c
new file mode 100644
index 000000000000..e69682c95ea2
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/properties.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Google LLC
+ */
+
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/string.h>
+#include <linux/unaligned/le_memmove.h>
+
+/* Operation code; what the EC should do with the property */
+enum ec_property_op {
+	EC_OP_GET = 0,
+	EC_OP_SET = 1,
+};
+
+struct ec_property_request {
+	u8 op; /* One of enum ec_property_op */
+	u8 property_id[4]; /* The 32 bit PID is stored Little Endian */
+	u8 length;
+	u8 data[WILCO_EC_PROPERTY_MAX_SIZE];
+} __packed;
+
+struct ec_property_response {
+	u8 reserved[2];
+	u8 op; /* One of enum ec_property_op */
+	u8 property_id[4]; /* The 32 bit PID is stored Little Endian */
+	u8 length;
+	u8 data[WILCO_EC_PROPERTY_MAX_SIZE];
+} __packed;
+
+static int send_property_msg(struct wilco_ec_device *ec,
+			     struct ec_property_request *rq,
+			     struct ec_property_response *rs)
+{
+	struct wilco_ec_message ec_msg;
+	int ret;
+
+	memset(&ec_msg, 0, sizeof(ec_msg));
+	ec_msg.type = WILCO_EC_MSG_PROPERTY;
+	ec_msg.request_data = rq;
+	ec_msg.request_size = sizeof(*rq);
+	ec_msg.response_data = rs;
+	ec_msg.response_size = sizeof(*rs);
+
+	ret = wilco_ec_mailbox(ec, &ec_msg);
+	if (ret < 0)
+		return ret;
+	if (rs->op != rq->op)
+		return -EBADMSG;
+	if (memcmp(rq->property_id, rs->property_id, sizeof(rs->property_id)))
+		return -EBADMSG;
+
+	return 0;
+}
+
+int wilco_ec_get_property(struct wilco_ec_device *ec,
+			  struct wilco_ec_property_msg *prop_msg)
+{
+	struct ec_property_request rq;
+	struct ec_property_response rs;
+	int ret;
+
+	memset(&rq, 0, sizeof(rq));
+	rq.op = EC_OP_GET;
+	put_unaligned_le32(prop_msg->property_id, rq.property_id);
+
+	ret = send_property_msg(ec, &rq, &rs);
+	if (ret < 0)
+		return ret;
+
+	prop_msg->length = rs.length;
+	memcpy(prop_msg->data, rs.data, rs.length);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wilco_ec_get_property);
+
+int wilco_ec_set_property(struct wilco_ec_device *ec,
+			  struct wilco_ec_property_msg *prop_msg)
+{
+	struct ec_property_request rq;
+	struct ec_property_response rs;
+	int ret;
+
+	memset(&rq, 0, sizeof(rq));
+	rq.op = EC_OP_SET;
+	put_unaligned_le32(prop_msg->property_id, rq.property_id);
+	rq.length = prop_msg->length;
+	memcpy(rq.data, prop_msg->data, prop_msg->length);
+
+	ret = send_property_msg(ec, &rq, &rs);
+	if (ret < 0)
+		return ret;
+	if (rs.length != prop_msg->length)
+		return -EBADMSG;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wilco_ec_set_property);
+
+int wilco_ec_get_byte_property(struct wilco_ec_device *ec, u32 property_id,
+			       u8 *val)
+{
+	struct wilco_ec_property_msg msg;
+	int ret;
+
+	msg.property_id = property_id;
+
+	ret = wilco_ec_get_property(ec, &msg);
+	if (ret < 0)
+		return ret;
+	if (msg.length != 1)
+		return -EBADMSG;
+
+	*val = msg.data[0];
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(wilco_ec_get_byte_property);
+
+int wilco_ec_set_byte_property(struct wilco_ec_device *ec, u32 property_id,
+			       u8 val)
+{
+	struct wilco_ec_property_msg msg;
+
+	msg.property_id = property_id;
+	msg.data[0] = val;
+	msg.length = 1;
+
+	return wilco_ec_set_property(ec, &msg);
+}
+EXPORT_SYMBOL_GPL(wilco_ec_set_byte_property);
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
index 1ff224793c99..50a21bd5fd44 100644
--- a/include/linux/platform_data/wilco-ec.h
+++ b/include/linux/platform_data/wilco-ec.h
@@ -123,4 +123,75 @@ struct wilco_ec_message {
  */
 int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
 
+/*
+ * A Property is typically a data item that is stored to NVRAM
+ * by the EC. Each of these data items has an index associated
+ * with it, known as the Property ID (PID). Properties may have
+ * variable lengths, up to a max of WILCO_EC_PROPERTY_MAX_SIZE
+ * bytes. Properties can be simple integers, or they may be more
+ * complex binary data.
+ */
+
+#define WILCO_EC_PROPERTY_MAX_SIZE	4
+
+/**
+ * struct ec_property_set_msg - Message to get or set a property.
+ * @property_id: Which property to get or set.
+ * @length: Number of bytes of |data| that are used.
+ * @data: Actual property data.
+ */
+struct wilco_ec_property_msg {
+	u32 property_id;
+	int length;
+	u8 data[WILCO_EC_PROPERTY_MAX_SIZE];
+};
+
+/**
+ * wilco_ec_get_property() - Retrieve a property from the EC.
+ * @ec: Embedded Controller device.
+ * @prop_msg: Message for request and response.
+ *
+ * The property_id field of |prop_msg| should be filled before calling this
+ * function. The result will be stored in the data and length fields.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int wilco_ec_get_property(struct wilco_ec_device *ec,
+			  struct wilco_ec_property_msg *prop_msg);
+
+/**
+ * wilco_ec_set_property() - Store a property on the EC.
+ * @ec: Embedded Controller device.
+ * @prop_msg: Message for request and response.
+ *
+ * The property_id, length, and data fields of |prop_msg| should be
+ * filled before calling this function.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int wilco_ec_set_property(struct wilco_ec_device *ec,
+			  struct wilco_ec_property_msg *prop_msg);
+
+/**
+ * wilco_ec_get_byte_property() - Retrieve a byte-size property from the EC.
+ * @ec: Embedded Controller device.
+ * @property_id: Which property to retrieve.
+ * @val: The result value, will be filled by this function.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int wilco_ec_get_byte_property(struct wilco_ec_device *ec, u32 property_id,
+			       u8 *val);
+
+/**
+ * wilco_ec_get_byte_property() - Store a byte-size property on the EC.
+ * @ec: Embedded Controller device.
+ * @property_id: Which property to store.
+ * @val: Value to store.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int wilco_ec_set_byte_property(struct wilco_ec_device *ec, u32 property_id,
+			       u8 val);
+
 #endif /* WILCO_EC_H */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v7 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver
  2019-04-22 18:07 [PATCH v7 1/2] platform/chrome: wilco_ec: Add property helper library Nick Crews
@ 2019-04-22 18:07 ` Nick Crews
  2019-04-23 15:01   ` Enric Balletbo i Serra
  2019-04-23 14:43 ` [PATCH v7 1/2] platform/chrome: wilco_ec: Add property helper library Enric Balletbo i Serra
  1 sibling, 1 reply; 5+ messages in thread
From: Nick Crews @ 2019-04-22 18:07 UTC (permalink / raw)
  To: enric.balletbo, bleung, sre, linux-pm
  Cc: linux-kernel, dlaurie, lamzin, bartfab, derat, dtor, sjg,
	jchwong, Nick Crews

Add control of the charging algorithm used on Wilco devices.
See Documentation/ABI/testing/sysfs-class-power-wilco for the
userspace interface and other info.

Signed-off-by: Nick Crews <ncrews@chromium.org>
---
v6 changes:
-Remove CHARGE_MODE_ILLEGAL from enum charge_mode. It's not a
 public type, and error checking could be performed in other ways.
-Split up the commit so properties are added in a first commit
-Move CONFIG_CHARGER_WILCO to the power/supply Kconfig
-Use PTR_ERR_OR_ZERO() macro in probe()
v5 changes:
-Remove OP_SYNC, it has no immediate use case.
-Merge properties.h into wilco-ec.h
-Remove enum get_set_sync_op from the public interface,
 since without OP_SYNC they are irrelevant.
-Fix Kconfigs and Makefiles so they actually work
 with the v4 changes
-Tweak some formatting, spacing, and comments
-Fix validation of charge_type so illegal values
 can't be set. Before negative error codes were
 accidentally getting casted to positive numbers
-Remove more unneeded parentheses.
v4 changes:
-Use put_unaligned_le32() to store PID in request.
-Move implementation from
 drivers/platform/chrome/wilco_ec/charge_config.c to
 drivers/power/supply/wilco_charger.c
-Move drivers/platform/chrome/wilco_ec/properties.h to
 include/linux/platform_data/wilco-ec-properties.h
-Remove parentheses in switch statement in psp_val_to_charge_mode()
-Check for any negatvie return code from psp_val_to_charge_mode()
 instead of just -EINVAL so its less brittle
-Tweak comments in wilco-ec-properties.h
v3 changes:
-Add this changelog
-Fix commit message tags
v2 changes:
-Update Documentation to say KernelVersion 5.2
-Update Documentation to explain Trickle mode better.
-rename things from using *PCC* to *CHARGE*
-Split up conversions between POWER_SUPPLY_PROP_CHARGE_TYPE values
and Wilco EC codes
-Use devm_ flavor of power_supply_register(), which simplifies things
-Add extra error checking on property messages received from the EC
-Fix bug in memcpy() calls in properties.c
-Refactor fill_property_id()
-Add valid input checks to charge_type
-Properly convert charge_type when get()ting

 .../ABI/testing/sysfs-class-power-wilco       |  30 +++
 drivers/platform/chrome/wilco_ec/core.c       |  13 ++
 drivers/power/supply/Kconfig                  |   9 +
 drivers/power/supply/Makefile                 |   1 +
 drivers/power/supply/wilco-charger.c          | 187 ++++++++++++++++++
 include/linux/platform_data/wilco-ec.h        |   2 +
 6 files changed, 242 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-power-wilco
 create mode 100644 drivers/power/supply/wilco-charger.c

diff --git a/Documentation/ABI/testing/sysfs-class-power-wilco b/Documentation/ABI/testing/sysfs-class-power-wilco
new file mode 100644
index 000000000000..7f3b01310476
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-power-wilco
@@ -0,0 +1,30 @@
+What:		/sys/class/power_supply/wilco_charger/charge_type
+Date:		April 2019
+KernelVersion:	5.2
+Description:
+		What charging algorithm to use:
+
+		Standard: Fully charges battery at a standard rate.
+		Adaptive: Battery settings adaptively optimized based on
+			typical battery usage pattern.
+		Fast: Battery charges over a shorter period.
+		Trickle: Extends battery lifespan, intended for users who
+			primarily use their Chromebook while connected to AC.
+		Custom: A low and high threshold percentage is specified.
+			Charging begins when level drops below
+			charge_control_start_threshold, and ceases when
+			level is above charge_control_end_threshold.
+
+What:		/sys/class/power_supply/wilco_charger/charge_control_start_threshold
+Date:		April 2019
+KernelVersion:	5.2
+Description:
+		Used when charge_type="Custom", as described above. Measured in
+		percentages. The valid range is [50, 95].
+
+What:		/sys/class/power_supply/wilco_charger/charge_control_end_threshold
+Date:		April 2019
+KernelVersion:	5.2
+Description:
+		Used when charge_type="Custom", as described above. Measured in
+		percentages. The valid range is [55, 100].
diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
index 05e1e2be1c91..a8e3ef59f4ea 100644
--- a/drivers/platform/chrome/wilco_ec/core.c
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -89,8 +89,20 @@ static int wilco_ec_probe(struct platform_device *pdev)
 		goto unregister_debugfs;
 	}
 
+	/* Register child device to be found by charger config driver. */
+	ec->charger_pdev = platform_device_register_data(dev, "wilco-charger",
+							 PLATFORM_DEVID_AUTO,
+							 NULL, 0);
+	if (IS_ERR(ec->charger_pdev)) {
+		dev_err(dev, "Failed to create charger platform device\n");
+		ret = PTR_ERR(ec->charger_pdev);
+		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 +114,7 @@ static int wilco_ec_remove(struct platform_device *pdev)
 {
 	struct wilco_ec_device *ec = platform_get_drvdata(pdev);
 
+	platform_device_unregister(ec->charger_pdev);
 	platform_device_unregister(ec->rtc_pdev);
 	if (ec->debugfs_pdev)
 		platform_device_unregister(ec->debugfs_pdev);
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index e901b9879e7e..a958b17b9c9d 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -660,4 +660,13 @@ config FUEL_GAUGE_SC27XX
 	 Say Y here to enable support for fuel gauge with SC27XX
 	 PMIC chips.
 
+config CHARGER_WILCO
+	tristate "Wilco EC based charger for ChromeOS"
+	depends on WILCO_EC
+	help
+	  say Y here to enable control of the charging routines performed
+	  by the Embedded Controller on the Chromebook named Wilco. Further
+	  information can be found in
+	  Documentation/ABI/testing/sysfs-class-power-wilco)
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index b731c2a9b695..2b603a142701 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -87,3 +87,4 @@ obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
 obj-$(CONFIG_CHARGER_CROS_USBPD)	+= cros_usbpd-charger.o
 obj-$(CONFIG_CHARGER_SC2731)	+= sc2731_charger.o
 obj-$(CONFIG_FUEL_GAUGE_SC27XX)	+= sc27xx_fuel_gauge.o
+obj-$(CONFIG_CHARGER_WILCO)	+= wilco-charger.o
diff --git a/drivers/power/supply/wilco-charger.c b/drivers/power/supply/wilco-charger.c
new file mode 100644
index 000000000000..7ed78c9fd828
--- /dev/null
+++ b/drivers/power/supply/wilco-charger.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Charging control driver for the Wilco EC
+ *
+ * Copyright 2019 Google LLC
+ *
+ * See Documentation/ABI/testing/sysfs-class-power and
+ * Documentation/ABI/testing/sysfs-class-power-wilco for userspace interface
+ * and other info.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/power_supply.h>
+
+#define DRV_NAME "wilco-charger"
+
+/* Property IDs and related EC constants */
+#define PID_CHARGE_MODE		0x0710
+#define PID_CHARGE_LOWER_LIMIT	0x0711
+#define PID_CHARGE_UPPER_LIMIT	0x0712
+
+enum charge_mode {
+	CHARGE_MODE_STD = 1,	/* Used for Standard */
+	CHARGE_MODE_EXP = 2,	/* Express Charge, used for Fast */
+	CHARGE_MODE_AC = 3,	/* Mostly AC use, used for Trickle */
+	CHARGE_MODE_AUTO = 4,	/* Used for Adaptive */
+	CHARGE_MODE_CUSTOM = 5,	/* Used for Custom */
+};
+
+#define CHARGE_LOWER_LIMIT_MIN	50
+#define CHARGE_LOWER_LIMIT_MAX	95
+#define CHARGE_UPPER_LIMIT_MIN	55
+#define CHARGE_UPPER_LIMIT_MAX	100
+
+/* Convert from POWER_SUPPLY_PROP_CHARGE_TYPE value to the EC's charge mode */
+static int psp_val_to_charge_mode(int psp_val)
+{
+	switch (psp_val) {
+	case POWER_SUPPLY_CHARGE_TYPE_TRICKLE:
+		return CHARGE_MODE_AC;
+	case POWER_SUPPLY_CHARGE_TYPE_FAST:
+		return CHARGE_MODE_EXP;
+	case POWER_SUPPLY_CHARGE_TYPE_STANDARD:
+		return CHARGE_MODE_STD;
+	case POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE:
+		return CHARGE_MODE_AUTO;
+	case POWER_SUPPLY_CHARGE_TYPE_CUSTOM:
+		return CHARGE_MODE_CUSTOM;
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Convert from EC's charge mode to POWER_SUPPLY_PROP_CHARGE_TYPE value */
+static int charge_mode_to_psp_val(enum charge_mode mode)
+{
+	switch (mode) {
+	case CHARGE_MODE_AC:
+		return POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+	case CHARGE_MODE_EXP:
+		return POWER_SUPPLY_CHARGE_TYPE_FAST;
+	case CHARGE_MODE_STD:
+		return POWER_SUPPLY_CHARGE_TYPE_STANDARD;
+	case CHARGE_MODE_AUTO:
+		return POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE;
+	case CHARGE_MODE_CUSTOM:
+		return POWER_SUPPLY_CHARGE_TYPE_CUSTOM;
+	default:
+		return -EINVAL;
+	}
+}
+
+static enum power_supply_property wilco_charge_props[] = {
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD,
+	POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
+};
+
+static int wilco_charge_get_property(struct power_supply *psy,
+				     enum power_supply_property psp,
+				     union power_supply_propval *val)
+{
+	struct wilco_ec_device *ec = power_supply_get_drvdata(psy);
+	u32 property_id;
+	int ret;
+	u8 raw;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		property_id = PID_CHARGE_MODE;
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
+		property_id = PID_CHARGE_LOWER_LIMIT;
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
+		property_id = PID_CHARGE_UPPER_LIMIT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = wilco_ec_get_byte_property(ec, property_id, &raw);
+	if (ret < 0)
+		return ret;
+	if (property_id == PID_CHARGE_MODE) {
+		ret = charge_mode_to_psp_val(raw);
+		if (ret < 0)
+			return -EBADMSG;
+		raw = ret;
+	}
+	val->intval = raw;
+
+	return 0;
+}
+
+static int wilco_charge_set_property(struct power_supply *psy,
+				     enum power_supply_property psp,
+				     const union power_supply_propval *val)
+{
+	struct wilco_ec_device *ec = power_supply_get_drvdata(psy);
+	int mode;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		mode = psp_val_to_charge_mode(val->intval);
+		if (mode < 0)
+			return -EINVAL;
+		return wilco_ec_set_byte_property(ec, PID_CHARGE_MODE, mode);
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
+		if (val->intval < CHARGE_LOWER_LIMIT_MIN ||
+		    val->intval > CHARGE_LOWER_LIMIT_MAX)
+			return -EINVAL;
+		return wilco_ec_set_byte_property(ec, PID_CHARGE_LOWER_LIMIT,
+						  val->intval);
+	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
+		if (val->intval < CHARGE_UPPER_LIMIT_MIN ||
+		    val->intval > CHARGE_UPPER_LIMIT_MAX)
+			return -EINVAL;
+		return wilco_ec_set_byte_property(ec, PID_CHARGE_UPPER_LIMIT,
+						  val->intval);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int wilco_charge_property_is_writeable(struct power_supply *psy,
+					      enum power_supply_property psp)
+{
+	return 1;
+}
+
+static const struct power_supply_desc wilco_ps_desc = {
+	.properties		= wilco_charge_props,
+	.num_properties		= ARRAY_SIZE(wilco_charge_props),
+	.get_property		= wilco_charge_get_property,
+	.set_property		= wilco_charge_set_property,
+	.property_is_writeable	= wilco_charge_property_is_writeable,
+	.name			= "wilco-charger",
+	.type			= POWER_SUPPLY_TYPE_MAINS,
+};
+
+static int wilco_charge_probe(struct platform_device *pdev)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+	struct power_supply_config psy_cfg = {};
+	struct power_supply *psy;
+
+	psy_cfg.drv_data = ec;
+	psy = devm_power_supply_register(&pdev->dev, &wilco_ps_desc, &psy_cfg);
+
+	return PTR_ERR_OR_ZERO(psy);
+}
+
+static struct platform_driver wilco_charge_driver = {
+	.probe	= wilco_charge_probe,
+	.driver = {
+		.name = DRV_NAME,
+	}
+};
+module_platform_driver(wilco_charge_driver);
+
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_AUTHOR("Nick Crews <ncrews@chromium.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Wilco EC charge control driver");
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
index 50a21bd5fd44..66d9f177bec2 100644
--- a/include/linux/platform_data/wilco-ec.h
+++ b/include/linux/platform_data/wilco-ec.h
@@ -32,6 +32,7 @@
  * @data_size: Size of the data buffer used for EC communication.
  * @debugfs_pdev: The child platform_device used by the debugfs sub-driver.
  * @rtc_pdev: The child platform_device used by the RTC sub-driver.
+ * @charger_pdev: Child platform_device used by the charger config sub-driver.
  */
 struct wilco_ec_device {
 	struct device *dev;
@@ -43,6 +44,7 @@ struct wilco_ec_device {
 	size_t data_size;
 	struct platform_device *debugfs_pdev;
 	struct platform_device *rtc_pdev;
+	struct platform_device *charger_pdev;
 };
 
 /**
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 1/2] platform/chrome: wilco_ec: Add property helper library
  2019-04-22 18:07 [PATCH v7 1/2] platform/chrome: wilco_ec: Add property helper library Nick Crews
  2019-04-22 18:07 ` [PATCH v7 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver Nick Crews
@ 2019-04-23 14:43 ` Enric Balletbo i Serra
  1 sibling, 0 replies; 5+ messages in thread
From: Enric Balletbo i Serra @ 2019-04-23 14:43 UTC (permalink / raw)
  To: Nick Crews, bleung, sre, linux-pm
  Cc: linux-kernel, dlaurie, lamzin, bartfab, derat, dtor, sjg, jchwong



On 22/4/19 20:07, Nick Crews wrote:
> A Property is typically a data item that is stored to NVRAM
> by the EC. Each of these data items has an index associated
> with it, known as the Property ID (PID). Properties may have
> variable lengths, up to a max of WILCO_EC_PROPERTY_MAX_SIZE
> bytes. Properties can be simple integers, or they may be more
> complex binary data.
> 
> This patch adds support for getting and setting properties.
> This will be useful for setting the charge algorithm and charge
> schedules, which all use properties.
> 
> Signed-off-by: Nick Crews <ncrews@chromium.org>

For my own reference:
  Acked-for-chrome-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

That means that this patch is pushed out as testing in for-next branch to let
the autobuilders play with it. If all goes well your patches would be in the
chrome-platform-5.2 or chrome-platform-5.3 (as we're already at rc6) branch
'soonish'.

Sebastian, note that this patch should be picked before you apply

[PATCH v7 2/2] power_supply: platform/chrome: wilco_ec: Add charging config  driver

So let me know if you plan to pick that for 5.2 or is more suitable for 5.3

Thanks,
 Enric

> ---
> v7 changes:
> -Remove bogus gerrit FROMLIST tag in commit title
> v6 changes:
> -Add EC_* prefix to enum property_ops so they are more unique.
> -Split up the commit so properties are added in a first commit
> v5 changes:
> -Remove OP_SYNC, it has no immediate use case.
> -Merge properties.h into wilco-ec.h
> -Remove enum get_set_sync_op from the public interface,
>  since without OP_SYNC they are irrelevant.
> -Fix Kconfigs and Makefiles so they actually work
>  with the v4 changes
> -Tweak some formatting, spacing, and comments
> -Fix validation of charge_type so illegal values
>  can't be set. Before negative error codes were
>  accidentally getting casted to positive numbers
> -Remove more unneeded parentheses.
> v4 changes:
> -Use put_unaligned_le32() to store PID in request.
> -Move implementation from
>  drivers/platform/chrome/wilco_ec/charge_config.c to
>  drivers/power/supply/wilco_charger.c
> -Move drivers/platform/chrome/wilco_ec/properties.h to
>  include/linux/platform_data/wilco-ec-properties.h
> -Remove parentheses in switch statement in psp_val_to_charge_mode()
> -Check for any negatvie return code from psp_val_to_charge_mode()
>  instead of just -EINVAL so its less brittle
> -Tweak comments in wilco-ec-properties.h
> v3 changes:
> -Add this changelog
> -Fix commit message tags
> v2 changes:
> -Update Documentation to say KernelVersion 5.2
> -Update Documentation to explain Trickle mode better.
> -rename things from using *PCC* to *CHARGE*
> -Split up conversions between POWER_SUPPLY_PROP_CHARGE_TYPE values
> and Wilco EC codes
> -Use devm_ flavor of power_supply_register(), which simplifies things
> -Add extra error checking on property messages received from the EC
> -Fix bug in memcpy() calls in properties.c
> -Refactor fill_property_id()
> -Add valid input checks to charge_type
> -Properly convert charge_type when get()ting
> 
>  drivers/platform/chrome/wilco_ec/Makefile     |   2 +-
>  drivers/platform/chrome/wilco_ec/properties.c | 132 ++++++++++++++++++
>  include/linux/platform_data/wilco-ec.h        |  71 ++++++++++
>  3 files changed, 204 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/platform/chrome/wilco_ec/properties.c
> 
> diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
> index 063e7fb4ea17..29b734137786 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 properties.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/properties.c b/drivers/platform/chrome/wilco_ec/properties.c
> new file mode 100644
> index 000000000000..e69682c95ea2
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/properties.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 Google LLC
> + */
> +
> +#include <linux/platform_data/wilco-ec.h>
> +#include <linux/string.h>
> +#include <linux/unaligned/le_memmove.h>
> +
> +/* Operation code; what the EC should do with the property */
> +enum ec_property_op {
> +	EC_OP_GET = 0,
> +	EC_OP_SET = 1,
> +};
> +
> +struct ec_property_request {
> +	u8 op; /* One of enum ec_property_op */
> +	u8 property_id[4]; /* The 32 bit PID is stored Little Endian */
> +	u8 length;
> +	u8 data[WILCO_EC_PROPERTY_MAX_SIZE];
> +} __packed;
> +
> +struct ec_property_response {
> +	u8 reserved[2];
> +	u8 op; /* One of enum ec_property_op */
> +	u8 property_id[4]; /* The 32 bit PID is stored Little Endian */
> +	u8 length;
> +	u8 data[WILCO_EC_PROPERTY_MAX_SIZE];
> +} __packed;
> +
> +static int send_property_msg(struct wilco_ec_device *ec,
> +			     struct ec_property_request *rq,
> +			     struct ec_property_response *rs)
> +{
> +	struct wilco_ec_message ec_msg;
> +	int ret;
> +
> +	memset(&ec_msg, 0, sizeof(ec_msg));
> +	ec_msg.type = WILCO_EC_MSG_PROPERTY;
> +	ec_msg.request_data = rq;
> +	ec_msg.request_size = sizeof(*rq);
> +	ec_msg.response_data = rs;
> +	ec_msg.response_size = sizeof(*rs);
> +
> +	ret = wilco_ec_mailbox(ec, &ec_msg);
> +	if (ret < 0)
> +		return ret;
> +	if (rs->op != rq->op)
> +		return -EBADMSG;
> +	if (memcmp(rq->property_id, rs->property_id, sizeof(rs->property_id)))
> +		return -EBADMSG;
> +
> +	return 0;
> +}
> +
> +int wilco_ec_get_property(struct wilco_ec_device *ec,
> +			  struct wilco_ec_property_msg *prop_msg)
> +{
> +	struct ec_property_request rq;
> +	struct ec_property_response rs;
> +	int ret;
> +
> +	memset(&rq, 0, sizeof(rq));
> +	rq.op = EC_OP_GET;
> +	put_unaligned_le32(prop_msg->property_id, rq.property_id);
> +
> +	ret = send_property_msg(ec, &rq, &rs);
> +	if (ret < 0)
> +		return ret;
> +
> +	prop_msg->length = rs.length;
> +	memcpy(prop_msg->data, rs.data, rs.length);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(wilco_ec_get_property);
> +
> +int wilco_ec_set_property(struct wilco_ec_device *ec,
> +			  struct wilco_ec_property_msg *prop_msg)
> +{
> +	struct ec_property_request rq;
> +	struct ec_property_response rs;
> +	int ret;
> +
> +	memset(&rq, 0, sizeof(rq));
> +	rq.op = EC_OP_SET;
> +	put_unaligned_le32(prop_msg->property_id, rq.property_id);
> +	rq.length = prop_msg->length;
> +	memcpy(rq.data, prop_msg->data, prop_msg->length);
> +
> +	ret = send_property_msg(ec, &rq, &rs);
> +	if (ret < 0)
> +		return ret;
> +	if (rs.length != prop_msg->length)
> +		return -EBADMSG;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(wilco_ec_set_property);
> +
> +int wilco_ec_get_byte_property(struct wilco_ec_device *ec, u32 property_id,
> +			       u8 *val)
> +{
> +	struct wilco_ec_property_msg msg;
> +	int ret;
> +
> +	msg.property_id = property_id;
> +
> +	ret = wilco_ec_get_property(ec, &msg);
> +	if (ret < 0)
> +		return ret;
> +	if (msg.length != 1)
> +		return -EBADMSG;
> +
> +	*val = msg.data[0];
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(wilco_ec_get_byte_property);
> +
> +int wilco_ec_set_byte_property(struct wilco_ec_device *ec, u32 property_id,
> +			       u8 val)
> +{
> +	struct wilco_ec_property_msg msg;
> +
> +	msg.property_id = property_id;
> +	msg.data[0] = val;
> +	msg.length = 1;
> +
> +	return wilco_ec_set_property(ec, &msg);
> +}
> +EXPORT_SYMBOL_GPL(wilco_ec_set_byte_property);
> diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> index 1ff224793c99..50a21bd5fd44 100644
> --- a/include/linux/platform_data/wilco-ec.h
> +++ b/include/linux/platform_data/wilco-ec.h
> @@ -123,4 +123,75 @@ struct wilco_ec_message {
>   */
>  int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
>  
> +/*
> + * A Property is typically a data item that is stored to NVRAM
> + * by the EC. Each of these data items has an index associated
> + * with it, known as the Property ID (PID). Properties may have
> + * variable lengths, up to a max of WILCO_EC_PROPERTY_MAX_SIZE
> + * bytes. Properties can be simple integers, or they may be more
> + * complex binary data.
> + */
> +
> +#define WILCO_EC_PROPERTY_MAX_SIZE	4
> +
> +/**
> + * struct ec_property_set_msg - Message to get or set a property.
> + * @property_id: Which property to get or set.
> + * @length: Number of bytes of |data| that are used.
> + * @data: Actual property data.
> + */
> +struct wilco_ec_property_msg {
> +	u32 property_id;
> +	int length;
> +	u8 data[WILCO_EC_PROPERTY_MAX_SIZE];
> +};
> +
> +/**
> + * wilco_ec_get_property() - Retrieve a property from the EC.
> + * @ec: Embedded Controller device.
> + * @prop_msg: Message for request and response.
> + *
> + * The property_id field of |prop_msg| should be filled before calling this
> + * function. The result will be stored in the data and length fields.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int wilco_ec_get_property(struct wilco_ec_device *ec,
> +			  struct wilco_ec_property_msg *prop_msg);
> +
> +/**
> + * wilco_ec_set_property() - Store a property on the EC.
> + * @ec: Embedded Controller device.
> + * @prop_msg: Message for request and response.
> + *
> + * The property_id, length, and data fields of |prop_msg| should be
> + * filled before calling this function.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int wilco_ec_set_property(struct wilco_ec_device *ec,
> +			  struct wilco_ec_property_msg *prop_msg);
> +
> +/**
> + * wilco_ec_get_byte_property() - Retrieve a byte-size property from the EC.
> + * @ec: Embedded Controller device.
> + * @property_id: Which property to retrieve.
> + * @val: The result value, will be filled by this function.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int wilco_ec_get_byte_property(struct wilco_ec_device *ec, u32 property_id,
> +			       u8 *val);
> +
> +/**
> + * wilco_ec_get_byte_property() - Store a byte-size property on the EC.
> + * @ec: Embedded Controller device.
> + * @property_id: Which property to store.
> + * @val: Value to store.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int wilco_ec_set_byte_property(struct wilco_ec_device *ec, u32 property_id,
> +			       u8 val);
> +
>  #endif /* WILCO_EC_H */
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver
  2019-04-22 18:07 ` [PATCH v7 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver Nick Crews
@ 2019-04-23 15:01   ` Enric Balletbo i Serra
  2019-05-01 16:37     ` Nick Crews
  0 siblings, 1 reply; 5+ messages in thread
From: Enric Balletbo i Serra @ 2019-04-23 15:01 UTC (permalink / raw)
  To: Nick Crews, bleung, sre, linux-pm
  Cc: linux-kernel, dlaurie, lamzin, bartfab, derat, dtor, sjg, jchwong

Sebastian,

On 22/4/19 20:07, Nick Crews wrote:
> Add control of the charging algorithm used on Wilco devices.
> See Documentation/ABI/testing/sysfs-class-power-wilco for the
> userspace interface and other info.
> 
> Signed-off-by: Nick Crews <ncrews@chromium.org>

Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

> ---
> v6 changes:
> -Remove CHARGE_MODE_ILLEGAL from enum charge_mode. It's not a
>  public type, and error checking could be performed in other ways.
> -Split up the commit so properties are added in a first commit
> -Move CONFIG_CHARGER_WILCO to the power/supply Kconfig
> -Use PTR_ERR_OR_ZERO() macro in probe()
> v5 changes:
> -Remove OP_SYNC, it has no immediate use case.
> -Merge properties.h into wilco-ec.h
> -Remove enum get_set_sync_op from the public interface,
>  since without OP_SYNC they are irrelevant.
> -Fix Kconfigs and Makefiles so they actually work
>  with the v4 changes
> -Tweak some formatting, spacing, and comments
> -Fix validation of charge_type so illegal values
>  can't be set. Before negative error codes were
>  accidentally getting casted to positive numbers
> -Remove more unneeded parentheses.
> v4 changes:
> -Use put_unaligned_le32() to store PID in request.
> -Move implementation from
>  drivers/platform/chrome/wilco_ec/charge_config.c to
>  drivers/power/supply/wilco_charger.c
> -Move drivers/platform/chrome/wilco_ec/properties.h to
>  include/linux/platform_data/wilco-ec-properties.h
> -Remove parentheses in switch statement in psp_val_to_charge_mode()
> -Check for any negatvie return code from psp_val_to_charge_mode()
>  instead of just -EINVAL so its less brittle
> -Tweak comments in wilco-ec-properties.h
> v3 changes:
> -Add this changelog
> -Fix commit message tags
> v2 changes:
> -Update Documentation to say KernelVersion 5.2
> -Update Documentation to explain Trickle mode better.
> -rename things from using *PCC* to *CHARGE*
> -Split up conversions between POWER_SUPPLY_PROP_CHARGE_TYPE values
> and Wilco EC codes
> -Use devm_ flavor of power_supply_register(), which simplifies things
> -Add extra error checking on property messages received from the EC
> -Fix bug in memcpy() calls in properties.c
> -Refactor fill_property_id()
> -Add valid input checks to charge_type
> -Properly convert charge_type when get()ting
> 
>  .../ABI/testing/sysfs-class-power-wilco       |  30 +++
>  drivers/platform/chrome/wilco_ec/core.c       |  13 ++
>  drivers/power/supply/Kconfig                  |   9 +
>  drivers/power/supply/Makefile                 |   1 +
>  drivers/power/supply/wilco-charger.c          | 187 ++++++++++++++++++
>  include/linux/platform_data/wilco-ec.h        |   2 +

This touches two subsystems. Sebastian, what is your preference to handle this?
Go all through power/supply? or chrome-platform tree? Or do you prefer have this
split between subsystems (I think is possible in this case).

All this after you're fine with it, of course.

>  6 files changed, 242 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-power-wilco
>  create mode 100644 drivers/power/supply/wilco-charger.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-power-wilco b/Documentation/ABI/testing/sysfs-class-power-wilco
> new file mode 100644
> index 000000000000..7f3b01310476
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-power-wilco
> @@ -0,0 +1,30 @@
> +What:		/sys/class/power_supply/wilco_charger/charge_type
> +Date:		April 2019
> +KernelVersion:	5.2
> +Description:
> +		What charging algorithm to use:
> +
> +		Standard: Fully charges battery at a standard rate.
> +		Adaptive: Battery settings adaptively optimized based on
> +			typical battery usage pattern.
> +		Fast: Battery charges over a shorter period.
> +		Trickle: Extends battery lifespan, intended for users who
> +			primarily use their Chromebook while connected to AC.
> +		Custom: A low and high threshold percentage is specified.
> +			Charging begins when level drops below
> +			charge_control_start_threshold, and ceases when
> +			level is above charge_control_end_threshold.
> +
> +What:		/sys/class/power_supply/wilco_charger/charge_control_start_threshold
> +Date:		April 2019
> +KernelVersion:	5.2
> +Description:
> +		Used when charge_type="Custom", as described above. Measured in
> +		percentages. The valid range is [50, 95].
> +
> +What:		/sys/class/power_supply/wilco_charger/charge_control_end_threshold
> +Date:		April 2019
> +KernelVersion:	5.2
> +Description:
> +		Used when charge_type="Custom", as described above. Measured in
> +		percentages. The valid range is [55, 100].
> diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
> index 05e1e2be1c91..a8e3ef59f4ea 100644
> --- a/drivers/platform/chrome/wilco_ec/core.c
> +++ b/drivers/platform/chrome/wilco_ec/core.c
> @@ -89,8 +89,20 @@ static int wilco_ec_probe(struct platform_device *pdev)
>  		goto unregister_debugfs;
>  	}
>  
> +	/* Register child device to be found by charger config driver. */
> +	ec->charger_pdev = platform_device_register_data(dev, "wilco-charger",
> +							 PLATFORM_DEVID_AUTO,
> +							 NULL, 0);
> +	if (IS_ERR(ec->charger_pdev)) {
> +		dev_err(dev, "Failed to create charger platform device\n");
> +		ret = PTR_ERR(ec->charger_pdev);
> +		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 +114,7 @@ static int wilco_ec_remove(struct platform_device *pdev)
>  {
>  	struct wilco_ec_device *ec = platform_get_drvdata(pdev);
>  
> +	platform_device_unregister(ec->charger_pdev);
>  	platform_device_unregister(ec->rtc_pdev);
>  	if (ec->debugfs_pdev)
>  		platform_device_unregister(ec->debugfs_pdev);
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index e901b9879e7e..a958b17b9c9d 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -660,4 +660,13 @@ config FUEL_GAUGE_SC27XX
>  	 Say Y here to enable support for fuel gauge with SC27XX
>  	 PMIC chips.
>  
> +config CHARGER_WILCO
> +	tristate "Wilco EC based charger for ChromeOS"
> +	depends on WILCO_EC
> +	help
> +	  say Y here to enable control of the charging routines performed

nit: Capitalize

> +	  by the Embedded Controller on the Chromebook named Wilco. Further
> +	  information can be found in
> +	  Documentation/ABI/testing/sysfs-class-power-wilco)

nit: Without final parentheses?

> +
>  endif # POWER_SUPPLY
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index b731c2a9b695..2b603a142701 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -87,3 +87,4 @@ obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
>  obj-$(CONFIG_CHARGER_CROS_USBPD)	+= cros_usbpd-charger.o
>  obj-$(CONFIG_CHARGER_SC2731)	+= sc2731_charger.o
>  obj-$(CONFIG_FUEL_GAUGE_SC27XX)	+= sc27xx_fuel_gauge.o
> +obj-$(CONFIG_CHARGER_WILCO)	+= wilco-charger.o
> diff --git a/drivers/power/supply/wilco-charger.c b/drivers/power/supply/wilco-charger.c
> new file mode 100644
> index 000000000000..7ed78c9fd828
> --- /dev/null
> +++ b/drivers/power/supply/wilco-charger.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Charging control driver for the Wilco EC
> + *
> + * Copyright 2019 Google LLC
> + *
> + * See Documentation/ABI/testing/sysfs-class-power and
> + * Documentation/ABI/testing/sysfs-class-power-wilco for userspace interface
> + * and other info.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/wilco-ec.h>
> +#include <linux/power_supply.h>
> +
> +#define DRV_NAME "wilco-charger"
> +
> +/* Property IDs and related EC constants */
> +#define PID_CHARGE_MODE		0x0710
> +#define PID_CHARGE_LOWER_LIMIT	0x0711
> +#define PID_CHARGE_UPPER_LIMIT	0x0712
> +
> +enum charge_mode {
> +	CHARGE_MODE_STD = 1,	/* Used for Standard */
> +	CHARGE_MODE_EXP = 2,	/* Express Charge, used for Fast */
> +	CHARGE_MODE_AC = 3,	/* Mostly AC use, used for Trickle */
> +	CHARGE_MODE_AUTO = 4,	/* Used for Adaptive */
> +	CHARGE_MODE_CUSTOM = 5,	/* Used for Custom */
> +};
> +
> +#define CHARGE_LOWER_LIMIT_MIN	50
> +#define CHARGE_LOWER_LIMIT_MAX	95
> +#define CHARGE_UPPER_LIMIT_MIN	55
> +#define CHARGE_UPPER_LIMIT_MAX	100
> +
> +/* Convert from POWER_SUPPLY_PROP_CHARGE_TYPE value to the EC's charge mode */
> +static int psp_val_to_charge_mode(int psp_val)
> +{
> +	switch (psp_val) {
> +	case POWER_SUPPLY_CHARGE_TYPE_TRICKLE:
> +		return CHARGE_MODE_AC;
> +	case POWER_SUPPLY_CHARGE_TYPE_FAST:
> +		return CHARGE_MODE_EXP;
> +	case POWER_SUPPLY_CHARGE_TYPE_STANDARD:
> +		return CHARGE_MODE_STD;
> +	case POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE:
> +		return CHARGE_MODE_AUTO;
> +	case POWER_SUPPLY_CHARGE_TYPE_CUSTOM:
> +		return CHARGE_MODE_CUSTOM;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +/* Convert from EC's charge mode to POWER_SUPPLY_PROP_CHARGE_TYPE value */
> +static int charge_mode_to_psp_val(enum charge_mode mode)
> +{
> +	switch (mode) {
> +	case CHARGE_MODE_AC:
> +		return POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> +	case CHARGE_MODE_EXP:
> +		return POWER_SUPPLY_CHARGE_TYPE_FAST;
> +	case CHARGE_MODE_STD:
> +		return POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> +	case CHARGE_MODE_AUTO:
> +		return POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE;
> +	case CHARGE_MODE_CUSTOM:
> +		return POWER_SUPPLY_CHARGE_TYPE_CUSTOM;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static enum power_supply_property wilco_charge_props[] = {
> +	POWER_SUPPLY_PROP_CHARGE_TYPE,
> +	POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD,
> +	POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
> +};
> +
> +static int wilco_charge_get_property(struct power_supply *psy,
> +				     enum power_supply_property psp,
> +				     union power_supply_propval *val)
> +{
> +	struct wilco_ec_device *ec = power_supply_get_drvdata(psy);
> +	u32 property_id;
> +	int ret;
> +	u8 raw;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
> +		property_id = PID_CHARGE_MODE;
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
> +		property_id = PID_CHARGE_LOWER_LIMIT;
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> +		property_id = PID_CHARGE_UPPER_LIMIT;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = wilco_ec_get_byte_property(ec, property_id, &raw);
> +	if (ret < 0)
> +		return ret;
> +	if (property_id == PID_CHARGE_MODE) {
> +		ret = charge_mode_to_psp_val(raw);
> +		if (ret < 0)
> +			return -EBADMSG;
> +		raw = ret;
> +	}
> +	val->intval = raw;
> +
> +	return 0;
> +}
> +
> +static int wilco_charge_set_property(struct power_supply *psy,
> +				     enum power_supply_property psp,
> +				     const union power_supply_propval *val)
> +{
> +	struct wilco_ec_device *ec = power_supply_get_drvdata(psy);
> +	int mode;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
> +		mode = psp_val_to_charge_mode(val->intval);
> +		if (mode < 0)
> +			return -EINVAL;
> +		return wilco_ec_set_byte_property(ec, PID_CHARGE_MODE, mode);
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
> +		if (val->intval < CHARGE_LOWER_LIMIT_MIN ||
> +		    val->intval > CHARGE_LOWER_LIMIT_MAX)
> +			return -EINVAL;
> +		return wilco_ec_set_byte_property(ec, PID_CHARGE_LOWER_LIMIT,
> +						  val->intval);
> +	case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
> +		if (val->intval < CHARGE_UPPER_LIMIT_MIN ||
> +		    val->intval > CHARGE_UPPER_LIMIT_MAX)
> +			return -EINVAL;
> +		return wilco_ec_set_byte_property(ec, PID_CHARGE_UPPER_LIMIT,
> +						  val->intval);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int wilco_charge_property_is_writeable(struct power_supply *psy,
> +					      enum power_supply_property psp)
> +{
> +	return 1;
> +}
> +
> +static const struct power_supply_desc wilco_ps_desc = {
> +	.properties		= wilco_charge_props,
> +	.num_properties		= ARRAY_SIZE(wilco_charge_props),
> +	.get_property		= wilco_charge_get_property,
> +	.set_property		= wilco_charge_set_property,
> +	.property_is_writeable	= wilco_charge_property_is_writeable,
> +	.name			= "wilco-charger",

nit: DRV_NAME

> +	.type			= POWER_SUPPLY_TYPE_MAINS,
> +};
> +
> +static int wilco_charge_probe(struct platform_device *pdev)
> +{
> +	struct wilco_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> +	struct power_supply_config psy_cfg = {};
> +	struct power_supply *psy;
> +
> +	psy_cfg.drv_data = ec;
> +	psy = devm_power_supply_register(&pdev->dev, &wilco_ps_desc, &psy_cfg);
> +
> +	return PTR_ERR_OR_ZERO(psy);
> +}
> +
> +static struct platform_driver wilco_charge_driver = {
> +	.probe	= wilco_charge_probe,
> +	.driver = {
> +		.name = DRV_NAME,
> +	}
> +};
> +module_platform_driver(wilco_charge_driver);
> +
> +MODULE_ALIAS("platform:" DRV_NAME);
> +MODULE_AUTHOR("Nick Crews <ncrews@chromium.org>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Wilco EC charge control driver");
> diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> index 50a21bd5fd44..66d9f177bec2 100644
> --- a/include/linux/platform_data/wilco-ec.h
> +++ b/include/linux/platform_data/wilco-ec.h
> @@ -32,6 +32,7 @@
>   * @data_size: Size of the data buffer used for EC communication.
>   * @debugfs_pdev: The child platform_device used by the debugfs sub-driver.
>   * @rtc_pdev: The child platform_device used by the RTC sub-driver.
> + * @charger_pdev: Child platform_device used by the charger config sub-driver.
>   */
>  struct wilco_ec_device {
>  	struct device *dev;
> @@ -43,6 +44,7 @@ struct wilco_ec_device {
>  	size_t data_size;
>  	struct platform_device *debugfs_pdev;
>  	struct platform_device *rtc_pdev;
> +	struct platform_device *charger_pdev;
>  };
>  
>  /**
> 

Thanks,
 Enric

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver
  2019-04-23 15:01   ` Enric Balletbo i Serra
@ 2019-05-01 16:37     ` Nick Crews
  0 siblings, 0 replies; 5+ messages in thread
From: Nick Crews @ 2019-05-01 16:37 UTC (permalink / raw)
  To: Enric Balletbo i Serra, Sebastian Reichel
  Cc: Benson Leung, linux-pm, linux-kernel, Duncan Laurie, Oleh Lamzin,
	Bartosz Fabianowski, Daniel Erat, Dmitry Torokhov, Simon Glass,
	jchwong

Hi Enric and Sebastian,

I sent out a v8 to address Enric's nits:
https://lore.kernel.org/patchwork/patch/1065815/

Thanks,
Nick

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-05-01 16:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-22 18:07 [PATCH v7 1/2] platform/chrome: wilco_ec: Add property helper library Nick Crews
2019-04-22 18:07 ` [PATCH v7 2/2] power_supply: platform/chrome: wilco_ec: Add charging config driver Nick Crews
2019-04-23 15:01   ` Enric Balletbo i Serra
2019-05-01 16:37     ` Nick Crews
2019-04-23 14:43 ` [PATCH v7 1/2] platform/chrome: wilco_ec: Add property helper library Enric Balletbo i Serra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).