All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v0 0/2] Type-C charger support using power_supply
@ 2020-04-20 16:36 Mathew King
  2020-04-20 16:36 ` [PATCH v0 1/2] typec: Move typec class structs into a header file Mathew King
  2020-04-20 16:36 ` [PATCH v0 2/2] typec: Add Type-C charger Mathew King
  0 siblings, 2 replies; 6+ messages in thread
From: Mathew King @ 2020-04-20 16:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathew King, Heikki Krogerus, Benson Leung, Greg Kroah-Hartman,
	linux-usb, linux-pm

I am looking to expose Type-C charging ports using a power_supply class. In this
patch series I have done this by creating a config option to enable this by
adding support directly into the typec driver. I would like some feedback on
this general approach.

I have been testing on a system that uses an ACPI implementation of UCSI and
things are working as expected.

Mathew King (2):
  typec: Move typec class structs into a header file
  typec: Add Type-C charger

 drivers/usb/typec/Kconfig   |  11 ++
 drivers/usb/typec/Makefile  |   1 +
 drivers/usb/typec/charger.c | 204 ++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/charger.h |  33 ++++++
 drivers/usb/typec/class.c   | 108 ++++++++-----------
 drivers/usb/typec/class.h   |  63 +++++++++++
 6 files changed, 356 insertions(+), 64 deletions(-)
 create mode 100644 drivers/usb/typec/charger.c
 create mode 100644 drivers/usb/typec/charger.h
 create mode 100644 drivers/usb/typec/class.h

-- 
2.26.1.301.g55bc3eb7cb9-goog


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

* [PATCH v0 1/2] typec: Move typec class structs into a header file
  2020-04-20 16:36 [PATCH v0 0/2] Type-C charger support using power_supply Mathew King
@ 2020-04-20 16:36 ` Mathew King
  2020-04-20 16:36 ` [PATCH v0 2/2] typec: Add Type-C charger Mathew King
  1 sibling, 0 replies; 6+ messages in thread
From: Mathew King @ 2020-04-20 16:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathew King, Heikki Krogerus, Benson Leung, Greg Kroah-Hartman,
	linux-usb, linux-pm

Move the Type-C data structures into a separate header file so that they
can be used by in other places.

Signed-off-by: Mathew King <mathewk@chromium.org>
---
 drivers/usb/typec/class.c | 56 +----------------------------------
 drivers/usb/typec/class.h | 61 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 55 deletions(-)
 create mode 100644 drivers/usb/typec/class.h

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 8d894bdff77d..9a1fdce137b9 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -13,61 +13,7 @@
 #include <linux/slab.h>
 
 #include "bus.h"
-
-struct typec_plug {
-	struct device			dev;
-	enum typec_plug_index		index;
-	struct ida			mode_ids;
-};
-
-struct typec_cable {
-	struct device			dev;
-	enum typec_plug_type		type;
-	struct usb_pd_identity		*identity;
-	unsigned int			active:1;
-};
-
-struct typec_partner {
-	struct device			dev;
-	unsigned int			usb_pd:1;
-	struct usb_pd_identity		*identity;
-	enum typec_accessory		accessory;
-	struct ida			mode_ids;
-};
-
-struct typec_port {
-	unsigned int			id;
-	struct device			dev;
-	struct ida			mode_ids;
-
-	int				prefer_role;
-	enum typec_data_role		data_role;
-	enum typec_role			pwr_role;
-	enum typec_role			vconn_role;
-	enum typec_pwr_opmode		pwr_opmode;
-	enum typec_port_type		port_type;
-	struct mutex			port_type_lock;
-
-	enum typec_orientation		orientation;
-	struct typec_switch		*sw;
-	struct typec_mux		*mux;
-
-	const struct typec_capability	*cap;
-	const struct typec_operations   *ops;
-};
-
-#define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
-#define to_typec_plug(_dev_) container_of(_dev_, struct typec_plug, dev)
-#define to_typec_cable(_dev_) container_of(_dev_, struct typec_cable, dev)
-#define to_typec_partner(_dev_) container_of(_dev_, struct typec_partner, dev)
-
-static const struct device_type typec_partner_dev_type;
-static const struct device_type typec_cable_dev_type;
-static const struct device_type typec_plug_dev_type;
-
-#define is_typec_partner(_dev_) (_dev_->type == &typec_partner_dev_type)
-#define is_typec_cable(_dev_) (_dev_->type == &typec_cable_dev_type)
-#define is_typec_plug(_dev_) (_dev_->type == &typec_plug_dev_type)
+#include "class.h"
 
 static DEFINE_IDA(typec_index_ida);
 static struct class *typec_class;
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
new file mode 100644
index 000000000000..ec933dfe1323
--- /dev/null
+++ b/drivers/usb/typec/class.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __USB_TYPEC_CLASS_H__
+#define __USB_TYPEC_CLASS_H__
+
+struct typec_plug {
+	struct device			dev;
+	enum typec_plug_index		index;
+	struct ida			mode_ids;
+};
+
+struct typec_cable {
+	struct device			dev;
+	enum typec_plug_type		type;
+	struct usb_pd_identity		*identity;
+	unsigned int			active:1;
+};
+
+struct typec_partner {
+	struct device			dev;
+	unsigned int			usb_pd:1;
+	struct usb_pd_identity		*identity;
+	enum typec_accessory		accessory;
+	struct ida			mode_ids;
+};
+
+struct typec_port {
+	unsigned int			id;
+	struct device			dev;
+	struct ida			mode_ids;
+
+	int				prefer_role;
+	enum typec_data_role		data_role;
+	enum typec_role			pwr_role;
+	enum typec_role			vconn_role;
+	enum typec_pwr_opmode		pwr_opmode;
+	enum typec_port_type		port_type;
+	struct mutex			port_type_lock;
+
+	enum typec_orientation		orientation;
+	struct typec_switch		*sw;
+	struct typec_mux		*mux;
+
+	const struct typec_capability	*cap;
+	const struct typec_operations   *ops;
+};
+
+#define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
+#define to_typec_plug(_dev_) container_of(_dev_, struct typec_plug, dev)
+#define to_typec_cable(_dev_) container_of(_dev_, struct typec_cable, dev)
+#define to_typec_partner(_dev_) container_of(_dev_, struct typec_partner, dev)
+
+static const struct device_type typec_partner_dev_type;
+static const struct device_type typec_cable_dev_type;
+static const struct device_type typec_plug_dev_type;
+
+#define is_typec_partner(_dev_) (_dev_->type == &typec_partner_dev_type)
+#define is_typec_cable(_dev_) (_dev_->type == &typec_cable_dev_type)
+#define is_typec_plug(_dev_) (_dev_->type == &typec_plug_dev_type)
+
+#endif /* __USB_TYPEC_CLASS_H__ */
-- 
2.26.1.301.g55bc3eb7cb9-goog


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

