Date:   Mon, 24 Apr 2017 17:57:52 +0200
From:   Mats Karrman <mats.dev.list@...il.com>
To:     Guenter Roeck <linux@...ck-us.net>,
        Felipe Balbi <felipe.balbi@...ux.intel.com>
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Badhri Jagan Sridharan <badhri@...gle.com>,
        Oliver Neukum <oneukum@...e.com>,
        Guenter Roeck <groeck@...omium.org>
Subject: Re: [RFC PATCH v5 1/2] usb: typec: USB Type-C Port Manager (tcpm)

Hello Guenter,

Some corrections and suggestions follow.

And then of course it would be nice to factor out the alternate mode handling
so new mode handlers could be registered as needed :)

BR // Mats


On 04/22/2017 12:15 AM, Guenter Roeck wrote:
> From: Guenter Roeck <groeck@...omium.org>
>
> This driver implements the USB Type-C Power Delivery state machine
> for both source and sink ports. Alternate mode support is not
> fully implemented.
>
> The driver attaches to the USB Type-C class code implemented in
> the following patches.
>
> 	usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY
> 	usb: USB Type-C connector class
>
> This driver only implements the state machine. Lower level drivers are
> responsible for
> - Reporting VBUS status and activating VBUS
> - Setting CC lines and providing CC line status
> - Setting line polarity
> - Activating and deactivating VCONN
> - Setting the current limit
> - Activating and deactivating PD message transfers
> - Sending and receiving PD messages
>
> The driver provides both a functional API as well as callbacks for
> lower level drivers.
>
> Signed-off-by: Guenter Roeck <groeck@...omium.org>
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
> v5:
> - Adjust to infrastructure API changes
> - Minor API change to low level driver
>   [set_pd_header -> set_roles]
> - Set default state depending on preferred role
> - Improve Try.SRC and Try.SNK implementation
> - Do not report stale identity information to infrastructure
> - Fix crash seen if a SVD message was received while disconnected
> - Improve accessory handling
> - If a role change is requested and the partner does not support PD,
>   request a port reset
> - Set state to SRC_READY after PD_N_CAPS_COUNT if the partner does not
>   respond to capabilities messages
> - Update PD_T_SEND_SOURCE_CAP to improve compatibility with slow partners
> - Coding style fixes
>

...

> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> new file mode 100644
> index 000000000000..1a82dddb243d
> --- /dev/null
> +++ b/drivers/usb/typec/tcpm.c
> @@ -0,0 +1,3443 @@
> +/*
> + * Copyright 2015-2016 Google, Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * USB Power Delivery protocol stack.
> + */
> +

...

