[PATCH v4 00/13] staging: typec: tcpci: move out of staging

[PATCH v4 00/13] staging: typec: tcpci: move out of staging

[Li Jun]

This patch set attempts to move the tcpci driver out of staging by fix
some tcpci driver issues and define typec and power delivery device
properties, the changes are verified on NXP PTN5110, which is a standard
tcpci typec port controller device with power delivery support, tested
power source and sink with drp config.

Changes for v4:
- Remove max-sink-* properties as we will purge max_snk_* in tcpm,
  see patch set[4].
- Get typec power and data type value via name string(patch 5).
- Move finding typec and pd properties code from tcpci to tcpm(patch 6)
- Add a compatible string for nxp ptn5110 typec controller in tcpci driver.
  (patch 3)
- Add set cc for drp toggling without try.src/snk in tcpm(patch 10), then
  patch 11 can only update CCx bits for keep disconnect cc line open.
- Update op-sink-microwatt-hours example value to be the right value in
  micorwatts, and accordingly divide 1000 to get its miliwatts value
  in patch 6.
- Add Guenter's Reviewed-by for patch(8/9/12)

[4] https://www.spinics.net/lists/linux-usb/msg167261.html

Changes for v3:
- Use 2 properties to separate power and data capability of typec port:
  "power-type" and "data-type", this is based on Heikki's typec class code
  change[2]. use "try-power-role" to present if the typec port can support
  Try.SNK or Try.SRC.
- 4 sink properties(max_sink_mv/ma/mw and op_sink_mw) are kept because the
  counterpart code is back, see revert patch[3], meanwhile I post a patch
  to fix the reported problem of current source pdo select machinism(which
  completely ignored those 4 sink settings), to see if we can keep current
  code, once it was discussed and have conclusion I can update this
  accordingly.
- Use fwnode to get the connector node for dt setting parse.

Main changes for v2:
- Typec properties are based on general usb connector bindings[1] proposed
  by Andrzej Hajda, use the standard unit suffixes as defined in
  property-units.txt.
- Add 2 infra APIs to get power sink and source config from dt.
- Don't change the set_cc api, to keep the uncontacted cc line open,
  set cc1/cc2 to be open in tcpci driver when set polarity.
- Directly enable vbus detect in tcpci driver rather than add a API.
- Details added in each patch.

[1] https://patchwork.kernel.org/patch/10231447/
[2] https://patchwork.kernel.org/patch/10276483/
[3] https://www.spinics.net/lists/linux-usb/msg166366.html

Li Jun (13):
  dt-bindings: connector: add properties for typec
  dt-bindings: usb: add documentation for typec port controller(TCPCI)
  staging: typec: tcpci: add compatible string for nxp ptn5110
  usb: typec: add fwnode to tcpc
  usb: typec: add API to get typec basic port power and data config
  usb: typec: tcpm: support get typec and pd config from device
    properties
  staging: typec: tcpci: register port before request irq
  staging: typec: tcpci: enable vbus detection
  typec: tcpm: add starting value for drp toggling
  usb: typec: tcpm: set cc for drp toggling attach
  staging: typec: tcpci: keep the not connecting cc line open
  staging: typec: tcpci: Only touch target bit when enable vconn
  staging: typec: tcpci: move tcpci driver out of staging

 .../bindings/connector/usb-connector.txt           |  39 ++
 .../devicetree/bindings/usb/typec-tcpci.txt        |  33 ++
 drivers/staging/Kconfig                            |   2 -
 drivers/staging/Makefile                           |   1 -
 drivers/staging/typec/Kconfig                      |  14 -
 drivers/staging/typec/Makefile                     |   1 -
 drivers/staging/typec/TODO                         |   5 -
 drivers/staging/typec/tcpci.c                      | 596 --------------------
 drivers/staging/typec/tcpci.h                      | 138 -----
 drivers/usb/typec/Kconfig                          |   7 +
 drivers/usb/typec/Makefile                         |   1 +
 drivers/usb/typec/class.c                          |  52 ++
 drivers/usb/typec/tcpci.c                          | 611 +++++++++++++++++++++
 drivers/usb/typec/tcpci.h                          | 138 +++++
 drivers/usb/typec/tcpm.c                           | 156 +++++-
 include/linux/usb/tcpm.h                           |   2 +
 include/linux/usb/typec.h                          |   3 +
 17 files changed, 1012 insertions(+), 787 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/typec-tcpci.txt
 delete mode 100644 drivers/staging/typec/Kconfig
 delete mode 100644 drivers/staging/typec/Makefile
 delete mode 100644 drivers/staging/typec/TODO
 delete mode 100644 drivers/staging/typec/tcpci.c
 delete mode 100644 drivers/staging/typec/tcpci.h
 create mode 100644 drivers/usb/typec/tcpci.c
 create mode 100644 drivers/usb/typec/tcpci.h

-- 
2.7.4

[PATCH v4 01/13] dt-bindings: connector: add properties for typec

[Li Jun]

Add bingdings supported by current typec driver, so user can pass
all those properties via dt.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 .../bindings/connector/usb-connector.txt           | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
index e1463f1..922f22b 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.txt
+++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
@@ -15,6 +15,29 @@ Optional properties:
 - type: size of the connector, should be specified in case of USB-A, USB-B
   non-fullsize connectors: "mini", "micro".
 
+Optional properties for usb-c-connector:
+- power-type: should be one of "source", "sink" or "dual"(DRP) if typec
+  connector has power support.
+- try-power-role: preferred power role if "dual"(DRP) can support Try.SNK
+  or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
+- data-type: should be one of "host", "device", "dual"(DRD) if typec
+  connector supports USB data.
+
+Required properties for usb-c-connector with power delivery support:
+- source-pdos: An array of u32 with each entry providing supported power
+  source data object(PDO), the detailed bit definitions of PDO can be found
+  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
+  Source_Capabilities Message, the order of each entry(PDO) should follow
+  the PD spec chapter 6.4.1. Required for power source and power dual role.
+- sink-pdos: An array of u32 with each entry providing supported power
+  sink data object(PDO), the detailed bit definitions of PDO can be found
+  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
+  Sink Capabilities Message, the order of each entry(PDO) should follow
+  the PD spec chapter 6.4.1. Required for power sink and power dual role.
+- op-sink-microwatt-hours: Sink required operating power in micro
+  watt-hours, if source offered power is less then it, Capability Mismatch
+  is set, required for power sink and power dual role.
+
 Required nodes:
 - any data bus to the connector should be modeled using the OF graph bindings
   specified in bindings/graph.txt, unless the bus is between parent node and
@@ -73,3 +96,19 @@ ccic: s2mm005@33 {
 		};
 	};
 };
+
+3. USB-C connector attached to a typec port controller(ptn5110), which has
+power delivery support and enables drp.
+
+typec: ptn5110@50 {
+	...
+	usb_con: connector {
+		compatible = "usb-c-connector";
+		label = "USB-C";
+		power-type = "dual";
+		try-power-role = "sink";
+		source-pdos = <0x380190c8>;
+		sink-pdos = <0x380190c8 0x3802d0c8>;
+		op-sink-microwatt-hours = <9000000>;
+	};
+};
-- 
2.7.4

[PATCH v4 02/13] dt-bindings: usb: add documentation for typec port controller(TCPCI)

[Li Jun]

TCPCI stands for typec port controller interface, its implementation
has full typec port control with power delivery support, it's a
standard i2c slave with GPIO input as irq interface, detail see spec
"Universal Serial Bus Type-C Port Controller Interface Specification
Revision 1.0, Version 1.1"

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 .../devicetree/bindings/usb/typec-tcpci.txt        | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
new file mode 100644
index 0000000..7a7a8e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
@@ -0,0 +1,33 @@
+TCPCI(Typec port cotroller interface) binding
+---------------------------------------------
+
+Required properties:
+- compatible:       should be "usb-tcpci,chip-specific-string".
+- reg:              the i2c slave address of typec port controller device.
+- interrupt-parent: the phandle to the interrupt controller which provides
+                    the interrupt.
+- interrupts:       interrupt specification for tcpci alert.
+
+Required sub-node:
+- connector: The "usb-c-connector" attached to the tcpci chip, the bindings
+  of connector node are specified in
+  Documentation/devicetree/bindings/connector/usb-connector.txt
+
+Example:
+
+ptn5110@50 {
+	compatible = "usb-tcpci,ptn5110";
+	reg = <0x50>;
+	interrupt-parent = <&gpio3>;
+	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+	usb_con: connector {
+		compatible = "usb-c-connector";
+		label = "USB-C";
+		port-type = "dual";
+		try-power-role = "sink"
+		source-pdos = <0x380190c8>;
+		sink-pdos = <0x380190c8 0x3802d0c8>;
+		op-sink-microwatt-hours = <9000000>;
+	};
+};
-- 
2.7.4

[PATCH v4 03/13] staging: typec: tcpci: add compatible string for nxp ptn5110

[Li Jun]

