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

Groups > linux.kernel > #1467568 > unrolled thread

[PATCHv6 0/3] USB Type-C Connector class

Started byHeikki Krogerus <heikki.krogerus@linux.intel.com>
First post2016-08-22 14:10 +0200
Last post2016-08-30 19:10 +0200
Articles 13 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCHv6 0/3] USB Type-C Connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-08-22 14:10 +0200
    [PATCHv6 3/3] mfd: intel_soc_pmic_bxtwc: add support for USB Type-C PHY on WhiskeyCove Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-08-22 14:10 +0200
    [PATCHv6 2/3] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-08-22 14:10 +0200
    Re: [PATCHv6 1/3] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-08-25 14:10 +0200
      Re: [PATCHv6 1/3] usb: USB Type-C connector class Vincent Palatin <vpalatin@chromium.org> - 2016-08-26 15:20 +0200
        Re: [PATCHv6 1/3] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-08-26 16:20 +0200
          Re: [PATCHv6 1/3] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-08-29 15:10 +0200
            Re: [PATCHv6 1/3] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-08-29 15:50 +0200
              Re: [PATCHv6 1/3] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-08-29 16:20 +0200
                Re: [PATCHv6 1/3] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-08-29 22:10 +0200
                  Re: [PATCHv6 1/3] usb: USB Type-C connector class Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-08-30 10:30 +0200
                    Re: [PATCHv6 1/3] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-08-30 17:30 +0200
                    Re: [PATCHv6 1/3] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2016-08-30 19:10 +0200

#1467568 — [PATCHv6 0/3] USB Type-C Connector class

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-08-22 14:10 +0200
Subject[PATCHv6 0/3] USB Type-C Connector class
Message-ID<s8Wdb-66j-1@gated-at.bofh.it>
The USB Type-C class is meant to provide unified interface to the
userspace to present the USB Type-C ports in a system.

Changes since v5:
- Only updating the roles based on driver notifications
- Added MODULE_ALIAS for the WhiskeyCove module
- Including the patch that creates the actual platform device for the
  WhiskeyCove Type-C PHY in this series.

Changes since v4:
- Remove the port lock completely

Changes since v3:
- Documentation cleanup as proposed by Roger Quadros
- Setting partner altmodes member to NULL on removal and fixing a
  warning, as proposed by Guenter Roeck
- Added the following attributes for partners and cables:
  * supports_usb_power_delivery
  * id_header_vdo
- "id_header_vdo" is visible only when the partner or cable supports
  USB Power Delivery communication.
- Partner attribute "accessory" is hidden when the partner type is not
  "Accessory".

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 (3):
  usb: USB Type-C connector class
  usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY
  mfd: intel_soc_pmic_bxtwc: add support for USB Type-C PHY on
    WhiskeyCove

 Documentation/ABI/testing/sysfs-class-typec |  199 +++++
 Documentation/usb/typec.txt                 |  103 +++
 MAINTAINERS                                 |    9 +
 drivers/mfd/intel_soc_pmic_bxtwc.c          |   11 +
 drivers/usb/Kconfig                         |    2 +
 drivers/usb/Makefile                        |    2 +
 drivers/usb/typec/Kconfig                   |   21 +
 drivers/usb/typec/Makefile                  |    2 +
 drivers/usb/typec/typec.c                   | 1090 +++++++++++++++++++++++++++
 drivers/usb/typec/typec_wcove.c             |  372 +++++++++
 include/linux/usb/typec.h                   |  260 +++++++
 11 files changed, 2071 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]