> +
> +static void svdm_consume_modes(struct tcpm_port *port, const __le32 *payload,
> +			       int cnt)
> +{
> +	struct pd_mode_data *pmdata = &port->mode_data;
> +	struct typec_altmode_desc *paltmode;
> +	struct typec_mode_desc *pmode;
> +	int i;
> +
> +	if (pmdata->altmodes >= ARRAY_SIZE(port->partner_altmode)) {
> +		/* Already logged in svdm_consume_svids() */
> +		return;
> +	}
> +
> +	paltmode = &pmdata->altmode_desc[pmdata->altmodes];
> +	memset(paltmode, 0, sizeof(*paltmode));
> +
> +	paltmode->svid = pmdata->svids[pmdata->svid_index];
> +
> +	tcpm_log(port, " Alternate mode %d: SVID 0x%04x",
> +		 pmdata->altmodes, paltmode->svid);
> +
> +	for (i = 1; i < cnt && paltmode->n_modes < ALTMODE_MAX_MODES; i++) {
> +		pmode = &paltmode->modes[paltmode->n_modes];
> +		memset(pmode, 0, sizeof(*pmode));
> +		pmode->vdo = le32_to_cpu(payload[i]);
> +		pmode->index = i - 1;
> +		paltmode->n_modes++;
> +		tcpm_log(port, "  VDO %d: 0x%04x",

Should be 4 bytes:
-		tcpm_log(port, "  VDO %d: 0x%04x",
+		tcpm_log(port, "  VDO %d: 0x%08x",

>
> +			 pmode->index, pmode->vdo);
> +	}
> +	port->partner_altmode[pmdata->altmodes] =
> +		typec_partner_register_altmode(port->partner, paltmode);
> +	if (port->partner_altmode[pmdata->altmodes] == NULL) {
> +		tcpm_log(port,
> +			 "Failed to register alternate modes for SVID 0x%04x",
> +			 paltmode->svid);
> +		return;
> +	}
> +	pmdata->altmodes++;
> +}
> +

...

> +
> +/*
> + * PD (data, control) command handling functions
> + */
> +static void tcpm_pd_data_request(struct tcpm_port *port,
> +				 const struct pd_message *msg)
> +{
> +	enum pd_data_msg_type type = pd_header_type_le(msg->header);
> +	unsigned int cnt = pd_header_cnt_le(msg->header);
> +	unsigned int i;
> +
> +	switch (type) {
> +	case PD_DATA_SOURCE_CAP:
> +		if (port->pwr_role != TYPEC_SINK)
> +			break;
> +
> +		memcpy(&port->source_caps, msg->payload, cnt * sizeof(u32));

Endianess:
-		memcpy(&port->source_caps, msg->payload, cnt * sizeof(u32));
+		for (i = 0; i < cnt; i++)
+			port->source_caps[i] = le32_to_cpu(msg->payload[i]);

>
> +		port->nr_source_caps = cnt;
> +

This, or make a separate logging function for this message. There may be more
situations where more verbose logging is of use so maybe the whole loging
business could be factored out and we could have something usbmon like in the
future :)

+#ifdef CONFIG_DEBUG_FS
+		/* Build a log line from message data */

> +		for (i = 0; i < cnt; i++) {
> +			u32 pdo = port->source_caps[i];
> +			enum pd_pdo_type type = pdo_type(pdo);
> +			char msg[64];
> +
> +			switch (type) {
> +			case PDO_TYPE_FIXED:
> +				scnprintf(msg, sizeof(msg),
> +					  "%u mV, %u mA [%s%s%s%s%s%s]",
> +					  pdo_fixed_voltage(pdo),
> +					  pdo_max_current(pdo),
> +					  (pdo & PDO_FIXED_DUAL_ROLE) ?
> +								"R" : "",
> +					  (pdo & PDO_FIXED_SUSPEND) ?
> +								"S" : "",
> +					  (pdo & PDO_FIXED_HIGHER_CAP) ?
> +								"H" : "",
> +					  (pdo & PDO_FIXED_USB_COMM) ?
> +								"U" : "",
> +					  (pdo & PDO_FIXED_DATA_SWAP) ?
> +								"D" : "",
> +					  (pdo & PDO_FIXED_EXTPOWER) ?
> +								"E" : "");
> +				break;
> +			case PDO_TYPE_VAR:
> +				scnprintf(msg, sizeof(msg),
> +					  "%u-%u mV, %u mA",
> +					  pdo_min_voltage(pdo),
> +					  pdo_max_voltage(pdo),
> +					  pdo_max_current(pdo));
> +				break;
> +			case PDO_TYPE_BATT:
> +				scnprintf(msg, sizeof(msg),
> +					  "%u-%u mV, %u mW",
> +					  pdo_min_voltage(pdo),
> +					  pdo_max_voltage(pdo),
> +					  pdo_max_power(pdo));
> +				break;
> +			default:
> +				strcpy(msg, "undefined");
> +				break;
> +			}
> +			tcpm_log(port, " PDO %d: type %d, %s",
> +				 i, type, msg);
> +		}

+#endif
+

> +		/*
> +		 * This message may be received even if VBUS is not
> +		 * present. This is quite unexpected; see USB PD
> +		 * specification, sections 8.3.3.6.3.1 and 8.3.3.6.3.2.
> +		 * However, at the same time, we must be ready to
> +		 * receive this message and respond to it 15ms after
> +		 * receiving PS_RDY during power swap operations, no matter
> +		 * if VBUS is available or not (USB PD specification,
> +		 * section 6.5.9.2).
> +		 * So we need to accept the message either way,
> +		 * but be prepared to keep waiting for VBUS after it was
> +		 * handled.
> +		 */
> +		tcpm_set_state(port, SNK_NEGOTIATE_CAPABILITIES, 0);
> +		break;
> +	case PD_DATA_REQUEST:
> +		if (port->pwr_role != TYPEC_SOURCE ||
> +		    cnt != 1) {
> +			tcpm_queue_message(port, PD_MSG_CTRL_REJECT);
> +			break;
> +		}
> +		port->sink_request = msg->payload[0];

Endianess:
-		port->sink_request = msg->payload[0];
+		port->sink_request = le32_to_cpu(msg->payload[0]);

>
> +		tcpm_set_state(port, SRC_NEGOTIATE_CAPABILITIES, 0);
> +		break;
> +	case PD_DATA_SINK_CAP:
> +		/* We don't do anything with this at the moment... */
> +		for (i = 0; i < cnt; i++)
> +			port->sink_caps[i] = le32_to_cpu(msg->payload[i]);
> +		port->nr_sink_caps = cnt;
> +		break;
> +	case PD_DATA_VENDOR_DEF:
> +		tcpm_handle_vdm_request(port, msg->payload, cnt);
> +		break;
> +	case PD_DATA_BIST:
> +		if (port->state == SRC_READY || port->state == SNK_READY) {
> +			port->bist_request = msg->payload[0];

Endianess:
-			port->bist_request = msg->payload[0];
+			port->bist_request = le32_to_cpu(msg->payload[0]);

>
> +			tcpm_set_state(port, BIST_RX, 0);
> +		}
> +		break;
> +	default:
> +		tcpm_log(port, "Unhandled data message type %#x", type);
> +		break;
> +	}
> +}
> +

...

> +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 ||
> +	    !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)
> +		return ERR_PTR(-EINVAL);
> +
> +	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> +	if (!port)
> +		return ERR_PTR(-ENOMEM);
> +
> +	port->dev = dev;
> +	port->tcpc = tcpc;
> +
> +	mutex_init(&port->lock);
> +	mutex_init(&port->swap_lock);
> +
> +	port->wq = create_singlethread_workqueue(dev_name(dev));
> +	if (!port->wq)
> +		return ERR_PTR(-ENOMEM);
> +	INIT_DELAYED_WORK(&port->state_machine, tcpm_state_machine_work);
> +	INIT_DELAYED_WORK(&port->vdm_state_machine, vdm_state_machine_work);
> +	INIT_WORK(&port->event_work, tcpm_pd_event_handler);
> +
> +	spin_lock_init(&port->pd_event_lock);
> +
> +	init_completion(&port->tx_complete);
> +	init_completion(&port->swap_complete);
> +
> +	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->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;
> +	else
> +		port->try_role = TYPEC_NO_PREFERRED_ROLE;
> +
> +	port->typec_caps.prefer_role = tcpc->config->default_role;
> +	port->typec_caps.type = tcpc->config->type;
> +	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;
> +	port->typec_caps.pr_set = tcpm_pr_set;
> +	port->typec_caps.vconn_set = tcpm_vconn_set;
> +	port->typec_caps.try_role = tcpm_try_role;
> +
> +	port->partner_desc.identity = &port->partner_ident;
> +
> +	/*
> +	 * TODO:
> +	 *  - alt_modes, set_alt_mode
> +	 *  - {debug,audio}_accessory
> +	 */
> +
> +	port->typec_port = typec_register_port(port->dev, &port->typec_caps);
> +	if (IS_ERR(port->typec_port)) {
> +		err = PTR_ERR(port->typec_port);
> +		goto out_destroy_wq;
> +	}
> +
> +	if (tcpc->config->alt_modes) {
> +		struct typec_altmode_desc *paltmode = tcpc->config->alt_modes;

Allow constant initializers (see also tcpm.h below + I sent a patch to fix
typec_register_altmode() to usb list):
-               struct typec_altmode_desc *paltmode = tcpc->config->alt_modes;
+               const struct typec_altmode_desc *paltmode =
+                       tcpc->config->alt_modes;

> +
> +		i = 0;
> +		while (paltmode->svid && i < ARRAY_SIZE(port->port_altmode)) {
> +			port->port_altmode[i] =
> +			  typec_port_register_altmode(port->typec_port,
> +						      paltmode);
> +			if (!port->port_altmode[i]) {
> +				tcpm_log(port,
> +					 "%s: failed to register port alternate mode 0x%x",
> +					 dev_name(dev), paltmode->svid);
> +				break;
> +			}
> +			i++;
> +			paltmode++;
> +		}
> +	}
> +
> +	tcpm_debugfs_init(port);
> +	mutex_lock(&port->lock);
> +	tcpm_init(port);
> +	mutex_unlock(&port->lock);
> +
> +	tcpm_log(port, "%s: registered", dev_name(dev));
> +	return port;
> +
> +out_destroy_wq:
> +	destroy_workqueue(port->wq);
> +	return ERR_PTR(err);
> +}
> +EXPORT_SYMBOL_GPL(tcpm_register_port);
> +