Add nxp ptn5110 typec controller compatible string: usb-tcpci,ptn5110,
which is a standard tcpci chip with power delivery support.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/staging/typec/tcpci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 076d97e..ed76327 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -576,6 +576,7 @@ MODULE_DEVICE_TABLE(i2c, tcpci_id);
 #ifdef CONFIG_OF
 static const struct of_device_id tcpci_of_match[] = {
 	{ .compatible = "usb,tcpci", },
+	{ .compatible = "usb-tcpci,ptn5110", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, tcpci_of_match);
-- 
2.7.4

[PATCH v4 04/13] usb: typec: add fwnode to tcpc

[Li Jun]

Add fwnode handle to get the fwnode so we can get typec configs
it contains.

Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/staging/typec/tcpci.c | 14 +++++++-------
 drivers/usb/typec/tcpm.c      |  1 +
 include/linux/usb/tcpm.h      |  2 ++
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index ed76327..4f7ad10 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/usb/pd.h>
 #include <linux/usb/tcpm.h>
@@ -463,17 +464,16 @@ static const struct regmap_config tcpci_regmap_config = {
 	.max_register = 0x7F, /* 0x80 .. 0xFF are vendor defined */
 };
 
-static const struct tcpc_config tcpci_tcpc_config = {
-	.type = TYPEC_PORT_DFP,
-	.default_role = TYPEC_SINK,
-};
-
 static int tcpci_parse_config(struct tcpci *tcpci)
 {
 	tcpci->controls_vbus = true; /* XXX */
 
-	/* TODO: Populate struct tcpc_config from ACPI/device-tree */
-	tcpci->tcpc.config = &tcpci_tcpc_config;
+	tcpci->tcpc.fwnode = device_get_named_child_node(tcpci->dev,
+							 "connector");
+	if (!tcpci->tcpc.fwnode) {
+		dev_err(tcpci->dev, "Can't find connector node.\n");
+		return -EINVAL;
+	}
 
 	return 0;
 }
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index a7b82d9..be9b009 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -3709,6 +3709,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	else
 		port->try_role = TYPEC_NO_PREFERRED_ROLE;
 
+	port->typec_caps.fwnode = tcpc->fwnode;
 	port->typec_caps.prefer_role = tcpc->config->default_role;
 	port->typec_caps.type = tcpc->config->type;
 	port->typec_caps.data = tcpc->config->data;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index f0d839d..e2e2db3 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -115,6 +115,7 @@ enum tcpc_mux_mode {
 /**
  * struct tcpc_dev - Port configuration and callback functions
  * @config:	Pointer to port configuration
+ * @fwnode:	Pointer to port fwnode
  * @get_vbus:	Called to read current VBUS state
  * @get_current_limit:
  *		Optional; called by the tcpm core when configured as a snk
@@ -143,6 +144,7 @@ enum tcpc_mux_mode {
  */
 struct tcpc_dev {
 	const struct tcpc_config *config;
+	struct fwnode_handle *fwnode;
 
 	int (*init)(struct tcpc_dev *dev);
 	int (*get_vbus)(struct tcpc_dev *dev);
-- 
2.7.4

[PATCH v4 05/13] usb: typec: add API to get typec basic port power and data config

[Li Jun]

This patch adds 3 APIs to get the typec port power and data type,
and preferred power role by its name string.

Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/class.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/typec.h |  3 +++
 2 files changed, 55 insertions(+)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 53df10d..5981e18 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -9,6 +9,7 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/usb/typec.h>
 #include <linux/usb/typec_mux.h>
@@ -802,6 +803,12 @@ static const char * const typec_port_types[] = {
 	[TYPEC_PORT_DRP] = "dual",
 };
 
+static const char * const typec_data_types[] = {
+	[TYPEC_PORT_DFP] = "host",
+	[TYPEC_PORT_UFP] = "device",
+	[TYPEC_PORT_DRD] = "dual",
+};
+
 static const char * const typec_port_types_drp[] = {
 	[TYPEC_PORT_SRC] = "dual [source] sink",
 	[TYPEC_PORT_SNK] = "dual source [sink]",
@@ -1252,6 +1259,51 @@ void typec_set_pwr_opmode(struct typec_port *port,
 }
 EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
 
+/**
+ * typec_find_power_type - Get the typec port power type
+ * @name: port type string
+ *
+ * This routine is used to find the typec_port_type by its string name.
+ *
+ * Returns typec_port_type if success, otherwise negative error code.
+ */
+int typec_find_power_type(const char *name)
+{
+	return match_string(typec_port_types, ARRAY_SIZE(typec_port_types),
+			    name);
+}
+EXPORT_SYMBOL_GPL(typec_find_power_type);
+
+/**
+ * typec_find_preferred_role - Find the typec drp port preferred power role
+ * @name: power role string
+ *
+ * This routine is used to find the typec_role by its string name of
+ * preferred power role(Try.SRC or Try.SNK).
+ *
+ * Returns typec_role if success, otherwise negative error code.
+ */
+int typec_find_preferred_role(const char *name)
+{
+	return match_string(typec_roles, ARRAY_SIZE(typec_roles), name);
+}
+EXPORT_SYMBOL_GPL(typec_find_preferred_role);
+
+/**
+ * typec_find_data_type - Get the typec port data capability
+ * @name: data type string
+ *
+ * This routine is used to find the typec_port_data by its string name.
+ *
+ * Returns typec_port_data if success, otherwise negative error code.
+ */
+int typec_find_data_type(const char *name)
+{
+	return match_string(typec_data_types, ARRAY_SIZE(typec_data_types),
+			    name);
+}
+EXPORT_SYMBOL_GPL(typec_find_data_type);
+
 /* ------------------------------------------ */
 /* API for Multiplexer/DeMultiplexer Switches */
 
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 672b39b..00c93e7 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -267,4 +267,7 @@ int typec_set_orientation(struct typec_port *port,
 			  enum typec_orientation orientation);
 int typec_set_mode(struct typec_port *port, int mode);
 
+int typec_find_power_type(const char *name);
+int typec_find_preferred_role(const char *name);
+int typec_find_data_type(const char *name);
 #endif /* __LINUX_USB_TYPEC_H */
-- 
2.7.4

[PATCH v4 06/13] usb: typec: tcpm: support get typec and pd config from device properties

[Li Jun]

This patch adds support of get typec and power delivery config from
firmware description.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/tcpm.c | 140 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 115 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index be9b009..1744041b 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/proc_fs.h>
+#include <linux/property.h>
 #include <linux/sched/clock.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
@@ -3569,6 +3570,82 @@ static int tcpm_copy_vdos(u32 *dest_vdo, const u32 *src_vdo,
 	return nr_vdo;
 }
 
+static int tcpm_fw_get_caps(struct tcpm_port *port,
+			    struct fwnode_handle *fwnode)
+{
+	const char *cap_str;
+	int ret;
+	u32 mw;
+
+	if (!port || !fwnode)
+		return -EINVAL;
+
+	/* USB data support is optional */
+	ret = fwnode_property_read_string(fwnode, "data-type", &cap_str);
+	if (ret == 0) {
+		port->typec_caps.data = typec_find_data_type(cap_str);
+		if (port->typec_caps.data < 0)
+			return -EINVAL;
+	}
+
+	ret = fwnode_property_read_string(fwnode, "power-type", &cap_str);
+	if (ret < 0)
+		return ret;
+
+	port->typec_caps.type = typec_find_power_type(cap_str);
+	if (port->typec_caps.type < 0)
+		return -EINVAL;
+	port->port_type = port->typec_caps.type;
+
+	if (port->port_type == TYPEC_PORT_SNK)
+		goto sink;
+
+	/* Get soruce pdos */
+	ret = fwnode_property_read_u32_array(fwnode, "source-pdos",
+					     NULL, 0);
+	if (ret <= 0)
+		return -EINVAL;
+
+	port->nr_src_pdo = min(ret, PDO_MAX_OBJECTS);
+	ret = fwnode_property_read_u32_array(fwnode, "source-pdos",
+					     port->src_pdo, port->nr_src_pdo);
+	if ((ret < 0) || tcpm_validate_caps(port, port->src_pdo,
+					    port->nr_src_pdo))
+		return -EINVAL;
+
+	if (port->port_type == TYPEC_PORT_SRC)
+		return 0;
+
+	/* Get the preferred power role for DRP */
+	ret = fwnode_property_read_string(fwnode, "try-power-role", &cap_str);
+	if (ret < 0)
+		return ret;
+
+	port->typec_caps.prefer_role = typec_find_preferred_role(cap_str);
+	if (port->typec_caps.prefer_role < 0)
+		return -EINVAL;
+sink:
+	/* Get sink pdos */
+	ret = fwnode_property_read_u32_array(fwnode, "sink-pdos",
+					     NULL, 0);
+	if (ret <= 0)
+		return -EINVAL;
+
+	port->nr_snk_pdo = min(ret, PDO_MAX_OBJECTS);
+	ret = fwnode_property_read_u32_array(fwnode, "sink-pdos",
+					     port->snk_pdo, port->nr_snk_pdo);
+	if ((ret < 0) || tcpm_validate_caps(port, port->snk_pdo,
+					    port->nr_snk_pdo))
+		return -EINVAL;
+
+	if (fwnode_property_read_u32(fwnode, "op-sink-microwatt-hours",
+				     &mw) < 0)
+		return -EINVAL;
+	port->operating_snk_mw = mw / 1000;
+
+	return 0;
+}
+
 int tcpm_update_source_capabilities(struct tcpm_port *port, const u32 *pdo,
 				    unsigned int nr_pdo)
 {
@@ -3643,12 +3720,39 @@ static int nr_type_pdos(const u32 *pdo, unsigned int nr_pdo,
 	return count;
 }
 
+static int tcpm_copy_caps(struct tcpm_port *port,
+			  const struct tcpc_config *tcfg)
+{
+	if (tcpm_validate_caps(port, tcfg->src_pdo, tcfg->nr_src_pdo) ||
+	    tcpm_validate_caps(port, tcfg->snk_pdo, tcfg->nr_snk_pdo))
+		return -EINVAL;
+
+	port->nr_src_pdo = tcpm_copy_pdos(port->src_pdo, tcfg->src_pdo,
+					  tcfg->nr_src_pdo);
+	port->nr_snk_pdo = tcpm_copy_pdos(port->snk_pdo, tcfg->snk_pdo,
+					  tcfg->nr_snk_pdo);
+
+	port->nr_snk_vdo = tcpm_copy_vdos(port->snk_vdo, tcfg->snk_vdo,
+					  tcfg->nr_snk_vdo);
+
+	port->max_snk_mv = tcfg->max_snk_mv;
+	port->max_snk_ma = tcfg->max_snk_ma;
+	port->max_snk_mw = tcfg->max_snk_mw;
+	port->operating_snk_mw = tcfg->operating_snk_mw;
+
+	port->typec_caps.prefer_role = tcfg->default_role;
+	port->typec_caps.type = tcfg->type;
+	port->typec_caps.data = tcfg->data;
+
+	return 0;
+}
+
 struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 {
 	struct tcpm_port *port;
 	int i, err;
 
-	if (!dev || !tcpc || !tcpc->config ||
+	if (!dev || !tcpc ||
 	    !tcpc->get_vbus || !tcpc->set_cc || !tcpc->get_cc ||
 	    !tcpc->set_polarity || !tcpc->set_vconn || !tcpc->set_vbus ||
 	    !tcpc->set_pd_rx || !tcpc->set_roles || !tcpc->pd_transmit)
@@ -3677,17 +3781,13 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	init_completion(&port->swap_complete);
 	tcpm_debugfs_init(port);
 
-	if (tcpm_validate_caps(port, tcpc->config->src_pdo,
-			       tcpc->config->nr_src_pdo) ||
-	    tcpm_validate_caps(port, tcpc->config->snk_pdo,
-			       tcpc->config->nr_snk_pdo)) {
-		err = -EINVAL;
+	if (tcpc->config)
+		err = tcpm_copy_caps(port, tcpc->config);
+	else
+		err = tcpm_fw_get_caps(port, tcpc->fwnode);
+	if (err < 0)
 		goto out_destroy_wq;
-	}
-	port->nr_src_pdo = tcpm_copy_pdos(port->src_pdo, tcpc->config->src_pdo,
-					  tcpc->config->nr_src_pdo);
-	port->nr_snk_pdo = tcpm_copy_pdos(port->snk_pdo, tcpc->config->snk_pdo,
-					  tcpc->config->nr_snk_pdo);
+
 	port->nr_fixed =  nr_type_pdos(port->snk_pdo,
 				       port->nr_snk_pdo,
 				       PDO_TYPE_FIXED);
@@ -3697,22 +3797,12 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	port->nr_batt = nr_type_pdos(port->snk_pdo,
 				     port->nr_snk_pdo,
 				     PDO_TYPE_BATT);
-	port->nr_snk_vdo = tcpm_copy_vdos(port->snk_vdo, tcpc->config->snk_vdo,
-					  tcpc->config->nr_snk_vdo);
-
-	port->max_snk_mv = tcpc->config->max_snk_mv;
-	port->max_snk_ma = tcpc->config->max_snk_ma;
-	port->max_snk_mw = tcpc->config->max_snk_mw;
-	port->operating_snk_mw = tcpc->config->operating_snk_mw;
-	if (!tcpc->config->try_role_hw)
-		port->try_role = tcpc->config->default_role;
+	if (!tcpc->config || !tcpc->config->try_role_hw)
+		port->try_role = port->typec_caps.prefer_role;
 	else
 		port->try_role = TYPEC_NO_PREFERRED_ROLE;
 
 	port->typec_caps.fwnode = tcpc->fwnode;
-	port->typec_caps.prefer_role = tcpc->config->default_role;
-	port->typec_caps.type = tcpc->config->type;
-	port->typec_caps.data = tcpc->config->data;
 	port->typec_caps.revision = 0x0120;	/* Type-C spec release 1.2 */
 	port->typec_caps.pd_revision = 0x0200;	/* USB-PD spec release 2.0 */
 	port->typec_caps.dr_set = tcpm_dr_set;
@@ -3722,7 +3812,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	port->typec_caps.port_type_set = tcpm_port_type_set;
 
 	port->partner_desc.identity = &port->partner_ident;
-	port->port_type = tcpc->config->type;
+	port->port_type = port->typec_caps.type;
 
 	port->role_sw = usb_role_switch_get(port->dev);
 	if (IS_ERR(port->role_sw)) {
@@ -3736,7 +3826,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 		goto out_destroy_wq;
 	}
 
-	if (tcpc->config->alt_modes) {
+	if (tcpc->config && tcpc->config->alt_modes) {
 		const struct typec_altmode_desc *paltmode = tcpc->config->alt_modes;
 
 		i = 0;
-- 
2.7.4

[PATCH v4 07/13] staging: typec: tcpci: register port before request irq

[Li Jun]

With that we can clear any pending events and the port is registered
so driver can be ready to handle typec events once we request irq.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/staging/typec/tcpci.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 4f7ad10..9e0014b 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -537,25 +537,26 @@ static int tcpci_probe(struct i2c_client *client,
 	if (IS_ERR(chip->data.regmap))
 		return PTR_ERR(chip->data.regmap);
 
+	i2c_set_clientdata(client, chip);
+
 	/* Disable chip interrupts before requesting irq */
 	err = regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val,
 			       sizeof(u16));
 	if (err < 0)
 		return err;
 
+	chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
+	if (PTR_ERR_OR_ZERO(chip->tcpci))
+		return PTR_ERR(chip->tcpci);
+
 	err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
 					_tcpci_irq,
 					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
 					dev_name(&client->dev), chip);
 	if (err < 0)
-		return err;
+		tcpci_unregister_port(chip->tcpci);
 
-	chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
-	if (PTR_ERR_OR_ZERO(chip->tcpci))
-		return PTR_ERR(chip->tcpci);
-
-	i2c_set_clientdata(client, chip);
-	return 0;
+	return err;
 }
 
 static int tcpci_remove(struct i2c_client *client)
-- 
2.7.4

[PATCH v4 08/13] staging: typec: tcpci: enable vbus detection

[Li Jun]

TCPCI implementation may need SW to enable VBUS detection to generate
power status events.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/staging/typec/tcpci.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 9e0014b..d5b4e4e 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -373,6 +373,12 @@ static int tcpci_init(struct tcpc_dev *tcpc)
 	if (ret < 0)
 		return ret;
 
+	/* Enable Vbus detection */
+	ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
+			   TCPC_CMD_ENABLE_VBUS_DETECT);
+	if (ret < 0)
+		return ret;
+
 	reg = TCPC_ALERT_TX_SUCCESS | TCPC_ALERT_TX_FAILED |
 		TCPC_ALERT_TX_DISCARDED | TCPC_ALERT_RX_STATUS |
 		TCPC_ALERT_RX_HARD_RST | TCPC_ALERT_CC_STATUS;
-- 
2.7.4

[PATCH v4 09/13] typec: tcpm: add starting value for drp toggling

[Li Jun]

As DRP port autonomously toggles the Rp/Rd need a start value to
begin with, so add one parameter for it in tcpm_start_drp_toggling.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/tcpm.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 1744041b..218c230 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1964,15 +1964,15 @@ static int tcpm_set_charge(struct tcpm_port *port, bool charge)
 	return 0;
 }
 
-static bool tcpm_start_drp_toggling(struct tcpm_port *port)
+static bool tcpm_start_drp_toggling(struct tcpm_port *port,
+				    enum typec_cc_status cc)
 {
 	int ret;
 
 	if (port->tcpc->start_drp_toggling &&
 	    port->port_type == TYPEC_PORT_DRP) {
 		tcpm_log_force(port, "Start DRP toggling");
-		ret = port->tcpc->start_drp_toggling(port->tcpc,
-						     tcpm_rp_cc(port));
+		ret = port->tcpc->start_drp_toggling(port->tcpc, cc);
 		if (!ret)
 			return true;
 	}
@@ -2282,7 +2282,7 @@ static void run_state_machine(struct tcpm_port *port)
 		if (!port->non_pd_role_swap)
 			tcpm_swap_complete(port, -ENOTCONN);
 		tcpm_src_detach(port);
-		if (tcpm_start_drp_toggling(port)) {
+		if (tcpm_start_drp_toggling(port, tcpm_rp_cc(port))) {
 			tcpm_set_state(port, DRP_TOGGLING, 0);
 			break;
 		}
@@ -2454,7 +2454,7 @@ static void run_state_machine(struct tcpm_port *port)
 		if (!port->non_pd_role_swap)
 			tcpm_swap_complete(port, -ENOTCONN);
 		tcpm_snk_detach(port);
-		if (tcpm_start_drp_toggling(port)) {
+		if (tcpm_start_drp_toggling(port, TYPEC_CC_RD)) {
 			tcpm_set_state(port, DRP_TOGGLING, 0);
 			break;
 		}
-- 
2.7.4

[PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach

[Li Jun]

In case of drp toggling, we may need set correct cc value for role control
after attach as it may never been set.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/tcpm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 218c230..72d4232 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
 	tcpm_set_attached_state(port, false);
 	port->try_src_count = 0;
 	port->try_snk_count = 0;
+	port->cc_req = 0;
 }
 
 static void tcpm_detach(struct tcpm_port *port)
@@ -2361,6 +2362,8 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 
 	case SRC_ATTACHED:
+		if (!port->cc_req)
+			tcpm_set_cc(port, tcpm_rp_cc(port));
 		ret = tcpm_src_attach(port);
 		tcpm_set_state(port, SRC_UNATTACHED,
 			       ret < 0 ? 0 : PD_T_PS_SOURCE_ON);
@@ -2531,6 +2534,8 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
 		break;
 	case SNK_ATTACHED:
+		if (!port->cc_req)
+			tcpm_set_cc(port, TYPEC_CC_RD);
 		ret = tcpm_snk_attach(port);
 		if (ret < 0)
 			tcpm_set_state(port, SNK_UNATTACHED, 0);
-- 
2.7.4

[PATCH v4 11/13] staging: typec: tcpci: keep the not connecting cc line open

[Li Jun]

While set polarity, we should keep the not connecting cc line to be
open.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/staging/typec/tcpci.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index d5b4e4e..b58bd59 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -185,15 +185,25 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
 			      enum typec_cc_polarity polarity)
 {
 	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+	unsigned int reg;
 	int ret;
 
-	ret = regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
-			   (polarity == TYPEC_POLARITY_CC2) ?
-			   TCPC_TCPC_CTRL_ORIENTATION : 0);
+	/* Keep the disconnect cc line open */
+	ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, &reg);
 	if (ret < 0)
 		return ret;
 
-	return 0;
+	if (polarity == TYPEC_POLARITY_CC2)
+		reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT;
+	else
+		reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT;
+	ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
+	if (ret < 0)
+		return ret;
+
+	return regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
+			   (polarity == TYPEC_POLARITY_CC2) ?
+			   TCPC_TCPC_CTRL_ORIENTATION : 0);
 }
 
 static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
-- 
2.7.4

[PATCH v4 12/13] staging: typec: tcpci: Only touch target bit when enable vconn

[Li Jun]

We need regmap_update_bits to avoid touch any other bits when
enable or disable vconn.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/staging/typec/tcpci.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index b58bd59..0d0c971 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -218,12 +218,9 @@ static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
 			return ret;
 	}
 
-	ret = regmap_write(tcpci->regmap, TCPC_POWER_CTRL,
-			   enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	return regmap_update_bits(tcpci->regmap, TCPC_POWER_CTRL,
+				TCPC_POWER_CTRL_VCONN_ENABLE,
+				enable ? TCPC_POWER_CTRL_VCONN_ENABLE : 0);
 }
 
 static int tcpci_set_roles(struct tcpc_dev *tcpc, bool attached,
-- 
2.7.4

[PATCH v4 13/13] staging: typec: tcpci: move tcpci driver out of staging

[Li Jun]

Move TCPCI(Typec port controller interface) driver out of staging.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/staging/Kconfig                |  2 --
 drivers/staging/Makefile               |  1 -
 drivers/staging/typec/Kconfig          | 14 --------------
 drivers/staging/typec/Makefile         |  1 -
 drivers/staging/typec/TODO             |  5 -----
 drivers/usb/typec/Kconfig              |  7 +++++++
 drivers/usb/typec/Makefile             |  1 +
 drivers/{staging => usb}/typec/tcpci.c |  0
 drivers/{staging => usb}/typec/tcpci.h |  0
 9 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index e95ab68..751937d 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -116,8 +116,6 @@ source "drivers/staging/vc04_services/Kconfig"
 
 source "drivers/staging/ccree/Kconfig"
 
-source "drivers/staging/typec/Kconfig"
-
 source "drivers/staging/vboxvideo/Kconfig"
 
 source "drivers/staging/pi433/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index af8cd6a..039693e 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -2,7 +2,6 @@
 # Makefile for staging directory
 
 obj-y				+= media/
-obj-y				+= typec/
 obj-$(CONFIG_IPX)		+= ipx/
 obj-$(CONFIG_NCP_FS)		+= ncpfs/
 obj-$(CONFIG_IRDA)		+= irda/net/
diff --git a/drivers/staging/typec/Kconfig b/drivers/staging/typec/Kconfig
deleted file mode 100644
index 5359f55..0000000
--- a/drivers/staging/typec/Kconfig
+++ /dev/null
@@ -1,14 +0,0 @@
-menu "USB Power Delivery and Type-C drivers"
-
-if TYPEC_TCPM
-
-config TYPEC_TCPCI
-	tristate "Type-C Port Controller Interface driver"
-	depends on I2C
-	select REGMAP_I2C
-	help
-	  Type-C Port Controller driver for TCPCI-compliant controller.
-
-endif
-
-endmenu
diff --git a/drivers/staging/typec/Makefile b/drivers/staging/typec/Makefile
deleted file mode 100644
index 53d649a..0000000
--- a/drivers/staging/typec/Makefile
+++ /dev/null
@@ -1 +0,0 @@
-obj-$(CONFIG_TYPEC_TCPCI)	+= tcpci.o
diff --git a/drivers/staging/typec/TODO b/drivers/staging/typec/TODO
deleted file mode 100644
index 53fe2f7..0000000
--- a/drivers/staging/typec/TODO
+++ /dev/null
@@ -1,5 +0,0 @@
-tcpci:
-- Test with real hardware
-
-Please send patches to Guenter Roeck <linux@roeck-us.net> and copy
-Heikki Krogerus <heikki.krogerus@linux.intel.com>.
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 030f88c..2b8d3c2 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -55,6 +55,13 @@ config TYPEC_TCPM
 
 if TYPEC_TCPM
 
+config TYPEC_TCPCI
+	tristate "Type-C Port Controller Interface driver"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Type-C Port Controller driver for TCPCI-compliant controller.
+
 source "drivers/usb/typec/fusb302/Kconfig"
 
 config TYPEC_WCOVE
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 1f599a6..02758a1 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_TYPEC_WCOVE)	+= typec_wcove.o
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)	+= tps6598x.o
 obj-$(CONFIG_TYPEC)		+= mux/
+obj-$(CONFIG_TYPEC_TCPCI)	+= tcpci.o
diff --git a/drivers/staging/typec/tcpci.c b/drivers/usb/typec/tcpci.c
similarity index 100%
rename from drivers/staging/typec/tcpci.c
rename to drivers/usb/typec/tcpci.c
diff --git a/drivers/staging/typec/tcpci.h b/drivers/usb/typec/tcpci.h
similarity index 100%
rename from drivers/staging/typec/tcpci.h
rename to drivers/usb/typec/tcpci.h
-- 
2.7.4

Re: [PATCH v4 07/13] staging: typec: tcpci: register port before request irq

[Dan Carpenter]

On Thu, Mar 29, 2018 at 12:06:12AM +0800, Li Jun wrote:
> With that we can clear any pending events and the port is registered
> so driver can be ready to handle typec events once we request irq.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> Signed-off-by: Li Jun <jun.li@nxp.com>

These sign offs aren't clear.

Sign offs mean that you handled the patch but didn't include any of
SCO's copyrighted UNIX code into it.  Normally they're in the order of
who touched the code.  So Peter touched the code first.  Should he get
authorship credit?  How did he touch the code first if he didn't write
the code?  It doesn't make sense.

> ---
>  drivers/staging/typec/tcpci.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index 4f7ad10..9e0014b 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -537,25 +537,26 @@ static int tcpci_probe(struct i2c_client *client,
>  	if (IS_ERR(chip->data.regmap))
>  		return PTR_ERR(chip->data.regmap);
>  
> +	i2c_set_clientdata(client, chip);
> +
>  	/* Disable chip interrupts before requesting irq */
>  	err = regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val,
>  			       sizeof(u16));
>  	if (err < 0)
>  		return err;
>  
> +	chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> +	if (PTR_ERR_OR_ZERO(chip->tcpci))
> +		return PTR_ERR(chip->tcpci);

When a function returns both error pointers and NULL that means that
NULL is a secial case of success.  Like for example:

	p->my_feature = get_optional_feature();

If it returns NULL that means the optional feature isn't there, but it's
fine because it's optional.  But if it returns an error pointer that
means the feature is there but the hardware is buggy or something so
we shouldn't continue.

If you return PTR_ERR(NULL) that means success.

I don't think this code makes sense just from looking at it and also
when I checked tcpci_register_port() doesn't return NULL.



> +
>  	err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>  					_tcpci_irq,
>  					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
>  					dev_name(&client->dev), chip);
>  	if (err < 0)
> -		return err;
> +		tcpci_unregister_port(chip->tcpci);

Can you put the "return err;" back, because that's better style.  It's
better to keep the error path and success path separate if you can.

	if (err < 0) {
		tcpci_unregister_port(chip->tcpci);
		return err;
	}

	return 0;


regards,
dan carpenter

Re: [PATCH v4 04/13] usb: typec: add fwnode to tcpc

[Heikki Krogerus]

Hi,

On Thu, Mar 29, 2018 at 12:06:09AM +0800, Li Jun wrote:
> Add fwnode handle to get the fwnode so we can get typec configs
> it contains.
> 
> Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  drivers/staging/typec/tcpci.c | 14 +++++++-------
>  drivers/usb/typec/tcpm.c      |  1 +
>  include/linux/usb/tcpm.h      |  2 ++
>  3 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index ed76327..4f7ad10 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -10,6 +10,7 @@
>  #include <linux/module.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> +#include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/usb/pd.h>
>  #include <linux/usb/tcpm.h>
> @@ -463,17 +464,16 @@ static const struct regmap_config tcpci_regmap_config = {
>  	.max_register = 0x7F, /* 0x80 .. 0xFF are vendor defined */
>  };
>  
> -static const struct tcpc_config tcpci_tcpc_config = {
> -	.type = TYPEC_PORT_DFP,
> -	.default_role = TYPEC_SINK,
> -};
> -
>  static int tcpci_parse_config(struct tcpci *tcpci)
>  {
>  	tcpci->controls_vbus = true; /* XXX */
>  
> -	/* TODO: Populate struct tcpc_config from ACPI/device-tree */
> -	tcpci->tcpc.config = &tcpci_tcpc_config;

That will break bisectablitity. tcpm.c is still accessing the config
at this point.

Just leave those untouched in here, and clean-up in separate patch
that comes after the patch that prepares tcpm.c.


Thanks,

-- 
heikki

Re: [PATCH v4 01/13] dt-bindings: connector: add properties for typec

[Mats Karrman]

Hi Li,

On 03/28/2018 06:06 PM, Li Jun wrote:

> Add bingdings supported by current typec driver, so user can pass
> all those properties via dt.
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  .../bindings/connector/usb-connector.txt           | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
> index e1463f1..922f22b 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -15,6 +15,29 @@ Optional properties:
>  - type: size of the connector, should be specified in case of USB-A, USB-B
>    non-fullsize connectors: "mini", "micro".
>  
> +Optional properties for usb-c-connector:
> +- power-type: should be one of "source", "sink" or "dual"(DRP) if typec
> +  connector has power support.
> +- try-power-role: preferred power role if "dual"(DRP) can support Try.SNK
> +  or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
> +- data-type: should be one of "host", "device", "dual"(DRD) if typec
> +  connector supports USB data.
> +
> +Required properties for usb-c-connector with power delivery support:
> +- source-pdos: An array of u32 with each entry providing supported power
> +  source data object(PDO), the detailed bit definitions of PDO can be found
> +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> +  Source_Capabilities Message, the order of each entry(PDO) should follow
> +  the PD spec chapter 6.4.1. Required for power source and power dual role.
> +- sink-pdos: An array of u32 with each entry providing supported power
> +  sink data object(PDO), the detailed bit definitions of PDO can be found
> +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
> +  Sink Capabilities Message, the order of each entry(PDO) should follow
> +  the PD spec chapter 6.4.1. Required for power sink and power dual role.
> +- op-sink-microwatt-hours: Sink required operating power in micro
> +  watt-hours, if source offered power is less then it, Capability Mismatch
> +  is set, required for power sink and power dual role.

This doesn't make sense. The unit of power is watt (W), watt-hour on the other hand
is a measurement of energy. I think "op-sink-microwatt" is what we want here.

// Mats

> +
>  Required nodes:
>  - any data bus to the connector should be modeled using the OF graph bindings
>    specified in bindings/graph.txt, unless the bus is between parent node and
> @@ -73,3 +96,19 @@ ccic: s2mm005@33 {
>  		};
>  	};
>  };
> +
> +3. USB-C connector attached to a typec port controller(ptn5110), which has
> +power delivery support and enables drp.
> +
> +typec: ptn5110@50 {
> +	...
> +	usb_con: connector {
> +		compatible = "usb-c-connector";
> +		label = "USB-C";
> +		power-type = "dual";
> +		try-power-role = "sink";
> +		source-pdos = <0x380190c8>;
> +		sink-pdos = <0x380190c8 0x3802d0c8>;
> +		op-sink-microwatt-hours = <9000000>;
> +	};
> +};
>

