[PATCH 1/5] extcon: Allow extcon drivers to specify the extcon name

[PATCH 1/5] extcon: Allow extcon drivers to specify the extcon name

[Hans de Goede]

The parent device name is not necessarily always useful, e.g.
with i2c devices it may simply be e.g.: "0-0022" and it also depends
on the i2c-bus number which depends on probe ordering.

This commit allows drivers to set their own, more useful name,
avoiding the problems with some i2c-device names.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index f422a78..f8d3c1b 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1117,7 +1117,8 @@ int extcon_dev_register(struct extcon_dev *edev)
 	edev->dev.class = extcon_class;
 	edev->dev.release = extcon_dev_release;
 
-	edev->name = dev_name(edev->dev.parent);
+	if (!edev->name)
+		edev->name = dev_name(edev->dev.parent);
 	if (IS_ERR_OR_NULL(edev->name)) {
 		dev_err(&edev->dev,
 			"extcon device name is null\n");
-- 
2.9.3

[PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Hans de Goede]

Add support for USB TYPE-C cable detection on systems using a
FUSB302 USB TYPE-C controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/Kconfig          |   8 +
 drivers/extcon/Makefile         |   1 +
 drivers/extcon/extcon-fusb302.c | 782 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 791 insertions(+)
 create mode 100644 drivers/extcon/extcon-fusb302.c

diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index 32f2dc8..562db5b 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -35,6 +35,14 @@ config EXTCON_AXP288
 	  Say Y here to enable support for USB peripheral detection
 	  and USB MUX switching by X-Power AXP288 PMIC.
 
+config EXTCON_FUSB302
+	tristate "FUSB302 USB TYPE-C controller support"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say Y here to enable support for USB TYPE-C cable detection using
+	  a FUSB302 USB TYPE-C controller.
+
 config EXTCON_GPIO
 	tristate "GPIO extcon support"
 	depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
index ecfa958..e5eb493 100644
--- a/drivers/extcon/Makefile
+++ b/drivers/extcon/Makefile
@@ -7,6 +7,7 @@ extcon-core-objs		+= extcon.o devres.o
 obj-$(CONFIG_EXTCON_ADC_JACK)	+= extcon-adc-jack.o
 obj-$(CONFIG_EXTCON_ARIZONA)	+= extcon-arizona.o
 obj-$(CONFIG_EXTCON_AXP288)	+= extcon-axp288.o
+obj-$(CONFIG_EXTCON_FUSB302)	+= extcon-fusb302.o
 obj-$(CONFIG_EXTCON_GPIO)	+= extcon-gpio.o
 obj-$(CONFIG_EXTCON_INTEL_INT3496) += extcon-intel-int3496.o
 obj-$(CONFIG_EXTCON_INTEL_CHT_WC) += extcon-intel-cht-wc.o
diff --git a/drivers/extcon/extcon-fusb302.c b/drivers/extcon/extcon-fusb302.c
new file mode 100644
index 0000000..577adb9
--- /dev/null
+++ b/drivers/extcon/extcon-fusb302.c
@@ -0,0 +1,782 @@
+/*
+ * Extcon USB-C cable detection driver for FUSB302 USB TYPE-C controller
+ *
+ * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/extcon.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+#include "extcon.h"
+
+#define REG_DEVICE_ID				0x01
+#define DEVICE_ID_VER_MASK			GENMASK(7, 4)
+#define DEVICE_ID_FUSB302_VER			0x80
+
+#define REG_SWITCHES0				0x02
+#define SWITCHES0_PDWN1				BIT(0)
+#define SWITCHES0_PDWN2				BIT(1)
+#define SWITCHES0_PDWN				GENMASK(1, 0)
+#define SWITCHES0_MEAS_CC1			BIT(2)
+#define SWITCHES0_MEAS_CC2			BIT(3)
+#define SWITCHES0_VCONN_CC1			BIT(4)
+#define SWITCHES0_VCONN_CC2			BIT(5)
+#define SWITCHES0_PU_EN1			BIT(6)
+#define SWITCHES0_PU_EN2			BIT(7)
+
+#define REG_MEASURE				0x04
+#define MEASURE_MDAC_MASK			GENMASK(5, 0)
+/* Datasheet says MDAC must be set to 0x34 / 2226mV for vRd-3.0 detection */
+#define MEASURE_MDAC_SNK_VRD30			0x34
+/* MDAC must be set to 0x25 / 1600mV for disconnect det. with 80uA host-cur */
+#define MEASURE_MDAC_SRC_80UA_HOST_CUR		0x25
+
+#define REG_CONTROL0				0x06
+#define CONTROL0_HOST_CUR_MASK			GENMASK(3, 2)
+#define CONTROL0_HOST_CUR_DISABLED		(0 << 2)
+#define CONTROL0_HOST_CUR_80UA			(1 << 2)
+#define CONTROL0_HOST_CUR_180UA			(2 << 2)
+#define CONTROL0_HOST_CUR_330UA			(3 << 2)
+#define CONTROL0_INT_MASK			BIT(5)
+
+#define REG_CONTROL2				0x08
+#define CONTROL2_TOGGLE				BIT(0)
+#define CONTROL2_MODE_MASK			GENMASK(2, 1)
+#define CONTROL2_MODE_DRP			(1 << 1)
+#define CONTROL2_MODE_SNK			(2 << 1)
+#define CONTROL2_MODE_SRC			(3 << 1)
+#define CONTROL2_SAVE_PWR_MASK			GENMASK(7, 6)
+#define CONTROL2_SAVE_PWR_DISABLED		(0 << 6)
+#define CONTROL2_SAVE_PWR_40MS			(1 << 6)
+#define CONTROL2_SAVE_PWR_80MS			(2 << 6)
+#define CONTROL2_SAVE_PWR_160MS			(3 << 6)
+
+#define REG_MASK1				0x0a
+/* REG_MASK1 value for low-power / disabled state from datasheet */
+#define MASK1_DISABLED				0xfe
+#define MASK1_COMP_CHNG				BIT(5)
+#define MASK1_VBUSOK				BIT(7)
+
+#define REG_POWER				0x0b
+/* REG_POWER values for disabled and normal state from datasheet */
+#define POWER_DISABLED				BIT(0)
+#define POWER_NORMAL				GENMASK(2, 0)
+
+#define REG_RESET				0x0c
+#define RESET_SW_RESET				BIT(0)
+
+#define REG_OCP					0x0d
+
+#define REG_MASKA				0x0e
+/* REG_MASKA value for low-power / disabled state from datasheet */
+#define MASKA_DISABLED				0xbf
+
+#define REG_MASKB				0x0f
+/* REG_MASKB value for low-power / disabled state from datasheet */
+#define MASKB_DISABLED				0x01
+
+#define REG_STATUS1A				0x3d
+#define STATUS1A_TOGSS_MASK			GENMASK(5, 3)
+#define STATUS1A_TOGSS_SRC_CC1			(1 << 3)
+#define STATUS1A_TOGSS_SRC_CC2			(2 << 3)
+#define STATUS1A_TOGSS_SNK_CC1			(5 << 3)
+#define STATUS1A_TOGSS_SNK_CC2			(6 << 3)
+
+#define REG_INTERRUPTA				0x3e
+#define INTERRUPTA_TOGDONE			BIT(6)
+
+#define REG_STATUS0				0x40
+#define STATUS0_BC_LEVEL_MASK			GENMASK(1, 0)
+#define STATUS0_BC_LEVEL_VRA			0
+#define STATUS0_BC_LEVEL_VRDUSB			1
+#define STATUS0_BC_LEVEL_VRD15			2
+#define STATUS0_BC_LEVEL_VRD30			3
+#define STATUS0_COMP				BIT(5)
+#define STATUS0_VBUSOK				BIT(7)
+
+#define REG_INTERRUPT				0x42
+#define INTERRUPT_BC_LVL			BIT(0)
+#define INTERRUPT_COMP_CHNG			BIT(5)
+#define INTERRUPT_VBUSOK			BIT(7)
+
+/* Timeouts from the FUSB302 datasheet */
+#define TTOGCYCLE				msecs_to_jiffies(40 + 60 + 40)
+
+/* Timeouts from the USB-C specification */
+#define TPDDEBOUNCE				msecs_to_jiffies(20)
+#define TDRPTRY_MS				75
+#define TDRPTRY					msecs_to_jiffies(TDRPTRY_MS)
+#define TDRPTRYWAIT				msecs_to_jiffies(400)
+#define TVBUSON					msecs_to_jiffies(275)
+
+enum typec_port_type {
+	TYPEC_PORT_DFP,
+	TYPEC_PORT_UFP,
+	TYPEC_PORT_DRP,
+};
+
+enum typec_role {
+	TYPEC_SINK,
+	TYPEC_SOURCE,
+};
+
+enum fusb302_state {
+	DISABLED_SNK, /* UFP */
+	DISABLED_SRC, /* DFP */
+	DISABLED_DRP,
+	UNATTACHED_SNK,
+	ATTACHED_SNK,
+	UNATTACHED_SRC,
+	ATTACHED_SRC,
+};
+/* For debugging */
+static __maybe_unused const char *fusb302_state_str[] = {
+	"DISABLED_SNK",
+	"DISABLED_SRC",
+	"DISABLED_DRP",
+	"UNATTACHED_SNK",
+	"ATTACHED_SNK",
+	"UNATTACHED_SRC",
+	"ATTACHED_SRC",
+};
+
+enum fusb302_event {
+	TOGGLE_DONE,
+	BC_LVL_CHANGE,
+	VBUS_VALID,
+	VBUS_INVALID,
+	COMP_LOW,
+	COMP_HIGH,
+	TIMEOUT,
+	FALLBACK_TIMEOUT,
+};
+/* For debugging */
+static __maybe_unused const char *fusb302_event_str[] = {
+	"TOGGLE_DONE",
+	"BC_LVL_CHANGE",
+	"VBUS_VALID",
+	"VBUS_INVALID",
+	"COMP_LOW",
+	"COMP_HIGH",
+	"TIMEOUT",
+	"FALLBACK_TIMEOUT",
+};
+
+static const unsigned int fusb302_extcon_cables[] = {
+	EXTCON_USB,
+	EXTCON_USB_HOST,
+	EXTCON_CHG_USB_SDP,
+	EXTCON_CHG_USB_CDP,
+	EXTCON_CHG_USB_FAST,
+	EXTCON_NONE,
+};
+
+struct fusb302_data {
+	struct device *dev;
+	struct regmap *regmap;
+	struct extcon_dev *edev;
+	struct mutex lock;
+	struct delayed_work timeout;
+	struct delayed_work fallback_timeout;
+	struct delayed_work bc_work;
+	enum fusb302_state state;
+	enum typec_port_type port_type;
+	enum typec_role preferred_role;
+	int status0;
+	int status1a;
+	unsigned int snk_cable_id;
+};
+
+static void fusb302_write_reg(struct fusb302_data *data, int reg, int val)
+{
+	int ret;
+
+	ret = regmap_write(data->regmap, reg, val);
+	if (ret)
+		dev_err(data->dev, "Error writing reg %02x: %d\n", reg, ret);
+}
+
+static int fusb302_read_reg(struct fusb302_data *data, int reg)
+{
+	int ret, val;
+
+	ret = regmap_read(data->regmap, reg, &val);
+	if (ret) {
+		dev_err(data->dev, "Error reading reg %02x: %d\n", reg, ret);
+		return 0;
+	}
+
+	return val;
+}
+
+static void fusb302_update_bits(struct fusb302_data *data, int reg,
+				int mask, int val)
+{
+	int ret;
+
+	ret = regmap_update_bits(data->regmap, reg, mask, val);
+	if (ret)
+		dev_err(data->dev, "Error modifying reg %02x: %d\n", reg, ret);
+}
+
+/* Small helper to sync EXTCON_CHG_USB_SDP and EXTCON_USB state */
+static void fusb302_set_extcon_state(struct fusb302_data *data,
+				     unsigned int cable, bool state)
+{
+	extcon_set_state_sync(data->edev, cable, state);
+	if (cable == EXTCON_CHG_USB_SDP)
+		extcon_set_state_sync(data->edev, EXTCON_USB, state);
+}
+
+/* Helper for the disabled states */
+static void fusb302_disable(struct fusb302_data *data)
+{
+	fusb302_write_reg(data, REG_POWER, POWER_DISABLED);
+	fusb302_write_reg(data, REG_MASK1, MASK1_DISABLED);
+	fusb302_write_reg(data, REG_MASKA, MASKA_DISABLED);
+	fusb302_write_reg(data, REG_MASKB, MASKB_DISABLED);
+	fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
+}
+
+/*
+ * fusb302_set_state() and fusb302_handle_event() implement the 3 state
+ * machines from the datasheet folded into 1 state-machine for code re-use.
+ */
+static void fusb302_set_state(struct fusb302_data *data,
+			      enum fusb302_state state)
+{
+	int status, switches0 = 0;
+	enum fusb302_state old_state = data->state;
+	union extcon_property_value prop;
+
+	/* Kill pending timeouts from previous state */
+	cancel_delayed_work(&data->timeout);
+	cancel_delayed_work(&data->fallback_timeout);
+	cancel_delayed_work(&data->bc_work);
+
+	dev_dbg(data->dev, "New state %s\n", fusb302_state_str[state]);
+
+	switch (old_state) {
+	case ATTACHED_SNK:
+		fusb302_set_extcon_state(data, data->snk_cable_id, false);
+		break;
+	case ATTACHED_SRC:
+		fusb302_set_extcon_state(data, EXTCON_USB_HOST, false);
+		break;
+	default:
+		break; /* Do nothing */
+	}
+
+	switch (state) {
+	case DISABLED_SNK:
+		fusb302_disable(data);
+		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
+				    CONTROL2_MODE_SNK);
+		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
+				    CONTROL2_TOGGLE);
+		break;
+
+	case DISABLED_SRC:
+		fusb302_disable(data);
+		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
+				    CONTROL2_MODE_SRC);
+		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
+				    CONTROL2_TOGGLE);
+		break;
+
+	case DISABLED_DRP:
+		fusb302_disable(data);
+		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
+				    CONTROL2_MODE_DRP);
+		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
+				    CONTROL2_TOGGLE);
+		break;
+
+	case UNATTACHED_SNK:
+		fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
+
+		/* Enable pull-down on CC1 / CC2 based on orientation */
+		switch (data->status1a & STATUS1A_TOGSS_MASK) {
+		case STATUS1A_TOGSS_SNK_CC1:
+			switches0 = SWITCHES0_PDWN1 | SWITCHES0_MEAS_CC1;
+			prop.intval = USB_TYPEC_POLARITY_NORMAL;
+			break;
+		case STATUS1A_TOGSS_SNK_CC2:
+			switches0 = SWITCHES0_PDWN2 | SWITCHES0_MEAS_CC2;
+			prop.intval = USB_TYPEC_POLARITY_FLIP;
+			break;
+		}
+		fusb302_write_reg(data, REG_SWITCHES0, switches0);
+		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
+		extcon_set_property(data->edev, EXTCON_USB,
+				    EXTCON_PROP_USB_TYPEC_POLARITY, prop);
+
+		/* Enable VBUSOK and COMP irq at 2.24V for BC detection */
+		fusb302_update_bits(data, REG_MASK1,
+				    MASK1_VBUSOK | MASK1_COMP_CHNG, 0);
+		fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
+				    MEASURE_MDAC_SNK_VRD30);
+
+		status = fusb302_read_reg(data, REG_STATUS0);
+		if (status & STATUS0_VBUSOK) {
+			/* Go straight to ATTACHED_SNK */
+			fusb302_set_state(data, ATTACHED_SNK);
+			return;
+		}
+
+		mod_delayed_work(system_wq, &data->timeout, TVBUSON);
+		break;
+
+	case ATTACHED_SNK:
+		mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
+		break;
+
+	case UNATTACHED_SRC:
+		fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
+
+		/* Enable pull-up / Vconn on CC1 / CC2 based on orientation */
+		switch (data->status1a & STATUS1A_TOGSS_MASK) {
+		case STATUS1A_TOGSS_SRC_CC1:
+			switches0 = SWITCHES0_PU_EN1 | SWITCHES0_VCONN_CC2 |
+				    SWITCHES0_MEAS_CC1;
+			prop.intval = USB_TYPEC_POLARITY_NORMAL;
+			break;
+		case STATUS1A_TOGSS_SRC_CC2:
+			switches0 = SWITCHES0_PU_EN2 | SWITCHES0_VCONN_CC1 |
+				    SWITCHES0_MEAS_CC2;
+			prop.intval = USB_TYPEC_POLARITY_FLIP;
+			break;
+		}
+		fusb302_write_reg(data, REG_SWITCHES0, switches0);
+		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
+		extcon_set_property(data->edev, EXTCON_USB,
+				    EXTCON_PROP_USB_TYPEC_POLARITY, prop);
+
+		/* Enable COMP irq at 1.6V for detach detection */
+		fusb302_update_bits(data, REG_MASK1, MASK1_COMP_CHNG, 0);
+		fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
+				    MEASURE_MDAC_SRC_80UA_HOST_CUR);
+
+		status = fusb302_read_reg(data, REG_STATUS0);
+		if (!(status & STATUS0_COMP)) {
+			/* Go straight to ATTACHED_SRC */
+			fusb302_set_state(data, ATTACHED_SRC);
+			return;
+		}
+
+		mod_delayed_work(system_wq, &data->timeout, TPDDEBOUNCE);
+		break;
+
+	case ATTACHED_SRC:
+		fusb302_set_extcon_state(data, EXTCON_USB_HOST, true);
+		break;
+	}
+
+	data->state = state;
+}
+
+static void fusb302_set_state_disabled(struct fusb302_data *data)
+{
+
+	switch (data->port_type) {
+	case TYPEC_PORT_UFP:
+		fusb302_set_state(data, DISABLED_SNK);
+		break;
+	case TYPEC_PORT_DFP:
+		fusb302_set_state(data, DISABLED_SRC);
+		break;
+	case TYPEC_PORT_DRP:
+		fusb302_set_state(data, DISABLED_DRP);
+		break;
+	}
+}
+
+static void fusb302_handle_disabled_snk_event(struct fusb302_data *data,
+					      int event)
+{
+	switch (event) {
+	case TOGGLE_DONE:
+		switch (data->status1a & STATUS1A_TOGSS_MASK) {
+		case STATUS1A_TOGSS_SNK_CC1:
+		case STATUS1A_TOGSS_SNK_CC2:
+			fusb302_set_state(data, UNATTACHED_SNK);
+			break;
+		}
+		break;
+
+	case TIMEOUT:
+		/* Cannot become snk fallback to src */
+		fusb302_set_state(data, DISABLED_SRC);
+		mod_delayed_work(system_wq, &data->fallback_timeout,
+				 TDRPTRYWAIT);
+		break;
+
+	case FALLBACK_TIMEOUT:
+		/* Both states failed return to disabled drp state */
+		fusb302_set_state(data, DISABLED_DRP);
+		break;
+	}
+}
+
+static void fusb302_handle_disabled_src_event(struct fusb302_data *data,
+					      int event)
+{
+	switch (event) {
+	case TOGGLE_DONE:
+		switch (data->status1a & STATUS1A_TOGSS_MASK) {
+		case STATUS1A_TOGSS_SRC_CC1:
+		case STATUS1A_TOGSS_SRC_CC2:
+			fusb302_set_state(data, UNATTACHED_SRC);
+			break;
+		}
+		break;
+
+	case TIMEOUT:
+		/* Cannot become src fallback to snk */
+		fusb302_set_state(data, DISABLED_SNK);
+		mod_delayed_work(system_wq, &data->fallback_timeout,
+				 TDRPTRYWAIT);
+		break;
+
+	case FALLBACK_TIMEOUT:
+		/* Both states failed return to disabled drp state */
+		fusb302_set_state(data, DISABLED_DRP);
+		break;
+	}
+}
+
+static void fusb302_handle_disabled_drp_event(struct fusb302_data *data,
+					      int event)
+{
+	switch (event) {
+	case TOGGLE_DONE:
+		switch (data->status1a & STATUS1A_TOGSS_MASK) {
+		case STATUS1A_TOGSS_SNK_CC1:
+		case STATUS1A_TOGSS_SNK_CC2:
+			if (data->preferred_role == TYPEC_SINK) {
+				/* Jump directly to UNATTACHED_SNK */
+				fusb302_set_state(data, UNATTACHED_SNK);
+			} else {
+				/* Try to become src */
+				fusb302_set_state(data, DISABLED_SNK);
+				mod_delayed_work(system_wq, &data->timeout,
+						 TDRPTRY);
+			}
+			break;
+		case STATUS1A_TOGSS_SRC_CC1:
+		case STATUS1A_TOGSS_SRC_CC2:
+			if (data->preferred_role == TYPEC_SOURCE) {
+				/* Jump directly to UNATTACHED_SRC */
+				fusb302_set_state(data, UNATTACHED_SRC);
+			} else {
+				/*
+				 * The USB-C spec says we must enable pull-downs
+				 * and then wait tDRPTry before checking to
+				 * avoid endless role-bouncing if both ends
+				 * prefer the snk role.
+				 */
+				fusb302_write_reg(data, REG_SWITCHES0,
+						  SWITCHES0_PDWN);
+				fusb302_update_bits(data, REG_CONTROL2,
+						    CONTROL2_TOGGLE, 0);
+				msleep(TDRPTRY_MS);
+				/*
+				 * Use the toggle engine to do the src
+				 * detection to keep things the same as for
+				 * directly entering the src role.
+				 */
+				fusb302_set_state(data, DISABLED_SNK);
+				mod_delayed_work(system_wq, &data->timeout,
+						 TTOGCYCLE);
+			}
+			break;
+		}
+		break;
+	}
+}
+
+static void fusb302_handle_unattached_snk_event(struct fusb302_data *data,
+					      int event)
+{
+	switch (event) {
+	case VBUS_VALID: /* Cable attached */
+		fusb302_set_state(data, ATTACHED_SNK);
+		break;
+	case TIMEOUT:
+		fusb302_set_state_disabled(data);
+		break;
+	}
+}
+
+static void fusb302_handle_attached_snk_event(struct fusb302_data *data,
+					      int event)
+{
+	switch (event) {
+	case BC_LVL_CHANGE:
+	case COMP_LOW:
+	case COMP_HIGH:
+		mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
+		break;
+	case VBUS_INVALID: /* Cable detached */
+		fusb302_set_state_disabled(data);
+		break;
+	}
+}
+
+static void fusb302_handle_unattached_src_event(struct fusb302_data *data,
+					      int event)
+{
+	switch (event) {
+	case COMP_LOW: /* Cable attached */
+		fusb302_set_state(data, ATTACHED_SRC);
+		break;
+	case TIMEOUT:
+		fusb302_set_state_disabled(data);
+		break;
+	}
+}
+
+static void fusb302_handle_attached_src_event(struct fusb302_data *data,
+					      int event)
+{
+	switch (event) {
+	case COMP_HIGH: /* Cable detached */
+		fusb302_set_state_disabled(data);
+		break;
+	}
+}
+
+static void fusb302_handle_event(struct fusb302_data *data, int event)
+{
+
+	mutex_lock(&data->lock);
+
+	dev_dbg(data->dev, "Handling state %s event %s status %02x %02x\n",
+		fusb302_state_str[data->state], fusb302_event_str[event],
+		fusb302_read_reg(data, REG_STATUS0),
+		fusb302_read_reg(data, REG_STATUS1A));
+
+	switch (data->state) {
+	case DISABLED_SNK:
+		fusb302_handle_disabled_snk_event(data, event);
+		break;
+	case DISABLED_SRC:
+		fusb302_handle_disabled_src_event(data, event);
+		break;
+	case DISABLED_DRP:
+		fusb302_handle_disabled_drp_event(data, event);
+		break;
+	case UNATTACHED_SNK:
+		fusb302_handle_unattached_snk_event(data, event);
+		break;
+	case ATTACHED_SNK:
+		fusb302_handle_attached_snk_event(data, event);
+		break;
+	case UNATTACHED_SRC:
+		fusb302_handle_unattached_src_event(data, event);
+		break;
+	case ATTACHED_SRC:
+		fusb302_handle_attached_src_event(data, event);
+		break;
+	}
+	mutex_unlock(&data->lock);
+}
+
+static void fusb302_timeout(struct work_struct *work)
+{
+	struct fusb302_data *data =
+		container_of(work, struct fusb302_data, timeout.work);
+
+	fusb302_handle_event(data, TIMEOUT);
+}
+
+static void fusb302_fallback_timeout(struct work_struct *work)
+{
+	struct fusb302_data *data =
+		container_of(work, struct fusb302_data, fallback_timeout.work);
+
+	fusb302_handle_event(data, FALLBACK_TIMEOUT);
+}
+
+static void fusb302_report_bc_level(struct work_struct *work)
+{
+	struct fusb302_data *data =
+		container_of(work, struct fusb302_data, bc_work.work);
+
+	/* Clear old charger cable id */
+	fusb302_set_extcon_state(data, data->snk_cable_id, false);
+
+	if (data->status0 & STATUS0_COMP) {
+		dev_warn(data->dev, "vRd over maximum, assuming 500mA source\n");
+		data->snk_cable_id = EXTCON_CHG_USB_SDP;
+	} else {
+		switch (data->status0 & STATUS0_BC_LEVEL_MASK) {
+		case STATUS0_BC_LEVEL_VRA:
+		case STATUS0_BC_LEVEL_VRDUSB:
+			data->snk_cable_id = EXTCON_CHG_USB_SDP;
+			break;
+		case STATUS0_BC_LEVEL_VRD15:
+			data->snk_cable_id = EXTCON_CHG_USB_CDP;
+			break;
+		case STATUS0_BC_LEVEL_VRD30:
+			/* Use CHG_USB_FAST to indicate 3A capability */
+			data->snk_cable_id = EXTCON_CHG_USB_FAST;
+			break;
+		}
+	}
+	fusb302_set_extcon_state(data, data->snk_cable_id, true);
+}
+
+static irqreturn_t fusb302_irq_handler_thread(int irq, void *handler_data)
+{
+	struct fusb302_data *data = handler_data;
+	int interrupt, interrupta;
+
+	interrupt = fusb302_read_reg(data, REG_INTERRUPT);
+	interrupta = fusb302_read_reg(data, REG_INTERRUPTA);
+
+	if (interrupt)
+		data->status0 = fusb302_read_reg(data, REG_STATUS0);
+
+	if (interrupta)
+		data->status1a = fusb302_read_reg(data, REG_STATUS1A);
+
+	dev_dbg(data->dev, "Handling interrupt %02x %02x status %02x %02x\n",
+		interrupt, interrupta, data->status0, data->status1a);
+
+	if (interrupt & INTERRUPT_BC_LVL)
+		fusb302_handle_event(data, BC_LVL_CHANGE);
+
+	if (interrupt & INTERRUPT_COMP_CHNG) {
+		if (data->status0 & STATUS0_COMP)
+			fusb302_handle_event(data, COMP_HIGH);
+		else
+			fusb302_handle_event(data, COMP_LOW);
+	}
+
+	if (interrupt & INTERRUPT_VBUSOK) {
+		if (data->status0 & STATUS0_VBUSOK)
+			fusb302_handle_event(data, VBUS_VALID);
+		else
+			fusb302_handle_event(data, VBUS_INVALID);
+	}
+
+	if (interrupta & INTERRUPTA_TOGDONE)
+		fusb302_handle_event(data, TOGGLE_DONE);
+
+	return IRQ_HANDLED;
+}
+
+static const struct regmap_config fusb302_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.val_format_endian = REGMAP_ENDIAN_NATIVE,
+};
+
+static int fusb302_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct fusb302_data *data;
+	int ret;
+
+	if (!client->irq) {
+		dev_err(dev, "Error irq not specified\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->dev = dev;
+	/* TODO make these 2 configurable using device-properties */
+	data->port_type = TYPEC_PORT_DRP;
+	data->preferred_role = TYPEC_SINK;
+
+	data->regmap = devm_regmap_init_i2c(client, &fusb302_regmap_config);
+	if (IS_ERR(data->regmap)) {
+		ret = PTR_ERR(data->regmap);
+		dev_err(dev, "Error to initializing regmap: %d\n", ret);
+		return ret;
+	}
+
+	mutex_init(&data->lock);
+	INIT_DELAYED_WORK(&data->timeout, fusb302_timeout);
+	INIT_DELAYED_WORK(&data->fallback_timeout, fusb302_fallback_timeout);
+	INIT_DELAYED_WORK(&data->bc_work, fusb302_report_bc_level);
+
+	data->edev = devm_extcon_dev_allocate(dev, fusb302_extcon_cables);
+	if (IS_ERR(data->edev))
+		return PTR_ERR(data->edev);
+
+	data->edev->name = "fusb302";
+
+	ret = devm_extcon_dev_register(dev, data->edev);
+	if (ret)
+		return ret;
+
+	extcon_set_property_capability(data->edev, EXTCON_USB,
+				       EXTCON_PROP_USB_TYPEC_POLARITY);
+
+	fusb302_write_reg(data, REG_RESET, RESET_SW_RESET);
+	usleep_range(10000, 20000);
+
+	/* Enable power-saving */
+	fusb302_update_bits(data, REG_CONTROL2, CONTROL2_SAVE_PWR_MASK,
+			    CONTROL2_SAVE_PWR_40MS);
+
+	fusb302_set_state_disabled(data);
+
+	ret = devm_request_threaded_irq(dev, client->irq, NULL,
+			fusb302_irq_handler_thread,
+			IRQF_TRIGGER_LOW | IRQF_ONESHOT, "fusb302", data);
+	if (ret) {
+		dev_err(dev, "Error requesting irq: %d\n", ret);
+		return ret;
+	}
+
+	fusb302_update_bits(data, REG_CONTROL0, CONTROL0_INT_MASK, 0);
+
+	i2c_set_clientdata(client, data);
+	return 0;
+}
+
+static int fusb302_remove(struct i2c_client *client)
+{
+	struct fusb302_data *data = i2c_get_clientdata(client);
+
+	devm_free_irq(data->dev, client->irq, data);
+	cancel_delayed_work_sync(&data->timeout);
+	cancel_delayed_work_sync(&data->fallback_timeout);
+
+	return 0;
+}
+
+static const struct i2c_device_id fusb302_i2c_ids[] = {
+	{ "fusb302" },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, fusb302_i2c_ids);
+
+static struct i2c_driver fusb302_driver = {
+	.probe_new = fusb302_probe,
+	.remove = fusb302_remove,
+	.id_table = fusb302_i2c_ids,
+	.driver = {
+		.name = "fusb302",
+	},
+};
+module_i2c_driver(fusb302_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("FUSB302 USB TYPE-C controller Driver");
-- 
2.9.3

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Andy Shevchenko]

+Cc: Heikki.

He might comment on this.

On Fri, Apr 21, 2017 at 4:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Add support for USB TYPE-C cable detection on systems using a
> FUSB302 USB TYPE-C controller.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/Kconfig          |   8 +
>  drivers/extcon/Makefile         |   1 +
>  drivers/extcon/extcon-fusb302.c | 782 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 791 insertions(+)
>  create mode 100644 drivers/extcon/extcon-fusb302.c
>
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index 32f2dc8..562db5b 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -35,6 +35,14 @@ config EXTCON_AXP288
>           Say Y here to enable support for USB peripheral detection
>           and USB MUX switching by X-Power AXP288 PMIC.
>
> +config EXTCON_FUSB302
> +       tristate "FUSB302 USB TYPE-C controller support"
> +       depends on I2C
> +       select REGMAP_I2C
> +       help
> +         Say Y here to enable support for USB TYPE-C cable detection using
> +         a FUSB302 USB TYPE-C controller.
> +
>  config EXTCON_GPIO
>         tristate "GPIO extcon support"
>         depends on GPIOLIB || COMPILE_TEST
> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> index ecfa958..e5eb493 100644
> --- a/drivers/extcon/Makefile
> +++ b/drivers/extcon/Makefile
> @@ -7,6 +7,7 @@ extcon-core-objs                += extcon.o devres.o
>  obj-$(CONFIG_EXTCON_ADC_JACK)  += extcon-adc-jack.o
>  obj-$(CONFIG_EXTCON_ARIZONA)   += extcon-arizona.o
>  obj-$(CONFIG_EXTCON_AXP288)    += extcon-axp288.o
> +obj-$(CONFIG_EXTCON_FUSB302)   += extcon-fusb302.o
>  obj-$(CONFIG_EXTCON_GPIO)      += extcon-gpio.o
>  obj-$(CONFIG_EXTCON_INTEL_INT3496) += extcon-intel-int3496.o
>  obj-$(CONFIG_EXTCON_INTEL_CHT_WC) += extcon-intel-cht-wc.o
> diff --git a/drivers/extcon/extcon-fusb302.c b/drivers/extcon/extcon-fusb302.c
> new file mode 100644
> index 0000000..577adb9
> --- /dev/null
> +++ b/drivers/extcon/extcon-fusb302.c
> @@ -0,0 +1,782 @@
> +/*
> + * Extcon USB-C cable detection driver for FUSB302 USB TYPE-C controller
> + *
> + * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/extcon.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +#include "extcon.h"
> +
> +#define REG_DEVICE_ID                          0x01
> +#define DEVICE_ID_VER_MASK                     GENMASK(7, 4)
> +#define DEVICE_ID_FUSB302_VER                  0x80
> +
> +#define REG_SWITCHES0                          0x02
> +#define SWITCHES0_PDWN1                                BIT(0)
> +#define SWITCHES0_PDWN2                                BIT(1)
> +#define SWITCHES0_PDWN                         GENMASK(1, 0)
> +#define SWITCHES0_MEAS_CC1                     BIT(2)
> +#define SWITCHES0_MEAS_CC2                     BIT(3)
> +#define SWITCHES0_VCONN_CC1                    BIT(4)
> +#define SWITCHES0_VCONN_CC2                    BIT(5)
> +#define SWITCHES0_PU_EN1                       BIT(6)
> +#define SWITCHES0_PU_EN2                       BIT(7)
> +
> +#define REG_MEASURE                            0x04
> +#define MEASURE_MDAC_MASK                      GENMASK(5, 0)
> +/* Datasheet says MDAC must be set to 0x34 / 2226mV for vRd-3.0 detection */
> +#define MEASURE_MDAC_SNK_VRD30                 0x34
> +/* MDAC must be set to 0x25 / 1600mV for disconnect det. with 80uA host-cur */
> +#define MEASURE_MDAC_SRC_80UA_HOST_CUR         0x25
> +
> +#define REG_CONTROL0                           0x06
> +#define CONTROL0_HOST_CUR_MASK                 GENMASK(3, 2)
> +#define CONTROL0_HOST_CUR_DISABLED             (0 << 2)
> +#define CONTROL0_HOST_CUR_80UA                 (1 << 2)
> +#define CONTROL0_HOST_CUR_180UA                        (2 << 2)
> +#define CONTROL0_HOST_CUR_330UA                        (3 << 2)
> +#define CONTROL0_INT_MASK                      BIT(5)
> +
> +#define REG_CONTROL2                           0x08
> +#define CONTROL2_TOGGLE                                BIT(0)
> +#define CONTROL2_MODE_MASK                     GENMASK(2, 1)
> +#define CONTROL2_MODE_DRP                      (1 << 1)
> +#define CONTROL2_MODE_SNK                      (2 << 1)
> +#define CONTROL2_MODE_SRC                      (3 << 1)
> +#define CONTROL2_SAVE_PWR_MASK                 GENMASK(7, 6)
> +#define CONTROL2_SAVE_PWR_DISABLED             (0 << 6)
> +#define CONTROL2_SAVE_PWR_40MS                 (1 << 6)
> +#define CONTROL2_SAVE_PWR_80MS                 (2 << 6)
> +#define CONTROL2_SAVE_PWR_160MS                        (3 << 6)
> +
> +#define REG_MASK1                              0x0a
> +/* REG_MASK1 value for low-power / disabled state from datasheet */
> +#define MASK1_DISABLED                         0xfe
> +#define MASK1_COMP_CHNG                                BIT(5)
> +#define MASK1_VBUSOK                           BIT(7)
> +
> +#define REG_POWER                              0x0b
> +/* REG_POWER values for disabled and normal state from datasheet */
> +#define POWER_DISABLED                         BIT(0)
> +#define POWER_NORMAL                           GENMASK(2, 0)
> +
> +#define REG_RESET                              0x0c
> +#define RESET_SW_RESET                         BIT(0)
> +
> +#define REG_OCP                                        0x0d
> +
> +#define REG_MASKA                              0x0e
> +/* REG_MASKA value for low-power / disabled state from datasheet */
> +#define MASKA_DISABLED                         0xbf
> +
> +#define REG_MASKB                              0x0f
> +/* REG_MASKB value for low-power / disabled state from datasheet */
> +#define MASKB_DISABLED                         0x01
> +
> +#define REG_STATUS1A                           0x3d
> +#define STATUS1A_TOGSS_MASK                    GENMASK(5, 3)
> +#define STATUS1A_TOGSS_SRC_CC1                 (1 << 3)
> +#define STATUS1A_TOGSS_SRC_CC2                 (2 << 3)
> +#define STATUS1A_TOGSS_SNK_CC1                 (5 << 3)
> +#define STATUS1A_TOGSS_SNK_CC2                 (6 << 3)
> +
> +#define REG_INTERRUPTA                         0x3e
> +#define INTERRUPTA_TOGDONE                     BIT(6)
> +
> +#define REG_STATUS0                            0x40
> +#define STATUS0_BC_LEVEL_MASK                  GENMASK(1, 0)
> +#define STATUS0_BC_LEVEL_VRA                   0
> +#define STATUS0_BC_LEVEL_VRDUSB                        1
> +#define STATUS0_BC_LEVEL_VRD15                 2
> +#define STATUS0_BC_LEVEL_VRD30                 3
> +#define STATUS0_COMP                           BIT(5)
> +#define STATUS0_VBUSOK                         BIT(7)
> +
> +#define REG_INTERRUPT                          0x42
> +#define INTERRUPT_BC_LVL                       BIT(0)
> +#define INTERRUPT_COMP_CHNG                    BIT(5)
> +#define INTERRUPT_VBUSOK                       BIT(7)
> +
> +/* Timeouts from the FUSB302 datasheet */
> +#define TTOGCYCLE                              msecs_to_jiffies(40 + 60 + 40)
> +
> +/* Timeouts from the USB-C specification */
> +#define TPDDEBOUNCE                            msecs_to_jiffies(20)
> +#define TDRPTRY_MS                             75
> +#define TDRPTRY                                        msecs_to_jiffies(TDRPTRY_MS)
> +#define TDRPTRYWAIT                            msecs_to_jiffies(400)
> +#define TVBUSON                                        msecs_to_jiffies(275)
> +
> +enum typec_port_type {
> +       TYPEC_PORT_DFP,
> +       TYPEC_PORT_UFP,
> +       TYPEC_PORT_DRP,
> +};
> +
> +enum typec_role {
> +       TYPEC_SINK,
> +       TYPEC_SOURCE,
> +};
> +
> +enum fusb302_state {
> +       DISABLED_SNK, /* UFP */
> +       DISABLED_SRC, /* DFP */
> +       DISABLED_DRP,
> +       UNATTACHED_SNK,
> +       ATTACHED_SNK,
> +       UNATTACHED_SRC,
> +       ATTACHED_SRC,
> +};
> +/* For debugging */
> +static __maybe_unused const char *fusb302_state_str[] = {
> +       "DISABLED_SNK",
> +       "DISABLED_SRC",
> +       "DISABLED_DRP",
> +       "UNATTACHED_SNK",
> +       "ATTACHED_SNK",
> +       "UNATTACHED_SRC",
> +       "ATTACHED_SRC",
> +};
> +
> +enum fusb302_event {
> +       TOGGLE_DONE,
> +       BC_LVL_CHANGE,
> +       VBUS_VALID,
> +       VBUS_INVALID,
> +       COMP_LOW,
> +       COMP_HIGH,
> +       TIMEOUT,
> +       FALLBACK_TIMEOUT,
> +};
> +/* For debugging */
> +static __maybe_unused const char *fusb302_event_str[] = {
> +       "TOGGLE_DONE",
> +       "BC_LVL_CHANGE",
> +       "VBUS_VALID",
> +       "VBUS_INVALID",
> +       "COMP_LOW",
> +       "COMP_HIGH",
> +       "TIMEOUT",
> +       "FALLBACK_TIMEOUT",
> +};
> +
> +static const unsigned int fusb302_extcon_cables[] = {
> +       EXTCON_USB,
> +       EXTCON_USB_HOST,
> +       EXTCON_CHG_USB_SDP,
> +       EXTCON_CHG_USB_CDP,
> +       EXTCON_CHG_USB_FAST,
> +       EXTCON_NONE,
> +};
> +
> +struct fusb302_data {
> +       struct device *dev;
> +       struct regmap *regmap;
> +       struct extcon_dev *edev;
> +       struct mutex lock;
> +       struct delayed_work timeout;
> +       struct delayed_work fallback_timeout;
> +       struct delayed_work bc_work;
> +       enum fusb302_state state;
> +       enum typec_port_type port_type;
> +       enum typec_role preferred_role;
> +       int status0;
> +       int status1a;
> +       unsigned int snk_cable_id;
> +};
> +
> +static void fusb302_write_reg(struct fusb302_data *data, int reg, int val)
> +{
> +       int ret;
> +
> +       ret = regmap_write(data->regmap, reg, val);
> +       if (ret)
> +               dev_err(data->dev, "Error writing reg %02x: %d\n", reg, ret);
> +}
> +
> +static int fusb302_read_reg(struct fusb302_data *data, int reg)
> +{
> +       int ret, val;
> +
> +       ret = regmap_read(data->regmap, reg, &val);
> +       if (ret) {
> +               dev_err(data->dev, "Error reading reg %02x: %d\n", reg, ret);
> +               return 0;
> +       }
> +
> +       return val;
> +}
> +
> +static void fusb302_update_bits(struct fusb302_data *data, int reg,
> +                               int mask, int val)
> +{
> +       int ret;
> +
> +       ret = regmap_update_bits(data->regmap, reg, mask, val);
> +       if (ret)
> +               dev_err(data->dev, "Error modifying reg %02x: %d\n", reg, ret);
> +}
> +
> +/* Small helper to sync EXTCON_CHG_USB_SDP and EXTCON_USB state */
> +static void fusb302_set_extcon_state(struct fusb302_data *data,
> +                                    unsigned int cable, bool state)
> +{
> +       extcon_set_state_sync(data->edev, cable, state);
> +       if (cable == EXTCON_CHG_USB_SDP)
> +               extcon_set_state_sync(data->edev, EXTCON_USB, state);
> +}
> +
> +/* Helper for the disabled states */
> +static void fusb302_disable(struct fusb302_data *data)
> +{
> +       fusb302_write_reg(data, REG_POWER, POWER_DISABLED);
> +       fusb302_write_reg(data, REG_MASK1, MASK1_DISABLED);
> +       fusb302_write_reg(data, REG_MASKA, MASKA_DISABLED);
> +       fusb302_write_reg(data, REG_MASKB, MASKB_DISABLED);
> +       fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
> +}
> +
> +/*
> + * fusb302_set_state() and fusb302_handle_event() implement the 3 state
> + * machines from the datasheet folded into 1 state-machine for code re-use.
> + */
> +static void fusb302_set_state(struct fusb302_data *data,
> +                             enum fusb302_state state)
> +{
> +       int status, switches0 = 0;
> +       enum fusb302_state old_state = data->state;
> +       union extcon_property_value prop;
> +
> +       /* Kill pending timeouts from previous state */
> +       cancel_delayed_work(&data->timeout);
> +       cancel_delayed_work(&data->fallback_timeout);
> +       cancel_delayed_work(&data->bc_work);
> +
> +       dev_dbg(data->dev, "New state %s\n", fusb302_state_str[state]);
> +
> +       switch (old_state) {
> +       case ATTACHED_SNK:
> +               fusb302_set_extcon_state(data, data->snk_cable_id, false);
> +               break;
> +       case ATTACHED_SRC:
> +               fusb302_set_extcon_state(data, EXTCON_USB_HOST, false);
> +               break;
> +       default:
> +               break; /* Do nothing */
> +       }
> +
> +       switch (state) {
> +       case DISABLED_SNK:
> +               fusb302_disable(data);
> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
> +                                   CONTROL2_MODE_SNK);
> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
> +                                   CONTROL2_TOGGLE);
> +               break;
> +
> +       case DISABLED_SRC:
> +               fusb302_disable(data);
> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
> +                                   CONTROL2_MODE_SRC);
> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
> +                                   CONTROL2_TOGGLE);
> +               break;
> +
> +       case DISABLED_DRP:
> +               fusb302_disable(data);
> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
> +                                   CONTROL2_MODE_DRP);
> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
> +                                   CONTROL2_TOGGLE);
> +               break;
> +
> +       case UNATTACHED_SNK:
> +               fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
> +
> +               /* Enable pull-down on CC1 / CC2 based on orientation */
> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +               case STATUS1A_TOGSS_SNK_CC1:
> +                       switches0 = SWITCHES0_PDWN1 | SWITCHES0_MEAS_CC1;
> +                       prop.intval = USB_TYPEC_POLARITY_NORMAL;
> +                       break;
> +               case STATUS1A_TOGSS_SNK_CC2:
> +                       switches0 = SWITCHES0_PDWN2 | SWITCHES0_MEAS_CC2;
> +                       prop.intval = USB_TYPEC_POLARITY_FLIP;
> +                       break;
> +               }
> +               fusb302_write_reg(data, REG_SWITCHES0, switches0);
> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
> +               extcon_set_property(data->edev, EXTCON_USB,
> +                                   EXTCON_PROP_USB_TYPEC_POLARITY, prop);
> +
> +               /* Enable VBUSOK and COMP irq at 2.24V for BC detection */
> +               fusb302_update_bits(data, REG_MASK1,
> +                                   MASK1_VBUSOK | MASK1_COMP_CHNG, 0);
> +               fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
> +                                   MEASURE_MDAC_SNK_VRD30);
> +
> +               status = fusb302_read_reg(data, REG_STATUS0);
> +               if (status & STATUS0_VBUSOK) {
> +                       /* Go straight to ATTACHED_SNK */
> +                       fusb302_set_state(data, ATTACHED_SNK);
> +                       return;
> +               }
> +
> +               mod_delayed_work(system_wq, &data->timeout, TVBUSON);
> +               break;
> +
> +       case ATTACHED_SNK:
> +               mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
> +               break;
> +
> +       case UNATTACHED_SRC:
> +               fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
> +
> +               /* Enable pull-up / Vconn on CC1 / CC2 based on orientation */
> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +               case STATUS1A_TOGSS_SRC_CC1:
> +                       switches0 = SWITCHES0_PU_EN1 | SWITCHES0_VCONN_CC2 |
> +                                   SWITCHES0_MEAS_CC1;
> +                       prop.intval = USB_TYPEC_POLARITY_NORMAL;
> +                       break;
> +               case STATUS1A_TOGSS_SRC_CC2:
> +                       switches0 = SWITCHES0_PU_EN2 | SWITCHES0_VCONN_CC1 |
> +                                   SWITCHES0_MEAS_CC2;
> +                       prop.intval = USB_TYPEC_POLARITY_FLIP;
> +                       break;
> +               }
> +               fusb302_write_reg(data, REG_SWITCHES0, switches0);
> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
> +               extcon_set_property(data->edev, EXTCON_USB,
> +                                   EXTCON_PROP_USB_TYPEC_POLARITY, prop);
> +
> +               /* Enable COMP irq at 1.6V for detach detection */
> +               fusb302_update_bits(data, REG_MASK1, MASK1_COMP_CHNG, 0);
> +               fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
> +                                   MEASURE_MDAC_SRC_80UA_HOST_CUR);
> +
> +               status = fusb302_read_reg(data, REG_STATUS0);
> +               if (!(status & STATUS0_COMP)) {
> +                       /* Go straight to ATTACHED_SRC */
> +                       fusb302_set_state(data, ATTACHED_SRC);
> +                       return;
> +               }
> +
> +               mod_delayed_work(system_wq, &data->timeout, TPDDEBOUNCE);
> +               break;
> +
> +       case ATTACHED_SRC:
> +               fusb302_set_extcon_state(data, EXTCON_USB_HOST, true);
> +               break;
> +       }
> +
> +       data->state = state;
> +}
> +
> +static void fusb302_set_state_disabled(struct fusb302_data *data)
> +{
> +
> +       switch (data->port_type) {
> +       case TYPEC_PORT_UFP:
> +               fusb302_set_state(data, DISABLED_SNK);
> +               break;
> +       case TYPEC_PORT_DFP:
> +               fusb302_set_state(data, DISABLED_SRC);
> +               break;
> +       case TYPEC_PORT_DRP:
> +               fusb302_set_state(data, DISABLED_DRP);
> +               break;
> +       }
> +}
> +
> +static void fusb302_handle_disabled_snk_event(struct fusb302_data *data,
> +                                             int event)
> +{
> +       switch (event) {
> +       case TOGGLE_DONE:
> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +               case STATUS1A_TOGSS_SNK_CC1:
> +               case STATUS1A_TOGSS_SNK_CC2:
> +                       fusb302_set_state(data, UNATTACHED_SNK);
> +                       break;
> +               }
> +               break;
> +
> +       case TIMEOUT:
> +               /* Cannot become snk fallback to src */
> +               fusb302_set_state(data, DISABLED_SRC);
> +               mod_delayed_work(system_wq, &data->fallback_timeout,
> +                                TDRPTRYWAIT);
> +               break;
> +
> +       case FALLBACK_TIMEOUT:
> +               /* Both states failed return to disabled drp state */
> +               fusb302_set_state(data, DISABLED_DRP);
> +               break;
> +       }
> +}
> +
> +static void fusb302_handle_disabled_src_event(struct fusb302_data *data,
> +                                             int event)
> +{
> +       switch (event) {
> +       case TOGGLE_DONE:
> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +               case STATUS1A_TOGSS_SRC_CC1:
> +               case STATUS1A_TOGSS_SRC_CC2:
> +                       fusb302_set_state(data, UNATTACHED_SRC);
> +                       break;
> +               }
> +               break;
> +
> +       case TIMEOUT:
> +               /* Cannot become src fallback to snk */
> +               fusb302_set_state(data, DISABLED_SNK);
> +               mod_delayed_work(system_wq, &data->fallback_timeout,
> +                                TDRPTRYWAIT);
> +               break;
> +
> +       case FALLBACK_TIMEOUT:
> +               /* Both states failed return to disabled drp state */
> +               fusb302_set_state(data, DISABLED_DRP);
> +               break;
> +       }
> +}
> +
> +static void fusb302_handle_disabled_drp_event(struct fusb302_data *data,
> +                                             int event)
> +{
> +       switch (event) {
> +       case TOGGLE_DONE:
> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +               case STATUS1A_TOGSS_SNK_CC1:
> +               case STATUS1A_TOGSS_SNK_CC2:
> +                       if (data->preferred_role == TYPEC_SINK) {
> +                               /* Jump directly to UNATTACHED_SNK */
> +                               fusb302_set_state(data, UNATTACHED_SNK);
> +                       } else {
> +                               /* Try to become src */
> +                               fusb302_set_state(data, DISABLED_SNK);
> +                               mod_delayed_work(system_wq, &data->timeout,
> +                                                TDRPTRY);
> +                       }
> +                       break;
> +               case STATUS1A_TOGSS_SRC_CC1:
> +               case STATUS1A_TOGSS_SRC_CC2:
> +                       if (data->preferred_role == TYPEC_SOURCE) {
> +                               /* Jump directly to UNATTACHED_SRC */
> +                               fusb302_set_state(data, UNATTACHED_SRC);
> +                       } else {
> +                               /*
> +                                * The USB-C spec says we must enable pull-downs
> +                                * and then wait tDRPTry before checking to
> +                                * avoid endless role-bouncing if both ends
> +                                * prefer the snk role.
> +                                */
> +                               fusb302_write_reg(data, REG_SWITCHES0,
> +                                                 SWITCHES0_PDWN);
> +                               fusb302_update_bits(data, REG_CONTROL2,
> +                                                   CONTROL2_TOGGLE, 0);
> +                               msleep(TDRPTRY_MS);
> +                               /*
> +                                * Use the toggle engine to do the src
> +                                * detection to keep things the same as for
> +                                * directly entering the src role.
> +                                */
> +                               fusb302_set_state(data, DISABLED_SNK);
> +                               mod_delayed_work(system_wq, &data->timeout,
> +                                                TTOGCYCLE);
> +                       }
> +                       break;
> +               }
> +               break;
> +       }
> +}
> +
> +static void fusb302_handle_unattached_snk_event(struct fusb302_data *data,
> +                                             int event)
> +{
> +       switch (event) {
> +       case VBUS_VALID: /* Cable attached */
> +               fusb302_set_state(data, ATTACHED_SNK);
> +               break;
> +       case TIMEOUT:
> +               fusb302_set_state_disabled(data);
> +               break;
> +       }
> +}
> +
> +static void fusb302_handle_attached_snk_event(struct fusb302_data *data,
> +                                             int event)
> +{
> +       switch (event) {
> +       case BC_LVL_CHANGE:
> +       case COMP_LOW:
> +       case COMP_HIGH:
> +               mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
> +               break;
> +       case VBUS_INVALID: /* Cable detached */
> +               fusb302_set_state_disabled(data);
> +               break;
> +       }
> +}
> +
> +static void fusb302_handle_unattached_src_event(struct fusb302_data *data,
> +                                             int event)
> +{
> +       switch (event) {
> +       case COMP_LOW: /* Cable attached */
> +               fusb302_set_state(data, ATTACHED_SRC);
> +               break;
> +       case TIMEOUT:
> +               fusb302_set_state_disabled(data);
> +               break;
> +       }
> +}
> +
> +static void fusb302_handle_attached_src_event(struct fusb302_data *data,
> +                                             int event)
> +{
> +       switch (event) {
> +       case COMP_HIGH: /* Cable detached */
> +               fusb302_set_state_disabled(data);
> +               break;
> +       }
> +}
> +
> +static void fusb302_handle_event(struct fusb302_data *data, int event)
> +{
> +
> +       mutex_lock(&data->lock);
> +
> +       dev_dbg(data->dev, "Handling state %s event %s status %02x %02x\n",
> +               fusb302_state_str[data->state], fusb302_event_str[event],
> +               fusb302_read_reg(data, REG_STATUS0),
> +               fusb302_read_reg(data, REG_STATUS1A));
> +
> +       switch (data->state) {
> +       case DISABLED_SNK:
> +               fusb302_handle_disabled_snk_event(data, event);
> +               break;
> +       case DISABLED_SRC:
> +               fusb302_handle_disabled_src_event(data, event);
> +               break;
> +       case DISABLED_DRP:
> +               fusb302_handle_disabled_drp_event(data, event);
> +               break;
> +       case UNATTACHED_SNK:
> +               fusb302_handle_unattached_snk_event(data, event);
> +               break;
> +       case ATTACHED_SNK:
> +               fusb302_handle_attached_snk_event(data, event);
> +               break;
> +       case UNATTACHED_SRC:
> +               fusb302_handle_unattached_src_event(data, event);
> +               break;
> +       case ATTACHED_SRC:
> +               fusb302_handle_attached_src_event(data, event);
> +               break;
> +       }
> +       mutex_unlock(&data->lock);
> +}
> +
> +static void fusb302_timeout(struct work_struct *work)
> +{
> +       struct fusb302_data *data =
> +               container_of(work, struct fusb302_data, timeout.work);
> +
> +       fusb302_handle_event(data, TIMEOUT);
> +}
> +
> +static void fusb302_fallback_timeout(struct work_struct *work)
> +{
> +       struct fusb302_data *data =
> +               container_of(work, struct fusb302_data, fallback_timeout.work);
> +
> +       fusb302_handle_event(data, FALLBACK_TIMEOUT);
> +}
> +
> +static void fusb302_report_bc_level(struct work_struct *work)
> +{
> +       struct fusb302_data *data =
> +               container_of(work, struct fusb302_data, bc_work.work);
> +
> +       /* Clear old charger cable id */
> +       fusb302_set_extcon_state(data, data->snk_cable_id, false);
> +
> +       if (data->status0 & STATUS0_COMP) {
> +               dev_warn(data->dev, "vRd over maximum, assuming 500mA source\n");
> +               data->snk_cable_id = EXTCON_CHG_USB_SDP;
> +       } else {
> +               switch (data->status0 & STATUS0_BC_LEVEL_MASK) {
> +               case STATUS0_BC_LEVEL_VRA:
> +               case STATUS0_BC_LEVEL_VRDUSB:
> +                       data->snk_cable_id = EXTCON_CHG_USB_SDP;
> +                       break;
> +               case STATUS0_BC_LEVEL_VRD15:
> +                       data->snk_cable_id = EXTCON_CHG_USB_CDP;
> +                       break;
> +               case STATUS0_BC_LEVEL_VRD30:
> +                       /* Use CHG_USB_FAST to indicate 3A capability */
> +                       data->snk_cable_id = EXTCON_CHG_USB_FAST;
> +                       break;
> +               }
> +       }
> +       fusb302_set_extcon_state(data, data->snk_cable_id, true);
> +}
> +
> +static irqreturn_t fusb302_irq_handler_thread(int irq, void *handler_data)
> +{
> +       struct fusb302_data *data = handler_data;
> +       int interrupt, interrupta;
> +
> +       interrupt = fusb302_read_reg(data, REG_INTERRUPT);
> +       interrupta = fusb302_read_reg(data, REG_INTERRUPTA);
> +
> +       if (interrupt)
> +               data->status0 = fusb302_read_reg(data, REG_STATUS0);
> +
> +       if (interrupta)
> +               data->status1a = fusb302_read_reg(data, REG_STATUS1A);
> +
> +       dev_dbg(data->dev, "Handling interrupt %02x %02x status %02x %02x\n",
> +               interrupt, interrupta, data->status0, data->status1a);
> +
> +       if (interrupt & INTERRUPT_BC_LVL)
> +               fusb302_handle_event(data, BC_LVL_CHANGE);
> +
> +       if (interrupt & INTERRUPT_COMP_CHNG) {
> +               if (data->status0 & STATUS0_COMP)
> +                       fusb302_handle_event(data, COMP_HIGH);
> +               else
> +                       fusb302_handle_event(data, COMP_LOW);
> +       }
> +
> +       if (interrupt & INTERRUPT_VBUSOK) {
> +               if (data->status0 & STATUS0_VBUSOK)
> +                       fusb302_handle_event(data, VBUS_VALID);
> +               else
> +                       fusb302_handle_event(data, VBUS_INVALID);
> +       }
> +
> +       if (interrupta & INTERRUPTA_TOGDONE)
> +               fusb302_handle_event(data, TOGGLE_DONE);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static const struct regmap_config fusb302_regmap_config = {
> +       .reg_bits = 8,
> +       .val_bits = 8,
> +       .val_format_endian = REGMAP_ENDIAN_NATIVE,
> +};
> +
> +static int fusb302_probe(struct i2c_client *client)
> +{
> +       struct device *dev = &client->dev;
> +       struct fusb302_data *data;
> +       int ret;
> +
> +       if (!client->irq) {
> +               dev_err(dev, "Error irq not specified\n");
> +               return -EINVAL;
> +       }
> +
> +       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +
> +       data->dev = dev;
> +       /* TODO make these 2 configurable using device-properties */
> +       data->port_type = TYPEC_PORT_DRP;
> +       data->preferred_role = TYPEC_SINK;
> +
> +       data->regmap = devm_regmap_init_i2c(client, &fusb302_regmap_config);
> +       if (IS_ERR(data->regmap)) {
> +               ret = PTR_ERR(data->regmap);
> +               dev_err(dev, "Error to initializing regmap: %d\n", ret);
> +               return ret;
> +       }
> +
> +       mutex_init(&data->lock);
> +       INIT_DELAYED_WORK(&data->timeout, fusb302_timeout);
> +       INIT_DELAYED_WORK(&data->fallback_timeout, fusb302_fallback_timeout);
> +       INIT_DELAYED_WORK(&data->bc_work, fusb302_report_bc_level);
> +
> +       data->edev = devm_extcon_dev_allocate(dev, fusb302_extcon_cables);
> +       if (IS_ERR(data->edev))
> +               return PTR_ERR(data->edev);
> +
> +       data->edev->name = "fusb302";
> +
> +       ret = devm_extcon_dev_register(dev, data->edev);
> +       if (ret)
> +               return ret;
> +
> +       extcon_set_property_capability(data->edev, EXTCON_USB,
> +                                      EXTCON_PROP_USB_TYPEC_POLARITY);
> +
> +       fusb302_write_reg(data, REG_RESET, RESET_SW_RESET);
> +       usleep_range(10000, 20000);
> +
> +       /* Enable power-saving */
> +       fusb302_update_bits(data, REG_CONTROL2, CONTROL2_SAVE_PWR_MASK,
> +                           CONTROL2_SAVE_PWR_40MS);
> +
> +       fusb302_set_state_disabled(data);
> +
> +       ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +                       fusb302_irq_handler_thread,
> +                       IRQF_TRIGGER_LOW | IRQF_ONESHOT, "fusb302", data);
> +       if (ret) {
> +               dev_err(dev, "Error requesting irq: %d\n", ret);
> +               return ret;
> +       }
> +
> +       fusb302_update_bits(data, REG_CONTROL0, CONTROL0_INT_MASK, 0);
> +
> +       i2c_set_clientdata(client, data);
> +       return 0;
> +}
> +
> +static int fusb302_remove(struct i2c_client *client)
> +{
> +       struct fusb302_data *data = i2c_get_clientdata(client);
> +
> +       devm_free_irq(data->dev, client->irq, data);
> +       cancel_delayed_work_sync(&data->timeout);
> +       cancel_delayed_work_sync(&data->fallback_timeout);
> +
> +       return 0;
> +}
> +
> +static const struct i2c_device_id fusb302_i2c_ids[] = {
> +       { "fusb302" },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(i2c, fusb302_i2c_ids);
> +
> +static struct i2c_driver fusb302_driver = {
> +       .probe_new = fusb302_probe,
> +       .remove = fusb302_remove,
> +       .id_table = fusb302_i2c_ids,
> +       .driver = {
> +               .name = "fusb302",
> +       },
> +};
> +module_i2c_driver(fusb302_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
> +MODULE_DESCRIPTION("FUSB302 USB TYPE-C controller Driver");
> --
> 2.9.3
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Heikki Krogerus]

+Guenter

On Fri, Apr 21, 2017 at 09:51:50PM +0300, Andy Shevchenko wrote:
> +Cc: Heikki.
> 
> He might comment on this.

Thanks Andy.

> On Fri, Apr 21, 2017 at 4:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Add support for USB TYPE-C cable detection on systems using a
> > FUSB302 USB TYPE-C controller.
> >
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> >  drivers/extcon/Kconfig          |   8 +
> >  drivers/extcon/Makefile         |   1 +
> >  drivers/extcon/extcon-fusb302.c | 782 ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 791 insertions(+)
> >  create mode 100644 drivers/extcon/extcon-fusb302.c

There is now the typec class in linux-next that really needs to be
used with all USB Type-C PHYs like fusb302.

Unless I'm mistaken, Guenter has also written a driver for fusb302. I
have not seen it, but if I understood correctly, that driver will
register itself with the upcoming tcpm (USB Type-C Port Manager) [1],
which in practice would mean we can take properly advantage of the USB
PD transceiver on fusb302.

Guenter! Can you publish the fusb302 driver?


[1] https://chromium-review.googlesource.com/c/389916/


> > diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> > index 32f2dc8..562db5b 100644
> > --- a/drivers/extcon/Kconfig
> > +++ b/drivers/extcon/Kconfig
> > @@ -35,6 +35,14 @@ config EXTCON_AXP288
> >           Say Y here to enable support for USB peripheral detection
> >           and USB MUX switching by X-Power AXP288 PMIC.
> >
> > +config EXTCON_FUSB302
> > +       tristate "FUSB302 USB TYPE-C controller support"
> > +       depends on I2C
> > +       select REGMAP_I2C
> > +       help
> > +         Say Y here to enable support for USB TYPE-C cable detection using
> > +         a FUSB302 USB TYPE-C controller.
> > +
> >  config EXTCON_GPIO
> >         tristate "GPIO extcon support"
> >         depends on GPIOLIB || COMPILE_TEST
> > diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> > index ecfa958..e5eb493 100644
> > --- a/drivers/extcon/Makefile
> > +++ b/drivers/extcon/Makefile
> > @@ -7,6 +7,7 @@ extcon-core-objs                += extcon.o devres.o
> >  obj-$(CONFIG_EXTCON_ADC_JACK)  += extcon-adc-jack.o
> >  obj-$(CONFIG_EXTCON_ARIZONA)   += extcon-arizona.o
> >  obj-$(CONFIG_EXTCON_AXP288)    += extcon-axp288.o
> > +obj-$(CONFIG_EXTCON_FUSB302)   += extcon-fusb302.o
> >  obj-$(CONFIG_EXTCON_GPIO)      += extcon-gpio.o
> >  obj-$(CONFIG_EXTCON_INTEL_INT3496) += extcon-intel-int3496.o
> >  obj-$(CONFIG_EXTCON_INTEL_CHT_WC) += extcon-intel-cht-wc.o
> > diff --git a/drivers/extcon/extcon-fusb302.c b/drivers/extcon/extcon-fusb302.c
> > new file mode 100644
> > index 0000000..577adb9
> > --- /dev/null
> > +++ b/drivers/extcon/extcon-fusb302.c
> > @@ -0,0 +1,782 @@
> > +/*
> > + * Extcon USB-C cable detection driver for FUSB302 USB TYPE-C controller
> > + *
> > + * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/extcon.h>
> > +#include <linux/i2c.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +#include <linux/workqueue.h>
> > +#include "extcon.h"
> > +
> > +#define REG_DEVICE_ID                          0x01
> > +#define DEVICE_ID_VER_MASK                     GENMASK(7, 4)
> > +#define DEVICE_ID_FUSB302_VER                  0x80
> > +
> > +#define REG_SWITCHES0                          0x02
> > +#define SWITCHES0_PDWN1                                BIT(0)
> > +#define SWITCHES0_PDWN2                                BIT(1)
> > +#define SWITCHES0_PDWN                         GENMASK(1, 0)
> > +#define SWITCHES0_MEAS_CC1                     BIT(2)
> > +#define SWITCHES0_MEAS_CC2                     BIT(3)
> > +#define SWITCHES0_VCONN_CC1                    BIT(4)
> > +#define SWITCHES0_VCONN_CC2                    BIT(5)
> > +#define SWITCHES0_PU_EN1                       BIT(6)
> > +#define SWITCHES0_PU_EN2                       BIT(7)
> > +
> > +#define REG_MEASURE                            0x04
> > +#define MEASURE_MDAC_MASK                      GENMASK(5, 0)
> > +/* Datasheet says MDAC must be set to 0x34 / 2226mV for vRd-3.0 detection */
> > +#define MEASURE_MDAC_SNK_VRD30                 0x34
> > +/* MDAC must be set to 0x25 / 1600mV for disconnect det. with 80uA host-cur */
> > +#define MEASURE_MDAC_SRC_80UA_HOST_CUR         0x25
> > +
> > +#define REG_CONTROL0                           0x06
> > +#define CONTROL0_HOST_CUR_MASK                 GENMASK(3, 2)
> > +#define CONTROL0_HOST_CUR_DISABLED             (0 << 2)
> > +#define CONTROL0_HOST_CUR_80UA                 (1 << 2)
> > +#define CONTROL0_HOST_CUR_180UA                        (2 << 2)
> > +#define CONTROL0_HOST_CUR_330UA                        (3 << 2)
> > +#define CONTROL0_INT_MASK                      BIT(5)
> > +
> > +#define REG_CONTROL2                           0x08
> > +#define CONTROL2_TOGGLE                                BIT(0)
> > +#define CONTROL2_MODE_MASK                     GENMASK(2, 1)
> > +#define CONTROL2_MODE_DRP                      (1 << 1)
> > +#define CONTROL2_MODE_SNK                      (2 << 1)
> > +#define CONTROL2_MODE_SRC                      (3 << 1)
> > +#define CONTROL2_SAVE_PWR_MASK                 GENMASK(7, 6)
> > +#define CONTROL2_SAVE_PWR_DISABLED             (0 << 6)
> > +#define CONTROL2_SAVE_PWR_40MS                 (1 << 6)
> > +#define CONTROL2_SAVE_PWR_80MS                 (2 << 6)
> > +#define CONTROL2_SAVE_PWR_160MS                        (3 << 6)
> > +
> > +#define REG_MASK1                              0x0a
> > +/* REG_MASK1 value for low-power / disabled state from datasheet */
> > +#define MASK1_DISABLED                         0xfe
> > +#define MASK1_COMP_CHNG                                BIT(5)
> > +#define MASK1_VBUSOK                           BIT(7)
> > +
> > +#define REG_POWER                              0x0b
> > +/* REG_POWER values for disabled and normal state from datasheet */
> > +#define POWER_DISABLED                         BIT(0)
> > +#define POWER_NORMAL                           GENMASK(2, 0)
> > +
> > +#define REG_RESET                              0x0c
> > +#define RESET_SW_RESET                         BIT(0)
> > +
> > +#define REG_OCP                                        0x0d
> > +
> > +#define REG_MASKA                              0x0e
> > +/* REG_MASKA value for low-power / disabled state from datasheet */
> > +#define MASKA_DISABLED                         0xbf
> > +
> > +#define REG_MASKB                              0x0f
> > +/* REG_MASKB value for low-power / disabled state from datasheet */
> > +#define MASKB_DISABLED                         0x01
> > +
> > +#define REG_STATUS1A                           0x3d
> > +#define STATUS1A_TOGSS_MASK                    GENMASK(5, 3)
> > +#define STATUS1A_TOGSS_SRC_CC1                 (1 << 3)
> > +#define STATUS1A_TOGSS_SRC_CC2                 (2 << 3)
> > +#define STATUS1A_TOGSS_SNK_CC1                 (5 << 3)
> > +#define STATUS1A_TOGSS_SNK_CC2                 (6 << 3)
> > +
> > +#define REG_INTERRUPTA                         0x3e
> > +#define INTERRUPTA_TOGDONE                     BIT(6)
> > +
> > +#define REG_STATUS0                            0x40
> > +#define STATUS0_BC_LEVEL_MASK                  GENMASK(1, 0)
> > +#define STATUS0_BC_LEVEL_VRA                   0
> > +#define STATUS0_BC_LEVEL_VRDUSB                        1
> > +#define STATUS0_BC_LEVEL_VRD15                 2
> > +#define STATUS0_BC_LEVEL_VRD30                 3
> > +#define STATUS0_COMP                           BIT(5)
> > +#define STATUS0_VBUSOK                         BIT(7)
> > +
> > +#define REG_INTERRUPT                          0x42
> > +#define INTERRUPT_BC_LVL                       BIT(0)
> > +#define INTERRUPT_COMP_CHNG                    BIT(5)
> > +#define INTERRUPT_VBUSOK                       BIT(7)
> > +
> > +/* Timeouts from the FUSB302 datasheet */
> > +#define TTOGCYCLE                              msecs_to_jiffies(40 + 60 + 40)
> > +
> > +/* Timeouts from the USB-C specification */
> > +#define TPDDEBOUNCE                            msecs_to_jiffies(20)
> > +#define TDRPTRY_MS                             75
> > +#define TDRPTRY                                        msecs_to_jiffies(TDRPTRY_MS)
> > +#define TDRPTRYWAIT                            msecs_to_jiffies(400)
> > +#define TVBUSON                                        msecs_to_jiffies(275)
> > +
> > +enum typec_port_type {
> > +       TYPEC_PORT_DFP,
> > +       TYPEC_PORT_UFP,
> > +       TYPEC_PORT_DRP,
> > +};
> > +
> > +enum typec_role {
> > +       TYPEC_SINK,
> > +       TYPEC_SOURCE,
> > +};
> > +
> > +enum fusb302_state {
> > +       DISABLED_SNK, /* UFP */
> > +       DISABLED_SRC, /* DFP */
> > +       DISABLED_DRP,
> > +       UNATTACHED_SNK,
> > +       ATTACHED_SNK,
> > +       UNATTACHED_SRC,
> > +       ATTACHED_SRC,
> > +};
> > +/* For debugging */
> > +static __maybe_unused const char *fusb302_state_str[] = {
> > +       "DISABLED_SNK",
> > +       "DISABLED_SRC",
> > +       "DISABLED_DRP",
> > +       "UNATTACHED_SNK",
> > +       "ATTACHED_SNK",
> > +       "UNATTACHED_SRC",
> > +       "ATTACHED_SRC",
> > +};
> > +
> > +enum fusb302_event {
> > +       TOGGLE_DONE,
> > +       BC_LVL_CHANGE,
> > +       VBUS_VALID,
> > +       VBUS_INVALID,
> > +       COMP_LOW,
> > +       COMP_HIGH,
> > +       TIMEOUT,
> > +       FALLBACK_TIMEOUT,
> > +};
> > +/* For debugging */
> > +static __maybe_unused const char *fusb302_event_str[] = {
> > +       "TOGGLE_DONE",
> > +       "BC_LVL_CHANGE",
> > +       "VBUS_VALID",
> > +       "VBUS_INVALID",
> > +       "COMP_LOW",
> > +       "COMP_HIGH",
> > +       "TIMEOUT",
> > +       "FALLBACK_TIMEOUT",
> > +};
> > +
> > +static const unsigned int fusb302_extcon_cables[] = {
> > +       EXTCON_USB,
> > +       EXTCON_USB_HOST,
> > +       EXTCON_CHG_USB_SDP,
> > +       EXTCON_CHG_USB_CDP,
> > +       EXTCON_CHG_USB_FAST,
> > +       EXTCON_NONE,
> > +};
> > +
> > +struct fusb302_data {
> > +       struct device *dev;
> > +       struct regmap *regmap;
> > +       struct extcon_dev *edev;
> > +       struct mutex lock;
> > +       struct delayed_work timeout;
> > +       struct delayed_work fallback_timeout;
> > +       struct delayed_work bc_work;
> > +       enum fusb302_state state;
> > +       enum typec_port_type port_type;
> > +       enum typec_role preferred_role;
> > +       int status0;
> > +       int status1a;
> > +       unsigned int snk_cable_id;
> > +};
> > +
> > +static void fusb302_write_reg(struct fusb302_data *data, int reg, int val)
> > +{
> > +       int ret;
> > +
> > +       ret = regmap_write(data->regmap, reg, val);
> > +       if (ret)
> > +               dev_err(data->dev, "Error writing reg %02x: %d\n", reg, ret);
> > +}
> > +
> > +static int fusb302_read_reg(struct fusb302_data *data, int reg)
> > +{
> > +       int ret, val;
> > +
> > +       ret = regmap_read(data->regmap, reg, &val);
> > +       if (ret) {
> > +               dev_err(data->dev, "Error reading reg %02x: %d\n", reg, ret);
> > +               return 0;
> > +       }
> > +
> > +       return val;
> > +}
> > +
> > +static void fusb302_update_bits(struct fusb302_data *data, int reg,
> > +                               int mask, int val)
> > +{
> > +       int ret;
> > +
> > +       ret = regmap_update_bits(data->regmap, reg, mask, val);
> > +       if (ret)
> > +               dev_err(data->dev, "Error modifying reg %02x: %d\n", reg, ret);
> > +}
> > +
> > +/* Small helper to sync EXTCON_CHG_USB_SDP and EXTCON_USB state */
> > +static void fusb302_set_extcon_state(struct fusb302_data *data,
> > +                                    unsigned int cable, bool state)
> > +{
> > +       extcon_set_state_sync(data->edev, cable, state);
> > +       if (cable == EXTCON_CHG_USB_SDP)
> > +               extcon_set_state_sync(data->edev, EXTCON_USB, state);
> > +}
> > +
> > +/* Helper for the disabled states */
> > +static void fusb302_disable(struct fusb302_data *data)
> > +{
> > +       fusb302_write_reg(data, REG_POWER, POWER_DISABLED);
> > +       fusb302_write_reg(data, REG_MASK1, MASK1_DISABLED);
> > +       fusb302_write_reg(data, REG_MASKA, MASKA_DISABLED);
> > +       fusb302_write_reg(data, REG_MASKB, MASKB_DISABLED);
> > +       fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
> > +}
> > +
> > +/*
> > + * fusb302_set_state() and fusb302_handle_event() implement the 3 state
> > + * machines from the datasheet folded into 1 state-machine for code re-use.
> > + */
> > +static void fusb302_set_state(struct fusb302_data *data,
> > +                             enum fusb302_state state)
> > +{
> > +       int status, switches0 = 0;
> > +       enum fusb302_state old_state = data->state;
> > +       union extcon_property_value prop;
> > +
> > +       /* Kill pending timeouts from previous state */
> > +       cancel_delayed_work(&data->timeout);
> > +       cancel_delayed_work(&data->fallback_timeout);
> > +       cancel_delayed_work(&data->bc_work);
> > +
> > +       dev_dbg(data->dev, "New state %s\n", fusb302_state_str[state]);
> > +
> > +       switch (old_state) {
> > +       case ATTACHED_SNK:
> > +               fusb302_set_extcon_state(data, data->snk_cable_id, false);
> > +               break;
> > +       case ATTACHED_SRC:
> > +               fusb302_set_extcon_state(data, EXTCON_USB_HOST, false);
> > +               break;
> > +       default:
> > +               break; /* Do nothing */
> > +       }
> > +
> > +       switch (state) {
> > +       case DISABLED_SNK:
> > +               fusb302_disable(data);
> > +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
> > +                                   CONTROL2_MODE_SNK);
> > +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
> > +                                   CONTROL2_TOGGLE);
> > +               break;
> > +
> > +       case DISABLED_SRC:
> > +               fusb302_disable(data);
> > +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
> > +                                   CONTROL2_MODE_SRC);
> > +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
> > +                                   CONTROL2_TOGGLE);
> > +               break;
> > +
> > +       case DISABLED_DRP:
> > +               fusb302_disable(data);
> > +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
> > +                                   CONTROL2_MODE_DRP);
> > +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
> > +                                   CONTROL2_TOGGLE);
> > +               break;
> > +
> > +       case UNATTACHED_SNK:
> > +               fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
> > +
> > +               /* Enable pull-down on CC1 / CC2 based on orientation */
> > +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> > +               case STATUS1A_TOGSS_SNK_CC1:
> > +                       switches0 = SWITCHES0_PDWN1 | SWITCHES0_MEAS_CC1;
> > +                       prop.intval = USB_TYPEC_POLARITY_NORMAL;
> > +                       break;
> > +               case STATUS1A_TOGSS_SNK_CC2:
> > +                       switches0 = SWITCHES0_PDWN2 | SWITCHES0_MEAS_CC2;
> > +                       prop.intval = USB_TYPEC_POLARITY_FLIP;
> > +                       break;
> > +               }
> > +               fusb302_write_reg(data, REG_SWITCHES0, switches0);
> > +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
> > +               extcon_set_property(data->edev, EXTCON_USB,
> > +                                   EXTCON_PROP_USB_TYPEC_POLARITY, prop);
> > +
> > +               /* Enable VBUSOK and COMP irq at 2.24V for BC detection */
> > +               fusb302_update_bits(data, REG_MASK1,
> > +                                   MASK1_VBUSOK | MASK1_COMP_CHNG, 0);
> > +               fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
> > +                                   MEASURE_MDAC_SNK_VRD30);
> > +
> > +               status = fusb302_read_reg(data, REG_STATUS0);
> > +               if (status & STATUS0_VBUSOK) {
> > +                       /* Go straight to ATTACHED_SNK */
> > +                       fusb302_set_state(data, ATTACHED_SNK);
> > +                       return;
> > +               }
> > +
> > +               mod_delayed_work(system_wq, &data->timeout, TVBUSON);
> > +               break;
> > +
> > +       case ATTACHED_SNK:
> > +               mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
> > +               break;
> > +
> > +       case UNATTACHED_SRC:
> > +               fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
> > +
> > +               /* Enable pull-up / Vconn on CC1 / CC2 based on orientation */
> > +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> > +               case STATUS1A_TOGSS_SRC_CC1:
> > +                       switches0 = SWITCHES0_PU_EN1 | SWITCHES0_VCONN_CC2 |
> > +                                   SWITCHES0_MEAS_CC1;
> > +                       prop.intval = USB_TYPEC_POLARITY_NORMAL;
> > +                       break;
> > +               case STATUS1A_TOGSS_SRC_CC2:
> > +                       switches0 = SWITCHES0_PU_EN2 | SWITCHES0_VCONN_CC1 |
> > +                                   SWITCHES0_MEAS_CC2;
> > +                       prop.intval = USB_TYPEC_POLARITY_FLIP;
> > +                       break;
> > +               }
> > +               fusb302_write_reg(data, REG_SWITCHES0, switches0);
> > +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
> > +               extcon_set_property(data->edev, EXTCON_USB,
> > +                                   EXTCON_PROP_USB_TYPEC_POLARITY, prop);
> > +
> > +               /* Enable COMP irq at 1.6V for detach detection */
> > +               fusb302_update_bits(data, REG_MASK1, MASK1_COMP_CHNG, 0);
> > +               fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
> > +                                   MEASURE_MDAC_SRC_80UA_HOST_CUR);
> > +
> > +               status = fusb302_read_reg(data, REG_STATUS0);
> > +               if (!(status & STATUS0_COMP)) {
> > +                       /* Go straight to ATTACHED_SRC */
> > +                       fusb302_set_state(data, ATTACHED_SRC);
> > +                       return;
> > +               }
> > +
> > +               mod_delayed_work(system_wq, &data->timeout, TPDDEBOUNCE);
> > +               break;
> > +
> > +       case ATTACHED_SRC:
> > +               fusb302_set_extcon_state(data, EXTCON_USB_HOST, true);
> > +               break;
> > +       }
> > +
> > +       data->state = state;
> > +}
> > +
> > +static void fusb302_set_state_disabled(struct fusb302_data *data)
> > +{
> > +
> > +       switch (data->port_type) {
> > +       case TYPEC_PORT_UFP:
> > +               fusb302_set_state(data, DISABLED_SNK);
> > +               break;
> > +       case TYPEC_PORT_DFP:
> > +               fusb302_set_state(data, DISABLED_SRC);
> > +               break;
> > +       case TYPEC_PORT_DRP:
> > +               fusb302_set_state(data, DISABLED_DRP);
> > +               break;
> > +       }
> > +}
> > +
> > +static void fusb302_handle_disabled_snk_event(struct fusb302_data *data,
> > +                                             int event)
> > +{
> > +       switch (event) {
> > +       case TOGGLE_DONE:
> > +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> > +               case STATUS1A_TOGSS_SNK_CC1:
> > +               case STATUS1A_TOGSS_SNK_CC2:
> > +                       fusb302_set_state(data, UNATTACHED_SNK);
> > +                       break;
> > +               }
> > +               break;
> > +
> > +       case TIMEOUT:
> > +               /* Cannot become snk fallback to src */
> > +               fusb302_set_state(data, DISABLED_SRC);
> > +               mod_delayed_work(system_wq, &data->fallback_timeout,
> > +                                TDRPTRYWAIT);
> > +               break;
> > +
> > +       case FALLBACK_TIMEOUT:
> > +               /* Both states failed return to disabled drp state */
> > +               fusb302_set_state(data, DISABLED_DRP);
> > +               break;
> > +       }
> > +}
> > +
> > +static void fusb302_handle_disabled_src_event(struct fusb302_data *data,
> > +                                             int event)
> > +{
> > +       switch (event) {
> > +       case TOGGLE_DONE:
> > +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> > +               case STATUS1A_TOGSS_SRC_CC1:
> > +               case STATUS1A_TOGSS_SRC_CC2:
> > +                       fusb302_set_state(data, UNATTACHED_SRC);
> > +                       break;
> > +               }
> > +               break;
> > +
> > +       case TIMEOUT:
> > +               /* Cannot become src fallback to snk */
> > +               fusb302_set_state(data, DISABLED_SNK);
> > +               mod_delayed_work(system_wq, &data->fallback_timeout,
> > +                                TDRPTRYWAIT);
> > +               break;
> > +
> > +       case FALLBACK_TIMEOUT:
> > +               /* Both states failed return to disabled drp state */
> > +               fusb302_set_state(data, DISABLED_DRP);
> > +               break;
> > +       }
> > +}
> > +
> > +static void fusb302_handle_disabled_drp_event(struct fusb302_data *data,
> > +                                             int event)
> > +{
> > +       switch (event) {
> > +       case TOGGLE_DONE:
> > +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
> > +               case STATUS1A_TOGSS_SNK_CC1:
> > +               case STATUS1A_TOGSS_SNK_CC2:
> > +                       if (data->preferred_role == TYPEC_SINK) {
> > +                               /* Jump directly to UNATTACHED_SNK */
> > +                               fusb302_set_state(data, UNATTACHED_SNK);
> > +                       } else {
> > +                               /* Try to become src */
> > +                               fusb302_set_state(data, DISABLED_SNK);
> > +                               mod_delayed_work(system_wq, &data->timeout,
> > +                                                TDRPTRY);
> > +                       }
> > +                       break;
> > +               case STATUS1A_TOGSS_SRC_CC1:
> > +               case STATUS1A_TOGSS_SRC_CC2:
> > +                       if (data->preferred_role == TYPEC_SOURCE) {
> > +                               /* Jump directly to UNATTACHED_SRC */
> > +                               fusb302_set_state(data, UNATTACHED_SRC);
> > +                       } else {
> > +                               /*
> > +                                * The USB-C spec says we must enable pull-downs
> > +                                * and then wait tDRPTry before checking to
> > +                                * avoid endless role-bouncing if both ends
> > +                                * prefer the snk role.
> > +                                */
> > +                               fusb302_write_reg(data, REG_SWITCHES0,
> > +                                                 SWITCHES0_PDWN);
> > +                               fusb302_update_bits(data, REG_CONTROL2,
> > +                                                   CONTROL2_TOGGLE, 0);
> > +                               msleep(TDRPTRY_MS);
> > +                               /*
> > +                                * Use the toggle engine to do the src
> > +                                * detection to keep things the same as for
> > +                                * directly entering the src role.
> > +                                */
> > +                               fusb302_set_state(data, DISABLED_SNK);
> > +                               mod_delayed_work(system_wq, &data->timeout,
> > +                                                TTOGCYCLE);
> > +                       }
> > +                       break;
> > +               }
> > +               break;
> > +       }
> > +}
> > +
> > +static void fusb302_handle_unattached_snk_event(struct fusb302_data *data,
> > +                                             int event)
> > +{
> > +       switch (event) {
> > +       case VBUS_VALID: /* Cable attached */
> > +               fusb302_set_state(data, ATTACHED_SNK);
> > +               break;
> > +       case TIMEOUT:
> > +               fusb302_set_state_disabled(data);
> > +               break;
> > +       }
> > +}
> > +
> > +static void fusb302_handle_attached_snk_event(struct fusb302_data *data,
> > +                                             int event)
> > +{
> > +       switch (event) {
> > +       case BC_LVL_CHANGE:
> > +       case COMP_LOW:
> > +       case COMP_HIGH:
> > +               mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
> > +               break;
> > +       case VBUS_INVALID: /* Cable detached */
> > +               fusb302_set_state_disabled(data);
> > +               break;
> > +       }
> > +}
> > +
> > +static void fusb302_handle_unattached_src_event(struct fusb302_data *data,
> > +                                             int event)
> > +{
> > +       switch (event) {
> > +       case COMP_LOW: /* Cable attached */
> > +               fusb302_set_state(data, ATTACHED_SRC);
> > +               break;
> > +       case TIMEOUT:
> > +               fusb302_set_state_disabled(data);
> > +               break;
> > +       }
> > +}
> > +
> > +static void fusb302_handle_attached_src_event(struct fusb302_data *data,
> > +                                             int event)
> > +{
> > +       switch (event) {
> > +       case COMP_HIGH: /* Cable detached */
> > +               fusb302_set_state_disabled(data);
> > +               break;
> > +       }
> > +}
> > +
> > +static void fusb302_handle_event(struct fusb302_data *data, int event)
> > +{
> > +
> > +       mutex_lock(&data->lock);
> > +
> > +       dev_dbg(data->dev, "Handling state %s event %s status %02x %02x\n",
> > +               fusb302_state_str[data->state], fusb302_event_str[event],
> > +               fusb302_read_reg(data, REG_STATUS0),
> > +               fusb302_read_reg(data, REG_STATUS1A));
> > +
> > +       switch (data->state) {
> > +       case DISABLED_SNK:
> > +               fusb302_handle_disabled_snk_event(data, event);
> > +               break;
> > +       case DISABLED_SRC:
> > +               fusb302_handle_disabled_src_event(data, event);
> > +               break;
> > +       case DISABLED_DRP:
> > +               fusb302_handle_disabled_drp_event(data, event);
> > +               break;
> > +       case UNATTACHED_SNK:
> > +               fusb302_handle_unattached_snk_event(data, event);
> > +               break;
> > +       case ATTACHED_SNK:
> > +               fusb302_handle_attached_snk_event(data, event);
> > +               break;
> > +       case UNATTACHED_SRC:
> > +               fusb302_handle_unattached_src_event(data, event);
> > +               break;
> > +       case ATTACHED_SRC:
> > +               fusb302_handle_attached_src_event(data, event);
> > +               break;
> > +       }
> > +       mutex_unlock(&data->lock);
> > +}
> > +
> > +static void fusb302_timeout(struct work_struct *work)
> > +{
> > +       struct fusb302_data *data =
> > +               container_of(work, struct fusb302_data, timeout.work);
> > +
> > +       fusb302_handle_event(data, TIMEOUT);
> > +}
> > +
> > +static void fusb302_fallback_timeout(struct work_struct *work)
> > +{
> > +       struct fusb302_data *data =
> > +               container_of(work, struct fusb302_data, fallback_timeout.work);
> > +
> > +       fusb302_handle_event(data, FALLBACK_TIMEOUT);
> > +}
> > +
> > +static void fusb302_report_bc_level(struct work_struct *work)
> > +{
> > +       struct fusb302_data *data =
> > +               container_of(work, struct fusb302_data, bc_work.work);
> > +
> > +       /* Clear old charger cable id */
> > +       fusb302_set_extcon_state(data, data->snk_cable_id, false);
> > +
> > +       if (data->status0 & STATUS0_COMP) {
> > +               dev_warn(data->dev, "vRd over maximum, assuming 500mA source\n");
> > +               data->snk_cable_id = EXTCON_CHG_USB_SDP;
> > +       } else {
> > +               switch (data->status0 & STATUS0_BC_LEVEL_MASK) {
> > +               case STATUS0_BC_LEVEL_VRA:
> > +               case STATUS0_BC_LEVEL_VRDUSB:
> > +                       data->snk_cable_id = EXTCON_CHG_USB_SDP;
> > +                       break;
> > +               case STATUS0_BC_LEVEL_VRD15:
> > +                       data->snk_cable_id = EXTCON_CHG_USB_CDP;
> > +                       break;
> > +               case STATUS0_BC_LEVEL_VRD30:
> > +                       /* Use CHG_USB_FAST to indicate 3A capability */
> > +                       data->snk_cable_id = EXTCON_CHG_USB_FAST;
> > +                       break;
> > +               }
> > +       }
> > +       fusb302_set_extcon_state(data, data->snk_cable_id, true);
> > +}
> > +
> > +static irqreturn_t fusb302_irq_handler_thread(int irq, void *handler_data)
> > +{
> > +       struct fusb302_data *data = handler_data;
> > +       int interrupt, interrupta;
> > +
> > +       interrupt = fusb302_read_reg(data, REG_INTERRUPT);
> > +       interrupta = fusb302_read_reg(data, REG_INTERRUPTA);
> > +
> > +       if (interrupt)
> > +               data->status0 = fusb302_read_reg(data, REG_STATUS0);
> > +
> > +       if (interrupta)
> > +               data->status1a = fusb302_read_reg(data, REG_STATUS1A);
> > +
> > +       dev_dbg(data->dev, "Handling interrupt %02x %02x status %02x %02x\n",
> > +               interrupt, interrupta, data->status0, data->status1a);
> > +
> > +       if (interrupt & INTERRUPT_BC_LVL)
> > +               fusb302_handle_event(data, BC_LVL_CHANGE);
> > +
> > +       if (interrupt & INTERRUPT_COMP_CHNG) {
> > +               if (data->status0 & STATUS0_COMP)
> > +                       fusb302_handle_event(data, COMP_HIGH);
> > +               else
> > +                       fusb302_handle_event(data, COMP_LOW);
> > +       }
> > +
> > +       if (interrupt & INTERRUPT_VBUSOK) {
> > +               if (data->status0 & STATUS0_VBUSOK)
> > +                       fusb302_handle_event(data, VBUS_VALID);
> > +               else
> > +                       fusb302_handle_event(data, VBUS_INVALID);
> > +       }
> > +
> > +       if (interrupta & INTERRUPTA_TOGDONE)
> > +               fusb302_handle_event(data, TOGGLE_DONE);
> > +
> > +       return IRQ_HANDLED;
> > +}
> > +
> > +static const struct regmap_config fusb302_regmap_config = {
> > +       .reg_bits = 8,
> > +       .val_bits = 8,
> > +       .val_format_endian = REGMAP_ENDIAN_NATIVE,
> > +};
> > +
> > +static int fusb302_probe(struct i2c_client *client)
> > +{
> > +       struct device *dev = &client->dev;
> > +       struct fusb302_data *data;
> > +       int ret;
> > +
> > +       if (!client->irq) {
> > +               dev_err(dev, "Error irq not specified\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > +       if (!data)
> > +               return -ENOMEM;
> > +
> > +       data->dev = dev;
> > +       /* TODO make these 2 configurable using device-properties */
> > +       data->port_type = TYPEC_PORT_DRP;
> > +       data->preferred_role = TYPEC_SINK;
> > +
> > +       data->regmap = devm_regmap_init_i2c(client, &fusb302_regmap_config);
> > +       if (IS_ERR(data->regmap)) {
> > +               ret = PTR_ERR(data->regmap);
> > +               dev_err(dev, "Error to initializing regmap: %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       mutex_init(&data->lock);
> > +       INIT_DELAYED_WORK(&data->timeout, fusb302_timeout);
> > +       INIT_DELAYED_WORK(&data->fallback_timeout, fusb302_fallback_timeout);
> > +       INIT_DELAYED_WORK(&data->bc_work, fusb302_report_bc_level);
> > +
> > +       data->edev = devm_extcon_dev_allocate(dev, fusb302_extcon_cables);
> > +       if (IS_ERR(data->edev))
> > +               return PTR_ERR(data->edev);
> > +
> > +       data->edev->name = "fusb302";
> > +
> > +       ret = devm_extcon_dev_register(dev, data->edev);
> > +       if (ret)
> > +               return ret;
> > +
> > +       extcon_set_property_capability(data->edev, EXTCON_USB,
> > +                                      EXTCON_PROP_USB_TYPEC_POLARITY);
> > +
> > +       fusb302_write_reg(data, REG_RESET, RESET_SW_RESET);
> > +       usleep_range(10000, 20000);
> > +
> > +       /* Enable power-saving */
> > +       fusb302_update_bits(data, REG_CONTROL2, CONTROL2_SAVE_PWR_MASK,
> > +                           CONTROL2_SAVE_PWR_40MS);
> > +
> > +       fusb302_set_state_disabled(data);
> > +
> > +       ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > +                       fusb302_irq_handler_thread,
> > +                       IRQF_TRIGGER_LOW | IRQF_ONESHOT, "fusb302", data);
> > +       if (ret) {
> > +               dev_err(dev, "Error requesting irq: %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       fusb302_update_bits(data, REG_CONTROL0, CONTROL0_INT_MASK, 0);
> > +
> > +       i2c_set_clientdata(client, data);
> > +       return 0;
> > +}
> > +
> > +static int fusb302_remove(struct i2c_client *client)
> > +{
> > +       struct fusb302_data *data = i2c_get_clientdata(client);
> > +
> > +       devm_free_irq(data->dev, client->irq, data);
> > +       cancel_delayed_work_sync(&data->timeout);
> > +       cancel_delayed_work_sync(&data->fallback_timeout);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct i2c_device_id fusb302_i2c_ids[] = {
> > +       { "fusb302" },
> > +       { },
> > +};
> > +MODULE_DEVICE_TABLE(i2c, fusb302_i2c_ids);
> > +
> > +static struct i2c_driver fusb302_driver = {
> > +       .probe_new = fusb302_probe,
> > +       .remove = fusb302_remove,
> > +       .id_table = fusb302_i2c_ids,
> > +       .driver = {
> > +               .name = "fusb302",
> > +       },
> > +};
> > +module_i2c_driver(fusb302_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
> > +MODULE_DESCRIPTION("FUSB302 USB TYPE-C controller Driver");
> > --
> > 2.9.3
> >

Br,

-- 
heikki

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Guenter Roeck]

On 04/24/2017 04:02 AM, Heikki Krogerus wrote:
> +Guenter
>
> On Fri, Apr 21, 2017 at 09:51:50PM +0300, Andy Shevchenko wrote:
>> +Cc: Heikki.
>>
>> He might comment on this.
>
> Thanks Andy.
>
>> On Fri, Apr 21, 2017 at 4:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Add support for USB TYPE-C cable detection on systems using a
>>> FUSB302 USB TYPE-C controller.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>  drivers/extcon/Kconfig          |   8 +
>>>  drivers/extcon/Makefile         |   1 +
>>>  drivers/extcon/extcon-fusb302.c | 782 ++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 791 insertions(+)
>>>  create mode 100644 drivers/extcon/extcon-fusb302.c
>
> There is now the typec class in linux-next that really needs to be
> used with all USB Type-C PHYs like fusb302.
>
> Unless I'm mistaken, Guenter has also written a driver for fusb302. I

Not me; it was Nathan.

> have not seen it, but if I understood correctly, that driver will
> register itself with the upcoming tcpm (USB Type-C Port Manager) [1],
> which in practice would mean we can take properly advantage of the USB
> PD transceiver on fusb302.
>
> Guenter! Can you publish the fusb302 driver?
>

Working on it.

Guenter

>
> [1] https://chromium-review.googlesource.com/c/389916/
>
>
>>> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
>>> index 32f2dc8..562db5b 100644
>>> --- a/drivers/extcon/Kconfig
>>> +++ b/drivers/extcon/Kconfig
>>> @@ -35,6 +35,14 @@ config EXTCON_AXP288
>>>           Say Y here to enable support for USB peripheral detection
>>>           and USB MUX switching by X-Power AXP288 PMIC.
>>>
>>> +config EXTCON_FUSB302
>>> +       tristate "FUSB302 USB TYPE-C controller support"
>>> +       depends on I2C
>>> +       select REGMAP_I2C
>>> +       help
>>> +         Say Y here to enable support for USB TYPE-C cable detection using
>>> +         a FUSB302 USB TYPE-C controller.
>>> +
>>>  config EXTCON_GPIO
>>>         tristate "GPIO extcon support"
>>>         depends on GPIOLIB || COMPILE_TEST
>>> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
>>> index ecfa958..e5eb493 100644
>>> --- a/drivers/extcon/Makefile
>>> +++ b/drivers/extcon/Makefile
>>> @@ -7,6 +7,7 @@ extcon-core-objs                += extcon.o devres.o
>>>  obj-$(CONFIG_EXTCON_ADC_JACK)  += extcon-adc-jack.o
>>>  obj-$(CONFIG_EXTCON_ARIZONA)   += extcon-arizona.o
>>>  obj-$(CONFIG_EXTCON_AXP288)    += extcon-axp288.o
>>> +obj-$(CONFIG_EXTCON_FUSB302)   += extcon-fusb302.o
>>>  obj-$(CONFIG_EXTCON_GPIO)      += extcon-gpio.o
>>>  obj-$(CONFIG_EXTCON_INTEL_INT3496) += extcon-intel-int3496.o
>>>  obj-$(CONFIG_EXTCON_INTEL_CHT_WC) += extcon-intel-cht-wc.o
>>> diff --git a/drivers/extcon/extcon-fusb302.c b/drivers/extcon/extcon-fusb302.c
>>> new file mode 100644
>>> index 0000000..577adb9
>>> --- /dev/null
>>> +++ b/drivers/extcon/extcon-fusb302.c
>>> @@ -0,0 +1,782 @@
>>> +/*
>>> + * Extcon USB-C cable detection driver for FUSB302 USB TYPE-C controller
>>> + *
>>> + * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/extcon.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/workqueue.h>
>>> +#include "extcon.h"
>>> +
>>> +#define REG_DEVICE_ID                          0x01
>>> +#define DEVICE_ID_VER_MASK                     GENMASK(7, 4)
>>> +#define DEVICE_ID_FUSB302_VER                  0x80
>>> +
>>> +#define REG_SWITCHES0                          0x02
>>> +#define SWITCHES0_PDWN1                                BIT(0)
>>> +#define SWITCHES0_PDWN2                                BIT(1)
>>> +#define SWITCHES0_PDWN                         GENMASK(1, 0)
>>> +#define SWITCHES0_MEAS_CC1                     BIT(2)
>>> +#define SWITCHES0_MEAS_CC2                     BIT(3)
>>> +#define SWITCHES0_VCONN_CC1                    BIT(4)
>>> +#define SWITCHES0_VCONN_CC2                    BIT(5)
>>> +#define SWITCHES0_PU_EN1                       BIT(6)
>>> +#define SWITCHES0_PU_EN2                       BIT(7)
>>> +
>>> +#define REG_MEASURE                            0x04
>>> +#define MEASURE_MDAC_MASK                      GENMASK(5, 0)
>>> +/* Datasheet says MDAC must be set to 0x34 / 2226mV for vRd-3.0 detection */
>>> +#define MEASURE_MDAC_SNK_VRD30                 0x34
>>> +/* MDAC must be set to 0x25 / 1600mV for disconnect det. with 80uA host-cur */
>>> +#define MEASURE_MDAC_SRC_80UA_HOST_CUR         0x25
>>> +
>>> +#define REG_CONTROL0                           0x06
>>> +#define CONTROL0_HOST_CUR_MASK                 GENMASK(3, 2)
>>> +#define CONTROL0_HOST_CUR_DISABLED             (0 << 2)
>>> +#define CONTROL0_HOST_CUR_80UA                 (1 << 2)
>>> +#define CONTROL0_HOST_CUR_180UA                        (2 << 2)
>>> +#define CONTROL0_HOST_CUR_330UA                        (3 << 2)
>>> +#define CONTROL0_INT_MASK                      BIT(5)
>>> +
>>> +#define REG_CONTROL2                           0x08
>>> +#define CONTROL2_TOGGLE                                BIT(0)
>>> +#define CONTROL2_MODE_MASK                     GENMASK(2, 1)
>>> +#define CONTROL2_MODE_DRP                      (1 << 1)
>>> +#define CONTROL2_MODE_SNK                      (2 << 1)
>>> +#define CONTROL2_MODE_SRC                      (3 << 1)
>>> +#define CONTROL2_SAVE_PWR_MASK                 GENMASK(7, 6)
>>> +#define CONTROL2_SAVE_PWR_DISABLED             (0 << 6)
>>> +#define CONTROL2_SAVE_PWR_40MS                 (1 << 6)
>>> +#define CONTROL2_SAVE_PWR_80MS                 (2 << 6)
>>> +#define CONTROL2_SAVE_PWR_160MS                        (3 << 6)
>>> +
>>> +#define REG_MASK1                              0x0a
>>> +/* REG_MASK1 value for low-power / disabled state from datasheet */
>>> +#define MASK1_DISABLED                         0xfe
>>> +#define MASK1_COMP_CHNG                                BIT(5)
>>> +#define MASK1_VBUSOK                           BIT(7)
>>> +
>>> +#define REG_POWER                              0x0b
>>> +/* REG_POWER values for disabled and normal state from datasheet */
>>> +#define POWER_DISABLED                         BIT(0)
>>> +#define POWER_NORMAL                           GENMASK(2, 0)
>>> +
>>> +#define REG_RESET                              0x0c
>>> +#define RESET_SW_RESET                         BIT(0)
>>> +
>>> +#define REG_OCP                                        0x0d
>>> +
>>> +#define REG_MASKA                              0x0e
>>> +/* REG_MASKA value for low-power / disabled state from datasheet */
>>> +#define MASKA_DISABLED                         0xbf
>>> +
>>> +#define REG_MASKB                              0x0f
>>> +/* REG_MASKB value for low-power / disabled state from datasheet */
>>> +#define MASKB_DISABLED                         0x01
>>> +
>>> +#define REG_STATUS1A                           0x3d
>>> +#define STATUS1A_TOGSS_MASK                    GENMASK(5, 3)
>>> +#define STATUS1A_TOGSS_SRC_CC1                 (1 << 3)
>>> +#define STATUS1A_TOGSS_SRC_CC2                 (2 << 3)
>>> +#define STATUS1A_TOGSS_SNK_CC1                 (5 << 3)
>>> +#define STATUS1A_TOGSS_SNK_CC2                 (6 << 3)
>>> +
>>> +#define REG_INTERRUPTA                         0x3e
>>> +#define INTERRUPTA_TOGDONE                     BIT(6)
>>> +
>>> +#define REG_STATUS0                            0x40
>>> +#define STATUS0_BC_LEVEL_MASK                  GENMASK(1, 0)
>>> +#define STATUS0_BC_LEVEL_VRA                   0
>>> +#define STATUS0_BC_LEVEL_VRDUSB                        1
>>> +#define STATUS0_BC_LEVEL_VRD15                 2
>>> +#define STATUS0_BC_LEVEL_VRD30                 3
>>> +#define STATUS0_COMP                           BIT(5)
>>> +#define STATUS0_VBUSOK                         BIT(7)
>>> +
>>> +#define REG_INTERRUPT                          0x42
>>> +#define INTERRUPT_BC_LVL                       BIT(0)
>>> +#define INTERRUPT_COMP_CHNG                    BIT(5)
>>> +#define INTERRUPT_VBUSOK                       BIT(7)
>>> +
>>> +/* Timeouts from the FUSB302 datasheet */
>>> +#define TTOGCYCLE                              msecs_to_jiffies(40 + 60 + 40)
>>> +
>>> +/* Timeouts from the USB-C specification */
>>> +#define TPDDEBOUNCE                            msecs_to_jiffies(20)
>>> +#define TDRPTRY_MS                             75
>>> +#define TDRPTRY                                        msecs_to_jiffies(TDRPTRY_MS)
>>> +#define TDRPTRYWAIT                            msecs_to_jiffies(400)
>>> +#define TVBUSON                                        msecs_to_jiffies(275)
>>> +
>>> +enum typec_port_type {
>>> +       TYPEC_PORT_DFP,
>>> +       TYPEC_PORT_UFP,
>>> +       TYPEC_PORT_DRP,
>>> +};
>>> +
>>> +enum typec_role {
>>> +       TYPEC_SINK,
>>> +       TYPEC_SOURCE,
>>> +};
>>> +
>>> +enum fusb302_state {
>>> +       DISABLED_SNK, /* UFP */
>>> +       DISABLED_SRC, /* DFP */
>>> +       DISABLED_DRP,
>>> +       UNATTACHED_SNK,
>>> +       ATTACHED_SNK,
>>> +       UNATTACHED_SRC,
>>> +       ATTACHED_SRC,
>>> +};
>>> +/* For debugging */
>>> +static __maybe_unused const char *fusb302_state_str[] = {
>>> +       "DISABLED_SNK",
>>> +       "DISABLED_SRC",
>>> +       "DISABLED_DRP",
>>> +       "UNATTACHED_SNK",
>>> +       "ATTACHED_SNK",
>>> +       "UNATTACHED_SRC",
>>> +       "ATTACHED_SRC",
>>> +};
>>> +
>>> +enum fusb302_event {
>>> +       TOGGLE_DONE,
>>> +       BC_LVL_CHANGE,
>>> +       VBUS_VALID,
>>> +       VBUS_INVALID,
>>> +       COMP_LOW,
>>> +       COMP_HIGH,
>>> +       TIMEOUT,
>>> +       FALLBACK_TIMEOUT,
>>> +};
>>> +/* For debugging */
>>> +static __maybe_unused const char *fusb302_event_str[] = {
>>> +       "TOGGLE_DONE",
>>> +       "BC_LVL_CHANGE",
>>> +       "VBUS_VALID",
>>> +       "VBUS_INVALID",
>>> +       "COMP_LOW",
>>> +       "COMP_HIGH",
>>> +       "TIMEOUT",
>>> +       "FALLBACK_TIMEOUT",
>>> +};
>>> +
>>> +static const unsigned int fusb302_extcon_cables[] = {
>>> +       EXTCON_USB,
>>> +       EXTCON_USB_HOST,
>>> +       EXTCON_CHG_USB_SDP,
>>> +       EXTCON_CHG_USB_CDP,
>>> +       EXTCON_CHG_USB_FAST,
>>> +       EXTCON_NONE,
>>> +};
>>> +
>>> +struct fusb302_data {
>>> +       struct device *dev;
>>> +       struct regmap *regmap;
>>> +       struct extcon_dev *edev;
>>> +       struct mutex lock;
>>> +       struct delayed_work timeout;
>>> +       struct delayed_work fallback_timeout;
>>> +       struct delayed_work bc_work;
>>> +       enum fusb302_state state;
>>> +       enum typec_port_type port_type;
>>> +       enum typec_role preferred_role;
>>> +       int status0;
>>> +       int status1a;
>>> +       unsigned int snk_cable_id;
>>> +};
>>> +
>>> +static void fusb302_write_reg(struct fusb302_data *data, int reg, int val)
>>> +{
>>> +       int ret;
>>> +
>>> +       ret = regmap_write(data->regmap, reg, val);
>>> +       if (ret)
>>> +               dev_err(data->dev, "Error writing reg %02x: %d\n", reg, ret);
>>> +}
>>> +
>>> +static int fusb302_read_reg(struct fusb302_data *data, int reg)
>>> +{
>>> +       int ret, val;
>>> +
>>> +       ret = regmap_read(data->regmap, reg, &val);
>>> +       if (ret) {
>>> +               dev_err(data->dev, "Error reading reg %02x: %d\n", reg, ret);
>>> +               return 0;
>>> +       }
>>> +
>>> +       return val;
>>> +}
>>> +
>>> +static void fusb302_update_bits(struct fusb302_data *data, int reg,
>>> +                               int mask, int val)
>>> +{
>>> +       int ret;
>>> +
>>> +       ret = regmap_update_bits(data->regmap, reg, mask, val);
>>> +       if (ret)
>>> +               dev_err(data->dev, "Error modifying reg %02x: %d\n", reg, ret);
>>> +}
>>> +
>>> +/* Small helper to sync EXTCON_CHG_USB_SDP and EXTCON_USB state */
>>> +static void fusb302_set_extcon_state(struct fusb302_data *data,
>>> +                                    unsigned int cable, bool state)
>>> +{
>>> +       extcon_set_state_sync(data->edev, cable, state);
>>> +       if (cable == EXTCON_CHG_USB_SDP)
>>> +               extcon_set_state_sync(data->edev, EXTCON_USB, state);
>>> +}
>>> +
>>> +/* Helper for the disabled states */
>>> +static void fusb302_disable(struct fusb302_data *data)
>>> +{
>>> +       fusb302_write_reg(data, REG_POWER, POWER_DISABLED);
>>> +       fusb302_write_reg(data, REG_MASK1, MASK1_DISABLED);
>>> +       fusb302_write_reg(data, REG_MASKA, MASKA_DISABLED);
>>> +       fusb302_write_reg(data, REG_MASKB, MASKB_DISABLED);
>>> +       fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
>>> +}
>>> +
>>> +/*
>>> + * fusb302_set_state() and fusb302_handle_event() implement the 3 state
>>> + * machines from the datasheet folded into 1 state-machine for code re-use.
>>> + */
>>> +static void fusb302_set_state(struct fusb302_data *data,
>>> +                             enum fusb302_state state)
>>> +{
>>> +       int status, switches0 = 0;
>>> +       enum fusb302_state old_state = data->state;
>>> +       union extcon_property_value prop;
>>> +
>>> +       /* Kill pending timeouts from previous state */
>>> +       cancel_delayed_work(&data->timeout);
>>> +       cancel_delayed_work(&data->fallback_timeout);
>>> +       cancel_delayed_work(&data->bc_work);
>>> +
>>> +       dev_dbg(data->dev, "New state %s\n", fusb302_state_str[state]);
>>> +
>>> +       switch (old_state) {
>>> +       case ATTACHED_SNK:
>>> +               fusb302_set_extcon_state(data, data->snk_cable_id, false);
>>> +               break;
>>> +       case ATTACHED_SRC:
>>> +               fusb302_set_extcon_state(data, EXTCON_USB_HOST, false);
>>> +               break;
>>> +       default:
>>> +               break; /* Do nothing */
>>> +       }
>>> +
>>> +       switch (state) {
>>> +       case DISABLED_SNK:
>>> +               fusb302_disable(data);
>>> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
>>> +                                   CONTROL2_MODE_SNK);
>>> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
>>> +                                   CONTROL2_TOGGLE);
>>> +               break;
>>> +
>>> +       case DISABLED_SRC:
>>> +               fusb302_disable(data);
>>> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
>>> +                                   CONTROL2_MODE_SRC);
>>> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
>>> +                                   CONTROL2_TOGGLE);
>>> +               break;
>>> +
>>> +       case DISABLED_DRP:
>>> +               fusb302_disable(data);
>>> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
>>> +                                   CONTROL2_MODE_DRP);
>>> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
>>> +                                   CONTROL2_TOGGLE);
>>> +               break;
>>> +
>>> +       case UNATTACHED_SNK:
>>> +               fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
>>> +
>>> +               /* Enable pull-down on CC1 / CC2 based on orientation */
>>> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
>>> +               case STATUS1A_TOGSS_SNK_CC1:
>>> +                       switches0 = SWITCHES0_PDWN1 | SWITCHES0_MEAS_CC1;
>>> +                       prop.intval = USB_TYPEC_POLARITY_NORMAL;
>>> +                       break;
>>> +               case STATUS1A_TOGSS_SNK_CC2:
>>> +                       switches0 = SWITCHES0_PDWN2 | SWITCHES0_MEAS_CC2;
>>> +                       prop.intval = USB_TYPEC_POLARITY_FLIP;
>>> +                       break;
>>> +               }
>>> +               fusb302_write_reg(data, REG_SWITCHES0, switches0);
>>> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
>>> +               extcon_set_property(data->edev, EXTCON_USB,
>>> +                                   EXTCON_PROP_USB_TYPEC_POLARITY, prop);
>>> +
>>> +               /* Enable VBUSOK and COMP irq at 2.24V for BC detection */
>>> +               fusb302_update_bits(data, REG_MASK1,
>>> +                                   MASK1_VBUSOK | MASK1_COMP_CHNG, 0);
>>> +               fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
>>> +                                   MEASURE_MDAC_SNK_VRD30);
>>> +
>>> +               status = fusb302_read_reg(data, REG_STATUS0);
>>> +               if (status & STATUS0_VBUSOK) {
>>> +                       /* Go straight to ATTACHED_SNK */
>>> +                       fusb302_set_state(data, ATTACHED_SNK);
>>> +                       return;
>>> +               }
>>> +
>>> +               mod_delayed_work(system_wq, &data->timeout, TVBUSON);
>>> +               break;
>>> +
>>> +       case ATTACHED_SNK:
>>> +               mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
>>> +               break;
>>> +
>>> +       case UNATTACHED_SRC:
>>> +               fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
>>> +
>>> +               /* Enable pull-up / Vconn on CC1 / CC2 based on orientation */
>>> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
>>> +               case STATUS1A_TOGSS_SRC_CC1:
>>> +                       switches0 = SWITCHES0_PU_EN1 | SWITCHES0_VCONN_CC2 |
>>> +                                   SWITCHES0_MEAS_CC1;
>>> +                       prop.intval = USB_TYPEC_POLARITY_NORMAL;
>>> +                       break;
>>> +               case STATUS1A_TOGSS_SRC_CC2:
>>> +                       switches0 = SWITCHES0_PU_EN2 | SWITCHES0_VCONN_CC1 |
>>> +                                   SWITCHES0_MEAS_CC2;
>>> +                       prop.intval = USB_TYPEC_POLARITY_FLIP;
>>> +                       break;
>>> +               }
>>> +               fusb302_write_reg(data, REG_SWITCHES0, switches0);
>>> +               fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
>>> +               extcon_set_property(data->edev, EXTCON_USB,
>>> +                                   EXTCON_PROP_USB_TYPEC_POLARITY, prop);
>>> +
>>> +               /* Enable COMP irq at 1.6V for detach detection */
>>> +               fusb302_update_bits(data, REG_MASK1, MASK1_COMP_CHNG, 0);
>>> +               fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
>>> +                                   MEASURE_MDAC_SRC_80UA_HOST_CUR);
>>> +
>>> +               status = fusb302_read_reg(data, REG_STATUS0);
>>> +               if (!(status & STATUS0_COMP)) {
>>> +                       /* Go straight to ATTACHED_SRC */
>>> +                       fusb302_set_state(data, ATTACHED_SRC);
>>> +                       return;
>>> +               }
>>> +
>>> +               mod_delayed_work(system_wq, &data->timeout, TPDDEBOUNCE);
>>> +               break;
>>> +
>>> +       case ATTACHED_SRC:
>>> +               fusb302_set_extcon_state(data, EXTCON_USB_HOST, true);
>>> +               break;
>>> +       }
>>> +
>>> +       data->state = state;
>>> +}
>>> +
>>> +static void fusb302_set_state_disabled(struct fusb302_data *data)
>>> +{
>>> +
>>> +       switch (data->port_type) {
>>> +       case TYPEC_PORT_UFP:
>>> +               fusb302_set_state(data, DISABLED_SNK);
>>> +               break;
>>> +       case TYPEC_PORT_DFP:
>>> +               fusb302_set_state(data, DISABLED_SRC);
>>> +               break;
>>> +       case TYPEC_PORT_DRP:
>>> +               fusb302_set_state(data, DISABLED_DRP);
>>> +               break;
>>> +       }
>>> +}
>>> +
>>> +static void fusb302_handle_disabled_snk_event(struct fusb302_data *data,
>>> +                                             int event)
>>> +{
>>> +       switch (event) {
>>> +       case TOGGLE_DONE:
>>> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
>>> +               case STATUS1A_TOGSS_SNK_CC1:
>>> +               case STATUS1A_TOGSS_SNK_CC2:
>>> +                       fusb302_set_state(data, UNATTACHED_SNK);
>>> +                       break;
>>> +               }
>>> +               break;
>>> +
>>> +       case TIMEOUT:
>>> +               /* Cannot become snk fallback to src */
>>> +               fusb302_set_state(data, DISABLED_SRC);
>>> +               mod_delayed_work(system_wq, &data->fallback_timeout,
>>> +                                TDRPTRYWAIT);
>>> +               break;
>>> +
>>> +       case FALLBACK_TIMEOUT:
>>> +               /* Both states failed return to disabled drp state */
>>> +               fusb302_set_state(data, DISABLED_DRP);
>>> +               break;
>>> +       }
>>> +}
>>> +
>>> +static void fusb302_handle_disabled_src_event(struct fusb302_data *data,
>>> +                                             int event)
>>> +{
>>> +       switch (event) {
>>> +       case TOGGLE_DONE:
>>> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
>>> +               case STATUS1A_TOGSS_SRC_CC1:
>>> +               case STATUS1A_TOGSS_SRC_CC2:
>>> +                       fusb302_set_state(data, UNATTACHED_SRC);
>>> +                       break;
>>> +               }
>>> +               break;
>>> +
>>> +       case TIMEOUT:
>>> +               /* Cannot become src fallback to snk */
>>> +               fusb302_set_state(data, DISABLED_SNK);
>>> +               mod_delayed_work(system_wq, &data->fallback_timeout,
>>> +                                TDRPTRYWAIT);
>>> +               break;
>>> +
>>> +       case FALLBACK_TIMEOUT:
>>> +               /* Both states failed return to disabled drp state */
>>> +               fusb302_set_state(data, DISABLED_DRP);
>>> +               break;
>>> +       }
>>> +}
>>> +
>>> +static void fusb302_handle_disabled_drp_event(struct fusb302_data *data,
>>> +                                             int event)
>>> +{
>>> +       switch (event) {
>>> +       case TOGGLE_DONE:
>>> +               switch (data->status1a & STATUS1A_TOGSS_MASK) {
>>> +               case STATUS1A_TOGSS_SNK_CC1:
>>> +               case STATUS1A_TOGSS_SNK_CC2:
>>> +                       if (data->preferred_role == TYPEC_SINK) {
>>> +                               /* Jump directly to UNATTACHED_SNK */
>>> +                               fusb302_set_state(data, UNATTACHED_SNK);
>>> +                       } else {
>>> +                               /* Try to become src */
>>> +                               fusb302_set_state(data, DISABLED_SNK);
>>> +                               mod_delayed_work(system_wq, &data->timeout,
>>> +                                                TDRPTRY);
>>> +                       }
>>> +                       break;
>>> +               case STATUS1A_TOGSS_SRC_CC1:
>>> +               case STATUS1A_TOGSS_SRC_CC2:
>>> +                       if (data->preferred_role == TYPEC_SOURCE) {
>>> +                               /* Jump directly to UNATTACHED_SRC */
>>> +                               fusb302_set_state(data, UNATTACHED_SRC);
>>> +                       } else {
>>> +                               /*
>>> +                                * The USB-C spec says we must enable pull-downs
>>> +                                * and then wait tDRPTry before checking to
>>> +                                * avoid endless role-bouncing if both ends
>>> +                                * prefer the snk role.
>>> +                                */
>>> +                               fusb302_write_reg(data, REG_SWITCHES0,
>>> +                                                 SWITCHES0_PDWN);
>>> +                               fusb302_update_bits(data, REG_CONTROL2,
>>> +                                                   CONTROL2_TOGGLE, 0);
>>> +                               msleep(TDRPTRY_MS);
>>> +                               /*
>>> +                                * Use the toggle engine to do the src
>>> +                                * detection to keep things the same as for
>>> +                                * directly entering the src role.
>>> +                                */
>>> +                               fusb302_set_state(data, DISABLED_SNK);
>>> +                               mod_delayed_work(system_wq, &data->timeout,
>>> +                                                TTOGCYCLE);
>>> +                       }
>>> +                       break;
>>> +               }
>>> +               break;
>>> +       }
>>> +}
>>> +
>>> +static void fusb302_handle_unattached_snk_event(struct fusb302_data *data,
>>> +                                             int event)
>>> +{
>>> +       switch (event) {
>>> +       case VBUS_VALID: /* Cable attached */
>>> +               fusb302_set_state(data, ATTACHED_SNK);
>>> +               break;
>>> +       case TIMEOUT:
>>> +               fusb302_set_state_disabled(data);
>>> +               break;
>>> +       }
>>> +}
>>> +
>>> +static void fusb302_handle_attached_snk_event(struct fusb302_data *data,
>>> +                                             int event)
>>> +{
>>> +       switch (event) {
>>> +       case BC_LVL_CHANGE:
>>> +       case COMP_LOW:
>>> +       case COMP_HIGH:
>>> +               mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
>>> +               break;
>>> +       case VBUS_INVALID: /* Cable detached */
>>> +               fusb302_set_state_disabled(data);
>>> +               break;
>>> +       }
>>> +}
>>> +
>>> +static void fusb302_handle_unattached_src_event(struct fusb302_data *data,
>>> +                                             int event)
>>> +{
>>> +       switch (event) {
>>> +       case COMP_LOW: /* Cable attached */
>>> +               fusb302_set_state(data, ATTACHED_SRC);
>>> +               break;
>>> +       case TIMEOUT:
>>> +               fusb302_set_state_disabled(data);
>>> +               break;
>>> +       }
>>> +}
>>> +
>>> +static void fusb302_handle_attached_src_event(struct fusb302_data *data,
>>> +                                             int event)
>>> +{
>>> +       switch (event) {
>>> +       case COMP_HIGH: /* Cable detached */
>>> +               fusb302_set_state_disabled(data);
>>> +               break;
>>> +       }
>>> +}
>>> +
>>> +static void fusb302_handle_event(struct fusb302_data *data, int event)
>>> +{
>>> +
>>> +       mutex_lock(&data->lock);
>>> +
>>> +       dev_dbg(data->dev, "Handling state %s event %s status %02x %02x\n",
>>> +               fusb302_state_str[data->state], fusb302_event_str[event],
>>> +               fusb302_read_reg(data, REG_STATUS0),
>>> +               fusb302_read_reg(data, REG_STATUS1A));
>>> +
>>> +       switch (data->state) {
>>> +       case DISABLED_SNK:
>>> +               fusb302_handle_disabled_snk_event(data, event);
>>> +               break;
>>> +       case DISABLED_SRC:
>>> +               fusb302_handle_disabled_src_event(data, event);
>>> +               break;
>>> +       case DISABLED_DRP:
>>> +               fusb302_handle_disabled_drp_event(data, event);
>>> +               break;
>>> +       case UNATTACHED_SNK:
>>> +               fusb302_handle_unattached_snk_event(data, event);
>>> +               break;
>>> +       case ATTACHED_SNK:
>>> +               fusb302_handle_attached_snk_event(data, event);
>>> +               break;
>>> +       case UNATTACHED_SRC:
>>> +               fusb302_handle_unattached_src_event(data, event);
>>> +               break;
>>> +       case ATTACHED_SRC:
>>> +               fusb302_handle_attached_src_event(data, event);
>>> +               break;
>>> +       }
>>> +       mutex_unlock(&data->lock);
>>> +}
>>> +
>>> +static void fusb302_timeout(struct work_struct *work)
>>> +{
>>> +       struct fusb302_data *data =
>>> +               container_of(work, struct fusb302_data, timeout.work);
>>> +
>>> +       fusb302_handle_event(data, TIMEOUT);
>>> +}
>>> +
>>> +static void fusb302_fallback_timeout(struct work_struct *work)
>>> +{
>>> +       struct fusb302_data *data =
>>> +               container_of(work, struct fusb302_data, fallback_timeout.work);
>>> +
>>> +       fusb302_handle_event(data, FALLBACK_TIMEOUT);
>>> +}
>>> +
>>> +static void fusb302_report_bc_level(struct work_struct *work)
>>> +{
>>> +       struct fusb302_data *data =
>>> +               container_of(work, struct fusb302_data, bc_work.work);
>>> +
>>> +       /* Clear old charger cable id */
>>> +       fusb302_set_extcon_state(data, data->snk_cable_id, false);
>>> +
>>> +       if (data->status0 & STATUS0_COMP) {
>>> +               dev_warn(data->dev, "vRd over maximum, assuming 500mA source\n");
>>> +               data->snk_cable_id = EXTCON_CHG_USB_SDP;
>>> +       } else {
>>> +               switch (data->status0 & STATUS0_BC_LEVEL_MASK) {
>>> +               case STATUS0_BC_LEVEL_VRA:
>>> +               case STATUS0_BC_LEVEL_VRDUSB:
>>> +                       data->snk_cable_id = EXTCON_CHG_USB_SDP;
>>> +                       break;
>>> +               case STATUS0_BC_LEVEL_VRD15:
>>> +                       data->snk_cable_id = EXTCON_CHG_USB_CDP;
>>> +                       break;
>>> +               case STATUS0_BC_LEVEL_VRD30:
>>> +                       /* Use CHG_USB_FAST to indicate 3A capability */
>>> +                       data->snk_cable_id = EXTCON_CHG_USB_FAST;
>>> +                       break;
>>> +               }
>>> +       }
>>> +       fusb302_set_extcon_state(data, data->snk_cable_id, true);
>>> +}
>>> +
>>> +static irqreturn_t fusb302_irq_handler_thread(int irq, void *handler_data)
>>> +{
>>> +       struct fusb302_data *data = handler_data;
>>> +       int interrupt, interrupta;
>>> +
>>> +       interrupt = fusb302_read_reg(data, REG_INTERRUPT);
>>> +       interrupta = fusb302_read_reg(data, REG_INTERRUPTA);
>>> +
>>> +       if (interrupt)
>>> +               data->status0 = fusb302_read_reg(data, REG_STATUS0);
>>> +
>>> +       if (interrupta)
>>> +               data->status1a = fusb302_read_reg(data, REG_STATUS1A);
>>> +
>>> +       dev_dbg(data->dev, "Handling interrupt %02x %02x status %02x %02x\n",
>>> +               interrupt, interrupta, data->status0, data->status1a);
>>> +
>>> +       if (interrupt & INTERRUPT_BC_LVL)
>>> +               fusb302_handle_event(data, BC_LVL_CHANGE);
>>> +
>>> +       if (interrupt & INTERRUPT_COMP_CHNG) {
>>> +               if (data->status0 & STATUS0_COMP)
>>> +                       fusb302_handle_event(data, COMP_HIGH);
>>> +               else
>>> +                       fusb302_handle_event(data, COMP_LOW);
>>> +       }
>>> +
>>> +       if (interrupt & INTERRUPT_VBUSOK) {
>>> +               if (data->status0 & STATUS0_VBUSOK)
>>> +                       fusb302_handle_event(data, VBUS_VALID);
>>> +               else
>>> +                       fusb302_handle_event(data, VBUS_INVALID);
>>> +       }
>>> +
>>> +       if (interrupta & INTERRUPTA_TOGDONE)
>>> +               fusb302_handle_event(data, TOGGLE_DONE);
>>> +
>>> +       return IRQ_HANDLED;
>>> +}
>>> +
>>> +static const struct regmap_config fusb302_regmap_config = {
>>> +       .reg_bits = 8,
>>> +       .val_bits = 8,
>>> +       .val_format_endian = REGMAP_ENDIAN_NATIVE,
>>> +};
>>> +
>>> +static int fusb302_probe(struct i2c_client *client)
>>> +{
>>> +       struct device *dev = &client->dev;
>>> +       struct fusb302_data *data;
>>> +       int ret;
>>> +
>>> +       if (!client->irq) {
>>> +               dev_err(dev, "Error irq not specified\n");
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>>> +       if (!data)
>>> +               return -ENOMEM;
>>> +
>>> +       data->dev = dev;
>>> +       /* TODO make these 2 configurable using device-properties */
>>> +       data->port_type = TYPEC_PORT_DRP;
>>> +       data->preferred_role = TYPEC_SINK;
>>> +
>>> +       data->regmap = devm_regmap_init_i2c(client, &fusb302_regmap_config);
>>> +       if (IS_ERR(data->regmap)) {
>>> +               ret = PTR_ERR(data->regmap);
>>> +               dev_err(dev, "Error to initializing regmap: %d\n", ret);
>>> +               return ret;
>>> +       }
>>> +
>>> +       mutex_init(&data->lock);
>>> +       INIT_DELAYED_WORK(&data->timeout, fusb302_timeout);
>>> +       INIT_DELAYED_WORK(&data->fallback_timeout, fusb302_fallback_timeout);
>>> +       INIT_DELAYED_WORK(&data->bc_work, fusb302_report_bc_level);
>>> +
>>> +       data->edev = devm_extcon_dev_allocate(dev, fusb302_extcon_cables);
>>> +       if (IS_ERR(data->edev))
>>> +               return PTR_ERR(data->edev);
>>> +
>>> +       data->edev->name = "fusb302";
>>> +
>>> +       ret = devm_extcon_dev_register(dev, data->edev);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       extcon_set_property_capability(data->edev, EXTCON_USB,
>>> +                                      EXTCON_PROP_USB_TYPEC_POLARITY);
>>> +
>>> +       fusb302_write_reg(data, REG_RESET, RESET_SW_RESET);
>>> +       usleep_range(10000, 20000);
>>> +
>>> +       /* Enable power-saving */
>>> +       fusb302_update_bits(data, REG_CONTROL2, CONTROL2_SAVE_PWR_MASK,
>>> +                           CONTROL2_SAVE_PWR_40MS);
>>> +
>>> +       fusb302_set_state_disabled(data);
>>> +
>>> +       ret = devm_request_threaded_irq(dev, client->irq, NULL,
>>> +                       fusb302_irq_handler_thread,
>>> +                       IRQF_TRIGGER_LOW | IRQF_ONESHOT, "fusb302", data);
>>> +       if (ret) {
>>> +               dev_err(dev, "Error requesting irq: %d\n", ret);
>>> +               return ret;
>>> +       }
>>> +
>>> +       fusb302_update_bits(data, REG_CONTROL0, CONTROL0_INT_MASK, 0);
>>> +
>>> +       i2c_set_clientdata(client, data);
>>> +       return 0;
>>> +}
>>> +
>>> +static int fusb302_remove(struct i2c_client *client)
>>> +{
>>> +       struct fusb302_data *data = i2c_get_clientdata(client);
>>> +
>>> +       devm_free_irq(data->dev, client->irq, data);
>>> +       cancel_delayed_work_sync(&data->timeout);
>>> +       cancel_delayed_work_sync(&data->fallback_timeout);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static const struct i2c_device_id fusb302_i2c_ids[] = {
>>> +       { "fusb302" },
>>> +       { },
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, fusb302_i2c_ids);
>>> +
>>> +static struct i2c_driver fusb302_driver = {
>>> +       .probe_new = fusb302_probe,
>>> +       .remove = fusb302_remove,
>>> +       .id_table = fusb302_i2c_ids,
>>> +       .driver = {
>>> +               .name = "fusb302",
>>> +       },
>>> +};
>>> +module_i2c_driver(fusb302_driver);
>>> +
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
>>> +MODULE_DESCRIPTION("FUSB302 USB TYPE-C controller Driver");
>>> --
>>> 2.9.3
>>>
>
> Br,
>

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Hans de Goede]

Hi,

On 24-04-17 15:12, Guenter Roeck wrote:
> On 04/24/2017 04:02 AM, Heikki Krogerus wrote:
>> +Guenter
>>
>> On Fri, Apr 21, 2017 at 09:51:50PM +0300, Andy Shevchenko wrote:
>>> +Cc: Heikki.
>>>
>>> He might comment on this.
>>
>> Thanks Andy.
>>
>>> On Fri, Apr 21, 2017 at 4:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Add support for USB TYPE-C cable detection on systems using a
>>>> FUSB302 USB TYPE-C controller.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>  drivers/extcon/Kconfig          |   8 +
>>>>  drivers/extcon/Makefile         |   1 +
>>>>  drivers/extcon/extcon-fusb302.c | 782 ++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 791 insertions(+)
>>>>  create mode 100644 drivers/extcon/extcon-fusb302.c
>>
>> There is now the typec class in linux-next that really needs to be
>> used with all USB Type-C PHYs like fusb302.
>>
>> Unless I'm mistaken, Guenter has also written a driver for fusb302. I
> 
> Not me; it was Nathan.
> 
>> have not seen it, but if I understood correctly, that driver will
>> register itself with the upcoming tcpm (USB Type-C Port Manager) [1],
>> which in practice would mean we can take properly advantage of the USB
>> PD transceiver on fusb302.
>>
>> Guenter! Can you publish the fusb302 driver?
>>
> 
> Working on it.

Ok, I see this has landed in staging. So lets go with this driver
then and not mine (I did not get around to the PD stuff yet, so
that is fine).

Question, how is this supposed to interface with the rest of the
kernel ? Specifically the commit message for the tcpm frameworks
says:

"This driver only implements the state machine. Lower level drivers are
responsible for
- Reporting VBUS status and activating VBUS
- Setting CC lines and providing CC line status
- Setting line polarity
- Activating and deactivating VCONN
- Setting the current limit
- Activating and deactivating PD message transfers
- Sending and receiving PD messages"

But the FUSB302 cannot set the current limit...

The device I'm working on:
http://www.gpd.hk/gpdwin.asp

Has the following more or less relevant components:

-Intel Cherry Trail Z8700 SoC
-Intel Cherry Trail Whiskey Cove PMIC (which is different from the Broxton Whiskey Cove PMIC),
  this PMIC uses an external charger and fuel-gauge:
-TI bq24292i charger
-Maxim MAX17047 fuel-gauge
-PI3USB30532 USB TYPE-C mux

Currently I only have the USB-C connector on the
device working in USB-2 mode (I did not realize
the USB-3 bits were wired up at first).

The device ships with a charger with an USB-A
receptacle and an USB-2 only USB-A to USB-C
cable.

The way I've hooked up USB-2 charging is like this:
The Whiskey Cove PMIC has built in USB-2 BC detection, and
I've written an extcon driver for this which reports one
of the following cables depended on the BC detection:

         EXTCON_CHG_USB_SDP,
         EXTCON_CHG_USB_CDP,
         EXTCON_CHG_USB_DCP,

And I've modified the bq24290_charger driver to (optionally)
receive an extcon name through device-properties and if
it does, then set the input current based on the detected
charger type (0.5 A for SDP, 1.5A for CDP, 2A for DCP).

Most of the patches for this are upstream. But once we
hookup the FUSB302 driver we need to propagate the current
limit it has negotiated to the bq24290_charger driver.

Which is part of the reason why I wrote the 5th patch of
my original series, let me reply to Heikki's question about
that one here as it is related:

On 24-04-17 16:25, Heikki Krogerus wrote:

 > Doesn't this mean you have several extcon_dev registered for a
 > single port?

Yes this is unfortunate, but the primary consumer of extcon
info is the kernel itself and we can teach it to look at the
right one.

 > I'm not completely sure how extcon framework is meant to
 > be used, but shouldn't a single extcon_dev represent all the types of
 > connectors a port is meant to support?

Ideally, yes. But here we've 2 chips / drivers connected
to the same connector (more even, but 2 which provide extcon
related info).

As explained above the device ships with an USB-2 charger +
USB-A to USB-C cable, that cable correctly gets seen by the
FUSB302 as a TYPEC_CC_RP_DEF cable, which means limiting input
current to 0.5A, but as the FUSB302 datasheet says:

"The host software is expected to follow the USB Type-C
specification for charging current priority based on
feedback from the FUSB302 detection, external BC1.2 detection
and any USB Power Delivery communication.

The FUSB302 does not integrate BC1.2 charger
detection which is assumed available in the USB
transceiver or USB charger in the system."

So when we see TYPEC_CC_RP_DEF we need to ask the
Cherry Trail PMIC (in my case) to do BC detection
and use the result of that, otherwise we end up
setting input current limit way too low.

 >> Since the Type-C controller info is incomplete and needs to be
 >> supplemented with the BC1.2 detection result in some cases, this
 >> commit makes the intel-cht-wc extcon driver monitor the extcon device
 >> registered by the Type-C controller and report that as our extcon state
 >> except when BC-1.2 detection should be used. This allows extcon
 >> consumers on these boards to keep monitoring only the intel-cht-wc extcon
 >> and then get complete extcon info from that, which combines the Type-C
 >> and BC-1.2 info.
 >
 > I don't really understand why does this need to be done in such a
 > complex manner? Both components should just report the result and be
 > done with it, and there is no need for any coupling between the two.
 > If there is a consumer for the power, the driver for it can decide on
 > its own the limits and maximum power from the two sources.

To me making the combination of the 2 sources the problem
of the consumer seems just wrong, as you mentioned
above there should really be only one extcon for the
connector, likewise their should be only one definitive
source on what the input current limit is.

TL;DR: We need some way to combine the current limit info
from TYPE-C and USB-2 BC detection into a single source and
to propagate that current to drivers which can actually set
the current-limit.

Note I'm happy to use something else then extcon for this,
but we do need some way to combine + propagate the
current-limit.

Likewise we need a way to tell another driver to drive VBus
on the USB-C connector. In my case this is again done by the
bq24290_charger driver, which currently does this based on the
combination of EXTCON_CHG_USB_* not being set on the extcon +
EXTCON_USB_HOST being set.

One possible solution would be for the
drivers/extcon/extcon-intel-cht-wc.c to somehow hook into
the tpmc device and to get notifications of state changes,
then it could reflect the info from the FUSB302 in its
extcon info, not sure if that is the best design though.

What also seems to missing is for a way to hookup a
mux driver to deal with upside-down plug-ins and
alt-mode switching.

Regards,

Hans

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Heikki Krogerus]