...

> +
> +MODULE_AUTHOR("Guenter Roeck <groeck@...omium.org>");
> +MODULE_DESCRIPTION("USB Type-C Port Manager");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/typec/tcpm.h b/drivers/usb/typec/tcpm.h
> new file mode 100644
> index 000000000000..870d03e1bee3
> --- /dev/null
> +++ b/drivers/usb/typec/tcpm.h
> @@ -0,0 +1,150 @@
> +/*
> + * Copyright 2015-2016 Google, Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __LINUX_USB_TCPM_H
> +#define __LINUX_USB_TCPM_H
> +
> +#include <linux/bitops.h>
> +#include <linux/usb/typec.h>
> +#include <linux/usb/pd.h>
> +
> +enum typec_cc_status {
> +	TYPEC_CC_OPEN,
> +	TYPEC_CC_RA,
> +	TYPEC_CC_RD,
> +	TYPEC_CC_RP_DEF,
> +	TYPEC_CC_RP_1_5,
> +	TYPEC_CC_RP_3_0,
> +};
> +
> +enum typec_cc_polarity {
> +	TYPEC_POLARITY_CC1,
> +	TYPEC_POLARITY_CC2,
> +};
> +
> +/* Time to wait for TCPC to complete transmit */
> +#define PD_T_TCPC_TX_TIMEOUT  100
> +
> +enum tcpm_transmit_status {
> +	TCPC_TX_SUCCESS = 0,
> +	TCPC_TX_DISCARDED = 1,
> +	TCPC_TX_FAILED = 2,
> +};
> +
> +enum tcpm_transmit_type {
> +	TCPC_TX_SOP = 0,
> +	TCPC_TX_SOP_PRIME = 1,
> +	TCPC_TX_SOP_PRIME_PRIME = 2,
> +	TCPC_TX_SOP_DEBUG_PRIME = 3,
> +	TCPC_TX_SOP_DEBUG_PRIME_PRIME = 4,
> +	TCPC_TX_HARD_RESET = 5,
> +	TCPC_TX_CABLE_RESET = 6,
> +	TCPC_TX_BIST_MODE_2 = 7
> +};
> +
> +struct tcpc_config {
> +	const u32 *src_pdo;
> +	unsigned int nr_src_pdo;
> +
> +	const u32 *snk_pdo;
> +	unsigned int nr_snk_pdo;
> +
> +	unsigned int max_snk_mv;
> +	unsigned int max_snk_ma;
> +	unsigned int max_snk_mw;
> +	unsigned int operating_snk_mw;
> +
> +	enum typec_port_type type;
> +	enum typec_role default_role;
> +	bool try_role_hw;	/* try.{src,snk} implemented in hardware */
> +
> +	struct typec_altmode_desc *alt_modes;

Allow for constant initializers:
-	struct typec_altmode_desc *alt_modes;
+	const struct typec_altmode_desc *alt_modes;

>
> +};
> +
> +enum tcpc_usb_switch {
> +	TCPC_USB_SWITCH_CONNECT,
> +	TCPC_USB_SWITCH_DISCONNECT,
> +	TCPC_USB_SWITCH_RESTORE,	/* TODO FIXME */
> +};
> +
> +/* Mux state attributes */
> +#define TCPC_MUX_USB_ENABLED		BIT(0)	/* USB enabled */
> +#define TCPC_MUX_DP_ENABLED		BIT(1)	/* DP enabled */
> +#define TCPC_MUX_POLARITY_INVERTED	BIT(2)	/* Polarity inverted */
> +
> +/* Mux modes, decoded to attributes */
> +enum tcpc_mux_mode {
> +	TYPEC_MUX_NONE	= 0,				/* Open switch */
> +	TYPEC_MUX_USB	= TCPC_MUX_USB_ENABLED,		/* USB only */
> +	TYPEC_MUX_DP	= TCPC_MUX_DP_ENABLED,		/* DP only */
> +	TYPEC_MUX_DOCK	= TCPC_MUX_USB_ENABLED |	/* Both USB and DP */
> +			  TCPC_MUX_DP_ENABLED,
> +};
> +
> +struct tcpc_mux_dev {
> +	int (*set)(struct tcpc_mux_dev *dev, enum tcpc_mux_mode mux_mode,
> +		   enum tcpc_usb_switch usb_config,
> +		   enum typec_cc_polarity polarity);
> +	bool dfp_only;
> +	void *priv_data;
> +};
> +
> +struct tcpc_dev {
> +	const struct tcpc_config *config;
> +
> +	int (*init)(struct tcpc_dev *dev);
> +	int (*get_vbus)(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,
> +		      enum typec_cc_status *cc2);
> +	int (*set_polarity)(struct tcpc_dev *dev,
> +			    enum typec_cc_polarity polarity);
> +	int (*set_vconn)(struct tcpc_dev *dev, bool on);
> +	int (*set_vbus)(struct tcpc_dev *dev, bool on, bool charge);
> +	int (*set_current_limit)(struct tcpc_dev *dev, u32 max_ma, u32 mv);
> +	int (*set_pd_rx)(struct tcpc_dev *dev, bool on);
> +	int (*set_roles)(struct tcpc_dev *dev, bool attached,
> +			 enum typec_role role, enum typec_data_role data);
> +	int (*start_drp_toggling)(struct tcpc_dev *dev,
> +				  enum typec_cc_status cc);
> +	int (*try_role)(struct tcpc_dev *dev, int role);
> +	int (*pd_transmit)(struct tcpc_dev *dev, enum tcpm_transmit_type type,
> +			   const struct pd_message *msg);
> +	struct tcpc_mux_dev *mux;
> +};
> +
> +struct tcpm_port;
> +
> +struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc);
> +void tcpm_unregister_port(struct tcpm_port *port);
> +
> +void tcpm_update_source_capabilities(struct tcpm_port *port, const u32 *pdo,
> +				     unsigned int nr_pdo);
> +void tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo,
> +				   unsigned int nr_pdo,
> +				   unsigned int max_snk_mv,
> +				   unsigned int max_snk_ma,
> +				   unsigned int max_snk_mw,
> +				   unsigned int operating_snk_mw);
> +
> +void tcpm_vbus_change(struct tcpm_port *port);
> +void tcpm_cc_change(struct tcpm_port *port);
> +void tcpm_pd_receive(struct tcpm_port *port,
> +		     const struct pd_message *msg);
> +void tcpm_pd_transmit_complete(struct tcpm_port *port,
> +			       enum tcpm_transmit_status status);
> +void tcpm_pd_hard_reset(struct tcpm_port *port);
> +void tcpm_tcpc_reset(struct tcpm_port *port);
> +
> +#endif /* __LINUX_USB_TCPM_H */