Re: [PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach

[Mats Karrman]

Hi Li,

On 03/28/2018 06:06 PM, Li Jun wrote:

> In case of drp toggling, we may need set correct cc value for role control
> after attach as it may never been set.
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  drivers/usb/typec/tcpm.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 218c230..72d4232 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
>  	tcpm_set_attached_state(port, false);
>  	port->try_src_count = 0;
>  	port->try_snk_count = 0;
> +	port->cc_req = 0;

I don't think it's OK to use "0" here. cc_req is an enum so why not use "|TYPEC_CC_OPEN"?|

>  }
>  
>  static void tcpm_detach(struct tcpm_port *port)
> @@ -2361,6 +2362,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		break;
>  
>  	case SRC_ATTACHED:
> +		if (!port->cc_req)

        	if (port->cc_req == |TYPEC_CC_OPEN)|

> +			tcpm_set_cc(port, tcpm_rp_cc(port));
>  		ret = tcpm_src_attach(port);
>  		tcpm_set_state(port, SRC_UNATTACHED,
>  			       ret < 0 ? 0 : PD_T_PS_SOURCE_ON);
> @@ -2531,6 +2534,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
>  		break;
>  	case SNK_ATTACHED:
> +		if (!port->cc_req)

Ditto.

> +			tcpm_set_cc(port, TYPEC_CC_RD);
>  		ret = tcpm_snk_attach(port);
>  		if (ret < 0)
>  			tcpm_set_state(port, SNK_UNATTACHED, 0);