#1467572 — [PATCHv6 3/3] mfd: intel_soc_pmic_bxtwc: add support for USB Type-C PHY on WhiskeyCove

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-08-22 14:10 +0200
Subject[PATCHv6 3/3] mfd: intel_soc_pmic_bxtwc: add support for USB Type-C PHY on WhiskeyCove
Message-ID<s8Wdc-66j-15@gated-at.bofh.it>
In reply to#1467568
Intel WhiskeyCove PMIC has also a USB Type-C PHY, so let's
create a device for it.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/intel_soc_pmic_bxtwc.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mfd/intel_soc_pmic_bxtwc.c b/drivers/mfd/intel_soc_pmic_bxtwc.c
index b942876..0e61dde 100644
--- a/drivers/mfd/intel_soc_pmic_bxtwc.c
+++ b/drivers/mfd/intel_soc_pmic_bxtwc.c
@@ -84,6 +84,7 @@ enum bxtwc_irqs_level2 {
 	BXTWC_THRM2_IRQ,
 	BXTWC_BCU_IRQ,
 	BXTWC_ADC_IRQ,
+	BXTWC_USBC_IRQ,
 	BXTWC_CHGR0_IRQ,
 	BXTWC_CHGR1_IRQ,
 	BXTWC_GPIO0_IRQ,
@@ -109,6 +110,7 @@ static const struct regmap_irq bxtwc_regmap_irqs_level2[] = {
 	REGMAP_IRQ_REG(BXTWC_THRM2_IRQ, 2, 0xff),
 	REGMAP_IRQ_REG(BXTWC_BCU_IRQ, 3, 0x1f),
 	REGMAP_IRQ_REG(BXTWC_ADC_IRQ, 4, 0xff),
+	REGMAP_IRQ_REG(BXTWC_USBC_IRQ, 5, BIT(5)),
 	REGMAP_IRQ_REG(BXTWC_CHGR0_IRQ, 5, 0x1f),
 	REGMAP_IRQ_REG(BXTWC_CHGR1_IRQ, 6, 0x1f),
 	REGMAP_IRQ_REG(BXTWC_GPIO0_IRQ, 7, 0xff),
@@ -143,6 +145,10 @@ static struct resource adc_resources[] = {
 	DEFINE_RES_IRQ_NAMED(BXTWC_ADC_IRQ, "ADC"),
 };
 
+static struct resource usbc_resources[] = {
+	DEFINE_RES_IRQ(BXTWC_USBC_IRQ),
+};
+
 static struct resource charger_resources[] = {
 	DEFINE_RES_IRQ_NAMED(BXTWC_CHGR0_IRQ, "CHARGER"),
 	DEFINE_RES_IRQ_NAMED(BXTWC_CHGR1_IRQ, "CHARGER1"),
@@ -170,6 +176,11 @@ static struct mfd_cell bxt_wc_dev[] = {
 		.resources = thermal_resources,
 	},
 	{
+		.name = "bxt_wcove_usbc",
+		.num_resources = ARRAY_SIZE(usbc_resources),
+		.resources = usbc_resources,
+	},
+	{
 		.name = "bxt_wcove_ext_charger",
 		.num_resources = ARRAY_SIZE(charger_resources),
 		.resources = charger_resources,
-- 
2.8.1

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

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

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-08-22 14:10 +0200
Subject[PATCHv6 2/3] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY
Message-ID<s8Wdc-66j-29@gated-at.bofh.it>
In reply to#1467568
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 | 372 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 387 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..7116491
--- /dev/null
+++ b/drivers/usb/typec/typec_wcove.c
@@ -0,0 +1,372 @@
+/**
+ * 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 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;
+	}
+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;
+
+	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;
+
+	if (!acpi_check_dsm(ACPI_HANDLE(&pdev->dev), uuid.b, 0, 0x1f)) {
+		dev_err(&pdev->dev, "Missing _DSM functions\n");
+		return -ENODEV;
+	}
+
+	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);
+
+	/* 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");
+MODULE_ALIAS("platform:bxt_wcove_usbc");
-- 
2.8.1

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

#1470098 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-08-25 14:10 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<sa1DP-Mw-15@gated-at.bofh.it>
In reply to#1467568
Hi,

On Wed, Aug 24, 2016 at 04:08:23PM +0200, Vincent Palatin wrote:
> Sorry if I'm making redundant comments with previous discussions, I
> might have missed a few threads.
> 
> 
> On Mon, Aug 22, 2016 at 2:05 PM, Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> > The purpose of USB Type-C connector class is to provide
> > unified interface for the user space to get the status and
> > basic information about USB Type-C connectors on a system,
> > control over data role swapping, and when the port supports
> > USB Power Delivery, also control over power role swapping
> > and Alternate Modes.
> >
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  Documentation/ABI/testing/sysfs-class-typec |  199 +++++
> >  Documentation/usb/typec.txt                 |  103 +++
> >  MAINTAINERS                                 |    9 +
> >  drivers/usb/Kconfig                         |    2 +
> >  drivers/usb/Makefile                        |    2 +
> >  drivers/usb/typec/Kconfig                   |    7 +
> >  drivers/usb/typec/Makefile                  |    1 +
> >  drivers/usb/typec/typec.c                   | 1090 +++++++++++++++++++++++++++
> >  include/linux/usb/typec.h                   |  260 +++++++
> >  9 files changed, 1673 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 include/linux/usb/typec.h
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > new file mode 100644
> > index 0000000..e6179d3
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -0,0 +1,199 @@
> > +USB Type-C port devices (eg. /sys/class/typec/usbc0/)
> > +
> > +What:          /sys/class/typec/<port>/current_data_role
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               The current USB data role the port is operating in. This
> > +               attribute can be used for requesting data role swapping on the
> > +               port.
> 
> role swapping is sometimes a long operation. maybe we need to say
> explicitly whether the 'write' is synchronous and returns when the
> swap has succeeded / failed or asynchronous (and requires polling
> current_data_role afterwards to know the result ?)

OK.

> > +
> > +               Valid values:
> > +               - host
> > +               - device
> 
> the USB workgroup has settled for DFP/UFP rather than host/device ?

(I don't think the workgroup has settled on anything.)

I already proposed DFP/UFP, but it did not fly. The naming really does
not need to reflect the spec exactly, especially since there is no
guarantee they will not change the names in future versions of the
spec. "host/device", "source/sink" are completely understandable,
unlike DRP/UFP.

> 
> > +
> > +What:          /sys/class/typec/<port>/current_power_role
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               The current power role of the port. This attribute can be used
> > +               to request power role swap on the port when the port supports
> 
> ditto
> 
> 
> > +               USB Power Delivery.
> > +
> > +               Valid values:
> > +               - source
> > +               - sink
> > +
> > +What:          /sys/class/typec/<port>/current_vconn_role
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows the current VCONN role of the port. This attribute can be
> > +               used to request VCONN role swap on the port when the port
> > +               supports USB Power Delivery.
> > +
> > +               Valid values are:
> > +               - source
> > +               - sink
> 
> 
> either we are currently sourcing vconn or not, but even if you are
> not, you are probably not a vconn sink either (ie only vconn-powered
> accessory are, your usual linux-powered laptop/phone is probably not)

It's not relevant to know whether the vconn is being actually used or
not here. I'm not sure what's your point?

And vconn does not only supply vonn-powered accessories, but
powered cables as well.

> > +
> > +What:          /sys/class/typec/<port>/power_operation_mode
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows the current power operational mode the port is in.
> > +
> > +               Valid values:
> > +               - USB - Normal power levels defined in USB specifications
> > +               - BC1.2 - Power levels defined in Battery Charging Specification
> > +                         v1.2
> > +               - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C
> > +                                   specification.
> > +               - USB Type-C 3.0A - Higher 3A current defined in USB Type-C
> > +                                   specification.
> > +                - USB Power Delivery - The voltages and currents defined in USB
> > +                                      Power Delivery specification
> > +
> > +What:          /sys/class/typec/<port>/preferred_role
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               The user space can notify the driver about the preferred role.
> > +               It should be handled as enabling of Try.SRC or Try.SNK, as
> > +               defined in USB Type-C specification, in the port drivers. By
> > +               default there is no preferred role.
> > +
> > +               Valid values:
> > +               - host
> > +               - device
> > +               - For example "none" to remove preference (anything else except
> > +                 "host" or "device")
> 
> 
> host/device are not really power roles, source/sink are (or SNK/SRC)

Try.SRC/SNK are primarily designed for systems that don't implement
USB PD, and therefore source = host and sink = device. But even when
USB PD is supported, the initial data role will be host in case of
source and device in case of sink.

And as said before, the naming does not need to match that of the spec
here.

> > +
> > +What:          /sys/class/typec/<port>/supported_accessory_modes
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Lists the Accessory Modes, defined in the USB Type-C
> > +               specification, the port supports.
> 
> 
> there aren't many modes defined in the type-C spec (most of them are
> in other specs or proprietary) overall I don't really what modes my
> port supports (and the list might be open-ended, e.g. user space
> implementations), I'm really interested in what the partner port
> supports.

I think you are mixing Accessory Modes and Alternate Modes (most
likely because of the vconn-powered accessory concept). Note that
vconn-powered accessory is actually just a sink that implements an
alternate mode.

You can have vendor specific alternate modes, but the accessory modes
are what the spec defines, so Audio and Debug (and Digital Audio in
the future).

> 
> > +
> > +What:          /sys/class/typec/<port>/supported_data_roles
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Lists the USB data roles the port is capable of supporting.
> > +
> > +               Valid values:
> > +               - device
> > +               - host
> > +               - device, host (DRD as defined in USB Type-C specification v1.2)
> > +
> > +What:          /sys/class/typec/<port>/supported_power_roles
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Lists the power roles the port is capable of supporting.
> > +
> > +               Valid values:
> > +               - source
> > +               - sink
> > +
> > +What:          /sys/class/typec/<port>/supports_usb_power_delivery
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows if the port supports USB Power Delivery.
> > +               - 1 if USB Power Delivery is supported
> > +               - 0 when it's not
> > +
> > +
> > +USB Type-C partner devices (eg. /sys/class/typec/usbc0-partner/)
> > +
> > +What:          /sys/class/typec/<port>-partner/accessory
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               The attribute is visible only when the partner's type is
> > +               "Accessory". The type can be read from its own attribute.
> > +
> > +               Shows the name of the Accessory Mode. The Accessory Modes are
> > +               defined in USB Type-C Specification.
> > +
> > +What:          /sys/class/typec/<port>-partner/type
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows the type of the partner. Can be one of the following:
> > +               - USB - When the partner is normal USB host/peripheral.
> > +               - Charger - When the partner has been identified as dedicated
> > +                           charger.
> > +               - Alternate Mode - When the partner supports Alternate Modes.
> > +               - Accessory - When the partner is one of the accessories with
> > +                             specific Accessory Mode defined in USB Type-C
> > +                             specification.
> 
> 
> where a dock would be classified ?

A dock is just USB PD capable device with a bunch of alternate modes
that is attached to the port. There is no specific identifier for a
"dock".

> > +
> > +USB Type-C cable devices (eg. /sys/class/typec/usbc0-cable/)
> > +
> > +Note: Electronically Marked Cables will have a device also for one cable plug
> > +(eg. /sys/class/typec/usbc0-plug0). If the cable is active and has also SOP
> > +Double Prime controller (USB Power Deliver specification ch. 2.4) it will have
> > +second device also for the other plug. Both plugs may have their alternate modes
> > +as described in USB Type-C and USB Power Delivery specifications.
> > +
> > +What:          /sys/class/typec/<port>-cable/active
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows if the cable is active or passive.
> > +
> > +               Valid values:
> > +               - 0 when the cable is passive
> > +               - 1 when the cable is active
> > +
> > +What:          /sys/class/typec/<port>-cable/plug_type
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows type of the plug on the cable:
> > +               - Type-A - Standard A
> > +               - Type-B - Standard B
> > +               - Type-C - USB Type-C
> > +               - Captive - Non-standard
> > +
> > +
> > +Alternate Mode devices (For example,
> > +/sys/class/typec/usbc0-partner/usbc0-partner.svid:xxxx/). The ports, partners
> > +and cable plugs can have alternate modes.
> > +
> > +What:          /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/active
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows if the mode is active or not. The attribute can be used
> > +               for entering/exiting the mode with partners and cable plugs, and
> > +               with the port alternate modes it can be used for disabling
> > +               support for specific alternate modes.
> > +
> > +What:          /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/description
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows description of the mode. The description is optional for
> > +               the drivers, just like with the Billboard Devices.
> > +
> > +What:          /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/vdo
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows the VDO in hexadecimal returned from the Discover Modes
> > +               command.
> > +
> > +What:          /sys/class/typec/<port>/<port>.svid:<svid>/<mode>/supported_roles
> > +Date:          June 2016
> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > +Description:
> > +               Shows the roles, source or sink, the mode is supported with.
> > +
> > +               This attribute is available for the devices describing the
> > +               alternate modes a port supports, and it will not be exposed with
> > +               the devices presenting the alternate modes the partners or cable
> > +               plugs support.
> > diff --git a/Documentation/usb/typec.txt b/Documentation/usb/typec.txt
> > new file mode 100644
> > index 0000000..dce5f07
> > --- /dev/null
> > +++ b/Documentation/usb/typec.txt
> > @@ -0,0 +1,103 @@
> > +USB Type-C connector class
> > +==========================
> > +
> > +Introduction
> > +------------
> > +The typec class is meant for describing the USB Type-C ports in a system to the
> > +user space in unified fashion. The class is designed to provide nothing else
> > +except the user space interface implementation in hope that it can be utilized
> > +on as many platforms as possible.
> > +
> > +The platforms are expected to register every USB Type-C port they have with the
> > +class. In a normal case the registration will be done by a USB Type-C or PD PHY
> > +driver, but it may be a driver for firmware interface such as UCSI, driver for
> > +USB PD controller or even driver for Thunderbolt3 controller. This document
> > +considers the component registering the USB Type-C ports with the class as "port
> > +driver".
> > +
> > +On top of showing the capabilities, the class also offer the user space control
> > +over the roles and alternate modes they support when the port driver is capable
> > +of supporting those features.
> > +
> > +The class provides an API for the port drivers described in this document. The
> > +attributes are described in Documentation/ABI/testing/sysfs-class-typec.
> > +
> > +
> > +Interface
> > +---------
> > +Every port will be presented as its own device under /sys/class/typec/. The
> > +first port will be named "usbc0", the second "usbc1" and so on.
> 
> 
> I would need a way to map /sys/bus/usb/ entries with /sys/class/typec/ entries.

This needs planning, however this should not effect the class driver
at this point. The linking needs to happen in the drivers registering
the ports at least in the beginning. I fear there isn't a single
method that could be used on all types of platforms.

With ACPI we can find the port with the companion ACPI device for
the port itself. I don't know is this documented anywhere, but the
ACPI device object of the port is provided as a child for the typec
devices on boards that I've seen so far. I have no idea how the
linking even could be done in DT.

> > +
> > +When connected, the partner will be presented also as its own device under
> > +/sys/class/typec/. The parent of the partner device will always be the port. The
> > +partner attached to port "usbc0" will be named "usbc0-partner". Full patch to
> 
> s/patch/path/

OK.

<snip>

> > +/*
> > + * typec_altmode_update_active - Notify about Enter/Exit mode
> > + * @alt: Handle to the Alternate Mode
> > + * @mode: Mode id
> > + * @active: True when the mode has been enterred
> > + */
> > +void typec_altmode_update_active(struct typec_altmode *alt, int mode,
> > +                                bool active)
> > +{
> > +       struct typec_mode *m = alt->modes + mode;
> > +       char dir[6];
> > +
> > +       m->active = active;
> > +       sprintf(dir, "mode%d", mode);
> 
> 
> maybe we need a safety check here and verify that `mode` is a single
> digit or clip properly the string ?

Sure.

> 
> > +       sysfs_notify(&alt->dev.kobj, dir, "active");
> > +}
> > +EXPORT_SYMBOL(typec_altmode_update_active);


Thanks for the review,

-- 
heikki

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

#1470718 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromVincent Palatin <vpalatin@chromium.org>
Date2016-08-26 15:20 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<sapd7-7yi-19@gated-at.bofh.it>
In reply to#1470098
On Thu, Aug 25, 2016 at 1:59 PM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> Hi,
>
> On Wed, Aug 24, 2016 at 04:08:23PM +0200, Vincent Palatin wrote:
>> Sorry if I'm making redundant comments with previous discussions, I
>> might have missed a few threads.
>>
>>
>> On Mon, Aug 22, 2016 at 2:05 PM, Heikki Krogerus
>> <heikki.krogerus@linux.intel.com> wrote:
>> > The purpose of USB Type-C connector class is to provide
>> > unified interface for the user space to get the status and
>> > basic information about USB Type-C connectors on a system,
>> > control over data role swapping, and when the port supports
>> > USB Power Delivery, also control over power role swapping
>> > and Alternate Modes.
>> >
>> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > ---
>> >  Documentation/ABI/testing/sysfs-class-typec |  199 +++++
>> >  Documentation/usb/typec.txt                 |  103 +++
>> >  MAINTAINERS                                 |    9 +
>> >  drivers/usb/Kconfig                         |    2 +
>> >  drivers/usb/Makefile                        |    2 +
>> >  drivers/usb/typec/Kconfig                   |    7 +
>> >  drivers/usb/typec/Makefile                  |    1 +
>> >  drivers/usb/typec/typec.c                   | 1090 +++++++++++++++++++++++++++
>> >  include/linux/usb/typec.h                   |  260 +++++++
>> >  9 files changed, 1673 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 include/linux/usb/typec.h
>> >
>> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
>> > new file mode 100644
>> > index 0000000..e6179d3
>> > --- /dev/null
>> > +++ b/Documentation/ABI/testing/sysfs-class-typec
>> > @@ -0,0 +1,199 @@
>> > +USB Type-C port devices (eg. /sys/class/typec/usbc0/)
>> > +
>> > +What:          /sys/class/typec/<port>/current_data_role
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               The current USB data role the port is operating in. This
>> > +               attribute can be used for requesting data role swapping on the
>> > +               port.
>>
>> role swapping is sometimes a long operation. maybe we need to say
>> explicitly whether the 'write' is synchronous and returns when the
>> swap has succeeded / failed or asynchronous (and requires polling
>> current_data_role afterwards to know the result ?)
>
> OK.
>
>> > +
>> > +               Valid values:
>> > +               - host
>> > +               - device
>>
>> the USB workgroup has settled for DFP/UFP rather than host/device ?
>
> (I don't think the workgroup has settled on anything.)
>
> I already proposed DFP/UFP, but it did not fly. The naming really does
> not need to reflect the spec exactly, especially since there is no
> guarantee they will not change the names in future versions of the
> spec. "host/device", "source/sink" are completely understandable,
> unlike DRP/UFP.

OK.

>>
>> > +
>> > +What:          /sys/class/typec/<port>/current_power_role
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               The current power role of the port. This attribute can be used
>> > +               to request power role swap on the port when the port supports
>>
>> ditto
>>
>>
>> > +               USB Power Delivery.
>> > +
>> > +               Valid values:
>> > +               - source
>> > +               - sink
>> > +
>> > +What:          /sys/class/typec/<port>/current_vconn_role
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows the current VCONN role of the port. This attribute can be
>> > +               used to request VCONN role swap on the port when the port
>> > +               supports USB Power Delivery.
>> > +
>> > +               Valid values are:
>> > +               - source
>> > +               - sink
>>
>>
>> either we are currently sourcing vconn or not, but even if you are
>> not, you are probably not a vconn sink either (ie only vconn-powered
>> accessory are, your usual linux-powered laptop/phone is probably not)
>
> It's not relevant to know whether the vconn is being actually used or
> not here. I'm not sure what's your point?


My point was: saying we are a VCONN "sink" just because we are not
currently sourcing vconn is usually not true.


>
> And vconn does not only supply vonn-powered accessories, but
> powered cables as well.
>
>> > +
>> > +What:          /sys/class/typec/<port>/power_operation_mode
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows the current power operational mode the port is in.
>> > +
>> > +               Valid values:
>> > +               - USB - Normal power levels defined in USB specifications
>> > +               - BC1.2 - Power levels defined in Battery Charging Specification
>> > +                         v1.2
>> > +               - USB Type-C 1.5A - Higher 1.5A current defined in USB Type-C
>> > +                                   specification.
>> > +               - USB Type-C 3.0A - Higher 3A current defined in USB Type-C
>> > +                                   specification.
>> > +                - USB Power Delivery - The voltages and currents defined in USB
>> > +                                      Power Delivery specification
>> > +
>> > +What:          /sys/class/typec/<port>/preferred_role
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               The user space can notify the driver about the preferred role.
>> > +               It should be handled as enabling of Try.SRC or Try.SNK, as
>> > +               defined in USB Type-C specification, in the port drivers. By
>> > +               default there is no preferred role.
>> > +
>> > +               Valid values:
>> > +               - host
>> > +               - device
>> > +               - For example "none" to remove preference (anything else except
>> > +                 "host" or "device")
>>
>>
>> host/device are not really power roles, source/sink are (or SNK/SRC)
>
> Try.SRC/SNK are primarily designed for systems that don't implement
> USB PD, and therefore source = host and sink = device. But even when
> USB PD is supported, the initial data role will be host in case of
> source and device in case of sink.
>
> And as said before, the naming does not need to match that of the spec
> here.

sure it does not, I just found it confusing since we are referring
only to the power role.

>
>> > +
>> > +What:          /sys/class/typec/<port>/supported_accessory_modes
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Lists the Accessory Modes, defined in the USB Type-C
>> > +               specification, the port supports.
>>
>>
>> there aren't many modes defined in the type-C spec (most of them are
>> in other specs or proprietary) overall I don't really what modes my
>> port supports (and the list might be open-ended, e.g. user space
>> implementations), I'm really interested in what the partner port
>> supports.
>
> I think you are mixing Accessory Modes and Alternate Modes (most
> likely because of the vconn-powered accessory concept).

Indeed I was.

> Note that
> vconn-powered accessory is actually just a sink that implements an
> alternate mode.
>
> You can have vendor specific alternate modes, but the accessory modes
> are what the spec defines, so Audio and Debug (and Digital Audio in
> the future).
>
>>
>> > +
>> > +What:          /sys/class/typec/<port>/supported_data_roles
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Lists the USB data roles the port is capable of supporting.
>> > +
>> > +               Valid values:
>> > +               - device
>> > +               - host
>> > +               - device, host (DRD as defined in USB Type-C specification v1.2)
>> > +
>> > +What:          /sys/class/typec/<port>/supported_power_roles
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Lists the power roles the port is capable of supporting.
>> > +
>> > +               Valid values:
>> > +               - source
>> > +               - sink
>> > +
>> > +What:          /sys/class/typec/<port>/supports_usb_power_delivery
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows if the port supports USB Power Delivery.
>> > +               - 1 if USB Power Delivery is supported
>> > +               - 0 when it's not
>> > +
>> > +
>> > +USB Type-C partner devices (eg. /sys/class/typec/usbc0-partner/)
>> > +
>> > +What:          /sys/class/typec/<port>-partner/accessory
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               The attribute is visible only when the partner's type is
>> > +               "Accessory". The type can be read from its own attribute.
>> > +
>> > +               Shows the name of the Accessory Mode. The Accessory Modes are
>> > +               defined in USB Type-C Specification.
>> > +
>> > +What:          /sys/class/typec/<port>-partner/type
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows the type of the partner. Can be one of the following:
>> > +               - USB - When the partner is normal USB host/peripheral.
>> > +               - Charger - When the partner has been identified as dedicated
>> > +                           charger.
>> > +               - Alternate Mode - When the partner supports Alternate Modes.
>> > +               - Accessory - When the partner is one of the accessories with
>> > +                             specific Accessory Mode defined in USB Type-C
>> > +                             specification.
>>
>>
>> where a dock would be classified ?
>
> A dock is just USB PD capable device with a bunch of alternate modes
> that is attached to the port. There is no specific identifier for a
> "dock".

My remark was a bit too stern,
I meant a dock might be 'USB' 'Charger' 'Alternate Mode' , all at the
same time or alternately depending what you plug in.
I don't really see those types as mutually exclusive.


>
>> > +
>> > +USB Type-C cable devices (eg. /sys/class/typec/usbc0-cable/)
>> > +
>> > +Note: Electronically Marked Cables will have a device also for one cable plug
>> > +(eg. /sys/class/typec/usbc0-plug0). If the cable is active and has also SOP
>> > +Double Prime controller (USB Power Deliver specification ch. 2.4) it will have
>> > +second device also for the other plug. Both plugs may have their alternate modes
>> > +as described in USB Type-C and USB Power Delivery specifications.
>> > +
>> > +What:          /sys/class/typec/<port>-cable/active
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows if the cable is active or passive.
>> > +
>> > +               Valid values:
>> > +               - 0 when the cable is passive
>> > +               - 1 when the cable is active
>> > +
>> > +What:          /sys/class/typec/<port>-cable/plug_type
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows type of the plug on the cable:
>> > +               - Type-A - Standard A
>> > +               - Type-B - Standard B
>> > +               - Type-C - USB Type-C
>> > +               - Captive - Non-standard
>> > +
>> > +
>> > +Alternate Mode devices (For example,
>> > +/sys/class/typec/usbc0-partner/usbc0-partner.svid:xxxx/). The ports, partners
>> > +and cable plugs can have alternate modes.
>> > +
>> > +What:          /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/active
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows if the mode is active or not. The attribute can be used
>> > +               for entering/exiting the mode with partners and cable plugs, and
>> > +               with the port alternate modes it can be used for disabling
>> > +               support for specific alternate modes.
>> > +
>> > +What:          /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/description
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows description of the mode. The description is optional for
>> > +               the drivers, just like with the Billboard Devices.
>> > +
>> > +What:          /sys/class/typec/<dev>/<dev>.svid:<svid>/<mode>/vdo
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows the VDO in hexadecimal returned from the Discover Modes
>> > +               command.
>> > +
>> > +What:          /sys/class/typec/<port>/<port>.svid:<svid>/<mode>/supported_roles
>> > +Date:          June 2016
>> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > +Description:
>> > +               Shows the roles, source or sink, the mode is supported with.
>> > +
>> > +               This attribute is available for the devices describing the
>> > +               alternate modes a port supports, and it will not be exposed with
>> > +               the devices presenting the alternate modes the partners or cable
>> > +               plugs support.
>> > diff --git a/Documentation/usb/typec.txt b/Documentation/usb/typec.txt
>> > new file mode 100644
>> > index 0000000..dce5f07
>> > --- /dev/null
>> > +++ b/Documentation/usb/typec.txt
>> > @@ -0,0 +1,103 @@
>> > +USB Type-C connector class
>> > +==========================
>> > +
>> > +Introduction
>> > +------------
>> > +The typec class is meant for describing the USB Type-C ports in a system to the
>> > +user space in unified fashion. The class is designed to provide nothing else
>> > +except the user space interface implementation in hope that it can be utilized
>> > +on as many platforms as possible.
>> > +
>> > +The platforms are expected to register every USB Type-C port they have with the
>> > +class. In a normal case the registration will be done by a USB Type-C or PD PHY
>> > +driver, but it may be a driver for firmware interface such as UCSI, driver for
>> > +USB PD controller or even driver for Thunderbolt3 controller. This document
>> > +considers the component registering the USB Type-C ports with the class as "port
>> > +driver".
>> > +
>> > +On top of showing the capabilities, the class also offer the user space control
>> > +over the roles and alternate modes they support when the port driver is capable
>> > +of supporting those features.
>> > +
>> > +The class provides an API for the port drivers described in this document. The
>> > +attributes are described in Documentation/ABI/testing/sysfs-class-typec.
>> > +
>> > +
>> > +Interface
>> > +---------
>> > +Every port will be presented as its own device under /sys/class/typec/. The
>> > +first port will be named "usbc0", the second "usbc1" and so on.
>>
>>
>> I would need a way to map /sys/bus/usb/ entries with /sys/class/typec/ entries.
>
> This needs planning, however this should not effect the class driver
> at this point. The linking needs to happen in the drivers registering
> the ports at least in the beginning. I fear there isn't a single
> method that could be used on all types of platforms.
>
> With ACPI we can find the port with the companion ACPI device for
> the port itself. I don't know is this documented anywhere, but the
> ACPI device object of the port is provided as a child for the typec
> devices on boards that I've seen so far. I have no idea how the
> linking even could be done in DT.
>
>> > +
>> > +When connected, the partner will be presented also as its own device under
>> > +/sys/class/typec/. The parent of the partner device will always be the port. The
>> > +partner attached to port "usbc0" will be named "usbc0-partner". Full patch to
>>
>> s/patch/path/
>
> OK.
>
> <snip>
>
>> > +/*
>> > + * typec_altmode_update_active - Notify about Enter/Exit mode
>> > + * @alt: Handle to the Alternate Mode
>> > + * @mode: Mode id
>> > + * @active: True when the mode has been enterred
>> > + */
>> > +void typec_altmode_update_active(struct typec_altmode *alt, int mode,
>> > +                                bool active)
>> > +{
>> > +       struct typec_mode *m = alt->modes + mode;
>> > +       char dir[6];
>> > +
>> > +       m->active = active;
>> > +       sprintf(dir, "mode%d", mode);
>>
>>
>> maybe we need a safety check here and verify that `mode` is a single
>> digit or clip properly the string ?
>
> Sure.
>
>>
>> > +       sysfs_notify(&alt->dev.kobj, dir, "active");
>> > +}
>> > +EXPORT_SYMBOL(typec_altmode_update_active);
>
>
> Thanks for the review,
>
> --
> heikki

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

#1470738 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-08-26 16:20 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<saq9c-879-13@gated-at.bofh.it>
In reply to#1470718
Hi Vincent,

On Fri, Aug 26, 2016 at 03:16:16PM +0200, Vincent Palatin wrote:
> >> > +What:          /sys/class/typec/<port>/current_vconn_role
> >> > +Date:          June 2016
> >> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >> > +Description:
> >> > +               Shows the current VCONN role of the port. This attribute can be
> >> > +               used to request VCONN role swap on the port when the port
> >> > +               supports USB Power Delivery.
> >> > +
> >> > +               Valid values are:
> >> > +               - source
> >> > +               - sink
> >>
> >>
> >> either we are currently sourcing vconn or not, but even if you are
> >> not, you are probably not a vconn sink either (ie only vconn-powered
> >> accessory are, your usual linux-powered laptop/phone is probably not)
> >
> > It's not relevant to know whether the vconn is being actually used or
> > not here. I'm not sure what's your point?
> 
> 
> My point was: saying we are a VCONN "sink" just because we are not
> currently sourcing vconn is usually not true.

OK, I understand your point now. You are correct. I think we need to
change this attribute and call it "vconn_source" that reports "1" or
"0".

I'll change that and send one more version of these on Monday
(hopefully the last one) unless somebody disagrees.

> >> > +What:          /sys/class/typec/<port>-partner/type
> >> > +Date:          June 2016
> >> > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >> > +Description:
> >> > +               Shows the type of the partner. Can be one of the following:
> >> > +               - USB - When the partner is normal USB host/peripheral.
> >> > +               - Charger - When the partner has been identified as dedicated
> >> > +                           charger.
> >> > +               - Alternate Mode - When the partner supports Alternate Modes.
> >> > +               - Accessory - When the partner is one of the accessories with
> >> > +                             specific Accessory Mode defined in USB Type-C
> >> > +                             specification.
> >>
> >>
> >> where a dock would be classified ?
> >
> > A dock is just USB PD capable device with a bunch of alternate modes
> > that is attached to the port. There is no specific identifier for a
> > "dock".
> 
> My remark was a bit too stern,
> I meant a dock might be 'USB' 'Charger' 'Alternate Mode' , all at the
> same time or alternately depending what you plug in.
> I don't really see those types as mutually exclusive.

So USB type means the partner does not have alternate modes (I'll
clear that in the documentation), Charger is a dedicated charger and
therefore can not be anything else (no USB, no alternate modes).

To answer your original question, a dock would be reported as
Alternate Mode.


Thanks,

-- 
heikki

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

#1471821 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromGuenter Roeck <linux@roeck-us.net>
Date2016-08-29 15:10 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<sbuu5-7IC-21@gated-at.bofh.it>
In reply to#1470738
Heikki,

On 08/26/2016 07:07 AM, Heikki Krogerus wrote:
>
>>>>> +What:          /sys/class/typec/<port>-partner/type
>>>>> +Date:          June 2016
>>>>> +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>>>> +Description:
>>>>> +               Shows the type of the partner. Can be one of the following:
>>>>> +               - USB - When the partner is normal USB host/peripheral.
>>>>> +               - Charger - When the partner has been identified as dedicated
>>>>> +                           charger.
>>>>> +               - Alternate Mode - When the partner supports Alternate Modes.
>>>>> +               - Accessory - When the partner is one of the accessories with
>>>>> +                             specific Accessory Mode defined in USB Type-C
>>>>> +                             specification.
>>>>
>>>>
>>>> where a dock would be classified ?
>>>
>>> A dock is just USB PD capable device with a bunch of alternate modes
>>> that is attached to the port. There is no specific identifier for a
>>> "dock".
>>
>> My remark was a bit too stern,
>> I meant a dock might be 'USB' 'Charger' 'Alternate Mode' , all at the
>> same time or alternately depending what you plug in.
>> I don't really see those types as mutually exclusive.
>
> So USB type means the partner does not have alternate modes (I'll
> clear that in the documentation), Charger is a dedicated charger and
> therefore can not be anything else (no USB, no alternate modes).
>

This is probably the most difficult attribute to support.

Many PD capable chargers support alternate modes (for firmware upgrades).
As I mentioned earlier, it is difficult to match reported Type-C partner
types (or really anything reported in the SVDM Identity command)
to the above types.

Does it really make sense to deviate that much from the Type-C specification ?
I can understand why you hesitate to use DFP / UFP, as those terms are
really hard to understand for the non-initiated. However, here it is really
difficult to even determine which value to set. The best I can come up with is

- Not PD capable. Report USB (obviously includes non-PD capable chargers)
- PD capable, supports alternate modes. Report as Alternate Mode (including
   PD chargers supporting alternate modes)
- PD capable, does not support alternate modes. Report as Accessory if
   connected as accessory, as charger if we the port is connected as sink,
   USB otherwise

Overall this is quite vague and, especially for chargers, most of the time
misses the point.

I would really prefer if we could stay closer to the specification in this
case, and not try to merge multiple orthogonal attributes into one.

Thanks,
Guenter

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

#1471856 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-08-29 15:50 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<sbv6N-7Wm-13@gated-at.bofh.it>
In reply to#1471821
On Mon, Aug 29, 2016 at 06:04:52AM -0700, Guenter Roeck wrote:
> Heikki,
> 
> On 08/26/2016 07:07 AM, Heikki Krogerus wrote:
> > 
> > > > > > +What:          /sys/class/typec/<port>-partner/type
> > > > > > +Date:          June 2016
> > > > > > +Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > > > > +Description:
> > > > > > +               Shows the type of the partner. Can be one of the following:
> > > > > > +               - USB - When the partner is normal USB host/peripheral.
> > > > > > +               - Charger - When the partner has been identified as dedicated
> > > > > > +                           charger.
> > > > > > +               - Alternate Mode - When the partner supports Alternate Modes.
> > > > > > +               - Accessory - When the partner is one of the accessories with
> > > > > > +                             specific Accessory Mode defined in USB Type-C
> > > > > > +                             specification.
> > > > > 
> > > > > 
> > > > > where a dock would be classified ?
> > > > 
> > > > A dock is just USB PD capable device with a bunch of alternate modes
> > > > that is attached to the port. There is no specific identifier for a
> > > > "dock".
> > > 
> > > My remark was a bit too stern,
> > > I meant a dock might be 'USB' 'Charger' 'Alternate Mode' , all at the
> > > same time or alternately depending what you plug in.
> > > I don't really see those types as mutually exclusive.
> > 
> > So USB type means the partner does not have alternate modes (I'll
> > clear that in the documentation), Charger is a dedicated charger and
> > therefore can not be anything else (no USB, no alternate modes).
> > 
> 
> This is probably the most difficult attribute to support.
> 
> Many PD capable chargers support alternate modes (for firmware upgrades).
> As I mentioned earlier, it is difficult to match reported Type-C partner
> types (or really anything reported in the SVDM Identity command)
> to the above types.
> 
> Does it really make sense to deviate that much from the Type-C specification ?
> I can understand why you hesitate to use DFP / UFP, as those terms are
> really hard to understand for the non-initiated. However, here it is really
> difficult to even determine which value to set. The best I can come up with is
> 
> - Not PD capable. Report USB (obviously includes non-PD capable chargers)
> - PD capable, supports alternate modes. Report as Alternate Mode (including
>   PD chargers supporting alternate modes)
> - PD capable, does not support alternate modes. Report as Accessory if
>   connected as accessory, as charger if we the port is connected as sink,
>   USB otherwise
> 
> Overall this is quite vague and, especially for chargers, most of the time
> misses the point.
> 
> I would really prefer if we could stay closer to the specification in this
> case, and not try to merge multiple orthogonal attributes into one.

OK. So what would you propose?


Thanks,

-- 
heikki

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

#1471871 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-08-29 16:20 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<sbvzP-8mr-7@gated-at.bofh.it>
In reply to#1471856
Hi Guenter,

> > Overall this is quite vague and, especially for chargers, most of the time
> > misses the point.
> > 
> > I would really prefer if we could stay closer to the specification in this
> > case, and not try to merge multiple orthogonal attributes into one.
> 
> OK. So what would you propose?

I'm actually only conserned about the accessory case, as there we are
really not a source/sink/DRP, nor are we DPF/UFP/DRD. Should we use
this attribute to only express if the type of the partner is "normal"
or an accessory?


Thanks,

-- 
heikki

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

#1472096 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromGuenter Roeck <linux@roeck-us.net>
Date2016-08-29 22:10 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<sbB2y-3pN-19@gated-at.bofh.it>
In reply to#1471871
Hello Heikki,

On Mon, Aug 29, 2016 at 05:07:39PM +0300, Heikki Krogerus wrote:
> Hi Guenter,
> 
> > > Overall this is quite vague and, especially for chargers, most of the time
> > > misses the point.
> > > 
> > > I would really prefer if we could stay closer to the specification in this
> > > case, and not try to merge multiple orthogonal attributes into one.
> > 
> > OK. So what would you propose?
> 
> I'm actually only conserned about the accessory case, as there we are
> really not a source/sink/DRP, nor are we DPF/UFP/DRD. Should we use
> this attribute to only express if the type of the partner is "normal"
> or an accessory?
> 

We currently have three attributes to cover accessory modes.

supported_accessory_modes
	Lists the Accessory Modes, defined in the USB Type-C
	specification, the port supports.

	[ This is a bit vague. I think we should list the actual strings.
	  The modes are called "Audio Adapter Accessory Mode" and "Debug
	  Accessory Mode", yet the reported text is "Audio" and "Debug".
	  Also, "Digital Audio" isn't supported as of specification revision
	  1.2. So the strings doesn't exactly follow the specification. ]

accessory
	Shows the name of the Accessory Mode. The Accessory Modes are
	defined in USB Type-C Specification.

type
	Shows the type of the partner.

One of the possible accessory modes is TYPEC_ACCESSORY_NONE.

If you are only interested in accessory mode support, maybe we don't need
the 'type' attribute at all. We could make the 'accessory' attribute always
visible and display one of "none", "Audio", "Debug", or "Digital Audio".
It might also make sense to rename the attribute to "accessory_mode".

On a side note, while looking into this, I noticed the following:

+       if (port->cap->accessory)
+               for (accessory = port->cap->accessory, i = 0;
+                    i < port->cap->num_accessory; accessory++, i++)
+                       ret += sprintf(buf, "%s\n",
+                                      typec_accessory_modes[*accessory]);

This means the list of supported accessories always starts with ", ".

Thanks,
Guenter

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

#1472314 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromHeikki Krogerus <heikki.krogerus@linux.intel.com>
Date2016-08-30 10:30 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<sbMAG-2mW-27@gated-at.bofh.it>
In reply to#1472096
Hi Guenter,

On Mon, Aug 29, 2016 at 11:50:49AM -0700, Guenter Roeck wrote:
> Hello Heikki,
> 
> On Mon, Aug 29, 2016 at 05:07:39PM +0300, Heikki Krogerus wrote:
> > Hi Guenter,
> > 
> > > > Overall this is quite vague and, especially for chargers, most of the time
> > > > misses the point.
> > > > 
> > > > I would really prefer if we could stay closer to the specification in this
> > > > case, and not try to merge multiple orthogonal attributes into one.
> > > 
> > > OK. So what would you propose?
> > 
> > I'm actually only conserned about the accessory case, as there we are
> > really not a source/sink/DRP, nor are we DPF/UFP/DRD. Should we use
> > this attribute to only express if the type of the partner is "normal"
> > or an accessory?
> > 
> 
> We currently have three attributes to cover accessory modes.
> 
> supported_accessory_modes
> 	Lists the Accessory Modes, defined in the USB Type-C
> 	specification, the port supports.
> 
> 	[ This is a bit vague. I think we should list the actual strings.
> 	  The modes are called "Audio Adapter Accessory Mode" and "Debug
> 	  Accessory Mode", yet the reported text is "Audio" and "Debug".
> 	  Also, "Digital Audio" isn't supported as of specification revision
> 	  1.2. So the strings doesn't exactly follow the specification. ]

I'm fine if we want to use more precise strings.

> accessory
> 	Shows the name of the Accessory Mode. The Accessory Modes are
> 	defined in USB Type-C Specification.
> 
> type
> 	Shows the type of the partner.
> 
> One of the possible accessory modes is TYPEC_ACCESSORY_NONE.
> 
> If you are only interested in accessory mode support, maybe we don't need
> the 'type' attribute at all. We could make the 'accessory' attribute always
> visible and display one of "none", "Audio", "Debug", or "Digital Audio".
> It might also make sense to rename the attribute to "accessory_mode".

That works for me.

How about if I add the "supports_usb_power_delivery" attribute for the
partners instead to give some details about them. Any objections?

> On a side note, while looking into this, I noticed the following:
> 
> +       if (port->cap->accessory)
> +               for (accessory = port->cap->accessory, i = 0;
> +                    i < port->cap->num_accessory; accessory++, i++)
> +                       ret += sprintf(buf, "%s\n",
> +                                      typec_accessory_modes[*accessory]);
> 
> This means the list of supported accessories always starts with ", ".

Where does it print ", "?

I'm not sure what is wrong here, but I'll update this code in any
case. I'll change the accessory member in typec_capability into fixed
size array to make it easier to deal with for now.


Thanks,

-- 
heikki

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

#1472556 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromGuenter Roeck <linux@roeck-us.net>
Date2016-08-30 17:30 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<sbT97-6Di-15@gated-at.bofh.it>
In reply to#1472314
Hello Heikki,

On Tue, Aug 30, 2016 at 11:22:27AM +0300, Heikki Krogerus wrote:
> > 
> > If you are only interested in accessory mode support, maybe we don't need
> > the 'type' attribute at all. We could make the 'accessory' attribute always
> > visible and display one of "none", "Audio", "Debug", or "Digital Audio".
> > It might also make sense to rename the attribute to "accessory_mode".
> 
> That works for me.
> 
> How about if I add the "supports_usb_power_delivery" attribute for the
> partners instead to give some details about them. Any objections?
> 
At first glance, the attribute name looks a bit awkward. Let me look
into the specification to see what might make sense to report. On top of my
head, I don't recall if we are able to report this for a dock which isn't
currently connected to power.

> > On a side note, while looking into this, I noticed the following:
> > 
> > +       if (port->cap->accessory)
> > +               for (accessory = port->cap->accessory, i = 0;
> > +                    i < port->cap->num_accessory; accessory++, i++)
> > +                       ret += sprintf(buf, "%s\n",
> > +                                      typec_accessory_modes[*accessory]);
> > 
> > This means the list of supported accessories always starts with ", ".
> 
> Where does it print ", "?
> 
> I'm not sure what is wrong here, but I'll update this code in any

Nothing. Looks like I lost my ability to read code. Somehow the ',' above made
it into the string. There is some inconsistency in the output when compared to
the other "supported" attributes, though. Here the supported modes are printed
in consecutive lines; elsewhere they are printed in a single line with ',' as
separator.

Thanks,
Guenter

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

#1472656 — Re: [PATCHv6 1/3] usb: USB Type-C connector class

FromGuenter Roeck <linux@roeck-us.net>
Date2016-08-30 19:10 +0200
SubjectRe: [PATCHv6 1/3] usb: USB Type-C connector class
Message-ID<sbUHT-7Jg-21@gated-at.bofh.it>
In reply to#1472314
Heikki,

On Tue, Aug 30, 2016 at 11:22:27AM +0300, Heikki Krogerus wrote:
> 
> How about if I add the "supports_usb_power_delivery" attribute for the
> partners instead to give some details about them. Any objections?
> 
After looking into the code again, I assume the idea is to have the existing
supports_usb_power_delivery attribute report if the local port supports the
PD, and to have the partner attribute report if the partner supports the PD
protocol. In other words, it would report the value of usb_pd in struct
typec_partner.

If so, I am ok with it. You might actually consider adding the same attribute
to the cable attributes as well.

Thanks,
Guenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web