...

>
> diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
> new file mode 100644
> index 000000000000..ff2d0248f21a
> --- /dev/null
> +++ b/include/linux/usb/pd_vdo.h
> @@ -0,0 +1,407 @@
> +/*
> + * Copyright 2015-2016 Google, Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __LINUX_USB_PD_VDO_H
> +#define __LINUX_USB_PD_VDO_H
> +
> +#include <linux/usb/pd.h>
> +
> +/*
> + * VDO : Vendor Defined Message Object
> + * VDM object is minimum of VDM header + 6 additional data objects.
> + */
> +
> +/*
> + * VDM header
> + * ----------
> + * <31:16>  :: SVID
> + * <15>     :: VDM type ( 1b == structured, 0b == unstructured )
> + * <14:13>  :: Structured VDM version (can only be 00 == 1.0 currently)
> + * <12:11>  :: reserved
> + * <10:8>   :: object position (1-7 valid ... used for enter/exit mode only)
> + * <7:6>    :: command type (SVDM only?)
> + * <5>      :: reserved (SVDM), command type (UVDM)
> + * <4:0>    :: command
> + */
> +#define VDO_MAX_SIZE 7
> +#define VDO(vid, type, custom)				\
> +	(((vid) << 16) |				\
> +	 ((type) << 15) |				\
> +	 ((custom) & 0x7FFF))
> +
> +#define VDO_SVDM_TYPE		(1 << 15)
> +#define VDO_SVDM_VERS(x)	((x) << 13)
> +#define VDO_OPOS(x)		((x) << 8)
> +#define VDO_CMDT(x)		((x) << 6)
> +#define VDO_OPOS_MASK		VDO_OPOS(0x7)
> +#define VDO_CMDT_MASK		VDO_CMDT(0x3)
> +
> +#define CMDT_INIT		0
> +#define CMDT_RSP_ACK		1
> +#define CMDT_RSP_NAK		2
> +#define CMDT_RSP_BUSY		3
> +
> +/* reserved for SVDM ... for Google UVDM */
> +#define VDO_SRC_INITIATOR	(0 << 5)
> +#define VDO_SRC_RESPONDER	(1 << 5)
> +
> +#define CMD_DISCOVER_IDENT	1
> +#define CMD_DISCOVER_SVID	2
> +#define CMD_DISCOVER_MODES	3
> +#define CMD_ENTER_MODE		4
> +#define CMD_EXIT_MODE		5
> +#define CMD_ATTENTION		6
> +#define CMD_DP_STATUS		16
> +#define CMD_DP_CONFIG		17
> +
> +#define VDO_CMD_VENDOR(x)    (((10 + (x)) & 0x1f))
> +
> +/* ChromeOS specific commands */
> +#define VDO_CMD_VERSION		VDO_CMD_VENDOR(0)
> +#define VDO_CMD_SEND_INFO	VDO_CMD_VENDOR(1)
> +#define VDO_CMD_READ_INFO	VDO_CMD_VENDOR(2)
> +#define VDO_CMD_REBOOT		VDO_CMD_VENDOR(5)
> +#define VDO_CMD_FLASH_ERASE	VDO_CMD_VENDOR(6)
> +#define VDO_CMD_FLASH_WRITE	VDO_CMD_VENDOR(7)
> +#define VDO_CMD_ERASE_SIG	VDO_CMD_VENDOR(8)
> +#define VDO_CMD_PING_ENABLE	VDO_CMD_VENDOR(10)
> +#define VDO_CMD_CURRENT		VDO_CMD_VENDOR(11)
> +#define VDO_CMD_FLIP		VDO_CMD_VENDOR(12)
> +#define VDO_CMD_GET_LOG		VDO_CMD_VENDOR(13)
> +#define VDO_CMD_CCD_EN		VDO_CMD_VENDOR(14)
> +
> +#define PD_VDO_VID(vdo)		((vdo) >> 16)
> +#define PD_VDO_SVDM(vdo)	(((vdo) >> 15) & 1)
> +#define PD_VDO_OPOS(vdo)	(((vdo) >> 8) & 0x7)
> +#define PD_VDO_CMD(vdo)		((vdo) & 0x1f)
> +#define PD_VDO_CMDT(vdo)	(((vdo) >> 6) & 0x3)
> +
> +/*
> + * SVDM Identity request -> response
> + *
> + * Request is simply properly formatted SVDM header
> + *
> + * Response is 4 data objects:
> + * [0] :: SVDM header
> + * [1] :: Identitiy header
> + * [2] :: Cert Stat VDO
> + * [3] :: (Product | Cable) VDO
> + * [4] :: AMA VDO
> + *
> + */
> +#define VDO_INDEX_HDR		0
> +#define VDO_INDEX_IDH		1
> +#define VDO_INDEX_CSTAT		2
> +#define VDO_INDEX_CABLE		3
> +#define VDO_INDEX_PRODUCT	3
> +#define VDO_INDEX_AMA		4
> +
> +/*
> + * SVDM Identity Header
> + * --------------------
> + * <31>     :: data capable as a USB host
> + * <30>     :: data capable as a USB device
> + * <29:27>  :: product type
> + * <26>     :: modal operation supported (1b == yes)
> + * <25:16>  :: SBZ

SBZ = "Shall Be Zero"? (9 places in this file)
Isn't "Reserved" more commonly understood?

>
> + * <15:0>   :: USB-IF assigned VID for this cable vendor
> + */
> +#define IDH_PTYPE_UNDEF		0
> +#define IDH_PTYPE_HUB		1
> +#define IDH_PTYPE_PERIPH	2
> +#define IDH_PTYPE_PCABLE	3
> +#define IDH_PTYPE_ACABLE	4
> +#define IDH_PTYPE_AMA		5
> +
> +#define VDO_IDH(usbh, usbd, ptype, is_modal, vid)		\
> +	((usbh) << 31 | (usbd) << 30 | ((ptype) & 0x7) << 27	\
> +	 | (is_modal) << 26 | ((vid) & 0xffff))
> +
> +#define PD_IDH_PTYPE(vdo)	(((vdo) >> 27) & 0x7)
> +#define PD_IDH_VID(vdo)		((vdo) & 0xffff)
> +#define PD_IDH_MODAL_SUPP(vdo)	((vdo) & (1 << 26))
> +
> +/*
> + * Cert Stat VDO
> + * -------------
> + * <31:20> : SBZ
> + * <19:0>  : USB-IF assigned TID for this cable
> + */
> +#define VDO_CSTAT(tid)		((tid) & 0xfffff)
> +#define PD_CSTAT_TID(vdo)	((vdo) & 0xfffff)