// Mats


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach

[Guenter Roeck]

On Thu, Mar 29, 2018 at 12:06:15AM +0800, Li Jun wrote:
> In case of drp toggling, we may need set correct cc value for role control
> after attach as it may never been set.
> 

Isn't CC set by the lower level driver in this case ? In other words, is it ever
necessary to call back into the low level driver to set CC again ? Doing that in
attached state seems a bit late.

It may make more sense to update port->cc_req when the state machine leaves
DRP_TOGGLING state, ie in _tcpm_cc_change(), and to do it without callback
into the low level driver (it should not be necessary).

Guenter

> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  drivers/usb/typec/tcpm.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 218c230..72d4232 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
>  	tcpm_set_attached_state(port, false);
>  	port->try_src_count = 0;
>  	port->try_snk_count = 0;
> +	port->cc_req = 0;
>  }
>  
>  static void tcpm_detach(struct tcpm_port *port)
> @@ -2361,6 +2362,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		break;
>  
>  	case SRC_ATTACHED:
> +		if (!port->cc_req)
> +			tcpm_set_cc(port, tcpm_rp_cc(port));
>  		ret = tcpm_src_attach(port);
>  		tcpm_set_state(port, SRC_UNATTACHED,
>  			       ret < 0 ? 0 : PD_T_PS_SOURCE_ON);
> @@ -2531,6 +2534,8 @@ static void run_state_machine(struct tcpm_port *port)
>  		tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
>  		break;
>  	case SNK_ATTACHED:
> +		if (!port->cc_req)
> +			tcpm_set_cc(port, TYPEC_CC_RD);
>  		ret = tcpm_snk_attach(port);
>  		if (ret < 0)
>  			tcpm_set_state(port, SNK_UNATTACHED, 0);
> -- 
> 2.7.4
> 

Re: [PATCH v4 11/13] staging: typec: tcpci: keep the not connecting cc line open

[Guenter Roeck]

On 03/28/2018 09:06 AM, Li Jun wrote:
> While set polarity, we should keep the not connecting cc line to be
> open.
> 

The more I look at this code, the more I am confused by it.

The original code doesn't touch the CC lines. This function only sets the polarity.
Is it really appropriate to touch the CC lines in the same function ?

Guenter

> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>   drivers/staging/typec/tcpci.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index d5b4e4e..b58bd59 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -185,15 +185,25 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
>   			      enum typec_cc_polarity polarity)
>   {
>   	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> +	unsigned int reg;
>   	int ret;
>   
> -	ret = regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
> -			   (polarity == TYPEC_POLARITY_CC2) ?
> -			   TCPC_TCPC_CTRL_ORIENTATION : 0);
> +	/* Keep the disconnect cc line open */
> +	ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, &reg);
>   	if (ret < 0)
>   		return ret;
>   
> -	return 0;
> +	if (polarity == TYPEC_POLARITY_CC2)
> +		reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT;
> +	else
> +		reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT;
> +	ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	return regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
> +			   (polarity == TYPEC_POLARITY_CC2) ?
> +			   TCPC_TCPC_CTRL_ORIENTATION : 0);
>   }
>   
>   static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
> 

RE: [PATCH v4 07/13] staging: typec: tcpci: register port before request irq

[Jun Li]

Hi
> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: 2018年3月29日 18:52
> To: Jun Li <jun.li@nxp.com>
> Cc: robh+dt@kernel.org; gregkh@linuxfoundation.org;
> heikki.krogerus@linux.intel.com; linux@roeck-us.net;
> devel@driverdev.osuosl.org; devicetree@vger.kernel.org; Peter Chen
> <peter.chen@nxp.com>; linux-usb@vger.kernel.org; a.hajda@samsung.com;
> dl-linux-imx <linux-imx@nxp.com>; shufan_lee@richtek.com
> Subject: Re: [PATCH v4 07/13] staging: typec: tcpci: register port before request
> irq
> 
> On Thu, Mar 29, 2018 at 12:06:12AM +0800, Li Jun wrote:
> > With that we can clear any pending events and the port is registered
> > so driver can be ready to handle typec events once we request irq.
> >
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> 
> These sign offs aren't clear.
> 
> Sign offs mean that you handled the patch but didn't include any of SCO's
> copyrighted UNIX code into it.  Normally they're in the order of who touched
> the code.  So Peter touched the code first.  Should he get authorship credit?

I will change the patch author to be Peter as he touched the code first.

> How did he touch the code first if he didn't write the code?  It doesn't make
> sense.
> 
> > ---
> >  drivers/staging/typec/tcpci.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/typec/tcpci.c
> > b/drivers/staging/typec/tcpci.c index 4f7ad10..9e0014b 100644
> > --- a/drivers/staging/typec/tcpci.c
> > +++ b/drivers/staging/typec/tcpci.c
> > @@ -537,25 +537,26 @@ static int tcpci_probe(struct i2c_client *client,
> >  	if (IS_ERR(chip->data.regmap))
> >  		return PTR_ERR(chip->data.regmap);
> >
> > +	i2c_set_clientdata(client, chip);
> > +
> >  	/* Disable chip interrupts before requesting irq */
> >  	err = regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val,
> >  			       sizeof(u16));
> >  	if (err < 0)
> >  		return err;
> >
> > +	chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> > +	if (PTR_ERR_OR_ZERO(chip->tcpci))
> > +		return PTR_ERR(chip->tcpci);
> 
> When a function returns both error pointers and NULL that means that NULL is a
> secial case of success.  Like for example:
> 
> 	p->my_feature = get_optional_feature();
> 
> If it returns NULL that means the optional feature isn't there, but it's fine because
> it's optional.  But if it returns an error pointer that means the feature is there
> but the hardware is buggy or something so we shouldn't continue.
> 
> If you return PTR_ERR(NULL) that means success.
> 
> I don't think this code makes sense just from looking at it and also when I
> checked tcpci_register_port() doesn't return NULL.

This patch is to change the sequence of register port and request irq,
if error code checking of original code has the problem, I think that
should be another patch to fix it, I can do that later.

> 
> 
> 
> > +
> >  	err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> >  					_tcpci_irq,
> >  					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> >  					dev_name(&client->dev), chip);
> >  	if (err < 0)
> > -		return err;
> > +		tcpci_unregister_port(chip->tcpci);
> 
> Can you put the "return err;" back, because that's better style.  It's better to
> keep the error path and success path separate if you can.
> 
> 	if (err < 0) {
> 		tcpci_unregister_port(chip->tcpci);
> 		return err;
> 	}
> 
> 	return 0;
> 

OK, I will change as you suggested, thanks.

Li Jun
> 
> regards,
> dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v4 04/13] usb: typec: add fwnode to tcpc

[Jun Li]

Hi
> -----Original Message-----
> From: Heikki Krogerus [mailto:heikki.krogerus@linux.intel.com]
> Sent: 2018年3月29日 20:58
> To: Jun Li <jun.li@nxp.com>
> Cc: robh+dt@kernel.org; gregkh@linuxfoundation.org; linux@roeck-us.net;
> a.hajda@samsung.com; shufan_lee@richtek.com; Peter Chen
> <peter.chen@nxp.com>; devicetree@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 04/13] usb: typec: add fwnode to tcpc
> 
> Hi,
> 
> On Thu, Mar 29, 2018 at 12:06:09AM +0800, Li Jun wrote:
> > Add fwnode handle to get the fwnode so we can get typec configs it
> > contains.
> >
> > Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  drivers/staging/typec/tcpci.c | 14 +++++++-------
> >  drivers/usb/typec/tcpm.c      |  1 +
> >  include/linux/usb/tcpm.h      |  2 ++
> >  3 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/typec/tcpci.c
> > b/drivers/staging/typec/tcpci.c index ed76327..4f7ad10 100644
> > --- a/drivers/staging/typec/tcpci.c
> > +++ b/drivers/staging/typec/tcpci.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/module.h>
> >  #include <linux/i2c.h>
> >  #include <linux/interrupt.h>
> > +#include <linux/property.h>
> >  #include <linux/regmap.h>
> >  #include <linux/usb/pd.h>
> >  #include <linux/usb/tcpm.h>
> > @@ -463,17 +464,16 @@ static const struct regmap_config
> tcpci_regmap_config = {
> >  	.max_register = 0x7F, /* 0x80 .. 0xFF are vendor defined */  };
> >
> > -static const struct tcpc_config tcpci_tcpc_config = {
> > -	.type = TYPEC_PORT_DFP,
> > -	.default_role = TYPEC_SINK,
> > -};
> > -
> >  static int tcpci_parse_config(struct tcpci *tcpci)  {
> >  	tcpci->controls_vbus = true; /* XXX */
> >
> > -	/* TODO: Populate struct tcpc_config from ACPI/device-tree */
> > -	tcpci->tcpc.config = &tcpci_tcpc_config;
> 
> That will break bisectablitity. tcpm.c is still accessing the config at this point.
> 

Yes, good catch.

> Just leave those untouched in here, and clean-up in separate patch that comes
> after the patch that prepares tcpm.c.

I will change in next version, thanks.

Li Jun
> 
> 
> Thanks,
> 
> --
> heikki
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v4 01/13] dt-bindings: connector: add properties for typec

[Jun Li]

Hi
> -----Original Message-----
> From: Mats Karrman [mailto:mats.dev.list@gmail.com]
> Sent: 2018年3月30日 3:54
> To: Jun Li <jun.li@nxp.com>; robh+dt@kernel.org; gregkh@linuxfoundation.org;
> heikki.krogerus@linux.intel.com; linux@roeck-us.net
> Cc: a.hajda@samsung.com; shufan_lee@richtek.com; Peter Chen
> <peter.chen@nxp.com>; devicetree@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 01/13] dt-bindings: connector: add properties for typec
> 
> Hi Li,
> 
> On 03/28/2018 06:06 PM, Li Jun wrote:
> 
> > Add bingdings supported by current typec driver, so user can pass all
> > those properties via dt.
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  .../bindings/connector/usb-connector.txt           | 39
> ++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > index e1463f1..922f22b 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > @@ -15,6 +15,29 @@ Optional properties:
> >  - type: size of the connector, should be specified in case of USB-A, USB-B
> >    non-fullsize connectors: "mini", "micro".
> >
> > +Optional properties for usb-c-connector:
> > +- power-type: should be one of "source", "sink" or "dual"(DRP) if
> > +typec
> > +  connector has power support.
> > +- try-power-role: preferred power role if "dual"(DRP) can support
> > +Try.SNK
> > +  or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
> > +- data-type: should be one of "host", "device", "dual"(DRD) if typec
> > +  connector supports USB data.
> > +
> > +Required properties for usb-c-connector with power delivery support:
> > +- source-pdos: An array of u32 with each entry providing supported
> > +power
> > +  source data object(PDO), the detailed bit definitions of PDO can be
> > +found
> > +  in "Universal Serial Bus Power Delivery Specification" chapter
> > +6.4.1.2
> > +  Source_Capabilities Message, the order of each entry(PDO) should
> > +follow
> > +  the PD spec chapter 6.4.1. Required for power source and power dual role.
> > +- sink-pdos: An array of u32 with each entry providing supported
> > +power
> > +  sink data object(PDO), the detailed bit definitions of PDO can be
> > +found
> > +  in "Universal Serial Bus Power Delivery Specification" chapter
> > +6.4.1.3
> > +  Sink Capabilities Message, the order of each entry(PDO) should
> > +follow
> > +  the PD spec chapter 6.4.1. Required for power sink and power dual role.
> > +- op-sink-microwatt-hours: Sink required operating power in micro
> > +  watt-hours, if source offered power is less then it, Capability
> > +Mismatch
> > +  is set, required for power sink and power dual role.
> 
> This doesn't make sense. The unit of power is watt (W), watt-hour on the other
> hand is a measurement of energy. I think "op-sink-microwatt" is what we want
> here.

Yes, you are right, microwatt is what I should use here. I will change.

Li Jun
> 
> // Mats
> 
> > +
> >  Required nodes:
> >  - any data bus to the connector should be modeled using the OF graph
> bindings
> >    specified in bindings/graph.txt, unless the bus is between parent
> > node and @@ -73,3 +96,19 @@ ccic: s2mm005@33 {
> >  		};
> >  	};
> >  };
> > +
> > +3. USB-C connector attached to a typec port controller(ptn5110),
> > +which has power delivery support and enables drp.
> > +
> > +typec: ptn5110@50 {
> > +	...
> > +	usb_con: connector {
> > +		compatible = "usb-c-connector";
> > +		label = "USB-C";
> > +		power-type = "dual";
> > +		try-power-role = "sink";
> > +		source-pdos = <0x380190c8>;
> > +		sink-pdos = <0x380190c8 0x3802d0c8>;
> > +		op-sink-microwatt-hours = <9000000>;
> > +	};
> > +};
> >
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach

[Jun Li]

