All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] usb: typec: tcpm: Extend the matching rules on PPS APDO selection
@ 2018-12-17 13:22 ` Kyle Tso
  0 siblings, 0 replies; 6+ messages in thread
From: Kyle Tso @ 2018-12-17 13:22 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, Adam.Thomson.Opensource
  Cc: badhri, linux-usb, linux-kernel, Kyle Tso

Current matching rules ensure that the voltage range of selected Source
Capability is entirely within the range defined in one of the Sink
Capabilities. This is reasonable but not practical because Sink may not
support wide range of voltage when sinking power while Source could
advertise its capabilities in relatively wider range. For example, a
Source PDO advertising 3.3V-11V@3A (9V Prog of Fixed Nominal Voltage)
will not be selected if the Sink requires 5V-12V@3A PPS power. However,
the Sink could work well if the requested voltage range in RDOs is
5V-11V@3A.

Currently accepted:
		|--------- source -----|
	|----------- sink ---------------|

Currently not accepted:
			|--------- source -----|
	|----------- sink ---------------|

	|--------- source -----|
		|----------- sink ---------------|

	|--------- source -----------------|
		|------ sink -------|

To improve the usability, change the matching rules to what listed
below:
a. The Source PDO is selectable if any portion of the voltage range
   overlaps one of the Sink PDO's voltage range.
b. The maximum operational voltage will be the lower one between the
   selected Source PDO and the matching Sink PDO.
c. The maximum power will be the maximum operational voltage times the
   maximum current defined in the selected Source PDO
d. Select the Source PDO with the highest maximum power

Signed-off-by: Kyle Tso <kyletso@google.com>

---
Changelog since v1:
- updated the commit message as suggested by Guenter Roeck <linux@roeck-us.net>

Changelog since v2:
- fixed the coding style problems as suggested by Heikki Krogerus
  <heikki.krogerus@linux.intel.com>
- Corrected a wrong word in the commit message as suggested by Adam Thomson
  <Adam.Thomson.Opensource@diasemi.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 3620efee2688..4bc29b586698 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -2213,7 +2213,8 @@ 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;
-	u32 pdo;
+	unsigned int max_op_mv;
+	u32 pdo, src, snk;
 	unsigned int src_pdo = 0, snk_pdo = 0;
 
 	/*
@@ -2263,16 +2264,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port)
 					continue;
 				}
 
-				if (max_src_mv <= max_snk_mv &&
-				    min_src_mv >= min_snk_mv) {
+				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 &&
-					     min_src_mv > max_mv) ||
+					     max_op_mv > max_mv) ||
 					    src_mw > max_mw) {
 						src_pdo = i;
 						snk_pdo = j;
 						max_mw = src_mw;
-						max_mv = max_src_mv;
+						max_mv = max_op_mv;
 					}
 				}
 			}
@@ -2285,16 +2288,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port)
 	}
 
 	if (src_pdo) {
-		pdo = port->source_caps[src_pdo];
-
-		port->pps_data.min_volt = pdo_pps_apdo_min_voltage(pdo);
-		port->pps_data.max_volt = pdo_pps_apdo_max_voltage(pdo);
-		port->pps_data.max_curr =
-			min_pps_apdo_current(pdo, port->snk_pdo[snk_pdo]);
-		port->pps_data.out_volt =
-			min(pdo_pps_apdo_max_voltage(pdo), port->pps_data.out_volt);
-		port->pps_data.op_curr =
-			min(port->pps_data.max_curr, port->pps_data.op_curr);
+		src = port->source_caps[src_pdo];
+		snk = port->snk_pdo[snk_pdo];
+
+		port->pps_data.min_volt = max(pdo_pps_apdo_min_voltage(src),
+					      pdo_pps_apdo_min_voltage(snk));
+		port->pps_data.max_volt = min(pdo_pps_apdo_max_voltage(src),
+					      pdo_pps_apdo_max_voltage(snk));
+		port->pps_data.max_curr = min_pps_apdo_current(src, snk);
+		port->pps_data.out_volt = min(port->pps_data.max_volt,
+					      port->pps_data.out_volt);
+		port->pps_data.op_curr = min(port->pps_data.max_curr,
+					     port->pps_data.op_curr);
 	}
 
 	return src_pdo;
-- 
2.20.0.405.gbc1bbc6f85-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [v3] usb: typec: tcpm: Extend the matching rules on PPS APDO selection
@ 2018-12-17 13:22 ` Kyle Tso
  0 siblings, 0 replies; 6+ messages in thread