Hi Hans,

On Fri, May 12, 2017 at 11:22:46PM +0200, Hans de Goede wrote:
> On 24-04-17 16:25, Heikki Krogerus wrote:
> 
> > Doesn't this mean you have several extcon_dev registered for a
> > single port?
> 
> Yes this is unfortunate, but the primary consumer of extcon
> info is the kernel itself and we can teach it to look at the
> right one.
> 
> > I'm not completely sure how extcon framework is meant to
> > be used, but shouldn't a single extcon_dev represent all the types of
> > connectors a port is meant to support?
> 
> Ideally, yes. But here we've 2 chips / drivers connected
> to the same connector (more even, but 2 which provide extcon
> related info).
> 
> As explained above the device ships with an USB-2 charger +
> USB-A to USB-C cable, that cable correctly gets seen by the
> FUSB302 as a TYPEC_CC_RP_DEF cable, which means limiting input
> current to 0.5A, but as the FUSB302 datasheet says:
> 
> "The host software is expected to follow the USB Type-C
> specification for charging current priority based on
> feedback from the FUSB302 detection, external BC1.2 detection
> and any USB Power Delivery communication.
> 
> The FUSB302 does not integrate BC1.2 charger
> detection which is assumed available in the USB
> transceiver or USB charger in the system."
> 
> So when we see TYPEC_CC_RP_DEF we need to ask the
> Cherry Trail PMIC (in my case) to do BC detection
> and use the result of that, otherwise we end up
> setting input current limit way too low.