Hi
> -----Original Message-----
> From: Mats Karrman [mailto:mats.dev.list@gmail.com]
> Sent: 2018年3月30日 5:19
> To: Jun Li <jun.li@nxp.com>; robh+dt@kernel.org; gregkh@linuxfoundation.org;
> heikki.krogerus@linux.intel.com; linux@roeck-us.net
> Cc: a.hajda@samsung.com; shufan_lee@richtek.com; Peter Chen
> <peter.chen@nxp.com>; devicetree@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach
> 
> Hi Li,
> 
> On 03/28/2018 06:06 PM, Li Jun wrote:
> 
> > In case of drp toggling, we may need set correct cc value for role
> > control after attach as it may never been set.
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  drivers/usb/typec/tcpm.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index
> > 218c230..72d4232 100644
> > --- a/drivers/usb/typec/tcpm.c
> > +++ b/drivers/usb/typec/tcpm.c
> > @@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
> >  	tcpm_set_attached_state(port, false);
> >  	port->try_src_count = 0;
> >  	port->try_snk_count = 0;
> > +	port->cc_req = 0;
> 
> I don't think it's OK to use "0" here. cc_req is an enum so why not use
> "|TYPEC_CC_OPEN"?|
> 

I will change to be TYPEC_CC_OPEN, also other place.

Li Jun
> >  }
> >
> >  static void tcpm_detach(struct tcpm_port *port) @@ -2361,6 +2362,8 @@
> > static void run_state_machine(struct tcpm_port *port)
> >  		break;
> >
> >  	case SRC_ATTACHED:
> > +		if (!port->cc_req)
> 
>         	if (port->cc_req == |TYPEC_CC_OPEN)|
> 
> > +			tcpm_set_cc(port, tcpm_rp_cc(port));
> >  		ret = tcpm_src_attach(port);
> >  		tcpm_set_state(port, SRC_UNATTACHED,
> >  			       ret < 0 ? 0 : PD_T_PS_SOURCE_ON); @@ -2531,6 +2534,8
> @@
> > static void run_state_machine(struct tcpm_port *port)
> >  		tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
> >  		break;
> >  	case SNK_ATTACHED:
> > +		if (!port->cc_req)
> 
> Ditto.
> 
> > +			tcpm_set_cc(port, TYPEC_CC_RD);
> >  		ret = tcpm_snk_attach(port);
> >  		if (ret < 0)
> >  			tcpm_set_state(port, SNK_UNATTACHED, 0);
> 
> // Mats
> 

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach

[Jun Li]

> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter Roeck
> Sent: 2018年3月30日 6:49
> To: Jun Li <jun.li@nxp.com>
> Cc: robh+dt@kernel.org; gregkh@linuxfoundation.org;
> heikki.krogerus@linux.intel.com; a.hajda@samsung.com;
> shufan_lee@richtek.com; Peter Chen <peter.chen@nxp.com>;
> devicetree@vger.kernel.org; linux-usb@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 10/13] usb: typec: tcpm: set cc for drp toggling attach
> 
> On Thu, Mar 29, 2018 at 12:06:15AM +0800, Li Jun wrote:
> > In case of drp toggling, we may need set correct cc value for role
> > control after attach as it may never been set.
> >
> 
> Isn't CC set by the lower level driver in this case ? In other words, is it ever
> necessary to call back into the low level driver to set CC again ? Doing that in
> attached state seems a bit late.

In question case(drp without try.src or try.snk), you can see tcpm never call
tcpm_set_cc() from drp toggling to attached state, start_drp_toggling
set cc at the beginning in this case, but the value is the *start* value, may not
the right value to match its final role.

Per tcpci spec
Figure 4-16. DRP Initialization and Connection Detection
After debounce, tcpm should set cc(again, but may different value) and polarity.

ShuFan encountered this case as I understood on his setup like below:
- Tcpc start drp toggling with Rp/Rp,
- Connect to adapter with also Rp/Rp
- After connection resolved, the tcpc becomes a sink(HW present Rd), but
role control register value is still Rp/Rp.
- This should be corrected to set cc for keep the un-contacted cc line open.

> 
> It may make more sense to update port->cc_req when the state machine leaves
> DRP_TOGGLING state, ie in _tcpm_cc_change(), and to do it without callback into
> the low level driver (it should not be necessary).

Currently there is no user of cc_req flag, so I can't catch the real intention
of this flag, as of now it's only set in tcpm_set_cc(), so I understood it's
for if tcpm already set tcpc a specific role.

Li Jun
> 
> Guenter
> 
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  drivers/usb/typec/tcpm.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index
> > 218c230..72d4232 100644
> > --- a/drivers/usb/typec/tcpm.c
> > +++ b/drivers/usb/typec/tcpm.c
> > @@ -2126,6 +2126,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
> >  	tcpm_set_attached_state(port, false);
> >  	port->try_src_count = 0;
> >  	port->try_snk_count = 0;
> > +	port->cc_req = 0;
> >  }
> >
> >  static void tcpm_detach(struct tcpm_port *port) @@ -2361,6 +2362,8 @@
> > static void run_state_machine(struct tcpm_port *port)
> >  		break;
> >
> >  	case SRC_ATTACHED:
> > +		if (!port->cc_req)
> > +			tcpm_set_cc(port, tcpm_rp_cc(port));
> >  		ret = tcpm_src_attach(port);
> >  		tcpm_set_state(port, SRC_UNATTACHED,
> >  			       ret < 0 ? 0 : PD_T_PS_SOURCE_ON); @@ -2531,6 +2534,8
> @@
> > static void run_state_machine(struct tcpm_port *port)
> >  		tcpm_set_state(port, SNK_UNATTACHED, PD_T_PD_DEBOUNCE);
> >  		break;
> >  	case SNK_ATTACHED:
> > +		if (!port->cc_req)
> > +			tcpm_set_cc(port, TYPEC_CC_RD);
> >  		ret = tcpm_snk_attach(port);
> >  		if (ret < 0)
> >  			tcpm_set_state(port, SNK_UNATTACHED, 0);
> > --
> > 2.7.4
> >
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v4 11/13] staging: typec: tcpci: keep the not connecting cc line open

[Jun Li]

Hi
> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter Roeck
> Sent: 2018年3月30日 23:16
> To: Jun Li <jun.li@nxp.com>; robh+dt@kernel.org; gregkh@linuxfoundation.org;
> heikki.krogerus@linux.intel.com
> Cc: a.hajda@samsung.com; shufan_lee@richtek.com; Peter Chen
> <peter.chen@nxp.com>; devicetree@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 11/13] staging: typec: tcpci: keep the not connecting cc
> line open
> 
> On 03/28/2018 09:06 AM, Li Jun wrote:
> > While set polarity, we should keep the not connecting cc line to be
> > open.
> >
> 
> The more I look at this code, the more I am confused by it.
> 
> The original code doesn't touch the CC lines. This function only sets the polarity.
> Is it really appropriate to touch the CC lines in the same function ?
> 

Yes, I didn't find a more proper place to do this, either I change the
tcpc->set_cc() interface with orientation/polarity parameter to know
which cc line I should keep it open, or do it in low level driver like
this, do you have any suggestion how this can be done?(I guess
both cc lines have the same state after attached with current code
of all tcpm users, but this should be resolved as it's break PD compliance
test) 

thanks
Li Jun

> Guenter
> 
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >   drivers/staging/typec/tcpci.c | 18 ++++++++++++++----
> >   1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/typec/tcpci.c
> > b/drivers/staging/typec/tcpci.c index d5b4e4e..b58bd59 100644
> > --- a/drivers/staging/typec/tcpci.c
> > +++ b/drivers/staging/typec/tcpci.c
> > @@ -185,15 +185,25 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> >   			      enum typec_cc_polarity polarity)
> >   {
> >   	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > +	unsigned int reg;
> >   	int ret;
> >
> > -	ret = regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
> > -			   (polarity == TYPEC_POLARITY_CC2) ?
> > -			   TCPC_TCPC_CTRL_ORIENTATION : 0);
> > +	/* Keep the disconnect cc line open */
> > +	ret = regmap_read(tcpci->regmap, TCPC_ROLE_CTRL, &reg);
> >   	if (ret < 0)
> >   		return ret;
> >
> > -	return 0;
> > +	if (polarity == TYPEC_POLARITY_CC2)
> > +		reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC1_SHIFT;
> > +	else
> > +		reg |= TCPC_ROLE_CTRL_CC_OPEN << TCPC_ROLE_CTRL_CC2_SHIFT;
> > +	ret = regmap_write(tcpci->regmap, TCPC_ROLE_CTRL, reg);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
> > +			   (polarity == TYPEC_POLARITY_CC2) ?
> > +			   TCPC_TCPC_CTRL_ORIENTATION : 0);
> >   }
> >
> >   static int tcpci_set_vconn(struct tcpc_dev *tcpc, bool enable)
> >

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 07/13] staging: typec: tcpci: register port before request irq

[Dan Carpenter]

On Sat, Mar 31, 2018 at 03:09:44AM +0000, Jun Li wrote:
> This patch is to change the sequence of register port and request irq,
> if error code checking of original code has the problem, I think that
> should be another patch to fix it, I can do that later.

Fair enough.  That's reasonable.  Thanks!

regards,
dan carpenter

Re: [PATCH v4 01/13] dt-bindings: connector: add properties for typec

[Andrzej Hajda]

On 28.03.2018 18:06, Li Jun wrote:
> Add bingdings supported by current typec driver, so user can pass
> all those properties via dt.
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  .../bindings/connector/usb-connector.txt           | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
> index e1463f1..922f22b 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -15,6 +15,29 @@ Optional properties:
>  - type: size of the connector, should be specified in case of USB-A, USB-B
>    non-fullsize connectors: "mini", "micro".
>  
> +Optional properties for usb-c-connector:
> +- power-type: should be one of "source", "sink" or "dual"(DRP) if typec
> +  connector has power support.
> +- try-power-role: preferred power role if "dual"(DRP) can support Try.SNK
> +  or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
> +- data-type: should be one of "host", "device", "dual"(DRD) if typec
> +  connector supports USB data.
> +
> +Required properties for usb-c-connector with power delivery support:
> +- source-pdos: An array of u32 with each entry providing supported power
> +  source data object(PDO), the detailed bit definitions of PDO can be found
> +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> +  Source_Capabilities Message, the order of each entry(PDO) should follow
> +  the PD spec chapter 6.4.1. Required for power source and power dual role.
> +- sink-pdos: An array of u32 with each entry providing supported power
> +  sink data object(PDO), the detailed bit definitions of PDO can be found
> +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
> +  Sink Capabilities Message, the order of each entry(PDO) should follow
> +  the PD spec chapter 6.4.1. Required for power sink and power dual role.
> +- op-sink-microwatt-hours: Sink required operating power in micro
> +  watt-hours, if source offered power is less then it, Capability Mismatch
> +  is set, required for power sink and power dual role.

I have lurked into specs and I am not sure what is the relation of this
field with PD protocol, there is Minimum Operating Power, Maximum
Operating Power and Operating Power present in different RDOs, there are
also similar fields in sink PDOs.
I guess it can be one of them, which one? why other ones are not
provided? What if this or any other property can be calculated only in
runtime?
I am not sure if any of them should be marked "required".

> +
>  Required nodes:
>  - any data bus to the connector should be modeled using the OF graph bindings
>    specified in bindings/graph.txt, unless the bus is between parent node and
> @@ -73,3 +96,19 @@ ccic: s2mm005@33 {
>  		};
>  	};
>  };
> +
> +3. USB-C connector attached to a typec port controller(ptn5110), which has
> +power delivery support and enables drp.
> +
> +typec: ptn5110@50 {
> +	...
> +	usb_con: connector {
> +		compatible = "usb-c-connector";
> +		label = "USB-C";
> +		power-type = "dual";
> +		try-power-role = "sink";
> +		source-pdos = <0x380190c8>;

I understand from DT specification point of view cryptic numbers are OK,
but for sake of readability I strongly suggest use/define macros similar
to the ones present already in kernel:
    source-pdos = < PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) >;

It much less error prone, and makes review process much more easier.

Regards
Andrzej

> +		sink-pdos = <0x380190c8 0x3802d0c8>;
> +		op-sink-microwatt-hours = <9000000>;
> +	};
> +};


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec port controller(TCPCI)