In spec (USB PD, R2.0 v1.3) it's
"<31:0> : 32-bit unsigned integer, XID : Assigned by USB-IF"

>
> +
> +/*
> + * Product VDO
> + * -----------
> + * <31:16> : USB Product ID
> + * <15:0>  : USB bcdDevice
> + */
> +#define VDO_PRODUCT(pid, bcd)	(((pid) & 0xffff) << 16 | ((bcd) & 0xffff))
> +#define PD_PRODUCT_PID(vdo)	(((vdo) >> 16) & 0xffff)
> +
> +/*
> + * Cable VDO
> + * ---------
> + * <31:28> :: Cable HW version
> + * <27:24> :: Cable FW version
> + * <23:20> :: SBZ
> + * <19:18> :: type-C to Type-A/B/C (00b == A, 01 == B, 10 == C)
> + * <17>    :: Type-C to Plug/Receptacle (0b == plug, 1b == receptacle)
> + * <16:13> :: cable latency (0001 == <10ns(~1m length))
> + * <12:11> :: cable termination type (11b == both ends active VCONN req)
> + * <10>    :: SSTX1 Directionality support (0b == fixed, 1b == cfgable)
> + * <9>     :: SSTX2 Directionality support
> + * <8>     :: SSRX1 Directionality support
> + * <7>     :: SSRX2 Directionality support
> + * <6:5>   :: Vbus current handling capability
> + * <4>     :: Vbus through cable (0b == no, 1b == yes)
> + * <3>     :: SOP" controller present? (0b == no, 1b == yes)
> + * <2:0>   :: USB SS Signaling support
> + */
> +#define CABLE_ATYPE		0
> +#define CABLE_BTYPE		1
> +#define CABLE_CTYPE		2
> +#define CABLE_PLUG		0
> +#define CABLE_RECEPTACLE	1
> +#define CABLE_CURR_1A5		0
> +#define CABLE_CURR_3A		1
> +#define CABLE_CURR_5A		2
> +#define CABLE_USBSS_U2_ONLY	0
> +#define CABLE_USBSS_U31_GEN1	1
> +#define CABLE_USBSS_U31_GEN2	2
> +#define VDO_CABLE(hw, fw, cbl, gdr, lat, term, tx1d, tx2d, rx1d, rx2d, cur,\
> +		  vps, sopp, usbss) \
> +	(((hw) & 0x7) << 28 | ((fw) & 0x7) << 24 | ((cbl) & 0x3) << 18	\
> +	 | (gdr) << 17 | ((lat) & 0x7) << 13 | ((term) & 0x3) << 11	\
> +	 | (tx1d) << 10 | (tx2d) << 9 | (rx1d) << 8 | (rx2d) << 7	\
> +	 | ((cur) & 0x3) << 5 | (vps) << 4 | (sopp) << 3		\
> +	 | ((usbss) & 0x7))
> +
> +/*
> + * AMA VDO
> + * ---------
> + * <31:28> :: Cable HW version
> + * <27:24> :: Cable FW version
> + * <23:12> :: SBZ
> + * <11>    :: SSTX1 Directionality support (0b == fixed, 1b == cfgable)
> + * <10>    :: SSTX2 Directionality support
> + * <9>     :: SSRX1 Directionality support
> + * <8>     :: SSRX2 Directionality support
> + * <7:5>   :: Vconn power
> + * <4>     :: Vconn power required
> + * <3>     :: Vbus power required
> + * <2:0>   :: USB SS Signaling support
> + */
> +#define VDO_AMA(hw, fw, tx1d, tx2d, rx1d, rx2d, vcpwr, vcr, vbr, usbss) \
> +	(((hw) & 0x7) << 28 | ((fw) & 0x7) << 24			\
> +	 | (tx1d) << 11 | (tx2d) << 10 | (rx1d) << 9 | (rx2d) << 8	\
> +	 | ((vcpwr) & 0x3) << 5 | (vcr) << 4 | (vbr) << 3		\

VCONN power field is 3 bits
-	 | ((vcpwr) & 0x3) << 5 | (vcr) << 4 | (vbr) << 3               \
+	 | ((vcpwr) & 0x7) << 5 | (vcr) << 4 | (vbr) << 3               \

> +	 | ((usbss) & 0x7))
> +
> +#define PD_VDO_AMA_VCONN_REQ(vdo)	(((vdo) >> 4) & 1)
> +#define PD_VDO_AMA_VBUS_REQ(vdo)	(((vdo) >> 3) & 1)
> +
> +#define AMA_VCONN_PWR_1W	0
> +#define AMA_VCONN_PWR_1W5	1
> +#define AMA_VCONN_PWR_2W	2
> +#define AMA_VCONN_PWR_3W	3
> +#define AMA_VCONN_PWR_4W	4
> +#define AMA_VCONN_PWR_5W	5
> +#define AMA_VCONN_PWR_6W	6
> +#define AMA_USBSS_U2_ONLY	0
> +#define AMA_USBSS_U31_GEN1	1
> +#define AMA_USBSS_U31_GEN2	2
> +#define AMA_USBSS_BBONLY	3
> +
> +/*
> + * SVDM Discover SVIDs request -> response
> + *
> + * Request is properly formatted VDM Header with discover SVIDs command.
> + * Response is a set of SVIDs of all all supported SVIDs with all zero's to
> + * mark the end of SVIDs.  If more than 12 SVIDs are supported command SHOULD be
> + * repeated.
> + */
> +#define VDO_SVID(svid0, svid1)	(((svid0) & 0xffff) << 16 | ((svid1) & 0xffff))
> +#define PD_VDO_SVID_SVID0(vdo)	((vdo) >> 16)
> +#define PD_VDO_SVID_SVID1(vdo)	((vdo) & 0xffff)
> +
> +/*
> + * Google modes capabilities
> + * <31:8> : reserved
> + * <7:0>  : mode
> + */
> +#define VDO_MODE_GOOGLE(mode) ((mode) & 0xff)
> +
> +#define MODE_GOOGLE_FU 1 /* Firmware Update mode */

