Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1427844 > unrolled thread

[PATCHv3 0/2] USB Type-C Connector class

Back to article view | Back to linux.kernel

Contents

  [PATCHv3 0/2] USB Type-C Connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-21 17:00 +0200
    [PATCHv3 2/2] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-21 17:00 +0200
    Re: [PATCHv3 1/2] usb: USB Type-C connector class Oliver Neukum <oneukum@suse.com> - 2016-06-21 22:30 +0200
      Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-22 12:00 +0200
        Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-22 12:10 +0200
          Re: [PATCHv3 1/2] usb: USB Type-C connector class Oliver Neukum <oneukum@suse.com> - 2016-06-22 14:00 +0200
        Re: [PATCHv3 1/2] usb: USB Type-C connector class Oliver Neukum <oneukum@suse.com> - 2016-06-22 12:20 +0200
          Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-22 13:50 +0200
            Re: [PATCHv3 1/2] usb: USB Type-C connector class Oliver Neukum <oneukum@suse.com> - 2016-06-22 16:00 +0200
              Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-22 16:50 +0200
                Re: [PATCHv3 1/2] usb: USB Type-C connector class Oliver Neukum <oneukum@suse.com> - 2016-06-22 19:20 +0200
                  Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-23 10:30 +0200
                    Re: [PATCHv3 1/2] usb: USB Type-C connector class Oliver Neukum <oneukum@suse.com> - 2016-06-23 10:50 +0200
                      Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-23 14:10 +0200
                        Re: [PATCHv3 1/2] usb: USB Type-C connector class Roger Quadros <rogerq@ti.com> - 2016-06-23 14:30 +0200
                          Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-23 15:20 +0200
                        Re: [PATCHv3 1/2] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-06-23 15:30 +0200
    Re: [PATCHv3 0/2] USB Type-C Connector class Guenter Roeck <linux@roeck-us.net> - 2016-06-22 00:30 +0200
      Re: [PATCHv3 0/2] USB Type-C Connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-22 12:00 +0200
        Re: [PATCHv3 0/2] USB Type-C Connector class Guenter Roeck <linux@roeck-us.net> - 2016-06-22 15:30 +0200
    Re: [PATCHv3 1/2] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-06-23 00:00 +0200
      Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-23 10:30 +0200
    Re: [PATCHv3 1/2] usb: USB Type-C connector class Roger Quadros <rogerq@ti.com> - 2016-06-23 14:00 +0200
      Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-23 15:10 +0200
    Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-27 14:20 +0200
      Re: [PATCHv3 1/2] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-06-27 15:50 +0200
        Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-28 15:20 +0200
          Re: [PATCHv3 1/2] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-06-28 15:30 +0200
      Re: [PATCHv3 1/2] usb: USB Type-C connector class Rajaram R <rajaram.officemail@gmail.com> - 2016-06-29 11:00 +0200
        Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-29 12:40 +0200
          Re: [PATCHv3 1/2] usb: USB Type-C connector class Rajaram R <rajaram.officemail@gmail.com> - 2016-06-29 13:00 +0200
            Re: [PATCHv3 1/2] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-06-29 13:30 +0200
              Re: [PATCHv3 1/2] usb: USB Type-C connector class Oliver Neukum <oneukum@suse.com> - 2016-07-04 11:00 +0200

Page 1 of 2  [1] 2  Next page →

#1427844 — [PATCHv3 0/2] USB Type-C Connector class

Hi,

I'm considering all the RFCs I send after v1 as v2 (I don't remember
how many I send). Hope this is OK and hope there is nothing big
missing anymore (or broken).

Sorry about the delay. I've been really busy with some internal tasks.
I'm guessing we missed v4.8 with this thing. I'm sorry about that.

I'm including in this series a driver for the Broxton PMIC USB Type-C
PHY.


Changes since v2:
- Notification on role and alternate mode changes
- cleanups

Changes since v1:
- Completely rewrote alternate mode support
- Patners, cables and cable plugs presented as devices.


Heikki Krogerus (2):
  usb: USB Type-C connector class
  usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY

 Documentation/ABI/testing/sysfs-class-typec |  163 ++++
 Documentation/usb/typec.txt                 |  101 +++
 MAINTAINERS                                 |    9 +
 drivers/usb/Kconfig                         |    2 +
 drivers/usb/Makefile                        |    2 +
 drivers/usb/typec/Kconfig                   |   21 +
 drivers/usb/typec/Makefile                  |    2 +
 drivers/usb/typec/typec.c                   | 1173 +++++++++++++++++++++++++++
 drivers/usb/typec/typec_wcove.c             |  376 +++++++++
 include/linux/usb/typec.h                   |  255 ++++++
 10 files changed, 2104 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-typec
 create mode 100644 Documentation/usb/typec.txt
 create mode 100644 drivers/usb/typec/Kconfig
 create mode 100644 drivers/usb/typec/Makefile
 create mode 100644 drivers/usb/typec/typec.c
 create mode 100644 drivers/usb/typec/typec_wcove.c
 create mode 100644 include/linux/usb/typec.h

-- 
2.8.1

[toc] | [next] | [standalone]

#1427854 — [PATCHv3 2/2] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY

This adds driver for the USB Type-C PHY on Intel WhiskeyCove
PMIC which is available on some of the Intel Broxton SoC
based platforms.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/Kconfig       |  14 ++
 drivers/usb/typec/Makefile      |   1 +
 drivers/usb/typec/typec_wcove.c | 376 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 391 insertions(+)
 create mode 100644 drivers/usb/typec/typec_wcove.c

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index b229fb9..7a345a4 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -4,4 +4,18 @@ menu "USB PD and Type-C drivers"
 config TYPEC
 	tristate
 