[Rob Herring]

On Thu, Mar 29, 2018 at 12:06:07AM +0800, Li Jun wrote:
> TCPCI stands for typec port controller interface, its implementation
> has full typec port control with power delivery support, it's a
> standard i2c slave with GPIO input as irq interface, detail see spec
> "Universal Serial Bus Type-C Port Controller Interface Specification
> Revision 1.0, Version 1.1"
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  .../devicetree/bindings/usb/typec-tcpci.txt        | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> new file mode 100644
> index 0000000..7a7a8e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> @@ -0,0 +1,33 @@
> +TCPCI(Typec port cotroller interface) binding
> +---------------------------------------------
> +
> +Required properties:
> +- compatible:       should be "usb-tcpci,chip-specific-string".

Compatible strings should be in the form of "<vendor prefix>,<device>"

> +- reg:              the i2c slave address of typec port controller device.
> +- interrupt-parent: the phandle to the interrupt controller which provides
> +                    the interrupt.
> +- interrupts:       interrupt specification for tcpci alert.
> +
> +Required sub-node:
> +- connector: The "usb-c-connector" attached to the tcpci chip, the bindings
> +  of connector node are specified in
> +  Documentation/devicetree/bindings/connector/usb-connector.txt
> +
> +Example:
> +
> +ptn5110@50 {
> +	compatible = "usb-tcpci,ptn5110";
> +	reg = <0x50>;
> +	interrupt-parent = <&gpio3>;
> +	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> +
> +	usb_con: connector {

How is the OF graph done in this case? You need some link to the USB 
controller.

> +		compatible = "usb-c-connector";
> +		label = "USB-C";
> +		port-type = "dual";
> +		try-power-role = "sink"
> +		source-pdos = <0x380190c8>;
> +		sink-pdos = <0x380190c8 0x3802d0c8>;
> +		op-sink-microwatt-hours = <9000000>;
> +	};
> +};
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 01/13] dt-bindings: connector: add properties for typec

[Jun Li]

2018-04-03 16:29 GMT+08:00 Andrzej Hajda <a.hajda@samsung.com>:
>
> On 28.03.2018 18:06, Li Jun wrote:
> > Add bingdings supported by current typec driver, so user can pass
> > all those properties via dt.
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  .../bindings/connector/usb-connector.txt           | 39 ++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > index e1463f1..922f22b 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > @@ -15,6 +15,29 @@ Optional properties:
> >  - type: size of the connector, should be specified in case of USB-A, USB-B
> >    non-fullsize connectors: "mini", "micro".
> >
> > +Optional properties for usb-c-connector:
> > +- power-type: should be one of "source", "sink" or "dual"(DRP) if typec
> > +  connector has power support.
> > +- try-power-role: preferred power role if "dual"(DRP) can support Try.SNK
> > +  or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
> > +- data-type: should be one of "host", "device", "dual"(DRD) if typec
> > +  connector supports USB data.
> > +
> > +Required properties for usb-c-connector with power delivery support:
> > +- source-pdos: An array of u32 with each entry providing supported power
> > +  source data object(PDO), the detailed bit definitions of PDO can be found
> > +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> > +  Source_Capabilities Message, the order of each entry(PDO) should follow
> > +  the PD spec chapter 6.4.1. Required for power source and power dual role.
> > +- sink-pdos: An array of u32 with each entry providing supported power
> > +  sink data object(PDO), the detailed bit definitions of PDO can be found
> > +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
> > +  Sink Capabilities Message, the order of each entry(PDO) should follow
> > +  the PD spec chapter 6.4.1. Required for power sink and power dual role.
> > +- op-sink-microwatt-hours: Sink required operating power in micro
> > +  watt-hours, if source offered power is less then it, Capability Mismatch
> > +  is set, required for power sink and power dual role.
>
> I have lurked into specs and I am not sure what is the relation of this
> field with PD protocol, there is Minimum Operating Power, Maximum
> Operating Power and Operating Power present in different RDOs, there are
> also similar fields in sink PDOs.
> I guess it can be one of them, which one?

Per current usage of op-sink-microwatt(operating_snk_mw in tcpm),
this property is actually for max operating power, which is only used to
judge capability mismatch, i.e. the required max current/power is larger
than the selected source PDO can provide.

After checking the PD spec about this, basically I think this property also
can be removed, we can judge the capability mismatch by compare the
selected source pdo and sink pdo, considering there are existing users
of this field(operating_snk_mw in tcpm), I will check how this can be done.

I am removing the other 3 max_snk_*, see [1]

[1] https://www.spinics.net/lists/linux-usb/msg167262.html

> why other ones are not
> provided? What if this or any other property can be calculated only in
> runtime?
> I am not sure if any of them should be marked "required".
>
> > +
> >  Required nodes:
> >  - any data bus to the connector should be modeled using the OF graph bindings
> >    specified in bindings/graph.txt, unless the bus is between parent node and
> > @@ -73,3 +96,19 @@ ccic: s2mm005@33 {
> >               };
> >       };
> >  };
> > +
> > +3. USB-C connector attached to a typec port controller(ptn5110), which has
> > +power delivery support and enables drp.
> > +
> > +typec: ptn5110@50 {
> > +     ...
> > +     usb_con: connector {
> > +             compatible = "usb-c-connector";
> > +             label = "USB-C";
> > +             power-type = "dual";
> > +             try-power-role = "sink";
> > +             source-pdos = <0x380190c8>;
>
> I understand from DT specification point of view cryptic numbers are OK,
> but for sake of readability I strongly suggest use/define macros similar
> to the ones present already in kernel:
>     source-pdos = < PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) >;

I agree it's easier to read, so with the proposed way, we need maintain
a copy of those macros for dts(e.g. under dt-binding/usb/pd.h), right?

thanks
Jun
>
> It much less error prone, and makes review process much more easier.
>
> Regards
> Andrzej
>
> > +             sink-pdos = <0x380190c8 0x3802d0c8>;
> > +             op-sink-microwatt-hours = <9000000>;
> > +     };
> > +};
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec port controller(TCPCI)

[Jun Li]

Hi
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 2018年4月10日 4:04
> To: Jun Li <jun.li@nxp.com>
> Cc: gregkh@linuxfoundation.org; heikki.krogerus@linux.intel.com;
> linux@roeck-us.net; a.hajda@samsung.com; shufan_lee@richtek.com; Peter
> Chen <peter.chen@nxp.com>; devicetree@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec
> port controller(TCPCI)
> 
> On Thu, Mar 29, 2018 at 12:06:07AM +0800, Li Jun wrote:
> > TCPCI stands for typec port controller interface, its implementation
> > has full typec port control with power delivery support, it's a
> > standard i2c slave with GPIO input as irq interface, detail see spec
> > "Universal Serial Bus Type-C Port Controller Interface Specification
> > Revision 1.0, Version 1.1"
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  .../devicetree/bindings/usb/typec-tcpci.txt        | 33
> ++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > new file mode 100644
> > index 0000000..7a7a8e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > @@ -0,0 +1,33 @@
> > +TCPCI(Typec port cotroller interface) binding
> > +---------------------------------------------
> > +
> > +Required properties:
> > +- compatible:       should be "usb-tcpci,chip-specific-string".
> 
> Compatible strings should be in the form of "<vendor prefix>,<device>"
> 

OK, I will list the specific compatible string here and change
my example case to be "nxp,ptn5110".

> > +- reg:              the i2c slave address of typec port controller device.
> > +- interrupt-parent: the phandle to the interrupt controller which provides
> > +                    the interrupt.
> > +- interrupts:       interrupt specification for tcpci alert.
> > +
> > +Required sub-node:
> > +- connector: The "usb-c-connector" attached to the tcpci chip, the
> > +bindings
> > +  of connector node are specified in
> > +  Documentation/devicetree/bindings/connector/usb-connector.txt
> > +
> > +Example:
> > +
> > +ptn5110@50 {
> > +	compatible = "usb-tcpci,ptn5110";
> > +	reg = <0x50>;
> > +	interrupt-parent = <&gpio3>;
> > +	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> > +
> > +	usb_con: connector {
> 
> How is the OF graph done in this case? You need some link to the USB controller.

The platform(i.MX8MQ EVK) for this is still on the way of start upstream, I was
Planning to add this part with enabling USB3 function, as of how this will be done,
I only have usb3 ss data(no display port or Sideband), is something like below OK?

typec: ptn5110@50 {
	compatible = "nxp,ptn5110";
	...

	usb_con: connector {
		compatible = "usb-c-connector";
		label = "USB-C";
		...

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@1 {
				reg = <1>;
				usb_con_ss: endpoint {
				remote-endpoint = <&usb3_phy_ss>;
				};
			};
        	};
	};
};

&usb3_phy0 {
	status = "okay";
	
	port {
		usb3_phy_ss: endpoint {
			remote-endpoint = <&usb_con_ss>;
		};
	};
}

Thanks
Jun
> 
> > +		compatible = "usb-c-connector";
> > +		label = "USB-C";
> > +		port-type = "dual";
> > +		try-power-role = "sink"
> > +		source-pdos = <0x380190c8>;
> > +		sink-pdos = <0x380190c8 0x3802d0c8>;
> > +		op-sink-microwatt-hours = <9000000>;
> > +	};
> > +};
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree"
> > in the body of a message to majordomo@vger.kernel.org More majordomo
> > info at
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger
> > .kernel.org%2Fmajordomo-info.html&data=02%7C01%7Cjun.li%40nxp.com%7C
> 86
> >
> a7c0da18204df434d208d59e550c27%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %
> >
> 7C0%7C636589010562193065&sdata=0%2FmoDrqWn9YghWGucWYnMd1YK0BO2
> dVgp%2Fa
> > KNZZZ%2BXE%3D&reserved=0
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec port controller(TCPCI)

[Rob Herring]

On Mon, Apr 16, 2018 at 6:54 AM, Jun Li <jun.li@nxp.com> wrote:
> Hi
>> -----Original Message-----
>> From: Rob Herring [mailto:robh@kernel.org]
>> Sent: 2018年4月10日 4:04
>> To: Jun Li <jun.li@nxp.com>
>> Cc: gregkh@linuxfoundation.org; heikki.krogerus@linux.intel.com;
>> linux@roeck-us.net; a.hajda@samsung.com; shufan_lee@richtek.com; Peter
>> Chen <peter.chen@nxp.com>; devicetree@vger.kernel.org;
>> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
>> devel@driverdev.osuosl.org
>> Subject: Re: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec
>> port controller(TCPCI)
>>
>> On Thu, Mar 29, 2018 at 12:06:07AM +0800, Li Jun wrote:

[...]

