| Message ID | 20230731162159.19483-1-kyletso@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 40f362ffa5e9ddf413825c99e9121db0ab59301f |
| Headers | show |
| Series | usb: typec: tcpm: Refactor the PPS APDO selection | expand |
On Tue, Aug 01, 2023 at 12:21:59AM +0800, Kyle Tso wrote: > In current design of the PPS APDO selection, TCPM power supply only > accepts the requested voltage which is inside the range of the selected > PPS profile. To extend the flexibility and usability, remove the checks > about the voltage range in current profile. And try to search all PPS > APDOs of the Source that fit the requested voltage. > > Also remove some redundant checks in tcpm_pd_build_pps_request. > > Signed-off-by: Kyle Tso <kyletso@google.com> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/tcpm/tcpm.c | 122 ++++++---------------------------- > 1 file changed, 21 insertions(+), 101 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index 829d75ebab42..9c496b8302b4 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -3253,23 +3253,12 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo, > return ret; > } > > -#define min_pps_apdo_current(x, y) \ > - min(pdo_pps_apdo_max_current(x), pdo_pps_apdo_max_current(y)) > - > static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) > { > - unsigned int i, j, max_mw = 0, max_mv = 0; > - unsigned int min_src_mv, max_src_mv, src_ma, src_mw; > - unsigned int min_snk_mv, max_snk_mv; > - unsigned int max_op_mv; > - u32 pdo, src, snk; > - unsigned int src_pdo = 0, snk_pdo = 0; > + unsigned int i, src_ma, max_temp_mw = 0, max_op_ma, op_mw; > + unsigned int src_pdo = 0; > + u32 pdo, src; > > - /* > - * Select the source PPS APDO providing the most power while staying > - * within the board's limits. We skip the first PDO as this is always > - * 5V 3A. > - */ > for (i = 1; i < port->nr_source_caps; ++i) { > pdo = port->source_caps[i]; > > @@ -3280,54 +3269,17 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) > continue; > } > > - min_src_mv = pdo_pps_apdo_min_voltage(pdo); > - max_src_mv = pdo_pps_apdo_max_voltage(pdo); > - src_ma = pdo_pps_apdo_max_current(pdo); > - src_mw = (src_ma * max_src_mv) / 1000; > - > - /* > - * Now search through the sink PDOs to find a matching > - * PPS APDO. Again skip the first sink PDO as this will > - * always be 5V 3A. > - */ > - for (j = 1; j < port->nr_snk_pdo; j++) { > - pdo = port->snk_pdo[j]; > - > - switch (pdo_type(pdo)) { > - case PDO_TYPE_APDO: > - if (pdo_apdo_type(pdo) != APDO_TYPE_PPS) { > - tcpm_log(port, > - "Not PPS APDO (sink), ignoring"); > - continue; > - } > - > - min_snk_mv = > - pdo_pps_apdo_min_voltage(pdo); > - max_snk_mv = > - pdo_pps_apdo_max_voltage(pdo); > - break; > - default: > - tcpm_log(port, > - "Not APDO type (sink), ignoring"); > - continue; > - } > + if (port->pps_data.req_out_volt > pdo_pps_apdo_max_voltage(pdo) || > + port->pps_data.req_out_volt < pdo_pps_apdo_min_voltage(pdo)) > + continue; > > - if (min_src_mv <= max_snk_mv && > - max_src_mv >= min_snk_mv) { > - max_op_mv = min(max_src_mv, max_snk_mv); > - src_mw = (max_op_mv * src_ma) / 1000; > - /* Prefer higher voltages if available */ > - if ((src_mw == max_mw && > - max_op_mv > max_mv) || > - src_mw > max_mw) { > - src_pdo = i; > - snk_pdo = j; > - max_mw = src_mw; > - max_mv = max_op_mv; > - } > - } > + src_ma = pdo_pps_apdo_max_current(pdo); > + max_op_ma = min(src_ma, port->pps_data.req_op_curr); > + op_mw = max_op_ma * port->pps_data.req_out_volt / 1000; > + if (op_mw > max_temp_mw) { > + src_pdo = i; > + max_temp_mw = op_mw; > } > - > break; > default: > tcpm_log(port, "Not APDO type (source), ignoring"); > @@ -3337,16 +3289,10 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) > > if (src_pdo) { > src = port->source_caps[src_pdo]; > - snk = port->snk_pdo[snk_pdo]; > - > - port->pps_data.req_min_volt = max(pdo_pps_apdo_min_voltage(src), > - pdo_pps_apdo_min_voltage(snk)); > - port->pps_data.req_max_volt = min(pdo_pps_apdo_max_voltage(src), > - pdo_pps_apdo_max_voltage(snk)); > - port->pps_data.req_max_curr = min_pps_apdo_current(src, snk); > - port->pps_data.req_out_volt = min(port->pps_data.req_max_volt, > - max(port->pps_data.req_min_volt, > - port->pps_data.req_out_volt)); > + > + port->pps_data.req_min_volt = pdo_pps_apdo_min_voltage(src); > + port->pps_data.req_max_volt = pdo_pps_apdo_max_voltage(src); > + port->pps_data.req_max_curr = pdo_pps_apdo_max_current(src); > port->pps_data.req_op_curr = min(port->pps_data.req_max_curr, > port->pps_data.req_op_curr); > } > @@ -3464,32 +3410,16 @@ static int tcpm_pd_send_request(struct tcpm_port *port) > static int tcpm_pd_build_pps_request(struct tcpm_port *port, u32 *rdo) > { > unsigned int out_mv, op_ma, op_mw, max_mv, max_ma, flags; > - enum pd_pdo_type type; > unsigned int src_pdo_index; > - u32 pdo; > > src_pdo_index = tcpm_pd_select_pps_apdo(port); > if (!src_pdo_index) > return -EOPNOTSUPP; > > - pdo = port->source_caps[src_pdo_index]; > - type = pdo_type(pdo); > - > - switch (type) { > - case PDO_TYPE_APDO: > - if (pdo_apdo_type(pdo) != APDO_TYPE_PPS) { > - tcpm_log(port, "Invalid APDO selected!"); > - return -EINVAL; > - } > - max_mv = port->pps_data.req_max_volt; > - max_ma = port->pps_data.req_max_curr; > - out_mv = port->pps_data.req_out_volt; > - op_ma = port->pps_data.req_op_curr; > - break; > - default: > - tcpm_log(port, "Invalid PDO selected!"); > - return -EINVAL; > - } > + max_mv = port->pps_data.req_max_volt; > + max_ma = port->pps_data.req_max_curr; > + out_mv = port->pps_data.req_out_volt; > + op_ma = port->pps_data.req_op_curr; > > flags = RDO_USB_COMM | RDO_NO_SUSPEND; > > @@ -5882,12 +5812,6 @@ static int tcpm_pps_set_out_volt(struct tcpm_port *port, u16 req_out_volt) > goto port_unlock; > } > > - if (req_out_volt < port->pps_data.min_volt || > - req_out_volt > port->pps_data.max_volt) { > - ret = -EINVAL; > - goto port_unlock; > - } > - > target_mw = (port->current_limit * req_out_volt) / 1000; > if (target_mw < port->operating_snk_mw) { > ret = -EINVAL; > @@ -6440,11 +6364,7 @@ static int tcpm_psy_set_prop(struct power_supply *psy, > ret = tcpm_psy_set_online(port, val); > break; > case POWER_SUPPLY_PROP_VOLTAGE_NOW: > - if (val->intval < port->pps_data.min_volt * 1000 || > - val->intval > port->pps_data.max_volt * 1000) > - ret = -EINVAL; > - else > - ret = tcpm_pps_set_out_volt(port, val->intval / 1000); > + ret = tcpm_pps_set_out_volt(port, val->intval / 1000); > break; > case POWER_SUPPLY_PROP_CURRENT_NOW: > if (val->intval > port->pps_data.max_curr * 1000) > -- > 2.41.0.487.g6d72f3e995-goog
On 7/31/23 09:21, Kyle Tso wrote: > In current design of the PPS APDO selection, TCPM power supply only > accepts the requested voltage which is inside the range of the selected > PPS profile. To extend the flexibility and usability, remove the checks > about the voltage range in current profile. And try to search all PPS > APDOs of the Source that fit the requested voltage. > > Also remove some redundant checks in tcpm_pd_build_pps_request. > > Signed-off-by: Kyle Tso <kyletso@google.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/usb/typec/tcpm/tcpm.c | 122 ++++++---------------------------- > 1 file changed, 21 insertions(+), 101 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index 829d75ebab42..9c496b8302b4 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -3253,23 +3253,12 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo, > return ret; > } > > -#define min_pps_apdo_current(x, y) \ > - min(pdo_pps_apdo_max_current(x), pdo_pps_apdo_max_current(y)) > - > static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) > { > - unsigned int i, j, max_mw = 0, max_mv = 0; > - unsigned int min_src_mv, max_src_mv, src_ma, src_mw; > - unsigned int min_snk_mv, max_snk_mv; > - unsigned int max_op_mv; > - u32 pdo, src, snk; > - unsigned int src_pdo = 0, snk_pdo = 0; > + unsigned int i, src_ma, max_temp_mw = 0, max_op_ma, op_mw; > + unsigned int src_pdo = 0; > + u32 pdo, src; > > - /* > - * Select the source PPS APDO providing the most power while staying > - * within the board's limits. We skip the first PDO as this is always > - * 5V 3A. > - */ > for (i = 1; i < port->nr_source_caps; ++i) { > pdo = port->source_caps[i]; > > @@ -3280,54 +3269,17 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) > continue; > } > > - min_src_mv = pdo_pps_apdo_min_voltage(pdo); > - max_src_mv = pdo_pps_apdo_max_voltage(pdo); > - src_ma = pdo_pps_apdo_max_current(pdo); > - src_mw = (src_ma * max_src_mv) / 1000; > - > - /* > - * Now search through the sink PDOs to find a matching > - * PPS APDO. Again skip the first sink PDO as this will > - * always be 5V 3A. > - */ > - for (j = 1; j < port->nr_snk_pdo; j++) { > - pdo = port->snk_pdo[j]; > - > - switch (pdo_type(pdo)) { > - case PDO_TYPE_APDO: > - if (pdo_apdo_type(pdo) != APDO_TYPE_PPS) { > - tcpm_log(port, > - "Not PPS APDO (sink), ignoring"); > - continue; > - } > - > - min_snk_mv = > - pdo_pps_apdo_min_voltage(pdo); > - max_snk_mv = > - pdo_pps_apdo_max_voltage(pdo); > - break; > - default: > - tcpm_log(port, > - "Not APDO type (sink), ignoring"); > - continue; > - } > + if (port->pps_data.req_out_volt > pdo_pps_apdo_max_voltage(pdo) || > + port->pps_data.req_out_volt < pdo_pps_apdo_min_voltage(pdo)) > + continue; > > - if (min_src_mv <= max_snk_mv && > - max_src_mv >= min_snk_mv) { > - max_op_mv = min(max_src_mv, max_snk_mv); > - src_mw = (max_op_mv * src_ma) / 1000; > - /* Prefer higher voltages if available */ > - if ((src_mw == max_mw && > - max_op_mv > max_mv) || > - src_mw > max_mw) { > - src_pdo = i; > - snk_pdo = j; > - max_mw = src_mw; > - max_mv = max_op_mv; > - } > - } > + src_ma = pdo_pps_apdo_max_current(pdo); > + max_op_ma = min(src_ma, port->pps_data.req_op_curr); > + op_mw = max_op_ma * port->pps_data.req_out_volt / 1000; > + if (op_mw > max_temp_mw) { > + src_pdo = i; > + max_temp_mw = op_mw; > } > - > break; > default: > tcpm_log(port, "Not APDO type (source), ignoring"); > @@ -3337,16 +3289,10 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) > > if (src_pdo) { > src = port->source_caps[src_pdo]; > - snk = port->snk_pdo[snk_pdo]; > - > - port->pps_data.req_min_volt = max(pdo_pps_apdo_min_voltage(src), > - pdo_pps_apdo_min_voltage(snk)); > - port->pps_data.req_max_volt = min(pdo_pps_apdo_max_voltage(src), > - pdo_pps_apdo_max_voltage(snk)); > - port->pps_data.req_max_curr = min_pps_apdo_current(src, snk); > - port->pps_data.req_out_volt = min(port->pps_data.req_max_volt, > - max(port->pps_data.req_min_volt, > - port->pps_data.req_out_volt)); > + > + port->pps_data.req_min_volt = pdo_pps_apdo_min_voltage(src); > + port->pps_data.req_max_volt = pdo_pps_apdo_max_voltage(src); > + port->pps_data.req_max_curr = pdo_pps_apdo_max_current(src); > port->pps_data.req_op_curr = min(port->pps_data.req_max_curr, > port->pps_data.req_op_curr); > } > @@ -3464,32 +3410,16 @@ static int tcpm_pd_send_request(struct tcpm_port *port) > static int tcpm_pd_build_pps_request(struct tcpm_port *port, u32 *rdo) > { > unsigned int out_mv, op_ma, op_mw, max_mv, max_ma, flags; > - enum pd_pdo_type type; > unsigned int src_pdo_index; > - u32 pdo; > > src_pdo_index = tcpm_pd_select_pps_apdo(port); > if (!src_pdo_index) > return -EOPNOTSUPP; > > - pdo = port->source_caps[src_pdo_index]; > - type = pdo_type(pdo); > - > - switch (type) { > - case PDO_TYPE_APDO: > - if (pdo_apdo_type(pdo) != APDO_TYPE_PPS) { > - tcpm_log(port, "Invalid APDO selected!"); > - return -EINVAL; > - } > - max_mv = port->pps_data.req_max_volt; > - max_ma = port->pps_data.req_max_curr; > - out_mv = port->pps_data.req_out_volt; > - op_ma = port->pps_data.req_op_curr; > - break; > - default: > - tcpm_log(port, "Invalid PDO selected!"); > - return -EINVAL; > - } > + max_mv = port->pps_data.req_max_volt; > + max_ma = port->pps_data.req_max_curr; > + out_mv = port->pps_data.req_out_volt; > + op_ma = port->pps_data.req_op_curr; > > flags = RDO_USB_COMM | RDO_NO_SUSPEND; > > @@ -5882,12 +5812,6 @@ static int tcpm_pps_set_out_volt(struct tcpm_port *port, u16 req_out_volt) > goto port_unlock; > } > > - if (req_out_volt < port->pps_data.min_volt || > - req_out_volt > port->pps_data.max_volt) { > - ret = -EINVAL; > - goto port_unlock; > - } > - > target_mw = (port->current_limit * req_out_volt) / 1000; > if (target_mw < port->operating_snk_mw) { > ret = -EINVAL; > @@ -6440,11 +6364,7 @@ static int tcpm_psy_set_prop(struct power_supply *psy, > ret = tcpm_psy_set_online(port, val); > break; > case POWER_SUPPLY_PROP_VOLTAGE_NOW: > - if (val->intval < port->pps_data.min_volt * 1000 || > - val->intval > port->pps_data.max_volt * 1000) > - ret = -EINVAL; > - else > - ret = tcpm_pps_set_out_volt(port, val->intval / 1000); > + ret = tcpm_pps_set_out_volt(port, val->intval / 1000); > break; > case POWER_SUPPLY_PROP_CURRENT_NOW: > if (val->intval > port->pps_data.max_curr * 1000)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 829d75ebab42..9c496b8302b4 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -3253,23 +3253,12 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo, return ret; } -#define min_pps_apdo_current(x, y) \ - min(pdo_pps_apdo_max_current(x), pdo_pps_apdo_max_current(y)) - static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) { - unsigned int i, j, max_mw = 0, max_mv = 0; - unsigned int min_src_mv, max_src_mv, src_ma, src_mw; - unsigned int min_snk_mv, max_snk_mv; - unsigned int max_op_mv; - u32 pdo, src, snk; - unsigned int src_pdo = 0, snk_pdo = 0; + unsigned int i, src_ma, max_temp_mw = 0, max_op_ma, op_mw; + unsigned int src_pdo = 0; + u32 pdo, src; - /* - * Select the source PPS APDO providing the most power while staying - * within the board's limits. We skip the first PDO as this is always - * 5V 3A. - */ for (i = 1; i < port->nr_source_caps; ++i) { pdo = port->source_caps[i]; @@ -3280,54 +3269,17 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) continue; } - min_src_mv = pdo_pps_apdo_min_voltage(pdo); - max_src_mv = pdo_pps_apdo_max_voltage(pdo); - src_ma = pdo_pps_apdo_max_current(pdo); - src_mw = (src_ma * max_src_mv) / 1000; - - /* - * Now search through the sink PDOs to find a matching - * PPS APDO. Again skip the first sink PDO as this will - * always be 5V 3A. - */ - for (j = 1; j < port->nr_snk_pdo; j++) { - pdo = port->snk_pdo[j]; - - switch (pdo_type(pdo)) { - case PDO_TYPE_APDO: - if (pdo_apdo_type(pdo) != APDO_TYPE_PPS) { - tcpm_log(port, - "Not PPS APDO (sink), ignoring"); - continue; - } - - min_snk_mv = - pdo_pps_apdo_min_voltage(pdo); - max_snk_mv = - pdo_pps_apdo_max_voltage(pdo); - break; - default: - tcpm_log(port, - "Not APDO type (sink), ignoring"); - continue; - } + if (port->pps_data.req_out_volt > pdo_pps_apdo_max_voltage(pdo) || + port->pps_data.req_out_volt < pdo_pps_apdo_min_voltage(pdo)) + continue; - if (min_src_mv <= max_snk_mv && - max_src_mv >= min_snk_mv) { - max_op_mv = min(max_src_mv, max_snk_mv); - src_mw = (max_op_mv * src_ma) / 1000; - /* Prefer higher voltages if available */ - if ((src_mw == max_mw && - max_op_mv > max_mv) || - src_mw > max_mw) { - src_pdo = i; - snk_pdo = j; - max_mw = src_mw; - max_mv = max_op_mv; - } - } + src_ma = pdo_pps_apdo_max_current(pdo); + max_op_ma = min(src_ma, port->pps_data.req_op_curr); + op_mw = max_op_ma * port->pps_data.req_out_volt / 1000; + if (op_mw > max_temp_mw) { + src_pdo = i; + max_temp_mw = op_mw; } - break; default: tcpm_log(port, "Not APDO type (source), ignoring"); @@ -3337,16 +3289,10 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port) if (src_pdo) { src = port->source_caps[src_pdo]; - snk = port->snk_pdo[snk_pdo]; - - port->pps_data.req_min_volt = max(pdo_pps_apdo_min_voltage(src), - pdo_pps_apdo_min_voltage(snk)); - port->pps_data.req_max_volt = min(pdo_pps_apdo_max_voltage(src), - pdo_pps_apdo_max_voltage(snk)); - port->pps_data.req_max_curr = min_pps_apdo_current(src, snk); - port->pps_data.req_out_volt = min(port->pps_data.req_max_volt, - max(port->pps_data.req_min_volt, - port->pps_data.req_out_volt)); + + port->pps_data.req_min_volt = pdo_pps_apdo_min_voltage(src); + port->pps_data.req_max_volt = pdo_pps_apdo_max_voltage(src); + port->pps_data.req_max_curr = pdo_pps_apdo_max_current(src); port->pps_data.req_op_curr = min(port->pps_data.req_max_curr, port->pps_data.req_op_curr); } @@ -3464,32 +3410,16 @@ static int tcpm_pd_send_request(struct tcpm_port *port) static int tcpm_pd_build_pps_request(struct tcpm_port *port, u32 *rdo) { unsigned int out_mv, op_ma, op_mw, max_mv, max_ma, flags; - enum pd_pdo_type type; unsigned int src_pdo_index; - u32 pdo; src_pdo_index = tcpm_pd_select_pps_apdo(port); if (!src_pdo_index) return -EOPNOTSUPP; - pdo = port->source_caps[src_pdo_index]; - type = pdo_type(pdo); - - switch (type) { - case PDO_TYPE_APDO: - if (pdo_apdo_type(pdo) != APDO_TYPE_PPS) { - tcpm_log(port, "Invalid APDO selected!"); - return -EINVAL; - } - max_mv = port->pps_data.req_max_volt; - max_ma = port->pps_data.req_max_curr; - out_mv = port->pps_data.req_out_volt; - op_ma = port->pps_data.req_op_curr; - break; - default: - tcpm_log(port, "Invalid PDO selected!"); - return -EINVAL; - } + max_mv = port->pps_data.req_max_volt; + max_ma = port->pps_data.req_max_curr; + out_mv = port->pps_data.req_out_volt; + op_ma = port->pps_data.req_op_curr; flags = RDO_USB_COMM | RDO_NO_SUSPEND; @@ -5882,12 +5812,6 @@ static int tcpm_pps_set_out_volt(struct tcpm_port *port, u16 req_out_volt) goto port_unlock; } - if (req_out_volt < port->pps_data.min_volt || - req_out_volt > port->pps_data.max_volt) { - ret = -EINVAL; - goto port_unlock; - } - target_mw = (port->current_limit * req_out_volt) / 1000; if (target_mw < port->operating_snk_mw) { ret = -EINVAL; @@ -6440,11 +6364,7 @@ static int tcpm_psy_set_prop(struct power_supply *psy, ret = tcpm_psy_set_online(port, val); break; case POWER_SUPPLY_PROP_VOLTAGE_NOW: - if (val->intval < port->pps_data.min_volt * 1000 || - val->intval > port->pps_data.max_volt * 1000) - ret = -EINVAL; - else - ret = tcpm_pps_set_out_volt(port, val->intval / 1000); + ret = tcpm_pps_set_out_volt(port, val->intval / 1000); break; case POWER_SUPPLY_PROP_CURRENT_NOW: if (val->intval > port->pps_data.max_curr * 1000)
In current design of the PPS APDO selection, TCPM power supply only accepts the requested voltage which is inside the range of the selected PPS profile. To extend the flexibility and usability, remove the checks about the voltage range in current profile. And try to search all PPS APDOs of the Source that fit the requested voltage. Also remove some redundant checks in tcpm_pd_build_pps_request. Signed-off-by: Kyle Tso <kyletso@google.com> --- drivers/usb/typec/tcpm/tcpm.c | 122 ++++++---------------------------- 1 file changed, 21 insertions(+), 101 deletions(-)