+config TYPEC_WCOVE
+	tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
+	depends on ACPI
+	depends on INTEL_SOC_PMIC
+	depends on INTEL_PMC_IPC
+	select TYPEC
+	help
+	  This driver adds support for USB Type-C detection on Intel Broxton
+	  platforms that have Intel Whiskey Cove PMIC. The driver can detect the
+	  role and cable orientation.
+
+	  To compile this driver as module, choose M here: the module will be
+	  called typec_wcove
+
 endmenu
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 1012a8b..b9cb862 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_TYPEC)		+= typec.o
+obj-$(CONFIG_TYPEC_WCOVE)	+= typec_wcove.o
diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c
new file mode 100644
index 0000000..4309ac6
--- /dev/null
+++ b/drivers/usb/typec/typec_wcove.c
@@ -0,0 +1,376 @@
+/**
+ * typec_wcove.c - WhiskeyCove PMIC USB Type-C PHY driver
+ *
+ * Copyright (C) 2016 Intel Corporation
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.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/acpi.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/usb/typec.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/intel_soc_pmic.h>
+
+/* Register offsets */
+#define WCOVE_CHGRIRQ0		0x4e09
+#define WCOVE_PHYCTRL		0x5e07
+
+#define USBC_CONTROL1		0x7001
+#define USBC_CONTROL2		0x7002
+#define USBC_CONTROL3		0x7003
+#define USBC_CC1_CTRL		0x7004
+#define USBC_CC2_CTRL		0x7005
+#define USBC_STATUS1		0x7007
+#define USBC_STATUS2		0x7008
+#define USBC_STATUS3		0x7009
+#define USBC_IRQ1		0x7015
+#define USBC_IRQ2		0x7016
+#define USBC_IRQMASK1		0x7017
+#define USBC_IRQMASK2		0x7018
+
+/* Register bits */
+
+#define USBC_CONTROL1_MODE_DRP(r)	((r & ~0x7) | 4)
+
+#define USBC_CONTROL2_UNATT_SNK		BIT(0)
+#define USBC_CONTROL2_UNATT_SRC		BIT(1)
+#define USBC_CONTROL2_DIS_ST		BIT(2)
+
+#define USBC_CONTROL3_PD_DIS		BIT(1)
+
+#define USBC_CC_CTRL_VCONN_EN		BIT(1)
+
+#define USBC_STATUS1_DET_ONGOING	BIT(6)
+#define USBC_STATUS1_RSLT(r)		(r & 0xf)
+#define USBC_RSLT_NOTHING		0
+#define USBC_RSLT_SRC_DEFAULT		1
+#define USBC_RSLT_SRC_1_5A		2
+#define USBC_RSLT_SRC_3_0A		3
+#define USBC_RSLT_SNK			4
+#define USBC_RSLT_DEBUG_ACC		5
+#define USBC_RSLT_AUDIO_ACC		6
+#define USBC_RSLT_UNDEF			15
+#define USBC_STATUS1_ORIENT(r)		((r >> 4) & 0x3)
+#define USBC_ORIENT_NORMAL		1
+#define USBC_ORIENT_REVERSE		2
+
+#define USBC_STATUS2_VBUS_REQ		BIT(5)
+
+#define USBC_IRQ1_ADCDONE1		BIT(2)
+#define USBC_IRQ1_OVERTEMP		BIT(1)
+#define USBC_IRQ1_SHORT			BIT(0)
+
+#define USBC_IRQ2_CC_CHANGE		BIT(7)
+#define USBC_IRQ2_RX_PD			BIT(6)
+#define USBC_IRQ2_RX_HR			BIT(5)
+#define USBC_IRQ2_RX_CR			BIT(4)
+#define USBC_IRQ2_TX_SUCCESS		BIT(3)
+#define USBC_IRQ2_TX_FAIL		BIT(2)
+
+#define USBC_IRQMASK1_ALL	(USBC_IRQ1_ADCDONE1 | USBC_IRQ1_OVERTEMP | \
+				 USBC_IRQ1_SHORT)
+
+#define USBC_IRQMASK2_ALL	(USBC_IRQ2_CC_CHANGE | USBC_IRQ2_RX_PD | \
+				 USBC_IRQ2_RX_HR | USBC_IRQ2_RX_CR | \
+				 USBC_IRQ2_TX_SUCCESS | USBC_IRQ2_TX_FAIL)
+
+struct wcove_typec {
+	struct mutex lock; /* device lock */
+	struct device *dev;
+	struct regmap *regmap;
+	struct typec_port *port;
+	struct completion complete;
+	struct typec_capability cap;
+	struct typec_connection con;
+	struct typec_partner partner;
+};
+
+enum wcove_typec_func {
+	WCOVE_FUNC_DRIVE_VBUS = 1,
+	WCOVE_FUNC_ORIENTATION,
+	WCOVE_FUNC_ROLE,
+	WCOVE_FUNC_DRIVE_VCONN,
+};
+
+enum wcove_typec_orientation {
+	WCOVE_ORIENTATION_NORMAL,
+	WCOVE_ORIENTATION_REVERSE,
+};
+
+enum wcove_typec_role {
+	WCOVE_ROLE_HOST,
+	WCOVE_ROLE_DEVICE,
+};
+
+static uuid_le uuid = UUID_LE(0x482383f0, 0x2876, 0x4e49,
+			      0x86, 0x85, 0xdb, 0x66, 0x21, 0x1a, 0xf0, 0x37);
+
+static int wcove_typec_func(struct wcove_typec *wcove,
+			    enum wcove_typec_func func, int param)
+{
+	union acpi_object *obj;
+	union acpi_object tmp;
+	union acpi_object argv4 = ACPI_INIT_DSM_ARGV4(1, &tmp);
+
+	tmp.type = ACPI_TYPE_INTEGER;
+	tmp.integer.value = param;
+
+	obj = acpi_evaluate_dsm(ACPI_HANDLE(wcove->dev), uuid.b, 1, func,
+				&argv4);
+	if (!obj) {
+		dev_err(wcove->dev, "%s: failed to evaluate _DSM\n", __func__);
+		return -EIO;
+	}
+
+	ACPI_FREE(obj);
+	return 0;
+}
+
+static void wcove_typec_device_mode(struct wcove_typec *wcove)
+{
+	wcove->partner.type = TYPEC_PARTNER_USB;
+	wcove->con.partner = &wcove->partner;
+	wcove->con.pwr_role = TYPEC_SINK;
+	wcove->con.vconn_role = TYPEC_SINK;
+	wcove_typec_func(wcove, WCOVE_FUNC_ROLE, WCOVE_ROLE_DEVICE);
+	typec_connect(wcove->port, &wcove->con);
+}
+
+static irqreturn_t wcove_typec_irq(int irq, void *data)
+{
+	struct wcove_typec *wcove = data;
+	unsigned int cc1_ctrl;
+	unsigned int cc2_ctrl;
+	unsigned int cc_irq1;
+	unsigned int cc_irq2;
+	unsigned int status1;
+	unsigned int status2;
+	int ret;
+
+	mutex_lock(&wcove->lock);
+
+	ret = regmap_read(wcove->regmap, USBC_IRQ1, &cc_irq1);
+	if (ret)
+		goto err;
+
+	ret = regmap_read(wcove->regmap, USBC_IRQ2, &cc_irq2);
+	if (ret)
+		goto err;
+
+	ret = regmap_read(wcove->regmap, USBC_STATUS1, &status1);
+	if (ret)
+		goto err;
+
+	ret = regmap_read(wcove->regmap, USBC_STATUS2, &status2);
+	if (ret)
+		goto err;
+
+	ret = regmap_read(wcove->regmap, USBC_CC1_CTRL, &cc1_ctrl);
+	if (ret)
+		goto err;
+
+	ret = regmap_read(wcove->regmap, USBC_CC2_CTRL, &cc2_ctrl);
+	if (ret)
+		goto err;
+
+	if (cc_irq1) {
+		if (cc_irq1 & USBC_IRQ1_OVERTEMP)
+			dev_err(wcove->dev, "VCONN Switch Over Temperature!\n");
+		if (cc_irq1 & USBC_IRQ1_SHORT)
+			dev_err(wcove->dev, "VCONN Switch Short Circuit!\n");
+		regmap_write(wcove->regmap, USBC_IRQ1, cc_irq1);
+	}
+
+	if (cc_irq2) {
+		regmap_write(wcove->regmap, USBC_IRQ2, cc_irq2);
+		/*
+		 * Ingoring any PD communication interrupts until the PD stack
+		 * is in place
+		 */
+		if (cc_irq2 & ~USBC_IRQ2_CC_CHANGE) {
+			dev_WARN(wcove->dev, "USB PD handling missing\n");
+			goto err;
+		}
+	}
+
+	if (status1 & USBC_STATUS1_DET_ONGOING)
+		goto out;
+
+	if (USBC_STATUS1_RSLT(status1) == USBC_RSLT_NOTHING) {
+		if (wcove->con.partner) {
+			typec_disconnect(wcove->port);
+			memset(&wcove->con, 0, sizeof(wcove->con));
+			memset(&wcove->partner, 0, sizeof(wcove->partner));
+		}
+
+		wcove_typec_func(wcove, WCOVE_FUNC_ORIENTATION,
+				 WCOVE_ORIENTATION_NORMAL);
+		/* Host mode by default */
+		wcove_typec_func(wcove, WCOVE_FUNC_ROLE, WCOVE_ROLE_HOST);
+		goto out;
+	}
+
+	if (wcove->con.partner)
+		goto out;
+
+	switch (USBC_STATUS1_ORIENT(status1)) {
+	case USBC_ORIENT_NORMAL:
+		wcove_typec_func(wcove, WCOVE_FUNC_ORIENTATION,
+				 WCOVE_ORIENTATION_NORMAL);
+		break;
+	case USBC_ORIENT_REVERSE:
+		wcove_typec_func(wcove, WCOVE_FUNC_ORIENTATION,
+				 WCOVE_ORIENTATION_REVERSE);
+	default:
+		break;
+	}
+
+	switch (USBC_STATUS1_RSLT(status1)) {
+	case USBC_RSLT_SRC_DEFAULT:
+		wcove->con.pwr_opmode = TYPEC_PWR_MODE_USB;
+		wcove_typec_device_mode(wcove);
+		break;
+	case USBC_RSLT_SRC_1_5A:
+		wcove->con.pwr_opmode = TYPEC_PWR_MODE_1_5A;
+		wcove_typec_device_mode(wcove);
+		break;
+	case USBC_RSLT_SRC_3_0A:
+		wcove->con.pwr_opmode = TYPEC_PWR_MODE_3_0A;
+		wcove_typec_device_mode(wcove);
+		break;
+	case USBC_RSLT_SNK:
+		wcove->partner.type = TYPEC_PARTNER_USB;
+		wcove->con.partner = &wcove->partner;
+		wcove->con.data_role = TYPEC_HOST;
+		wcove->con.pwr_role = TYPEC_SOURCE;
+		wcove->con.vconn_role = TYPEC_SOURCE;
+		wcove_typec_func(wcove, WCOVE_FUNC_ROLE, WCOVE_ROLE_HOST);
+		typec_connect(wcove->port, &wcove->con);
+		break;
+	case USBC_RSLT_DEBUG_ACC:
+		wcove->partner.accessory = TYPEC_ACCESSORY_DEBUG;
+		wcove->partner.type = TYPEC_PARTNER_ACCESSORY;
+		wcove->con.partner = &wcove->partner;
+		typec_connect(wcove->port, &wcove->con);
+		break;
+	case USBC_RSLT_AUDIO_ACC:
+		wcove->partner.accessory = TYPEC_ACCESSORY_AUDIO;
+		wcove->partner.type = TYPEC_PARTNER_ACCESSORY;
+		wcove->con.partner = &wcove->partner;
+		typec_connect(wcove->port, &wcove->con);
+		break;
+	default:
+		dev_WARN(wcove->dev, "%s Undefined result\n", __func__);
+		goto err;
+	}
+
+	if (!completion_done(&wcove->complete))
+		complete(&wcove->complete);
+out:
+	/* If either CC pins is requesting VCONN, we turn it on */
+	if ((cc1_ctrl & USBC_CC_CTRL_VCONN_EN) ||
+	    (cc2_ctrl &	USBC_CC_CTRL_VCONN_EN))
+		wcove_typec_func(wcove, WCOVE_FUNC_DRIVE_VCONN, true);
+	else
+		wcove_typec_func(wcove, WCOVE_FUNC_DRIVE_VCONN, false);
+
+	/* Relying on the FSM to know when we need to drive VBUS. */
+	wcove_typec_func(wcove, WCOVE_FUNC_DRIVE_VBUS,
+			 !!(status2 & USBC_STATUS2_VBUS_REQ));
+err:
+	/* REVISIT: Clear WhiskeyCove CHGR Type-C interrupt */
+	regmap_write(wcove->regmap, WCOVE_CHGRIRQ0, BIT(5));
+
+	mutex_unlock(&wcove->lock);
+	return IRQ_HANDLED;
+}
+
+static int wcove_typec_probe(struct platform_device *pdev)
+{
+	struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent);
+	struct wcove_typec *wcove;
+	unsigned int val;
+	int ret;
+
+	wcove = devm_kzalloc(&pdev->dev, sizeof(*wcove), GFP_KERNEL);
+	if (!wcove)
+		return -ENOMEM;
+
+	init_completion(&wcove->complete);
+	mutex_init(&wcove->lock);
+	wcove->dev = &pdev->dev;
+	wcove->regmap = pmic->regmap;
+
+	ret = regmap_irq_get_virq(pmic->irq_chip_data_level2,
+				  platform_get_irq(pdev, 0));
+	if (ret < 0)
+		return ret;
+
+	ret = devm_request_threaded_irq(&pdev->dev, ret, NULL,
+					wcove_typec_irq, IRQF_ONESHOT,
+					"wcove_typec", wcove);
+	if (ret)
+		return ret;
+
+	wcove->cap.type = TYPEC_PORT_DRP;
+
+	wcove->port = typec_register_port(&pdev->dev, &wcove->cap);
+	if (IS_ERR(wcove->port))
+		return PTR_ERR(wcove->port);
+
+	if (!acpi_check_dsm(ACPI_HANDLE(&pdev->dev), uuid.b, 0, 0x1f)) {
+		dev_err(&pdev->dev, "Missing _DSM functions\n");
+		return -ENODEV;
+	}
+
+	/* Make sure the PD PHY is disabled until PD stack is ready */
+	regmap_read(wcove->regmap, USBC_CONTROL3, &val);
+	regmap_write(wcove->regmap, USBC_CONTROL3, val | USBC_CONTROL3_PD_DIS);
+
+	/* DRP mode without accessory support */
+	regmap_read(wcove->regmap, USBC_CONTROL1, &val);
+	regmap_write(wcove->regmap, USBC_CONTROL1, USBC_CONTROL1_MODE_DRP(val));
+
+	/* Unmask everything */
+	regmap_read(wcove->regmap, USBC_IRQMASK1, &val);
+	regmap_write(wcove->regmap, USBC_IRQMASK1, val & ~USBC_IRQMASK1_ALL);
+	regmap_read(wcove->regmap, USBC_IRQMASK2, &val);
+	regmap_write(wcove->regmap, USBC_IRQMASK2, val & ~USBC_IRQMASK2_ALL);
+
+	platform_set_drvdata(pdev, wcove);
+	return 0;
+}
+
+static int wcove_typec_remove(struct platform_device *pdev)
+{
+	struct wcove_typec *wcove = platform_get_drvdata(pdev);
+	unsigned int val;
+
+	/* Mask everything */
+	regmap_read(wcove->regmap, USBC_IRQMASK1, &val);
+	regmap_write(wcove->regmap, USBC_IRQMASK1, val | USBC_IRQMASK1_ALL);
+	regmap_read(wcove->regmap, USBC_IRQMASK2, &val);
+	regmap_write(wcove->regmap, USBC_IRQMASK2, val | USBC_IRQMASK2_ALL);
+
+	typec_unregister_port(wcove->port);
+	return 0;
+}
+
+static struct platform_driver wcove_typec_driver = {
+	.driver = {
+		.name		= "bxt_wcove_usbc",
+	},
+	.probe			= wcove_typec_probe,
+	.remove			= wcove_typec_remove,
+};
+
+module_platform_driver(wcove_typec_driver);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("WhiskeyCove PMIC USB Type-C PHY driver");
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]

