* [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps
@ 2017-11-04 19:22 Badhri Jagan Sridharan
2017-11-04 19:22 ` [PATCH 2/2 v5] typec: tcpm: Only request matching pdos Badhri Jagan Sridharan
2017-11-07 11:00 ` [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps Heikki Krogerus
0 siblings, 2 replies; 6+ messages in thread
From: Badhri Jagan Sridharan @ 2017-11-04 19:22 UTC (permalink / raw)
To: heikki.krogerus, gregkh, dan.carpenter, linux
Cc: linux-usb, linux-kernel, Badhri Jagan Sridharan
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>
---
hangelog 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
drivers/usb/typec/tcpm.c | 115 +++++++++++++++++++++++++++++++++++++++++++----
include/linux/usb/pd.h | 2 +
include/linux/usb/tcpm.h | 16 +++----
3 files changed, 116 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 2b735f3e5765..8b30ab69ed79 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1256,6 +1256,85 @@ static void vdm_state_machine_work(struct work_struct *work)
mutex_unlock(&port->lock);
}
+static const char * const pdo_err[] = {
+ "",
+ " err: source/sink caps should atleast have vSafe5V",
+ " err: vSafe5V Fixed Supply Object Shall always be the first object",
+ " err: PDOs should be in the following order: Fixed; Battery; Variable",
+ " err: Fixed supply pdos should be in increasing order of their fixed voltage",
+ " err: Variable/Battery supply pdos should be in increasing order of their minimum voltage",
+ " err: Variable/Batt supply pdos cannot have same min/max voltage",
+};
+
+static int 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 1;
+
+ /* 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 2;
+
+ for (i = 1; i < nr_pdo; i++) {
+ if (pdo_type(pdo[i]) < pdo_type(pdo[i - 1])) {
+ return 3;
+ } 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 4;
+ 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 5;
+ 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 6;
+ break;
+ default:
+ tcpm_log_force(port, " Unknown pdo type");
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
+ unsigned int nr_pdo)
+{
+ int err_index = tcpm_caps_err(port, pdo, nr_pdo);
+
+ if (err_index) {
+ tcpm_log_force(port, " %s", pdo_err[err_index]);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
/*
* PD (data, control) command handling functions
*/
@@ -1278,6 +1357,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
@@ -3444,9 +3526,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) {
@@ -3466,16 +3551,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;
@@ -3494,6 +3583,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);
@@ -3529,7 +3619,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,
@@ -3584,7 +3682,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.403.gc27cc4dac6-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2 v5] typec: tcpm: Only request matching pdos
2017-11-04 19:22 [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps Badhri Jagan Sridharan
@ 2017-11-04 19:22 ` Badhri Jagan Sridharan
2017-11-07 12:07 ` Heikki Krogerus
2017-11-07 11:00 ` [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps Heikki Krogerus
1 sibling, 1 reply; 6+ messages in thread
From: Badhri Jagan Sridharan @ 2017-11-04 19:22 UTC (permalink / raw)
To: heikki.krogerus, gregkh, dan.carpenter, linux
Cc: linux-usb, linux-kernel, Badhri Jagan Sridharan
At present, TCPM code assumes that local device supports
variable/batt pdos and always selects the pdo with highest
possible power within the board limit. This assumption
might not hold good for all devices. To overcome this,
this patch makes TCPM only accept a source_pdo when there is
a matching sink pdo.
For Fixed pdos: The voltage should match between the
incoming source_cap and the registered snk_pdo
For Variable/Batt pdos: The incoming source_cap voltage
range should fall within the registered snk_pdo's voltage
range.
Also, when the cap_mismatch bit is set, the max_power/current
should be set to the max_current/power of the sink_pdo.
This is according to:
"If the Capability Mismatch bit is set to one
The Maximum Operating Current/Power field may contain a value
larger than the maximum current/power offered in the Source
Capabilities message’s PDO as referenced by the Object position field.
This enables the Sink to indicate that it requires more current/power
than is being offered. If the Sink requires a different voltage this
will be indicated by its Sink Capabilities message.
Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
---
Changelog since v1:
- Rebased the patch on top of drivers/usb/type/tcpm.c
- Added duplicate pdo check for variable/batt pdo.
- Fixed tabs as suggested by dan.carpenter@oracle.com
Changelog since v2:
- Rebase
Changelog since v3:
- Refactored code fixed formatting issues as suggested
by heikki.krogerus@linux.intel.com
Changelog since v4:
- Rebase
drivers/usb/typec/tcpm.c | 147 +++++++++++++++++++++++++++++++++++------------
1 file changed, 111 insertions(+), 36 deletions(-)
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 8b30ab69ed79..9cd8fff12809 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -261,6 +261,9 @@ struct tcpm_port {
unsigned int nr_src_pdo;
u32 snk_pdo[PDO_MAX_OBJECTS];
unsigned int nr_snk_pdo;
+ unsigned int nr_fixed; /* number of fixed sink PDOs */
+ unsigned int nr_var; /* number of variable sink PDOs */
+ unsigned int nr_batt; /* number of battery sink PDOs */
u32 snk_vdo[VDO_MAX_OBJECTS];
unsigned int nr_snk_vdo;
@@ -1761,39 +1764,86 @@ static int tcpm_pd_check_request(struct tcpm_port *port)
return 0;
}
-static int tcpm_pd_select_pdo(struct tcpm_port *port)
+#define min_power(x, y) min(pdo_max_power(x), pdo_max_power(y))
+#define min_current(x, y) min(pdo_max_current(x), pdo_max_current(y))
+
+static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo)
{
- unsigned int i, max_mw = 0, max_mv = 0;
+ unsigned int i, j, max_mw = 0, max_mv = 0, mw = 0, mv = 0, ma = 0;
int ret = -EINVAL;
/*
- * Select the source PDO providing the most power while staying within
- * the board's voltage limits. Prefer PDO providing exp
+ * Select the source PDO providing the most power which has a
+ * matchig sink cap.
*/
for (i = 0; i < port->nr_source_caps; i++) {
u32 pdo = port->source_caps[i];
enum pd_pdo_type type = pdo_type(pdo);
- unsigned int mv, ma, mw;
- if (type == PDO_TYPE_FIXED)
- mv = pdo_fixed_voltage(pdo);
- else
- mv = pdo_min_voltage(pdo);
-
- if (type == PDO_TYPE_BATT) {
- mw = pdo_max_power(pdo);
- } else {
- ma = min(pdo_max_current(pdo),
- port->max_snk_ma);
- mw = ma * mv / 1000;
- }
-
- /* Perfer higher voltages if available */
- if ((mw > max_mw || (mw == max_mw && mv > max_mv)) &&
- mv <= port->max_snk_mv) {
- ret = i;
- max_mw = mw;
- max_mv = mv;
+ if (type == PDO_TYPE_FIXED) {
+ for (j = 0; j < port->nr_fixed; j++) {
+ if (pdo_fixed_voltage(pdo) ==
+ pdo_fixed_voltage(port->snk_pdo[j])) {
+ ma = min_current(pdo, port->snk_pdo[j]);
+ mv = pdo_fixed_voltage(pdo);
+ mw = ma * mv / 1000;
+ if (mw > max_mw ||
+ (mw == max_mw && mv > max_mv)) {
+ ret = i;
+ *sink_pdo = j;
+ max_mw = mw;
+ max_mv = mv;
+ }
+ /* There could only be one fixed pdo
+ * at a specific voltage level.
+ * So breaking here.
+ */
+ break;
+ }
+ }
+ } else if (type == PDO_TYPE_BATT) {
+ for (j = port->nr_fixed;
+ j < port->nr_fixed +
+ port->nr_batt;
+ j++) {
+ if (pdo_min_voltage(pdo) >=
+ pdo_min_voltage(port->snk_pdo[j]) &&
+ pdo_max_voltage(pdo) <=
+ pdo_max_voltage(port->snk_pdo[j])) {
+ mw = min_power(pdo, port->snk_pdo[j]);
+ mv = pdo_min_voltage(pdo);
+ if (mw > max_mw ||
+ (mw == max_mw && mv > max_mv)) {
+ ret = i;
+ *sink_pdo = j;
+ max_mw = mw;
+ max_mv = mv;
+ }
+ }
+ }
+ } else if (type == PDO_TYPE_VAR) {
+ for (j = port->nr_fixed +
+ port->nr_batt;
+ j < port->nr_fixed +
+ port->nr_batt +
+ port->nr_var;
+ j++) {
+ if (pdo_min_voltage(pdo) >=
+ pdo_min_voltage(port->snk_pdo[j]) &&
+ pdo_max_voltage(pdo) <=
+ pdo_max_voltage(port->snk_pdo[j])) {
+ ma = min_current(pdo, port->snk_pdo[j]);
+ mv = pdo_min_voltage(pdo);
+ mw = ma * mv / 1000;
+ if (mw > max_mw ||
+ (mw == max_mw && mv > max_mv)) {
+ ret = i;
+ *sink_pdo = j;
+ max_mw = mw;
+ max_mv = mv;
+ }
+ }
+ }
}
}
@@ -1805,13 +1855,14 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo)
unsigned int mv, ma, mw, flags;
unsigned int max_ma, max_mw;
enum pd_pdo_type type;
- int index;
- u32 pdo;
+ int index, snk_pdo_index;
+ u32 pdo, matching_snk_pdo;
- index = tcpm_pd_select_pdo(port);
+ index = tcpm_pd_select_pdo(port, &snk_pdo_index);
if (index < 0)
return -EINVAL;
pdo = port->source_caps[index];
+ matching_snk_pdo = port->snk_pdo[snk_pdo_index];
type = pdo_type(pdo);
if (type == PDO_TYPE_FIXED)
@@ -1819,26 +1870,28 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo)
else
mv = pdo_min_voltage(pdo);
- /* Select maximum available current within the board's power limit */
+ /* Select maximum available current within the sink pdo's limit */
if (type == PDO_TYPE_BATT) {
- mw = pdo_max_power(pdo);
- ma = 1000 * min(mw, port->max_snk_mw) / mv;
+ mw = min_power(pdo, matching_snk_pdo);
+ ma = 1000 * mw / mv;
} else {
- ma = min(pdo_max_current(pdo),
- 1000 * port->max_snk_mw / mv);
+ ma = min_current(pdo, matching_snk_pdo);
+ mw = ma * mv / 1000;
}
- ma = min(ma, port->max_snk_ma);
flags = RDO_USB_COMM | RDO_NO_SUSPEND;
/* Set mismatch bit if offered power is less than operating power */
- mw = ma * mv / 1000;
max_ma = ma;
max_mw = mw;
if (mw < port->operating_snk_mw) {
flags |= RDO_CAP_MISMATCH;
- max_mw = port->operating_snk_mw;
- max_ma = max_mw * 1000 / mv;
+ if (type == PDO_TYPE_BATT &&
+ (pdo_max_power(matching_snk_pdo) > pdo_max_power(pdo)))
+ max_mw = pdo_max_power(matching_snk_pdo);
+ else if (pdo_max_current(matching_snk_pdo) >
+ pdo_max_current(pdo))
+ max_ma = pdo_max_current(matching_snk_pdo);
}
tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s polarity=%d",
@@ -3587,6 +3640,19 @@ int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo,
}
EXPORT_SYMBOL_GPL(tcpm_update_sink_capabilities);
+static int nr_type_pdos(const u32 *pdo, unsigned int nr_pdo,
+ enum pd_pdo_type type)
+{
+ int count = 0;
+ int i;
+
+ for (i = 0; i < nr_pdo; i++) {
+ if (pdo_type(pdo[i]) == type)
+ count++;
+ }
+ return count;
+}
+
struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
{
struct tcpm_port *port;
@@ -3632,6 +3698,15 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
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);
+ port->nr_var = nr_type_pdos(port->snk_pdo,
+ port->nr_snk_pdo,
+ PDO_TYPE_VAR);
+ 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);
--
2.15.0.403.gc27cc4dac6-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps
2017-11-04 19:22 [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps Badhri Jagan Sridharan
2017-11-04 19:22 ` [PATCH 2/2 v5] typec: tcpm: Only request matching pdos Badhri Jagan Sridharan
@ 2017-11-07 11:00 ` Heikki Krogerus
2017-11-13 0:05 ` Badhri Jagan Sridharan
1 sibling, 1 reply; 6+ messages in thread
From: Heikki Krogerus @ 2017-11-07 11:00 UTC (permalink / raw)
To: Badhri Jagan Sridharan
Cc: gregkh, dan.carpenter, linux, linux-usb, linux-kernel
On Sat, Nov 04, 2017 at 12:22:12PM -0700, Badhri Jagan Sridharan wrote:
> 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>
It would have been nice to have enum for the return values used in
tcpm_cap_err() IMHO, but never mind. Is Guenter OK with these?
FWIW:
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Thanks,
> ---
> hangelog 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
>
> drivers/usb/typec/tcpm.c | 115 +++++++++++++++++++++++++++++++++++++++++++----
> include/linux/usb/pd.h | 2 +
> include/linux/usb/tcpm.h | 16 +++----
> 3 files changed, 116 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 2b735f3e5765..8b30ab69ed79 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -1256,6 +1256,85 @@ static void vdm_state_machine_work(struct work_struct *work)
> mutex_unlock(&port->lock);
> }
>
> +static const char * const pdo_err[] = {
> + "",
> + " err: source/sink caps should atleast have vSafe5V",
> + " err: vSafe5V Fixed Supply Object Shall always be the first object",
> + " err: PDOs should be in the following order: Fixed; Battery; Variable",
> + " err: Fixed supply pdos should be in increasing order of their fixed voltage",
> + " err: Variable/Battery supply pdos should be in increasing order of their minimum voltage",
> + " err: Variable/Batt supply pdos cannot have same min/max voltage",
> +};
> +
> +static int 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 1;
> +
> + /* 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 2;
> +
> + for (i = 1; i < nr_pdo; i++) {
> + if (pdo_type(pdo[i]) < pdo_type(pdo[i - 1])) {
> + return 3;
> + } 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 4;
> + 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 5;
> + 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 6;
> + break;
> + default:
> + tcpm_log_force(port, " Unknown pdo type");
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
> + unsigned int nr_pdo)
> +{
> + int err_index = tcpm_caps_err(port, pdo, nr_pdo);
> +
> + if (err_index) {
> + tcpm_log_force(port, " %s", pdo_err[err_index]);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> /*
> * PD (data, control) command handling functions
> */
> @@ -1278,6 +1357,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
> @@ -3444,9 +3526,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) {
> @@ -3466,16 +3551,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;
> @@ -3494,6 +3583,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);
>
> @@ -3529,7 +3619,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,
> @@ -3584,7 +3682,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.403.gc27cc4dac6-goog
--
heikki
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v5] typec: tcpm: Only request matching pdos
2017-11-04 19:22 ` [PATCH 2/2 v5] typec: tcpm: Only request matching pdos Badhri Jagan Sridharan
@ 2017-11-07 12:07 ` Heikki Krogerus
2017-11-13 0:12 ` Badhri Jagan Sridharan
0 siblings, 1 reply; 6+ messages in thread
From: Heikki Krogerus @ 2017-11-07 12:07 UTC (permalink / raw)
To: Badhri Jagan Sridharan
Cc: gregkh, dan.carpenter, linux, linux-usb, linux-kernel
On Sat, Nov 04, 2017 at 12:22:13PM -0700, Badhri Jagan Sridharan wrote:
> At present, TCPM code assumes that local device supports
> variable/batt pdos and always selects the pdo with highest
> possible power within the board limit. This assumption
> might not hold good for all devices. To overcome this,
> this patch makes TCPM only accept a source_pdo when there is
> a matching sink pdo.
>
> For Fixed pdos: The voltage should match between the
> incoming source_cap and the registered snk_pdo
> For Variable/Batt pdos: The incoming source_cap voltage
> range should fall within the registered snk_pdo's voltage
> range.
>
> Also, when the cap_mismatch bit is set, the max_power/current
> should be set to the max_current/power of the sink_pdo.
> This is according to:
>
> "If the Capability Mismatch bit is set to one
> The Maximum Operating Current/Power field may contain a value
> larger than the maximum current/power offered in the Source
> Capabilities message???s PDO as referenced by the Object position field.
> This enables the Sink to indicate that it requires more current/power
> than is being offered. If the Sink requires a different voltage this
> will be indicated by its Sink Capabilities message.
>
> Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
> ---
> Changelog since v1:
> - Rebased the patch on top of drivers/usb/type/tcpm.c
> - Added duplicate pdo check for variable/batt pdo.
> - Fixed tabs as suggested by dan.carpenter@oracle.com
>
> Changelog since v2:
> - Rebase
>
> Changelog since v3:
> - Refactored code fixed formatting issues as suggested
> by heikki.krogerus@linux.intel.com
>
> Changelog since v4:
> - Rebase
I'm fine with this, but in case you'll prepare one more version, I'll
put a few really minor nits below. I would prefer though that Guenter
gave his ACK to any patches touching tcpm.c or tcpci.c.
But FWIW:
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Thanks,
> drivers/usb/typec/tcpm.c | 147 +++++++++++++++++++++++++++++++++++------------
> 1 file changed, 111 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 8b30ab69ed79..9cd8fff12809 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -261,6 +261,9 @@ struct tcpm_port {
> unsigned int nr_src_pdo;
> u32 snk_pdo[PDO_MAX_OBJECTS];
> unsigned int nr_snk_pdo;
> + unsigned int nr_fixed; /* number of fixed sink PDOs */
> + unsigned int nr_var; /* number of variable sink PDOs */
> + unsigned int nr_batt; /* number of battery sink PDOs */
> u32 snk_vdo[VDO_MAX_OBJECTS];
> unsigned int nr_snk_vdo;
>
> @@ -1761,39 +1764,86 @@ static int tcpm_pd_check_request(struct tcpm_port *port)
> return 0;
> }
>
> -static int tcpm_pd_select_pdo(struct tcpm_port *port)
> +#define min_power(x, y) min(pdo_max_power(x), pdo_max_power(y))
> +#define min_current(x, y) min(pdo_max_current(x), pdo_max_current(y))
> +
> +static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo)
To me it's a bit confusing that your are returning the port's source
capability index as the return value, and handling the sink pdo index
as pointer argument. It would be more clear to just return
success/failure (0/-EINVAL), and handled both indexes as arguments to
the function.
> {
> - unsigned int i, max_mw = 0, max_mv = 0;
> + unsigned int i, j, max_mw = 0, max_mv = 0, mw = 0, mv = 0, ma = 0;
> int ret = -EINVAL;
>
> /*
> - * Select the source PDO providing the most power while staying within
> - * the board's voltage limits. Prefer PDO providing exp
> + * Select the source PDO providing the most power which has a
> + * matchig sink cap.
> */
> for (i = 0; i < port->nr_source_caps; i++) {
> u32 pdo = port->source_caps[i];
> enum pd_pdo_type type = pdo_type(pdo);
> - unsigned int mv, ma, mw;
>
> - if (type == PDO_TYPE_FIXED)
> - mv = pdo_fixed_voltage(pdo);
> - else
> - mv = pdo_min_voltage(pdo);
> -
> - if (type == PDO_TYPE_BATT) {
> - mw = pdo_max_power(pdo);
> - } else {
> - ma = min(pdo_max_current(pdo),
> - port->max_snk_ma);
> - mw = ma * mv / 1000;
> - }
> -
> - /* Perfer higher voltages if available */
> - if ((mw > max_mw || (mw == max_mw && mv > max_mv)) &&
> - mv <= port->max_snk_mv) {
> - ret = i;
> - max_mw = mw;
> - max_mv = mv;
> + if (type == PDO_TYPE_FIXED) {
> + for (j = 0; j < port->nr_fixed; j++) {
> + if (pdo_fixed_voltage(pdo) ==
> + pdo_fixed_voltage(port->snk_pdo[j])) {
> + ma = min_current(pdo, port->snk_pdo[j]);
> + mv = pdo_fixed_voltage(pdo);
> + mw = ma * mv / 1000;
> + if (mw > max_mw ||
> + (mw == max_mw && mv > max_mv)) {
> + ret = i;
> + *sink_pdo = j;
> + max_mw = mw;
> + max_mv = mv;
> + }
> + /* There could only be one fixed pdo
> + * at a specific voltage level.
> + * So breaking here.
> + */
> + break;
> + }
> + }
> + } else if (type == PDO_TYPE_BATT) {
> + for (j = port->nr_fixed;
> + j < port->nr_fixed +
> + port->nr_batt;
> + j++) {
> + if (pdo_min_voltage(pdo) >=
> + pdo_min_voltage(port->snk_pdo[j]) &&
> + pdo_max_voltage(pdo) <=
> + pdo_max_voltage(port->snk_pdo[j])) {
> + mw = min_power(pdo, port->snk_pdo[j]);
> + mv = pdo_min_voltage(pdo);
> + if (mw > max_mw ||
> + (mw == max_mw && mv > max_mv)) {
> + ret = i;
> + *sink_pdo = j;
> + max_mw = mw;
> + max_mv = mv;
> + }
> + }
> + }
> + } else if (type == PDO_TYPE_VAR) {
> + for (j = port->nr_fixed +
> + port->nr_batt;
> + j < port->nr_fixed +
> + port->nr_batt +
> + port->nr_var;
> + j++) {
> + if (pdo_min_voltage(pdo) >=
> + pdo_min_voltage(port->snk_pdo[j]) &&
> + pdo_max_voltage(pdo) <=
> + pdo_max_voltage(port->snk_pdo[j])) {
> + ma = min_current(pdo, port->snk_pdo[j]);
> + mv = pdo_min_voltage(pdo);
> + mw = ma * mv / 1000;
> + if (mw > max_mw ||
> + (mw == max_mw && mv > max_mv)) {
> + ret = i;
> + *sink_pdo = j;
> + max_mw = mw;
> + max_mv = mv;
> + }
> + }
> + }
> }
> }
>
> @@ -1805,13 +1855,14 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo)
> unsigned int mv, ma, mw, flags;
> unsigned int max_ma, max_mw;
> enum pd_pdo_type type;
> - int index;
> - u32 pdo;
> + int index, snk_pdo_index;
> + u32 pdo, matching_snk_pdo;
>
> - index = tcpm_pd_select_pdo(port);
> + index = tcpm_pd_select_pdo(port, &snk_pdo_index);
> if (index < 0)
> return -EINVAL;
> pdo = port->source_caps[index];
> + matching_snk_pdo = port->snk_pdo[snk_pdo_index];
> type = pdo_type(pdo);
>
> if (type == PDO_TYPE_FIXED)
> @@ -1819,26 +1870,28 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo)
> else
> mv = pdo_min_voltage(pdo);
>
> - /* Select maximum available current within the board's power limit */
> + /* Select maximum available current within the sink pdo's limit */
> if (type == PDO_TYPE_BATT) {
> - mw = pdo_max_power(pdo);
> - ma = 1000 * min(mw, port->max_snk_mw) / mv;
> + mw = min_power(pdo, matching_snk_pdo);
> + ma = 1000 * mw / mv;
> } else {
> - ma = min(pdo_max_current(pdo),
> - 1000 * port->max_snk_mw / mv);
> + ma = min_current(pdo, matching_snk_pdo);
> + mw = ma * mv / 1000;
> }
> - ma = min(ma, port->max_snk_ma);
>
> flags = RDO_USB_COMM | RDO_NO_SUSPEND;
>
> /* Set mismatch bit if offered power is less than operating power */
> - mw = ma * mv / 1000;
> max_ma = ma;
> max_mw = mw;
> if (mw < port->operating_snk_mw) {
> flags |= RDO_CAP_MISMATCH;
> - max_mw = port->operating_snk_mw;
> - max_ma = max_mw * 1000 / mv;
> + if (type == PDO_TYPE_BATT &&
> + (pdo_max_power(matching_snk_pdo) > pdo_max_power(pdo)))
> + max_mw = pdo_max_power(matching_snk_pdo);
> + else if (pdo_max_current(matching_snk_pdo) >
> + pdo_max_current(pdo))
> + max_ma = pdo_max_current(matching_snk_pdo);
After thinking about this a bit more:
if (pdo_max_power(matching_snk_pdo) > pdo_max_power(pdo))
if (type == PDO_TYPE_BATT)
max_mw = pdo_max_power(matching_snk_pdo);
else
max_ma = pdo_max_current(matching_snk_pdo);
> }
>
> tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s polarity=%d",
> @@ -3587,6 +3640,19 @@ int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo,
> }
> EXPORT_SYMBOL_GPL(tcpm_update_sink_capabilities);
>
> +static int nr_type_pdos(const u32 *pdo, unsigned int nr_pdo,
> + enum pd_pdo_type type)
> +{
> + int count = 0;
> + int i;
> +
> + for (i = 0; i < nr_pdo; i++) {
> + if (pdo_type(pdo[i]) == type)
> + count++;
> + }
> + return count;
> +}
> +
> struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
> {
> struct tcpm_port *port;
> @@ -3632,6 +3698,15 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
> 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);
> + port->nr_var = nr_type_pdos(port->snk_pdo,
> + port->nr_snk_pdo,
> + PDO_TYPE_VAR);
> + 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);
>
> --
> 2.15.0.403.gc27cc4dac6-goog
--
heikki
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps
2017-11-07 11:00 ` [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps Heikki Krogerus
@ 2017-11-13 0:05 ` Badhri Jagan Sridharan
0 siblings, 0 replies; 6+ messages in thread
From: Badhri Jagan Sridharan @ 2017-11-13 0:05 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Greg Kroah-Hartman, Dan Carpenter, Guenter Roeck, USB, LKML
Thanks Heikki !
I am making another version addressing your nits as well.
Guenter, Are you fine with the patch as well ?
Regards,
Badhri
On Tue, Nov 7, 2017 at 3:00 AM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> On Sat, Nov 04, 2017 at 12:22:12PM -0700, Badhri Jagan Sridharan wrote:
>> 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>
>
> It would have been nice to have enum for the return values used in
> tcpm_cap_err() IMHO, but never mind. Is Guenter OK with these?
>
> FWIW:
>
> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
>
> Thanks,
>
>> ---
>> hangelog 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
>>
>> drivers/usb/typec/tcpm.c | 115 +++++++++++++++++++++++++++++++++++++++++++----
>> include/linux/usb/pd.h | 2 +
>> include/linux/usb/tcpm.h | 16 +++----
>> 3 files changed, 116 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
>> index 2b735f3e5765..8b30ab69ed79 100644
>> --- a/drivers/usb/typec/tcpm.c
>> +++ b/drivers/usb/typec/tcpm.c
>> @@ -1256,6 +1256,85 @@ static void vdm_state_machine_work(struct work_struct *work)
>> mutex_unlock(&port->lock);
>> }
>>
>> +static const char * const pdo_err[] = {
>> + "",
>> + " err: source/sink caps should atleast have vSafe5V",
>> + " err: vSafe5V Fixed Supply Object Shall always be the first object",
>> + " err: PDOs should be in the following order: Fixed; Battery; Variable",
>> + " err: Fixed supply pdos should be in increasing order of their fixed voltage",
>> + " err: Variable/Battery supply pdos should be in increasing order of their minimum voltage",
>> + " err: Variable/Batt supply pdos cannot have same min/max voltage",
>> +};
>> +
>> +static int 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 1;
>> +
>> + /* 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 2;
>> +
>> + for (i = 1; i < nr_pdo; i++) {
>> + if (pdo_type(pdo[i]) < pdo_type(pdo[i - 1])) {
>> + return 3;
>> + } 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 4;
>> + 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 5;
>> + 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 6;
>> + break;
>> + default:
>> + tcpm_log_force(port, " Unknown pdo type");
>> + }
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
>> + unsigned int nr_pdo)
>> +{
>> + int err_index = tcpm_caps_err(port, pdo, nr_pdo);
>> +
>> + if (err_index) {
>> + tcpm_log_force(port, " %s", pdo_err[err_index]);
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /*
>> * PD (data, control) command handling functions
>> */
>> @@ -1278,6 +1357,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
>> @@ -3444,9 +3526,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) {
>> @@ -3466,16 +3551,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;
>> @@ -3494,6 +3583,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);
>>
>> @@ -3529,7 +3619,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,
>> @@ -3584,7 +3682,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.403.gc27cc4dac6-goog
>
> --
> heikki
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2 v5] typec: tcpm: Only request matching pdos
2017-11-07 12:07 ` Heikki Krogerus
@ 2017-11-13 0:12 ` Badhri Jagan Sridharan
0 siblings, 0 replies; 6+ messages in thread
From: Badhri Jagan Sridharan @ 2017-11-13 0:12 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Greg Kroah-Hartman, Dan Carpenter, Guenter Roeck, USB, LKML
On Tue, Nov 7, 2017 at 4:07 AM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> On Sat, Nov 04, 2017 at 12:22:13PM -0700, Badhri Jagan Sridharan wrote:
>> At present, TCPM code assumes that local device supports
>> variable/batt pdos and always selects the pdo with highest
>> possible power within the board limit. This assumption
>> might not hold good for all devices. To overcome this,
>> this patch makes TCPM only accept a source_pdo when there is
>> a matching sink pdo.
>>
>> For Fixed pdos: The voltage should match between the
>> incoming source_cap and the registered snk_pdo
>> For Variable/Batt pdos: The incoming source_cap voltage
>> range should fall within the registered snk_pdo's voltage
>> range.
>>
>> Also, when the cap_mismatch bit is set, the max_power/current
>> should be set to the max_current/power of the sink_pdo.
>> This is according to:
>>
>> "If the Capability Mismatch bit is set to one
>> The Maximum Operating Current/Power field may contain a value
>> larger than the maximum current/power offered in the Source
>> Capabilities message???s PDO as referenced by the Object position field.
>> This enables the Sink to indicate that it requires more current/power
>> than is being offered. If the Sink requires a different voltage this
>> will be indicated by its Sink Capabilities message.
>>
>> Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
>> ---
>> Changelog since v1:
>> - Rebased the patch on top of drivers/usb/type/tcpm.c
>> - Added duplicate pdo check for variable/batt pdo.
>> - Fixed tabs as suggested by dan.carpenter@oracle.com
>>
>> Changelog since v2:
>> - Rebase
>>
>> Changelog since v3:
>> - Refactored code fixed formatting issues as suggested
>> by heikki.krogerus@linux.intel.com
>>
>> Changelog since v4:
>> - Rebase
>
> I'm fine with this, but in case you'll prepare one more version, I'll
> put a few really minor nits below. I would prefer though that Guenter
> gave his ACK to any patches touching tcpm.c or tcpci.c.
>
> But FWIW:
>
> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Thanks Heikki ! Addressing the minor nits as well.
Guenter, Are you OK with this patch ?
Regards,
Badhri
>
>
> Thanks,
>
>> drivers/usb/typec/tcpm.c | 147 +++++++++++++++++++++++++++++++++++------------
>> 1 file changed, 111 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
>> index 8b30ab69ed79..9cd8fff12809 100644
>> --- a/drivers/usb/typec/tcpm.c
>> +++ b/drivers/usb/typec/tcpm.c
>> @@ -261,6 +261,9 @@ struct tcpm_port {
>> unsigned int nr_src_pdo;
>> u32 snk_pdo[PDO_MAX_OBJECTS];
>> unsigned int nr_snk_pdo;
>> + unsigned int nr_fixed; /* number of fixed sink PDOs */
>> + unsigned int nr_var; /* number of variable sink PDOs */
>> + unsigned int nr_batt; /* number of battery sink PDOs */
>> u32 snk_vdo[VDO_MAX_OBJECTS];
>> unsigned int nr_snk_vdo;
>>
>> @@ -1761,39 +1764,86 @@ static int tcpm_pd_check_request(struct tcpm_port *port)
>> return 0;
>> }
>>
>> -static int tcpm_pd_select_pdo(struct tcpm_port *port)
>> +#define min_power(x, y) min(pdo_max_power(x), pdo_max_power(y))
>> +#define min_current(x, y) min(pdo_max_current(x), pdo_max_current(y))
>> +
>> +static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo)
>
> To me it's a bit confusing that your are returning the port's source
> capability index as the return value, and handling the sink pdo index
> as pointer argument. It would be more clear to just return
> success/failure (0/-EINVAL), and handled both indexes as arguments to
> the function.
Addressing this in the next patch.
>
>> {
>> - unsigned int i, max_mw = 0, max_mv = 0;
>> + unsigned int i, j, max_mw = 0, max_mv = 0, mw = 0, mv = 0, ma = 0;
>> int ret = -EINVAL;
>>
>> /*
>> - * Select the source PDO providing the most power while staying within
>> - * the board's voltage limits. Prefer PDO providing exp
>> + * Select the source PDO providing the most power which has a
>> + * matchig sink cap.
>> */
>> for (i = 0; i < port->nr_source_caps; i++) {
>> u32 pdo = port->source_caps[i];
>> enum pd_pdo_type type = pdo_type(pdo);
>> - unsigned int mv, ma, mw;
>>
>> - if (type == PDO_TYPE_FIXED)
>> - mv = pdo_fixed_voltage(pdo);
>> - else
>> - mv = pdo_min_voltage(pdo);
>> -
>> - if (type == PDO_TYPE_BATT) {
>> - mw = pdo_max_power(pdo);
>> - } else {
>> - ma = min(pdo_max_current(pdo),
>> - port->max_snk_ma);
>> - mw = ma * mv / 1000;
>> - }
>> -
>> - /* Perfer higher voltages if available */
>> - if ((mw > max_mw || (mw == max_mw && mv > max_mv)) &&
>> - mv <= port->max_snk_mv) {
>> - ret = i;
>> - max_mw = mw;
>> - max_mv = mv;
>> + if (type == PDO_TYPE_FIXED) {
>> + for (j = 0; j < port->nr_fixed; j++) {
>> + if (pdo_fixed_voltage(pdo) ==
>> + pdo_fixed_voltage(port->snk_pdo[j])) {
>> + ma = min_current(pdo, port->snk_pdo[j]);
>> + mv = pdo_fixed_voltage(pdo);
>> + mw = ma * mv / 1000;
>> + if (mw > max_mw ||
>> + (mw == max_mw && mv > max_mv)) {
>> + ret = i;
>> + *sink_pdo = j;
>> + max_mw = mw;
>> + max_mv = mv;
>> + }
>> + /* There could only be one fixed pdo
>> + * at a specific voltage level.
>> + * So breaking here.
>> + */
>> + break;
>> + }
>> + }
>> + } else if (type == PDO_TYPE_BATT) {
>> + for (j = port->nr_fixed;
>> + j < port->nr_fixed +
>> + port->nr_batt;
>> + j++) {
>> + if (pdo_min_voltage(pdo) >=
>> + pdo_min_voltage(port->snk_pdo[j]) &&
>> + pdo_max_voltage(pdo) <=
>> + pdo_max_voltage(port->snk_pdo[j])) {
>> + mw = min_power(pdo, port->snk_pdo[j]);
>> + mv = pdo_min_voltage(pdo);
>> + if (mw > max_mw ||
>> + (mw == max_mw && mv > max_mv)) {
>> + ret = i;
>> + *sink_pdo = j;
>> + max_mw = mw;
>> + max_mv = mv;
>> + }
>> + }
>> + }
>> + } else if (type == PDO_TYPE_VAR) {
>> + for (j = port->nr_fixed +
>> + port->nr_batt;
>> + j < port->nr_fixed +
>> + port->nr_batt +
>> + port->nr_var;
>> + j++) {
>> + if (pdo_min_voltage(pdo) >=
>> + pdo_min_voltage(port->snk_pdo[j]) &&
>> + pdo_max_voltage(pdo) <=
>> + pdo_max_voltage(port->snk_pdo[j])) {
>> + ma = min_current(pdo, port->snk_pdo[j]);
>> + mv = pdo_min_voltage(pdo);
>> + mw = ma * mv / 1000;
>> + if (mw > max_mw ||
>> + (mw == max_mw && mv > max_mv)) {
>> + ret = i;
>> + *sink_pdo = j;
>> + max_mw = mw;
>> + max_mv = mv;
>> + }
>> + }
>> + }
>> }
>> }
>>
>> @@ -1805,13 +1855,14 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo)
>> unsigned int mv, ma, mw, flags;
>> unsigned int max_ma, max_mw;
>> enum pd_pdo_type type;
>> - int index;
>> - u32 pdo;
>> + int index, snk_pdo_index;
>> + u32 pdo, matching_snk_pdo;
>>
>> - index = tcpm_pd_select_pdo(port);
>> + index = tcpm_pd_select_pdo(port, &snk_pdo_index);
>> if (index < 0)
>> return -EINVAL;
>> pdo = port->source_caps[index];
>> + matching_snk_pdo = port->snk_pdo[snk_pdo_index];
>> type = pdo_type(pdo);
>>
>> if (type == PDO_TYPE_FIXED)
>> @@ -1819,26 +1870,28 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo)
>> else
>> mv = pdo_min_voltage(pdo);
>>
>> - /* Select maximum available current within the board's power limit */
>> + /* Select maximum available current within the sink pdo's limit */
>> if (type == PDO_TYPE_BATT) {
>> - mw = pdo_max_power(pdo);
>> - ma = 1000 * min(mw, port->max_snk_mw) / mv;
>> + mw = min_power(pdo, matching_snk_pdo);
>> + ma = 1000 * mw / mv;
>> } else {
>> - ma = min(pdo_max_current(pdo),
>> - 1000 * port->max_snk_mw / mv);
>> + ma = min_current(pdo, matching_snk_pdo);
>> + mw = ma * mv / 1000;
>> }
>> - ma = min(ma, port->max_snk_ma);
>>
>> flags = RDO_USB_COMM | RDO_NO_SUSPEND;
>>
>> /* Set mismatch bit if offered power is less than operating power */
>> - mw = ma * mv / 1000;
>> max_ma = ma;
>> max_mw = mw;
>> if (mw < port->operating_snk_mw) {
>> flags |= RDO_CAP_MISMATCH;
>> - max_mw = port->operating_snk_mw;
>> - max_ma = max_mw * 1000 / mv;
>> + if (type == PDO_TYPE_BATT &&
>> + (pdo_max_power(matching_snk_pdo) > pdo_max_power(pdo)))
>> + max_mw = pdo_max_power(matching_snk_pdo);
>> + else if (pdo_max_current(matching_snk_pdo) >
>> + pdo_max_current(pdo))
>> + max_ma = pdo_max_current(matching_snk_pdo);
>
> After thinking about this a bit more:
>
> if (pdo_max_power(matching_snk_pdo) > pdo_max_power(pdo))
> if (type == PDO_TYPE_BATT)
> max_mw = pdo_max_power(matching_snk_pdo);
> else
> max_ma = pdo_max_current(matching_snk_pdo);
>
Hmm.. Only BATT pdos have max_power field, VARIABLE and FIXED PDOs
only have max_current field.
Table 6-9 Fixed Supply PDO - Source : "B9…0 Maximum Current in 10mA units"
Table 6-11 Variable Supply (non-Battery) PDO - Source : "B9…0
Maximum Current in 10mA units "
Table 6-12 Battery Supply PDO - Source : " B9…0 Maximum Allowable
Power in 250mW units"
IMHO changing it this way is not accurate.
Makes sense ?
>> }
>>
>> tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s polarity=%d",
>> @@ -3587,6 +3640,19 @@ int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo,
>> }
>> EXPORT_SYMBOL_GPL(tcpm_update_sink_capabilities);
>>
>> +static int nr_type_pdos(const u32 *pdo, unsigned int nr_pdo,
>> + enum pd_pdo_type type)
>> +{
>> + int count = 0;
>> + int i;
>> +
>> + for (i = 0; i < nr_pdo; i++) {
>> + if (pdo_type(pdo[i]) == type)
>> + count++;
>> + }
>> + return count;
>> +}
>> +
>> struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
>> {
>> struct tcpm_port *port;
>> @@ -3632,6 +3698,15 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
>> 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);
>> + port->nr_var = nr_type_pdos(port->snk_pdo,
>> + port->nr_snk_pdo,
>> + PDO_TYPE_VAR);
>> + 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);
>>
>> --
>> 2.15.0.403.gc27cc4dac6-goog
>
> --
> heikki
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-13 0:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-04 19:22 [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps Badhri Jagan Sridharan
2017-11-04 19:22 ` [PATCH 2/2 v5] typec: tcpm: Only request matching pdos Badhri Jagan Sridharan
2017-11-07 12:07 ` Heikki Krogerus
2017-11-13 0:12 ` Badhri Jagan Sridharan
2017-11-07 11:00 ` [PATCH 1/2 v5] typec: tcpm: Validate source and sink caps Heikki Krogerus
2017-11-13 0:05 ` Badhri Jagan Sridharan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).