Skip to contentSkip to conversation

staging: typec: tcpm: Drop commented out code

PATCH In review Guenter Roeck 3 participants 8 messages Sep 12, 2017 – Sep 18, 2017

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/5 staging: typec: tcpm: Drop commented out code

Commented out code can be added as needed. Drop it. Also drop TODO and an obsolete XXX comment.

---
v2, v3: No change

 drivers/staging/typec/tcpm.c | 37 +------------------------------------
 1 file changed, 1 insertion(+), 36 deletions(-)
drivers/staging/typec/tcpm.c +1 -36
index 8af62e74d54c..cb25ec8334b0 100644
@@ -908,27 +908,6 @@ static void svdm_consume_identity(struct tcpm_port *port, const __le32 *payload,
 
 	memset(&port->mode_data, 0, sizeof(port->mode_data));
 
-#if 0 /* Not really a match */
-	switch (PD_IDH_PTYPE(vdo)) {
-	case IDH_PTYPE_UNDEF:
-		port->partner.type = TYPEC_PARTNER_NONE; /* no longer exists */
-		break;
-	case IDH_PTYPE_HUB:
-		break;
-	case IDH_PTYPE_PERIPH:
-		break;
-	case IDH_PTYPE_PCABLE:
-		break;
-	case IDH_PTYPE_ACABLE:
-		break;
-	case IDH_PTYPE_AMA:
-		port->partner.type = TYPEC_PARTNER_ALTMODE;
-		break;
-	default:
-		break;
-	}
-#endif
-
 	port->partner_ident.id_header = vdo;
 	port->partner_ident.cert_stat = le32_to_cpu(payload[VDO_INDEX_CSTAT]);
 	port->partner_ident.product = product;
@@ -1103,11 +1082,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
 				response[0] = VDO(svid, 1, CMD_DISCOVER_MODES);
 				rlen = 1;
 			} else {
-#if 0
-				response[0] = pd_dfp_enter_mode(port, 0, 0);
-				if (response[0])
-					rlen = 1;
-#endif
+				/* enter alternate mode if/when implemented */
 			}
 			break;
 		case CMD_ENTER_MODE:
@@ -1145,10 +1120,6 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
 
 	if (PD_VDO_SVDM(p0))
 		rlen = tcpm_pd_svdm(port, payload, cnt, response);