#1428135 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Tue, 2016-06-21 at 17:51 +0300, Heikki Krogerus wrote:
> +What:          /sys/class/typec/<port>/supported_data_roles
> +Data:          June 2016
> +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> +Description:
> +               Lists the USB data roles, host or device, the port is
> capable
> +               of supporting.

On third thought, this is a problem. Looking at 4.4.8.1
DEVICE_CAPABILITIES (Required) of USB Type-C Port Controller
Interface Specification we lack capability.

A port that can do DRP is not the same thing as a port that
can be switched between DFP and UFP. We cannot express that.

	Regards
		Oliver

[toc] | [prev] | [next] | [standalone]

#1428620 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Tue, Jun 21, 2016 at 10:25:05PM +0200, Oliver Neukum wrote:
> On Tue, 2016-06-21 at 17:51 +0300, Heikki Krogerus wrote:
> > +What:          /sys/class/typec/<port>/supported_data_roles
> > +Data:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Lists the USB data roles, host or device, the port is
> > capable
> > +               of supporting.
> 
> On third thought, this is a problem. Looking at 4.4.8.1
> DEVICE_CAPABILITIES (Required) of USB Type-C Port Controller
> Interface Specification we lack capability.
> 
> A port that can do DRP is not the same thing as a port that
> can be switched between DFP and UFP. We cannot express that.