I understand that.

> >> Since the Type-C controller info is incomplete and needs to be
> >> supplemented with the BC1.2 detection result in some cases, this
> >> commit makes the intel-cht-wc extcon driver monitor the extcon device
> >> registered by the Type-C controller and report that as our extcon state
> >> except when BC-1.2 detection should be used. This allows extcon
> >> consumers on these boards to keep monitoring only the intel-cht-wc extcon
> >> and then get complete extcon info from that, which combines the Type-C
> >> and BC-1.2 info.
> >
> > I don't really understand why does this need to be done in such a
> > complex manner? Both components should just report the result and be
> > done with it, and there is no need for any coupling between the two.
> > If there is a consumer for the power, the driver for it can decide on
> > its own the limits and maximum power from the two sources.
> 
> To me making the combination of the 2 sources the problem
> of the consumer seems just wrong, as you mentioned
> above there should really be only one extcon for the
> connector, likewise their should be only one definitive
> source on what the input current limit is.

Why? The consumer driver, bq24290 in this case, does not need to
understand it has multiple sources. It will just react to events from
*a* source, and the psy framework takes care of everything else. The
psy framework will need some tuning, but that should not be a problem.
I'm preparing support for _PCL (power consumer list) ACPI method
handling for the psy framework, so I have some patches already. The
idea is that in the future USB ports could have virtual power source
object with _PCL.