* [PATCH v0 2/2] typec: Add Type-C charger
  2020-04-20 16:36 [PATCH v0 0/2] Type-C charger support using power_supply Mathew King
  2020-04-20 16:36 ` [PATCH v0 1/2] typec: Move typec class structs into a header file Mathew King
@ 2020-04-20 16:36 ` Mathew King
  2020-04-21  8:44   ` Adam Thomson
  1 sibling, 1 reply; 6+ messages in thread
From: Mathew King @ 2020-04-20 16:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mathew King, Heikki Krogerus, Benson Leung, Greg Kroah-Hartman,
	linux-usb, linux-pm

Add an option to expose USB Type-C ports that can charge system
batteries as a power_supply class. This implementation only exposes
three properties of the power supply.

POWER_SUPPLY_PROP_ONLINE - Set to true if the Type-C port is configured
                           as a sink and is connected to a partner
POWER_SUPPLY_PROP_STATUS - Set to CHARGING if a partner is connected and
                           the port is a sink and set to NOT_CHARGING
                           otherwise
POWER_SUPPLY_PROP_USB_TYPE - When a partner is conneced set to TYPE_C,
                             TYPE_PD, or TYPE_PD_DRP depending on the
                             partner capibilities and set to
                             TYPE_UNKNOWN otherwise

This implementation can be expanded as the typec class is expaneded. In
particular the STATUS property should show more than CHARGING and
NOT_CHARGING. Also properties like VOLTAGE and CURRENT can be added when
the typec class supports getting PDOs.

Signed-off-by: Mathew King <mathewk@chromium.org>
---
 drivers/usb/typec/Kconfig   |  11 ++
 drivers/usb/typec/Makefile  |   1 +
 drivers/usb/typec/charger.c | 204 ++++++++++++++++++++++++++++++++++++
 drivers/usb/typec/charger.h |  33 ++++++
 drivers/usb/typec/class.c   |  48 +++++++--
 drivers/usb/typec/class.h   |   2 +
 6 files changed, 290 insertions(+), 9 deletions(-)
 create mode 100644 drivers/usb/typec/charger.c
 create mode 100644 drivers/usb/typec/charger.h

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index b4f2aac7ae8a..1040c990cb7e 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -46,6 +46,17 @@ menuconfig TYPEC
 
 if TYPEC
 
+config TYPEC_CHARGER
+	bool "Type-C Power Supply support"
+	depends on POWER_SUPPLY
+	help
+	  Say Y here to enable Type-C charging ports to be exposed as a power
+	  supply class.
+
+	  If you choose this option Type-C charger support will be built into
+	  the typec driver. This will expose all Type-C ports as a power_supply
+	  class.
+
 source "drivers/usb/typec/tcpm/Kconfig"
 
 source "drivers/usb/typec/ucsi/Kconfig"
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 7753a5c3cd46..6fc5424761a1 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_TYPEC)		+= typec.o
 typec-y				:= class.o mux.o bus.o
+typec-$(CONFIG_TYPEC_CHARGER)	+= charger.o
 obj-$(CONFIG_TYPEC)		+= altmodes/
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
diff --git a/drivers/usb/typec/charger.c b/drivers/usb/typec/charger.c
new file mode 100644
index 000000000000..07c3cd065be8
--- /dev/null
+++ b/drivers/usb/typec/charger.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Type-C Charger Class
+ *
+ * Copyright (C) 2020, Google LLC
+ * Author: Mathew King <mathewk@google.com>
+ */
+
+#include <linux/slab.h>
+
+#include "charger.h"
+#include "class.h"
+
+static enum power_supply_property typec_charger_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_USB_TYPE
+};
+
+static enum power_supply_usb_type typec_charger_usb_types[] = {
+	POWER_SUPPLY_USB_TYPE_UNKNOWN,
+	POWER_SUPPLY_USB_TYPE_C,
+	POWER_SUPPLY_USB_TYPE_PD,
+	POWER_SUPPLY_USB_TYPE_PD_DRP,
+};
+
+static int typec_charger_get_prop(struct power_supply *psy,
+				  enum power_supply_property psp,
+				  union power_supply_propval *val)
+{
+	struct typec_charger *charger = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = charger->psy_online;
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = charger->psy_status;
+		break;
+	case POWER_SUPPLY_PROP_USB_TYPE:
+		val->intval = charger->psy_usb_type;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int typec_charger_set_prop(struct power_supply *psy,
+				  enum power_supply_property psp,
+				  const union power_supply_propval *val)
+{
+	return -EINVAL;
+}
+
+static int typec_charger_is_writeable(struct power_supply *psy,
+				      enum power_supply_property psp)
+{
+	return 0;
+}
+
+/**
+ * typec_charger_changed - Notify of a Type-C charger change
+ * @charger: Type-C charger that changed
+ *
+ * Notifies the Type-C charger that one or more of its attributes may have
+ * changed.
+ */
+void typec_charger_changed(struct typec_charger *charger)
+{
+	int last_psy_status, last_psy_usb_type, last_psy_online;
+
+	last_psy_online = charger->psy_online;
+	last_psy_status = charger->psy_status;
+	last_psy_usb_type = charger->psy_usb_type;
+
+	if (!charger->partner) {
+		charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
+		charger->psy_online = 0;
+		charger->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		goto out_notify;
+	}
+
+	if (charger->port->pwr_role == TYPEC_SOURCE) {
+		charger->psy_online = 0;
+		charger->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		if (charger->partner->usb_pd)
+			charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_PD_DRP;
+		else
+			charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
+
+		goto out_notify;
+	}
+
+	charger->psy_online = 1;
+	charger->psy_status = POWER_SUPPLY_STATUS_CHARGING;
+
+	if (charger->partner->usb_pd)
+		charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_PD;
+	else
+		charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_C;
+
+out_notify:
+	if (last_psy_usb_type != charger->psy_usb_type ||
+	    last_psy_status != charger->psy_status ||
+	    last_psy_online != charger->psy_online)
+		power_supply_changed(charger->psy);
+}
+EXPORT_SYMBOL_GPL(typec_charger_changed);
+
+/**
+ * typec_register_charger - Register a USB Type-C Charger
+ * @port: Type-C port to register as a charger
+ *
+ * Registers a Type-C port as a charger.
+ *
+ * Returns handle to the charger on success or ERR_PTR on failure.
+ */
+struct typec_charger *typec_register_charger(struct typec_port *port)
+{
+	struct power_supply_config psy_cfg = {};
+	struct typec_charger *charger;
+	struct power_supply *psy;
+
+	charger = kzalloc(sizeof(struct typec_charger), GFP_KERNEL);
+	if (!port)
+		return ERR_PTR(-ENOMEM);
+
+	charger->port = port;
+	sprintf(charger->name, TYPEC_CHARGER_DIR_NAME, port->id);
+	charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
+	charger->psy_online = 0;
+	charger->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+
+	charger->psy_desc.name = charger->name;
+	charger->psy_desc.type = POWER_SUPPLY_TYPE_USB;
+	charger->psy_desc.get_property = typec_charger_get_prop;
+	charger->psy_desc.set_property = typec_charger_set_prop;
+	charger->psy_desc.property_is_writeable =
+		typec_charger_is_writeable;
+	charger->psy_desc.properties = typec_charger_props;
+	charger->psy_desc.num_properties =
+				ARRAY_SIZE(typec_charger_props);
+	charger->psy_desc.usb_types = typec_charger_usb_types;
+	charger->psy_desc.num_usb_types =
+			ARRAY_SIZE(typec_charger_usb_types);
+	psy_cfg.drv_data = charger;
+
+	psy = devm_power_supply_register_no_ws(&port->dev, &charger->psy_desc,
+					       &psy_cfg);
+	if (IS_ERR(psy)) {
+		dev_err(&port->dev, "Failed to register Type-C power supply\n");
+		return ERR_CAST(psy);
+	}
+	charger->psy = psy;
+
+	return charger;
+}
+EXPORT_SYMBOL_GPL(typec_register_charger);
+
+/**
+ * typec_unregister_charger - Unregister a USB Type-C Charger
+ * @charger: The charger to unregister
+ *
+ * Unregisters a charger created with typec_register_charger().
+ */
+void typec_unregister_charger(struct typec_charger *charger)
+{
+	if (!IS_ERR_OR_NULL(charger))
+		kfree(charger);
+}
+EXPORT_SYMBOL_GPL(typec_unregister_charger);
+
+/**
+ * typec_charger_register_partner - Register a partner with a USB Type-C Charger
+ * @charger: The charger to add the partner too
+ * @partner: The partner to add
+ *
+ * Add a partner to a Type-C charger to indicate that the partner is connected
+ * and may be charging.
+ */
+void typec_charger_register_partner(struct typec_charger *charger,
+				    struct typec_partner *partner)
+{
+	charger->partner = partner;
+	typec_charger_changed(charger);
+}
+EXPORT_SYMBOL_GPL(typec_charger_register_partner);
+
+/**
+ * typec_charger_unregister_partner - Unregister a USB Type-C Charger partner
+ * @charger: The charger to remove the partner from
+ *
+ * Remove partner added with typec_charger_register_partner().
+ */
+void typec_charger_unregister_partner(struct typec_charger *charger)
+{
+	if (!IS_ERR_OR_NULL(charger))
+		charger->partner = NULL;
+
+	typec_charger_changed(charger);
+}
+EXPORT_SYMBOL_GPL(typec_charger_unregister_partner);
diff --git a/drivers/usb/typec/charger.h b/drivers/usb/typec/charger.h
new file mode 100644
index 000000000000..32cdaa7c1a83
--- /dev/null
+++ b/drivers/usb/typec/charger.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __USB_TYPEC_CHARGER_H__
+#define __USB_TYPEC_CHARGER_H__
+
+#include <linux/power_supply.h>
+#include <linux/usb/typec.h>
+
+#include "class.h"
+
+#define TYPEC_CHARGER_DIR_NAME			"TYPEC_CHARGER%d"
+#define TYPEC_CHARGER_DIR_NAME_LENGTH		sizeof(TYPEC_CHARGER_DIR_NAME)
+
+struct typec_charger {
+	struct typec_port *port;
+	struct typec_partner *partner;
+	char name[TYPEC_CHARGER_DIR_NAME_LENGTH];
+	struct power_supply *psy;
+	struct power_supply_desc psy_desc;
+	int psy_usb_type;
+	int psy_online;
+	int psy_status;
+};
+
+struct typec_charger *typec_register_charger(struct typec_port *port);
+void typec_unregister_charger(struct typec_charger *charger);
+
+void typec_charger_register_partner(struct typec_charger *charger,
+				    struct typec_partner *partner);
+void typec_charger_unregister_partner(struct typec_charger *charger);
+void typec_charger_changed(struct typec_charger *charger);
+
+#endif /* __USB_TYPEC_CHARGER_H__ */
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9a1fdce137b9..1542d3af342c 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -13,6 +13,7 @@
 #include <linux/slab.h>
 
 #include "bus.h"
+#include "charger.h"
 #include "class.h"
 
 static DEFINE_IDA(typec_index_ida);
@@ -489,6 +490,12 @@ static void typec_partner_release(struct device *dev)
 {
 	struct typec_partner *partner = to_typec_partner(dev);
 
+	if (IS_ENABLED(CONFIG_TYPEC_CHARGER)) {
+		struct typec_port *port = to_typec_port(dev->parent);
+
+		typec_charger_unregister_partner(port->charger);
+	}
+
 	ida_destroy(&partner->mode_ids);
 	kfree(partner);
 }
@@ -580,6 +587,10 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
 		return ERR_PTR(ret);
 	}
 
+	if (IS_ENABLED(CONFIG_TYPEC_CHARGER) && port->charger) {
+		typec_charger_register_partner(port->charger, partner);
+	}
+
 	return partner;
 }
 EXPORT_SYMBOL_GPL(typec_register_partner);
@@ -1283,6 +1294,9 @@ static void typec_release(struct device *dev)
 {
 	struct typec_port *port = to_typec_port(dev);
 
+	if (IS_ENABLED(CONFIG_TYPEC_CHARGER))
+		typec_unregister_charger(port->charger);
+
 	ida_simple_remove(&typec_index_ida, port->id);
 	ida_destroy(&port->mode_ids);
 	typec_switch_put(port->sw);
@@ -1564,7 +1578,8 @@ struct typec_port *typec_register_port(struct device *parent,
 	id = ida_simple_get(&typec_index_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
 		kfree(port);
-		return ERR_PTR(id);
+		ret = id;
+		goto err_return;
 	}
 
 	switch (cap->type) {
@@ -1617,32 +1632,47 @@ struct typec_port *typec_register_port(struct device *parent,
 
 	port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL);
 	if (!port->cap) {
-		put_device(&port->dev);
-		return ERR_PTR(-ENOMEM);
+		ret = -ENOMEM;
+		goto err_put_device;
 	}
 
 	port->sw = typec_switch_get(&port->dev);
 	if (IS_ERR(port->sw)) {
 		ret = PTR_ERR(port->sw);
-		put_device(&port->dev);
-		return ERR_PTR(ret);
+		goto err_put_device;
 	}
 
 	port->mux = typec_mux_get(&port->dev, NULL);
 	if (IS_ERR(port->mux)) {
 		ret = PTR_ERR(port->mux);
-		put_device(&port->dev);
-		return ERR_PTR(ret);
+		goto err_put_device;
 	}
 
 	ret = device_add(&port->dev);
 	if (ret) {
 		dev_err(parent, "failed to register port (%d)\n", ret);
-		put_device(&port->dev);
-		return ERR_PTR(ret);
+		goto err_put_device;
+	}
+
+	if (IS_ENABLED(CONFIG_TYPEC_CHARGER)) {
+		port->charger = typec_register_charger(port);
+
+		if (IS_ERR(port->charger)) {
+			ret = PTR_ERR(port->charger);
+			goto err_device_del;
+		}
 	}
 
 	return port;
+
+err_device_del:
+	device_del(&port->dev);
+
+err_put_device:
+	put_device(&port->dev);
+
+err_return:
+	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(typec_register_port);
 
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index ec933dfe1323..0ff0a590d316 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -41,6 +41,8 @@ struct typec_port {
 	struct typec_switch		*sw;
 	struct typec_mux		*mux;
 
+	struct typec_charger		*charger;
+
 	const struct typec_capability	*cap;
 	const struct typec_operations   *ops;
 };
-- 
2.26.1.301.g55bc3eb7cb9-goog


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

* RE: [PATCH v0 2/2] typec: Add Type-C charger
  2020-04-20 16:36 ` [PATCH v0 2/2] typec: Add Type-C charger Mathew King