What do you mean? DRP means we support and are able to swap the data
role, but it just does not mean we can act as both source and sink. And
that information we already get from separate attribute:
"supported_power_roles".

But if the port is DRP, we will always be able to swap the data role
between DFP and UFP.


Thanks,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1428639 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Wed, Jun 22, 2016 at 12:50:16PM +0300, Heikki Krogerus wrote:
> On Tue, Jun 21, 2016 at 10:25:05PM +0200, Oliver Neukum wrote:
> > On Tue, 2016-06-21 at 17:51 +0300, Heikki Krogerus wrote:
> > > +What:          /sys/class/typec/<port>/supported_data_roles
> > > +Data:          June 2016
> > > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > +Description:
> > > +               Lists the USB data roles, host or device, the port is
> > > capable
> > > +               of supporting.
> > 
> > On third thought, this is a problem. Looking at 4.4.8.1
> > DEVICE_CAPABILITIES (Required) of USB Type-C Port Controller
> > Interface Specification we lack capability.
> > 
> > A port that can do DRP is not the same thing as a port that
> > can be switched between DFP and UFP. We cannot express that.
> 
> What do you mean? DRP means we support and are able to swap the data
> role, but it just does not mean we can act as both source and sink. And
> that information we already get from separate attribute:
> "supported_power_roles".
> 
> But if the port is DRP, we will always be able to swap the data role
> between DFP and UFP.

Just to clarify: DRP as it's defined in Type-C spec < 1.2 means the
data role, not power role. And that is what Universal Serial Bus
Type-CTM Port Controller specification is based on. Please correct me
if I'm wrong.


Thanks,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1428720 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Wed, 2016-06-22 at 13:03 +0300, Heikki Krogerus wrote:
> On Wed, Jun 22, 2016 at 12:50:16PM +0300, Heikki Krogerus wrote:
>  
> > But if the port is DRP, we will always be able to swap the data role
> > between DFP and UFP.
> 
> Just to clarify: DRP as it's defined in Type-C spec < 1.2 means the
> data role, not power role. And that is what Universal Serial Bus
> Type-CTM Port Controller specification is based on. Please correct me
> if I'm wrong.

Understood. That is exactly the problem. That spec defines two ways
to support host and client.

	Regards
		Oliver

[toc] | [prev] | [next] | [standalone]

#1428655 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Wed, 2016-06-22 at 12:50 +0300, Heikki Krogerus wrote:
> On Tue, Jun 21, 2016 at 10:25:05PM +0200, Oliver Neukum wrote:
> > On Tue, 2016-06-21 at 17:51 +0300, Heikki Krogerus wrote:
> > > +What:          /sys/class/typec/<port>/supported_data_roles
> > > +Data:          June 2016
> > > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > +Description:
> > > +               Lists the USB data roles, host or device, the port is
> > > capable
> > > +               of supporting.
> > 
> > On third thought, this is a problem. Looking at 4.4.8.1
> > DEVICE_CAPABILITIES (Required) of USB Type-C Port Controller
> > Interface Specification we lack capability.
> > 
> > A port that can do DRP is not the same thing as a port that
> > can be switched between DFP and UFP. We cannot express that.
> 
> What do you mean? DRP means we support and are able to swap the data