This is in any case separate issue compared to the problem of telling
the BC1.2 detection to start in case of TYPEC_CC_RP_DEF.

> TL;DR: We need some way to combine the current limit info
> from TYPE-C and USB-2 BC detection into a single source and
> to propagate that current to drivers which can actually set
> the current-limit.
> 
> Note I'm happy to use something else then extcon for this,
> but we do need some way to combine + propagate the
> current-limit.

That must be handled using psy framework. Otherwise we'll just be
trying to re-invent the wheel. There is no problem for a consumer to
have multiple sources.

There are several issues here..

> Likewise we need a way to tell another driver to drive VBus
> on the USB-C connector. In my case this is again done by the
> bq24290_charger driver, which currently does this based on the
> combination of EXTCON_CHG_USB_* not being set on the extcon +
> EXTCON_USB_HOST being set.

This is an other..

> One possible solution would be for the
> drivers/extcon/extcon-intel-cht-wc.c to somehow hook into
> the tpmc device and to get notifications of state changes,
> then it could reflect the info from the FUSB302 in its
> extcon info, not sure if that is the best design though.
> 
> What also seems to missing is for a way to hookup a
> mux driver to deal with upside-down plug-ins and
> alt-mode switching.

This is fourth issue, but let's not go into that now. Just in case you
guys are interested, right now I'm looking if we can use device graph
to link all the components related to an alt-mode:
Documentation/devicetree/bindings/graph.txt