@ 2020-04-21  8:44   ` Adam Thomson
  2020-04-21 12:41     ` Heikki Krogerus
  0 siblings, 1 reply; 6+ messages in thread
From: Adam Thomson @ 2020-04-21  8:44 UTC (permalink / raw)
  To: Mathew King, linux-kernel
  Cc: Heikki Krogerus, Benson Leung, Greg Kroah-Hartman, linux-usb, linux-pm

On 20 April 2020 17:37, Mathew King wrote:

> Add an option to expose USB Type-C ports that can charge system
> batteries as a power_supply class. This implementation only exposes
> three properties of the power supply.
> 
> POWER_SUPPLY_PROP_ONLINE - Set to true if the Type-C port is configured
>                            as a sink and is connected to a partner
> POWER_SUPPLY_PROP_STATUS - Set to CHARGING if a partner is connected and
>                            the port is a sink and set to NOT_CHARGING
>                            otherwise
> POWER_SUPPLY_PROP_USB_TYPE - When a partner is conneced set to TYPE_C,
>                              TYPE_PD, or TYPE_PD_DRP depending on the
>                              partner capibilities and set to
>                              TYPE_UNKNOWN otherwise
> 
> This implementation can be expanded as the typec class is expaneded. In
> particular the STATUS property should show more than CHARGING and
> NOT_CHARGING. Also properties like VOLTAGE and CURRENT can be added
> when
> the typec class supports getting PDOs.

Hmm, this functionally looks almost exactly like code already available in TCPM,
except a much smaller subset. This looks like it would duplicate that work so as
it stands doesn't feel sensible to me. It may be that the work in TCPM needs
refactoring, but I don't believe the two should coexist.

