| Message ID | 1521541570-25363-1-git-send-email-jun.li@nxp.com |
|---|---|
| Headers | show |
| Series | usb: typec: remove max_snk_* | expand |
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; > } > } > } > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
> -----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; > > > >
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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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