No. That is the error. We support them concurrently. And that is not
obvious. It is perfectly possible to support both but not concurrently.

> role, but it just does not mean we can act as both source and sink. And
> that information we already get from separate attribute:
> "supported_power_roles".

But it is different. Suppose we have a port that can be switched between
UFP and DFP, as the spec defines. If it is switched to DFP and we plug
in a DFP it will not work. UFP into UFP has the same result.

Plugging it into a DRP will always work.

It is true that both support host and device, but the capability of
the ports is different. And that is not expressed.

	Regards
		Oliver

[toc] | [prev] | [next] | [standalone]

#1428715 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Wed, Jun 22, 2016 at 12:14:55PM +0200, Oliver Neukum wrote:
> On Wed, 2016-06-22 at 12:50 +0300, Heikki Krogerus wrote:
> > On Tue, Jun 21, 2016 at 10:25:05PM +0200, Oliver Neukum wrote:
> > > On Tue, 2016-06-21 at 17:51 +0300, Heikki Krogerus wrote:
> > > > +What:          /sys/class/typec/<port>/supported_data_roles
> > > > +Data:          June 2016
> > > > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > > +Description:
> > > > +               Lists the USB data roles, host or device, the port is
> > > > capable
> > > > +               of supporting.
> > > 
> > > On third thought, this is a problem. Looking at 4.4.8.1
> > > DEVICE_CAPABILITIES (Required) of USB Type-C Port Controller
> > > Interface Specification we lack capability.
> > > 
> > > A port that can do DRP is not the same thing as a port that
> > > can be switched between DFP and UFP. We cannot express that.
> > 
> > What do you mean? DRP means we support and are able to swap the data
> 
> No. That is the error. We support them concurrently. And that is not
> obvious. It is perfectly possible to support both but not concurrently.
> 
> > role, but it just does not mean we can act as both source and sink. And
> > that information we already get from separate attribute:
> > "supported_power_roles".
> 
> But it is different. Suppose we have a port that can be switched between
> UFP and DFP, as the spec defines. If it is switched to DFP and we plug
> in a DFP it will not work. UFP into UFP has the same result.
> 
> Plugging it into a DRP will always work.
> 
> It is true that both support host and device, but the capability of
> the ports is different. And that is not expressed.

Sorry but I don't think I understand?

So if we can act only as UFP, the supported_data_roles will list:

        device

If we can act only as DFP, the supported_data_roles will list:

        host

If our port is DRD (which would be DRP in the port controller spec),
the supported_power_roles will list:

        device, host

And the power role, if the port is Source only, the
supported_power_roles will list:

        source

If the port is Sink only, the supported_power_roles will list:

        sink

If our port is DRP, the supported_power_roles will list:

        source, sink

What is there that is missing? We are able to express all the types of
"Roles Supported" that the DEVICE_CAPABILITIES define, no?


Thanks,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1428818 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Wed, 2016-06-22 at 14:44 +0300, Heikki Krogerus wrote:
> If our port is DRD (which would be DRP in the port controller spec),
> the supported_power_roles will list:
> 
>         device, host
> 
> And the power role, if the port is Source only, the
> supported_power_roles will list:
> 
>         source
> 
> If the port is Sink only, the supported_power_roles will list:
> 
>         sink
> 
> If our port is DRP, the supported_power_roles will list:
> 
>         source, sink
> 
> What is there that is missing? We are able to express all the types of
> "Roles Supported" that the DEVICE_CAPABILITIES define, no?

No, because these are distinct in time. Some ports are DRP so they
support

device, host

at the same time. Some ports can be switched between DFP and UFP
they then either support host or device. But you lose the information
that the ports can be switched.

	Regards
		Oliver

[toc] | [prev] | [next] | [standalone]

#1428864 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Wed, Jun 22, 2016 at 03:47:03PM +0200, Oliver Neukum wrote:
> On Wed, 2016-06-22 at 14:44 +0300, Heikki Krogerus wrote:
> > If our port is DRD (which would be DRP in the port controller spec),
> > the supported_power_roles will list:
> > 
> >         device, host
> > 
> > And the power role, if the port is Source only, the
> > supported_power_roles will list:
> > 
> >         source
> > 
> > If the port is Sink only, the supported_power_roles will list:
> > 
> >         sink
> > 
> > If our port is DRP, the supported_power_roles will list:
> > 
> >         source, sink
> > 
> > What is there that is missing? We are able to express all the types of
> > "Roles Supported" that the DEVICE_CAPABILITIES define, no?
> 
> No, because these are distinct in time. Some ports are DRP so they
> support
> 
> device, host
> 
> at the same time. Some ports can be switched between DFP and UFP
> they then either support host or device. But you lose the information
> that the ports can be switched.

You can't ever be host and device at the same time. Just like you
can't ever be source and sink at the same time.

Are we now talking about how should a port be advertised to the
partners? So basically, do you want to be able to program the port to
be DFP only, UFP only or DRP from user space?


Thanks,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1429012 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Wed, 2016-06-22 at 17:38 +0300, Heikki Krogerus wrote:
> On Wed, Jun 22, 2016 at 03:47:03PM +0200, Oliver Neukum wrote:
> > On Wed, 2016-06-22 at 14:44 +0300, Heikki Krogerus wrote:
> > > If our port is DRD (which would be DRP in the port controller spec),
> > > the supported_power_roles will list:
> > > 
> > >         device, host
> > > 
> > > And the power role, if the port is Source only, the
> > > supported_power_roles will list:
> > > 
> > >         source
> > > 
> > > If the port is Sink only, the supported_power_roles will list:
> > > 
> > >         sink
> > > 
> > > If our port is DRP, the supported_power_roles will list:
> > > 
> > >         source, sink
> > > 
> > > What is there that is missing? We are able to express all the types of
> > > "Roles Supported" that the DEVICE_CAPABILITIES define, no?
> > 
> > No, because these are distinct in time. Some ports are DRP so they
> > support
> > 
> > device, host
> > 
> > at the same time. Some ports can be switched between DFP and UFP
> > they then either support host or device. But you lose the information
> > that the ports can be switched.
> 
> You can't ever be host and device at the same time. Just like you
> can't ever be source and sink at the same time.

True, but you can be able to become host and device at the same time.
That is the purpose of a DRP port.

And you can be able to become a host and be able to become a device.
But not at the same time. These ports are switchable.

The current API cannot express the difference.

> Are we now talking about how should a port be advertised to the
> partners? So basically, do you want to be able to program the port to
> be DFP only, UFP only or DRP from user space?