From: Kyle Tso @ 2018-12-17 13:22 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, Adam.Thomson.Opensource
  Cc: badhri, linux-usb, linux-kernel, Kyle Tso

Current matching rules ensure that the voltage range of selected Source
Capability is entirely within the range defined in one of the Sink
Capabilities. This is reasonable but not practical because Sink may not
support wide range of voltage when sinking power while Source could
advertise its capabilities in relatively wider range. For example, a
Source PDO advertising 3.3V-11V@3A (9V Prog of Fixed Nominal Voltage)
will not be selected if the Sink requires 5V-12V@3A PPS power. However,
the Sink could work well if the requested voltage range in RDOs is
5V-11V@3A.

Currently accepted:
		|--------- source -----|
	|----------- sink ---------------|

Currently not accepted:
			|--------- source -----|
	|----------- sink ---------------|

	|--------- source -----|
		|----------- sink ---------------|

	|--------- source -----------------|
		|------ sink -------|

To improve the usability, change the matching rules to what listed
below:
a. The Source PDO is selectable if any portion of the voltage range
   overlaps one of the Sink PDO's voltage range.
b. The maximum operational voltage will be the lower one between the
   selected Source PDO and the matching Sink PDO.
c. The maximum power will be the maximum operational voltage times the
   maximum current defined in the selected Source PDO
d. Select the Source PDO with the highest maximum power

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changelog since v1:
- updated the commit message as suggested by Guenter Roeck <linux@roeck-us.net>

Changelog since v2:
- fixed the coding style problems as suggested by Heikki Krogerus
  <heikki.krogerus@linux.intel.com>
- Corrected a wrong word in the commit message as suggested by Adam Thomson
  <Adam.Thomson.Opensource@diasemi.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 3620efee2688..4bc29b586698 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -2213,7 +2213,8 @@ 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;
-	u32 pdo;
+	unsigned int max_op_mv;
+	u32 pdo, src, snk;
 	unsigned int src_pdo = 0, snk_pdo = 0;
 
 	/*
@@ -2263,16 +2264,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port)
 					continue;
 				}
 
-				if (max_src_mv <= max_snk_mv &&
-				    min_src_mv >= min_snk_mv) {
+				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 &&
-					     min_src_mv > max_mv) ||
+					     max_op_mv > max_mv) ||
 					    src_mw > max_mw) {
 						src_pdo = i;
 						snk_pdo = j;
 						max_mw = src_mw;
-						max_mv = max_src_mv;
+						max_mv = max_op_mv;
 					}
 				}
 			}
@@ -2285,16 +2288,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port)
 	}
 
 	if (src_pdo) {
-		pdo = port->source_caps[src_pdo];
-
-		port->pps_data.min_volt = pdo_pps_apdo_min_voltage(pdo);
-		port->pps_data.max_volt = pdo_pps_apdo_max_voltage(pdo);
-		port->pps_data.max_curr =
-			min_pps_apdo_current(pdo, port->snk_pdo[snk_pdo]);
-		port->pps_data.out_volt =
-			min(pdo_pps_apdo_max_voltage(pdo), port->pps_data.out_volt);
-		port->pps_data.op_curr =
-			min(port->pps_data.max_curr, port->pps_data.op_curr);
+		src = port->source_caps[src_pdo];
+		snk = port->snk_pdo[snk_pdo];
+
+		port->pps_data.min_volt = max(pdo_pps_apdo_min_voltage(src),
+					      pdo_pps_apdo_min_voltage(snk));
+		port->pps_data.max_volt = min(pdo_pps_apdo_max_voltage(src),
+					      pdo_pps_apdo_max_voltage(snk));
+		port->pps_data.max_curr = min_pps_apdo_current(src, snk);
+		port->pps_data.out_volt = min(port->pps_data.max_volt,
+					      port->pps_data.out_volt);
+		port->pps_data.op_curr = min(port->pps_data.max_curr,
+					     port->pps_data.op_curr);
 	}
 
 	return src_pdo;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH v3] usb: typec: tcpm: Extend the matching rules on PPS APDO selection
@ 2018-12-17 14:44   ` Opensource [Adam Thomson]
  0 siblings, 0 replies; 6+ messages in thread
From: Adam Thomson @ 2018-12-17 14:44 UTC (permalink / raw)
  To: Kyle Tso, linux, heikki.krogerus, gregkh, Adam Thomson
  Cc: badhri, linux-usb, linux-kernel

On 17 December 2018 13:22, Kyle Tso wrote:

> Current matching rules ensure that the voltage range of selected Source
> Capability is entirely within the range defined in one of the Sink Capabilities. This
> is reasonable but not practical because Sink may not support wide range of
> voltage when sinking power while Source could advertise its capabilities in
> relatively wider range. For example, a Source PDO advertising 3.3V-11V@3A (9V
> Prog of Fixed Nominal Voltage) will not be selected if the Sink requires 5V-
> 12V@3A PPS power. However, the Sink could work well if the requested voltage
> range in RDOs is 5V-11V@3A.
> 
> Currently accepted:
> 		|--------- source -----|
> 	|----------- sink ---------------|
> 
> Currently not accepted:
> 			|--------- source -----|
> 	|----------- sink ---------------|
> 
> 	|--------- source -----|
> 		|----------- sink ---------------|
> 
> 	|--------- source -----------------|
> 		|------ sink -------|
> 
> To improve the usability, change the matching rules to what listed
> below:
> a. The Source PDO is selectable if any portion of the voltage range
>    overlaps one of the Sink PDO's voltage range.
> b. The maximum operational voltage will be the lower one between the
>    selected Source PDO and the matching Sink PDO.
> c. The maximum power will be the maximum operational voltage times the
>    maximum current defined in the selected Source PDO d. Select the Source PDO
> with the highest maximum power
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Acked-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> 
> ---
> Changelog since v1:
> - updated the commit message as suggested by Guenter Roeck <linux@roeck-
> us.net>
> 
> Changelog since v2:
> - fixed the coding style problems as suggested by Heikki Krogerus
>   <heikki.krogerus@linux.intel.com>
> - Corrected a wrong word in the commit message as suggested by Adam
> Thomson
>   <Adam.Thomson.Opensource@diasemi.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 3620efee2688..4bc29b586698 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2213,7 +2213,8 @@ 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;
> -	u32 pdo;
> +	unsigned int max_op_mv;
> +	u32 pdo, src, snk;
>  	unsigned int src_pdo = 0, snk_pdo = 0;
> 
>  	/*
> @@ -2263,16 +2264,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct
> tcpm_port *port)
>  					continue;
>  				}
> 
> -				if (max_src_mv <= max_snk_mv &&
> -				    min_src_mv >= min_snk_mv) {
> +				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 &&
> -					     min_src_mv > max_mv) ||
> +					     max_op_mv > max_mv) ||
>  					    src_mw > max_mw) {
>  						src_pdo = i;
>  						snk_pdo = j;
>  						max_mw = src_mw;
> -						max_mv = max_src_mv;
> +						max_mv = max_op_mv;
>  					}
>  				}
>  			}
> @@ -2285,16 +2288,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct
> tcpm_port *port)
>  	}
> 
>  	if (src_pdo) {
> -		pdo = port->source_caps[src_pdo];
> -
> -		port->pps_data.min_volt = pdo_pps_apdo_min_voltage(pdo);
> -		port->pps_data.max_volt = pdo_pps_apdo_max_voltage(pdo);
> -		port->pps_data.max_curr =
> -			min_pps_apdo_current(pdo, port->snk_pdo[snk_pdo]);
> -		port->pps_data.out_volt =
> -			min(pdo_pps_apdo_max_voltage(pdo), port-
> >pps_data.out_volt);
> -		port->pps_data.op_curr =
> -			min(port->pps_data.max_curr, port->pps_data.op_curr);
> +		src = port->source_caps[src_pdo];
> +		snk = port->snk_pdo[snk_pdo];
> +
> +		port->pps_data.min_volt =
> max(pdo_pps_apdo_min_voltage(src),
> +					      pdo_pps_apdo_min_voltage(snk));
> +		port->pps_data.max_volt =
> min(pdo_pps_apdo_max_voltage(src),
> +					      pdo_pps_apdo_max_voltage(snk));
> +		port->pps_data.max_curr = min_pps_apdo_current(src, snk);
> +		port->pps_data.out_volt = min(port->pps_data.max_volt,
> +					      port->pps_data.out_volt);
> +		port->pps_data.op_curr = min(port->pps_data.max_curr,
> +					     port->pps_data.op_curr);
>  	}
> 
>  	return src_pdo;
> --
> 2.20.0.405.gbc1bbc6f85-goog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [v3] usb: typec: tcpm: Extend the matching rules on PPS APDO selection
@ 2018-12-17 14:44   ` Opensource [Adam Thomson]
  0 siblings, 0 replies; 6+ messages in thread
From: Opensource [Adam Thomson] @ 2018-12-17 14:44 UTC (permalink / raw)
  To: Kyle Tso, linux, heikki.krogerus, gregkh, Adam Thomson
  Cc: badhri, linux-usb, linux-kernel

On 17 December 2018 13:22, Kyle Tso wrote:

> Current matching rules ensure that the voltage range of selected Source
> Capability is entirely within the range defined in one of the Sink Capabilities. This
> is reasonable but not practical because Sink may not support wide range of
> voltage when sinking power while Source could advertise its capabilities in
> relatively wider range. For example, a Source PDO advertising 3.3V-11V@3A (9V
> Prog of Fixed Nominal Voltage) will not be selected if the Sink requires 5V-
> 12V@3A PPS power. However, the Sink could work well if the requested voltage
> range in RDOs is 5V-11V@3A.
> 
> Currently accepted:
> 		|--------- source -----|
> 	|----------- sink ---------------|
> 
> Currently not accepted:
> 			|--------- source -----|
> 	|----------- sink ---------------|
> 
> 	|--------- source -----|
> 		|----------- sink ---------------|
> 
> 	|--------- source -----------------|
> 		|------ sink -------|
> 
> To improve the usability, change the matching rules to what listed
> below:
> a. The Source PDO is selectable if any portion of the voltage range
>    overlaps one of the Sink PDO's voltage range.
> b. The maximum operational voltage will be the lower one between the
>    selected Source PDO and the matching Sink PDO.
> c. The maximum power will be the maximum operational voltage times the
>    maximum current defined in the selected Source PDO d. Select the Source PDO
> with the highest maximum power
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Acked-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>

> 
> ---
> Changelog since v1:
> - updated the commit message as suggested by Guenter Roeck <linux@roeck-
> us.net>
> 
> Changelog since v2:
> - fixed the coding style problems as suggested by Heikki Krogerus
>   <heikki.krogerus@linux.intel.com>
> - Corrected a wrong word in the commit message as suggested by Adam
> Thomson
>   <Adam.Thomson.Opensource@diasemi.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 3620efee2688..4bc29b586698 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2213,7 +2213,8 @@ 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;
> -	u32 pdo;
> +	unsigned int max_op_mv;
> +	u32 pdo, src, snk;
>  	unsigned int src_pdo = 0, snk_pdo = 0;
> 
>  	/*
> @@ -2263,16 +2264,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct
> tcpm_port *port)
>  					continue;
>  				}
> 
> -				if (max_src_mv <= max_snk_mv &&
> -				    min_src_mv >= min_snk_mv) {
> +				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 &&
> -					     min_src_mv > max_mv) ||
> +					     max_op_mv > max_mv) ||
>  					    src_mw > max_mw) {
>  						src_pdo = i;
>  						snk_pdo = j;
>  						max_mw = src_mw;
> -						max_mv = max_src_mv;
> +						max_mv = max_op_mv;
>  					}
>  				}
>  			}
> @@ -2285,16 +2288,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct
> tcpm_port *port)
>  	}
> 
>  	if (src_pdo) {
> -		pdo = port->source_caps[src_pdo];
> -
> -		port->pps_data.min_volt = pdo_pps_apdo_min_voltage(pdo);
> -		port->pps_data.max_volt = pdo_pps_apdo_max_voltage(pdo);
> -		port->pps_data.max_curr =
> -			min_pps_apdo_current(pdo, port->snk_pdo[snk_pdo]);
> -		port->pps_data.out_volt =
> -			min(pdo_pps_apdo_max_voltage(pdo), port-
> >pps_data.out_volt);
> -		port->pps_data.op_curr =
> -			min(port->pps_data.max_curr, port->pps_data.op_curr);
> +		src = port->source_caps[src_pdo];
> +		snk = port->snk_pdo[snk_pdo];
> +
> +		port->pps_data.min_volt =
> max(pdo_pps_apdo_min_voltage(src),
> +					      pdo_pps_apdo_min_voltage(snk));
> +		port->pps_data.max_volt =
> min(pdo_pps_apdo_max_voltage(src),
> +					      pdo_pps_apdo_max_voltage(snk));
> +		port->pps_data.max_curr = min_pps_apdo_current(src, snk);
> +		port->pps_data.out_volt = min(port->pps_data.max_volt,
> +					      port->pps_data.out_volt);
> +		port->pps_data.op_curr = min(port->pps_data.max_curr,
> +					     port->pps_data.op_curr);
>  	}
> 
>  	return src_pdo;
> --
> 2.20.0.405.gbc1bbc6f85-goog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] usb: typec: tcpm: Extend the matching rules on PPS APDO selection
@ 2018-12-18 12:00   ` Heikki Krogerus
  0 siblings, 0 replies; 6+ messages in thread
From: Heikki Krogerus @ 2018-12-18 12:00 UTC (permalink / raw)
  To: Kyle Tso
  Cc: linux, gregkh, Adam.Thomson.Opensource, badhri, linux-usb, linux-kernel

On Mon, Dec 17, 2018 at 09:22:13PM +0800, Kyle Tso wrote:
> Current matching rules ensure that the voltage range of selected Source
> Capability is entirely within the range defined in one of the Sink
> Capabilities. This is reasonable but not practical because Sink may not
> support wide range of voltage when sinking power while Source could
> advertise its capabilities in relatively wider range. For example, a
> Source PDO advertising 3.3V-11V@3A (9V Prog of Fixed Nominal Voltage)
> will not be selected if the Sink requires 5V-12V@3A PPS power. However,
> the Sink could work well if the requested voltage range in RDOs is
> 5V-11V@3A.
> 
> Currently accepted:
> 		|--------- source -----|
> 	|----------- sink ---------------|
> 
> Currently not accepted:
> 			|--------- source -----|
> 	|----------- sink ---------------|
> 
> 	|--------- source -----|
> 		|----------- sink ---------------|
> 
> 	|--------- source -----------------|
> 		|------ sink -------|
> 
> To improve the usability, change the matching rules to what listed
> below:
> a. The Source PDO is selectable if any portion of the voltage range
>    overlaps one of the Sink PDO's voltage range.
> b. The maximum operational voltage will be the lower one between the
>    selected Source PDO and the matching Sink PDO.
> c. The maximum power will be the maximum operational voltage times the
>    maximum current defined in the selected Source PDO
> d. Select the Source PDO with the highest maximum power
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changelog since v1:
> - updated the commit message as suggested by Guenter Roeck <linux@roeck-us.net>
> 
> Changelog since v2:
> - fixed the coding style problems as suggested by Heikki Krogerus
>   <heikki.krogerus@linux.intel.com>
> - Corrected a wrong word in the commit message as suggested by Adam Thomson
>   <Adam.Thomson.Opensource@diasemi.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 3620efee2688..4bc29b586698 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2213,7 +2213,8 @@ 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;
> -	u32 pdo;
> +	unsigned int max_op_mv;
> +	u32 pdo, src, snk;
>  	unsigned int src_pdo = 0, snk_pdo = 0;
>  
>  	/*
> @@ -2263,16 +2264,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port)
>  					continue;
>  				}
>  
> -				if (max_src_mv <= max_snk_mv &&
> -				    min_src_mv >= min_snk_mv) {
> +				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 &&
> -					     min_src_mv > max_mv) ||
> +					     max_op_mv > max_mv) ||
>  					    src_mw > max_mw) {
>  						src_pdo = i;
>  						snk_pdo = j;
>  						max_mw = src_mw;
> -						max_mv = max_src_mv;
> +						max_mv = max_op_mv;
>  					}
>  				}
>  			}
> @@ -2285,16 +2288,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port)
>  	}
>  
>  	if (src_pdo) {
> -		pdo = port->source_caps[src_pdo];
> -
> -		port->pps_data.min_volt = pdo_pps_apdo_min_voltage(pdo);
> -		port->pps_data.max_volt = pdo_pps_apdo_max_voltage(pdo);
> -		port->pps_data.max_curr =
> -			min_pps_apdo_current(pdo, port->snk_pdo[snk_pdo]);
> -		port->pps_data.out_volt =
> -			min(pdo_pps_apdo_max_voltage(pdo), port->pps_data.out_volt);
> -		port->pps_data.op_curr =
> -			min(port->pps_data.max_curr, port->pps_data.op_curr);
> +		src = port->source_caps[src_pdo];
> +		snk = port->snk_pdo[snk_pdo];
> +
> +		port->pps_data.min_volt = max(pdo_pps_apdo_min_voltage(src),
> +					      pdo_pps_apdo_min_voltage(snk));
> +		port->pps_data.max_volt = min(pdo_pps_apdo_max_voltage(src),
> +					      pdo_pps_apdo_max_voltage(snk));
> +		port->pps_data.max_curr = min_pps_apdo_current(src, snk);
> +		port->pps_data.out_volt = min(port->pps_data.max_volt,
> +					      port->pps_data.out_volt);
> +		port->pps_data.op_curr = min(port->pps_data.max_curr,
> +					     port->pps_data.op_curr);
>  	}
>  
>  	return src_pdo;
> -- 
> 2.20.0.405.gbc1bbc6f85-goog

thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [v3] usb: typec: tcpm: Extend the matching rules on PPS APDO selection
@ 2018-12-18 12:00   ` Heikki Krogerus
  0 siblings, 0 replies; 6+ messages in thread
From: Heikki Krogerus @ 2018-12-18 12:00 UTC (permalink / raw)
  To: Kyle Tso
  Cc: linux, gregkh, Adam.Thomson.Opensource, badhri, linux-usb, linux-kernel

On Mon, Dec 17, 2018 at 09:22:13PM +0800, Kyle Tso wrote:
> Current matching rules ensure that the voltage range of selected Source
> Capability is entirely within the range defined in one of the Sink
> Capabilities. This is reasonable but not practical because Sink may not
> support wide range of voltage when sinking power while Source could
> advertise its capabilities in relatively wider range. For example, a
> Source PDO advertising 3.3V-11V@3A (9V Prog of Fixed Nominal Voltage)
> will not be selected if the Sink requires 5V-12V@3A PPS power. However,
> the Sink could work well if the requested voltage range in RDOs is
> 5V-11V@3A.
> 
> Currently accepted:
> 		|--------- source -----|
> 	|----------- sink ---------------|
> 
> Currently not accepted:
> 			|--------- source -----|
> 	|----------- sink ---------------|
> 
> 	|--------- source -----|
> 		|----------- sink ---------------|
> 
> 	|--------- source -----------------|
> 		|------ sink -------|
> 
> To improve the usability, change the matching rules to what listed
> below:
> a. The Source PDO is selectable if any portion of the voltage range
>    overlaps one of the Sink PDO's voltage range.
> b. The maximum operational voltage will be the lower one between the
>    selected Source PDO and the matching Sink PDO.
> c. The maximum power will be the maximum operational voltage times the
>    maximum current defined in the selected Source PDO
> d. Select the Source PDO with the highest maximum power
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changelog since v1:
> - updated the commit message as suggested by Guenter Roeck <linux@roeck-us.net>
> 
> Changelog since v2:
> - fixed the coding style problems as suggested by Heikki Krogerus
>   <heikki.krogerus@linux.intel.com>
> - Corrected a wrong word in the commit message as suggested by Adam Thomson
>   <Adam.Thomson.Opensource@diasemi.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 3620efee2688..4bc29b586698 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2213,7 +2213,8 @@ 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;
> -	u32 pdo;
> +	unsigned int max_op_mv;
> +	u32 pdo, src, snk;
>  	unsigned int src_pdo = 0, snk_pdo = 0;
>  
>  	/*
> @@ -2263,16 +2264,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port)
>  					continue;
>  				}
>  
> -				if (max_src_mv <= max_snk_mv &&
> -				    min_src_mv >= min_snk_mv) {
> +				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 &&
> -					     min_src_mv > max_mv) ||
> +					     max_op_mv > max_mv) ||
>  					    src_mw > max_mw) {
>  						src_pdo = i;
>  						snk_pdo = j;
>  						max_mw = src_mw;
> -						max_mv = max_src_mv;
> +						max_mv = max_op_mv;
>  					}
>  				}
>  			}
> @@ -2285,16 +2288,18 @@ static unsigned int tcpm_pd_select_pps_apdo(struct tcpm_port *port)
>  	}
>  
>  	if (src_pdo) {
> -		pdo = port->source_caps[src_pdo];
> -
> -		port->pps_data.min_volt = pdo_pps_apdo_min_voltage(pdo);
> -		port->pps_data.max_volt = pdo_pps_apdo_max_voltage(pdo);
> -		port->pps_data.max_curr =
> -			min_pps_apdo_current(pdo, port->snk_pdo[snk_pdo]);
> -		port->pps_data.out_volt =
> -			min(pdo_pps_apdo_max_voltage(pdo), port->pps_data.out_volt);
> -		port->pps_data.op_curr =
> -			min(port->pps_data.max_curr, port->pps_data.op_curr);
> +		src = port->source_caps[src_pdo];
> +		snk = port->snk_pdo[snk_pdo];
> +
> +		port->pps_data.min_volt = max(pdo_pps_apdo_min_voltage(src),
> +					      pdo_pps_apdo_min_voltage(snk));
> +		port->pps_data.max_volt = min(pdo_pps_apdo_max_voltage(src),
> +					      pdo_pps_apdo_max_voltage(snk));
> +		port->pps_data.max_curr = min_pps_apdo_current(src, snk);
> +		port->pps_data.out_volt = min(port->pps_data.max_volt,
> +					      port->pps_data.out_volt);
> +		port->pps_data.op_curr = min(port->pps_data.max_curr,
> +					     port->pps_data.op_curr);
>  	}
>  
>  	return src_pdo;
> -- 
> 2.20.0.405.gbc1bbc6f85-goog

thanks,

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-12-18 12:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-17 13:22 [PATCH v3] usb: typec: tcpm: Extend the matching rules on PPS APDO selection Kyle Tso
2018-12-17 13:22 ` [v3] " Kyle Tso
2018-12-17 14:44 ` [PATCH v3] " Adam Thomson
2018-12-17 14:44   ` [v3] " Opensource [Adam Thomson]
2018-12-18 12:00 ` [PATCH v3] " Heikki Krogerus
2018-12-18 12:00   ` [v3] " Heikki Krogerus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.