All of lore.kernel.org
 help / color / mirror / Atom feed
From: Badhri Jagan Sridharan <badhri@google.com>
To: heikki.krogerus@linux.intel.com, gregkh@linuxfoundation.org,
	dan.carpenter@oracle.com, linux@roeck-us.net
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	Badhri Jagan Sridharan <Badhri@google.com>
Subject: [PATCH 1/2 v7] typec: tcpm: Validate source and sink caps
Date: Wed, 15 Nov 2017 17:01:55 -0800	[thread overview]
Message-ID: <20171116010156.25259-1-Badhri@google.com> (raw)

The source and sink caps should follow the following rules.
This patch validates whether the src_caps/snk_caps adheres
to it.

6.4.1 Capabilities Message
A Capabilities message (Source Capabilities message or Sink
Capabilities message) shall have at least one Power
Data Object for vSafe5V. The Capabilities message shall also
contain the sending Port’s information followed by up to
6 additional Power Data Objects. Power Data Objects in a
Capabilities message shall be sent in the following order:

1. The vSafe5V Fixed Supply Object shall always be the first object.
2. The remaining Fixed Supply Objects, if present, shall be sent
   in voltage order; lowest to highest.
3. The Battery Supply Objects, if present shall be sent in Minimum
   Voltage order; lowest to highest.
4. The Variable Supply (non-battery) Objects, if present, shall be
   sent in Minimum Voltage order; lowest to highest.

Errors in source/sink_caps of the local port will prevent
the port registration. Whereas, errors in source caps of partner
device would only log them.

Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>

---
Changelog since v1:
- rebased on top drivers/usb/typec/tcpm.c as suggested by
  gregkh@linuxfoundation.org
- renamed nr_snk_*_pdo as suggested by dan.carpenter@oracle.com
- removed stale comment as suggested by linux@roeck-us.net
- removed the tests for nr_snk_*_pdo as suggested by
  dan.carpenter@oracle.com
- Fixed sytling as suggested by dan.carpenter@oracle.com
- renamed tcpm_get_nr_type_pdos to nr_type_pdos as suggested by
  dan.carpenter@oracle.com
- fixed nr_type_pdos as suggested by dan.carpenter@oracle.com
- tcpm_pd_select_pdo now checks for all matching variable/batt pdos
  instead of the first matching one.

Changelog since v2:
- refactored error messages as suggested by
  heikki.krogerus@linux.intel.com

Changelog since v3:
- fixed formatting errors as suggested by
  heikki.krogerus@linux.intel.com

Changelog since v4:
- Reusing the macro

Changelog since v5:
- Moved to enum for pdo_cap_err messages as suggested by
 heikki.krogerus@linux.intel.com
- Ack by heikki.krogerus@linux.intel.com

Changelog since v6:
- Added Reviewed-by: Guenter Roeck <linux@roeck-us.net>

 drivers/usb/typec/tcpm.c | 130 +++++++++++++++++++++++++++++++++++++++++++----
 include/linux/usb/pd.h   |   2 +
 include/linux/usb/tcpm.h |  16 +++---
 3 files changed, 131 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index c166fc77dfb8..8b637a4b474b 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1247,6 +1247,100 @@ static void vdm_state_machine_work(struct work_struct *work)
 	mutex_unlock(&port->lock);
 }
 