That would be cool, but according to the spec this is an unalterable
attribute. Please look at section 4.4.8.1
It clearly describes different types of ports. We cannot express
the differences between the types described there with the current API.

	Regards
		Oliver

[toc] | [prev] | [next] | [standalone]

#1429566 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Wed, Jun 22, 2016 at 06:44:18PM +0200, Oliver Neukum wrote:
> On Wed, 2016-06-22 at 17:38 +0300, Heikki Krogerus wrote:
> > On Wed, Jun 22, 2016 at 03:47:03PM +0200, Oliver Neukum wrote:
> > > On Wed, 2016-06-22 at 14:44 +0300, Heikki Krogerus wrote:
> > > > If our port is DRD (which would be DRP in the port controller spec),
> > > > the supported_power_roles will list:
> > > > 
> > > >         device, host
> > > > 
> > > > And the power role, if the port is Source only, the
> > > > supported_power_roles will list:
> > > > 
> > > >         source
> > > > 
> > > > If the port is Sink only, the supported_power_roles will list:
> > > > 
> > > >         sink
> > > > 
> > > > If our port is DRP, the supported_power_roles will list:
> > > > 
> > > >         source, sink
> > > > 
> > > > What is there that is missing? We are able to express all the types of
> > > > "Roles Supported" that the DEVICE_CAPABILITIES define, no?
> > > 
> > > No, because these are distinct in time. Some ports are DRP so they
> > > support
> > > 
> > > device, host
> > > 
> > > at the same time. Some ports can be switched between DFP and UFP
> > > they then either support host or device. But you lose the information
> > > that the ports can be switched.
> > 
> > You can't ever be host and device at the same time. Just like you
> > can't ever be source and sink at the same time.
> 
> True, but you can be able to become host and device at the same time.

No you can't..

> That is the purpose of a DRP port.

No it's not. DRP means a port that can operate as _either_ Source
(host) or Sink (device), but not at the same time..

> And you can be able to become a host and be able to become a device.
> But not at the same time. These ports are switchable.
> 
> The current API cannot express the difference.

I think you have misunderstood something. The only case where the port
can be dual-role is if it's set to be DRP. Otherwise it's Source only
OR Sink only.

The "Role Supported" bits only tell us how we can program for example
the ROLE_CONTROL registers. I guess the "Roles Supported" bits in
DEVICE_CAPABILITIES are not explained properly, so let's go over them
here:

000b = Source _or_ Sink only
001b = Source only
010b = Sink only
011b = Sink only with support for autonomously detected accessory modes
100b = DRP only, and this I believe mean we can not program the port
        to be Sink only or Source only
101b = Source only OR Sink only OR DRP, plus ability to detect
        accessories and I guess also cables autonomously
110b = Source only OR Sink only OR DRP

So where the spec lists "Source, Sink", it actually should have said
"Source only OR Sink only".

But you still have only the following options for a port:
1) Source only (host)
2) Sink only (device)
3) DRP (device, host)


Thanks,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1429590 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Thu, 2016-06-23 at 11:23 +0300, Heikki Krogerus wrote:
> On Wed, Jun 22, 2016 at 06:44:18PM +0200, Oliver Neukum wrote:

> No it's not. DRP means a port that can operate as _either_ Source
> (host) or Sink (device), but not at the same time..

Yes, but it is unclear what you will be after a connection
and that's the point.

> > And you can be able to become a host and be able to become a device.
> > But not at the same time. These ports are switchable.
> > 
> > The current API cannot express the difference.
> 
> I think you have misunderstood something. The only case where the port
> can be dual-role is if it's set to be DRP. Otherwise it's Source only
> OR Sink only.
> 
> The "Role Supported" bits only tell us how we can program for example
> the ROLE_CONTROL registers. I guess the "Roles Supported" bits in
> DEVICE_CAPABILITIES are not explained properly, so let's go over them
> here:
> 
> 000b = Source _or_ Sink only
> 001b = Source only
> 010b = Sink only
> 011b = Sink only with support for autonomously detected accessory modes
> 100b = DRP only, and this I believe mean we can not program the port
>         to be Sink only or Source only

I think so, too.

> 101b = Source only OR Sink only OR DRP, plus ability to detect
>         accessories and I guess also cables autonomously
> 110b = Source only OR Sink only OR DRP
> 
> So where the spec lists "Source, Sink", it actually should have said
> "Source only OR Sink only".
> 
> But you still have only the following options for a port:
> 1) Source only (host)
> 2) Sink only (device)
> 3) DRP (device, host)

Yes, so you can map "000b = Source _or_ Sink only" to host or device
depending on the current setting. But then you lose the information
that it can be changed. It either will look like "001b" or "010b".
So we throw away information.

And you map "100b = DRP only" and "101b" and "110b" to host, device
which again drops information.

	Regards
		Oliver

[toc] | [prev] | [next] | [standalone]

#1429739 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

Hi Oliver,

On Thu, Jun 23, 2016 at 10:38:58AM +0200, Oliver Neukum wrote:
> On Thu, 2016-06-23 at 11:23 +0300, Heikki Krogerus wrote:
> > On Wed, Jun 22, 2016 at 06:44:18PM +0200, Oliver Neukum wrote:
> 
> > No it's not. DRP means a port that can operate as _either_ Source
> > (host) or Sink (device), but not at the same time..
> 
> Yes, but it is unclear what you will be after a connection
> and that's the point.

Which is a fact that we can do nothing about. The role after
connection with DRP ports will be dictated by the partner or selected
randomly in case the partner is also DRP. We can prefer a role, but
that in the end guarantees nothing. So if the role that we end up with
after connection (seen in current_data_role) does not satisfy us, all
we can do is try to swap it.

I'm not sure what is your point here.

> > > And you can be able to become a host and be able to become a device.
> > > But not at the same time. These ports are switchable.
> > > 
> > > The current API cannot express the difference.
> > 
> > I think you have misunderstood something. The only case where the port
> > can be dual-role is if it's set to be DRP. Otherwise it's Source only
> > OR Sink only.
> > 
> > The "Role Supported" bits only tell us how we can program for example
> > the ROLE_CONTROL registers. I guess the "Roles Supported" bits in
> > DEVICE_CAPABILITIES are not explained properly, so let's go over them
> > here:
> > 
> > 000b = Source _or_ Sink only
> > 001b = Source only
> > 010b = Sink only
> > 011b = Sink only with support for autonomously detected accessory modes
> > 100b = DRP only, and this I believe mean we can not program the port
> >         to be Sink only or Source only
> 
> I think so, too.
> 
> > 101b = Source only OR Sink only OR DRP, plus ability to detect
> >         accessories and I guess also cables autonomously
> > 110b = Source only OR Sink only OR DRP
> > 
> > So where the spec lists "Source, Sink", it actually should have said
> > "Source only OR Sink only".
> > 
> > But you still have only the following options for a port:
> > 1) Source only (host)
> > 2) Sink only (device)
> > 3) DRP (device, host)
> 
> Yes, so you can map "000b = Source _or_ Sink only" to host or device
> depending on the current setting. But then you lose the information
> that it can be changed. 

