HI,
On 13-03-18 12:36, Hans de Goede wrote:
Hi,
On 13-03-18 01:02, Li Jun wrote:
This patch tries to fix the problem describled on revert patch commit[1],
suppose any supported voltage ranges of sink should be describled by
variable pdo, so instead of revert the patch of only comparing source and
sink pdo to select one source pdo, this patch adds the match between source
fixed pdo and sink variable pdo.
[1] https://www.spinics.net/lists/linux-usb/msg166366.html
CC: Hans de Goede <hdegoede@xxxxxxxxxx>
CC: Guenter Roeck <linux@xxxxxxxxxxxx>
CC: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
CC: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx>
CC: Badhri Jagan Sridharan <Badhri@xxxxxxxxxx>
Signed-off-by: Li Jun <jun.li@xxxxxxx>
---
drivers/usb/typec/tcpm.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index ef3a60d..8a74a43 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1815,6 +1815,37 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
break;
}
}
+
+ /*
+ * If the source pdo has the same voltage with one
+ * fixed pdo, no need check variable pdo for it.
+ */
+ if (j < port->nr_fixed)
+ continue;
+
+ for (j = port->nr_fixed +
+ port->nr_batt;
+ j < port->nr_fixed +
+ port->nr_batt +
+ port->nr_var;
+ j++) {
+ if (pdo_fixed_voltage(pdo) >=
+ pdo_min_voltage(port->snk_pdo[j]) &&
+ pdo_fixed_voltage(pdo) <=
+ pdo_max_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 = 0;
+ *src_pdo = i;
+ *sink_pdo = j;
+ max_mw = mw;
+ max_mv = mv;
+ }
+ }
+ }
} else if (type == PDO_TYPE_BATT) {
for (j = port->nr_fixed;
j < port->nr_fixed +
First of all, this seems to be a fix on top of your previous,
reverted patch.
Since your patch has been reverted, please send a new fixed patch
replacing it.
As for the proposed fix, you are just fixing the fixed source,
variable snk cap case here. What if things are the other way
around, so fixed snk, variable source?
You really need to stop comparing fixed to fixed and variable
to variable, etc.
Instead do something like this
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 {
/* FIXME should not use max_snk_ma here */
ma = min(pdo_max_current(pdo),
port->max_snk_ma);
mw = ma * mv / 1000;
}
for (j = 0; j < port->nr_snk_pdo; j++) {
if (pdo_type(port->snk_pdo[j]) == PDO_TYPE_FIXED)
max_snk_mv = pdo_fixed_voltage(port->snk_pdo[j]);
/* FIXME also get ma / mw settings */
} else {
max_snk_mv = pdo_max_voltage(port->snk_pdo[j]);
/* FIXME also get ma / mw settings */
}
/* Prefer higher voltages if available */
if ((mw > max_mw || (mw == max_mw && mv > max_mv)) &&
mv <= max_snk_mv) {
ret = i;
max_mw = mw;
max_mv = mv;
}
}
}
Note the above example code only has been adjusted to compare the voltages
from the snk pdo-s it needs to be fixed to also deal with ma/mw
Oh and for your use-case where the snk can only handle certain voltages you
also need to have a min_snk_mv, so basically this:
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 {
/* FIXME should not use max_snk_ma here */
ma = min(pdo_max_current(pdo),
port->max_snk_ma);
mw = ma * mv / 1000;
}
for (j = 0; j < port->nr_snk_pdo; j++) {
if (pdo_type(port->snk_pdo[j]) == PDO_TYPE_FIXED)
min_snk_mv = pdo_fixed_voltage(port->snk_pdo[j]);
max_snk_mv = pdo_fixed_voltage(port->snk_pdo[j]);
/* FIXME also get ma / mw settings */
} else {
min_snk_mv = pdo_min_voltage(port->snk_pdo[j]);
max_snk_mv = pdo_max_voltage(port->snk_pdo[j]);
/* FIXME also get ma / mw settings */
}
/* Prefer higher voltages if available */
if ((mw > max_mw || (mw == max_mw && mv > max_mv)) &&
mv >= min_snk_mv && mv <= max_snk_mv) {
ret = i;
max_mw = mw;
max_mv = mv;
}
}
}
Note the above example code only has been adjusted to compare the voltages
from the snk pdo-s it needs to be fixed to also deal with ma/mw
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