--> include/linux/usb/pd_vdo_google.h ?
And keep this file for USB-IF def's.

>
> +
> +/*
> + * Mode Capabilities
> + *
> + * Number of VDOs supplied is SID dependent (but <= 6 VDOS?)
> + */
> +#define VDO_MODE_CNT_DISPLAYPORT 1

The VESA standard opens up for having other modes associated with DP_SID,
in that case distinguished by non zero values in bit 31:24 of the mode VDO.

Wouldn't it be nice to split out the DisplayPort related
stuff to a separate include, e.g. include/linux/usb/pd_vdo_dp.h?
(i.e. one include per mode family)

>
> +
> +/*
> + * DisplayPort modes capabilities
> + * -------------------------------
> + * <31:24> : SBZ
> + * <23:16> : UFP_D pin assignment supported
> + * <15:8>  : DFP_D pin assignment supported
> + * <7>     : USB 2.0 signaling (0b=yes, 1b=no)
> + * <6>     : Plug | Receptacle (0b == plug, 1b == receptacle)
> + * <5:2>   : xxx1: Supports DPv1.3, xx1x Supports USB Gen 2 signaling
> + *           Other bits are reserved.
> + * <1:0>   : signal direction ( 00b=rsv, 01b=sink, 10b=src 11b=both )
> + */
> +#define VDO_MODE_DP(snkp, srcp, usb, gdr, sign, sdir)			\
> +	(((snkp) & 0xff) << 16 | ((srcp) & 0xff) << 8			\
> +	 | ((usb) & 1) << 7 | ((gdr) & 1) << 6 | ((sign) & 0xF) << 2	\
> +	 | ((sdir) & 0x3))
> +#define PD_DP_PIN_CAPS(x) ((((x) >> 6) & 0x1) ? (((x) >> 16) & 0x3f)	\
> +			   : (((x) >> 8) & 0x3f))
> +
> +#define MODE_DP_PIN_A		0x01
> +#define MODE_DP_PIN_B		0x02
> +#define MODE_DP_PIN_C		0x04
> +#define MODE_DP_PIN_D		0x08
> +#define MODE_DP_PIN_E		0x10
> +#define MODE_DP_PIN_F		0x20
> +
> +/* Pin configs B/D/F support multi-function */
> +#define MODE_DP_PIN_MF_MASK	0x2a
> +/* Pin configs A/B support BR2 signaling levels */
> +#define MODE_DP_PIN_BR2_MASK	0x3
> +/* Pin configs C/D/E/F support DP signaling levels */
> +#define MODE_DP_PIN_DP_MASK	0x3c
> +
> +#define MODE_DP_V13		0x1
> +#define MODE_DP_GEN2		0x2

