typec: tcpm: Validate source and sink caps
Subsystems (3)
Badhri Jagan Sridharan
maintainerPATCH v3 1/2 typec: tcpm: Validate source and sink caps
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.
---
Changelog since v1:
- rebased on top drivers/usb/typec/tcpm.c as suggested by [email protected]
- renamed nr_snk_*_pdo as suggested by [email protected]
- removed stale comment as suggested by [email protected]
- removed the tests for nr_snk_*_pdo as suggested by [email protected]
- Fixed sytling as suggested by [email protected]
- renamed tcpm_get_nr_type_pdos to nr_type_pdos as suggested by [email protected]
- fixed nr_type_pdos as suggested by [email protected]
- 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
[email protected]
drivers/usb/typec/tcpm.c | 116 +++++++++++++++++++++++++++++++++++++++++++---- include/linux/usb/pd.h | 2 + include/linux/usb/tcpm.h | 4 +- 3 files changed, 111 insertions(+), 11 deletions(-)
drivers/usb/typec/tcpm.c +107 -9
@@ -1256,6 +1256,86 @@ 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 +1358,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 +3527,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 +3552,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 +3584,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 +3620,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 +3683,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);
include/linux/usb/pd.h +2 -0
@@ -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 */
include/linux/usb/tcpm.h +2 -2
@@ -183,9 +183,9 @@ 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, +int 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, +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,
Signature · 1 line
2.15.0.rc1.287.g2b38de12cc-goog
Badhri Jagan Sridharan
maintainerPATCH 2/2 typec: tcpm: Only request matching pdos
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.
---
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 [email protected]
Changelog since v2:
- Rebase
drivers/usb/typec/tcpm.c | 155 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 120 insertions(+), 35 deletions(-)
drivers/usb/typec/tcpm.c +120 -35
@@ -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;
@@ -1762,39 +1765,92 @@ static int tcpm_pd_check_request(struct tcpm_port *port)return 0 ; } -static int tcpm_pd_select_pdo (struct tcpm_port *port) +static void tcpm_pd_evaluate_pdo (struct tcpm_port *port,int src_pdo_index, +int snk_pdo_index,int *max_mw,int *max_mv, +int *selected_src_pdo, +int *matching_snk_pdo) { -unsigned int i, max_mw =0 , max_mv =0 ; +unsigned int mv =0 , ma =0 , mw =0 ; +u32 src_pdo = port->source_caps[src_pdo_index]; +u32 snk_pdo = port->snk_pdo[snk_pdo_index]; +enum pd_pdo_type type =pdo_type (src_pdo); + + mv = (type ==PDO_TYPE_FIXED ) ?pdo_fixed_voltage (src_pdo) : +pdo_min_voltage (src_pdo); + +if (type ==PDO_TYPE_BATT ) { + mw =min (pdo_max_power (src_pdo),pdo_max_power (snk_pdo)); + }else { + ma =min (pdo_max_current (src_pdo),pdo_max_current (snk_pdo)); + mw = ma * mv /1000 ; + } + +/* Perfer higher voltages if available */ +if (mw > *max_mw || (mw == *max_mw && mv > *max_mv)) { + *selected_src_pdo = src_pdo_index; + *matching_snk_pdo = snk_pdo_index; + *max_mw = mw; + *max_mv = mv; + } +} + +static int tcpm_pd_select_pdo (struct tcpm_port *port,int *sink_pdo) +{ +unsigned int i, j, max_mw =0 , max_mv =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])) { +tcpm_pd_evaluate_pdo (port, i, j, + &max_mw, &max_mv, + &ret, sink_pdo); +/* 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])) { +tcpm_pd_evaluate_pdo (port, i, j, + &max_mw, &max_mv, + &ret, sink_pdo); + } + } + }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])) { +tcpm_pd_evaluate_pdo (port, i, j, + &max_mw, &max_mv, + &ret, sink_pdo); + } + } } }
@@ -1806,13 +1862,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 )
@@ -1820,26 +1877,32 @@ 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 (pdo_max_power (pdo),pdo_max_power (matching_snk_pdo)); + ma =1000 * mw / mv; }else { ma =min (pdo_max_current (pdo), -1000 * port->max_snk_mw / mv); +pdo_max_current (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 ) { +if (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",
@@ -3588,6 +3651,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;
@@ -3633,6 +3709,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);
Signature · 1 line
2.15.0.rc1.287.g2b38de12cc-goog
Heikki Krogerus
maintainerRe: [PATCH 2/2] typec: tcpm: Only request matching pdos
Hi,
+Dan and Guenter
I'm sorry for the late reply. These slipped under my radar. I do have a one more proposal below, and a few nits..
Quoted 157 lines · 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_pdowhen there is a matching sink pdo.For Fixed pdos: The voltage should match between the incoming
source_capand the registeredsnk_pdoFor Variable/Batt pdos: The incomingsource_capvoltage range should fall within the registered snk_pdo's voltage range.Also, when the
cap_mismatchbit is set, the max_power/current should be set to the max_current/power of thesink_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 <[email protected]>---
Changelog since v1:
- Rebased the patch on top ofdrivers/usb/type/tcpm.c
- Added duplicate pdo check for variable/batt pdo.
- Fixed tabs as suggested by [email protected]Changelog since v2:
- Rebasedrivers/usb/typec/tcpm.c | 155 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 120 insertions(+), 35 deletions(-)drivers/usb/typec/tcpm.c +71
-24@@ -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;@@ -1762,39 +1765,92 @@ static int tcpm_pd_check_request(struct tcpm_port *port)return 0 ; } -static int tcpm_pd_select_pdo (struct tcpm_port *port) +static void tcpm_pd_evaluate_pdo (struct tcpm_port *port,int src_pdo_index, +int snk_pdo_index,int *max_mw,int *max_mv, +int *selected_src_pdo, +int *matching_snk_pdo) { -unsigned int i, max_mw =0 , max_mv =0 ; +unsigned int mv =0 , ma =0 , mw =0 ; +u32 src_pdo = port->source_caps[src_pdo_index]; +u32 snk_pdo = port->snk_pdo[snk_pdo_index]; +enum pd_pdo_type type =pdo_type (src_pdo); + + mv = (type ==PDO_TYPE_FIXED ) ?pdo_fixed_voltage (src_pdo) : +pdo_min_voltage (src_pdo); + +if (type ==PDO_TYPE_BATT ) { + mw =min (pdo_max_power (src_pdo),pdo_max_power (snk_pdo)); + }else { + ma =min (pdo_max_current (src_pdo),pdo_max_current (snk_pdo)); + mw = ma * mv /1000 ; + } + +/* Perfer higher voltages if available */ +if (mw > *max_mw || (mw == *max_mw && mv > *max_mv)) { + *selected_src_pdo = src_pdo_index; + *matching_snk_pdo = snk_pdo_index; + *max_mw = mw; + *max_mv = mv; + } +} + +static int tcpm_pd_select_pdo (struct tcpm_port *port,int *sink_pdo) +{ +unsigned int i, j, max_mw =0 , max_mv =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])) { +tcpm_pd_evaluate_pdo (port, i, j, + &max_mw, &max_mv, + &ret, sink_pdo); +/* 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])) { +tcpm_pd_evaluate_pdo (port, i, j, + &max_mw, &max_mv, + &ret, sink_pdo); + } + } + }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++) {
drivers/usb/typec/tcpm.c static int tcpm_pd_check_request(struct tcpm_port *port) · line 1854
Quoted 10 lines
drivers/usb/typec/tcpm.c +9
-0+if (pdo_min_voltage (pdo) >= +pdo_min_voltage (port->snk_pdo[j]) && +pdo_max_voltage (pdo) <= +pdo_max_voltage (port->snk_pdo[j])) { +tcpm_pd_evaluate_pdo (port, i, j, + &max_mw, &max_mv, + &ret, sink_pdo); + } + } }
Wouldn't it make sense to call tcpm_pd_evaluate_pdo() here ones and run those nested loops there? You are already checking the type there in any case.
Quoted 48 lines
drivers/usb/typec/tcpm.c +9
-10}@@ -1806,13 +1862,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 )@@ -1820,26 +1877,32 @@ 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 (pdo_max_power (pdo),pdo_max_power (matching_snk_pdo)); + ma =1000 * mw / mv; }else { ma =min (pdo_max_current (pdo), -1000 * port->max_snk_mw / mv); +pdo_max_current (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;
drivers/usb/typec/tcpm.c static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo) · line 1905
Quoted 10 lines
drivers/usb/typec/tcpm.c +9
-1- max_ma = max_mw *1000 / mv; +if (type ==PDO_TYPE_BATT ) { +if (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); + }
How about:
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);Though, that does not make it any more readable. Just a proposal.
drivers/usb/typec/tcpm.c int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo, · line 3655
Quoted 9 lines
drivers/usb/typec/tcpm.c +2
-0} tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s polarity=%d",@@ -3588,6 +3651,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)
Please align that properly.
Quoted 15 lines
drivers/usb/typec/tcpm.c +11
-0+{ +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;
drivers/usb/typec/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 3714
Quoted 6 lines
drivers/usb/typec/tcpm.c +3
-0@@ -3633,6 +3709,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 );
Ditto.
drivers/usb/typec/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 3717
Quoted 3 lines
drivers/usb/typec/tcpm.c +3
-0+ port->nr_var =nr_type_pdos (port->snk_pdo, + port->nr_snk_pdo, +PDO_TYPE_VAR );
Ditto.
drivers/usb/typec/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 3720
Quoted 3 lines
drivers/usb/typec/tcpm.c +3
-0+ port->nr_batt =nr_type_pdos (port->snk_pdo, + port->nr_snk_pdo, +PDO_TYPE_BATT );
Ditto.
Thanks,
Signature · 1 line
heikki
Badhri Jagan Sridharan
maintainerRe: [PATCH 2/2] typec: tcpm: Only request matching pdos
Quoted 171 lines · Heikki Krogerus
Hi,
+Dan and Guenter
I'm sorry for the late reply. These slipped under my radar. I do have a one more proposal below, and a few nits..
Quoted 163 lines · 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_pdowhen there is a matching sink pdo.For Fixed pdos: The voltage should match between the incoming
source_capand the registeredsnk_pdoFor Variable/Batt pdos: The incomingsource_capvoltage range should fall within the registered snk_pdo's voltage range.Also, when the
cap_mismatchbit is set, the max_power/current should be set to the max_current/power of thesink_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 <[email protected]>---
Changelog since v1:
- Rebased the patch on top ofdrivers/usb/type/tcpm.c
- Added duplicate pdo check for variable/batt pdo.
- Fixed tabs as suggested by [email protected]Changelog since v2:
- Rebasedrivers/usb/typec/tcpm.c | 155 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 120 insertions(+), 35 deletions(-)drivers/usb/typec/tcpm.c +77
-24@@ -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;@@ -1762,39 +1765,92 @@ static int tcpm_pd_check_request(struct tcpm_port *port)return 0 ; } -static int tcpm_pd_select_pdo (struct tcpm_port *port) +static void tcpm_pd_evaluate_pdo (struct tcpm_port *port,int src_pdo_index, +int snk_pdo_index,int *max_mw,int *max_mv, +int *selected_src_pdo, +int *matching_snk_pdo) { -unsigned int i, max_mw =0 , max_mv =0 ; +unsigned int mv =0 , ma =0 , mw =0 ; +u32 src_pdo = port->source_caps[src_pdo_index]; +u32 snk_pdo = port->snk_pdo[snk_pdo_index]; +enum pd_pdo_type type =pdo_type (src_pdo); + + mv = (type ==PDO_TYPE_FIXED ) ?pdo_fixed_voltage (src_pdo) : +pdo_min_voltage (src_pdo); + +if (type ==PDO_TYPE_BATT ) { + mw =min (pdo_max_power (src_pdo),pdo_max_power (snk_pdo)); + }else { + ma =min (pdo_max_current (src_pdo),pdo_max_current (snk_pdo)); + mw = ma * mv /1000 ; + } + +/* Perfer higher voltages if available */ +if (mw > *max_mw || (mw == *max_mw && mv > *max_mv)) { + *selected_src_pdo = src_pdo_index; + *matching_snk_pdo = snk_pdo_index; + *max_mw = mw; + *max_mv = mv; + } +} + +static int tcpm_pd_select_pdo (struct tcpm_port *port,int *sink_pdo) +{ +unsigned int i, j, max_mw =0 , max_mv =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])) { +tcpm_pd_evaluate_pdo (port, i, j, + &max_mw, &max_mv, + &ret, sink_pdo); +/* 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])) { +tcpm_pd_evaluate_pdo (port, i, j, + &max_mw, &max_mv, + &ret, sink_pdo); + } + } + }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])) { + tcpm_pd_evaluate_pdo(port, i, j, + &max_mw, &max_mv,
Quoted 8 lines
drivers/usb/typec/tcpm.c static int tcpm_pd_check_request(struct tcpm_port *port) · line 1854
Quoted 4 lines
drivers/usb/typec/tcpm.c +3
-0+ &ret, sink_pdo); + } + } }Wouldn't it make sense to call
tcpm_pd_evaluate_pdo()here ones and run those nested loops there? You are already checking the type there in any case.
Combined into one function and introduced couple of macros to keep up with the character limit instead. Let me know your thoughts.
Quoted 62 lines
Quoted 49 lines
drivers/usb/typec/tcpm.c +9
-10}@@ -1806,13 +1862,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 )@@ -1820,26 +1877,32 @@ 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 (pdo_max_power (pdo),pdo_max_power (matching_snk_pdo)); + ma =1000 * mw / mv; }else { ma =min (pdo_max_current (pdo), -1000 * port->max_snk_mw / mv); +pdo_max_current (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;drivers/usb/typec/tcpm.c static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo) · line 1905
Quoted 10 lines
drivers/usb/typec/tcpm.c +9
-1- max_ma = max_mw *1000 / mv; +if (type ==PDO_TYPE_BATT ) { +if (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); + }How about:
Quoted 7 lines
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);Though, that does not make it any more readable. Just a proposal.
Sure. Will do in next patch.
Quoted 41 lines
drivers/usb/typec/tcpm.c int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo, · line 3655
Quoted 10 lines
drivers/usb/typec/tcpm.c +2
-0} tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s polarity=%d",@@ -3588,6 +3651,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)Please align that properly.
Quoted 15 lines
drivers/usb/typec/tcpm.c +11
-0+{ +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;drivers/usb/typec/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 3714
Quoted 6 lines
drivers/usb/typec/tcpm.c +3
-0@@ -3633,6 +3709,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 );Ditto.
drivers/usb/typec/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 3717
Quoted 3 lines
drivers/usb/typec/tcpm.c +3
-0+ port->nr_var =nr_type_pdos (port->snk_pdo, + port->nr_snk_pdo, +PDO_TYPE_VAR );
Quoted 7 lines
Ditto.
drivers/usb/typec/tcpm.c struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) · line 3720
Quoted 3 lines
drivers/usb/typec/tcpm.c +3
-0+ port->nr_batt =nr_type_pdos (port->snk_pdo, + port->nr_snk_pdo, +PDO_TYPE_BATT );Ditto.
Will fix other styling issues as well.
Thanks.
Quoted 6 lines
Thanks,
--
heikki
Greg KH
maintainerQuoted 20 lines · 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.
Quoted 5 lines
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 <[email protected]>
I need someone else to review this...
{hint...}
Heikki Krogerus
maintainer+Dan and Guenter,
I don't have any problems with this, but since I had those few comments for the 2/2 of this series, few nitpics below..
Quoted 59 lines · 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 <[email protected]>---
Changelog since v1:
- rebased on topdrivers/usb/typec/tcpm.cas suggested by [email protected]
- renamed nr_snk_*_pdo as suggested by [email protected]
- removed stale comment as suggested by [email protected]
- removed the tests for nr_snk_*_pdo as suggested by [email protected]
- Fixed sytling as suggested by [email protected]
- renamedtcpm_get_nr_type_pdostonr_type_pdosas suggested by [email protected]
- fixednr_type_pdosas suggested by [email protected]
-tcpm_pd_select_pdonow checks for all matching variable/batt pdos instead of the first matching one.Changelog since v2:
- refactored error messages as suggested by
[email protected]drivers/usb/typec/tcpm.c | 116 +++++++++++++++++++++++++++++++++++++++++++---- include/linux/usb/pd.h | 2 + include/linux/usb/tcpm.h | 4 +- 3 files changed, 111 insertions(+), 11 deletions(-)drivers/usb/typec/tcpm.c +2
-0@@ -1256,6 +1256,86 @@ static void vdm_state_machine_work(struct work_struct *work)mutex_unlock (&port->lock); } +static const char *const pdo_err[] = { +"" ,
drivers/usb/typec/tcpm.c static void vdm_state_machine_work(struct work_struct *work) · line 1270
Quoted 10 lines
drivers/usb/typec/tcpm.c +10
-0+" 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)
Alignment.
Quoted 58 lines
drivers/usb/typec/tcpm.c +58
-0+{ +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);
drivers/usb/typec/tcpm.c static void vdm_state_machine_work(struct work_struct *work) · line 1338
Quoted 10 lines
drivers/usb/typec/tcpm.c +10
-0+ +if (err_index) { +tcpm_log_force (port," %s" , pdo_err[err_index]); +return -EINVAL ; + } + +return 0 ; +} + +
Extra line.
Quoted 106 lines
drivers/usb/typec/tcpm.c +27
-9/* * PD (data, control) command handling functions */ @@ -1278,6 +1358,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 +3527,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 +3552,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 +3584,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 +3620,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 +3683,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);include/linux/usb/pd.h +2
-0@@ -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 */ include/linux/usb/tcpm.h +0
-0@@ -183,9 +183,9 @@ struct tcpm_port;struct tcpm_port *tcpm_register_port (struct device *dev,struct tcpc_dev *tcpc);
include/linux/usb/tcpm.h struct tcpm_port; · line 191
Quoted 10 lines
include/linux/usb/tcpm.h +2
-2void tcpm_unregister_port (struct tcpm_port *port); -void tcpm_update_source_capabilities (struct tcpm_port *port,const u32 *pdo, +int 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, +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,
I think that's also misaligned.
You can run the checkpatch.pl script with --strict option and it will check also things like alignment and extra lines.
Thanks,
Signature · 1 line
heikki