The reason for that is because we should be able to use it also with
ACPI: Documentation/acpi/dsd/graph.txt


Back to this topic. I can see at least the following problems:

1. Tell the BC1.2 detection to start from fusb302 driver
2. Deliver the power limits to the discrete charger ic or battery driver
3. Tell what ever regulates VBUS to start driving VBUS.

You are trying to solve everything using extcon, and that is wrong.
The second problem definitely needs to be handled using psy framework.
The psy framework provides already everything needed for that.

I don't have suggestions for the other issues at this point. If extcon
can be used with those without any problems, OK. But let's try to
solve one problem at a time.


Thanks,

-- 
heikki

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Hans de Goede]

Hi,

On 05/16/2017 02:07 PM, Heikki Krogerus wrote:
> Hi Hans,
>
> On Fri, May 12, 2017 at 11:22:46PM +0200, Hans de Goede wrote:

<snip>

>> To me making the combination of the 2 sources the problem
>> of the consumer seems just wrong, as you mentioned
>> above there should really be only one extcon for the
>> connector, likewise their should be only one definitive
>> source on what the input current limit is.
>
> Why? The consumer driver, bq24290 in this case, does not need to
> understand it has multiple sources. It will just react to events from
> *a* source, and the psy framework takes care of everything else. The
> psy framework will need some tuning, but that should not be a problem.
> I'm preparing support for _PCL (power consumer list) ACPI method
> handling for the psy framework, so I have some patches already. The
> idea is that in the future USB ports could have virtual power source
> object with _PCL.
>
> This is in any case separate issue compared to the problem of telling
> the BC1.2 detection to start in case of TYPEC_CC_RP_DEF.
>
>> TL;DR: We need some way to combine the current limit info
>> from TYPE-C and USB-2 BC detection into a single source and
>> to propagate that current to drivers which can actually set
>> the current-limit.
>>
>> Note I'm happy to use something else then extcon for this,
>> but we do need some way to combine + propagate the
>> current-limit.
>
> That must be handled using psy framework. Otherwise we'll just be
> trying to re-invent the wheel. There is no problem for a consumer to
> have multiple sources.

But we don't really have multiple sources here, we have only
one source, the USB-C cable hooked to an external power-supply.
Just because 2 different pieces of hardware may be involved in
determining the current limit does not mean that we should
model this as 2 different sources. Just as you rightfully
pointed out that me using 2 extcon devices for the single
Type-C connector is wrong, modelling it as 2 sources is
wrong too.

<snip>

> Back to this topic. I can see at least the following problems:
>
> 1. Tell the BC1.2 detection to start from fusb302 driver
> 2. Deliver the power limits to the discrete charger ic or battery driver
> 3. Tell what ever regulates VBUS to start driving VBUS.
>
> You are trying to solve everything using extcon, and that is wrong.

As indicated I'm not stuck on using extcon, I started using it
in my paches because it is used to set the current limit in some
other places already, but I'm fine with using something else.

> The second problem definitely needs to be handled using psy framework.
> The psy framework provides already everything needed for that.

So above you're talking about sources to the bq24190, which if
I understand you correctly suggest that you want the tcpm code to
register a power-supply device per Type-C port ?  I'm not sure that
is a good idea, any registered power-supply devices will show up
under /sys/class/power_supply, currently on a typical system
there will be 2 entries under /sys/class/power_supply one for
the "Mains" or USB power input and one for the battery, adding
more entries there is likely to confuse userspace.

More in general having 2 power-supply devices for what is
in essence one power-input feels wrong.

We can still use the power-supply framework, but I would
envision this working like this:

The platform code which enumerates the type-c controller
sets a "input-current-power-supply-name" string device-property
on the tcpm's (parent)device. When this is set the tpcm code
will do a power_supply_get_by_name and set the input
current on the returned power_supply by setting the
POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT property.

For 3. (vbus) we could do something similar using a
"vbus-regulator-id" device-property and the regulator
framework, making the driver which controls Vbus register
itself as a regulator.

I can take a shot at implementing both suggestions, but
first lets try to get some general idea of how we want
to solve this.

Regards,

Hans

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Heikki Krogerus]

Hi,

On Wed, May 17, 2017 at 12:24:52AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 05/16/2017 02:07 PM, Heikki Krogerus wrote:
> > Hi Hans,
> > 
> > On Fri, May 12, 2017 at 11:22:46PM +0200, Hans de Goede wrote:
> 
> <snip>
> 
> > > To me making the combination of the 2 sources the problem
> > > of the consumer seems just wrong, as you mentioned
> > > above there should really be only one extcon for the
> > > connector, likewise their should be only one definitive
> > > source on what the input current limit is.
> > 
> > Why? The consumer driver, bq24290 in this case, does not need to
> > understand it has multiple sources. It will just react to events from
> > *a* source, and the psy framework takes care of everything else. The
> > psy framework will need some tuning, but that should not be a problem.
> > I'm preparing support for _PCL (power consumer list) ACPI method
> > handling for the psy framework, so I have some patches already. The
> > idea is that in the future USB ports could have virtual power source
> > object with _PCL.
> > 
> > This is in any case separate issue compared to the problem of telling
> > the BC1.2 detection to start in case of TYPEC_CC_RP_DEF.
> > 
> > > TL;DR: We need some way to combine the current limit info
> > > from TYPE-C and USB-2 BC detection into a single source and
> > > to propagate that current to drivers which can actually set
> > > the current-limit.
> > > 
> > > Note I'm happy to use something else then extcon for this,
> > > but we do need some way to combine + propagate the
> > > current-limit.
> > 
> > That must be handled using psy framework. Otherwise we'll just be
> > trying to re-invent the wheel. There is no problem for a consumer to
> > have multiple sources.
> 
> But we don't really have multiple sources here, we have only
> one source, the USB-C cable hooked to an external power-supply.
> Just because 2 different pieces of hardware may be involved in
> determining the current limit does not mean that we should
> model this as 2 different sources. Just as you rightfully
> pointed out that me using 2 extcon devices for the single
> Type-C connector is wrong, modelling it as 2 sources is
> wrong too.

The power supply devices don't represent the port like the extcon
device would. The power supply devices represent the two types
of external chargers we support. So BC1.2 and Type-C/PD source.

> > Back to this topic. I can see at least the following problems:
> > 
> > 1. Tell the BC1.2 detection to start from fusb302 driver
> > 2. Deliver the power limits to the discrete charger ic or battery driver
> > 3. Tell what ever regulates VBUS to start driving VBUS.
> > 
> > You are trying to solve everything using extcon, and that is wrong.
> 
> As indicated I'm not stuck on using extcon, I started using it
> in my paches because it is used to set the current limit in some
> other places already, but I'm fine with using something else.
> 
> > The second problem definitely needs to be handled using psy framework.
> > The psy framework provides already everything needed for that.
> 
> So above you're talking about sources to the bq24190, which if
> I understand you correctly suggest that you want the tcpm code to
> register a power-supply device per Type-C port ?

No, the underlying device driver (so fusb302) needs to register the
power supply at this point. We just notify the psy framework if the
limits we get from tcpm to the set_current_limit hook change.

> I'm not sure that
> is a good idea, any registered power-supply devices will show up
> under /sys/class/power_supply, currently on a typical system
> there will be 2 entries under /sys/class/power_supply one for
> the "Mains" or USB power input and one for the battery, adding
> more entries there is likely to confuse userspace.

The userspace uses the type attribute to differentiate the supplies.
Otherwise it would not be able to tell even which is the Battery and
which is Mains. Having more power supplies is not a problem.

> More in general having 2 power-supply devices for what is
> in essence one power-input feels wrong.
> 
> We can still use the power-supply framework, but I would
> envision this working like this:
> 
> The platform code which enumerates the type-c controller
> sets a "input-current-power-supply-name" string device-property
> on the tcpm's (parent)device. When this is set the tpcm code
> will do a power_supply_get_by_name and set the input
> current on the returned power_supply by setting the
> POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT property.

The psy framework is probable a bit messy at them moment. It
definitely does not do much protecting and in many cases one driver
registers a power supply and an other just takes it over, but that
should be avoided as it makes things difficult (painful) to maintain.
It also poses risks IMO. There certainly almost never seems to be a
real need for that.

In this case the driver for fusb302 registers a power supply that
provides properties like POWER_SUPPLY_PROP_VOLTAGE_MAX, etc. and
simply calls power_supply_changed() when ever needed (when we know the
limits and when a source is attached/de-attached -> changes PRESENT
& ONLINE properties). The fusb302 driver does not need to know if
there are any consumers for it or not. The platform driver that
registers the device for it will simply assign the consumer for it in
this case, but in the future we want to get that kind of detail from
the platform of course, so ACPI or DT.

The PMIC charger driver will similarly register a power supply device
and function pretty much exactly the same way.

The consumer, bq24190, will receive notification from psy framework to
its external_power_changed hook when ever either of the supplies
calls power_supply_changed(). It then needs to check the properties of
the supply (the external power) that are interesting to it in order to
set the limits accordingly (this btw. is where the psy API and the
class driver can be improved without much effort to make things easier
for the consumers). The consumer driver in any case does not need to
know what the supplies actually are, how many of them it has, or are
there any at all, just like the drivers for the supplies don't need to
know the consumers.

But in any case, we should never just pick a power supply and set its
values from outside its driver, even if the current API allows it.
Instead maybe we should try to prevent that by improving the API.

> For 3. (vbus) we could do something similar using a
> "vbus-regulator-id" device-property and the regulator
> framework, making the driver which controls Vbus register
> itself as a regulator.

The regulator framework does sound reasonable.

> I can take a shot at implementing both suggestions, but
> first lets try to get some general idea of how we want
> to solve this.

I hope I was able to explain myself, and make my case.

One more thing. I think we could actually build a little bit of
hierarchy and make the fusb302 the supply of, not bq24190, but the
PMIC instead. The PMIC would then be the only supply for the bq24190.

Then in case of TYPEC_CC_RP_DEF, the PMIC charger driver would always
know that it needs to execute the detection, and otherwise simply
passthough the limits. The driver for fusb302 would not need to do
anything extra in case of TYPEC_CC_RP_DEF.


Thanks,

-- 
heikki

USB TYPE-C support and the power-supply subsys (was Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support)

[Hans de Goede]

Hi,