No it can't. The idea with the Roles Supported bits is for the driver
to be able to select the most appropriate role that fits the abilities
of the platform.

The configuration of the port after probing the port controller will
never change. If you have initially configured the port to be Sink
only (so device), it most likely means your platform can not act as
Source even if the port controller would.

And if you want to change the configuration of the port, for example
if your platform is capable of supporting Source and Sink modes, but
your port controller is not capable of supporting DRP (which would be
pretty messed up situation) but instead forces you to choose between
Sink and Source, you would in practice in any case have to unregister,
reconfigure and register the port again.

But in most cases the platform will not support all the capabilities
the port controller will be capable of. If for example on your
platform you have only USB host controller, it just means you will
have to have port controller that returns either 000b, 001b, 101b or
110b in the supported roles bits. Otherwise it will no be usable on
your platform.

> So we throw away information.
> 
> And you map "100b = DRP only" and "101b" and "110b" to host, device

No I don't. If our platform can only support Sink mode, value "100b"
will not work and can not be ever registered, and values "101b" and
"110b" will report "device" in supported_data_roles.

And it is not the class that defines the capabilities of a port.
They are defined by the drivers that register the ports.

> which again drops information.

There is no use in knowing details about the port controller
capabilities like if a port could be configured to be Source or Sink
only instead of just DRP from the typec class point of view. Those
details are port controller specific, and completely out side the
scope of the class driver. Not all USB Type-C PHYs will be port
controllers and not all ports registered with the class will even have
a PHY to deal with. This means we will not even always be able to read
the same kinds of details of the port like we are with port
controllers.

So if you want to get the capabilities of the port controller in use,
the port controller driver will have to expose them to user space, not
the class.


Br,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1429758 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

Hi,

On 23/06/16 15:00, Heikki Krogerus wrote:
> Hi Oliver,
> 
> On Thu, Jun 23, 2016 at 10:38:58AM +0200, Oliver Neukum wrote:
>> On Thu, 2016-06-23 at 11:23 +0300, Heikki Krogerus wrote:
>>> On Wed, Jun 22, 2016 at 06:44:18PM +0200, Oliver Neukum wrote:
>>
>>> No it's not. DRP means a port that can operate as _either_ Source
>>> (host) or Sink (device), but not at the same time..
>>
>> Yes, but it is unclear what you will be after a connection
>> and that's the point.
> 
> Which is a fact that we can do nothing about. The role after
> connection with DRP ports will be dictated by the partner or selected
> randomly in case the partner is also DRP. We can prefer a role, but
> that in the end guarantees nothing. So if the role that we end up with
> after connection (seen in current_data_role) does not satisfy us, all
> we can do is try to swap it.
> 
> I'm not sure what is your point here.

What if the application wants to know exactly what role the device is
operating in at the current moment?

We need to have 2 distinct parameters.

1) supported_modes : host, device, host or device
2) current_mode: host, device, disconnected

--
cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1429808 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On Thu, Jun 23, 2016 at 03:25:46PM +0300, Roger Quadros wrote:
> Hi,
> 
> On 23/06/16 15:00, Heikki Krogerus wrote:
> > Hi Oliver,
> > 
> > On Thu, Jun 23, 2016 at 10:38:58AM +0200, Oliver Neukum wrote:
> >> On Thu, 2016-06-23 at 11:23 +0300, Heikki Krogerus wrote:
> >>> On Wed, Jun 22, 2016 at 06:44:18PM +0200, Oliver Neukum wrote:
> >>
> >>> No it's not. DRP means a port that can operate as _either_ Source
> >>> (host) or Sink (device), but not at the same time..
> >>
> >> Yes, but it is unclear what you will be after a connection
> >> and that's the point.
> > 
> > Which is a fact that we can do nothing about. The role after
> > connection with DRP ports will be dictated by the partner or selected
> > randomly in case the partner is also DRP. We can prefer a role, but
> > that in the end guarantees nothing. So if the role that we end up with
> > after connection (seen in current_data_role) does not satisfy us, all
> > we can do is try to swap it.
> > 
> > I'm not sure what is your point here.
> 
> What if the application wants to know exactly what role the device is
> operating in at the current moment?
> 
> We need to have 2 distinct parameters.
> 
> 1) supported_modes : host, device, host or device
> 2) current_mode: host, device, disconnected

And we already do. But that is not the topic of this thread. Oliver I
believe feels that we are not presenting all the capabilities of the
ports.


Cheers,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1429823 — Re: [PATCHv3 1/2] usb: USB Type-C connector class