> 
> Signed-off-by: Mathew King <mathewk@chromium.org>
> ---
>  drivers/usb/typec/Kconfig   |  11 ++
>  drivers/usb/typec/Makefile  |   1 +
>  drivers/usb/typec/charger.c | 204
> ++++++++++++++++++++++++++++++++++++
>  drivers/usb/typec/charger.h |  33 ++++++
>  drivers/usb/typec/class.c   |  48 +++++++--
>  drivers/usb/typec/class.h   |   2 +
>  6 files changed, 290 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/usb/typec/charger.c
>  create mode 100644 drivers/usb/typec/charger.h
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index b4f2aac7ae8a..1040c990cb7e 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -46,6 +46,17 @@ menuconfig TYPEC
> 
>  if TYPEC
> 
> +config TYPEC_CHARGER
> +	bool "Type-C Power Supply support"
> +	depends on POWER_SUPPLY
> +	help
> +	  Say Y here to enable Type-C charging ports to be exposed as a power
> +	  supply class.
> +
> +	  If you choose this option Type-C charger support will be built into
> +	  the typec driver. This will expose all Type-C ports as a power_supply
> +	  class.
> +
>  source "drivers/usb/typec/tcpm/Kconfig"
> 
>  source "drivers/usb/typec/ucsi/Kconfig"
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> index 7753a5c3cd46..6fc5424761a1 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_TYPEC)		+= typec.o
>  typec-y				:= class.o mux.o bus.o
> +typec-$(CONFIG_TYPEC_CHARGER)	+= charger.o
>  obj-$(CONFIG_TYPEC)		+= altmodes/
>  obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
>  obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
> diff --git a/drivers/usb/typec/charger.c b/drivers/usb/typec/charger.c
> new file mode 100644
> index 000000000000..07c3cd065be8
> --- /dev/null
> +++ b/drivers/usb/typec/charger.c
> @@ -0,0 +1,204 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * USB Type-C Charger Class
> + *
> + * Copyright (C) 2020, Google LLC
> + * Author: Mathew King <mathewk@google.com>
> + */
> +
> +#include <linux/slab.h>
> +
> +#include "charger.h"
> +#include "class.h"
> +
> +static enum power_supply_property typec_charger_props[] = {
> +	POWER_SUPPLY_PROP_ONLINE,
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_USB_TYPE
> +};
> +
> +static enum power_supply_usb_type typec_charger_usb_types[] = {
> +	POWER_SUPPLY_USB_TYPE_UNKNOWN,
> +	POWER_SUPPLY_USB_TYPE_C,
> +	POWER_SUPPLY_USB_TYPE_PD,
> +	POWER_SUPPLY_USB_TYPE_PD_DRP,
> +};
> +
> +static int typec_charger_get_prop(struct power_supply *psy,
> +				  enum power_supply_property psp,
> +				  union power_supply_propval *val)
> +{
> +	struct typec_charger *charger = power_supply_get_drvdata(psy);
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		val->intval = charger->psy_online;
> +		break;
> +	case POWER_SUPPLY_PROP_STATUS:
> +		val->intval = charger->psy_status;
> +		break;
> +	case POWER_SUPPLY_PROP_USB_TYPE:
> +		val->intval = charger->psy_usb_type;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int typec_charger_set_prop(struct power_supply *psy,
> +				  enum power_supply_property psp,
> +				  const union power_supply_propval *val)
> +{
> +	return -EINVAL;
> +}
> +
> +static int typec_charger_is_writeable(struct power_supply *psy,
> +				      enum power_supply_property psp)
> +{
> +	return 0;
> +}
> +
> +/**
> + * typec_charger_changed - Notify of a Type-C charger change
> + * @charger: Type-C charger that changed
> + *
> + * Notifies the Type-C charger that one or more of its attributes may have
> + * changed.
> + */
> +void typec_charger_changed(struct typec_charger *charger)
> +{
> +	int last_psy_status, last_psy_usb_type, last_psy_online;
> +
> +	last_psy_online = charger->psy_online;
> +	last_psy_status = charger->psy_status;
> +	last_psy_usb_type = charger->psy_usb_type;
> +
> +	if (!charger->partner) {
> +		charger->psy_usb_type =
> POWER_SUPPLY_USB_TYPE_UNKNOWN;
> +		charger->psy_online = 0;
> +		charger->psy_status =
> POWER_SUPPLY_STATUS_NOT_CHARGING;
> +		goto out_notify;
> +	}
> +
> +	if (charger->port->pwr_role == TYPEC_SOURCE) {
> +		charger->psy_online = 0;
> +		charger->psy_status =
> POWER_SUPPLY_STATUS_NOT_CHARGING;
> +		if (charger->partner->usb_pd)
> +			charger->psy_usb_type =
> POWER_SUPPLY_USB_TYPE_PD_DRP;
> +		else
> +			charger->psy_usb_type =
> POWER_SUPPLY_USB_TYPE_UNKNOWN;
> +
> +		goto out_notify;
> +	}
> +
> +	charger->psy_online = 1;
> +	charger->psy_status = POWER_SUPPLY_STATUS_CHARGING;
> +
> +	if (charger->partner->usb_pd)
> +		charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_PD;
> +	else
> +		charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_C;
> +
> +out_notify:
> +	if (last_psy_usb_type != charger->psy_usb_type ||
> +	    last_psy_status != charger->psy_status ||
> +	    last_psy_online != charger->psy_online)
> +		power_supply_changed(charger->psy);
> +}
> +EXPORT_SYMBOL_GPL(typec_charger_changed);
> +
> +/**
> + * typec_register_charger - Register a USB Type-C Charger
> + * @port: Type-C port to register as a charger
> + *
> + * Registers a Type-C port as a charger.
> + *
> + * Returns handle to the charger on success or ERR_PTR on failure.
> + */
> +struct typec_charger *typec_register_charger(struct typec_port *port)
> +{
> +	struct power_supply_config psy_cfg = {};
> +	struct typec_charger *charger;
> +	struct power_supply *psy;
> +
> +	charger = kzalloc(sizeof(struct typec_charger), GFP_KERNEL);
> +	if (!port)
> +		return ERR_PTR(-ENOMEM);
> +
> +	charger->port = port;
> +	sprintf(charger->name, TYPEC_CHARGER_DIR_NAME, port->id);
> +	charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
> +	charger->psy_online = 0;
> +	charger->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +
> +	charger->psy_desc.name = charger->name;
> +	charger->psy_desc.type = POWER_SUPPLY_TYPE_USB;
> +	charger->psy_desc.get_property = typec_charger_get_prop;
> +	charger->psy_desc.set_property = typec_charger_set_prop;
> +	charger->psy_desc.property_is_writeable =
> +		typec_charger_is_writeable;
> +	charger->psy_desc.properties = typec_charger_props;
> +	charger->psy_desc.num_properties =
> +				ARRAY_SIZE(typec_charger_props);
> +	charger->psy_desc.usb_types = typec_charger_usb_types;
> +	charger->psy_desc.num_usb_types =
> +			ARRAY_SIZE(typec_charger_usb_types);
> +	psy_cfg.drv_data = charger;
> +
> +	psy = devm_power_supply_register_no_ws(&port->dev, &charger-
> >psy_desc,
> +					       &psy_cfg);
> +	if (IS_ERR(psy)) {
> +		dev_err(&port->dev, "Failed to register Type-C power
> supply\n");
> +		return ERR_CAST(psy);
> +	}
> +	charger->psy = psy;
> +
> +	return charger;
> +}
> +EXPORT_SYMBOL_GPL(typec_register_charger);
> +
> +/**
> + * typec_unregister_charger - Unregister a USB Type-C Charger
> + * @charger: The charger to unregister
> + *
> + * Unregisters a charger created with typec_register_charger().
> + */
> +void typec_unregister_charger(struct typec_charger *charger)
> +{
> +	if (!IS_ERR_OR_NULL(charger))
> +		kfree(charger);
> +}
> +EXPORT_SYMBOL_GPL(typec_unregister_charger);
> +
> +/**
> + * typec_charger_register_partner - Register a partner with a USB Type-C
> Charger
> + * @charger: The charger to add the partner too
> + * @partner: The partner to add
> + *
> + * Add a partner to a Type-C charger to indicate that the partner is connected
> + * and may be charging.
> + */
> +void typec_charger_register_partner(struct typec_charger *charger,
> +				    struct typec_partner *partner)
> +{
> +	charger->partner = partner;
> +	typec_charger_changed(charger);
> +}
> +EXPORT_SYMBOL_GPL(typec_charger_register_partner);
> +
> +/**
> + * typec_charger_unregister_partner - Unregister a USB Type-C Charger partner
> + * @charger: The charger to remove the partner from
> + *
> + * Remove partner added with typec_charger_register_partner().
> + */
> +void typec_charger_unregister_partner(struct typec_charger *charger)
> +{
> +	if (!IS_ERR_OR_NULL(charger))
> +		charger->partner = NULL;
> +
> +	typec_charger_changed(charger);
> +}
> +EXPORT_SYMBOL_GPL(typec_charger_unregister_partner);
> diff --git a/drivers/usb/typec/charger.h b/drivers/usb/typec/charger.h
> new file mode 100644
> index 000000000000..32cdaa7c1a83
> --- /dev/null
> +++ b/drivers/usb/typec/charger.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __USB_TYPEC_CHARGER_H__
> +#define __USB_TYPEC_CHARGER_H__
> +
> +#include <linux/power_supply.h>
> +#include <linux/usb/typec.h>
> +
> +#include "class.h"
> +
> +#define TYPEC_CHARGER_DIR_NAME
> 	"TYPEC_CHARGER%d"
> +#define TYPEC_CHARGER_DIR_NAME_LENGTH
> 	sizeof(TYPEC_CHARGER_DIR_NAME)
> +
> +struct typec_charger {
> +	struct typec_port *port;
> +	struct typec_partner *partner;
> +	char name[TYPEC_CHARGER_DIR_NAME_LENGTH];
> +	struct power_supply *psy;
> +	struct power_supply_desc psy_desc;
> +	int psy_usb_type;
> +	int psy_online;
> +	int psy_status;
> +};
> +
> +struct typec_charger *typec_register_charger(struct typec_port *port);
> +void typec_unregister_charger(struct typec_charger *charger);
> +
> +void typec_charger_register_partner(struct typec_charger *charger,
> +				    struct typec_partner *partner);
> +void typec_charger_unregister_partner(struct typec_charger *charger);
> +void typec_charger_changed(struct typec_charger *charger);
> +
> +#endif /* __USB_TYPEC_CHARGER_H__ */
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 9a1fdce137b9..1542d3af342c 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -13,6 +13,7 @@
>  #include <linux/slab.h>
> 
>  #include "bus.h"
> +#include "charger.h"
>  #include "class.h"
> 
>  static DEFINE_IDA(typec_index_ida);
> @@ -489,6 +490,12 @@ static void typec_partner_release(struct device *dev)
>  {
>  	struct typec_partner *partner = to_typec_partner(dev);
> 
> +	if (IS_ENABLED(CONFIG_TYPEC_CHARGER)) {
> +		struct typec_port *port = to_typec_port(dev->parent);
> +
> +		typec_charger_unregister_partner(port->charger);
> +	}
> +
>  	ida_destroy(&partner->mode_ids);
>  	kfree(partner);
>  }
> @@ -580,6 +587,10 @@ struct typec_partner *typec_register_partner(struct
> typec_port *port,
>  		return ERR_PTR(ret);
>  	}
> 
> +	if (IS_ENABLED(CONFIG_TYPEC_CHARGER) && port->charger) {
> +		typec_charger_register_partner(port->charger, partner);
> +	}
> +
>  	return partner;
>  }
>  EXPORT_SYMBOL_GPL(typec_register_partner);
> @@ -1283,6 +1294,9 @@ static void typec_release(struct device *dev)
>  {
>  	struct typec_port *port = to_typec_port(dev);
> 
> +	if (IS_ENABLED(CONFIG_TYPEC_CHARGER))
> +		typec_unregister_charger(port->charger);
> +
>  	ida_simple_remove(&typec_index_ida, port->id);
>  	ida_destroy(&port->mode_ids);
>  	typec_switch_put(port->sw);
> @@ -1564,7 +1578,8 @@ struct typec_port *typec_register_port(struct device
> *parent,
>  	id = ida_simple_get(&typec_index_ida, 0, 0, GFP_KERNEL);
>  	if (id < 0) {
>  		kfree(port);
> -		return ERR_PTR(id);
> +		ret = id;
> +		goto err_return;
>  	}
> 
>  	switch (cap->type) {
> @@ -1617,32 +1632,47 @@ struct typec_port *typec_register_port(struct device
> *parent,
> 
>  	port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL);
>  	if (!port->cap) {
> -		put_device(&port->dev);
> -		return ERR_PTR(-ENOMEM);
> +		ret = -ENOMEM;
> +		goto err_put_device;
>  	}
> 
>  	port->sw = typec_switch_get(&port->dev);
>  	if (IS_ERR(port->sw)) {
>  		ret = PTR_ERR(port->sw);
> -		put_device(&port->dev);
> -		return ERR_PTR(ret);
> +		goto err_put_device;
>  	}
> 
>  	port->mux = typec_mux_get(&port->dev, NULL);
>  	if (IS_ERR(port->mux)) {
>  		ret = PTR_ERR(port->mux);
> -		put_device(&port->dev);
> -		return ERR_PTR(ret);
> +		goto err_put_device;
>  	}
> 
>  	ret = device_add(&port->dev);
>  	if (ret) {
>  		dev_err(parent, "failed to register port (%d)\n", ret);
> -		put_device(&port->dev);
> -		return ERR_PTR(ret);
> +		goto err_put_device;
> +	}
> +
> +	if (IS_ENABLED(CONFIG_TYPEC_CHARGER)) {
> +		port->charger = typec_register_charger(port);
> +
> +		if (IS_ERR(port->charger)) {
> +			ret = PTR_ERR(port->charger);
> +			goto err_device_del;
> +		}
>  	}
> 
>  	return port;
> +
> +err_device_del:
> +	device_del(&port->dev);
> +
> +err_put_device:
> +	put_device(&port->dev);
> +
> +err_return:
> +	return ERR_PTR(ret);
>  }
>  EXPORT_SYMBOL_GPL(typec_register_port);
> 
> diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> index ec933dfe1323..0ff0a590d316 100644
> --- a/drivers/usb/typec/class.h
> +++ b/drivers/usb/typec/class.h
> @@ -41,6 +41,8 @@ struct typec_port {
>  	struct typec_switch		*sw;
>  	struct typec_mux		*mux;
> 
> +	struct typec_charger		*charger;
> +
>  	const struct typec_capability	*cap;
>  	const struct typec_operations   *ops;
>  };
> --
> 2.26.1.301.g55bc3eb7cb9-goog


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

* Re: [PATCH v0 2/2] typec: Add Type-C charger
  2020-04-21  8:44   ` Adam Thomson
@ 2020-04-21 12:41     ` Heikki Krogerus
  2020-04-21 15:56       ` Mat King
  0 siblings, 1 reply; 6+ messages in thread
From: Heikki Krogerus @ 2020-04-21 12:41 UTC (permalink / raw)
  To: Adam Thomson
  Cc: Mathew King, linux-kernel, Benson Leung, Greg Kroah-Hartman,
	linux-usb, linux-pm

On Tue, Apr 21, 2020 at 08:44:38AM +0000, Adam Thomson wrote:
> On 20 April 2020 17:37, Mathew King wrote:
> 
> > Add an option to expose USB Type-C ports that can charge system
> > batteries as a power_supply class. This implementation only exposes
> > three properties of the power supply.
> > 
> > POWER_SUPPLY_PROP_ONLINE - Set to true if the Type-C port is configured
> >                            as a sink and is connected to a partner
> > POWER_SUPPLY_PROP_STATUS - Set to CHARGING if a partner is connected and
> >                            the port is a sink and set to NOT_CHARGING
> >                            otherwise
> > POWER_SUPPLY_PROP_USB_TYPE - When a partner is conneced set to TYPE_C,
> >                              TYPE_PD, or TYPE_PD_DRP depending on the
> >                              partner capibilities and set to
> >                              TYPE_UNKNOWN otherwise
> > 
> > This implementation can be expanded as the typec class is expaneded. In
> > particular the STATUS property should show more than CHARGING and
> > NOT_CHARGING. Also properties like VOLTAGE and CURRENT can be added
> > when
> > the typec class supports getting PDOs.
> 
> Hmm, this functionally looks almost exactly like code already available in TCPM,
> except a much smaller subset. This looks like it would duplicate that work so as
> it stands doesn't feel sensible to me. It may be that the work in TCPM needs
> refactoring, but I don't believe the two should coexist.

I agree. We can't register a psy for every port and partner
unconditionally like that.

I do think that for the sake of uniformity it would make sense to have
the Type-C subsystem supply API that the Type-C drivers can use for
registering the psy instead of every Type-C driver doing that directly
with the psy API. So this patch should first introduce that API
without doing anything automatically.

Once things settle down, we can consider taking care of the psy
registration for the drivers as well.

Ideally the psy(s) registered here would supply the same information
and functionality as the psy registered in TCPM. Then a separate patch
that follows (that is part of the series) could simply convert TCPM to
use this new API for registering the psy.

thanks,

> > 
> > Signed-off-by: Mathew King <mathewk@chromium.org>
> > ---
> >  drivers/usb/typec/Kconfig   |  11 ++
> >  drivers/usb/typec/Makefile  |   1 +
> >  drivers/usb/typec/charger.c | 204
> > ++++++++++++++++++++++++++++++++++++
> >  drivers/usb/typec/charger.h |  33 ++++++
> >  drivers/usb/typec/class.c   |  48 +++++++--
> >  drivers/usb/typec/class.h   |   2 +
> >  6 files changed, 290 insertions(+), 9 deletions(-)
> >  create mode 100644 drivers/usb/typec/charger.c
> >  create mode 100644 drivers/usb/typec/charger.h
> > 
> > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > index b4f2aac7ae8a..1040c990cb7e 100644
> > --- a/drivers/usb/typec/Kconfig
> > +++ b/drivers/usb/typec/Kconfig
> > @@ -46,6 +46,17 @@ menuconfig TYPEC
> > 
> >  if TYPEC
> > 
> > +config TYPEC_CHARGER
> > +	bool "Type-C Power Supply support"
> > +	depends on POWER_SUPPLY
> > +	help
> > +	  Say Y here to enable Type-C charging ports to be exposed as a power
> > +	  supply class.
> > +
> > +	  If you choose this option Type-C charger support will be built into
> > +	  the typec driver. This will expose all Type-C ports as a power_supply
> > +	  class.
> > +
> >  source "drivers/usb/typec/tcpm/Kconfig"
> > 
> >  source "drivers/usb/typec/ucsi/Kconfig"
> > diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> > index 7753a5c3cd46..6fc5424761a1 100644
> > --- a/drivers/usb/typec/Makefile
> > +++ b/drivers/usb/typec/Makefile
> > @@ -1,6 +1,7 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  obj-$(CONFIG_TYPEC)		+= typec.o
> >  typec-y				:= class.o mux.o bus.o
> > +typec-$(CONFIG_TYPEC_CHARGER)	+= charger.o
> >  obj-$(CONFIG_TYPEC)		+= altmodes/
> >  obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
> >  obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
> > diff --git a/drivers/usb/typec/charger.c b/drivers/usb/typec/charger.c
> > new file mode 100644
> > index 000000000000..07c3cd065be8
> > --- /dev/null
> > +++ b/drivers/usb/typec/charger.c
> > @@ -0,0 +1,204 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * USB Type-C Charger Class
> > + *
> > + * Copyright (C) 2020, Google LLC
> > + * Author: Mathew King <mathewk@google.com>
> > + */
> > +
> > +#include <linux/slab.h>
> > +
> > +#include "charger.h"
> > +#include "class.h"
> > +
> > +static enum power_supply_property typec_charger_props[] = {
> > +	POWER_SUPPLY_PROP_ONLINE,
> > +	POWER_SUPPLY_PROP_STATUS,
> > +	POWER_SUPPLY_PROP_USB_TYPE
> > +};
> > +
> > +static enum power_supply_usb_type typec_charger_usb_types[] = {
> > +	POWER_SUPPLY_USB_TYPE_UNKNOWN,
> > +	POWER_SUPPLY_USB_TYPE_C,
> > +	POWER_SUPPLY_USB_TYPE_PD,
> > +	POWER_SUPPLY_USB_TYPE_PD_DRP,
> > +};
> > +
> > +static int typec_charger_get_prop(struct power_supply *psy,
> > +				  enum power_supply_property psp,
> > +				  union power_supply_propval *val)
> > +{
> > +	struct typec_charger *charger = power_supply_get_drvdata(psy);
> > +
> > +	switch (psp) {
> > +	case POWER_SUPPLY_PROP_ONLINE:
> > +		val->intval = charger->psy_online;
> > +		break;
> > +	case POWER_SUPPLY_PROP_STATUS:
> > +		val->intval = charger->psy_status;
> > +		break;
> > +	case POWER_SUPPLY_PROP_USB_TYPE:
> > +		val->intval = charger->psy_usb_type;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int typec_charger_set_prop(struct power_supply *psy,
> > +				  enum power_supply_property psp,
> > +				  const union power_supply_propval *val)
> > +{
> > +	return -EINVAL;
> > +}
> > +
> > +static int typec_charger_is_writeable(struct power_supply *psy,
> > +				      enum power_supply_property psp)
> > +{
> > +	return 0;
> > +}
> > +
> > +/**
> > + * typec_charger_changed - Notify of a Type-C charger change
> > + * @charger: Type-C charger that changed
> > + *
> > + * Notifies the Type-C charger that one or more of its attributes may have
> > + * changed.
> > + */
> > +void typec_charger_changed(struct typec_charger *charger)
> > +{
> > +	int last_psy_status, last_psy_usb_type, last_psy_online;
> > +
> > +	last_psy_online = charger->psy_online;
> > +	last_psy_status = charger->psy_status;
> > +	last_psy_usb_type = charger->psy_usb_type;
> > +
> > +	if (!charger->partner) {
> > +		charger->psy_usb_type =
> > POWER_SUPPLY_USB_TYPE_UNKNOWN;
> > +		charger->psy_online = 0;
> > +		charger->psy_status =
> > POWER_SUPPLY_STATUS_NOT_CHARGING;
> > +		goto out_notify;
> > +	}
> > +
> > +	if (charger->port->pwr_role == TYPEC_SOURCE) {
> > +		charger->psy_online = 0;
> > +		charger->psy_status =
> > POWER_SUPPLY_STATUS_NOT_CHARGING;
> > +		if (charger->partner->usb_pd)
> > +			charger->psy_usb_type =
> > POWER_SUPPLY_USB_TYPE_PD_DRP;
> > +		else
> > +			charger->psy_usb_type =
> > POWER_SUPPLY_USB_TYPE_UNKNOWN;
> > +
> > +		goto out_notify;
> > +	}
> > +
> > +	charger->psy_online = 1;
> > +	charger->psy_status = POWER_SUPPLY_STATUS_CHARGING;
> > +
> > +	if (charger->partner->usb_pd)
> > +		charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_PD;
> > +	else
> > +		charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_C;
> > +
> > +out_notify:
> > +	if (last_psy_usb_type != charger->psy_usb_type ||
> > +	    last_psy_status != charger->psy_status ||
> > +	    last_psy_online != charger->psy_online)
> > +		power_supply_changed(charger->psy);
> > +}
> > +EXPORT_SYMBOL_GPL(typec_charger_changed);
> > +
> > +/**
> > + * typec_register_charger - Register a USB Type-C Charger
> > + * @port: Type-C port to register as a charger
> > + *
> > + * Registers a Type-C port as a charger.
> > + *
> > + * Returns handle to the charger on success or ERR_PTR on failure.
> > + */
> > +struct typec_charger *typec_register_charger(struct typec_port *port)
> > +{
> > +	struct power_supply_config psy_cfg = {};
> > +	struct typec_charger *charger;
> > +	struct power_supply *psy;
> > +
> > +	charger = kzalloc(sizeof(struct typec_charger), GFP_KERNEL);
> > +	if (!port)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	charger->port = port;
> > +	sprintf(charger->name, TYPEC_CHARGER_DIR_NAME, port->id);
> > +	charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
> > +	charger->psy_online = 0;
> > +	charger->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > +
> > +	charger->psy_desc.name = charger->name;
> > +	charger->psy_desc.type = POWER_SUPPLY_TYPE_USB;
> > +	charger->psy_desc.get_property = typec_charger_get_prop;
> > +	charger->psy_desc.set_property = typec_charger_set_prop;
> > +	charger->psy_desc.property_is_writeable =
> > +		typec_charger_is_writeable;
> > +	charger->psy_desc.properties = typec_charger_props;
> > +	charger->psy_desc.num_properties =
> > +				ARRAY_SIZE(typec_charger_props);
> > +	charger->psy_desc.usb_types = typec_charger_usb_types;
> > +	charger->psy_desc.num_usb_types =
> > +			ARRAY_SIZE(typec_charger_usb_types);
> > +	psy_cfg.drv_data = charger;
> > +
> > +	psy = devm_power_supply_register_no_ws(&port->dev, &charger-
> > >psy_desc,
> > +					       &psy_cfg);
> > +	if (IS_ERR(psy)) {
> > +		dev_err(&port->dev, "Failed to register Type-C power
> > supply\n");
> > +		return ERR_CAST(psy);
> > +	}
> > +	charger->psy = psy;
> > +
> > +	return charger;
> > +}
> > +EXPORT_SYMBOL_GPL(typec_register_charger);
> > +
> > +/**
> > + * typec_unregister_charger - Unregister a USB Type-C Charger
> > + * @charger: The charger to unregister
> > + *
> > + * Unregisters a charger created with typec_register_charger().
> > + */
> > +void typec_unregister_charger(struct typec_charger *charger)
> > +{
> > +	if (!IS_ERR_OR_NULL(charger))
> > +		kfree(charger);
> > +}
> > +EXPORT_SYMBOL_GPL(typec_unregister_charger);
> > +
> > +/**
> > + * typec_charger_register_partner - Register a partner with a USB Type-C
> > Charger
> > + * @charger: The charger to add the partner too
> > + * @partner: The partner to add
> > + *
> > + * Add a partner to a Type-C charger to indicate that the partner is connected
> > + * and may be charging.
> > + */
> > +void typec_charger_register_partner(struct typec_charger *charger,
> > +				    struct typec_partner *partner)
> > +{
> > +	charger->partner = partner;
> > +	typec_charger_changed(charger);
> > +}
> > +EXPORT_SYMBOL_GPL(typec_charger_register_partner);
> > +
> > +/**
> > + * typec_charger_unregister_partner - Unregister a USB Type-C Charger partner
> > + * @charger: The charger to remove the partner from
> > + *
> > + * Remove partner added with typec_charger_register_partner().
> > + */
> > +void typec_charger_unregister_partner(struct typec_charger *charger)
> > +{
> > +	if (!IS_ERR_OR_NULL(charger))
> > +		charger->partner = NULL;
> > +
> > +	typec_charger_changed(charger);
> > +}
> > +EXPORT_SYMBOL_GPL(typec_charger_unregister_partner);
> > diff --git a/drivers/usb/typec/charger.h b/drivers/usb/typec/charger.h
> > new file mode 100644
> > index 000000000000..32cdaa7c1a83
> > --- /dev/null
> > +++ b/drivers/usb/typec/charger.h
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef __USB_TYPEC_CHARGER_H__
> > +#define __USB_TYPEC_CHARGER_H__
> > +
> > +#include <linux/power_supply.h>
> > +#include <linux/usb/typec.h>
> > +
> > +#include "class.h"
> > +
> > +#define TYPEC_CHARGER_DIR_NAME
> > 	"TYPEC_CHARGER%d"
> > +#define TYPEC_CHARGER_DIR_NAME_LENGTH
> > 	sizeof(TYPEC_CHARGER_DIR_NAME)
> > +
> > +struct typec_charger {
> > +	struct typec_port *port;
> > +	struct typec_partner *partner;
> > +	char name[TYPEC_CHARGER_DIR_NAME_LENGTH];
> > +	struct power_supply *psy;
> > +	struct power_supply_desc psy_desc;
> > +	int psy_usb_type;
> > +	int psy_online;
> > +	int psy_status;
> > +};
> > +
> > +struct typec_charger *typec_register_charger(struct typec_port *port);
> > +void typec_unregister_charger(struct typec_charger *charger);
> > +
> > +void typec_charger_register_partner(struct typec_charger *charger,
> > +				    struct typec_partner *partner);
> > +void typec_charger_unregister_partner(struct typec_charger *charger);
> > +void typec_charger_changed(struct typec_charger *charger);
> > +
> > +#endif /* __USB_TYPEC_CHARGER_H__ */
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 9a1fdce137b9..1542d3af342c 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/slab.h>
> > 
> >  #include "bus.h"
> > +#include "charger.h"
> >  #include "class.h"
> > 
> >  static DEFINE_IDA(typec_index_ida);
> > @@ -489,6 +490,12 @@ static void typec_partner_release(struct device *dev)
> >  {
> >  	struct typec_partner *partner = to_typec_partner(dev);
> > 
> > +	if (IS_ENABLED(CONFIG_TYPEC_CHARGER)) {
> > +		struct typec_port *port = to_typec_port(dev->parent);
> > +
> > +		typec_charger_unregister_partner(port->charger);
> > +	}
> > +
> >  	ida_destroy(&partner->mode_ids);
> >  	kfree(partner);
> >  }
> > @@ -580,6 +587,10 @@ struct typec_partner *typec_register_partner(struct
> > typec_port *port,
> >  		return ERR_PTR(ret);
> >  	}
> > 
> > +	if (IS_ENABLED(CONFIG_TYPEC_CHARGER) && port->charger) {
> > +		typec_charger_register_partner(port->charger, partner);
> > +	}
> > +
> >  	return partner;
> >  }
> >  EXPORT_SYMBOL_GPL(typec_register_partner);
> > @@ -1283,6 +1294,9 @@ static void typec_release(struct device *dev)
> >  {
> >  	struct typec_port *port = to_typec_port(dev);
> > 
> > +	if (IS_ENABLED(CONFIG_TYPEC_CHARGER))
> > +		typec_unregister_charger(port->charger);
> > +
> >  	ida_simple_remove(&typec_index_ida, port->id);
> >  	ida_destroy(&port->mode_ids);
> >  	typec_switch_put(port->sw);
> > @@ -1564,7 +1578,8 @@ struct typec_port *typec_register_port(struct device
> > *parent,
> >  	id = ida_simple_get(&typec_index_ida, 0, 0, GFP_KERNEL);
> >  	if (id < 0) {
> >  		kfree(port);
> > -		return ERR_PTR(id);
> > +		ret = id;
> > +		goto err_return;
> >  	}
> > 
> >  	switch (cap->type) {
> > @@ -1617,32 +1632,47 @@ struct typec_port *typec_register_port(struct device
> > *parent,
> > 
> >  	port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL);
> >  	if (!port->cap) {
> > -		put_device(&port->dev);
> > -		return ERR_PTR(-ENOMEM);
> > +		ret = -ENOMEM;
> > +		goto err_put_device;
> >  	}
> > 
> >  	port->sw = typec_switch_get(&port->dev);
> >  	if (IS_ERR(port->sw)) {
> >  		ret = PTR_ERR(port->sw);
> > -		put_device(&port->dev);
> > -		return ERR_PTR(ret);
> > +		goto err_put_device;
> >  	}
> > 
> >  	port->mux = typec_mux_get(&port->dev, NULL);
> >  	if (IS_ERR(port->mux)) {
> >  		ret = PTR_ERR(port->mux);
> > -		put_device(&port->dev);
> > -		return ERR_PTR(ret);
> > +		goto err_put_device;
> >  	}
> > 
> >  	ret = device_add(&port->dev);
> >  	if (ret) {
> >  		dev_err(parent, "failed to register port (%d)\n", ret);
> > -		put_device(&port->dev);
> > -		return ERR_PTR(ret);
> > +		goto err_put_device;
> > +	}
> > +
> > +	if (IS_ENABLED(CONFIG_TYPEC_CHARGER)) {
> > +		port->charger = typec_register_charger(port);
> > +
> > +		if (IS_ERR(port->charger)) {
> > +			ret = PTR_ERR(port->charger);
> > +			goto err_device_del;
> > +		}
> >  	}
> > 
> >  	return port;
> > +
> > +err_device_del:
> > +	device_del(&port->dev);
> > +
> > +err_put_device:
> > +	put_device(&port->dev);
> > +
> > +err_return:
> > +	return ERR_PTR(ret);
> >  }
> >  EXPORT_SYMBOL_GPL(typec_register_port);
> > 
> > diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> > index ec933dfe1323..0ff0a590d316 100644
> > --- a/drivers/usb/typec/class.h
> > +++ b/drivers/usb/typec/class.h
> > @@ -41,6 +41,8 @@ struct typec_port {
> >  	struct typec_switch		*sw;
> >  	struct typec_mux		*mux;
> > 
> > +	struct typec_charger		*charger;
> > +
> >  	const struct typec_capability	*cap;
> >  	const struct typec_operations   *ops;
> >  };
> > --
> > 2.26.1.301.g55bc3eb7cb9-goog

-- 
heikki

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

* Re: [PATCH v0 2/2] typec: Add Type-C charger
  2020-04-21 12:41     ` Heikki Krogerus
@ 2020-04-21 15:56       ` Mat King
  0 siblings, 0 replies; 6+ messages in thread
From: Mat King @ 2020-04-21 15:56 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Adam Thomson, Mathew King, linux-kernel, Benson Leung,
	Greg Kroah-Hartman, linux-usb, linux-pm

On Tue, Apr 21, 2020 at 6:41 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Tue, Apr 21, 2020 at 08:44:38AM +0000, Adam Thomson wrote:
> > On 20 April 2020 17:37, Mathew King wrote:
> >
> > > Add an option to expose USB Type-C ports that can charge system
> > > batteries as a power_supply class. This implementation only exposes
> > > three properties of the power supply.
> > >
> > > POWER_SUPPLY_PROP_ONLINE - Set to true if the Type-C port is configured
> > >                            as a sink and is connected to a partner
> > > POWER_SUPPLY_PROP_STATUS - Set to CHARGING if a partner is connected and
> > >                            the port is a sink and set to NOT_CHARGING
> > >                            otherwise
> > > POWER_SUPPLY_PROP_USB_TYPE - When a partner is conneced set to TYPE_C,
> > >                              TYPE_PD, or TYPE_PD_DRP depending on the
> > >                              partner capibilities and set to
> > >                              TYPE_UNKNOWN otherwise
> > >
> > > This implementation can be expanded as the typec class is expaneded. In
> > > particular the STATUS property should show more than CHARGING and
> > > NOT_CHARGING. Also properties like VOLTAGE and CURRENT can be added
> > > when
> > > the typec class supports getting PDOs.
> >
> > Hmm, this functionally looks almost exactly like code already available in TCPM,
> > except a much smaller subset. This looks like it would duplicate that work so as
> > it stands doesn't feel sensible to me. It may be that the work in TCPM needs
> > refactoring, but I don't believe the two should coexist.
>
> I agree. We can't register a psy for every port and partner
> unconditionally like that.
>
> I do think that for the sake of uniformity it would make sense to have
> the Type-C subsystem supply API that the Type-C drivers can use for
> registering the psy instead of every Type-C driver doing that directly
> with the psy API. So this patch should first introduce that API
> without doing anything automatically.
>
> Once things settle down, we can consider taking care of the psy
> registration for the drivers as well.
>
> Ideally the psy(s) registered here would supply the same information
> and functionality as the psy registered in TCPM. Then a separate patch
> that follows (that is part of the series) could simply convert TCPM to
> use this new API for registering the psy.
>
> thanks,

Thank you for the feedback Adam and Heikki. I will work on improving
the port API so that the psy is not created unconditionally and I will
work on getting to parity with the TCPM psy so that it can be switched
to this new method.

>
> > >
> > > Signed-off-by: Mathew King <mathewk@chromium.org>
> > > ---
> > >  drivers/usb/typec/Kconfig   |  11 ++
> > >  drivers/usb/typec/Makefile  |   1 +
> > >  drivers/usb/typec/charger.c | 204
> > > ++++++++++++++++++++++++++++++++++++
> > >  drivers/usb/typec/charger.h |  33 ++++++
> > >  drivers/usb/typec/class.c   |  48 +++++++--
> > >  drivers/usb/typec/class.h   |   2 +
> > >  6 files changed, 290 insertions(+), 9 deletions(-)
> > >  create mode 100644 drivers/usb/typec/charger.c
> > >  create mode 100644 drivers/usb/typec/charger.h
> > >
> > > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > > index b4f2aac7ae8a..1040c990cb7e 100644
> > > --- a/drivers/usb/typec/Kconfig
> > > +++ b/drivers/usb/typec/Kconfig
> > > @@ -46,6 +46,17 @@ menuconfig TYPEC
> > >
> > >  if TYPEC
> > >
> > > +config TYPEC_CHARGER
> > > +   bool "Type-C Power Supply support"
> > > +   depends on POWER_SUPPLY
> > > +   help
> > > +     Say Y here to enable Type-C charging ports to be exposed as a power
> > > +     supply class.
> > > +
> > > +     If you choose this option Type-C charger support will be built into
> > > +     the typec driver. This will expose all Type-C ports as a power_supply
> > > +     class.
> > > +
> > >  source "drivers/usb/typec/tcpm/Kconfig"
> > >
> > >  source "drivers/usb/typec/ucsi/Kconfig"
> > > diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> > > index 7753a5c3cd46..6fc5424761a1 100644
> > > --- a/drivers/usb/typec/Makefile
> > > +++ b/drivers/usb/typec/Makefile
> > > @@ -1,6 +1,7 @@
> > >  # SPDX-License-Identifier: GPL-2.0
> > >  obj-$(CONFIG_TYPEC)                += typec.o
> > >  typec-y                            := class.o mux.o bus.o
> > > +typec-$(CONFIG_TYPEC_CHARGER)      += charger.o
> > >  obj-$(CONFIG_TYPEC)                += altmodes/
> > >  obj-$(CONFIG_TYPEC_TCPM)   += tcpm/
> > >  obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
> > > diff --git a/drivers/usb/typec/charger.c b/drivers/usb/typec/charger.c
> > > new file mode 100644
> > > index 000000000000..07c3cd065be8
> > > --- /dev/null
> > > +++ b/drivers/usb/typec/charger.c
> > > @@ -0,0 +1,204 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * USB Type-C Charger Class
> > > + *
> > > + * Copyright (C) 2020, Google LLC
> > > + * Author: Mathew King <mathewk@google.com>
> > > + */
> > > +
> > > +#include <linux/slab.h>
> > > +
> > > +#include "charger.h"
> > > +#include "class.h"
> > > +
> > > +static enum power_supply_property typec_charger_props[] = {
> > > +   POWER_SUPPLY_PROP_ONLINE,
> > > +   POWER_SUPPLY_PROP_STATUS,
> > > +   POWER_SUPPLY_PROP_USB_TYPE
> > > +};
> > > +
> > > +static enum power_supply_usb_type typec_charger_usb_types[] = {
> > > +   POWER_SUPPLY_USB_TYPE_UNKNOWN,
> > > +   POWER_SUPPLY_USB_TYPE_C,
> > > +   POWER_SUPPLY_USB_TYPE_PD,
> > > +   POWER_SUPPLY_USB_TYPE_PD_DRP,
> > > +};
> > > +
> > > +static int typec_charger_get_prop(struct power_supply *psy,
> > > +                             enum power_supply_property psp,
> > > +                             union power_supply_propval *val)
> > > +{
> > > +   struct typec_charger *charger = power_supply_get_drvdata(psy);
> > > +
> > > +   switch (psp) {
> > > +   case POWER_SUPPLY_PROP_ONLINE:
> > > +           val->intval = charger->psy_online;
> > > +           break;
> > > +   case POWER_SUPPLY_PROP_STATUS:
> > > +           val->intval = charger->psy_status;
> > > +           break;
> > > +   case POWER_SUPPLY_PROP_USB_TYPE:
> > > +           val->intval = charger->psy_usb_type;
> > > +           break;
> > > +   default:
> > > +           return -EINVAL;
> > > +   }
> > > +
> > > +   return 0;
> > > +}
> > > +
> > > +static int typec_charger_set_prop(struct power_supply *psy,
> > > +                             enum power_supply_property psp,
> > > +                             const union power_supply_propval *val)
> > > +{
> > > +   return -EINVAL;
> > > +}
> > > +
> > > +static int typec_charger_is_writeable(struct power_supply *psy,
> > > +                                 enum power_supply_property psp)
> > > +{
> > > +   return 0;
> > > +}
> > > +
> > > +/**
> > > + * typec_charger_changed - Notify of a Type-C charger change
> > > + * @charger: Type-C charger that changed
> > > + *
> > > + * Notifies the Type-C charger that one or more of its attributes may have
> > > + * changed.
> > > + */
> > > +void typec_charger_changed(struct typec_charger *charger)
> > > +{
> > > +   int last_psy_status, last_psy_usb_type, last_psy_online;
> > > +
> > > +   last_psy_online = charger->psy_online;
> > > +   last_psy_status = charger->psy_status;
> > > +   last_psy_usb_type = charger->psy_usb_type;
> > > +
> > > +   if (!charger->partner) {
> > > +           charger->psy_usb_type =
> > > POWER_SUPPLY_USB_TYPE_UNKNOWN;
> > > +           charger->psy_online = 0;
> > > +           charger->psy_status =
> > > POWER_SUPPLY_STATUS_NOT_CHARGING;
> > > +           goto out_notify;
> > > +   }
> > > +
> > > +   if (charger->port->pwr_role == TYPEC_SOURCE) {
> > > +           charger->psy_online = 0;
> > > +           charger->psy_status =
> > > POWER_SUPPLY_STATUS_NOT_CHARGING;
> > > +           if (charger->partner->usb_pd)
> > > +                   charger->psy_usb_type =
> > > POWER_SUPPLY_USB_TYPE_PD_DRP;
> > > +           else
> > > +                   charger->psy_usb_type =
> > > POWER_SUPPLY_USB_TYPE_UNKNOWN;
> > > +
> > > +           goto out_notify;
> > > +   }
> > > +
> > > +   charger->psy_online = 1;
> > > +   charger->psy_status = POWER_SUPPLY_STATUS_CHARGING;
> > > +
> > > +   if (charger->partner->usb_pd)
> > > +           charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_PD;
> > > +   else
> > > +           charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_C;
> > > +
> > > +out_notify:
> > > +   if (last_psy_usb_type != charger->psy_usb_type ||
> > > +       last_psy_status != charger->psy_status ||
> > > +       last_psy_online != charger->psy_online)
> > > +           power_supply_changed(charger->psy);
> > > +}
> > > +EXPORT_SYMBOL_GPL(typec_charger_changed);
> > > +
> > > +/**
> > > + * typec_register_charger - Register a USB Type-C Charger
> > > + * @port: Type-C port to register as a charger
> > > + *
> > > + * Registers a Type-C port as a charger.
> > > + *
> > > + * Returns handle to the charger on success or ERR_PTR on failure.
> > > + */
> > > +struct typec_charger *typec_register_charger(struct typec_port *port)
> > > +{
> > > +   struct power_supply_config psy_cfg = {};
> > > +   struct typec_charger *charger;
> > > +   struct power_supply *psy;
> > > +
> > > +   charger = kzalloc(sizeof(struct typec_charger), GFP_KERNEL);
> > > +   if (!port)
> > > +           return ERR_PTR(-ENOMEM);
> > > +
> > > +   charger->port = port;
> > > +   sprintf(charger->name, TYPEC_CHARGER_DIR_NAME, port->id);
> > > +   charger->psy_usb_type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
> > > +   charger->psy_online = 0;
> > > +   charger->psy_status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > > +
> > > +   charger->psy_desc.name = charger->name;
> > > +   charger->psy_desc.type = POWER_SUPPLY_TYPE_USB;
> > > +   charger->psy_desc.get_property = typec_charger_get_prop;
> > > +   charger->psy_desc.set_property = typec_charger_set_prop;
> > > +   charger->psy_desc.property_is_writeable =
> > > +           typec_charger_is_writeable;
> > > +   charger->psy_desc.properties = typec_charger_props;
> > > +   charger->psy_desc.num_properties =
> > > +                           ARRAY_SIZE(typec_charger_props);
> > > +   charger->psy_desc.usb_types = typec_charger_usb_types;
> > > +   charger->psy_desc.num_usb_types =
> > > +                   ARRAY_SIZE(typec_charger_usb_types);
> > > +   psy_cfg.drv_data = charger;
> > > +
> > > +   psy = devm_power_supply_register_no_ws(&port->dev, &charger-
> > > >psy_desc,
> > > +                                          &psy_cfg);
> > > +   if (IS_ERR(psy)) {
> > > +           dev_err(&port->dev, "Failed to register Type-C power
> > > supply\n");
> > > +           return ERR_CAST(psy);
> > > +   }
> > > +   charger->psy = psy;
> > > +
> > > +   return charger;
> > > +}
> > > +EXPORT_SYMBOL_GPL(typec_register_charger);
> > > +
> > > +/**
> > > + * typec_unregister_charger - Unregister a USB Type-C Charger
> > > + * @charger: The charger to unregister
> > > + *
> > > + * Unregisters a charger created with typec_register_charger().
> > > + */
> > > +void typec_unregister_charger(struct typec_charger *charger)
> > > +{
> > > +   if (!IS_ERR_OR_NULL(charger))
> > > +           kfree(charger);
> > > +}
> > > +EXPORT_SYMBOL_GPL(typec_unregister_charger);
> > > +
> > > +/**
> > > + * typec_charger_register_partner - Register a partner with a USB Type-C
> > > Charger
> > > + * @charger: The charger to add the partner too
> > > + * @partner: The partner to add
> > > + *
> > > + * Add a partner to a Type-C charger to indicate that the partner is connected
> > > + * and may be charging.
> > > + */
> > > +void typec_charger_register_partner(struct typec_charger *charger,
> > > +                               struct typec_partner *partner)
> > > +{
> > > +   charger->partner = partner;
> > > +   typec_charger_changed(charger);
> > > +}
> > > +EXPORT_SYMBOL_GPL(typec_charger_register_partner);
> > > +
> > > +/**
> > > + * typec_charger_unregister_partner - Unregister a USB Type-C Charger partner
> > > + * @charger: The charger to remove the partner from
> > > + *
> > > + * Remove partner added with typec_charger_register_partner().
> > > + */
> > > +void typec_charger_unregister_partner(struct typec_charger *charger)
> > > +{
> > > +   if (!IS_ERR_OR_NULL(charger))
> > > +           charger->partner = NULL;
> > > +
> > > +   typec_charger_changed(charger);
> > > +}
> > > +EXPORT_SYMBOL_GPL(typec_charger_unregister_partner);
> > > diff --git a/drivers/usb/typec/charger.h b/drivers/usb/typec/charger.h
> > > new file mode 100644
> > > index 000000000000..32cdaa7c1a83
> > > --- /dev/null
> > > +++ b/drivers/usb/typec/charger.h
> > > @@ -0,0 +1,33 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > > +#ifndef __USB_TYPEC_CHARGER_H__
> > > +#define __USB_TYPEC_CHARGER_H__
> > > +
> > > +#include <linux/power_supply.h>
> > > +#include <linux/usb/typec.h>
> > > +
> > > +#include "class.h"
> > > +
> > > +#define TYPEC_CHARGER_DIR_NAME
> > >     "TYPEC_CHARGER%d"
> > > +#define TYPEC_CHARGER_DIR_NAME_LENGTH
> > >     sizeof(TYPEC_CHARGER_DIR_NAME)
> > > +
> > > +struct typec_charger {
> > > +   struct typec_port *port;
> > > +   struct typec_partner *partner;
> > > +   char name[TYPEC_CHARGER_DIR_NAME_LENGTH];
> > > +   struct power_supply *psy;
> > > +   struct power_supply_desc psy_desc;
> > > +   int psy_usb_type;
> > > +   int psy_online;
> > > +   int psy_status;
> > > +};
> > > +
> > > +struct typec_charger *typec_register_charger(struct typec_port *port);
> > > +void typec_unregister_charger(struct typec_charger *charger);
> > > +
> > > +void typec_charger_register_partner(struct typec_charger *charger,
> > > +                               struct typec_partner *partner);
> > > +void typec_charger_unregister_partner(struct typec_charger *charger);
> > > +void typec_charger_changed(struct typec_charger *charger);
> > > +
> > > +#endif /* __USB_TYPEC_CHARGER_H__ */
> > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > > index 9a1fdce137b9..1542d3af342c 100644
> > > --- a/drivers/usb/typec/class.c
> > > +++ b/drivers/usb/typec/class.c
> > > @@ -13,6 +13,7 @@
> > >  #include <linux/slab.h>
> > >
> > >  #include "bus.h"
> > > +#include "charger.h"
> > >  #include "class.h"
> > >
> > >  static DEFINE_IDA(typec_index_ida);
> > > @@ -489,6 +490,12 @@ static void typec_partner_release(struct device *dev)
> > >  {
> > >     struct typec_partner *partner = to_typec_partner(dev);
> > >
> > > +   if (IS_ENABLED(CONFIG_TYPEC_CHARGER)) {
> > > +           struct typec_port *port = to_typec_port(dev->parent);
> > > +
> > > +           typec_charger_unregister_partner(port->charger);
> > > +   }
> > > +
> > >     ida_destroy(&partner->mode_ids);
> > >     kfree(partner);
> > >  }
> > > @@ -580,6 +587,10 @@ struct typec_partner *typec_register_partner(struct
> > > typec_port *port,
> > >             return ERR_PTR(ret);
> > >     }
> > >
> > > +   if (IS_ENABLED(CONFIG_TYPEC_CHARGER) && port->charger) {
> > > +           typec_charger_register_partner(port->charger, partner);
> > > +   }
> > > +
> > >     return partner;
> > >  }
> > >  EXPORT_SYMBOL_GPL(typec_register_partner);
> > > @@ -1283,6 +1294,9 @@ static void typec_release(struct device *dev)
> > >  {
> > >     struct typec_port *port = to_typec_port(dev);
> > >
> > > +   if (IS_ENABLED(CONFIG_TYPEC_CHARGER))
> > > +           typec_unregister_charger(port->charger);
> > > +
> > >     ida_simple_remove(&typec_index_ida, port->id);
> > >     ida_destroy(&port->mode_ids);
> > >     typec_switch_put(port->sw);
> > > @@ -1564,7 +1578,8 @@ struct typec_port *typec_register_port(struct device
> > > *parent,
> > >     id = ida_simple_get(&typec_index_ida, 0, 0, GFP_KERNEL);
> > >     if (id < 0) {
> > >             kfree(port);
> > > -           return ERR_PTR(id);
> > > +           ret = id;
> > > +           goto err_return;
> > >     }
> > >
> > >     switch (cap->type) {
> > > @@ -1617,32 +1632,47 @@ struct typec_port *typec_register_port(struct device
> > > *parent,
> > >
> > >     port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL);
> > >     if (!port->cap) {
> > > -           put_device(&port->dev);
> > > -           return ERR_PTR(-ENOMEM);
> > > +           ret = -ENOMEM;
> > > +           goto err_put_device;
> > >     }
> > >
> > >     port->sw = typec_switch_get(&port->dev);
> > >     if (IS_ERR(port->sw)) {
> > >             ret = PTR_ERR(port->sw);
> > > -           put_device(&port->dev);
> > > -           return ERR_PTR(ret);
> > > +           goto err_put_device;
> > >     }
> > >
> > >     port->mux = typec_mux_get(&port->dev, NULL);
> > >     if (IS_ERR(port->mux)) {
> > >             ret = PTR_ERR(port->mux);
> > > -           put_device(&port->dev);
> > > -           return ERR_PTR(ret);
> > > +           goto err_put_device;
> > >     }
> > >
> > >     ret = device_add(&port->dev);
> > >     if (ret) {
> > >             dev_err(parent, "failed to register port (%d)\n", ret);
> > > -           put_device(&port->dev);
> > > -           return ERR_PTR(ret);
> > > +           goto err_put_device;
> > > +   }
> > > +
> > > +   if (IS_ENABLED(CONFIG_TYPEC_CHARGER)) {
> > > +           port->charger = typec_register_charger(port);
> > > +
> > > +           if (IS_ERR(port->charger)) {
> > > +                   ret = PTR_ERR(port->charger);
> > > +                   goto err_device_del;
> > > +           }
> > >     }
> > >
> > >     return port;
> > > +
> > > +err_device_del:
> > > +   device_del(&port->dev);
> > > +
> > > +err_put_device:
> > > +   put_device(&port->dev);
> > > +
> > > +err_return:
> > > +   return ERR_PTR(ret);
> > >  }
> > >  EXPORT_SYMBOL_GPL(typec_register_port);
> > >
> > > diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> > > index ec933dfe1323..0ff0a590d316 100644
> > > --- a/drivers/usb/typec/class.h
> > > +++ b/drivers/usb/typec/class.h
> > > @@ -41,6 +41,8 @@ struct typec_port {
> > >     struct typec_switch             *sw;
> > >     struct typec_mux                *mux;
> > >
> > > +   struct typec_charger            *charger;
> > > +
> > >     const struct typec_capability   *cap;
> > >     const struct typec_operations   *ops;
> > >  };
> > > --
> > > 2.26.1.301.g55bc3eb7cb9-goog
>
> --
> heikki

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

end of thread, other threads:[~2020-04-21 15:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-20 16:36 [PATCH v0 0/2] Type-C charger support using power_supply Mathew King
2020-04-20 16:36 ` [PATCH v0 1/2] typec: Move typec class structs into a header file Mathew King
2020-04-20 16:36 ` [PATCH v0 2/2] typec: Add Type-C charger Mathew King
2020-04-21  8:44   ` Adam Thomson
2020-04-21 12:41     ` Heikki Krogerus
2020-04-21 15:56       ` Mat King

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.