On 17-05-17 13:45, Heikki Krogerus wrote:
> Hi,
> 
> On Wed, May 17, 2017 at 12:24:52AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 05/16/2017 02:07 PM, Heikki Krogerus wrote:
>> But we don't really have multiple sources here, we have only
>> one source, the USB-C cable hooked to an external power-supply.
>> Just because 2 different pieces of hardware may be involved in
>> determining the current limit does not mean that we should
>> model this as 2 different sources. Just as you rightfully
>> pointed out that me using 2 extcon devices for the single
>> Type-C connector is wrong, modelling it as 2 sources is
>> wrong too.
> 
> The power supply devices don't represent the port like the extcon
> device would. The power supply devices represent the two types
> of external chargers we support. So BC1.2 and Type-C/PD source.

Which are both USB chargers, and the TI bq24290 driver itself
also registers itself as a USB charger, continued below ...

>>> 1. Tell the BC1.2 detection to start from fusb302 driver
>>> 2. Deliver the power limits to the discrete charger ic or battery driver
>>> 3. Tell what ever regulates VBUS to start driving VBUS.

<snip>

>>> The second problem definitely needs to be handled using psy framework.
>>> The psy framework provides already everything needed for that.
>>
>> So above you're talking about sources to the bq24190, which if
>> I understand you correctly suggest that you want the tcpm code to
>> register a power-supply device per Type-C port ?
> 
> No, the underlying device driver (so fusb302) needs to register the
> power supply at this point. We just notify the psy framework if the
> limits we get from tcpm to the set_current_limit hook change.
> 
>> I'm not sure that
>> is a good idea, any registered power-supply devices will show up
>> under /sys/class/power_supply, currently on a typical system
>> there will be 2 entries under /sys/class/power_supply one for
>> the "Mains" or USB power input and one for the battery, adding
>> more entries there is likely to confuse userspace.
> 
> The userspace uses the type attribute to differentiate the supplies.
> Otherwise it would not be able to tell even which is the Battery and
> which is Mains. Having more power supplies is not a problem.

I disagree yes power-supply devices have a type, so if we do
as you suggest we end up with 3 power-supplies with type USB under
/sys/class/power_supply suggesting to userspace that the device
may be supplied with power through 3 different USB connectors.

This is simply just wrong. I understand that you want to decouple
the different drivers from each-other and I agree with that as
a goal.

But using the power-supply subsys in the way you suggests means
that the way we end up solving a problem which is purely
about different pieces of code in the kernel talking to each
other gets leaked to userspace and once we've done this userspace
may start depending on this and we cannot change things later.

TL;DR: I'm against the FUSB302 and the BC1.2 drivers each
registering a power-supply because:

1) There should be only one power-supply device registered
not 3, since there is only one power-input to the board, not 3.

2) Ideally the in kernel interface should not be visible to
userspace at all, we are still figuring all of this out and
we may need to change things later leaking internal kernel
details to userspace in something which will become an ABI
is not a good idea.

I've added Sebastian Reichel, the power-supply subsys
maintainer to the Cc so that he can weigh in on this.

>> More in general having 2 power-supply devices for what is
>> in essence one power-input feels wrong.
>>
>> We can still use the power-supply framework, but I would
>> envision this working like this:
>>
>> The platform code which enumerates the type-c controller
>> sets a "input-current-power-supply-name" string device-property
>> on the tcpm's (parent)device. When this is set the tpcm code
>> will do a power_supply_get_by_name and set the input
>> current on the returned power_supply by setting the
>> POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT property.
> 
> The psy framework is probable a bit messy at them moment. It
> definitely does not do much protecting and in many cases one driver
> registers a power supply and an other just takes it over, but that
> should be avoided as it makes things difficult (painful) to maintain.
> It also poses risks IMO. There certainly almost never seems to be a
> real need for that.
> 
> In this case the driver for fusb302 registers a power supply that
> provides properties like POWER_SUPPLY_PROP_VOLTAGE_MAX, etc. and
> simply calls power_supply_changed() when ever needed (when we know the
> limits and when a source is attached/de-attached -> changes PRESENT
> & ONLINE properties). The fusb302 driver does not need to know if
> there are any consumers for it or not. The platform driver that
> registers the device for it will simply assign the consumer for it in
> this case, but in the future we want to get that kind of detail from
> the platform of course, so ACPI or DT.
> 
> The PMIC charger driver will similarly register a power supply device
> and function pretty much exactly the same way.
> 
> The consumer, bq24190, will receive notification from psy framework to
> its external_power_changed hook when ever either of the supplies
> calls power_supply_changed(). It then needs to check the properties of
> the supply (the external power) that are interesting to it in order to
> set the limits accordingly (this btw. is where the psy API and the
> class driver can be improved without much effort to make things easier
> for the consumers). The consumer driver in any case does not need to
> know what the supplies actually are, how many of them it has, or are
> there any at all, just like the drivers for the supplies don't need to
> know the consumers.

I like parts of this idea, but as said I've trouble with exporting 3
power-supply devices to userspace for this.

I must also say that I'm not a fan of making the BC-1.2 charger
a separate power-supply and letting the consumer figure out which
one to use, but lets forget about the BC-1.2 charger for now and
focus on solving this for just setting the TYPE-C determined
input current limit for now.

<snip>

> I hope I was able to explain myself, and make my case.

Explain yes, but I'm really worried about the consequences of exporting
extra API/ABI to userspace for this, while we are purely trying to
solve in an kernel communication problem.

> One more thing. I think we could actually build a little bit of
> hierarchy and make the fusb302 the supply of, not bq24190, but the
> PMIC instead. The PMIC would then be the only supply for the bq24190.

Actually if you look at the schematic (I don't have a schematic for
my device, but I can deduce one) then the bq24190 is supplying power
to the pmic not the other-way around and the fusb302 is not supplying
power in anyway, it purely is negotiating things, but from an electrical
pov it is not supplying anything. Anyways as said lets forgot about
the PMIC doing BC1.2 detection for now and first focus on how we
can get the current-limit info from the fusb302 driver to the
power-supply device registered by the bq24190 driver.

Regards,

Hans

Re: USB TYPE-C support and the power-supply subsys (was Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support)

[Heikki Krogerus]

On Wed, May 17, 2017 at 04:47:14PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 17-05-17 13:45, Heikki Krogerus wrote:
> > Hi,
> > 
> > On Wed, May 17, 2017 at 12:24:52AM +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 05/16/2017 02:07 PM, Heikki Krogerus wrote:
> > > But we don't really have multiple sources here, we have only
> > > one source, the USB-C cable hooked to an external power-supply.
> > > Just because 2 different pieces of hardware may be involved in
> > > determining the current limit does not mean that we should
> > > model this as 2 different sources. Just as you rightfully
> > > pointed out that me using 2 extcon devices for the single
> > > Type-C connector is wrong, modelling it as 2 sources is
> > > wrong too.
> > 
> > The power supply devices don't represent the port like the extcon
> > device would. The power supply devices represent the two types
> > of external chargers we support. So BC1.2 and Type-C/PD source.
> 
> Which are both USB chargers, and the TI bq24290 driver itself
> also registers itself as a USB charger, continued below ...
> 
> > > > 1. Tell the BC1.2 detection to start from fusb302 driver
> > > > 2. Deliver the power limits to the discrete charger ic or battery driver
> > > > 3. Tell what ever regulates VBUS to start driving VBUS.
> 
> <snip>
> 
> > > > The second problem definitely needs to be handled using psy framework.
> > > > The psy framework provides already everything needed for that.
> > > 
> > > So above you're talking about sources to the bq24190, which if
> > > I understand you correctly suggest that you want the tcpm code to
> > > register a power-supply device per Type-C port ?
> > 
> > No, the underlying device driver (so fusb302) needs to register the
> > power supply at this point. We just notify the psy framework if the
> > limits we get from tcpm to the set_current_limit hook change.
> > 
> > > I'm not sure that
> > > is a good idea, any registered power-supply devices will show up
> > > under /sys/class/power_supply, currently on a typical system
> > > there will be 2 entries under /sys/class/power_supply one for
> > > the "Mains" or USB power input and one for the battery, adding
> > > more entries there is likely to confuse userspace.
> > 
> > The userspace uses the type attribute to differentiate the supplies.
> > Otherwise it would not be able to tell even which is the Battery and
> > which is Mains. Having more power supplies is not a problem.
> 
> I disagree yes power-supply devices have a type, so if we do
> as you suggest we end up with 3 power-supplies with type USB under
> /sys/class/power_supply suggesting to userspace that the device
> may be supplied with power through 3 different USB connectors.
> 
> This is simply just wrong. I understand that you want to decouple
> the different drivers from each-other and I agree with that as
> a goal.
> 
> But using the power-supply subsys in the way you suggests means
> that the way we end up solving a problem which is purely
> about different pieces of code in the kernel talking to each
> other gets leaked to userspace and once we've done this userspace
> may start depending on this and we cannot change things later.
> 
> TL;DR: I'm against the FUSB302 and the BC1.2 drivers each
> registering a power-supply because:
> 
> 1) There should be only one power-supply device registered
> not 3, since there is only one power-input to the board, not 3.
> 
> 2) Ideally the in kernel interface should not be visible to
> userspace at all, we are still figuring all of this out and
> we may need to change things later leaking internal kernel
> details to userspace in something which will become an ABI
> is not a good idea.
> 
> I've added Sebastian Reichel, the power-supply subsys
> maintainer to the Cc so that he can weigh in on this.
> 
> > > More in general having 2 power-supply devices for what is
> > > in essence one power-input feels wrong.
> > > 
> > > We can still use the power-supply framework, but I would
> > > envision this working like this:
> > > 
> > > The platform code which enumerates the type-c controller
> > > sets a "input-current-power-supply-name" string device-property
> > > on the tcpm's (parent)device. When this is set the tpcm code
> > > will do a power_supply_get_by_name and set the input
> > > current on the returned power_supply by setting the
> > > POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT property.
> > 
> > The psy framework is probable a bit messy at them moment. It
> > definitely does not do much protecting and in many cases one driver
> > registers a power supply and an other just takes it over, but that
> > should be avoided as it makes things difficult (painful) to maintain.
> > It also poses risks IMO. There certainly almost never seems to be a
> > real need for that.
> > 
> > In this case the driver for fusb302 registers a power supply that
> > provides properties like POWER_SUPPLY_PROP_VOLTAGE_MAX, etc. and
> > simply calls power_supply_changed() when ever needed (when we know the
> > limits and when a source is attached/de-attached -> changes PRESENT
> > & ONLINE properties). The fusb302 driver does not need to know if
> > there are any consumers for it or not. The platform driver that
> > registers the device for it will simply assign the consumer for it in
> > this case, but in the future we want to get that kind of detail from
> > the platform of course, so ACPI or DT.
> > 
> > The PMIC charger driver will similarly register a power supply device
> > and function pretty much exactly the same way.
> > 
> > The consumer, bq24190, will receive notification from psy framework to
> > its external_power_changed hook when ever either of the supplies
> > calls power_supply_changed(). It then needs to check the properties of
> > the supply (the external power) that are interesting to it in order to
> > set the limits accordingly (this btw. is where the psy API and the
> > class driver can be improved without much effort to make things easier
> > for the consumers). The consumer driver in any case does not need to
> > know what the supplies actually are, how many of them it has, or are
> > there any at all, just like the drivers for the supplies don't need to
> > know the consumers.
> 
> I like parts of this idea, but as said I've trouble with exporting 3
> power-supply devices to userspace for this.
> 
> I must also say that I'm not a fan of making the BC-1.2 charger
> a separate power-supply and letting the consumer figure out which
> one to use, but lets forget about the BC-1.2 charger for now and
> focus on solving this for just setting the TYPE-C determined
> input current limit for now.

OK, You have a point. I happy to agree that adding an other psy for
the BC1.2 charger alone is not the correct thing to do.

I'm mainly interested in just considering USB as a power supply with a
board like this, because really, that is what it is, but also by using
the power supply class properly (and possibly improving it a little),
we avoid unnecessary software couplings.

> > I hope I was able to explain myself, and make my case.
> 
> Explain yes, but I'm really worried about the consequences of exporting
> extra API/ABI to userspace for this, while we are purely trying to
> solve in an kernel communication problem.
> 
> > One more thing. I think we could actually build a little bit of
> > hierarchy and make the fusb302 the supply of, not bq24190, but the
> > PMIC instead. The PMIC would then be the only supply for the bq24190.
> 
> Actually if you look at the schematic (I don't have a schematic for
> my device, but I can deduce one) then the bq24190 is supplying power
> to the pmic not the other-way around and the fusb302 is not supplying
> power in anyway, it purely is negotiating things, but from an electrical
> pov it is not supplying anything. Anyways as said lets forgot about
> the PMIC doing BC1.2 detection for now and first focus on how we
> can get the current-limit info from the fusb302 driver to the
> power-supply device registered by the bq24190 driver.

I second that.


Thanks Hans,

-- 
heikki

Re: USB TYPE-C support and the power-supply subsys (was Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support)

[Heikki Krogerus]

On Thu, May 18, 2017 at 11:37:56AM +0300, Heikki Krogerus wrote:
> I'm mainly interested in just considering USB as a power supply with a
> board like this, because really, that is what it is, but also by using
> the power supply class properly (and possibly improving it a little),
> we avoid unnecessary software couplings.

"USB" -> "USB Type-C/PD". Sorry about that.

-- 
heikki

Re: USB TYPE-C support and the power-supply subsys (was Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support)

[Hans de Goede]

Hi,

On 18-05-17 10:37, Heikki Krogerus wrote:
> On Wed, May 17, 2017 at 04:47:14PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 17-05-17 13:45, Heikki Krogerus wrote:

<snip>

>>> In this case the driver for fusb302 registers a power supply that
>>> provides properties like POWER_SUPPLY_PROP_VOLTAGE_MAX, etc. and
>>> simply calls power_supply_changed() when ever needed (when we know the
>>> limits and when a source is attached/de-attached -> changes PRESENT
>>> & ONLINE properties). The fusb302 driver does not need to know if
>>> there are any consumers for it or not. The platform driver that
>>> registers the device for it will simply assign the consumer for it in
>>> this case, but in the future we want to get that kind of detail from
>>> the platform of course, so ACPI or DT.
>>>
>>> The PMIC charger driver will similarly register a power supply device
>>> and function pretty much exactly the same way.
>>>
>>> The consumer, bq24190, will receive notification from psy framework to
>>> its external_power_changed hook when ever either of the supplies
>>> calls power_supply_changed(). It then needs to check the properties of
>>> the supply (the external power) that are interesting to it in order to
>>> set the limits accordingly (this btw. is where the psy API and the
>>> class driver can be improved without much effort to make things easier
>>> for the consumers). The consumer driver in any case does not need to
>>> know what the supplies actually are, how many of them it has, or are
>>> there any at all, just like the drivers for the supplies don't need to
>>> know the consumers.
>>
>> I like parts of this idea, but as said I've trouble with exporting 3
>> power-supply devices to userspace for this.
>>
>> I must also say that I'm not a fan of making the BC-1.2 charger
>> a separate power-supply and letting the consumer figure out which
>> one to use, but lets forget about the BC-1.2 charger for now and
>> focus on solving this for just setting the TYPE-C determined
>> input current limit for now.
> 
> OK, You have a point. I happy to agree that adding an other psy for
> the BC1.2 charger alone is not the correct thing to do.
> 
> I'm mainly interested in just considering USB as a power supply with a
> board like this, because really, that is what it is, but also by using
> the power supply class properly (and possibly improving it a little),
> we avoid unnecessary software couplings.

Ok, so although I'm still not 100% sold on having the fusb302 driver
registering a power-supply is a good idea from an userspace API pov, I
from a design pov otherwise. And in a way it does make sense the fusb302
driver is a way to query (and in some case modify) settings of the external
power-supply connected to the USB-C port.

So maybe we need a new "External" power-supply type for this ? Either way
this should not be a real issue since userspace (upower) does not seem to
do much if anything with power-supply class devices with a type of USB.

So lets go with the plan to make the fusb302 driver register a power-supply
for now, which will be a supplier for (for example) the bq24190 charger.

2 questions about implementing this:

1) You said you want the power-supply code (get_property, etc.) to live
in the fusb302 driver, rather then in the tcpm code. I agree that the
fusb302 device should be the parent-device of the power-supply, but I can
see registering a power-supply and querying things like
POWER_SUPPLY_PROP_VOLTAGE_MAX being something which we will want in other
USB TYPE-C drivers too, so wouldn't it be better to have this code in
the generic tcpm bits ?

2) I could modify the bq24190 driver to check and update its
input-current-limit itself from its external_power_changed callback,
but this seems like something which many charger drivers will need
to implement so instead I think that drivers like bq24190 should just
be able to set a "sync_input_current_with_suppliers" flag and then
when a supplier calls power_supply_changed() have the core call
set_property PROP_INPUT_CURRENT_LIMIT. That way all we need to do
to get charger drivers to support this is set that flag rather then
copy and paste code. Or maybe a
power_supply_sync_input_current_with_suppliers() helper which drivers
can call from their external_power_changed callback. Thinking more
about this I think I like the helper function idea better.

Regards,

Hans

Re: USB TYPE-C support and the power-supply subsys (was Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support)

[Hans de Goede]

Hi,

On 19-05-17 22:12, Hans de Goede wrote:
> Hi,
> 
> On 18-05-17 10:37, Heikki Krogerus wrote:
>> On Wed, May 17, 2017 at 04:47:14PM +0200, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 17-05-17 13:45, Heikki Krogerus wrote:
> 
> <snip>
> 
>>>> In this case the driver for fusb302 registers a power supply that
>>>> provides properties like POWER_SUPPLY_PROP_VOLTAGE_MAX, etc. and
>>>> simply calls power_supply_changed() when ever needed (when we know the
>>>> limits and when a source is attached/de-attached -> changes PRESENT
>>>> & ONLINE properties). The fusb302 driver does not need to know if
>>>> there are any consumers for it or not. The platform driver that
>>>> registers the device for it will simply assign the consumer for it in
>>>> this case, but in the future we want to get that kind of detail from
>>>> the platform of course, so ACPI or DT.
>>>>
>>>> The PMIC charger driver will similarly register a power supply device
>>>> and function pretty much exactly the same way.
>>>>
>>>> The consumer, bq24190, will receive notification from psy framework to
>>>> its external_power_changed hook when ever either of the supplies
>>>> calls power_supply_changed(). It then needs to check the properties of
>>>> the supply (the external power) that are interesting to it in order to
>>>> set the limits accordingly (this btw. is where the psy API and the
>>>> class driver can be improved without much effort to make things easier
>>>> for the consumers). The consumer driver in any case does not need to
>>>> know what the supplies actually are, how many of them it has, or are
>>>> there any at all, just like the drivers for the supplies don't need to
>>>> know the consumers.
>>>
>>> I like parts of this idea, but as said I've trouble with exporting 3
>>> power-supply devices to userspace for this.
>>>
>>> I must also say that I'm not a fan of making the BC-1.2 charger
>>> a separate power-supply and letting the consumer figure out which
>>> one to use, but lets forget about the BC-1.2 charger for now and
>>> focus on solving this for just setting the TYPE-C determined
>>> input current limit for now.
>>
>> OK, You have a point. I happy to agree that adding an other psy for
>> the BC1.2 charger alone is not the correct thing to do.
>>
>> I'm mainly interested in just considering USB as a power supply with a
>> board like this, because really, that is what it is, but also by using
>> the power supply class properly (and possibly improving it a little),
>> we avoid unnecessary software couplings.
> 
> Ok, so although I'm still not 100% sold on having the fusb302 driver
> registering a power-supply is a good idea from an userspace API pov, I
> from a design pov otherwise. And in a way it does make sense the fusb302
> driver is a way to query (and in some case modify) settings of the external
> power-supply connected to the USB-C port.

Ugh I somewhat mangled the first sentence of this paragraph,
here is a correct version:

Ok, so although I'm still not 100% sold on having the fusb302 driver
registering a power-supply being a good idea from an userspace API pov, I
do actually like it from a design pov.
> So maybe we need a new "External" power-supply type for this ? Either way
> this should not be a real issue since userspace (upower) does not seem to
> do much if anything with power-supply class devices with a type of USB.
> 
> So lets go with the plan to make the fusb302 driver register a power-supply
> for now, which will be a supplier for (for example) the bq24190 charger.
> 
> 2 questions about implementing this:
> 
> 1) You said you want the power-supply code (get_property, etc.) to live
> in the fusb302 driver, rather then in the tcpm code. I agree that the
> fusb302 device should be the parent-device of the power-supply, but I can
> see registering a power-supply and querying things like
> POWER_SUPPLY_PROP_VOLTAGE_MAX being something which we will want in other
> USB TYPE-C drivers too, so wouldn't it be better to have this code in
> the generic tcpm bits ?
> 
> 2) I could modify the bq24190 driver to check and update its
> input-current-limit itself from its external_power_changed callback,
> but this seems like something which many charger drivers will need
> to implement so instead I think that drivers like bq24190 should just
> be able to set a "sync_input_current_with_suppliers" flag and then
> when a supplier calls power_supply_changed() have the core call
> set_property PROP_INPUT_CURRENT_LIMIT? That way all we need to do
> to get charger drivers to support this is set that flag rather then
> copy and paste code. Or maybe a
> power_supply_sync_input_current_with_suppliers() helper which drivers
> can call from their external_power_changed callback ?
 >> Thinking more about this I think I like the helper function idea better

Regards,

Hans

Re: USB TYPE-C support and the power-supply subsys (was Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support)

[Heikki Krogerus]

On Fri, May 19, 2017 at 10:12:14PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 18-05-17 10:37, Heikki Krogerus wrote:
> > On Wed, May 17, 2017 at 04:47:14PM +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 17-05-17 13:45, Heikki Krogerus wrote:
> 
> <snip>
> 
> > > > In this case the driver for fusb302 registers a power supply that
> > > > provides properties like POWER_SUPPLY_PROP_VOLTAGE_MAX, etc. and
> > > > simply calls power_supply_changed() when ever needed (when we know the
> > > > limits and when a source is attached/de-attached -> changes PRESENT
> > > > & ONLINE properties). The fusb302 driver does not need to know if
> > > > there are any consumers for it or not. The platform driver that
> > > > registers the device for it will simply assign the consumer for it in
> > > > this case, but in the future we want to get that kind of detail from
> > > > the platform of course, so ACPI or DT.
> > > > 
> > > > The PMIC charger driver will similarly register a power supply device
> > > > and function pretty much exactly the same way.
> > > > 
> > > > The consumer, bq24190, will receive notification from psy framework to
> > > > its external_power_changed hook when ever either of the supplies
> > > > calls power_supply_changed(). It then needs to check the properties of
> > > > the supply (the external power) that are interesting to it in order to
> > > > set the limits accordingly (this btw. is where the psy API and the
> > > > class driver can be improved without much effort to make things easier
> > > > for the consumers). The consumer driver in any case does not need to
> > > > know what the supplies actually are, how many of them it has, or are
> > > > there any at all, just like the drivers for the supplies don't need to
> > > > know the consumers.
> > > 
> > > I like parts of this idea, but as said I've trouble with exporting 3
> > > power-supply devices to userspace for this.
> > > 
> > > I must also say that I'm not a fan of making the BC-1.2 charger
> > > a separate power-supply and letting the consumer figure out which
> > > one to use, but lets forget about the BC-1.2 charger for now and
> > > focus on solving this for just setting the TYPE-C determined
> > > input current limit for now.
> > 
> > OK, You have a point. I happy to agree that adding an other psy for
> > the BC1.2 charger alone is not the correct thing to do.
> > 
> > I'm mainly interested in just considering USB as a power supply with a
> > board like this, because really, that is what it is, but also by using
> > the power supply class properly (and possibly improving it a little),
> > we avoid unnecessary software couplings.
> 
> Ok, so although I'm still not 100% sold on having the fusb302 driver
> registering a power-supply is a good idea from an userspace API pov, I
> from a design pov otherwise. And in a way it does make sense the fusb302
> driver is a way to query (and in some case modify) settings of the external
> power-supply connected to the USB-C port.
> 
> So maybe we need a new "External" power-supply type for this ? Either way
> this should not be a real issue since userspace (upower) does not seem to
> do much if anything with power-supply class devices with a type of USB.
> 
> So lets go with the plan to make the fusb302 driver register a power-supply
> for now, which will be a supplier for (for example) the bq24190 charger.

Couldn't we extend the psy framework a little so it would be able to
support classes of devices that are not being presented to the user
space as power supplies as such, but are possibly just part of a chain
that makes up a single power supply? I'm playing with ideas at this
point, I'm not sure if that is feasible, but it does not sound
impossible.

I've been playing with that idea for some time now. Maybe it's time to
really check out what could be done :-).

> 2 questions about implementing this:
> 
> 1) You said you want the power-supply code (get_property, etc.) to live
> in the fusb302 driver, rather then in the tcpm code. I agree that the
> fusb302 device should be the parent-device of the power-supply, but I can
> see registering a power-supply and querying things like
> POWER_SUPPLY_PROP_VOLTAGE_MAX being something which we will want in other
> USB TYPE-C drivers too, so wouldn't it be better to have this code in
> the generic tcpm bits ?

I was thinking that at first stage it would be better to mix the
support into the device drivers, and later move it to tcpm, but I'm
not against it.

Actually, I was hoping that we could at one point consider every USB
port as a potential "power supply", not only Type-C ports, and
therefore handle the support in USB core, but that at least is
something that we probable should not think about at this point.

> 2) I could modify the bq24190 driver to check and update its
> input-current-limit itself from its external_power_changed callback,
> but this seems like something which many charger drivers will need
> to implement so instead I think that drivers like bq24190 should just
> be able to set a "sync_input_current_with_suppliers" flag and then
> when a supplier calls power_supply_changed() have the core call
> set_property PROP_INPUT_CURRENT_LIMIT. That way all we need to do
> to get charger drivers to support this is set that flag rather then
> copy and paste code. Or maybe a
> power_supply_sync_input_current_with_suppliers() helper which drivers
> can call from their external_power_changed callback. Thinking more
> about this I think I like the helper function idea better.

That sounds reasonable to me.


Br,

-- 
heikki

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Guenter Roeck]

On Fri, May 12, 2017 at 11:22:46PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 24-04-17 15:12, Guenter Roeck wrote:
> >On 04/24/2017 04:02 AM, Heikki Krogerus wrote:
> >>+Guenter
> >>
> >>On Fri, Apr 21, 2017 at 09:51:50PM +0300, Andy Shevchenko wrote:
> >>>+Cc: Heikki.
> >>>
> >>>He might comment on this.
> >>
> >>Thanks Andy.
> >>
> >>>On Fri, Apr 21, 2017 at 4:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> >>>>Add support for USB TYPE-C cable detection on systems using a
> >>>>FUSB302 USB TYPE-C controller.
> >>>>
> >>>>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>>>---
> >>>> drivers/extcon/Kconfig          |   8 +
> >>>> drivers/extcon/Makefile         |   1 +
> >>>> drivers/extcon/extcon-fusb302.c | 782 ++++++++++++++++++++++++++++++++++++++++
> >>>> 3 files changed, 791 insertions(+)
> >>>> create mode 100644 drivers/extcon/extcon-fusb302.c
> >>
> >>There is now the typec class in linux-next that really needs to be
> >>used with all USB Type-C PHYs like fusb302.
> >>
> >>Unless I'm mistaken, Guenter has also written a driver for fusb302. I
> >
> >Not me; it was Nathan.
> >
> >>have not seen it, but if I understood correctly, that driver will
> >>register itself with the upcoming tcpm (USB Type-C Port Manager) [1],
> >>which in practice would mean we can take properly advantage of the USB
> >>PD transceiver on fusb302.
> >>
> >>Guenter! Can you publish the fusb302 driver?
> >>
> >
> >Working on it.
> 
> Ok, I see this has landed in staging. So lets go with this driver
> then and not mine (I did not get around to the PD stuff yet, so
> that is fine).
> 
> Question, how is this supposed to interface with the rest of the
> kernel ? Specifically the commit message for the tcpm frameworks
> says:
> 
> "This driver only implements the state machine. Lower level drivers are
> responsible for
> - Reporting VBUS status and activating VBUS
> - Setting CC lines and providing CC line status
> - Setting line polarity
> - Activating and deactivating VCONN
> - Setting the current limit
> - Activating and deactivating PD message transfers
> - Sending and receiving PD messages"
> 
> But the FUSB302 cannot set the current limit...
> 

There is an underlying driver which I didn't pay enough attention to.

Badhri, Yueyao, can we publish the pd engine code as well ?
If that doesn't solve the problem, it might at least give a starting
point.

Thanks,
Guenter

> The device I'm working on:
> http://www.gpd.hk/gpdwin.asp
> 
> Has the following more or less relevant components:
> 
> -Intel Cherry Trail Z8700 SoC
> -Intel Cherry Trail Whiskey Cove PMIC (which is different from the Broxton Whiskey Cove PMIC),
>  this PMIC uses an external charger and fuel-gauge:
> -TI bq24292i charger
> -Maxim MAX17047 fuel-gauge
> -PI3USB30532 USB TYPE-C mux
> 
> Currently I only have the USB-C connector on the
> device working in USB-2 mode (I did not realize
> the USB-3 bits were wired up at first).
> 
> The device ships with a charger with an USB-A
> receptacle and an USB-2 only USB-A to USB-C
> cable.
> 
> The way I've hooked up USB-2 charging is like this:
> The Whiskey Cove PMIC has built in USB-2 BC detection, and
> I've written an extcon driver for this which reports one
> of the following cables depended on the BC detection:
> 
>         EXTCON_CHG_USB_SDP,
>         EXTCON_CHG_USB_CDP,
>         EXTCON_CHG_USB_DCP,
> 
> And I've modified the bq24290_charger driver to (optionally)
> receive an extcon name through device-properties and if
> it does, then set the input current based on the detected
> charger type (0.5 A for SDP, 1.5A for CDP, 2A for DCP).
> 
> Most of the patches for this are upstream. But once we
> hookup the FUSB302 driver we need to propagate the current
> limit it has negotiated to the bq24290_charger driver.
> 
> Which is part of the reason why I wrote the 5th patch of
> my original series, let me reply to Heikki's question about
> that one here as it is related:
> 
> On 24-04-17 16:25, Heikki Krogerus wrote:
> 
> > Doesn't this mean you have several extcon_dev registered for a
> > single port?
> 
> Yes this is unfortunate, but the primary consumer of extcon
> info is the kernel itself and we can teach it to look at the
> right one.
> 
> > I'm not completely sure how extcon framework is meant to
> > be used, but shouldn't a single extcon_dev represent all the types of
> > connectors a port is meant to support?
> 
> Ideally, yes. But here we've 2 chips / drivers connected
> to the same connector (more even, but 2 which provide extcon
> related info).
> 
> As explained above the device ships with an USB-2 charger +
> USB-A to USB-C cable, that cable correctly gets seen by the
> FUSB302 as a TYPEC_CC_RP_DEF cable, which means limiting input
> current to 0.5A, but as the FUSB302 datasheet says:
> 
> "The host software is expected to follow the USB Type-C
> specification for charging current priority based on
> feedback from the FUSB302 detection, external BC1.2 detection
> and any USB Power Delivery communication.
> 
> The FUSB302 does not integrate BC1.2 charger
> detection which is assumed available in the USB
> transceiver or USB charger in the system."
> 
> So when we see TYPEC_CC_RP_DEF we need to ask the
> Cherry Trail PMIC (in my case) to do BC detection
> and use the result of that, otherwise we end up
> setting input current limit way too low.
> 
> >> Since the Type-C controller info is incomplete and needs to be
> >> supplemented with the BC1.2 detection result in some cases, this
> >> commit makes the intel-cht-wc extcon driver monitor the extcon device
> >> registered by the Type-C controller and report that as our extcon state
> >> except when BC-1.2 detection should be used. This allows extcon
> >> consumers on these boards to keep monitoring only the intel-cht-wc extcon
> >> and then get complete extcon info from that, which combines the Type-C
> >> and BC-1.2 info.
> >
> > I don't really understand why does this need to be done in such a
> > complex manner? Both components should just report the result and be
> > done with it, and there is no need for any coupling between the two.
> > If there is a consumer for the power, the driver for it can decide on
> > its own the limits and maximum power from the two sources.
> 
> To me making the combination of the 2 sources the problem
> of the consumer seems just wrong, as you mentioned
> above there should really be only one extcon for the
> connector, likewise their should be only one definitive
> source on what the input current limit is.
> 
> TL;DR: We need some way to combine the current limit info
> from TYPE-C and USB-2 BC detection into a single source and
> to propagate that current to drivers which can actually set
> the current-limit.
> 
> Note I'm happy to use something else then extcon for this,
> but we do need some way to combine + propagate the
> current-limit.
> 
> Likewise we need a way to tell another driver to drive VBus
> on the USB-C connector. In my case this is again done by the
> bq24290_charger driver, which currently does this based on the
> combination of EXTCON_CHG_USB_* not being set on the extcon +
> EXTCON_USB_HOST being set.
> 
> One possible solution would be for the
> drivers/extcon/extcon-intel-cht-wc.c to somehow hook into
> the tpmc device and to get notifications of state changes,
> then it could reflect the info from the FUSB302 in its
> extcon info, not sure if that is the best design though.
> 
> What also seems to missing is for a way to hookup a
> mux driver to deal with upside-down plug-ins and
> alt-mode switching.
> 
> Regards,
> 
> Hans

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2105 bytes --]