-#if 0
-	else
-		rlen = tcpm_pd_custom_vdm(port, cnt, payload, response);
-#endif
 
 	if (rlen > 0) {
 		tcpm_queue_vdm(port, response[0], &response[1], rlen - 1);
@@ -2442,7 +2413,6 @@ static void run_state_machine(struct tcpm_port *port)
 			tcpm_set_state(port, SNK_STARTUP, 0);
 		break;
 	case SNK_STARTUP:
-		/* XXX: callback into infrastructure */
 		opmode =  tcpm_get_pwr_opmode(port->polarity ?
 					      port->cc2 : port->cc1);
 		typec_set_pwr_opmode(port->typec_port, opmode);
@@ -3589,11 +3559,6 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 
 	port->partner_desc.identity = &port->partner_ident;
 	port->port_type = tcpc->config->type;
-	/*
-	 * TODO:
-	 *  - alt_modes, set_alt_mode
-	 *  - {debug,audio}_accessory
-	 */
 
 	port->typec_port = typec_register_port(port->dev, &port->typec_caps);
 	if (!port->typec_port) {
Signature · 1 line
2.7.4
#1 lore raw

PATCH v3 2/5 staging: typec: tcpm: Document data structures

Document struct tcpc_config and struct tcpc_dev.
Drop unused TCPC_USB_SWITCH_RESTORE.

---
v2, v3: No change

 drivers/staging/typec/tcpm.h | 57 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 7 deletions(-)
drivers/staging/typec/tcpm.h +50 -7
index 7e9a6b7b5cd6..073197f0d2bb 100644
@@ -54,6 +54,27 @@ enum tcpm_transmit_type {
 	TCPC_TX_BIST_MODE_2 = 7
 };
 
+/**
+ * struct tcpc_config - Port configuration
+ * @src_pdo:	PDO parameters sent to port partner as response to
+ *		PD_CTRL_GET_SOURCE_CAP message
+ * @nr_src_pdo:	Number of entries in @src_pdo
+ * @snk_pdo:	PDO parameters sent to partner as response to
+ *		PD_CTRL_GET_SINK_CAP message
+ * @nr_snk_pdo:	Number of entries in @snk_pdo
+ * @max_snk_mv:	Maximum acceptable sink voltage in mV
+ * @max_snk_ma:	Maximum sink current in mA
+ * @max_snk_mw:	Maximum required sink power in mW
+ * @operating_snk_mw:
+ *		Required operating sink power in mW
+ * @type:	Port type (TYPEC_PORT_DFP, TYPEC_PORT_UFP, or
+ *		TYPEC_PORT_DRP)
+ * @default_role:
+ *		Default port role (TYPEC_SINK or TYPEC_SOURCE).
+ *		Set to TYPEC_NO_PREFERRED_ROLE if no default role.
+ * @try_role_hw:True if try.{Src,Snk} is implemented in hardware
+ * @alt_modes:	List of supported alternate modes
+ */
 struct tcpc_config {
 	const u32 *src_pdo;
 	unsigned int nr_src_pdo;
@@ -79,7 +100,6 @@ struct tcpc_config {
 enum tcpc_usb_switch {
 	TCPC_USB_SWITCH_CONNECT,
 	TCPC_USB_SWITCH_DISCONNECT,
-	TCPC_USB_SWITCH_RESTORE,	/* TODO FIXME */
 };
 
 /* Mux state attributes */
@@ -104,17 +124,40 @@ struct tcpc_mux_dev {
 	void *priv_data;
 };
 
+/**
+ * struct tcpc_dev - Port configuration and callback functions
+ * @config:	Pointer to port configuration
+ * @get_vbus:	Called to read current VBUS state
+ * @get_current_limit:
+ *		Optional; called by the tcpm core when configured as a snk
+ *		and cc=Rp-def. This allows the tcpm to provide a fallback
+ *		current-limit detection method for the cc=Rp-def case.
+ *		For example, some tcpcs may include BC1.2 charger detection
+ *		and use that in this case.
+ * @set_cc:	Called to set value of CC pins
+ * @get_cc:	Called to read current CC pin values
+ * @set_polarity:
+ *		Called to set polarity
+ * @set_vconn:	Called to enable or disable VCONN
+ * @set_vbus:	Called to enable or disable VBUS
+ * @set_current_limit:
+ *		Optional; called to set current limit as negotiated
+ *		with partner.
+ * @set_pd_rx:	Called to enable or disable reception of PD messages
+ * @set_roles:	Called to set power and data roles
+ * @start_drp_toggling:
+ *		Optional; if supported by hardware, called to start DRP
+ *		toggling. DRP toggling is stopped automatically if
+ *		a connection is established.
+ * @try_role:	Optional; called to set a preferred role
+ * @pd_transmit:Called to transmit PD message
+ * @mux:	Pointer to multiplexer data
+ */
 struct tcpc_dev {
 	const struct tcpc_config *config;
 
 	int (*init)(struct tcpc_dev *dev);
 	int (*get_vbus)(struct tcpc_dev *dev);
-	/*
-	 * This optional callback gets called by the tcpm core when configured
-	 * as a snk and cc=Rp-def. This allows the tcpm to provide a fallback
-	 * current-limit detection method for the cc=Rp-def case. E.g. some
-	 * tcpcs may include BC1.2 charger detection and use that in this case.
-	 */
 	int (*get_current_limit)(struct tcpc_dev *dev);
 	int (*set_cc)(struct tcpc_dev *dev, enum typec_cc_status cc);
 	int (*get_cc)(struct tcpc_dev *dev, enum typec_cc_status *cc1,
Signature · 1 line
2.7.4
#2 lore raw

PATCH v3 3/5 staging: typec: pd: Document struct pd_message

In reply to Guenter Roeck

struct pd_message is the format of a PD message as seen on the wire.

---
v2: No change
v3: Fix document tag

 drivers/staging/typec/pd.h | 5 +++++
 1 file changed, 5 insertions(+)
drivers/staging/typec/pd.h +5 -0
index 30b32ad72acd..42a10883a2cb 100644
@@ -104,6 +104,11 @@ static inline unsigned int pd_header_msgid_le(__le16 header)
 
 #define PD_MAX_PAYLOAD		7
 
+/**
+ * struct pd_message - PD message as seen on wire
+ * @header:	PD message header
+ * @payload:	PD message payload
+ */
 struct pd_message {
 	__le16 header;
 	__le32 payload[PD_MAX_PAYLOAD];
Signature · 1 line
2.7.4
#3 lore raw

PATCH v3 4/5 typec: tcpm: Move out of staging

In reply to Guenter Roeck

Move tcpm (USB Type-C Port Manager) out of staging.

---
v2: Use format-patch -M
v3: No change

 drivers/staging/typec/Kconfig                         |  8 --------
 drivers/staging/typec/Makefile                        |  1 -
 drivers/staging/typec/TODO                            | 10 ----------
 drivers/staging/typec/fusb302/fusb302.c               |  4 ++--
 drivers/staging/typec/tcpci.c                         |  4 ++--
 drivers/usb/typec/Kconfig                             |  8 ++++++++
 drivers/usb/typec/Makefile                            |  1 +
 drivers/{staging => usb}/typec/tcpm.c                 |  9 ++++-----
 {drivers/staging/typec => include/linux/usb}/pd.h     |  0
 {drivers/staging/typec => include/linux/usb}/pd_bdo.h |  0
 {drivers/staging/typec => include/linux/usb}/pd_vdo.h |  0
 {drivers/staging/typec => include/linux/usb}/tcpm.h   |  0
 12 files changed, 17 insertions(+), 28 deletions(-)
 rename drivers/{staging => usb}/typec/tcpm.c (99%)
 rename {drivers/staging/typec => include/linux/usb}/pd.h (100%)
 rename {drivers/staging/typec => include/linux/usb}/pd_bdo.h (100%)
 rename {drivers/staging/typec => include/linux/usb}/pd_vdo.h (100%)
 rename {drivers/staging/typec => include/linux/usb}/tcpm.h (100%)
drivers/staging/typec/Kconfig +0 -8
index 37a0781b0d0c..31fad23c2553 100644
@@ -1,13 +1,5 @@
 menu "USB Power Delivery and Type-C drivers"
 
-config TYPEC_TCPM
-	tristate "USB Type-C Port Controller Manager"
-	depends on USB
-	select TYPEC
-	help
-	  The Type-C Port Controller Manager provides a USB PD and USB Type-C
-	  state machine for use with Type-C Port Controllers.
-
 if TYPEC_TCPM
 
 config TYPEC_TCPCI
drivers/staging/typec/Makefile +0 -1
index 30a7e29cbc9e..e1df3f0fde10 100644
@@ -1,3 +1,2 @@
-obj-$(CONFIG_TYPEC_TCPM)	+= tcpm.o
 obj-$(CONFIG_TYPEC_TCPCI)	+= tcpci.o
 obj-y				+= fusb302/
drivers/staging/typec/TODO +0 -10
index bc1f97a2d1bf..53fe2f726c88 100644
@@ -1,13 +1,3 @@
-tcpm:
-- Add documentation (at the very least for the API to low level drivers)
-- Split PD code into separate file
-- Check if it makes sense to use tracepoints instead of debugfs for debug logs
-- Implement Alternate Mode handling
-- Address "#if 0" code if not addressed with the above
-- Validate all comments marked with "XXX"; either address or remove comments
-- Add support for USB PD 3.0. While not mandatory, at least fast role swap
-  as well as authentication support would be very desirable.
-
 tcpci:
 - Test with real hardware
 
drivers/staging/typec/fusb302/fusb302.c +2 -2
index fc6a3cf74eb3..e790b67d4953 100644
@@ -37,11 +37,11 @@
 #include <linux/string.h>
 #include <linux/types.h>
 #include <linux/usb/typec.h>
+#include <linux/usb/tcpm.h>
+#include <linux/usb/pd.h>
 #include <linux/workqueue.h>
 
 #include "fusb302_reg.h"
-#include "../tcpm.h"
-#include "../pd.h"
 
 /*
  * When the device is SNK, BC_LVL interrupt is used to monitor cc pins
drivers/staging/typec/tcpci.c +2 -2
index df72d8b01e73..4636804ea1a4 100644
@@ -20,11 +20,11 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/regmap.h>
+#include <linux/usb/pd.h>
+#include <linux/usb/tcpm.h>
 #include <linux/usb/typec.h>
 
-#include "pd.h"
 #include "tcpci.h"
-#include "tcpm.h"
 
 #define PD_RETRY_COUNT 3
 
drivers/usb/typec/Kconfig +8 -0
index bc1b7745f1d4..888605860091 100644
@@ -4,6 +4,14 @@ menu "USB Power Delivery and Type-C drivers"
 config TYPEC
 	tristate
 
+config TYPEC_TCPM
+	tristate "USB Type-C Port Controller Manager"
+	depends on USB
+	select TYPEC
+	help
+	  The Type-C Port Controller Manager provides a USB PD and USB Type-C
+	  state machine for use with Type-C Port Controllers.
+
 config TYPEC_WCOVE
 	tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
 	depends on ACPI
drivers/usb/typec/Makefile +1 -0
index bc214f15f1b5..eb883984724b 100644
@@ -1,3 +1,4 @@
 obj-$(CONFIG_TYPEC)		+= typec.o
+obj-$(CONFIG_TYPEC_TCPM)	+= tcpm.o
 obj-$(CONFIG_TYPEC_WCOVE)	+= typec_wcove.o
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
drivers/usb/typec/tcpm.c renamed from drivers/staging/typec/tcpm.c +4 -5
similarity index 99%
rename from drivers/staging/typec/tcpm.c
rename to drivers/usb/typec/tcpm.c
index cb25ec8334b0..f557c479fdc2 100644
@@ -26,14 +26,13 @@
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/usb/pd.h>
+#include <linux/usb/pd_bdo.h>
+#include <linux/usb/pd_vdo.h>
+#include <linux/usb/tcpm.h>
 #include <linux/usb/typec.h>
 #include <linux/workqueue.h>
 
-#include "pd.h"
-#include "pd_vdo.h"
-#include "pd_bdo.h"
-#include "tcpm.h"
-
 #define FOREACH_STATE(S)			\
 	S(INVALID_STATE),			\
 	S(DRP_TOGGLING),			\
include/linux/usb/pd.h renamed from drivers/staging/typec/pd.h +0 -0
similarity index 100%
rename from drivers/staging/typec/pd.h
rename to include/linux/usb/pd.h
include/linux/usb/pd_bdo.h renamed from drivers/staging/typec/pd_bdo.h +0 -0
similarity index 100%
rename from drivers/staging/typec/pd_bdo.h
rename to include/linux/usb/pd_bdo.h
include/linux/usb/pd_vdo.h renamed from drivers/staging/typec/pd_vdo.h +0 -0
similarity index 100%
rename from drivers/staging/typec/pd_vdo.h
rename to include/linux/usb/pd_vdo.h
include/linux/usb/tcpm.h renamed from drivers/staging/typec/tcpm.h +0 -0
similarity index 100%
rename from drivers/staging/typec/tcpm.h
rename to include/linux/usb/tcpm.h
Signature · 1 line
2.7.4
#4 lore raw

PATCH v3 5/5 usb: typec: fusb302: Move out of staging

In reply to Guenter Roeck

The driver is in good enough shape to be moved out of staging. Do it.

---
v2: Use format-patch -M
v3: No change

 drivers/staging/typec/Kconfig                        |  2 --
 drivers/staging/typec/Makefile                       |  1 -
 drivers/staging/typec/fusb302/TODO                   | 10 ----------
 drivers/usb/typec/Kconfig                            |  6 ++++++
 drivers/usb/typec/Makefile                           |  1 +
 drivers/{staging => usb}/typec/fusb302/Kconfig       |  0
 drivers/{staging => usb}/typec/fusb302/Makefile      |  0
 drivers/{staging => usb}/typec/fusb302/fusb302.c     |  0
 drivers/{staging => usb}/typec/fusb302/fusb302_reg.h |  0
 9 files changed, 7 insertions(+), 13 deletions(-)
 delete mode 100644 drivers/staging/typec/fusb302/TODO
 rename drivers/{staging => usb}/typec/fusb302/Kconfig (100%)
 rename drivers/{staging => usb}/typec/fusb302/Makefile (100%)
 rename drivers/{staging => usb}/typec/fusb302/fusb302.c (100%)
 rename drivers/{staging => usb}/typec/fusb302/fusb302_reg.h (100%)
drivers/staging/typec/Kconfig +0 -2
index 31fad23c2553..5359f556d203 100644
@@ -9,8 +9,6 @@ config TYPEC_TCPCI
 	help
 	  Type-C Port Controller driver for TCPCI-compliant controller.
 
-source "drivers/staging/typec/fusb302/Kconfig"
-
 endif
 
 endmenu
drivers/staging/typec/Makefile +0 -1
index e1df3f0fde10..53d649abcb53 100644
@@ -1,2 +1 @@
 obj-$(CONFIG_TYPEC_TCPCI)	+= tcpci.o
-obj-y				+= fusb302/
drivers/staging/typec/fusb302/TODO deleted +0 -10
deleted file mode 100644
index 19b466eb585d..000000000000
@@ -1,10 +0,0 @@
-fusb302:
-- Find a better logging scheme, at least not having the same debugging/logging
-  code replicated here and in tcpm
-- Find a non-hacky way to coordinate between PM and I2C access
-- Documentation? The FUSB302 datasheet provides information on the chip to help
-  understand the code. But it may still be helpful to have a documentation.
-- We may want to replace the  "fcs,max-snk-microvolt", "fcs,max-snk-microamp",
-  "fcs,max-snk-microwatt" and "fcs,operating-snk-microwatt" device(tree)
-  properties with properties which are part of a generic type-c controller
-  devicetree binding.
drivers/usb/typec/Kconfig +6 -0
index 888605860091..819c0ed2b200 100644
@@ -12,6 +12,12 @@ config TYPEC_TCPM
 	  The Type-C Port Controller Manager provides a USB PD and USB Type-C
 	  state machine for use with Type-C Port Controllers.
 
+if TYPEC_TCPM
+
+source "drivers/usb/typec/fusb302/Kconfig"
+
+endif
+
 config TYPEC_WCOVE
 	tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
 	depends on ACPI
drivers/usb/typec/Makefile +1 -0
index eb883984724b..b77688ce1f16 100644
@@ -1,4 +1,5 @@
 obj-$(CONFIG_TYPEC)		+= typec.o
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm.o
+obj-y				+= fusb302/
 obj-$(CONFIG_TYPEC_WCOVE)	+= typec_wcove.o
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
drivers/usb/typec/fusb302/Kconfig renamed from drivers/staging/typec/fusb302/Kconfig +0 -0
similarity index 100%
rename from drivers/staging/typec/fusb302/Kconfig
rename to drivers/usb/typec/fusb302/Kconfig
drivers/usb/typec/fusb302/Makefile renamed from drivers/staging/typec/fusb302/Makefile +0 -0
similarity index 100%
rename from drivers/staging/typec/fusb302/Makefile
rename to drivers/usb/typec/fusb302/Makefile
drivers/usb/typec/fusb302/fusb302.c renamed from drivers/staging/typec/fusb302/fusb302.c +0 -0
similarity index 100%
rename from drivers/staging/typec/fusb302/fusb302.c
rename to drivers/usb/typec/fusb302/fusb302.c
drivers/usb/typec/fusb302/fusb302_reg.h renamed from drivers/staging/typec/fusb302/fusb302_reg.h +0 -0
similarity index 100%
rename from drivers/staging/typec/fusb302/fusb302_reg.h
rename to drivers/usb/typec/fusb302/fusb302_reg.h
Signature · 1 line
2.7.4
maintainer Acked-by on v3 1/5
#5 lore raw
In reply to Guenter Roeck
Quoted 9 lines · Guenter Roeck

Commented out code can be added as needed. Drop it. Also drop TODO and an obsolete XXX comment.

---
v2, v3: No change

 drivers/staging/typec/tcpm.c | 37 +------------------------------------
 1 file changed, 1 insertion(+), 36 deletions(-)

Nice! Just to be sure: The idea is to leave tcpci.c in staging, right?

In any case, FWIW, for all of these:

Signature · 1 line
heikki
#6 lore raw
Quoted 4 lines · Heikki Krogerus
Quoted 3 lines · Guenter Roeck

Commented out code can be added as needed. Drop it. Also drop TODO and an obsolete XXX comment.

Quoted 9 lines
Quoted 6 lines

---
v2, v3: No change

 drivers/staging/typec/tcpm.c | 37 +------------------------------------
 1 file changed, 1 insertion(+), 36 deletions(-)

Nice! Just to be sure: The idea is to leave tcpci.c in staging, right?

Yes, until we get test coverage or Greg gets tired of keeping it there (in that case we would probably have to drop it).

Guenter

#7 lore raw
Quoted 9 lines · Guenter Roeck
Quoted 8 lines · Heikki Krogerus
Quoted 7 lines · Guenter Roeck

Commented out code can be added as needed. Drop it. Also drop TODO and an obsolete XXX comment.

---
v2, v3: No change

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
 drivers/staging/typec/tcpm.c | 37 +------------------------------------
 1 file changed, 1 insertion(+), 36 deletions(-)

Nice! Just to be sure: The idea is to leave tcpci.c in staging, right?

Yes, until we get test coverage or Greg gets tired of keeping it there (in that case we would probably have to drop it).

Well, if it's not ever going to get fixed up, why can't we just drop it?

Anyway, I've pulled this series into the staging and USB trees, so all should be good for it now.

thanks,

greg k-h