MODE_DP_SIGN_...

>
> +
> +#define MODE_DP_SNK		0x1
> +#define MODE_DP_SRC		0x2

MODE_DP_SNK -> MODE_DP_CAP_UFP_D and MODE_DP_SRC -> MODE_DP_CAP_DFP_D
to not confuse it with power roles

>
> +#define MODE_DP_BOTH		0x3
> +
> +/*
> + * DisplayPort Status VDO
> + * ----------------------
> + * <31:9> : SBZ
> + * <8>    : IRQ_HPD : 1 == irq arrived since last message otherwise 0.
> + * <7>    : HPD state : 0 = HPD_LOW, 1 == HPD_HIGH
> + * <6>    : Exit DP Alt mode: 0 == maintain, 1 == exit
> + * <5>    : USB config : 0 == maintain current, 1 == switch to USB from DP
> + * <4>    : Multi-function preference : 0 == no pref, 1 == MF preferred.
> + * <3>    : enabled : is DPout on/off.
> + * <2>    : power low : 0 == normal or LPM disabled, 1 == DP disabled for LPM
> + * <1:0>  : connect status : 00b ==  no (DFP|UFP)_D is connected or disabled.
> + *          01b == DFP_D connected, 10b == UFP_D connected, 11b == both.
> + */
> +#define VDO_DP_STATUS(irq, lvl, amode, usbc, mf, en, lp, conn)		\
> +	(((irq) & 1) << 8 | ((lvl) & 1) << 7 | ((amode) & 1) << 6	\
> +	 | ((usbc) & 1) << 5 | ((mf) & 1) << 4 | ((en) & 1) << 3	\
> +	 | ((lp) & 1) << 2 | ((conn & 0x3) << 0))
> +
> +#define PD_VDO_DPSTS_HPD_IRQ(x)	(((x) >> 8) & 1)
> +#define PD_VDO_DPSTS_HPD_LVL(x)	(((x) >> 7) & 1)
> +#define PD_VDO_DPSTS_MF_PREF(x)	(((x) >> 4) & 1)
> +
> +/* Per DisplayPort Spec v1.3 Section 3.3, in uS */
> +#define HPD_USTREAM_DEBOUNCE_LVL	2000
> +#define HPD_USTREAM_DEBOUNCE_IRQ	250
> +#define HPD_DSTREAM_DEBOUNCE_IRQ	750	/* between 500-1000us */
> +
> +/*
> + * DisplayPort Configure VDO
> + * -------------------------
> + * <31:24> : SBZ
> + * <23:16> : SBZ
> + * <15:8>  : Pin assignment requested.  Choose one from mode caps.
> + * <7:6>   : SBZ
> + * <5:2>   : signalling : 1h == DP v1.3, 2h == Gen 2
> + *           Oh is only for USB, remaining values are reserved
> + * <1:0>   : cfg : 00 == USB, 01 == DFP_D, 10 == UFP_D, 11 == reserved
> + */
> +#define VDO_DP_CFG(pin, sig, cfg) \
> +	(((pin) & 0xff) << 8 | ((sig) & 0xf) << 2 | ((cfg) & 0x3))
> +
> +#define PD_DP_CFG_DPON(x)	((((x) & 0x3) == 1) || (((x) & 0x3) == 2))
> +/*
> + * Get the pin assignment mask
> + * for backward compatibility, if it is null,
> + * get the former sink pin assignment we used to be in <23:16>.
> + */
> +#define PD_DP_CFG_PIN(x) ((((x) >> 8) & 0xff) ? (((x) >> 8) & 0xff) \
> +					      : (((x) >> 16) & 0xff))