Hi Hans,

[auto build test ERROR on chanwoo-extcon/extcon-next]
[also build test ERROR on v4.11-rc7 next-20170421]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hans-de-Goede/extcon-Allow-extcon-drivers-to-specify-the-extcon-name/20170422-025657
base:   https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/extcon/extcon-fusb302.c: In function 'fusb302_set_state':
>> drivers/extcon/extcon-fusb302.c:311:18: error: 'USB_TYPEC_POLARITY_NORMAL' undeclared (first use in this function)
       prop.intval = USB_TYPEC_POLARITY_NORMAL;
                     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/extcon/extcon-fusb302.c:311:18: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/extcon/extcon-fusb302.c:315:18: error: 'USB_TYPEC_POLARITY_FLIP' undeclared (first use in this function)
       prop.intval = USB_TYPEC_POLARITY_FLIP;
                     ^~~~~~~~~~~~~~~~~~~~~~~

vim +/USB_TYPEC_POLARITY_NORMAL +311 drivers/extcon/extcon-fusb302.c

   305			fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
   306	
   307			/* Enable pull-down on CC1 / CC2 based on orientation */
   308			switch (data->status1a & STATUS1A_TOGSS_MASK) {
   309			case STATUS1A_TOGSS_SNK_CC1:
   310				switches0 = SWITCHES0_PDWN1 | SWITCHES0_MEAS_CC1;
 > 311				prop.intval = USB_TYPEC_POLARITY_NORMAL;
   312				break;
   313			case STATUS1A_TOGSS_SNK_CC2:
   314				switches0 = SWITCHES0_PDWN2 | SWITCHES0_MEAS_CC2;
 > 315				prop.intval = USB_TYPEC_POLARITY_FLIP;
   316				break;
   317			}
   318			fusb302_write_reg(data, REG_SWITCHES0, switches0);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59021 bytes --]

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Chanwoo Choi]

Hi Hans,

I'm sorry for late reply.
I'll check this thread and then give you my opinion.

Regards,
Chanwoo Choi

On 2017년 04월 21일 22:01, Hans de Goede wrote:
> Add support for USB TYPE-C cable detection on systems using a
> FUSB302 USB TYPE-C controller.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/Kconfig          |   8 +
>  drivers/extcon/Makefile         |   1 +
>  drivers/extcon/extcon-fusb302.c | 782 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 791 insertions(+)
>  create mode 100644 drivers/extcon/extcon-fusb302.c
> 
> diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
> index 32f2dc8..562db5b 100644
> --- a/drivers/extcon/Kconfig
> +++ b/drivers/extcon/Kconfig
> @@ -35,6 +35,14 @@ config EXTCON_AXP288
>  	  Say Y here to enable support for USB peripheral detection
>  	  and USB MUX switching by X-Power AXP288 PMIC.
>  
> +config EXTCON_FUSB302
> +	tristate "FUSB302 USB TYPE-C controller support"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  Say Y here to enable support for USB TYPE-C cable detection using
> +	  a FUSB302 USB TYPE-C controller.
> +
>  config EXTCON_GPIO
>  	tristate "GPIO extcon support"
>  	depends on GPIOLIB || COMPILE_TEST
> diff --git a/drivers/extcon/Makefile b/drivers/extcon/Makefile
> index ecfa958..e5eb493 100644
> --- a/drivers/extcon/Makefile
> +++ b/drivers/extcon/Makefile
> @@ -7,6 +7,7 @@ extcon-core-objs		+= extcon.o devres.o
>  obj-$(CONFIG_EXTCON_ADC_JACK)	+= extcon-adc-jack.o
>  obj-$(CONFIG_EXTCON_ARIZONA)	+= extcon-arizona.o
>  obj-$(CONFIG_EXTCON_AXP288)	+= extcon-axp288.o
> +obj-$(CONFIG_EXTCON_FUSB302)	+= extcon-fusb302.o
>  obj-$(CONFIG_EXTCON_GPIO)	+= extcon-gpio.o
>  obj-$(CONFIG_EXTCON_INTEL_INT3496) += extcon-intel-int3496.o
>  obj-$(CONFIG_EXTCON_INTEL_CHT_WC) += extcon-intel-cht-wc.o
> diff --git a/drivers/extcon/extcon-fusb302.c b/drivers/extcon/extcon-fusb302.c
> new file mode 100644
> index 0000000..577adb9
> --- /dev/null
> +++ b/drivers/extcon/extcon-fusb302.c
> @@ -0,0 +1,782 @@
> +/*
> + * Extcon USB-C cable detection driver for FUSB302 USB TYPE-C controller
> + *
> + * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/extcon.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +#include "extcon.h"
> +
> +#define REG_DEVICE_ID				0x01
> +#define DEVICE_ID_VER_MASK			GENMASK(7, 4)
> +#define DEVICE_ID_FUSB302_VER			0x80
> +
> +#define REG_SWITCHES0				0x02
> +#define SWITCHES0_PDWN1				BIT(0)
> +#define SWITCHES0_PDWN2				BIT(1)
> +#define SWITCHES0_PDWN				GENMASK(1, 0)
> +#define SWITCHES0_MEAS_CC1			BIT(2)
> +#define SWITCHES0_MEAS_CC2			BIT(3)
> +#define SWITCHES0_VCONN_CC1			BIT(4)
> +#define SWITCHES0_VCONN_CC2			BIT(5)
> +#define SWITCHES0_PU_EN1			BIT(6)
> +#define SWITCHES0_PU_EN2			BIT(7)
> +
> +#define REG_MEASURE				0x04
> +#define MEASURE_MDAC_MASK			GENMASK(5, 0)
> +/* Datasheet says MDAC must be set to 0x34 / 2226mV for vRd-3.0 detection */
> +#define MEASURE_MDAC_SNK_VRD30			0x34
> +/* MDAC must be set to 0x25 / 1600mV for disconnect det. with 80uA host-cur */
> +#define MEASURE_MDAC_SRC_80UA_HOST_CUR		0x25
> +
> +#define REG_CONTROL0				0x06
> +#define CONTROL0_HOST_CUR_MASK			GENMASK(3, 2)
> +#define CONTROL0_HOST_CUR_DISABLED		(0 << 2)
> +#define CONTROL0_HOST_CUR_80UA			(1 << 2)
> +#define CONTROL0_HOST_CUR_180UA			(2 << 2)
> +#define CONTROL0_HOST_CUR_330UA			(3 << 2)
> +#define CONTROL0_INT_MASK			BIT(5)
> +
> +#define REG_CONTROL2				0x08
> +#define CONTROL2_TOGGLE				BIT(0)
> +#define CONTROL2_MODE_MASK			GENMASK(2, 1)
> +#define CONTROL2_MODE_DRP			(1 << 1)
> +#define CONTROL2_MODE_SNK			(2 << 1)
> +#define CONTROL2_MODE_SRC			(3 << 1)
> +#define CONTROL2_SAVE_PWR_MASK			GENMASK(7, 6)
> +#define CONTROL2_SAVE_PWR_DISABLED		(0 << 6)
> +#define CONTROL2_SAVE_PWR_40MS			(1 << 6)
> +#define CONTROL2_SAVE_PWR_80MS			(2 << 6)
> +#define CONTROL2_SAVE_PWR_160MS			(3 << 6)
> +
> +#define REG_MASK1				0x0a
> +/* REG_MASK1 value for low-power / disabled state from datasheet */
> +#define MASK1_DISABLED				0xfe
> +#define MASK1_COMP_CHNG				BIT(5)
> +#define MASK1_VBUSOK				BIT(7)
> +
> +#define REG_POWER				0x0b
> +/* REG_POWER values for disabled and normal state from datasheet */
> +#define POWER_DISABLED				BIT(0)
> +#define POWER_NORMAL				GENMASK(2, 0)
> +
> +#define REG_RESET				0x0c
> +#define RESET_SW_RESET				BIT(0)
> +
> +#define REG_OCP					0x0d
> +
> +#define REG_MASKA				0x0e
> +/* REG_MASKA value for low-power / disabled state from datasheet */
> +#define MASKA_DISABLED				0xbf
> +
> +#define REG_MASKB				0x0f
> +/* REG_MASKB value for low-power / disabled state from datasheet */
> +#define MASKB_DISABLED				0x01
> +
> +#define REG_STATUS1A				0x3d
> +#define STATUS1A_TOGSS_MASK			GENMASK(5, 3)
> +#define STATUS1A_TOGSS_SRC_CC1			(1 << 3)
> +#define STATUS1A_TOGSS_SRC_CC2			(2 << 3)
> +#define STATUS1A_TOGSS_SNK_CC1			(5 << 3)
> +#define STATUS1A_TOGSS_SNK_CC2			(6 << 3)
> +
> +#define REG_INTERRUPTA				0x3e
> +#define INTERRUPTA_TOGDONE			BIT(6)
> +
> +#define REG_STATUS0				0x40
> +#define STATUS0_BC_LEVEL_MASK			GENMASK(1, 0)
> +#define STATUS0_BC_LEVEL_VRA			0
> +#define STATUS0_BC_LEVEL_VRDUSB			1
> +#define STATUS0_BC_LEVEL_VRD15			2
> +#define STATUS0_BC_LEVEL_VRD30			3
> +#define STATUS0_COMP				BIT(5)
> +#define STATUS0_VBUSOK				BIT(7)
> +
> +#define REG_INTERRUPT				0x42
> +#define INTERRUPT_BC_LVL			BIT(0)
> +#define INTERRUPT_COMP_CHNG			BIT(5)
> +#define INTERRUPT_VBUSOK			BIT(7)
> +
> +/* Timeouts from the FUSB302 datasheet */
> +#define TTOGCYCLE				msecs_to_jiffies(40 + 60 + 40)
> +
> +/* Timeouts from the USB-C specification */
> +#define TPDDEBOUNCE				msecs_to_jiffies(20)
> +#define TDRPTRY_MS				75
> +#define TDRPTRY					msecs_to_jiffies(TDRPTRY_MS)
> +#define TDRPTRYWAIT				msecs_to_jiffies(400)
> +#define TVBUSON					msecs_to_jiffies(275)
> +
> +enum typec_port_type {
> +	TYPEC_PORT_DFP,
> +	TYPEC_PORT_UFP,
> +	TYPEC_PORT_DRP,
> +};
> +
> +enum typec_role {
> +	TYPEC_SINK,
> +	TYPEC_SOURCE,
> +};
> +
> +enum fusb302_state {
> +	DISABLED_SNK, /* UFP */
> +	DISABLED_SRC, /* DFP */
> +	DISABLED_DRP,
> +	UNATTACHED_SNK,
> +	ATTACHED_SNK,
> +	UNATTACHED_SRC,
> +	ATTACHED_SRC,
> +};
> +/* For debugging */
> +static __maybe_unused const char *fusb302_state_str[] = {
> +	"DISABLED_SNK",
> +	"DISABLED_SRC",
> +	"DISABLED_DRP",
> +	"UNATTACHED_SNK",
> +	"ATTACHED_SNK",
> +	"UNATTACHED_SRC",
> +	"ATTACHED_SRC",
> +};
> +
> +enum fusb302_event {
> +	TOGGLE_DONE,
> +	BC_LVL_CHANGE,
> +	VBUS_VALID,
> +	VBUS_INVALID,
> +	COMP_LOW,
> +	COMP_HIGH,
> +	TIMEOUT,
> +	FALLBACK_TIMEOUT,
> +};
> +/* For debugging */
> +static __maybe_unused const char *fusb302_event_str[] = {
> +	"TOGGLE_DONE",
> +	"BC_LVL_CHANGE",
> +	"VBUS_VALID",
> +	"VBUS_INVALID",
> +	"COMP_LOW",
> +	"COMP_HIGH",
> +	"TIMEOUT",
> +	"FALLBACK_TIMEOUT",
> +};
> +
> +static const unsigned int fusb302_extcon_cables[] = {
> +	EXTCON_USB,
> +	EXTCON_USB_HOST,
> +	EXTCON_CHG_USB_SDP,
> +	EXTCON_CHG_USB_CDP,
> +	EXTCON_CHG_USB_FAST,
> +	EXTCON_NONE,
> +};
> +
> +struct fusb302_data {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct extcon_dev *edev;
> +	struct mutex lock;
> +	struct delayed_work timeout;
> +	struct delayed_work fallback_timeout;
> +	struct delayed_work bc_work;
> +	enum fusb302_state state;
> +	enum typec_port_type port_type;
> +	enum typec_role preferred_role;
> +	int status0;
> +	int status1a;
> +	unsigned int snk_cable_id;
> +};
> +
> +static void fusb302_write_reg(struct fusb302_data *data, int reg, int val)
> +{
> +	int ret;
> +
> +	ret = regmap_write(data->regmap, reg, val);
> +	if (ret)
> +		dev_err(data->dev, "Error writing reg %02x: %d\n", reg, ret);
> +}
> +
> +static int fusb302_read_reg(struct fusb302_data *data, int reg)
> +{
> +	int ret, val;
> +
> +	ret = regmap_read(data->regmap, reg, &val);
> +	if (ret) {
> +		dev_err(data->dev, "Error reading reg %02x: %d\n", reg, ret);
> +		return 0;
> +	}
> +
> +	return val;
> +}
> +
> +static void fusb302_update_bits(struct fusb302_data *data, int reg,
> +				int mask, int val)
> +{
> +	int ret;
> +
> +	ret = regmap_update_bits(data->regmap, reg, mask, val);
> +	if (ret)
> +		dev_err(data->dev, "Error modifying reg %02x: %d\n", reg, ret);
> +}
> +
> +/* Small helper to sync EXTCON_CHG_USB_SDP and EXTCON_USB state */
> +static void fusb302_set_extcon_state(struct fusb302_data *data,
> +				     unsigned int cable, bool state)
> +{
> +	extcon_set_state_sync(data->edev, cable, state);
> +	if (cable == EXTCON_CHG_USB_SDP)
> +		extcon_set_state_sync(data->edev, EXTCON_USB, state);
> +}
> +
> +/* Helper for the disabled states */
> +static void fusb302_disable(struct fusb302_data *data)
> +{
> +	fusb302_write_reg(data, REG_POWER, POWER_DISABLED);
> +	fusb302_write_reg(data, REG_MASK1, MASK1_DISABLED);
> +	fusb302_write_reg(data, REG_MASKA, MASKA_DISABLED);
> +	fusb302_write_reg(data, REG_MASKB, MASKB_DISABLED);
> +	fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
> +}
> +
> +/*
> + * fusb302_set_state() and fusb302_handle_event() implement the 3 state
> + * machines from the datasheet folded into 1 state-machine for code re-use.
> + */
> +static void fusb302_set_state(struct fusb302_data *data,
> +			      enum fusb302_state state)
> +{
> +	int status, switches0 = 0;
> +	enum fusb302_state old_state = data->state;
> +	union extcon_property_value prop;
> +
> +	/* Kill pending timeouts from previous state */
> +	cancel_delayed_work(&data->timeout);
> +	cancel_delayed_work(&data->fallback_timeout);
> +	cancel_delayed_work(&data->bc_work);
> +
> +	dev_dbg(data->dev, "New state %s\n", fusb302_state_str[state]);
> +
> +	switch (old_state) {
> +	case ATTACHED_SNK:
> +		fusb302_set_extcon_state(data, data->snk_cable_id, false);
> +		break;
> +	case ATTACHED_SRC:
> +		fusb302_set_extcon_state(data, EXTCON_USB_HOST, false);
> +		break;
> +	default:
> +		break; /* Do nothing */
> +	}
> +
> +	switch (state) {
> +	case DISABLED_SNK:
> +		fusb302_disable(data);
> +		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
> +				    CONTROL2_MODE_SNK);
> +		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
> +				    CONTROL2_TOGGLE);
> +		break;
> +
> +	case DISABLED_SRC:
> +		fusb302_disable(data);
> +		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
> +				    CONTROL2_MODE_SRC);
> +		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
> +				    CONTROL2_TOGGLE);
> +		break;
> +
> +	case DISABLED_DRP:
> +		fusb302_disable(data);
> +		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_MODE_MASK,
> +				    CONTROL2_MODE_DRP);
> +		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE,
> +				    CONTROL2_TOGGLE);
> +		break;
> +
> +	case UNATTACHED_SNK:
> +		fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
> +
> +		/* Enable pull-down on CC1 / CC2 based on orientation */
> +		switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +		case STATUS1A_TOGSS_SNK_CC1:
> +			switches0 = SWITCHES0_PDWN1 | SWITCHES0_MEAS_CC1;
> +			prop.intval = USB_TYPEC_POLARITY_NORMAL;
> +			break;
> +		case STATUS1A_TOGSS_SNK_CC2:
> +			switches0 = SWITCHES0_PDWN2 | SWITCHES0_MEAS_CC2;
> +			prop.intval = USB_TYPEC_POLARITY_FLIP;
> +			break;
> +		}
> +		fusb302_write_reg(data, REG_SWITCHES0, switches0);
> +		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
> +		extcon_set_property(data->edev, EXTCON_USB,
> +				    EXTCON_PROP_USB_TYPEC_POLARITY, prop);
> +
> +		/* Enable VBUSOK and COMP irq at 2.24V for BC detection */
> +		fusb302_update_bits(data, REG_MASK1,
> +				    MASK1_VBUSOK | MASK1_COMP_CHNG, 0);
> +		fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
> +				    MEASURE_MDAC_SNK_VRD30);
> +
> +		status = fusb302_read_reg(data, REG_STATUS0);
> +		if (status & STATUS0_VBUSOK) {
> +			/* Go straight to ATTACHED_SNK */
> +			fusb302_set_state(data, ATTACHED_SNK);
> +			return;
> +		}
> +
> +		mod_delayed_work(system_wq, &data->timeout, TVBUSON);
> +		break;
> +
> +	case ATTACHED_SNK:
> +		mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
> +		break;
> +
> +	case UNATTACHED_SRC:
> +		fusb302_write_reg(data, REG_POWER, POWER_NORMAL);
> +
> +		/* Enable pull-up / Vconn on CC1 / CC2 based on orientation */
> +		switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +		case STATUS1A_TOGSS_SRC_CC1:
> +			switches0 = SWITCHES0_PU_EN1 | SWITCHES0_VCONN_CC2 |
> +				    SWITCHES0_MEAS_CC1;
> +			prop.intval = USB_TYPEC_POLARITY_NORMAL;
> +			break;
> +		case STATUS1A_TOGSS_SRC_CC2:
> +			switches0 = SWITCHES0_PU_EN2 | SWITCHES0_VCONN_CC1 |
> +				    SWITCHES0_MEAS_CC2;
> +			prop.intval = USB_TYPEC_POLARITY_FLIP;
> +			break;
> +		}
> +		fusb302_write_reg(data, REG_SWITCHES0, switches0);
> +		fusb302_update_bits(data, REG_CONTROL2, CONTROL2_TOGGLE, 0);
> +		extcon_set_property(data->edev, EXTCON_USB,
> +				    EXTCON_PROP_USB_TYPEC_POLARITY, prop);
> +
> +		/* Enable COMP irq at 1.6V for detach detection */
> +		fusb302_update_bits(data, REG_MASK1, MASK1_COMP_CHNG, 0);
> +		fusb302_update_bits(data, REG_MEASURE, MEASURE_MDAC_MASK,
> +				    MEASURE_MDAC_SRC_80UA_HOST_CUR);
> +
> +		status = fusb302_read_reg(data, REG_STATUS0);
> +		if (!(status & STATUS0_COMP)) {
> +			/* Go straight to ATTACHED_SRC */
> +			fusb302_set_state(data, ATTACHED_SRC);
> +			return;
> +		}
> +
> +		mod_delayed_work(system_wq, &data->timeout, TPDDEBOUNCE);
> +		break;
> +
> +	case ATTACHED_SRC:
> +		fusb302_set_extcon_state(data, EXTCON_USB_HOST, true);
> +		break;
> +	}
> +
> +	data->state = state;
> +}
> +
> +static void fusb302_set_state_disabled(struct fusb302_data *data)
> +{
> +
> +	switch (data->port_type) {
> +	case TYPEC_PORT_UFP:
> +		fusb302_set_state(data, DISABLED_SNK);
> +		break;
> +	case TYPEC_PORT_DFP:
> +		fusb302_set_state(data, DISABLED_SRC);
> +		break;
> +	case TYPEC_PORT_DRP:
> +		fusb302_set_state(data, DISABLED_DRP);
> +		break;
> +	}
> +}
> +
> +static void fusb302_handle_disabled_snk_event(struct fusb302_data *data,
> +					      int event)
> +{
> +	switch (event) {
> +	case TOGGLE_DONE:
> +		switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +		case STATUS1A_TOGSS_SNK_CC1:
> +		case STATUS1A_TOGSS_SNK_CC2:
> +			fusb302_set_state(data, UNATTACHED_SNK);
> +			break;
> +		}
> +		break;
> +
> +	case TIMEOUT:
> +		/* Cannot become snk fallback to src */
> +		fusb302_set_state(data, DISABLED_SRC);
> +		mod_delayed_work(system_wq, &data->fallback_timeout,
> +				 TDRPTRYWAIT);
> +		break;
> +
> +	case FALLBACK_TIMEOUT:
> +		/* Both states failed return to disabled drp state */
> +		fusb302_set_state(data, DISABLED_DRP);
> +		break;
> +	}
> +}
> +
> +static void fusb302_handle_disabled_src_event(struct fusb302_data *data,
> +					      int event)
> +{
> +	switch (event) {
> +	case TOGGLE_DONE:
> +		switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +		case STATUS1A_TOGSS_SRC_CC1:
> +		case STATUS1A_TOGSS_SRC_CC2:
> +			fusb302_set_state(data, UNATTACHED_SRC);
> +			break;
> +		}
> +		break;
> +
> +	case TIMEOUT:
> +		/* Cannot become src fallback to snk */
> +		fusb302_set_state(data, DISABLED_SNK);
> +		mod_delayed_work(system_wq, &data->fallback_timeout,
> +				 TDRPTRYWAIT);
> +		break;
> +
> +	case FALLBACK_TIMEOUT:
> +		/* Both states failed return to disabled drp state */
> +		fusb302_set_state(data, DISABLED_DRP);
> +		break;
> +	}
> +}
> +
> +static void fusb302_handle_disabled_drp_event(struct fusb302_data *data,
> +					      int event)
> +{
> +	switch (event) {
> +	case TOGGLE_DONE:
> +		switch (data->status1a & STATUS1A_TOGSS_MASK) {
> +		case STATUS1A_TOGSS_SNK_CC1:
> +		case STATUS1A_TOGSS_SNK_CC2:
> +			if (data->preferred_role == TYPEC_SINK) {
> +				/* Jump directly to UNATTACHED_SNK */
> +				fusb302_set_state(data, UNATTACHED_SNK);
> +			} else {
> +				/* Try to become src */
> +				fusb302_set_state(data, DISABLED_SNK);
> +				mod_delayed_work(system_wq, &data->timeout,
> +						 TDRPTRY);
> +			}
> +			break;
> +		case STATUS1A_TOGSS_SRC_CC1:
> +		case STATUS1A_TOGSS_SRC_CC2:
> +			if (data->preferred_role == TYPEC_SOURCE) {
> +				/* Jump directly to UNATTACHED_SRC */
> +				fusb302_set_state(data, UNATTACHED_SRC);
> +			} else {
> +				/*
> +				 * The USB-C spec says we must enable pull-downs
> +				 * and then wait tDRPTry before checking to
> +				 * avoid endless role-bouncing if both ends
> +				 * prefer the snk role.
> +				 */
> +				fusb302_write_reg(data, REG_SWITCHES0,
> +						  SWITCHES0_PDWN);
> +				fusb302_update_bits(data, REG_CONTROL2,
> +						    CONTROL2_TOGGLE, 0);
> +				msleep(TDRPTRY_MS);
> +				/*
> +				 * Use the toggle engine to do the src
> +				 * detection to keep things the same as for
> +				 * directly entering the src role.
> +				 */
> +				fusb302_set_state(data, DISABLED_SNK);
> +				mod_delayed_work(system_wq, &data->timeout,
> +						 TTOGCYCLE);
> +			}
> +			break;
> +		}
> +		break;
> +	}
> +}
> +
> +static void fusb302_handle_unattached_snk_event(struct fusb302_data *data,
> +					      int event)
> +{
> +	switch (event) {
> +	case VBUS_VALID: /* Cable attached */
> +		fusb302_set_state(data, ATTACHED_SNK);
> +		break;
> +	case TIMEOUT:
> +		fusb302_set_state_disabled(data);
> +		break;
> +	}
> +}
> +
> +static void fusb302_handle_attached_snk_event(struct fusb302_data *data,
> +					      int event)
> +{
> +	switch (event) {
> +	case BC_LVL_CHANGE:
> +	case COMP_LOW:
> +	case COMP_HIGH:
> +		mod_delayed_work(system_wq, &data->bc_work, TPDDEBOUNCE);
> +		break;
> +	case VBUS_INVALID: /* Cable detached */
> +		fusb302_set_state_disabled(data);
> +		break;
> +	}
> +}
> +
> +static void fusb302_handle_unattached_src_event(struct fusb302_data *data,
> +					      int event)
> +{
> +	switch (event) {
> +	case COMP_LOW: /* Cable attached */
> +		fusb302_set_state(data, ATTACHED_SRC);
> +		break;
> +	case TIMEOUT:
> +		fusb302_set_state_disabled(data);
> +		break;
> +	}
> +}
> +
> +static void fusb302_handle_attached_src_event(struct fusb302_data *data,
> +					      int event)
> +{
> +	switch (event) {
> +	case COMP_HIGH: /* Cable detached */
> +		fusb302_set_state_disabled(data);
> +		break;
> +	}
> +}
> +
> +static void fusb302_handle_event(struct fusb302_data *data, int event)
> +{
> +
> +	mutex_lock(&data->lock);
> +
> +	dev_dbg(data->dev, "Handling state %s event %s status %02x %02x\n",
> +		fusb302_state_str[data->state], fusb302_event_str[event],
> +		fusb302_read_reg(data, REG_STATUS0),
> +		fusb302_read_reg(data, REG_STATUS1A));
> +
> +	switch (data->state) {
> +	case DISABLED_SNK:
> +		fusb302_handle_disabled_snk_event(data, event);
> +		break;
> +	case DISABLED_SRC:
> +		fusb302_handle_disabled_src_event(data, event);
> +		break;
> +	case DISABLED_DRP:
> +		fusb302_handle_disabled_drp_event(data, event);
> +		break;
> +	case UNATTACHED_SNK:
> +		fusb302_handle_unattached_snk_event(data, event);
> +		break;
> +	case ATTACHED_SNK:
> +		fusb302_handle_attached_snk_event(data, event);
> +		break;
> +	case UNATTACHED_SRC:
> +		fusb302_handle_unattached_src_event(data, event);
> +		break;
> +	case ATTACHED_SRC:
> +		fusb302_handle_attached_src_event(data, event);
> +		break;
> +	}
> +	mutex_unlock(&data->lock);
> +}
> +
> +static void fusb302_timeout(struct work_struct *work)
> +{
> +	struct fusb302_data *data =
> +		container_of(work, struct fusb302_data, timeout.work);
> +
> +	fusb302_handle_event(data, TIMEOUT);
> +}
> +
> +static void fusb302_fallback_timeout(struct work_struct *work)
> +{
> +	struct fusb302_data *data =
> +		container_of(work, struct fusb302_data, fallback_timeout.work);
> +
> +	fusb302_handle_event(data, FALLBACK_TIMEOUT);
> +}
> +
> +static void fusb302_report_bc_level(struct work_struct *work)
> +{
> +	struct fusb302_data *data =
> +		container_of(work, struct fusb302_data, bc_work.work);
> +
> +	/* Clear old charger cable id */
> +	fusb302_set_extcon_state(data, data->snk_cable_id, false);
> +
> +	if (data->status0 & STATUS0_COMP) {
> +		dev_warn(data->dev, "vRd over maximum, assuming 500mA source\n");
> +		data->snk_cable_id = EXTCON_CHG_USB_SDP;
> +	} else {
> +		switch (data->status0 & STATUS0_BC_LEVEL_MASK) {
> +		case STATUS0_BC_LEVEL_VRA:
> +		case STATUS0_BC_LEVEL_VRDUSB:
> +			data->snk_cable_id = EXTCON_CHG_USB_SDP;
> +			break;
> +		case STATUS0_BC_LEVEL_VRD15:
> +			data->snk_cable_id = EXTCON_CHG_USB_CDP;
> +			break;
> +		case STATUS0_BC_LEVEL_VRD30:
> +			/* Use CHG_USB_FAST to indicate 3A capability */
> +			data->snk_cable_id = EXTCON_CHG_USB_FAST;
> +			break;
> +		}
> +	}
> +	fusb302_set_extcon_state(data, data->snk_cable_id, true);
> +}
> +
> +static irqreturn_t fusb302_irq_handler_thread(int irq, void *handler_data)
> +{
> +	struct fusb302_data *data = handler_data;
> +	int interrupt, interrupta;
> +
> +	interrupt = fusb302_read_reg(data, REG_INTERRUPT);
> +	interrupta = fusb302_read_reg(data, REG_INTERRUPTA);
> +
> +	if (interrupt)
> +		data->status0 = fusb302_read_reg(data, REG_STATUS0);
> +
> +	if (interrupta)
> +		data->status1a = fusb302_read_reg(data, REG_STATUS1A);
> +
> +	dev_dbg(data->dev, "Handling interrupt %02x %02x status %02x %02x\n",
> +		interrupt, interrupta, data->status0, data->status1a);
> +
> +	if (interrupt & INTERRUPT_BC_LVL)
> +		fusb302_handle_event(data, BC_LVL_CHANGE);
> +
> +	if (interrupt & INTERRUPT_COMP_CHNG) {
> +		if (data->status0 & STATUS0_COMP)
> +			fusb302_handle_event(data, COMP_HIGH);
> +		else
> +			fusb302_handle_event(data, COMP_LOW);
> +	}
> +
> +	if (interrupt & INTERRUPT_VBUSOK) {
> +		if (data->status0 & STATUS0_VBUSOK)
> +			fusb302_handle_event(data, VBUS_VALID);
> +		else
> +			fusb302_handle_event(data, VBUS_INVALID);
> +	}
> +
> +	if (interrupta & INTERRUPTA_TOGDONE)
> +		fusb302_handle_event(data, TOGGLE_DONE);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct regmap_config fusb302_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.val_format_endian = REGMAP_ENDIAN_NATIVE,
> +};
> +
> +static int fusb302_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct fusb302_data *data;
> +	int ret;
> +
> +	if (!client->irq) {
> +		dev_err(dev, "Error irq not specified\n");
> +		return -EINVAL;
> +	}
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->dev = dev;
> +	/* TODO make these 2 configurable using device-properties */
> +	data->port_type = TYPEC_PORT_DRP;
> +	data->preferred_role = TYPEC_SINK;
> +
> +	data->regmap = devm_regmap_init_i2c(client, &fusb302_regmap_config);
> +	if (IS_ERR(data->regmap)) {
> +		ret = PTR_ERR(data->regmap);
> +		dev_err(dev, "Error to initializing regmap: %d\n", ret);
> +		return ret;
> +	}
> +
> +	mutex_init(&data->lock);
> +	INIT_DELAYED_WORK(&data->timeout, fusb302_timeout);
> +	INIT_DELAYED_WORK(&data->fallback_timeout, fusb302_fallback_timeout);
> +	INIT_DELAYED_WORK(&data->bc_work, fusb302_report_bc_level);
> +
> +	data->edev = devm_extcon_dev_allocate(dev, fusb302_extcon_cables);
> +	if (IS_ERR(data->edev))
> +		return PTR_ERR(data->edev);
> +
> +	data->edev->name = "fusb302";
> +
> +	ret = devm_extcon_dev_register(dev, data->edev);
> +	if (ret)
> +		return ret;
> +
> +	extcon_set_property_capability(data->edev, EXTCON_USB,
> +				       EXTCON_PROP_USB_TYPEC_POLARITY);
> +
> +	fusb302_write_reg(data, REG_RESET, RESET_SW_RESET);
> +	usleep_range(10000, 20000);
> +
> +	/* Enable power-saving */
> +	fusb302_update_bits(data, REG_CONTROL2, CONTROL2_SAVE_PWR_MASK,
> +			    CONTROL2_SAVE_PWR_40MS);
> +
> +	fusb302_set_state_disabled(data);
> +
> +	ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +			fusb302_irq_handler_thread,
> +			IRQF_TRIGGER_LOW | IRQF_ONESHOT, "fusb302", data);
> +	if (ret) {
> +		dev_err(dev, "Error requesting irq: %d\n", ret);
> +		return ret;
> +	}
> +
> +	fusb302_update_bits(data, REG_CONTROL0, CONTROL0_INT_MASK, 0);
> +
> +	i2c_set_clientdata(client, data);
> +	return 0;
> +}
> +
> +static int fusb302_remove(struct i2c_client *client)
> +{
> +	struct fusb302_data *data = i2c_get_clientdata(client);
> +
> +	devm_free_irq(data->dev, client->irq, data);
> +	cancel_delayed_work_sync(&data->timeout);
> +	cancel_delayed_work_sync(&data->fallback_timeout);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id fusb302_i2c_ids[] = {
> +	{ "fusb302" },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, fusb302_i2c_ids);
> +
> +static struct i2c_driver fusb302_driver = {
> +	.probe_new = fusb302_probe,
> +	.remove = fusb302_remove,
> +	.id_table = fusb302_i2c_ids,
> +	.driver = {
> +		.name = "fusb302",
> +	},
> +};
> +module_i2c_driver(fusb302_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
> +MODULE_DESCRIPTION("FUSB302 USB TYPE-C controller Driver");
> 

Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support

[Andy Shevchenko]

On Tue, May 23, 2017 at 12:27 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> Hi Hans,
>
> I'm sorry for late reply.
> I'll check this thread and then give you my opinion.

Just don't forget to Cc your mails to USB Type-C people (Heikki et al).

-- 
With Best Regards,
Andy Shevchenko

[PATCH 3/5] extcon: intel-cht-wc: Default to SDP on regmap io errors

[Hans de Goede]

In the theoretical case of regmap_read failing get_charger would return
an error and we would continue with EXTCON_NONE.

Since we've already seen Vbus at this point, we should really set
some other cable value to avoid other drivers enabling the 5V boost
converter on the OTG port, even though there is an external Vbus.

This commit modifies cht_wc_extcon_get_charger() to always return a
valid cable, falling back to EXTCON_CHG_USB_SDP in case of regmap_read
errors. This also removes the need for error-checking from callers of
cht_wc_extcon_get_charger().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-intel-cht-wc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 91a0023..8806427e 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -125,7 +125,7 @@ static int cht_wc_extcon_get_charger(struct cht_wc_extcon_data *ext,
 		ret = regmap_read(ext->regmap, CHT_WC_USBSRC, &usbsrc);
 		if (ret) {
 			dev_err(ext->dev, "Error reading usbsrc: %d\n", ret);
-			return ret;
+			return EXTCON_CHG_USB_SDP; /* Save fallback */
 		}
 
 		status = usbsrc & CHT_WC_USBSRC_STS_MASK;
@@ -230,9 +230,7 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
 		goto set_state;
 	}
 
-	ret = cht_wc_extcon_get_charger(ext, ignore_get_charger_errors);
-	if (ret >= 0)
-		cable = ret;
+	cable = cht_wc_extcon_get_charger(ext, ignore_get_charger_errors);
 
 charger_det_done:
 	/* Route D+ and D- to SoC for the host or gadget controller */
-- 
2.9.3

[PATCH 4/5] extcon: intel-cht-wc: Add new cht_wc_extcon_set_state helper

[Hans de Goede]

This allows a nice cleanup of cht_wc_extcon_pwrsrc_event, getting rid
of all the gotos in there.

This also is a preparation patch for adding USB Type-C controller
monitoring.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-intel-cht-wc.c | 54 ++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 8806427e..684f6b2 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -196,20 +196,40 @@ static void cht_wc_extcon_set_5v_boost(struct cht_wc_extcon_data *ext,
 }
 
 /* Small helper to sync EXTCON_CHG_USB_SDP and EXTCON_USB state */
-static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext,
-				    unsigned int cable, bool state)
+static void cht_wc_extcon_set_cable_state(struct cht_wc_extcon_data *ext,
+					  unsigned int cable, bool state)
 {
 	extcon_set_state_sync(ext->edev, cable, state);
 	if (cable == EXTCON_CHG_USB_SDP)
 		extcon_set_state_sync(ext->edev, EXTCON_USB, state);
 }
 
+static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext,
+				    unsigned int new_cable)
+{
+	if (new_cable == EXTCON_NONE && !ext->usb_host) {
+		/* Route D+ and D- to PMIC for future charger detection */
+		cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC);
+	} else {
+		/* Route D+ and D- to SoC for the host or gadget controller */
+		cht_wc_extcon_set_phymux(ext, MUX_SEL_SOC);
+	}
+
+	if (new_cable != ext->previous_cable) {
+		cht_wc_extcon_set_cable_state(ext, new_cable, true);
+		cht_wc_extcon_set_cable_state(ext, ext->previous_cable, false);
+		ext->previous_cable = new_cable;
+	}
+
+	extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
+}
+
 static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
 {
 	int ret, pwrsrc_sts, id;
 	unsigned int cable = EXTCON_NONE;
 	/* Ignore errors in host mode, as the 5v boost converter is on then */
-	bool ignore_get_charger_errors = ext->usb_host;
+	bool ignore_get_charger_err = ext->usb_host;
 
 	ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts);
 	if (ret) {
@@ -218,33 +238,13 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
 	}
 
 	id = cht_wc_extcon_get_id(ext, pwrsrc_sts);