+enum pdo_err {
+	PDO_NO_ERR,
+	PDO_ERR_NO_VSAFE5V,
+	PDO_ERR_VSAFE5V_NOT_FIRST,
+	PDO_ERR_PDO_TYPE_NOT_IN_ORDER,
+	PDO_ERR_FIXED_NOT_SORTED,
+	PDO_ERR_VARIABLE_BATT_NOT_SORTED,
+	PDO_ERR_DUPE_PDO,
+};
+
+static const char * const pdo_err_msg[] = {
+	[PDO_ERR_NO_VSAFE5V] =
+	" err: source/sink caps should atleast have vSafe5V",
+	[PDO_ERR_VSAFE5V_NOT_FIRST] =
+	" err: vSafe5V Fixed Supply Object Shall always be the first object",
+	[PDO_ERR_PDO_TYPE_NOT_IN_ORDER] =
+	" err: PDOs should be in the following order: Fixed; Battery; Variable",
+	[PDO_ERR_FIXED_NOT_SORTED] =
+	" err: Fixed supply pdos should be in increasing order of their fixed voltage",
+	[PDO_ERR_VARIABLE_BATT_NOT_SORTED] =
+	" err: Variable/Battery supply pdos should be in increasing order of their minimum voltage",
+	[PDO_ERR_DUPE_PDO] =
+	" err: Variable/Batt supply pdos cannot have same min/max voltage",
+};
+
+static enum pdo_err tcpm_caps_err(struct tcpm_port *port, const u32 *pdo,
+				  unsigned int nr_pdo)
+{
+	unsigned int i;
+
+	/* Should at least contain vSafe5v */
+	if (nr_pdo < 1)
+		return PDO_ERR_NO_VSAFE5V;
+
+	/* The vSafe5V Fixed Supply Object Shall always be the first object */
+	if (pdo_type(pdo[0]) != PDO_TYPE_FIXED ||
+	    pdo_fixed_voltage(pdo[0]) != VSAFE5V)
+		return PDO_ERR_VSAFE5V_NOT_FIRST;
+
+	for (i = 1; i < nr_pdo; i++) {
+		if (pdo_type(pdo[i]) < pdo_type(pdo[i - 1])) {
+			return PDO_ERR_PDO_TYPE_NOT_IN_ORDER;
+		} else if (pdo_type(pdo[i]) == pdo_type(pdo[i - 1])) {
+			enum pd_pdo_type type = pdo_type(pdo[i]);
+
+			switch (type) {
+			/*
+			 * The remaining Fixed Supply Objects, if
+			 * present, shall be sent in voltage order;
+			 * lowest to highest.
+			 */
+			case PDO_TYPE_FIXED:
+				if (pdo_fixed_voltage(pdo[i]) <=
+				    pdo_fixed_voltage(pdo[i - 1]))
+					return PDO_ERR_FIXED_NOT_SORTED;
+				break;
+			/*
+			 * The Battery Supply Objects and Variable
+			 * supply, if present shall be sent in Minimum
+			 * Voltage order; lowest to highest.
+			 */
+			case PDO_TYPE_VAR:
+			case PDO_TYPE_BATT:
+				if (pdo_min_voltage(pdo[i]) <
+				    pdo_min_voltage(pdo[i - 1]))
+					return PDO_ERR_VARIABLE_BATT_NOT_SORTED;
+				else if ((pdo_min_voltage(pdo[i]) ==
+					  pdo_min_voltage(pdo[i - 1])) &&
+					 (pdo_max_voltage(pdo[i]) ==
+					  pdo_min_voltage(pdo[i - 1])))
+					return PDO_ERR_DUPE_PDO;
+				break;
+			default:
+				tcpm_log_force(port, " Unknown pdo type");
+			}
+		}
+	}
+
+	return PDO_NO_ERR;
+}
+
+static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
+			      unsigned int nr_pdo)
+{
+	enum pdo_err err_index = tcpm_caps_err(port, pdo, nr_pdo);
+
+	if (err_index != PDO_NO_ERR) {
+		tcpm_log_force(port, " %s", pdo_err_msg[err_index]);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /*
  * PD (data, control) command handling functions
  */
@@ -1269,6 +1363,9 @@ static void tcpm_pd_data_request(struct tcpm_port *port,
 
 		tcpm_log_source_caps(port);
 
+		tcpm_validate_caps(port, port->source_caps,
+				   port->nr_source_caps);
+
 		/*
 		 * This message may be received even if VBUS is not
 		 * present. This is quite unexpected; see USB PD
@@ -3435,9 +3532,12 @@ static int tcpm_copy_vdos(u32 *dest_vdo, const u32 *src_vdo,
 	return nr_vdo;
 }
 
-void tcpm_update_source_capabilities(struct tcpm_port *port, const u32 *pdo,
-				     unsigned int nr_pdo)
+int tcpm_update_source_capabilities(struct tcpm_port *port, const u32 *pdo,
+				    unsigned int nr_pdo)
 {
+	if (tcpm_validate_caps(port, pdo, nr_pdo))
+		return -EINVAL;
+
 	mutex_lock(&port->lock);
 	port->nr_src_pdo = tcpm_copy_pdos(port->src_pdo, pdo, nr_pdo);
 	switch (port->state) {
@@ -3457,16 +3557,20 @@ void tcpm_update_source_capabilities(struct tcpm_port *port, const u32 *pdo,
 		break;
 	}
 	mutex_unlock(&port->lock);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(tcpm_update_source_capabilities);
 
-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)
+int 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)
 {
+	if (tcpm_validate_caps(port, pdo, nr_pdo))
+		return -EINVAL;
+
 	mutex_lock(&port->lock);
 	port->nr_snk_pdo = tcpm_copy_pdos(port->snk_pdo, pdo, nr_pdo);
 	port->max_snk_mv = max_snk_mv;
@@ -3485,6 +3589,7 @@ void tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo,
 		break;
 	}
 	mutex_unlock(&port->lock);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(tcpm_update_sink_capabilities);
 
@@ -3520,7 +3625,15 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 
 	init_completion(&port->tx_complete);
 	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;
+		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,
@@ -3575,7 +3688,6 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 		}
 	}
 
-	tcpm_debugfs_init(port);
 	mutex_lock(&port->lock);
 	tcpm_init(port);
 	mutex_unlock(&port->lock);
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index e00051ced806..b3d41d7409b3 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -148,6 +148,8 @@ enum pd_pdo_type {
 	(PDO_TYPE(PDO_TYPE_FIXED) | (flags) |		\
 	 PDO_FIXED_VOLT(mv) | PDO_FIXED_CURR(ma))
 
+#define VSAFE5V 5000 /* mv units */
+
 #define PDO_BATT_MAX_VOLT_SHIFT	20	/* 50mV units */
 #define PDO_BATT_MIN_VOLT_SHIFT	10	/* 50mV units */
 #define PDO_BATT_MAX_PWR_SHIFT	0	/* 250mW units */
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index 073197f0d2bb..ca1c0b57f03f 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -183,14 +183,14 @@ 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);
+int tcpm_update_source_capabilities(struct tcpm_port *port, const u32 *pdo,
+				    unsigned int nr_pdo);
+int 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);
-- 
2.15.0.448.gf294e3d99a-goog

             reply	other threads:[~2017-11-16  1:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-16  1:01 Badhri Jagan Sridharan [this message]
2017-11-16  1:01 ` [PATCH 2/2 v7] typec: tcpm: Only request matching pdos Badhri Jagan Sridharan
2017-11-23 11:10   ` Adam Thomson
2017-11-28  1:38     ` Badhri Jagan Sridharan
2017-11-28  2:06       ` Guenter Roeck
2017-11-28  9:31         ` Adam Thomson
2017-11-28  9:23       ` Adam Thomson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171116010156.25259-1-Badhri@google.com \
    --to=badhri@google.com \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@roeck-us.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.