On 06/23/2016 05:00 AM, Heikki Krogerus wrote:
> Hi Oliver,
>
> On Thu, Jun 23, 2016 at 10:38:58AM +0200, Oliver Neukum wrote:
>> On Thu, 2016-06-23 at 11:23 +0300, Heikki Krogerus wrote:
>>> On Wed, Jun 22, 2016 at 06:44:18PM +0200, Oliver Neukum wrote:
>>
>>> No it's not. DRP means a port that can operate as _either_ Source
>>> (host) or Sink (device), but not at the same time..
>>
>> Yes, but it is unclear what you will be after a connection
>> and that's the point.
>
> Which is a fact that we can do nothing about. The role after
> connection with DRP ports will be dictated by the partner or selected
> randomly in case the partner is also DRP. We can prefer a role, but
> that in the end guarantees nothing. So if the role that we end up with
> after connection (seen in current_data_role) does not satisfy us, all
> we can do is try to swap it.
>
> I'm not sure what is your point here.
>
>>>> And you can be able to become a host and be able to become a device.
>>>> But not at the same time. These ports are switchable.
>>>>
>>>> The current API cannot express the difference.
>>>
>>> I think you have misunderstood something. The only case where the port
>>> can be dual-role is if it's set to be DRP. Otherwise it's Source only
>>> OR Sink only.
>>>
>>> The "Role Supported" bits only tell us how we can program for example
>>> the ROLE_CONTROL registers. I guess the "Roles Supported" bits in
>>> DEVICE_CAPABILITIES are not explained properly, so let's go over them
>>> here:
>>>
>>> 000b = Source _or_ Sink only
>>> 001b = Source only
>>> 010b = Sink only
>>> 011b = Sink only with support for autonomously detected accessory modes
>>> 100b = DRP only, and this I believe mean we can not program the port
>>>          to be Sink only or Source only
>>
>> I think so, too.
>>
>>> 101b = Source only OR Sink only OR DRP, plus ability to detect
>>>          accessories and I guess also cables autonomously
>>> 110b = Source only OR Sink only OR DRP
>>>
>>> So where the spec lists "Source, Sink", it actually should have said
>>> "Source only OR Sink only".
>>>
>>> But you still have only the following options for a port:
>>> 1) Source only (host)
>>> 2) Sink only (device)
>>> 3) DRP (device, host)
>>
>> Yes, so you can map "000b = Source _or_ Sink only" to host or device
>> depending on the current setting. But then you lose the information
>> that it can be changed.
>
> No it can't. The idea with the Roles Supported bits is for the driver
> to be able to select the most appropriate role that fits the abilities
> of the platform.
>
> The configuration of the port after probing the port controller will
> never change. If you have initially configured the port to be Sink
> only (so device), it most likely means your platform can not act as
> Source even if the port controller would.
>
> And if you want to change the configuration of the port, for example
> if your platform is capable of supporting Source and Sink modes, but
> your port controller is not capable of supporting DRP (which would be
> pretty messed up situation) but instead forces you to choose between
> Sink and Source, you would in practice in any case have to unregister,
> reconfigure and register the port again.
>
> But in most cases the platform will not support all the capabilities
> the port controller will be capable of. If for example on your
> platform you have only USB host controller, it just means you will
> have to have port controller that returns either 000b, 001b, 101b or
> 110b in the supported roles bits. Otherwise it will no be usable on
> your platform.
>
>> So we throw away information.
>>
>> And you map "100b = DRP only" and "101b" and "110b" to host, device
>
> No I don't. If our platform can only support Sink mode, value "100b"
> will not work and can not be ever registered, and values "101b" and
> "110b" will report "device" in supported_data_roles.
>
> And it is not the class that defines the capabilities of a port.
> They are defined by the drivers that register the ports.
>
>> which again drops information.
>
> There is no use in knowing details about the port controller
> capabilities like if a port could be configured to be Source or Sink
> only instead of just DRP from the typec class point of view. Those
> details are port controller specific, and completely out side the
> scope of the class driver. Not all USB Type-C PHYs will be port
> controllers and not all ports registered with the class will even have
> a PHY to deal with. This means we will not even always be able to read
> the same kinds of details of the port like we are with port
> controllers.
>
> So if you want to get the capabilities of the port controller in use,
> the port controller driver will have to expose them to user space, not
> the class.
>
Agreed.

Guenter

[toc] | [prev] | [next] | [standalone]

#1428233

On Tue, Jun 21, 2016 at 05:51:49PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> I'm considering all the RFCs I send after v1 as v2 (I don't remember
> how many I send). Hope this is OK and hope there is nothing big
> missing anymore (or broken).
> 
> Sorry about the delay. I've been really busy with some internal tasks.
> I'm guessing we missed v4.8 with this thing. I'm sorry about that.
> 
> I'm including in this series a driver for the Broxton PMIC USB Type-C
> PHY.
> 
Can you by any chance push the current version into your repository
on github ?

Thanks,
Guenter

> 
> Changes since v2:
> - Notification on role and alternate mode changes
> - cleanups
> 
> Changes since v1:
> - Completely rewrote alternate mode support
> - Patners, cables and cable plugs presented as devices.
> 
> 
> Heikki Krogerus (2):
>   usb: USB Type-C connector class
>   usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY
> 
>  Documentation/ABI/testing/sysfs-class-typec |  163 ++++
>  Documentation/usb/typec.txt                 |  101 +++
>  MAINTAINERS                                 |    9 +
>  drivers/usb/Kconfig                         |    2 +
>  drivers/usb/Makefile                        |    2 +
>  drivers/usb/typec/Kconfig                   |   21 +
>  drivers/usb/typec/Makefile                  |    2 +
>  drivers/usb/typec/typec.c                   | 1173 +++++++++++++++++++++++++++
>  drivers/usb/typec/typec_wcove.c             |  376 +++++++++
>  include/linux/usb/typec.h                   |  255 ++++++
>  10 files changed, 2104 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-typec
>  create mode 100644 Documentation/usb/typec.txt
>  create mode 100644 drivers/usb/typec/Kconfig
>  create mode 100644 drivers/usb/typec/Makefile
>  create mode 100644 drivers/usb/typec/typec.c
>  create mode 100644 drivers/usb/typec/typec_wcove.c
>  create mode 100644 include/linux/usb/typec.h
> 
> -- 
> 2.8.1
> 

[toc] | [prev] | [next] | [standalone]

#1428626

Hi Guenter,

On Tue, Jun 21, 2016 at 03:25:55PM -0700, Guenter Roeck wrote:
> On Tue, Jun 21, 2016 at 05:51:49PM +0300, Heikki Krogerus wrote:
> > Hi,
> > 
> > I'm considering all the RFCs I send after v1 as v2 (I don't remember
> > how many I send). Hope this is OK and hope there is nothing big
> > missing anymore (or broken).
> > 
> > Sorry about the delay. I've been really busy with some internal tasks.
> > I'm guessing we missed v4.8 with this thing. I'm sorry about that.
> > 
> > I'm including in this series a driver for the Broxton PMIC USB Type-C
> > PHY.
> > 
> Can you by any chance push the current version into your repository
> on github ?

Sure, but Felipe has also put these patches to a branch testing/typec
in his tree:
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/log/?h=testing/typec


Cheers,

-- 
heikki

[toc] | [prev] | [next] | [standalone]

#1428784

On 06/22/2016 02:51 AM, Heikki Krogerus wrote:
> Hi Guenter,
>
> On Tue, Jun 21, 2016 at 03:25:55PM -0700, Guenter Roeck wrote:
>> On Tue, Jun 21, 2016 at 05:51:49PM +0300, Heikki Krogerus wrote:
>>> Hi,
>>>
>>> I'm considering all the RFCs I send after v1 as v2 (I don't remember
>>> how many I send). Hope this is OK and hope there is nothing big
>>> missing anymore (or broken).
>>>
>>> Sorry about the delay. I've been really busy with some internal tasks.
>>> I'm guessing we missed v4.8 with this thing. I'm sorry about that.
>>>
>>> I'm including in this series a driver for the Broxton PMIC USB Type-C
>>> PHY.
>>>
>> Can you by any chance push the current version into your repository
>> on github ?
>
> Sure, but Felipe has also put these patches to a branch testing/typec
> in his tree:
> https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/log/?h=testing/typec
>

That works just as well, of course.

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web