-	if (id == USB_ID_GND) {
-		/* The 5v boost causes a false VBUS / SDP detect, skip */
-		goto charger_det_done;
-	}
-
-	/* Plugged into a host/charger or not connected? */
-	if (!(pwrsrc_sts & CHT_WC_PWRSRC_VBUS)) {
-		/* Route D+ and D- to PMIC for future charger detection */
-		cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC);
-		goto set_state;
-	}
-
-	cable = cht_wc_extcon_get_charger(ext, ignore_get_charger_errors);
 
-charger_det_done:
-	/* Route D+ and D- to SoC for the host or gadget controller */
-	cht_wc_extcon_set_phymux(ext, MUX_SEL_SOC);
-
-set_state:
-	if (cable != ext->previous_cable) {
-		cht_wc_extcon_set_state(ext, cable, true);
-		cht_wc_extcon_set_state(ext, ext->previous_cable, false);
-		ext->previous_cable = cable;
-	}
+	/* When id == gnd the 5v boost causes a false VBUS detect */
+	if (id != USB_ID_GND && (pwrsrc_sts & CHT_WC_PWRSRC_VBUS))
+		cable = cht_wc_extcon_get_charger(ext, ignore_get_charger_err);
 
 	ext->usb_host = ((id == USB_ID_GND) || (id == USB_RID_A));
-	extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
+	cht_wc_extcon_set_state(ext, cable);
 }
 
 static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
-- 
2.9.3

Re: [PATCH 4/5] extcon: intel-cht-wc: Add new cht_wc_extcon_set_state helper

[Andy Shevchenko]

+Cc: Heikki.

It looks like you better Cc him with entire series.

On Fri, Apr 21, 2017 at 4:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> This allows a nice cleanup of cht_wc_extcon_pwrsrc_event, getting rid
> of all the gotos in there.
>
> This also is a preparation patch for adding USB Type-C controller
> monitoring.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-intel-cht-wc.c | 54 ++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
> index 8806427e..684f6b2 100644
> --- a/drivers/extcon/extcon-intel-cht-wc.c
> +++ b/drivers/extcon/extcon-intel-cht-wc.c
> @@ -196,20 +196,40 @@ static void cht_wc_extcon_set_5v_boost(struct cht_wc_extcon_data *ext,
>  }
>
>  /* Small helper to sync EXTCON_CHG_USB_SDP and EXTCON_USB state */
> -static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext,
> -                                   unsigned int cable, bool state)
> +static void cht_wc_extcon_set_cable_state(struct cht_wc_extcon_data *ext,
> +                                         unsigned int cable, bool state)
>  {
>         extcon_set_state_sync(ext->edev, cable, state);
>         if (cable == EXTCON_CHG_USB_SDP)
>                 extcon_set_state_sync(ext->edev, EXTCON_USB, state);
>  }
>
> +static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext,
> +                                   unsigned int new_cable)
> +{
> +       if (new_cable == EXTCON_NONE && !ext->usb_host) {
> +               /* Route D+ and D- to PMIC for future charger detection */
> +               cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC);
> +       } else {
> +               /* Route D+ and D- to SoC for the host or gadget controller */
> +               cht_wc_extcon_set_phymux(ext, MUX_SEL_SOC);
> +       }
> +
> +       if (new_cable != ext->previous_cable) {
> +               cht_wc_extcon_set_cable_state(ext, new_cable, true);
> +               cht_wc_extcon_set_cable_state(ext, ext->previous_cable, false);
> +               ext->previous_cable = new_cable;
> +       }
> +
> +       extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
> +}
> +
>  static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
>  {
>         int ret, pwrsrc_sts, id;
>         unsigned int cable = EXTCON_NONE;
>         /* Ignore errors in host mode, as the 5v boost converter is on then */
> -       bool ignore_get_charger_errors = ext->usb_host;
> +       bool ignore_get_charger_err = ext->usb_host;
>
>         ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts);
>         if (ret) {
> @@ -218,33 +238,13 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
>         }
>
>         id = cht_wc_extcon_get_id(ext, pwrsrc_sts);
> -       if (id == USB_ID_GND) {
> -               /* The 5v boost causes a false VBUS / SDP detect, skip */
> -               goto charger_det_done;
> -       }
> -
> -       /* Plugged into a host/charger or not connected? */
> -       if (!(pwrsrc_sts & CHT_WC_PWRSRC_VBUS)) {
> -               /* Route D+ and D- to PMIC for future charger detection */
> -               cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC);
> -               goto set_state;
> -       }
> -
> -       cable = cht_wc_extcon_get_charger(ext, ignore_get_charger_errors);
>
> -charger_det_done:
> -       /* Route D+ and D- to SoC for the host or gadget controller */
> -       cht_wc_extcon_set_phymux(ext, MUX_SEL_SOC);
> -
> -set_state:
> -       if (cable != ext->previous_cable) {
> -               cht_wc_extcon_set_state(ext, cable, true);
> -               cht_wc_extcon_set_state(ext, ext->previous_cable, false);
> -               ext->previous_cable = cable;
> -       }
> +       /* When id == gnd the 5v boost causes a false VBUS detect */
> +       if (id != USB_ID_GND && (pwrsrc_sts & CHT_WC_PWRSRC_VBUS))
> +               cable = cht_wc_extcon_get_charger(ext, ignore_get_charger_err);
>
>         ext->usb_host = ((id == USB_ID_GND) || (id == USB_RID_A));
> -       extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
> +       cht_wc_extcon_set_state(ext, cable);
>  }
>
>  static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
> --
> 2.9.3
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring external USB Type-C controller

[Hans de Goede]

On some boards the Whiskey Cove PMIC is combined with an external USB
Type-C controller, in this case extcon consumers should use the Type-C
extcon state, except when the USB Type-C controller detects a current
limit of 500 mA which may indicate USB-C to USB-A cable at which point
the extcon consumer should use the Whiskey Cove's BC-1.2 detection's
state.

Since the Type-C controller info is incomplete and needs to be
supplemented with the BC1.2 detection result in some cases, this
commit makes the intel-cht-wc extcon driver monitor the extcon device
registered by the Type-C controller and report that as our extcon state
except when BC-1.2 detection should be used. This allows extcon
consumers on these boards to keep monitoring only the intel-cht-wc extcon
and then get complete extcon info from that, which combines the Type-C
and BC-1.2 info.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/extcon/extcon-intel-cht-wc.c | 96 ++++++++++++++++++++++++++++++------
 1 file changed, 80 insertions(+), 16 deletions(-)

diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 684f6b2..e510188 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -15,6 +15,7 @@
  * more details.
  */
 
+#include <linux/acpi.h>
 #include <linux/extcon.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -88,6 +89,7 @@ static const unsigned int cht_wc_extcon_cables[] = {
 	EXTCON_CHG_USB_CDP,
 	EXTCON_CHG_USB_DCP,
 	EXTCON_CHG_USB_ACA,
+	EXTCON_CHG_USB_FAST,
 	EXTCON_NONE,
 };
 
@@ -95,6 +97,9 @@ struct cht_wc_extcon_data {
 	struct device *dev;
 	struct regmap *regmap;
 	struct extcon_dev *edev;
+	struct extcon_dev *usbc;
+	struct notifier_block usbc_nb;
+	struct work_struct work;
 	unsigned int previous_cable;
 	bool usb_host;
 };
@@ -224,8 +229,32 @@ static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext,
 	extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
 }
 
-static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
+static void cht_wc_extcon_usb_c_event(struct work_struct *work)
 {
+	struct cht_wc_extcon_data *ext =
+		container_of(work, struct cht_wc_extcon_data, work);
+	unsigned int cable = EXTCON_NONE;
+
+	if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_SDP) > 0) {
+		/*
+		 * USB-C to USB-A cable ? Try BC1.2 charger detection from
+		 * WC PMIC, ignore charger detection errors.
+		 */
+		cable = cht_wc_extcon_get_charger(ext, true);
+	} else if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_CDP) > 0) {
+		cable = EXTCON_CHG_USB_CDP;
+	} else if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_FAST) > 0) {
+		cable = EXTCON_CHG_USB_FAST;
+	}
+
+	ext->usb_host = (extcon_get_state(ext->usbc, EXTCON_USB_HOST) > 0);
+	cht_wc_extcon_set_state(ext, cable);
+}
+
+static void cht_wc_extcon_pwrsrc_event(struct work_struct *work)
+{
+	struct cht_wc_extcon_data *ext =
+		container_of(work, struct cht_wc_extcon_data, work);
 	int ret, pwrsrc_sts, id;
 	unsigned int cable = EXTCON_NONE;
 	/* Ignore errors in host mode, as the 5v boost converter is on then */
@@ -258,7 +287,7 @@ static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
 		return IRQ_NONE;
 	}
 
-	cht_wc_extcon_pwrsrc_event(ext);
+	schedule_work(&ext->work);
 
 	ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ, irqs);
 	if (ret) {
@@ -269,6 +298,17 @@ static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static int cht_wc_extcon_usbc_evt(struct notifier_block *nb,
+				  unsigned long event, void *param)
+{
+	struct cht_wc_extcon_data *ext =
+		container_of(nb, struct cht_wc_extcon_data, usbc_nb);
+
+	schedule_work(&ext->work);
+
+	return NOTIFY_OK;
+}
+
 static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
 {
 	int ret, mask, val;
@@ -300,6 +340,15 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
 	ext->regmap = pmic->regmap;
 	ext->previous_cable = EXTCON_NONE;
 
+	if (acpi_dev_present("INT33FE", NULL, -1)) {
+		ext->usbc = extcon_get_extcon_dev("fusb302");
+		if (!ext->usbc)
+			return -EPROBE_DEFER;
+
+		dev_info(&pdev->dev,
+			 "Using FUSB302 extcon for USB Type-C cable info\n");
+	}
+
 	/* Initialize extcon device */
 	ext->edev = devm_extcon_dev_allocate(ext->dev, cht_wc_extcon_cables);
 	if (IS_ERR(ext->edev))
@@ -335,25 +384,40 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
 	/* Route D+ and D- to PMIC for initial charger detection */
 	cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC);
 
-	/* Get initial state */
-	cht_wc_extcon_pwrsrc_event(ext);
-
-	ret = devm_request_threaded_irq(ext->dev, irq, NULL, cht_wc_extcon_isr,
-					IRQF_ONESHOT, pdev->name, ext);
-	if (ret) {
-		dev_err(ext->dev, "Error requesting interrupt: %d\n", ret);
-		goto disable_sw_control;
-	}
+	if (ext->usbc) {
+		INIT_WORK(&ext->work, cht_wc_extcon_usb_c_event);
+		ext->usbc_nb.notifier_call = cht_wc_extcon_usbc_evt;
+		ret = devm_extcon_register_notifier_all(&pdev->dev, ext->usbc,
+							&ext->usbc_nb);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"Error registering usbc extcon notifier: %d\n",
+				ret);
+			goto disable_sw_control;
+		}
+	} else {
+		INIT_WORK(&ext->work, cht_wc_extcon_pwrsrc_event);
+		ret = devm_request_threaded_irq(ext->dev, irq, NULL,
+						cht_wc_extcon_isr, IRQF_ONESHOT,
+						pdev->name, ext);
+		if (ret) {
+			dev_err(ext->dev, "Error requesting irq: %d\n", ret);
+			goto disable_sw_control;
+		}
 
-	/* Unmask irqs */
-	ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ_MASK,
+		/* Unmask irqs */
+		ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ_MASK,
 			   (int)~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_ID_GND |
 				  CHT_WC_PWRSRC_ID_FLOAT));
-	if (ret) {
-		dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
-		goto disable_sw_control;
+		if (ret) {
+			dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
+			goto disable_sw_control;
+		}
 	}
 
+	/* Get initial state */
+	schedule_work(&ext->work);
+
 	platform_set_drvdata(pdev, ext);
 
 	return 0;
-- 
2.9.3

Re: [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring external USB Type-C controller

[Andy Shevchenko]

+Cc: Heikki,

On Fri, Apr 21, 2017 at 4:01 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> On some boards the Whiskey Cove PMIC is combined with an external USB
> Type-C controller, in this case extcon consumers should use the Type-C
> extcon state, except when the USB Type-C controller detects a current
> limit of 500 mA which may indicate USB-C to USB-A cable at which point
> the extcon consumer should use the Whiskey Cove's BC-1.2 detection's
> state.
>
> Since the Type-C controller info is incomplete and needs to be
> supplemented with the BC1.2 detection result in some cases, this
> commit makes the intel-cht-wc extcon driver monitor the extcon device
> registered by the Type-C controller and report that as our extcon state
> except when BC-1.2 detection should be used. This allows extcon
> consumers on these boards to keep monitoring only the intel-cht-wc extcon
> and then get complete extcon info from that, which combines the Type-C
> and BC-1.2 info.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-intel-cht-wc.c | 96 ++++++++++++++++++++++++++++++------
>  1 file changed, 80 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
> index 684f6b2..e510188 100644
> --- a/drivers/extcon/extcon-intel-cht-wc.c
> +++ b/drivers/extcon/extcon-intel-cht-wc.c
> @@ -15,6 +15,7 @@
>   * more details.
>   */
>
> +#include <linux/acpi.h>
>  #include <linux/extcon.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> @@ -88,6 +89,7 @@ static const unsigned int cht_wc_extcon_cables[] = {
>         EXTCON_CHG_USB_CDP,
>         EXTCON_CHG_USB_DCP,
>         EXTCON_CHG_USB_ACA,
> +       EXTCON_CHG_USB_FAST,
>         EXTCON_NONE,
>  };
>
> @@ -95,6 +97,9 @@ struct cht_wc_extcon_data {
>         struct device *dev;
>         struct regmap *regmap;
>         struct extcon_dev *edev;
> +       struct extcon_dev *usbc;
> +       struct notifier_block usbc_nb;
> +       struct work_struct work;
>         unsigned int previous_cable;
>         bool usb_host;
>  };
> @@ -224,8 +229,32 @@ static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext,
>         extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
>  }
>
> -static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
> +static void cht_wc_extcon_usb_c_event(struct work_struct *work)
>  {
> +       struct cht_wc_extcon_data *ext =
> +               container_of(work, struct cht_wc_extcon_data, work);
> +       unsigned int cable = EXTCON_NONE;
> +
> +       if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_SDP) > 0) {
> +               /*
> +                * USB-C to USB-A cable ? Try BC1.2 charger detection from
> +                * WC PMIC, ignore charger detection errors.
> +                */
> +               cable = cht_wc_extcon_get_charger(ext, true);
> +       } else if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_CDP) > 0) {
> +               cable = EXTCON_CHG_USB_CDP;
> +       } else if (extcon_get_state(ext->usbc, EXTCON_CHG_USB_FAST) > 0) {
> +               cable = EXTCON_CHG_USB_FAST;
> +       }
> +
> +       ext->usb_host = (extcon_get_state(ext->usbc, EXTCON_USB_HOST) > 0);
> +       cht_wc_extcon_set_state(ext, cable);
> +}
> +
> +static void cht_wc_extcon_pwrsrc_event(struct work_struct *work)
> +{
> +       struct cht_wc_extcon_data *ext =
> +               container_of(work, struct cht_wc_extcon_data, work);
>         int ret, pwrsrc_sts, id;
>         unsigned int cable = EXTCON_NONE;
>         /* Ignore errors in host mode, as the 5v boost converter is on then */
> @@ -258,7 +287,7 @@ static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
>                 return IRQ_NONE;
>         }
>
> -       cht_wc_extcon_pwrsrc_event(ext);
> +       schedule_work(&ext->work);
>
>         ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ, irqs);
>         if (ret) {
> @@ -269,6 +298,17 @@ static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
>         return IRQ_HANDLED;
>  }
>
> +static int cht_wc_extcon_usbc_evt(struct notifier_block *nb,
> +                                 unsigned long event, void *param)
> +{
> +       struct cht_wc_extcon_data *ext =
> +               container_of(nb, struct cht_wc_extcon_data, usbc_nb);
> +
> +       schedule_work(&ext->work);
> +
> +       return NOTIFY_OK;
> +}
> +
>  static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
>  {
>         int ret, mask, val;
> @@ -300,6 +340,15 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>         ext->regmap = pmic->regmap;
>         ext->previous_cable = EXTCON_NONE;
>
> +       if (acpi_dev_present("INT33FE", NULL, -1)) {
> +               ext->usbc = extcon_get_extcon_dev("fusb302");
> +               if (!ext->usbc)
> +                       return -EPROBE_DEFER;
> +
> +               dev_info(&pdev->dev,
> +                        "Using FUSB302 extcon for USB Type-C cable info\n");
> +       }
> +
>         /* Initialize extcon device */
>         ext->edev = devm_extcon_dev_allocate(ext->dev, cht_wc_extcon_cables);
>         if (IS_ERR(ext->edev))
> @@ -335,25 +384,40 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
>         /* Route D+ and D- to PMIC for initial charger detection */
>         cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC);
>
> -       /* Get initial state */
> -       cht_wc_extcon_pwrsrc_event(ext);
> -
> -       ret = devm_request_threaded_irq(ext->dev, irq, NULL, cht_wc_extcon_isr,
> -                                       IRQF_ONESHOT, pdev->name, ext);
> -       if (ret) {
> -               dev_err(ext->dev, "Error requesting interrupt: %d\n", ret);
> -               goto disable_sw_control;
> -       }
> +       if (ext->usbc) {
> +               INIT_WORK(&ext->work, cht_wc_extcon_usb_c_event);
> +               ext->usbc_nb.notifier_call = cht_wc_extcon_usbc_evt;
> +               ret = devm_extcon_register_notifier_all(&pdev->dev, ext->usbc,
> +                                                       &ext->usbc_nb);
> +               if (ret) {
> +                       dev_err(&pdev->dev,
> +                               "Error registering usbc extcon notifier: %d\n",
> +                               ret);
> +                       goto disable_sw_control;
> +               }
> +       } else {
> +               INIT_WORK(&ext->work, cht_wc_extcon_pwrsrc_event);
> +               ret = devm_request_threaded_irq(ext->dev, irq, NULL,
> +                                               cht_wc_extcon_isr, IRQF_ONESHOT,
> +                                               pdev->name, ext);
> +               if (ret) {
> +                       dev_err(ext->dev, "Error requesting irq: %d\n", ret);
> +                       goto disable_sw_control;
> +               }
>
> -       /* Unmask irqs */
> -       ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ_MASK,
> +               /* Unmask irqs */
> +               ret = regmap_write(ext->regmap, CHT_WC_PWRSRC_IRQ_MASK,
>                            (int)~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_ID_GND |
>                                   CHT_WC_PWRSRC_ID_FLOAT));
> -       if (ret) {
> -               dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
> -               goto disable_sw_control;
> +               if (ret) {
> +                       dev_err(ext->dev, "Error writing irq-mask: %d\n", ret);
> +                       goto disable_sw_control;
> +               }
>         }
>
> +       /* Get initial state */
> +       schedule_work(&ext->work);
> +
>         platform_set_drvdata(pdev, ext);
>
>         return 0;
> --
> 2.9.3
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring external USB Type-C controller

[Heikki Krogerus]

Hi Hans,

On Fri, Apr 21, 2017 at 03:01:19PM +0200, Hans de Goede wrote:
> On some boards the Whiskey Cove PMIC is combined with an external USB
> Type-C controller, in this case extcon consumers should use the Type-C
> extcon state, except when the USB Type-C controller detects a current
> limit of 500 mA which may indicate USB-C to USB-A cable at which point
> the extcon consumer should use the Whiskey Cove's BC-1.2 detection's
> state.

Doesn't this mean you have several extcon_dev registered for a
single port? I'm not completely sure how extcon framework is meant to
be used, but shouldn't a single extcon_dev represent all the types of
connectors a port is meant to support?

> Since the Type-C controller info is incomplete and needs to be
> supplemented with the BC1.2 detection result in some cases, this
> commit makes the intel-cht-wc extcon driver monitor the extcon device
> registered by the Type-C controller and report that as our extcon state
> except when BC-1.2 detection should be used. This allows extcon
> consumers on these boards to keep monitoring only the intel-cht-wc extcon
> and then get complete extcon info from that, which combines the Type-C
> and BC-1.2 info.

I don't really understand why does this need to be done in such a
complex manner? Both components should just report the result and be
done with it, and there is no need for any coupling between the two.
If there is a consumer for the power, the driver for it can decide on
its own the limits and maximum power from the two sources.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon-intel-cht-wc.c | 96 ++++++++++++++++++++++++++++++------

I did not realize we had this driver. It really should register a psy.
We need to be able to advertise the results of the detection to the
consumers in order to support for example boards that have a discrete
charger ic or battery attached to the SoC instead of PMIC, and without
being forced to use board/platform specific quirks like this in the
driver.

This driver should just report the result of the detection forward and
let the psy framework take care of notifying the other components,
consumers and whatnot, if there are any.


Thanks,

-- 
heikki

Re: [PATCH 1/5] extcon: Allow extcon drivers to specify the extcon name

[Chanwoo Choi]

Hi Hans,

I'm sorry for late reply.

On 2017년 04월 21일 22:01, Hans de Goede wrote:
> The parent device name is not necessarily always useful, e.g.
> with i2c devices it may simply be e.g.: "0-0022" and it also depends
> on the i2c-bus number which depends on probe ordering.
> 
> This commit allows drivers to set their own, more useful name,
> avoiding the problems with some i2c-device names.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/extcon/extcon.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index f422a78..f8d3c1b 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -1117,7 +1117,8 @@ int extcon_dev_register(struct extcon_dev *edev)
>  	edev->dev.class = extcon_class;
>  	edev->dev.release = extcon_dev_release;
>  
> -	edev->name = dev_name(edev->dev.parent);
> +	if (!edev->name)
> +		edev->name = dev_name(edev->dev.parent);

I think that the variables of 'struct extcon_dev' should be modified by extcon core.
I don't want to touch the variables of 'struct extcon_dev' by extcon provider driver.

>  	if (IS_ERR_OR_NULL(edev->name)) {
>  		dev_err(&edev->dev,
>  			"extcon device name is null\n");
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics