[PATCH 0/5] usb: typec: remove max_snk_*

[PATCH 0/5] usb: typec: remove max_snk_*

[Li Jun]

This patch set is to remove max_snk_mv/ma/mw configs, as we should
define the sink capability by sink PDOs, the first patch update
the source PDO match policy by compare the voltage range between
source and sink PDOs no matter what type they are, the following
patchs remove those 3 variables from 2 existing users by adding
a variable PDO, then finial patch remove the max_snk_* from tcpm.

Li Jun (5):
  usb: typec: tcpm: source pdo selection update
  usb: typec: fusb302: remove max_snk_* for sink config
  dt-bindings: usb: fusb302: remove max-sink-* properties
  usb: typec: wcove: remove max_snk_* for sink config
  usb: typec: tcpm: remove max_snk_*

 .../devicetree/bindings/usb/fcs,fusb302.txt        |   6 --
 drivers/usb/typec/fusb302/fusb302.c                |  13 +--
 drivers/usb/typec/tcpm.c                           | 113 +++++++--------------
 drivers/usb/typec/typec_wcove.c                    |   4 +-
 include/linux/usb/tcpm.h                           |   9 --
 5 files changed, 39 insertions(+), 106 deletions(-)

-- 
2.7.4

[PATCH 1/5] usb: typec: tcpm: source pdo selection update

[Li Jun]

Instead of only compare between the same pdo type of sink and source,
this patch update the source pdo selection by checking the source pdo
voltage range is within that of sink.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/tcpm.c | 101 +++++++++++++++++------------------------------
 1 file changed, 37 insertions(+), 64 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index cd48a99..20f7ca8 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -1765,7 +1765,9 @@ static int tcpm_pd_check_request(struct tcpm_port *port)
 static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
 			      int *src_pdo)
 {
-	unsigned int i, j, max_mw = 0, max_mv = 0, mw = 0, mv = 0, ma = 0;
+	unsigned int i, j, max_src_mv = 0, min_src_mv = 0, max_mw = 0,
+		     max_mv = 0, src_mw = 0, src_ma = 0, max_snk_mv = 0,
+		     min_snk_mv = 0;
 	int ret = -EINVAL;
 
 	/*
@@ -1777,70 +1779,41 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
 		enum pd_pdo_type type = pdo_type(pdo);
 
 		if (type == PDO_TYPE_FIXED) {
-			for (j = 0; j < port->nr_fixed; j++) {
-				if (pdo_fixed_voltage(pdo) ==
-				    pdo_fixed_voltage(port->snk_pdo[j])) {
-					ma = min_current(pdo, port->snk_pdo[j]);
-					mv = pdo_fixed_voltage(pdo);
-					mw = ma * mv / 1000;
-					if (mw > max_mw ||
-					    (mw == max_mw && mv > max_mv)) {
-						ret = 0;
-						*src_pdo = i;
-						*sink_pdo = j;
-						max_mw = mw;
-						max_mv = mv;
-					}
-					/* There could only be one fixed pdo
-					 * at a specific voltage level.
-					 * So breaking here.
-					 */
-					break;
-				}
-			}
-		} else if (type == PDO_TYPE_BATT) {
-			for (j = port->nr_fixed;
-			     j < port->nr_fixed +
-				 port->nr_batt;
-			     j++) {
-				if (pdo_min_voltage(pdo) >=
-				     pdo_min_voltage(port->snk_pdo[j]) &&
-				     pdo_max_voltage(pdo) <=
-				     pdo_max_voltage(port->snk_pdo[j])) {
-					mw = min_power(pdo, port->snk_pdo[j]);
-					mv = pdo_min_voltage(pdo);
-					if (mw > max_mw ||
-					    (mw == max_mw && mv > max_mv)) {
-						ret = 0;
-						*src_pdo = i;
-						*sink_pdo = j;
-						max_mw = mw;
-						max_mv = mv;
-					}
-				}
+			max_src_mv = pdo_fixed_voltage(pdo);
+			min_src_mv = max_src_mv;
+		} else {
+			max_src_mv = pdo_max_voltage(pdo);
+			min_src_mv = pdo_min_voltage(pdo);
+		}
+
+		if (type == PDO_TYPE_BATT) {
+			src_mw = pdo_max_power(pdo);
+		} else {
+			src_ma = pdo_max_current(pdo);
+			src_mw = src_ma * min_src_mv / 1000;
+		}
+
+		for (j = 0; j < port->nr_snk_pdo; j++) {
+			pdo = port->snk_pdo[j];
+
+			if (pdo_type(pdo) == PDO_TYPE_FIXED) {
+				min_snk_mv = pdo_fixed_voltage(pdo);
+				max_snk_mv = pdo_fixed_voltage(pdo);
+			} else {
+				min_snk_mv = pdo_min_voltage(pdo);
+				max_snk_mv = pdo_max_voltage(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])) {
-					ma = min_current(pdo, port->snk_pdo[j]);
-					mv = pdo_min_voltage(pdo);
-					mw = ma * mv / 1000;
-					if (mw > max_mw ||
-					    (mw == max_mw && mv > max_mv)) {
-						ret = 0;
-						*src_pdo = i;
-						*sink_pdo = j;
-						max_mw = mw;
-						max_mv = mv;
-					}
+
+			if (max_src_mv <= max_snk_mv &&
+				min_src_mv >= min_snk_mv) {
+				/* Prefer higher voltages if available */
+				if ((src_mw == max_mw && min_src_mv > max_mv) ||
+							src_mw > max_mw) {
+					*src_pdo = i;
+					*sink_pdo = j;
+					max_mw = src_mw;
+					max_mv = min_src_mv;
+					ret = 0;
 				}
 			}
 		}
-- 
2.7.4

[PATCH 2/5] usb: typec: fusb302: remove max_snk_* for sink config

[Li Jun]

Since max_snk_* is to be deprecated, so remove max_snk_* by adding a
variable PDO for sink config.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/fusb302/fusb302.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c b/drivers/usb/typec/fusb302/fusb302.c
index da179aa..c183f46 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1207,6 +1207,7 @@ static const u32 src_pdo[] = {
 
 static const u32 snk_pdo[] = {
 	PDO_FIXED(5000, 400, PDO_FIXED_FLAGS),
+	PDO_VAR(800, 5000, 3000),
 };
 
 static const struct tcpc_config fusb302_tcpc_config = {
@@ -1214,9 +1215,6 @@ static const struct tcpc_config fusb302_tcpc_config = {
 	.nr_src_pdo = ARRAY_SIZE(src_pdo),
 	.snk_pdo = snk_pdo,
 	.nr_snk_pdo = ARRAY_SIZE(snk_pdo),
-	.max_snk_mv = 5000,
-	.max_snk_ma = 3000,
-	.max_snk_mw = 15000,
 	.operating_snk_mw = 2500,
 	.type = TYPEC_PORT_DRP,
 	.default_role = TYPEC_SINK,
@@ -1784,15 +1782,6 @@ static int fusb302_probe(struct i2c_client *client,
 	chip->tcpc_dev.config = &chip->tcpc_config;
 	mutex_init(&chip->lock);
 
-	if (!device_property_read_u32(dev, "fcs,max-sink-microvolt", &v))
-		chip->tcpc_config.max_snk_mv = v / 1000;
-
-	if (!device_property_read_u32(dev, "fcs,max-sink-microamp", &v))
-		chip->tcpc_config.max_snk_ma = v / 1000;
-
-	if (!device_property_read_u32(dev, "fcs,max-sink-microwatt", &v))
-		chip->tcpc_config.max_snk_mw = v / 1000;
-
 	if (!device_property_read_u32(dev, "fcs,operating-sink-microwatt", &v))
 		chip->tcpc_config.operating_snk_mw = v / 1000;
 
-- 
2.7.4

[PATCH 3/5] dt-bindings: usb: fusb302: remove max-sink-* properties

[Li Jun]

Remove max-sink-* properties since they are deprecated.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 Documentation/devicetree/bindings/usb/fcs,fusb302.txt | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
index 472facf..6087dc7 100644
--- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
+++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
@@ -6,12 +6,6 @@ Required properties :
 - interrupts             : Interrupt specifier
 
 Optional properties :
-- fcs,max-sink-microvolt : Maximum voltage to negotiate when configured as sink
-- fcs,max-sink-microamp  : Maximum current to negotiate when configured as sink
-- fcs,max-sink-microwatt : Maximum power to negotiate when configured as sink
-			   If this is less then max-sink-microvolt *
-			   max-sink-microamp then the configured current will
-			   be clamped.
 - fcs,operating-sink-microwatt :
 			   Minimum amount of power accepted from a sink
 			   when negotiating
-- 
2.7.4

[PATCH 4/5] usb: typec: wcove: remove max_snk_* for sink config

[Li Jun]

Since max_snk_* is to be deprecated, so remove max_snk_* by adding a
variable PDO for sink config.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/typec_wcove.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c
index 2e990e0..7c10e06 100644
--- a/drivers/usb/typec/typec_wcove.c
+++ b/drivers/usb/typec/typec_wcove.c
@@ -558,6 +558,7 @@ static const u32 src_pdo[] = {
 static const u32 snk_pdo[] = {
 	PDO_FIXED(5000, 500, PDO_FIXED_DUAL_ROLE | PDO_FIXED_DATA_SWAP |
 		  PDO_FIXED_USB_COMM),
+	PDO_VAR(5000, 12000, 3000),
 };
 
 static struct tcpc_config wcove_typec_config = {
@@ -566,9 +567,6 @@ static struct tcpc_config wcove_typec_config = {
 	.snk_pdo = snk_pdo,
 	.nr_snk_pdo = ARRAY_SIZE(snk_pdo),
 
-	.max_snk_mv = 12000,
-	.max_snk_ma = 3000,
-	.max_snk_mw = 36000,
 	.operating_snk_mw = 15000,
 
 	.type = TYPEC_PORT_DRP,
-- 
2.7.4

[PATCH 5/5] usb: typec: tcpm: remove max_snk_mv/ma/mw

[Li Jun]

Since there is no user of max_snk_*, so we can remove them from tcpm.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/tcpm.c | 12 ------------
 include/linux/usb/tcpm.h |  9 ---------
 2 files changed, 21 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 20f7ca8..8e3e051 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -258,9 +258,6 @@ struct tcpm_port {
 	u32 snk_vdo[VDO_MAX_OBJECTS];
 	unsigned int nr_snk_vdo;
 
-	unsigned int max_snk_mv;
-	unsigned int max_snk_ma;
-	unsigned int max_snk_mw;
 	unsigned int operating_snk_mw;
 
 	/* Requested current / voltage */
@@ -3582,9 +3579,6 @@ EXPORT_SYMBOL_GPL(tcpm_update_source_capabilities);
 
 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))
@@ -3592,9 +3586,6 @@ int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo,
 
 	mutex_lock(&port->lock);
 	port->nr_snk_pdo = tcpm_copy_pdos(port->snk_pdo, pdo, nr_pdo);
-	port->max_snk_mv = max_snk_mv;
-	port->max_snk_ma = max_snk_ma;
-	port->max_snk_mw = max_snk_mw;
 	port->operating_snk_mw = operating_snk_mw;
 
 	switch (port->state) {
@@ -3682,9 +3673,6 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	port->nr_snk_vdo = tcpm_copy_vdos(port->snk_vdo, tcpc->config->snk_vdo,
 					  tcpc->config->nr_snk_vdo);
 
-	port->max_snk_mv = tcpc->config->max_snk_mv;
-	port->max_snk_ma = tcpc->config->max_snk_ma;
-	port->max_snk_mw = tcpc->config->max_snk_mw;
 	port->operating_snk_mw = tcpc->config->operating_snk_mw;
 	if (!tcpc->config->try_role_hw)
 		port->try_role = tcpc->config->default_role;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index ca1c0b5..2649c97 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -62,9 +62,6 @@ enum tcpm_transmit_type {
  * @snk_pdo:	PDO parameters sent to partner as response to
  *		PD_CTRL_GET_SINK_CAP message
  * @nr_snk_pdo:	Number of entries in @snk_pdo
- * @max_snk_mv:	Maximum acceptable sink voltage in mV
- * @max_snk_ma:	Maximum sink current in mA
- * @max_snk_mw:	Maximum required sink power in mW
  * @operating_snk_mw:
  *		Required operating sink power in mW
  * @type:	Port type (TYPEC_PORT_DFP, TYPEC_PORT_UFP, or
@@ -85,9 +82,6 @@ struct tcpc_config {
 	const u32 *snk_vdo;
 	unsigned int nr_snk_vdo;
 
-	unsigned int max_snk_mv;
-	unsigned int max_snk_ma;
-	unsigned int max_snk_mw;
 	unsigned int operating_snk_mw;
 
 	enum typec_port_type type;
@@ -187,9 +181,6 @@ int tcpm_update_source_capabilities(struct tcpm_port *port, const u32 *pdo,
 				    unsigned int nr_pdo);
 int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo,
 				  unsigned int nr_pdo,
-				  unsigned int max_snk_mv,
-				  unsigned int max_snk_ma,
-				  unsigned int max_snk_mw,
 				  unsigned int operating_snk_mw);
 
 void tcpm_vbus_change(struct tcpm_port *port);
-- 
2.7.4

[PATCH 5/5] usb: typec: tcpm: remove max_snk_*

[Li Jun]

Since there is no user of max_snk_*, so we can remove them from tcpm.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/tcpm.c | 12 ------------
 include/linux/usb/tcpm.h |  9 ---------
 2 files changed, 21 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 20f7ca8..8e3e051 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -258,9 +258,6 @@ struct tcpm_port {
 	u32 snk_vdo[VDO_MAX_OBJECTS];
 	unsigned int nr_snk_vdo;
 
-	unsigned int max_snk_mv;
-	unsigned int max_snk_ma;
-	unsigned int max_snk_mw;
 	unsigned int operating_snk_mw;
 
 	/* Requested current / voltage */
@@ -3582,9 +3579,6 @@ EXPORT_SYMBOL_GPL(tcpm_update_source_capabilities);
 
 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))
@@ -3592,9 +3586,6 @@ int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo,
 
 	mutex_lock(&port->lock);
 	port->nr_snk_pdo = tcpm_copy_pdos(port->snk_pdo, pdo, nr_pdo);
-	port->max_snk_mv = max_snk_mv;
-	port->max_snk_ma = max_snk_ma;
-	port->max_snk_mw = max_snk_mw;
 	port->operating_snk_mw = operating_snk_mw;
 
 	switch (port->state) {
@@ -3682,9 +3673,6 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	port->nr_snk_vdo = tcpm_copy_vdos(port->snk_vdo, tcpc->config->snk_vdo,
 					  tcpc->config->nr_snk_vdo);
 
-	port->max_snk_mv = tcpc->config->max_snk_mv;
-	port->max_snk_ma = tcpc->config->max_snk_ma;
-	port->max_snk_mw = tcpc->config->max_snk_mw;
 	port->operating_snk_mw = tcpc->config->operating_snk_mw;
 	if (!tcpc->config->try_role_hw)
 		port->try_role = tcpc->config->default_role;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index ca1c0b5..2649c97 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -62,9 +62,6 @@ enum tcpm_transmit_type {
  * @snk_pdo:	PDO parameters sent to partner as response to
  *		PD_CTRL_GET_SINK_CAP message
  * @nr_snk_pdo:	Number of entries in @snk_pdo
- * @max_snk_mv:	Maximum acceptable sink voltage in mV
- * @max_snk_ma:	Maximum sink current in mA
- * @max_snk_mw:	Maximum required sink power in mW
  * @operating_snk_mw:
  *		Required operating sink power in mW
  * @type:	Port type (TYPEC_PORT_DFP, TYPEC_PORT_UFP, or
@@ -85,9 +82,6 @@ struct tcpc_config {
 	const u32 *snk_vdo;
 	unsigned int nr_snk_vdo;
 
-	unsigned int max_snk_mv;
-	unsigned int max_snk_ma;
-	unsigned int max_snk_mw;
 	unsigned int operating_snk_mw;
 
 	enum typec_port_type type;
@@ -187,9 +181,6 @@ int tcpm_update_source_capabilities(struct tcpm_port *port, const u32 *pdo,
 				    unsigned int nr_pdo);
 int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo,
 				  unsigned int nr_pdo,
-				  unsigned int max_snk_mv,
-				  unsigned int max_snk_ma,
-				  unsigned int max_snk_mw,
 				  unsigned int operating_snk_mw);
 
 void tcpm_vbus_change(struct tcpm_port *port);
-- 
2.7.4

Re: [PATCH 1/5] usb: typec: tcpm: source pdo selection update

[Hans de Goede]

Hi,

On 20-03-18 11:26, Li Jun wrote:
> Instead of only compare between the same pdo type of sink and source,
> this patch update the source pdo selection by checking the source pdo
> voltage range is within that of sink.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>

This patch applies on top of the broken reverted commit, but the
revert has already been merged by Linus:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/typec?id=6f566af3462897fc743e3af6ea8cc790a13148ba

This commit is not in usb-next because it went into 4.16 as fix
after 4.16-rc1. As already mentioned in another rhread this needs
to be back-merged into usb-next.

So this needs to be respun to apply on top of the revert so that
it can be merged without issues with Torvald's tree later.

Regards,

Hans



> ---
>   drivers/usb/typec/tcpm.c | 101 +++++++++++++++++------------------------------
>   1 file changed, 37 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index cd48a99..20f7ca8 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -1765,7 +1765,9 @@ static int tcpm_pd_check_request(struct tcpm_port *port)
>   static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
>   			      int *src_pdo)
>   {
> -	unsigned int i, j, max_mw = 0, max_mv = 0, mw = 0, mv = 0, ma = 0;
> +	unsigned int i, j, max_src_mv = 0, min_src_mv = 0, max_mw = 0,
> +		     max_mv = 0, src_mw = 0, src_ma = 0, max_snk_mv = 0,
> +		     min_snk_mv = 0;
>   	int ret = -EINVAL;
>   
>   	/*
> @@ -1777,70 +1779,41 @@ static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
>   		enum pd_pdo_type type = pdo_type(pdo);
>   
>   		if (type == PDO_TYPE_FIXED) {
> -			for (j = 0; j < port->nr_fixed; j++) {
> -				if (pdo_fixed_voltage(pdo) ==
> -				    pdo_fixed_voltage(port->snk_pdo[j])) {
> -					ma = min_current(pdo, port->snk_pdo[j]);
> -					mv = pdo_fixed_voltage(pdo);
> -					mw = ma * mv / 1000;
> -					if (mw > max_mw ||
> -					    (mw == max_mw && mv > max_mv)) {
> -						ret = 0;
> -						*src_pdo = i;
> -						*sink_pdo = j;
> -						max_mw = mw;
> -						max_mv = mv;
> -					}
> -					/* There could only be one fixed pdo
> -					 * at a specific voltage level.
> -					 * So breaking here.
> -					 */
> -					break;
> -				}
> -			}
> -		} else if (type == PDO_TYPE_BATT) {
> -			for (j = port->nr_fixed;
> -			     j < port->nr_fixed +
> -				 port->nr_batt;
> -			     j++) {
> -				if (pdo_min_voltage(pdo) >=
> -				     pdo_min_voltage(port->snk_pdo[j]) &&
> -				     pdo_max_voltage(pdo) <=
> -				     pdo_max_voltage(port->snk_pdo[j])) {
> -					mw = min_power(pdo, port->snk_pdo[j]);
> -					mv = pdo_min_voltage(pdo);
> -					if (mw > max_mw ||
> -					    (mw == max_mw && mv > max_mv)) {
> -						ret = 0;
> -						*src_pdo = i;
> -						*sink_pdo = j;
> -						max_mw = mw;
> -						max_mv = mv;
> -					}
> -				}
> +			max_src_mv = pdo_fixed_voltage(pdo);
> +			min_src_mv = max_src_mv;
> +		} else {
> +			max_src_mv = pdo_max_voltage(pdo);
> +			min_src_mv = pdo_min_voltage(pdo);
> +		}
> +
> +		if (type == PDO_TYPE_BATT) {
> +			src_mw = pdo_max_power(pdo);
> +		} else {
> +			src_ma = pdo_max_current(pdo);
> +			src_mw = src_ma * min_src_mv / 1000;
> +		}
> +
> +		for (j = 0; j < port->nr_snk_pdo; j++) {
> +			pdo = port->snk_pdo[j];
> +
> +			if (pdo_type(pdo) == PDO_TYPE_FIXED) {
> +				min_snk_mv = pdo_fixed_voltage(pdo);
> +				max_snk_mv = pdo_fixed_voltage(pdo);
> +			} else {
> +				min_snk_mv = pdo_min_voltage(pdo);
> +				max_snk_mv = pdo_max_voltage(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])) {
> -					ma = min_current(pdo, port->snk_pdo[j]);
> -					mv = pdo_min_voltage(pdo);
> -					mw = ma * mv / 1000;
> -					if (mw > max_mw ||
> -					    (mw == max_mw && mv > max_mv)) {
> -						ret = 0;
> -						*src_pdo = i;
> -						*sink_pdo = j;
> -						max_mw = mw;
> -						max_mv = mv;
> -					}
> +
> +			if (max_src_mv <= max_snk_mv &&
> +				min_src_mv >= min_snk_mv) {
> +				/* Prefer higher voltages if available */
> +				if ((src_mw == max_mw && min_src_mv > max_mv) ||
> +							src_mw > max_mw) {
> +					*src_pdo = i;
> +					*sink_pdo = j;
> +					max_mw = src_mw;
> +					max_mv = min_src_mv;
> +					ret = 0;
>   				}
>   			}
>   		}
> 

Re: [PATCH 2/5] usb: typec: fusb302: remove max_snk_* for sink config

[Hans de Goede]

Hi,

On 20-03-18 11:26, Li Jun wrote:
> Since max_snk_* is to be deprecated, so remove max_snk_* by adding a
> variable PDO for sink config.

NACK there are actual users of the device-properties which you are
now breaking. As I mentioned in another thread, you need to instead
add the pdo array to the fusb302_chip struct (so that it is no longer
const) and generate a PDO_VAR pdo based on the device-properties.

Regards,

Hans


> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>   drivers/usb/typec/fusb302/fusb302.c | 13 +------------
>   1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/typec/fusb302/fusb302.c b/drivers/usb/typec/fusb302/fusb302.c
> index da179aa..c183f46 100644
> --- a/drivers/usb/typec/fusb302/fusb302.c
> +++ b/drivers/usb/typec/fusb302/fusb302.c
> @@ -1207,6 +1207,7 @@ static const u32 src_pdo[] = {
>   
>   static const u32 snk_pdo[] = {
>   	PDO_FIXED(5000, 400, PDO_FIXED_FLAGS),
> +	PDO_VAR(800, 5000, 3000),
>   };
>   
>   static const struct tcpc_config fusb302_tcpc_config = {
> @@ -1214,9 +1215,6 @@ static const struct tcpc_config fusb302_tcpc_config = {
>   	.nr_src_pdo = ARRAY_SIZE(src_pdo),
>   	.snk_pdo = snk_pdo,
>   	.nr_snk_pdo = ARRAY_SIZE(snk_pdo),
> -	.max_snk_mv = 5000,
> -	.max_snk_ma = 3000,
> -	.max_snk_mw = 15000,
>   	.operating_snk_mw = 2500,
>   	.type = TYPEC_PORT_DRP,
>   	.default_role = TYPEC_SINK,
> @@ -1784,15 +1782,6 @@ static int fusb302_probe(struct i2c_client *client,
>   	chip->tcpc_dev.config = &chip->tcpc_config;
>   	mutex_init(&chip->lock);
>   
> -	if (!device_property_read_u32(dev, "fcs,max-sink-microvolt", &v))
> -		chip->tcpc_config.max_snk_mv = v / 1000;
> -
> -	if (!device_property_read_u32(dev, "fcs,max-sink-microamp", &v))
> -		chip->tcpc_config.max_snk_ma = v / 1000;
> -
> -	if (!device_property_read_u32(dev, "fcs,max-sink-microwatt", &v))
> -		chip->tcpc_config.max_snk_mw = v / 1000;
> -
>   	if (!device_property_read_u32(dev, "fcs,operating-sink-microwatt", &v))
>   		chip->tcpc_config.operating_snk_mw = v / 1000;
>   
> 

RE: [PATCH 1/5] usb: typec: tcpm: source pdo selection update

[Jun Li]

Hi
> -----Original Message-----
> From: Hans de Goede [mailto:hdegoede@redhat.com]
> Sent: 2018年3月20日 20:27
> To: Jun Li <jun.li@nxp.com>; gregkh@linuxfoundation.org;
> robh+dt@kernel.org; mark.rutland@arm.com;
> heikki.krogerus@linux.intel.com; linux@roeck-us.net; rmfrfs@gmail.com;
> yueyao.zhu@gmail.com
> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/5] usb: typec: tcpm: source pdo selection update
> 
> Hi,
> 
> On 20-03-18 11:26, Li Jun wrote:
> > Instead of only compare between the same pdo type of sink and source,
> > this patch update the source pdo selection by checking the source pdo
> > voltage range is within that of sink.
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> 
> This patch applies on top of the broken reverted commit, but the revert has
> already been merged by Linus:
> 
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ke
> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2
> Fcommit%2Fdrivers%2Fusb%2Ftypec%3Fid%3D6f566af3462897fc743e3af6ea
> 8cc790a13148ba&data=02%7C01%7Cjun.li%40nxp.com%7C188f2cdfa5eb44af
> d27808d58e5dd13c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C636571455959361056&sdata=CjnsG0rxIfJnm0K2tQyDbFxIP4Als%2BTT243Fh
> y7UYyI%3D&reserved=0
> 
> This commit is not in usb-next because it went into 4.16 as fix after 4.16-rc1.
> As already mentioned in another rhread this needs to be back-merged into
> usb-next.
> 
> So this needs to be respun to apply on top of the revert so that it can be
> merged without issues with Torvald's tree later.

OK, I will do the rebase in v2.

Thanks
Jun

RE: [PATCH 2/5] usb: typec: fusb302: remove max_snk_* for sink config

[Jun Li]

> -----Original Message-----
> From: Hans de Goede [mailto:hdegoede@redhat.com]
> Sent: 2018年3月20日 20:29
> To: Jun Li <jun.li@nxp.com>; gregkh@linuxfoundation.org;
> robh+dt@kernel.org; mark.rutland@arm.com;
> heikki.krogerus@linux.intel.com; linux@roeck-us.net; rmfrfs@gmail.com;
> yueyao.zhu@gmail.com
> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH 2/5] usb: typec: fusb302: remove max_snk_* for sink
> config
> 
> Hi,
> 
> On 20-03-18 11:26, Li Jun wrote:
> > Since max_snk_* is to be deprecated, so remove max_snk_* by adding a
> > variable PDO for sink config.
> 
> NACK there are actual users of the device-properties which you are now
> breaking. As I mentioned in another thread, you need to instead add the pdo
> array to the fusb302_chip struct (so that it is no longer
> const) and generate a PDO_VAR pdo based on the device-properties.
> 

OK, I did a search of those properties in dts dir of upstream kernel
but didn't find any user of them, if there are actual of users, I will create
a PDO_VAR pdo based on dt in v2.

Thanks
Jun Li

> Regards,
> 
> Hans
> 
> 
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >   drivers/usb/typec/fusb302/fusb302.c | 13 +------------
> >   1 file changed, 1 insertion(+), 12 deletions(-)
> >
> > diff --git a/drivers/usb/typec/fusb302/fusb302.c
> > b/drivers/usb/typec/fusb302/fusb302.c
> > index da179aa..c183f46 100644
> > --- a/drivers/usb/typec/fusb302/fusb302.c
> > +++ b/drivers/usb/typec/fusb302/fusb302.c
> > @@ -1207,6 +1207,7 @@ static const u32 src_pdo[] = {
> >
> >   static const u32 snk_pdo[] = {
> >   	PDO_FIXED(5000, 400, PDO_FIXED_FLAGS),
> > +	PDO_VAR(800, 5000, 3000),
> >   };
> >
> >   static const struct tcpc_config fusb302_tcpc_config = { @@ -1214,9
> > +1215,6 @@ static const struct tcpc_config fusb302_tcpc_config = {
> >   	.nr_src_pdo = ARRAY_SIZE(src_pdo),
> >   	.snk_pdo = snk_pdo,
> >   	.nr_snk_pdo = ARRAY_SIZE(snk_pdo),
> > -	.max_snk_mv = 5000,
> > -	.max_snk_ma = 3000,
> > -	.max_snk_mw = 15000,
> >   	.operating_snk_mw = 2500,
> >   	.type = TYPEC_PORT_DRP,
> >   	.default_role = TYPEC_SINK,
> > @@ -1784,15 +1782,6 @@ static int fusb302_probe(struct i2c_client
> *client,
> >   	chip->tcpc_dev.config = &chip->tcpc_config;
> >   	mutex_init(&chip->lock);
> >
> > -	if (!device_property_read_u32(dev, "fcs,max-sink-microvolt", &v))
> > -		chip->tcpc_config.max_snk_mv = v / 1000;
> > -
> > -	if (!device_property_read_u32(dev, "fcs,max-sink-microamp", &v))
> > -		chip->tcpc_config.max_snk_ma = v / 1000;
> > -
> > -	if (!device_property_read_u32(dev, "fcs,max-sink-microwatt", &v))
> > -		chip->tcpc_config.max_snk_mw = v / 1000;
> > -
> >   	if (!device_property_read_u32(dev, "fcs,operating-sink-microwatt", &v))
> >   		chip->tcpc_config.operating_snk_mw = v / 1000;
> >
> >

Re: [PATCH 2/5] usb: typec: fusb302: remove max_snk_* for sink config

[Hans de Goede]

Hi,

On 21-03-18 12:19, Jun Li wrote:
> 
>> -----Original Message-----
>> From: Hans de Goede [mailto:hdegoede@redhat.com]
>> Sent: 2018年3月20日 20:29
>> To: Jun Li <jun.li@nxp.com>; gregkh@linuxfoundation.org;
>> robh+dt@kernel.org; mark.rutland@arm.com;
>> heikki.krogerus@linux.intel.com; linux@roeck-us.net; rmfrfs@gmail.com;
>> yueyao.zhu@gmail.com
>> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx
>> <linux-imx@nxp.com>
>> Subject: Re: [PATCH 2/5] usb: typec: fusb302: remove max_snk_* for sink
>> config
>>
>> Hi,
>>
>> On 20-03-18 11:26, Li Jun wrote:
>>> Since max_snk_* is to be deprecated, so remove max_snk_* by adding a
>>> variable PDO for sink config.
>>
>> NACK there are actual users of the device-properties which you are now
>> breaking. As I mentioned in another thread, you need to instead add the pdo
>> array to the fusb302_chip struct (so that it is no longer
>> const) and generate a PDO_VAR pdo based on the device-properties.
>>
> 
> OK, I did a search of those properties in dts dir of upstream kernel
> but didn't find any user of them,

Note that these properties are not accessed through the dt APIs but through the
generic device-properties API. And these are set as device-properties by the
driver instantiating the fusb302 i2c-client on (some) x86 boards:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/platform/x86/intel_cht_int33fe.c

 > if there are actual of users, I will create
 > a PDO_VAR pdo based on dt in v2.

Thank you.

Regards,

Hans

RE: [PATCH 1/5] usb: typec: tcpm: source pdo selection update

[Jun Li]

Hi
> -----Original Message-----
> From: Jun Li
> Sent: 2018年3月21日 19:14
> To: Hans de Goede <hdegoede@redhat.com>; gregkh@linuxfoundation.org;
> robh+dt@kernel.org; mark.rutland@arm.com;
> heikki.krogerus@linux.intel.com; linux@roeck-us.net; rmfrfs@gmail.com;
> yueyao.zhu@gmail.com
> Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: RE: [PATCH 1/5] usb: typec: tcpm: source pdo selection update
> 
> Hi
> > -----Original Message-----
> > From: Hans de Goede [mailto:hdegoede@redhat.com]
> > Sent: 2018年3月20日 20:27
> > To: Jun Li <jun.li@nxp.com>; gregkh@linuxfoundation.org;
> > robh+dt@kernel.org; mark.rutland@arm.com;
> > heikki.krogerus@linux.intel.com; linux@roeck-us.net; rmfrfs@gmail.com;
> > yueyao.zhu@gmail.com
> > Cc: linux-usb@vger.kernel.org; devicetree@vger.kernel.org;
> > dl-linux-imx <linux-imx@nxp.com>
> > Subject: Re: [PATCH 1/5] usb: typec: tcpm: source pdo selection update
> >
> > Hi,
> >
> > On 20-03-18 11:26, Li Jun wrote:
> > > Instead of only compare between the same pdo type of sink and
> > > source, this patch update the source pdo selection by checking the
> > > source pdo voltage range is within that of sink.
> > >
> > > Signed-off-by: Li Jun <jun.li@nxp.com>
> >
> > This patch applies on top of the broken reverted commit, but the
> > revert has already been merged by Linus:
> >
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> > .ke
> >
> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2
> >
> Fcommit%2Fdrivers%2Fusb%2Ftypec%3Fid%3D6f566af3462897fc743e3af6ea
> >
> 8cc790a13148ba&data=02%7C01%7Cjun.li%40nxp.com%7C188f2cdfa5eb44af
> >
> d27808d58e5dd13c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> >
> C636571455959361056&sdata=CjnsG0rxIfJnm0K2tQyDbFxIP4Als%2BTT243Fh
> > y7UYyI%3D&reserved=0
> >
> > This commit is not in usb-next because it went into 4.16 as fix after 4.16-rc1.
> > As already mentioned in another rhread this needs to be back-merged
> > into usb-next.
> >
> > So this needs to be respun to apply on top of the revert so that it
> > can be merged without issues with Torvald's tree later.

What's the proper way to do this, should I re-post the original reverted
patch and my update one based on it(so 2 patches), or just a combination
(one patch)?

Jun
> 
> OK, I will do the rebase in v2.
> 
> Thanks
> Jun