>> > +ptn5110@50 {
>> > +   compatible = "usb-tcpci,ptn5110";
>> > +   reg = <0x50>;
>> > +   interrupt-parent = <&gpio3>;
>> > +   interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
>> > +
>> > +   usb_con: connector {
>>
>> How is the OF graph done in this case? You need some link to the USB controller.
>
> The platform(i.MX8MQ EVK) for this is still on the way of start upstream, I was
> Planning to add this part with enabling USB3 function, as of how this will be done,
> I only have usb3 ss data(no display port or Sideband), is something like below OK?
>
> typec: ptn5110@50 {
>         compatible = "nxp,ptn5110";
>         ...
>
>         usb_con: connector {
>                 compatible = "usb-c-connector";
>                 label = "USB-C";
>                 ...
>
>                 ports {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>
>                         port@1 {
>                                 reg = <1>;
>                                 usb_con_ss: endpoint {
>                                 remote-endpoint = <&usb3_phy_ss>;
>                                 };
>                         };
>                 };
>         };
> };
>
> &usb3_phy0 {
>         status = "okay";
>
>         port {
>                 usb3_phy_ss: endpoint {

Normally, the graph connection would be to the USB controller, not the
phy as the phy is just referred to with a "phys" property.

>                         remote-endpoint = <&usb_con_ss>;
>                 };
>         };
> }
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec port controller(TCPCI)

[Jun Li]

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 2018年4月16日 22:28
> To: Jun Li <jun.li@nxp.com>
> Cc: gregkh@linuxfoundation.org; heikki.krogerus@linux.intel.com;
> linux@roeck-us.net; a.hajda@samsung.com; shufan_lee@richtek.com; Peter
> Chen <peter.chen@nxp.com>; devicetree@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec
> port controller(TCPCI)
> 
> On Mon, Apr 16, 2018 at 6:54 AM, Jun Li <jun.li@nxp.com> wrote:
> > Hi
> >> -----Original Message-----
> >> From: Rob Herring [mailto:robh@kernel.org]
> >> Sent: 2018年4月10日 4:04
> >> To: Jun Li <jun.li@nxp.com>
> >> Cc: gregkh@linuxfoundation.org; heikki.krogerus@linux.intel.com;
> >> linux@roeck-us.net; a.hajda@samsung.com; shufan_lee@richtek.com;
> >> Peter Chen <peter.chen@nxp.com>; devicetree@vger.kernel.org;
> >> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> >> devel@driverdev.osuosl.org
> >> Subject: Re: [PATCH v4 02/13] dt-bindings: usb: add documentation for
> >> typec port controller(TCPCI)
> >>
> >> On Thu, Mar 29, 2018 at 12:06:07AM +0800, Li Jun wrote:
> 
> [...]
> 
> >> > +ptn5110@50 {
> >> > +   compatible = "usb-tcpci,ptn5110";
> >> > +   reg = <0x50>;
> >> > +   interrupt-parent = <&gpio3>;
> >> > +   interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> >> > +
> >> > +   usb_con: connector {
> >>
> >> How is the OF graph done in this case? You need some link to the USB
> controller.
> >
> > The platform(i.MX8MQ EVK) for this is still on the way of start
> > upstream, I was Planning to add this part with enabling USB3 function,
> > as of how this will be done, I only have usb3 ss data(no display port or
> Sideband), is something like below OK?
> >
> > typec: ptn5110@50 {
> >         compatible = "nxp,ptn5110";
> >         ...
> >
> >         usb_con: connector {
> >                 compatible = "usb-c-connector";
> >                 label = "USB-C";
> >                 ...
> >
> >                 ports {
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >
> >                         port@1 {
> >                                 reg = <1>;
> >                                 usb_con_ss: endpoint {
> >                                 remote-endpoint = <&usb3_phy_ss>;
> >                                 };
> >                         };
> >                 };
> >         };
> > };
> >
> > &usb3_phy0 {
> >         status = "okay";
> >
> >         port {
> >                 usb3_phy_ss: endpoint {
> 
> Normally, the graph connection would be to the USB controller, not the phy as
> the phy is just referred to with a "phys" property.

Understood, I will put this into a USB controller node. Thanks.

Jun
> 
> >                         remote-endpoint = <&usb_con_ss>;
> >                 };
> >         };
> > }
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec port controller(TCPCI)

[Mats Karrman]

Hi Li Jun,

Are you working on an updated version of this patch series?
I'm pondering other changes that builds on these patches (the documentation
and the fwnode added to the tcpc_dev and tcpm primarily).

Btw, there is a semi-colon missing in your example below.

BR // Mats

On 2018-03-28 18:06, Li Jun wrote:

> TCPCI stands for typec port controller interface, its implementation
> has full typec port control with power delivery support, it's a
> standard i2c slave with GPIO input as irq interface, detail see spec
> "Universal Serial Bus Type-C Port Controller Interface Specification
> Revision 1.0, Version 1.1"
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>   .../devicetree/bindings/usb/typec-tcpci.txt        | 33 ++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> new file mode 100644
> index 0000000..7a7a8e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> @@ -0,0 +1,33 @@
> +TCPCI(Typec port cotroller interface) binding
> +---------------------------------------------
> +
> +Required properties:
> +- compatible:       should be "usb-tcpci,chip-specific-string".
> +- reg:              the i2c slave address of typec port controller device.
> +- interrupt-parent: the phandle to the interrupt controller which provides
> +                    the interrupt.
> +- interrupts:       interrupt specification for tcpci alert.
> +
> +Required sub-node:
> +- connector: The "usb-c-connector" attached to the tcpci chip, the bindings
> +  of connector node are specified in
> +  Documentation/devicetree/bindings/connector/usb-connector.txt
> +
> +Example:
> +
> +ptn5110@50 {
> +	compatible = "usb-tcpci,ptn5110";
> +	reg = <0x50>;
> +	interrupt-parent = <&gpio3>;
> +	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> +
> +	usb_con: connector {
> +		compatible = "usb-c-connector";
> +		label = "USB-C";
> +		port-type = "dual";
> +		try-power-role = "sink"

Here!

> +		source-pdos = <0x380190c8>;
> +		sink-pdos = <0x380190c8 0x3802d0c8>;
> +		op-sink-microwatt-hours = <9000000>;
> +	};
> +};
>

Re: [PATCH v4 01/13] dt-bindings: connector: add properties for typec

[Heikki Krogerus]

Hi,

On Thu, Mar 29, 2018 at 12:06:06AM +0800, Li Jun wrote:
> +Optional properties for usb-c-connector:
> +- power-type: should be one of "source", "sink" or "dual"(DRP) if typec
> +  connector has power support.
> +- try-power-role: preferred power role if "dual"(DRP) can support Try.SNK
> +  or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
> +- data-type: should be one of "host", "device", "dual"(DRD) if typec
> +  connector supports USB data.

Please use "power-role" and "data-role".


Thanks,

-- 
heikki

RE: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec port controller(TCPCI)

[Jun Li]

Hi,
> -----Original Message-----
> From: Mats Karrman [mailto:mats.dev.list@gmail.com]
> Sent: 2018年4月30日 15:41
> To: Jun Li <jun.li@nxp.com>
> Cc: robh+dt@kernel.org; gregkh@linuxfoundation.org;
> heikki.krogerus@linux.intel.com; linux@roeck-us.net; a.hajda@samsung.com;
> shufan_lee@richtek.com; Peter Chen <peter.chen@nxp.com>;
> devicetree@vger.kernel.org; linux-usb@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 02/13] dt-bindings: usb: add documentation for typec
> port controller(TCPCI)
> 
> Hi Li Jun,
> 
> Are you working on an updated version of this patch series?
> I'm pondering other changes that builds on these patches (the documentation
> and the fwnode added to the tcpc_dev and tcpm primarily).

I am on a vacation and will be back tomorrow, I will post a new version soon.

> 
> Btw, there is a semi-colon missing in your example below.

Thanks, I will add it.

Li Jun
> 
> BR // Mats
> 
> On 2018-03-28 18:06, Li Jun wrote:
> 
> > TCPCI stands for typec port controller interface, its implementation
> > has full typec port control with power delivery support, it's a
> > standard i2c slave with GPIO input as irq interface, detail see spec
> > "Universal Serial Bus Type-C Port Controller Interface Specification
> > Revision 1.0, Version 1.1"
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >   .../devicetree/bindings/usb/typec-tcpci.txt        | 33
> ++++++++++++++++++++++
> >   1 file changed, 33 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > new file mode 100644
> > index 0000000..7a7a8e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/typec-tcpci.txt
> > @@ -0,0 +1,33 @@
> > +TCPCI(Typec port cotroller interface) binding
> > +---------------------------------------------
> > +
> > +Required properties:
> > +- compatible:       should be "usb-tcpci,chip-specific-string".
> > +- reg:              the i2c slave address of typec port controller device.
> > +- interrupt-parent: the phandle to the interrupt controller which provides
> > +                    the interrupt.
> > +- interrupts:       interrupt specification for tcpci alert.
> > +
> > +Required sub-node:
> > +- connector: The "usb-c-connector" attached to the tcpci chip, the
> > +bindings
> > +  of connector node are specified in
> > +  Documentation/devicetree/bindings/connector/usb-connector.txt
> > +
> > +Example:
> > +
> > +ptn5110@50 {
> > +	compatible = "usb-tcpci,ptn5110";
> > +	reg = <0x50>;
> > +	interrupt-parent = <&gpio3>;
> > +	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> > +
> > +	usb_con: connector {
> > +		compatible = "usb-c-connector";
> > +		label = "USB-C";
> > +		port-type = "dual";
> > +		try-power-role = "sink"
> 
> Here!
> 
> > +		source-pdos = <0x380190c8>;
> > +		sink-pdos = <0x380190c8 0x3802d0c8>;
> > +		op-sink-microwatt-hours = <9000000>;
> > +	};
> > +};
> >
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v4 01/13] dt-bindings: connector: add properties for typec

[Jun Li]

Hi
> -----Original Message-----
> From: Heikki Krogerus [mailto:heikki.krogerus@linux.intel.com]
> Sent: 2018年4月30日 19:24
> To: Jun Li <jun.li@nxp.com>
> Cc: robh+dt@kernel.org; gregkh@linuxfoundation.org; linux@roeck-us.net;
> a.hajda@samsung.com; shufan_lee@richtek.com; Peter Chen
> <peter.chen@nxp.com>; devicetree@vger.kernel.org;
> linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> devel@driverdev.osuosl.org
> Subject: Re: [PATCH v4 01/13] dt-bindings: connector: add properties for typec
> 
> Hi,
> 
> On Thu, Mar 29, 2018 at 12:06:06AM +0800, Li Jun wrote:
> > +Optional properties for usb-c-connector:
> > +- power-type: should be one of "source", "sink" or "dual"(DRP) if typec
> > +  connector has power support.
> > +- try-power-role: preferred power role if "dual"(DRP) can support Try.SNK
> > +  or Try.SRC, should be "sink" for Try.SNK or "source" for Try.SRC.
> > +- data-type: should be one of "host", "device", "dual"(DRD) if typec
> > +  connector supports USB data.
> 
> Please use "power-role" and "data-role".

OK. I will update these 2 property names as you suggested.

Li Jun
> 
> 
> Thanks,
> 
> --
> heikki
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel