RE: [PATCH v2 1/5] usb: typec: tcpm: pdo matching optimization

2018-04-09 Thread Jun Li
Hi
> -Original Message-
> From: Hans de Goede [mailto:hdego...@redhat.com]
> Sent: 2018年4月3日 23:17
> To: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org; robh...@kernel.org;
> mark.rutl...@arm.com; heikki.kroge...@linux.intel.com
> Cc: li...@roeck-us.net; rmf...@gmail.com; yueyao@gmail.com;
> linux-usb@vger.kernel.org; devicet...@vger.kernel.org; dl-linux-imx
> <linux-...@nxp.com>
> Subject: Re: [PATCH v2 1/5] usb: typec: tcpm: pdo matching optimization
> 
> Hi,
> 
> On 23-03-18 15:58, Li Jun wrote:
> > This patch is a combination of commit 57e6f0d7b804
> > ("typec: tcpm: Only request matching pdos") and source pdo selection
> > optimization based on it, instead of only compare between the same pdo
> > type of sink and source, we should check source pdo voltage range is
> > within the voltage range of one sink pdo.
> >
> > Signed-off-by: Li Jun <jun...@nxp.com>
> > ---
> >   drivers/usb/typec/tcpm.c | 127
> +--
> >   1 file changed, 90 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index
> > 677d121..50a1979 100644
> > --- a/drivers/usb/typec/tcpm.c
> > +++ b/drivers/usb/typec/tcpm.c
> > @@ -254,6 +254,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;
> >
> 
> These 3 variables are now no longer needed.

Yes, I will remove them.

> 
> > @@ -1772,40 +1775,65 @@ static int tcpm_pd_check_request(struct
> tcpm_port *port)
> > return 0;
> >   }
> >
> > -static int tcpm_pd_select_pdo(struct tcpm_port *port)
> > +#define min_power(x, y) min(pdo_max_power(x), pdo_max_power(y))
> > +#define min_current(x, y) min(pdo_max_current(x), pdo_max_current(y))
> > +
> > +static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
> > + int *src_pdo)
> >   {
> > -   unsigned int i, max_mw = 0, max_mv = 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;
> >
> > /*
> > -* 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_FIXED) {
> > +   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) {
> > -   mw = pdo_max_power(pdo);
> > +   src_mw = pdo_max_power(pdo);
> > } else {
> > -   ma = min(pdo_max_current(pdo),
> > -port->max_snk_ma);
> > -   mw = ma * mv / 1000;
> > +   src_ma = pdo_max_current(pdo);
> > +   src_mw = src_ma * min_src_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;
> > +   for (j = 0; j < port->nr_snk_pdo; j++) {
> > +   pdo = port->snk_pdo[j];
> > +
> > +   if (pdo_type(pdo) == PDO_TYPE_FIXED) {
> > + 

Re: [PATCH v2 1/5] usb: typec: tcpm: pdo matching optimization

2018-04-03 Thread Hans de Goede

Hi,

On 23-03-18 15:58, Li Jun wrote:

This patch is a combination of commit 57e6f0d7b804
("typec: tcpm: Only request matching pdos") and source
pdo selection optimization based on it, instead of only
compare between the same pdo type of sink and source,
we should check source pdo voltage range is within the
voltage range of one sink pdo.

Signed-off-by: Li Jun 
---
  drivers/usb/typec/tcpm.c | 127 +--
  1 file changed, 90 insertions(+), 37 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 677d121..50a1979 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -254,6 +254,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;
 


These 3 variables are now no longer needed.


@@ -1772,40 +1775,65 @@ static int tcpm_pd_check_request(struct tcpm_port *port)
return 0;
  }
  
-static int tcpm_pd_select_pdo(struct tcpm_port *port)

+#define min_power(x, y) min(pdo_max_power(x), pdo_max_power(y))
+#define min_current(x, y) min(pdo_max_current(x), pdo_max_current(y))
+
+static int tcpm_pd_select_pdo(struct tcpm_port *port, int *sink_pdo,
+ int *src_pdo)
  {
-   unsigned int i, max_mw = 0, max_mv = 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;
  
  	/*

-* 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_FIXED) {
+   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) {

-   mw = pdo_max_power(pdo);
+   src_mw = pdo_max_power(pdo);
} else {
-   ma = min(pdo_max_current(pdo),
-port->max_snk_ma);
-   mw = ma * mv / 1000;
+   src_ma = pdo_max_current(pdo);
+   src_mw = src_ma * min_src_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;
+   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);
+   }
+
+   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;
+   }
+   break;
+   }
}
+
}
  
  	return ret;

@@ -1816,13 +1844,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 src_pdo_index, snk_pdo_index;
+   u32 pdo,