>
> +/*
> + * ChromeOS specific PD device Hardware IDs. Used to identify unique
> + * products and used in VDO_INFO. Note this field is 10 bits.
> + */
> +#define USB_PD_HW_DEV_ID_RESERVED	0
> +#define USB_PD_HW_DEV_ID_ZINGER		1
> +#define USB_PD_HW_DEV_ID_MINIMUFFIN	2
> +#define USB_PD_HW_DEV_ID_DINGDONG	3
> +#define USB_PD_HW_DEV_ID_HOHO		4
> +#define USB_PD_HW_DEV_ID_HONEYBUNS	5
> +
> +/*
> + * ChromeOS specific VDO_CMD_READ_INFO responds with device info including:
> + * RW Hash: First 20 bytes of SHA-256 of RW (20 bytes)
> + * HW Device ID: unique descriptor for each ChromeOS model (2 bytes)
> + *               top 6 bits are minor revision, bottom 10 bits are major
> + * SW Debug Version: Software version useful for debugging (15 bits)
> + * IS RW: True if currently in RW, False otherwise (1 bit)
> + */
> +#define VDO_INFO(id, id_minor, ver, is_rw) ((id_minor) << 26 \
> +				| ((id) & 0x3ff) << 16 \
> +				| ((ver) & 0x7fff) << 1 \
> +				| ((is_rw) & 1))
> +#define VDO_INFO_HW_DEV_ID(x)	((x) >> 16)
> +#define VDO_INFO_SW_DBG_VER(x)	(((x) >> 1) & 0x7fff)
> +#define VDO_INFO_IS_RW(x)	((x) & 1)
> +
> +#define HW_DEV_ID_MAJ(x) ((x) & 0x3ff)
> +#define HW_DEV_ID_MIN(x) ((x) >> 10)

Chrome OS stuff --> include/linux/usb/pd_vdo_chrome.h?

>
> +
> +/* USB-IF SIDs */
> +#define USB_SID_PD		0xff00 /* power delivery */
> +#define USB_SID_DISPLAYPORT	0xff01
> +#define USB_SID_MHL		0xff02	/* Mobile High-Definition Link */
> +
> +#define USB_DP_TYPEC_URL	"http://help.vesa.org/dp-usb-type-c/"
> +
> +/* USB Vendor IDs */
> +#define USB_VID_APPLE		0x05ac
> +#define USB_VID_GOOGLE		0x18d1

I don't think USB_VID_... defines belong here

>
> +
> +/* VDM command timeouts (in ms) */
> +
> +#define PD_T_VDM_UNSTRUCTURED	500
> +#define PD_T_VDM_BUSY		100
> +#define PD_T_VDM_WAIT_MODE_E	100
> +#define PD_T_VDM_SNDR_RSP	30
> +#define PD_T_VDM_E_MODE		25
> +#define PD_T_VDM_RCVR_RSP	15
> +
> +/* Alternate mode support */
> +
> +#define SVID_DISCOVERY_MAX	16
> +
> +struct pd_mode_data {

Why is this type exposed outside tcpm? and why in this file?

>
> +	int svid_index;		/* current SVID index		*/
> +	int nsvids;
> +	u16 svids[SVID_DISCOVERY_MAX];
> +	int altmodes;		/* number of alternate modes	*/
> +	struct typec_altmode_desc altmode_desc[SVID_DISCOVERY_MAX];
> +};
> +
> +#endif /* __LINUX_USB_PD_VDO_H */
>

Powered by blists - more mailing lists