| Message ID | 1523944908-8350-1-git-send-email-jun.li@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 17-04-18 08:01, Li Jun wrote: > This patch is a further update for rdo based on [1], which > removed max_snk_ma/mv/mw but kept operating_snk_mw. > operating_snk_mw is only used to judge capability mismatch, > per PD spec, we can achieve this via compare the selected > source PDO and matching sink PDO, also after patch [1], we > don't limit the PDO matching between the same type, so the > rdo operating and max current/power calculation should be > updated accordingly. I do not believe that this is correct, lets take a device with a fusb302 tcpc with the following PDO-s: PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) PDO_VAR(5000, 12000, 3000); And an operating_snk_mw of 2500mW then according to your new code a charger which supplies 12V 2A will now get the mismatch bit set even though it is delivering 24W. I really don't see any way to fix this and I believe we should just keep the operating_snk_mw field. Regards Hans > [1]https://patchwork.kernel.org/patch/10342299/ > > Signed-off-by: Li Jun <jun.li@nxp.com> > --- > drivers/usb/typec/tcpm.c | 52 ++++++++++++++++++++++++++++++------------------ > 1 file changed, 33 insertions(+), 19 deletions(-) > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c > index 27192083..0be04b3 100644 > --- a/drivers/usb/typec/tcpm.c > +++ b/drivers/usb/typec/tcpm.c > @@ -1854,28 +1854,42 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo) > else > mv = pdo_min_voltage(pdo); > > - /* Select maximum available current within the sink pdo's limit */ > - if (type == PDO_TYPE_BATT) { > - mw = min_power(pdo, matching_snk_pdo); > - ma = 1000 * mw / mv; > - } else { > - ma = min_current(pdo, matching_snk_pdo); > - mw = ma * mv / 1000; > - } > - > flags = RDO_USB_COMM | RDO_NO_SUSPEND; > > - /* Set mismatch bit if offered power is less than operating power */ > - max_ma = ma; > - max_mw = mw; > - if (mw < port->operating_snk_mw) { > - flags |= RDO_CAP_MISMATCH; > - if (type == PDO_TYPE_BATT && > - (pdo_max_power(matching_snk_pdo) > pdo_max_power(pdo))) > - max_mw = pdo_max_power(matching_snk_pdo); > - else if (pdo_max_current(matching_snk_pdo) > > - pdo_max_current(pdo)) > + switch (type) { > + case PDO_TYPE_FIXED: > + case PDO_TYPE_VAR: > + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) > + max_ma = pdo_max_power(matching_snk_pdo) * 1000 / mv; > + else > max_ma = pdo_max_current(matching_snk_pdo); > + > + if (max_ma > pdo_max_current(pdo)) { > + flags |= RDO_CAP_MISMATCH; > + ma = pdo_max_current(pdo); > + } else { > + ma = max_ma; > + } > + break; > + case PDO_TYPE_BATT: > + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) > + max_mw = pdo_max_power(matching_snk_pdo); > + else > + max_mw = pdo_max_current(matching_snk_pdo) * > + pdo_min_voltage(matching_snk_pdo) / > + 1000; > + > + if (max_mw > pdo_max_power(pdo)) { > + flags |= RDO_CAP_MISMATCH; > + mw = pdo_max_power(pdo); > + } else { > + mw = max_mw; > + } > + > + ma = mw * 1000 / mv; > + break; > + default: > + break; > } > > tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s polarity=%d", > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Hans de Goede [mailto:hdegoede@redhat.com] > Sent: 2018年4月18日 19:40 > To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; > heikki.krogerus@linux.intel.com > Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx > <linux-imx@nxp.com> > Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw for rdo > > Hi, > > On 17-04-18 08:01, Li Jun wrote: > > This patch is a further update for rdo based on [1], which removed > > max_snk_ma/mv/mw but kept operating_snk_mw. > > operating_snk_mw is only used to judge capability mismatch, per PD > > spec, we can achieve this via compare the selected source PDO and > > matching sink PDO, also after patch [1], we don't limit the PDO > > matching between the same type, so the rdo operating and max > > current/power calculation should be updated accordingly. > > I do not believe that this is correct, lets take a device with a fusb302 tcpc with > the following PDO-s: > > PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) > PDO_VAR(5000, 12000, 3000); > > And an operating_snk_mw of 2500mW then according to your new code a > charger which supplies 12V 2A will now get the mismatch bit set even though it > is delivering 24W. > Per PD spec, my understanding is to judge capability mismatch, we should use *max* current/power, not this operating power. In your case(VAR PDO), the max current is 3000mA, can I say the max power is 3000mA * 5V = 15W? so the max current required at 12V is 15W / 12V = 1.25A, so yes, the 12V@2A is enough, do you think this kind of calculation is reasonable? > I really don't see any way to fix this and I believe we should just keep the > operating_snk_mw field. I had the same worry of breaking existing users, but found there is something not matching PD spec, so want to get comments, if we really need another input (beside sink PDO) in practice, we can keep it. Thanks Jun > > Regards > > Hans > > > > > [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > patchwork.kernel.org%2Fpatch%2F10342299%2F&data=02%7C01%7Cjun.li%4 > 0nxp > > .com%7C796c943804cc4a333a7c08d5a5212e41%7C686ea1d3bc2b4c6fa92cd > 99c5c30 > > > 1635%7C0%7C0%7C636596484284807487&sdata=BH74zzVppAi7Fa8jtRt2cwc > d%2F%2F > > o6idNC5hk5zyoWIA4%3D&reserved=0 > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > --- > > drivers/usb/typec/tcpm.c | 52 > ++++++++++++++++++++++++++++++------------------ > > 1 file changed, 33 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index > > 27192083..0be04b3 100644 > > --- a/drivers/usb/typec/tcpm.c > > +++ b/drivers/usb/typec/tcpm.c > > @@ -1854,28 +1854,42 @@ static int tcpm_pd_build_request(struct > tcpm_port *port, u32 *rdo) > > else > > mv = pdo_min_voltage(pdo); > > > > - /* Select maximum available current within the sink pdo's limit */ > > - if (type == PDO_TYPE_BATT) { > > - mw = min_power(pdo, matching_snk_pdo); > > - ma = 1000 * mw / mv; > > - } else { > > - ma = min_current(pdo, matching_snk_pdo); > > - mw = ma * mv / 1000; > > - } > > - > > flags = RDO_USB_COMM | RDO_NO_SUSPEND; > > > > - /* Set mismatch bit if offered power is less than operating power */ > > - max_ma = ma; > > - max_mw = mw; > > - if (mw < port->operating_snk_mw) { > > - flags |= RDO_CAP_MISMATCH; > > - if (type == PDO_TYPE_BATT && > > - (pdo_max_power(matching_snk_pdo) > pdo_max_power(pdo))) > > - max_mw = pdo_max_power(matching_snk_pdo); > > - else if (pdo_max_current(matching_snk_pdo) > > > - pdo_max_current(pdo)) > > + switch (type) { > > + case PDO_TYPE_FIXED: > > + case PDO_TYPE_VAR: > > + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) > > + max_ma = pdo_max_power(matching_snk_pdo) * 1000 / mv; > > + else > > max_ma = pdo_max_current(matching_snk_pdo); > > + > > + if (max_ma > pdo_max_current(pdo)) { > > + flags |= RDO_CAP_MISMATCH; > > + ma = pdo_max_current(pdo); > > + } else { > > + ma = max_ma; > > + } > > + break; > > + case PDO_TYPE_BATT: > > + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) > > + max_mw = pdo_max_power(matching_snk_pdo); > > + else > > + max_mw = pdo_max_current(matching_snk_pdo) * > > + pdo_min_voltage(matching_snk_pdo) / > > + 1000; > > + > > + if (max_mw > pdo_max_power(pdo)) { > > + flags |= RDO_CAP_MISMATCH; > > + mw = pdo_max_power(pdo); > > + } else { > > + mw = max_mw; > > + } > > + > > + ma = mw * 1000 / mv; > > + break; > > + default: > > + break; > > } > > > > tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s > polarity=%d", > >
Hi, On 20-04-18 11:18, Jun Li wrote: > >> -----Original Message----- >> From: Hans de Goede [mailto:hdegoede@redhat.com] >> Sent: 2018年4月18日 19:40 >> To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; >> heikki.krogerus@linux.intel.com >> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx >> <linux-imx@nxp.com> >> Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw for rdo >> >> Hi, >> >> On 17-04-18 08:01, Li Jun wrote: >>> This patch is a further update for rdo based on [1], which removed >>> max_snk_ma/mv/mw but kept operating_snk_mw. >>> operating_snk_mw is only used to judge capability mismatch, per PD >>> spec, we can achieve this via compare the selected source PDO and >>> matching sink PDO, also after patch [1], we don't limit the PDO >>> matching between the same type, so the rdo operating and max >>> current/power calculation should be updated accordingly. >> >> I do not believe that this is correct, lets take a device with a fusb302 tcpc with >> the following PDO-s: >> >> PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) >> PDO_VAR(5000, 12000, 3000); >> >> And an operating_snk_mw of 2500mW then according to your new code a >> charger which supplies 12V 2A will now get the mismatch bit set even though it >> is delivering 24W. >> > > Per PD spec, my understanding is to judge capability mismatch, we should > use *max* current/power, not this operating power. Ok, I'm not familiar with the PD spec, perhaps someone who knows it well can comment here ? > In your case(VAR PDO), the max current is 3000mA, can I say the max > power is 3000mA * 5V = 15W? so the max current required at 12V > is 15W / 12V = 1.25A, so yes, the 12V@2A is enough, do you think > this kind of calculation is reasonable? That does not sound unreasonable, the real question is when exactly should we set the capability mismatch bit, once we have that more clear how to implement this should become clear too. Regards, Hans > >> I really don't see any way to fix this and I believe we should just keep the >> operating_snk_mw field. > > I had the same worry of breaking existing users, but found there is something > not matching PD spec, so want to get comments, if we really need another input > (beside sink PDO) in practice, we can keep it. > > Thanks > Jun >> >> Regards >> >> Hans >> >> >> >>> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F >>> >> patchwork.kernel.org%2Fpatch%2F10342299%2F&data=02%7C01%7Cjun.li%4 >> 0nxp >>> .com%7C796c943804cc4a333a7c08d5a5212e41%7C686ea1d3bc2b4c6fa92cd >> 99c5c30 >>> >> 1635%7C0%7C0%7C636596484284807487&sdata=BH74zzVppAi7Fa8jtRt2cwc >> d%2F%2F >>> o6idNC5hk5zyoWIA4%3D&reserved=0 >>> >>> Signed-off-by: Li Jun <jun.li@nxp.com> >>> --- >>> drivers/usb/typec/tcpm.c | 52 >> ++++++++++++++++++++++++++++++------------------ >>> 1 file changed, 33 insertions(+), 19 deletions(-) >>> >>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index >>> 27192083..0be04b3 100644 >>> --- a/drivers/usb/typec/tcpm.c >>> +++ b/drivers/usb/typec/tcpm.c >>> @@ -1854,28 +1854,42 @@ static int tcpm_pd_build_request(struct >> tcpm_port *port, u32 *rdo) >>> else >>> mv = pdo_min_voltage(pdo); >>> >>> - /* Select maximum available current within the sink pdo's limit */ >>> - if (type == PDO_TYPE_BATT) { >>> - mw = min_power(pdo, matching_snk_pdo); >>> - ma = 1000 * mw / mv; >>> - } else { >>> - ma = min_current(pdo, matching_snk_pdo); >>> - mw = ma * mv / 1000; >>> - } >>> - >>> flags = RDO_USB_COMM | RDO_NO_SUSPEND; >>> >>> - /* Set mismatch bit if offered power is less than operating power */ >>> - max_ma = ma; >>> - max_mw = mw; >>> - if (mw < port->operating_snk_mw) { >>> - flags |= RDO_CAP_MISMATCH; >>> - if (type == PDO_TYPE_BATT && >>> - (pdo_max_power(matching_snk_pdo) > pdo_max_power(pdo))) >>> - max_mw = pdo_max_power(matching_snk_pdo); >>> - else if (pdo_max_current(matching_snk_pdo) > >>> - pdo_max_current(pdo)) >>> + switch (type) { >>> + case PDO_TYPE_FIXED: >>> + case PDO_TYPE_VAR: >>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) >>> + max_ma = pdo_max_power(matching_snk_pdo) * 1000 / mv; >>> + else >>> max_ma = pdo_max_current(matching_snk_pdo); >>> + >>> + if (max_ma > pdo_max_current(pdo)) { >>> + flags |= RDO_CAP_MISMATCH; >>> + ma = pdo_max_current(pdo); >>> + } else { >>> + ma = max_ma; >>> + } >>> + break; >>> + case PDO_TYPE_BATT: >>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) >>> + max_mw = pdo_max_power(matching_snk_pdo); >>> + else >>> + max_mw = pdo_max_current(matching_snk_pdo) * >>> + pdo_min_voltage(matching_snk_pdo) / >>> + 1000; >>> + >>> + if (max_mw > pdo_max_power(pdo)) { >>> + flags |= RDO_CAP_MISMATCH; >>> + mw = pdo_max_power(pdo); >>> + } else { >>> + mw = max_mw; >>> + } >>> + >>> + ma = mw * 1000 / mv; >>> + break; >>> + default: >>> + break; >>> } >>> >>> tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s >> polarity=%d", >>> -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Hans de Goede [mailto:hdegoede@redhat.com] > Sent: 2018年4月20日 17:21 > To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; > heikki.krogerus@linux.intel.com > Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx > <linux-imx@nxp.com> > Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw for rdo > > Hi, > > On 20-04-18 11:18, Jun Li wrote: > > > >> -----Original Message----- > >> From: Hans de Goede [mailto:hdegoede@redhat.com] > >> Sent: 2018年4月18日 19:40 > >> To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; > >> heikki.krogerus@linux.intel.com > >> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; > >> dl-linux-imx <linux-imx@nxp.com> > >> Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw > >> for rdo > >> > >> Hi, > >> > >> On 17-04-18 08:01, Li Jun wrote: > >>> This patch is a further update for rdo based on [1], which removed > >>> max_snk_ma/mv/mw but kept operating_snk_mw. > >>> operating_snk_mw is only used to judge capability mismatch, per PD > >>> spec, we can achieve this via compare the selected source PDO and > >>> matching sink PDO, also after patch [1], we don't limit the PDO > >>> matching between the same type, so the rdo operating and max > >>> current/power calculation should be updated accordingly. > >> > >> I do not believe that this is correct, lets take a device with a > >> fusb302 tcpc with the following PDO-s: > >> > >> PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) > >> PDO_VAR(5000, 12000, 3000); > >> > >> And an operating_snk_mw of 2500mW then according to your new code a > >> charger which supplies 12V 2A will now get the mismatch bit set even > >> though it is delivering 24W. > >> > > > > Per PD spec, my understanding is to judge capability mismatch, we > > should use *max* current/power, not this operating power. > > Ok, I'm not familiar with the PD spec, perhaps someone who knows it well can > comment here ? Hi Guenter, Could you please comment here? Thanks Jun > > > In your case(VAR PDO), the max current is 3000mA, can I say the max > > power is 3000mA * 5V = 15W? so the max current required at 12V is 15W > > / 12V = 1.25A, so yes, the 12V@2A is enough, do you think this kind of > > calculation is reasonable? > > That does not sound unreasonable, the real question is when exactly should we > set the capability mismatch bit, once we have that more clear how to > implement this should become clear too. Fully agree, below is the capability mismatch bit related description from PD spec: "If the Capability Mismatch bit is set to one The Maximum Operating Current/Power field may contain a value larger than the maximum current/power offered in the Source_Capabilities Message’s PDO as referenced by the Object position field. This enables the Sink to indicate that it requires more current/power than is being offered. If the Sink requires a different voltage this will be indicated by its Sink_Capabilities Message." "6.4.2.8 Maximum Operating Current The Maximum Operating Current field in the Request Message shall be set to the highest current the Sink will ever require. The difference between the Operating Current and Maximum Operating Current fields (when the GiveBack Flag is cleared) is used by the Device Policy Manager in the Source to calculate the size of the Power Reserve to be maintained (see Section 8.2.5.1). The Operating Current value shall be less than or equal to the Maximum Operating Current value. When the Capabilities Mismatch bit is set to zero the requested Maximum Operating Current shall be less than or equal to the current in the offered Source Capabilities since the Source will need to reserve this power for future use. ... When the Capabilities Mismatch bit is set to one the requested Maximum Operating Current may be greater than the current in the offered Source Capabilities since the Source will need this information to ascertain the Sink’s actual needs". > > Regards, > > Hans > > > > > > >> I really don't see any way to fix this and I believe we should just > >> keep the operating_snk_mw field. > > > > I had the same worry of breaking existing users, but found there is > > something not matching PD spec, so want to get comments, if we really > > need another input (beside sink PDO) in practice, we can keep it. > > > > Thanks > > Jun > >> > >> Regards > >> > >> Hans > >> > >> > >> > >>> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F% > >>> 2F > >>> > >> > patchwork.kernel.org%2Fpatch%2F10342299%2F&data=02%7C01%7Cjun.li%4 > >> 0nxp > >>> .com%7C796c943804cc4a333a7c08d5a5212e41%7C686ea1d3bc2b4c6fa9 > 2cd > >> 99c5c30 > >>> > >> > 1635%7C0%7C0%7C636596484284807487&sdata=BH74zzVppAi7Fa8jtRt2cwc > >> d%2F%2F > >>> o6idNC5hk5zyoWIA4%3D&reserved=0 > >>> > >>> Signed-off-by: Li Jun <jun.li@nxp.com> > >>> --- > >>> drivers/usb/typec/tcpm.c | 52 > >> ++++++++++++++++++++++++++++++------------------ > >>> 1 file changed, 33 insertions(+), 19 deletions(-) > >>> > >>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c > >>> index > >>> 27192083..0be04b3 100644 > >>> --- a/drivers/usb/typec/tcpm.c > >>> +++ b/drivers/usb/typec/tcpm.c > >>> @@ -1854,28 +1854,42 @@ static int tcpm_pd_build_request(struct > >> tcpm_port *port, u32 *rdo) > >>> else > >>> mv = pdo_min_voltage(pdo); > >>> > >>> - /* Select maximum available current within the sink pdo's limit */ > >>> - if (type == PDO_TYPE_BATT) { > >>> - mw = min_power(pdo, matching_snk_pdo); > >>> - ma = 1000 * mw / mv; > >>> - } else { > >>> - ma = min_current(pdo, matching_snk_pdo); > >>> - mw = ma * mv / 1000; > >>> - } > >>> - > >>> flags = RDO_USB_COMM | RDO_NO_SUSPEND; > >>> > >>> - /* Set mismatch bit if offered power is less than operating power */ > >>> - max_ma = ma; > >>> - max_mw = mw; > >>> - if (mw < port->operating_snk_mw) { > >>> - flags |= RDO_CAP_MISMATCH; > >>> - if (type == PDO_TYPE_BATT && > >>> - (pdo_max_power(matching_snk_pdo) > > pdo_max_power(pdo))) > >>> - max_mw = pdo_max_power(matching_snk_pdo); > >>> - else if (pdo_max_current(matching_snk_pdo) > > >>> - pdo_max_current(pdo)) > >>> + switch (type) { > >>> + case PDO_TYPE_FIXED: > >>> + case PDO_TYPE_VAR: > >>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) > >>> + max_ma = pdo_max_power(matching_snk_pdo) * 1000 / > mv; > >>> + else > >>> max_ma = pdo_max_current(matching_snk_pdo); > >>> + > >>> + if (max_ma > pdo_max_current(pdo)) { > >>> + flags |= RDO_CAP_MISMATCH; > >>> + ma = pdo_max_current(pdo); > >>> + } else { > >>> + ma = max_ma; > >>> + } > >>> + break; > >>> + case PDO_TYPE_BATT: > >>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) > >>> + max_mw = pdo_max_power(matching_snk_pdo); > >>> + else > >>> + max_mw = pdo_max_current(matching_snk_pdo) * > >>> + pdo_min_voltage(matching_snk_pdo) / > >>> + 1000; > >>> + > >>> + if (max_mw > pdo_max_power(pdo)) { > >>> + flags |= RDO_CAP_MISMATCH; > >>> + mw = pdo_max_power(pdo); > >>> + } else { > >>> + mw = max_mw; > >>> + } > >>> + > >>> + ma = mw * 1000 / mv; > >>> + break; > >>> + default: > >>> + break; > >>> } > >>> > >>> tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s > >> polarity=%d", > >>>
Hi, On 20-04-18 12:51, Jun Li wrote: > >> -----Original Message----- >> From: Hans de Goede [mailto:hdegoede@redhat.com] >> Sent: 2018年4月20日 17:21 >> To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; >> heikki.krogerus@linux.intel.com >> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx >> <linux-imx@nxp.com> >> Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw for rdo >> >> Hi, >> >> On 20-04-18 11:18, Jun Li wrote: >>> >>>> -----Original Message----- >>>> From: Hans de Goede [mailto:hdegoede@redhat.com] >>>> Sent: 2018年4月18日 19:40 >>>> To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; >>>> heikki.krogerus@linux.intel.com >>>> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; >>>> dl-linux-imx <linux-imx@nxp.com> >>>> Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw >>>> for rdo >>>> >>>> Hi, >>>> >>>> On 17-04-18 08:01, Li Jun wrote: >>>>> This patch is a further update for rdo based on [1], which removed >>>>> max_snk_ma/mv/mw but kept operating_snk_mw. >>>>> operating_snk_mw is only used to judge capability mismatch, per PD >>>>> spec, we can achieve this via compare the selected source PDO and >>>>> matching sink PDO, also after patch [1], we don't limit the PDO >>>>> matching between the same type, so the rdo operating and max >>>>> current/power calculation should be updated accordingly. >>>> >>>> I do not believe that this is correct, lets take a device with a >>>> fusb302 tcpc with the following PDO-s: >>>> >>>> PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) >>>> PDO_VAR(5000, 12000, 3000); >>>> >>>> And an operating_snk_mw of 2500mW then according to your new code a >>>> charger which supplies 12V 2A will now get the mismatch bit set even >>>> though it is delivering 24W. >>>> >>> >>> Per PD spec, my understanding is to judge capability mismatch, we >>> should use *max* current/power, not this operating power. >> >> Ok, I'm not familiar with the PD spec, perhaps someone who knows it well can >> comment here ? > > Hi Guenter, > Could you please comment here? > > Thanks > Jun > >> >>> In your case(VAR PDO), the max current is 3000mA, can I say the max >>> power is 3000mA * 5V = 15W? so the max current required at 12V is 15W >>> / 12V = 1.25A, so yes, the 12V@2A is enough, do you think this kind of >>> calculation is reasonable? >> >> That does not sound unreasonable, the real question is when exactly should we >> set the capability mismatch bit, once we have that more clear how to >> implement this should become clear too. > > Fully agree, below is the capability mismatch bit related description from PD spec: > > "If the Capability Mismatch bit is set to one > The Maximum Operating Current/Power field may contain a value larger than > the maximum current/power offered in the Source_Capabilities Message’s PDO > as referenced by the Object position field. This enables the Sink to indicate that > it requires more current/power than is being offered. If the Sink requires a > different voltage this will be indicated by its Sink_Capabilities Message." > > "6.4.2.8 Maximum Operating Current > The Maximum Operating Current field in the Request Message shall be set to > the highest current the Sink will ever require. The difference between > the Operating Current and Maximum Operating Current fields > (when the GiveBack Flag is cleared) is used by the Device Policy Manager > in the Source to calculate the size of the Power Reserve to be maintained > (see Section 8.2.5.1). The Operating Current value shall be less than or > equal to the Maximum Operating Current value. > When the Capabilities Mismatch bit is set to zero the requested Maximum Operating > Current shall be less than or equal to the current in the offered Source Capabilities > since the Source will need to reserve this power for future use. ... > > When the Capabilities Mismatch bit is set to one the requested Maximum > Operating Current may be greater than the current in the offered Source > Capabilities since the Source will need this information to ascertain > the Sink’s actual needs". Thank you for looking that up, so to what value are we setting the "Maximum Operating Current" field in the request we are sending? Regards, Hans > >> >> Regards, >> >> Hans >> >> >> >>> >>>> I really don't see any way to fix this and I believe we should just >>>> keep the operating_snk_mw field. >>> >>> I had the same worry of breaking existing users, but found there is >>> something not matching PD spec, so want to get comments, if we really >>> need another input (beside sink PDO) in practice, we can keep it. >>> >>> Thanks >>> Jun >>>> >>>> Regards >>>> >>>> Hans >>>> >>>> >>>> >>>>> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F% >>>>> 2F >>>>> >>>> >> patchwork.kernel.org%2Fpatch%2F10342299%2F&data=02%7C01%7Cjun.li%4 >>>> 0nxp >>>>> .com%7C796c943804cc4a333a7c08d5a5212e41%7C686ea1d3bc2b4c6fa9 >> 2cd >>>> 99c5c30 >>>>> >>>> >> 1635%7C0%7C0%7C636596484284807487&sdata=BH74zzVppAi7Fa8jtRt2cwc >>>> d%2F%2F >>>>> o6idNC5hk5zyoWIA4%3D&reserved=0 >>>>> >>>>> Signed-off-by: Li Jun <jun.li@nxp.com> >>>>> --- >>>>> drivers/usb/typec/tcpm.c | 52 >>>> ++++++++++++++++++++++++++++++------------------ >>>>> 1 file changed, 33 insertions(+), 19 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c >>>>> index >>>>> 27192083..0be04b3 100644 >>>>> --- a/drivers/usb/typec/tcpm.c >>>>> +++ b/drivers/usb/typec/tcpm.c >>>>> @@ -1854,28 +1854,42 @@ static int tcpm_pd_build_request(struct >>>> tcpm_port *port, u32 *rdo) >>>>> else >>>>> mv = pdo_min_voltage(pdo); >>>>> >>>>> - /* Select maximum available current within the sink pdo's limit */ >>>>> - if (type == PDO_TYPE_BATT) { >>>>> - mw = min_power(pdo, matching_snk_pdo); >>>>> - ma = 1000 * mw / mv; >>>>> - } else { >>>>> - ma = min_current(pdo, matching_snk_pdo); >>>>> - mw = ma * mv / 1000; >>>>> - } >>>>> - >>>>> flags = RDO_USB_COMM | RDO_NO_SUSPEND; >>>>> >>>>> - /* Set mismatch bit if offered power is less than operating power */ >>>>> - max_ma = ma; >>>>> - max_mw = mw; >>>>> - if (mw < port->operating_snk_mw) { >>>>> - flags |= RDO_CAP_MISMATCH; >>>>> - if (type == PDO_TYPE_BATT && >>>>> - (pdo_max_power(matching_snk_pdo) > >> pdo_max_power(pdo))) >>>>> - max_mw = pdo_max_power(matching_snk_pdo); >>>>> - else if (pdo_max_current(matching_snk_pdo) > >>>>> - pdo_max_current(pdo)) >>>>> + switch (type) { >>>>> + case PDO_TYPE_FIXED: >>>>> + case PDO_TYPE_VAR: >>>>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) >>>>> + max_ma = pdo_max_power(matching_snk_pdo) * 1000 / >> mv; >>>>> + else >>>>> max_ma = pdo_max_current(matching_snk_pdo); >>>>> + >>>>> + if (max_ma > pdo_max_current(pdo)) { >>>>> + flags |= RDO_CAP_MISMATCH; >>>>> + ma = pdo_max_current(pdo); >>>>> + } else { >>>>> + ma = max_ma; >>>>> + } >>>>> + break; >>>>> + case PDO_TYPE_BATT: >>>>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) >>>>> + max_mw = pdo_max_power(matching_snk_pdo); >>>>> + else >>>>> + max_mw = pdo_max_current(matching_snk_pdo) * >>>>> + pdo_min_voltage(matching_snk_pdo) / >>>>> + 1000; >>>>> + >>>>> + if (max_mw > pdo_max_power(pdo)) { >>>>> + flags |= RDO_CAP_MISMATCH; >>>>> + mw = pdo_max_power(pdo); >>>>> + } else { >>>>> + mw = max_mw; >>>>> + } >>>>> + >>>>> + ma = mw * 1000 / mv; >>>>> + break; >>>>> + default: >>>>> + break; >>>>> } >>>>> >>>>> tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s >>>> polarity=%d", >>>>> -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/20/2018 03:51 AM, Jun Li wrote: > >> -----Original Message----- >> From: Hans de Goede [mailto:hdegoede@redhat.com] >> Sent: 2018年4月20日 17:21 >> To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; >> heikki.krogerus@linux.intel.com >> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx >> <linux-imx@nxp.com> >> Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw for rdo >> >> Hi, >> >> On 20-04-18 11:18, Jun Li wrote: >>> >>>> -----Original Message----- >>>> From: Hans de Goede [mailto:hdegoede@redhat.com] >>>> Sent: 2018年4月18日 19:40 >>>> To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; >>>> heikki.krogerus@linux.intel.com >>>> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; >>>> dl-linux-imx <linux-imx@nxp.com> >>>> Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw >>>> for rdo >>>> >>>> Hi, >>>> >>>> On 17-04-18 08:01, Li Jun wrote: >>>>> This patch is a further update for rdo based on [1], which removed >>>>> max_snk_ma/mv/mw but kept operating_snk_mw. >>>>> operating_snk_mw is only used to judge capability mismatch, per PD >>>>> spec, we can achieve this via compare the selected source PDO and >>>>> matching sink PDO, also after patch [1], we don't limit the PDO >>>>> matching between the same type, so the rdo operating and max >>>>> current/power calculation should be updated accordingly. >>>> >>>> I do not believe that this is correct, lets take a device with a >>>> fusb302 tcpc with the following PDO-s: >>>> >>>> PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) >>>> PDO_VAR(5000, 12000, 3000); >>>> >>>> And an operating_snk_mw of 2500mW then according to your new code a >>>> charger which supplies 12V 2A will now get the mismatch bit set even >>>> though it is delivering 24W. >>>> >>> >>> Per PD spec, my understanding is to judge capability mismatch, we >>> should use *max* current/power, not this operating power. >> >> Ok, I'm not familiar with the PD spec, perhaps someone who knows it well can >> comment here ? > > Hi Guenter, > Could you please comment here? > I am not, and never was, the kind of person who would blindly follow a specification. I use specifications as guideline. For PD, when I implemented the code, I made sure that the implementation worked with as many power adapters and dongles as I could get my hand on. At this point in time, I would only change the code myself if it is known to be broken. Otherwise I would leave it alone. I would _never_ make a change because my interpretation of the specification is different, unless I can prove that the current code is wrong and causes problems. Unfortunately, I don't currently have a setup to test the current version of the code, nor the time to do all the testing. I am more and more concerned, though, that changes are made out of principle, not because something has been shown to be broken. But that is just a personal concern, and without re-testing everything I don't really have a strong case for or against those changes. Guenter > Thanks > Jun > >> >>> In your case(VAR PDO), the max current is 3000mA, can I say the max >>> power is 3000mA * 5V = 15W? so the max current required at 12V is 15W >>> / 12V = 1.25A, so yes, the 12V@2A is enough, do you think this kind of >>> calculation is reasonable? >> >> That does not sound unreasonable, the real question is when exactly should we >> set the capability mismatch bit, once we have that more clear how to >> implement this should become clear too. > > Fully agree, below is the capability mismatch bit related description from PD spec: > > "If the Capability Mismatch bit is set to one > The Maximum Operating Current/Power field may contain a value larger than > the maximum current/power offered in the Source_Capabilities Message’s PDO > as referenced by the Object position field. This enables the Sink to indicate that > it requires more current/power than is being offered. If the Sink requires a > different voltage this will be indicated by its Sink_Capabilities Message." > > "6.4.2.8 Maximum Operating Current > The Maximum Operating Current field in the Request Message shall be set to > the highest current the Sink will ever require. The difference between > the Operating Current and Maximum Operating Current fields > (when the GiveBack Flag is cleared) is used by the Device Policy Manager > in the Source to calculate the size of the Power Reserve to be maintained > (see Section 8.2.5.1). The Operating Current value shall be less than or > equal to the Maximum Operating Current value. > When the Capabilities Mismatch bit is set to zero the requested Maximum Operating > Current shall be less than or equal to the current in the offered Source Capabilities > since the Source will need to reserve this power for future use. ... > > When the Capabilities Mismatch bit is set to one the requested Maximum > Operating Current may be greater than the current in the offered Source > Capabilities since the Source will need this information to ascertain > the Sink’s actual needs". > >> >> Regards, >> >> Hans >> >> >> >>> >>>> I really don't see any way to fix this and I believe we should just >>>> keep the operating_snk_mw field. >>> >>> I had the same worry of breaking existing users, but found there is >>> something not matching PD spec, so want to get comments, if we really >>> need another input (beside sink PDO) in practice, we can keep it. >>> >>> Thanks >>> Jun >>>> >>>> Regards >>>> >>>> Hans >>>> >>>> >>>> >>>>> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F% >>>>> 2F >>>>> >>>> >> patchwork.kernel.org%2Fpatch%2F10342299%2F&data=02%7C01%7Cjun.li%4 >>>> 0nxp >>>>> .com%7C796c943804cc4a333a7c08d5a5212e41%7C686ea1d3bc2b4c6fa9 >> 2cd >>>> 99c5c30 >>>>> >>>> >> 1635%7C0%7C0%7C636596484284807487&sdata=BH74zzVppAi7Fa8jtRt2cwc >>>> d%2F%2F >>>>> o6idNC5hk5zyoWIA4%3D&reserved=0 >>>>> >>>>> Signed-off-by: Li Jun <jun.li@nxp.com> >>>>> --- >>>>> drivers/usb/typec/tcpm.c | 52 >>>> ++++++++++++++++++++++++++++++------------------ >>>>> 1 file changed, 33 insertions(+), 19 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c >>>>> index >>>>> 27192083..0be04b3 100644 >>>>> --- a/drivers/usb/typec/tcpm.c >>>>> +++ b/drivers/usb/typec/tcpm.c >>>>> @@ -1854,28 +1854,42 @@ static int tcpm_pd_build_request(struct >>>> tcpm_port *port, u32 *rdo) >>>>> else >>>>> mv = pdo_min_voltage(pdo); >>>>> >>>>> - /* Select maximum available current within the sink pdo's limit */ >>>>> - if (type == PDO_TYPE_BATT) { >>>>> - mw = min_power(pdo, matching_snk_pdo); >>>>> - ma = 1000 * mw / mv; >>>>> - } else { >>>>> - ma = min_current(pdo, matching_snk_pdo); >>>>> - mw = ma * mv / 1000; >>>>> - } >>>>> - >>>>> flags = RDO_USB_COMM | RDO_NO_SUSPEND; >>>>> >>>>> - /* Set mismatch bit if offered power is less than operating power */ >>>>> - max_ma = ma; >>>>> - max_mw = mw; >>>>> - if (mw < port->operating_snk_mw) { >>>>> - flags |= RDO_CAP_MISMATCH; >>>>> - if (type == PDO_TYPE_BATT && >>>>> - (pdo_max_power(matching_snk_pdo) > >> pdo_max_power(pdo))) >>>>> - max_mw = pdo_max_power(matching_snk_pdo); >>>>> - else if (pdo_max_current(matching_snk_pdo) > >>>>> - pdo_max_current(pdo)) >>>>> + switch (type) { >>>>> + case PDO_TYPE_FIXED: >>>>> + case PDO_TYPE_VAR: >>>>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) >>>>> + max_ma = pdo_max_power(matching_snk_pdo) * 1000 / >> mv; >>>>> + else >>>>> max_ma = pdo_max_current(matching_snk_pdo); >>>>> + >>>>> + if (max_ma > pdo_max_current(pdo)) { >>>>> + flags |= RDO_CAP_MISMATCH; >>>>> + ma = pdo_max_current(pdo); >>>>> + } else { >>>>> + ma = max_ma; >>>>> + } >>>>> + break; >>>>> + case PDO_TYPE_BATT: >>>>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) >>>>> + max_mw = pdo_max_power(matching_snk_pdo); >>>>> + else >>>>> + max_mw = pdo_max_current(matching_snk_pdo) * >>>>> + pdo_min_voltage(matching_snk_pdo) / >>>>> + 1000; >>>>> + >>>>> + if (max_mw > pdo_max_power(pdo)) { >>>>> + flags |= RDO_CAP_MISMATCH; >>>>> + mw = pdo_max_power(pdo); >>>>> + } else { >>>>> + mw = max_mw; >>>>> + } >>>>> + >>>>> + ma = mw * 1000 / mv; >>>>> + break; >>>>> + default: >>>>> + break; >>>>> } >>>>> >>>>> tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s >>>> polarity=%d", >>>>> -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 04/20/2018 04:13 PM, Guenter Roeck wrote: > On 04/20/2018 03:51 AM, Jun Li wrote: >> >>> -----Original Message----- >>> From: Hans de Goede [mailto:hdegoede@redhat.com] >>> Sent: 2018年4月20日 17:21 >>> To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; >>> heikki.krogerus@linux.intel.com >>> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx >>> <linux-imx@nxp.com> >>> Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw for rdo >>> >>> Hi, >>> >>> On 20-04-18 11:18, Jun Li wrote: >>>> >>>>> -----Original Message----- >>>>> From: Hans de Goede [mailto:hdegoede@redhat.com] >>>>> Sent: 2018年4月18日 19:40 >>>>> To: Jun Li <jun.li@nxp.com>; linux@roeck-us.net; >>>>> heikki.krogerus@linux.intel.com >>>>> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; >>>>> dl-linux-imx <linux-imx@nxp.com> >>>>> Subject: Re: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw >>>>> for rdo >>>>> >>>>> Hi, >>>>> >>>>> On 17-04-18 08:01, Li Jun wrote: >>>>>> This patch is a further update for rdo based on [1], which removed >>>>>> max_snk_ma/mv/mw but kept operating_snk_mw. >>>>>> operating_snk_mw is only used to judge capability mismatch, per PD >>>>>> spec, we can achieve this via compare the selected source PDO and >>>>>> matching sink PDO, also after patch [1], we don't limit the PDO >>>>>> matching between the same type, so the rdo operating and max >>>>>> current/power calculation should be updated accordingly. >>>>> >>>>> I do not believe that this is correct, lets take a device with a >>>>> fusb302 tcpc with the following PDO-s: >>>>> >>>>> PDO_FIXED(5000, 400, PDO_FIXED_FLAGS) >>>>> PDO_VAR(5000, 12000, 3000); >>>>> >>>>> And an operating_snk_mw of 2500mW then according to your new code a >>>>> charger which supplies 12V 2A will now get the mismatch bit set even >>>>> though it is delivering 24W. >>>>> >>>> >>>> Per PD spec, my understanding is to judge capability mismatch, we >>>> should use *max* current/power, not this operating power. >>> >>> Ok, I'm not familiar with the PD spec, perhaps someone who knows it well can >>> comment here ? >> >> Hi Guenter, >> Could you please comment here? >> > > I am not, and never was, the kind of person who would blindly follow a specification. > I use specifications as guideline. For PD, when I implemented the code, I made sure > that the implementation worked with as many power adapters and dongles as I could get > my hand on. > > At this point in time, I would only change the code myself if it is known to be broken. > Otherwise I would leave it alone. I would _never_ make a change because my interpretation > of the specification is different, unless I can prove that the current code is wrong and > causes problems. > > Unfortunately, I don't currently have a setup to test the current version of the code, > nor the time to do all the testing. I am more and more concerned, though, that changes > are made out of principle, not because something has been shown to be broken. But that is > just a personal concern, and without re-testing everything I don't really have a strong > case for or against those changes. AFAIK Jun Li's previous series was actually necessary because some boards only contain some input voltages rather then a range, so that series was triggered by an actual problem encountered with real hardware, right Jun Li ? But yes this patch feels like a change purely based on a (re)interpretation of the SPEC, so lets drop this change and keep the current behavior. Regards, Hans > > Guenter > >> Thanks >> Jun >> >>> >>>> In your case(VAR PDO), the max current is 3000mA, can I say the max >>>> power is 3000mA * 5V = 15W? so the max current required at 12V is 15W >>>> / 12V = 1.25A, so yes, the 12V@2A is enough, do you think this kind of >>>> calculation is reasonable? >>> >>> That does not sound unreasonable, the real question is when exactly should we >>> set the capability mismatch bit, once we have that more clear how to >>> implement this should become clear too. >> >> Fully agree, below is the capability mismatch bit related description from PD spec: >> >> "If the Capability Mismatch bit is set to one >> The Maximum Operating Current/Power field may contain a value larger than >> the maximum current/power offered in the Source_Capabilities Message’s PDO >> as referenced by the Object position field. This enables the Sink to indicate that >> it requires more current/power than is being offered. If the Sink requires a >> different voltage this will be indicated by its Sink_Capabilities Message." >> >> "6.4.2.8 Maximum Operating Current >> The Maximum Operating Current field in the Request Message shall be set to >> the highest current the Sink will ever require. The difference between >> the Operating Current and Maximum Operating Current fields >> (when the GiveBack Flag is cleared) is used by the Device Policy Manager >> in the Source to calculate the size of the Power Reserve to be maintained >> (see Section 8.2.5.1). The Operating Current value shall be less than or >> equal to the Maximum Operating Current value. >> When the Capabilities Mismatch bit is set to zero the requested Maximum Operating >> Current shall be less than or equal to the current in the offered Source Capabilities >> since the Source will need to reserve this power for future use. ... >> When the Capabilities Mismatch bit is set to one the requested Maximum >> Operating Current may be greater than the current in the offered Source >> Capabilities since the Source will need this information to ascertain >> the Sink’s actual needs". >> >>> >>> Regards, >>> >>> Hans >>> >>> >>> >>>> >>>>> I really don't see any way to fix this and I believe we should just >>>>> keep the operating_snk_mw field. >>>> >>>> I had the same worry of breaking existing users, but found there is >>>> something not matching PD spec, so want to get comments, if we really >>>> need another input (beside sink PDO) in practice, we can keep it. >>>> >>>> Thanks >>>> Jun >>>>> >>>>> Regards >>>>> >>>>> Hans >>>>> >>>>> >>>>> >>>>>> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F% >>>>>> 2F >>>>>> >>>>> >>> patchwork.kernel.org%2Fpatch%2F10342299%2F&data=02%7C01%7Cjun.li%4 >>>>> 0nxp >>>>>> .com%7C796c943804cc4a333a7c08d5a5212e41%7C686ea1d3bc2b4c6fa9 >>> 2cd >>>>> 99c5c30 >>>>>> >>>>> >>> 1635%7C0%7C0%7C636596484284807487&sdata=BH74zzVppAi7Fa8jtRt2cwc >>>>> d%2F%2F >>>>>> o6idNC5hk5zyoWIA4%3D&reserved=0 >>>>>> >>>>>> Signed-off-by: Li Jun <jun.li@nxp.com> >>>>>> --- >>>>>> drivers/usb/typec/tcpm.c | 52 >>>>> ++++++++++++++++++++++++++++++------------------ >>>>>> 1 file changed, 33 insertions(+), 19 deletions(-) >>>>>> >>>>>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c >>>>>> index >>>>>> 27192083..0be04b3 100644 >>>>>> --- a/drivers/usb/typec/tcpm.c >>>>>> +++ b/drivers/usb/typec/tcpm.c >>>>>> @@ -1854,28 +1854,42 @@ static int tcpm_pd_build_request(struct >>>>> tcpm_port *port, u32 *rdo) >>>>>> else >>>>>> mv = pdo_min_voltage(pdo); >>>>>> >>>>>> - /* Select maximum available current within the sink pdo's limit */ >>>>>> - if (type == PDO_TYPE_BATT) { >>>>>> - mw = min_power(pdo, matching_snk_pdo); >>>>>> - ma = 1000 * mw / mv; >>>>>> - } else { >>>>>> - ma = min_current(pdo, matching_snk_pdo); >>>>>> - mw = ma * mv / 1000; >>>>>> - } >>>>>> - >>>>>> flags = RDO_USB_COMM | RDO_NO_SUSPEND; >>>>>> >>>>>> - /* Set mismatch bit if offered power is less than operating power */ >>>>>> - max_ma = ma; >>>>>> - max_mw = mw; >>>>>> - if (mw < port->operating_snk_mw) { >>>>>> - flags |= RDO_CAP_MISMATCH; >>>>>> - if (type == PDO_TYPE_BATT && >>>>>> - (pdo_max_power(matching_snk_pdo) > >>> pdo_max_power(pdo))) >>>>>> - max_mw = pdo_max_power(matching_snk_pdo); >>>>>> - else if (pdo_max_current(matching_snk_pdo) > >>>>>> - pdo_max_current(pdo)) >>>>>> + switch (type) { >>>>>> + case PDO_TYPE_FIXED: >>>>>> + case PDO_TYPE_VAR: >>>>>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) >>>>>> + max_ma = pdo_max_power(matching_snk_pdo) * 1000 / >>> mv; >>>>>> + else >>>>>> max_ma = pdo_max_current(matching_snk_pdo); >>>>>> + >>>>>> + if (max_ma > pdo_max_current(pdo)) { >>>>>> + flags |= RDO_CAP_MISMATCH; >>>>>> + ma = pdo_max_current(pdo); >>>>>> + } else { >>>>>> + ma = max_ma; >>>>>> + } >>>>>> + break; >>>>>> + case PDO_TYPE_BATT: >>>>>> + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) >>>>>> + max_mw = pdo_max_power(matching_snk_pdo); >>>>>> + else >>>>>> + max_mw = pdo_max_current(matching_snk_pdo) * >>>>>> + pdo_min_voltage(matching_snk_pdo) / >>>>>> + 1000; >>>>>> + >>>>>> + if (max_mw > pdo_max_power(pdo)) { >>>>>> + flags |= RDO_CAP_MISMATCH; >>>>>> + mw = pdo_max_power(pdo); >>>>>> + } else { >>>>>> + mw = max_mw; >>>>>> + } >>>>>> + >>>>>> + ma = mw * 1000 / mv; >>>>>> + break; >>>>>> + default: >>>>>> + break; >>>>>> } >>>>>> >>>>>> tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s >>>>> polarity=%d", >>>>>> > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index 27192083..0be04b3 100644 --- a/drivers/usb/typec/tcpm.c +++ b/drivers/usb/typec/tcpm.c @@ -1854,28 +1854,42 @@ static int tcpm_pd_build_request(struct tcpm_port *port, u32 *rdo) else mv = pdo_min_voltage(pdo); - /* Select maximum available current within the sink pdo's limit */ - if (type == PDO_TYPE_BATT) { - mw = min_power(pdo, matching_snk_pdo); - ma = 1000 * mw / mv; - } else { - ma = min_current(pdo, matching_snk_pdo); - mw = ma * mv / 1000; - } - flags = RDO_USB_COMM | RDO_NO_SUSPEND; - /* Set mismatch bit if offered power is less than operating power */ - max_ma = ma; - max_mw = mw; - if (mw < port->operating_snk_mw) { - flags |= RDO_CAP_MISMATCH; - if (type == PDO_TYPE_BATT && - (pdo_max_power(matching_snk_pdo) > pdo_max_power(pdo))) - max_mw = pdo_max_power(matching_snk_pdo); - else if (pdo_max_current(matching_snk_pdo) > - pdo_max_current(pdo)) + switch (type) { + case PDO_TYPE_FIXED: + case PDO_TYPE_VAR: + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) + max_ma = pdo_max_power(matching_snk_pdo) * 1000 / mv; + else max_ma = pdo_max_current(matching_snk_pdo); + + if (max_ma > pdo_max_current(pdo)) { + flags |= RDO_CAP_MISMATCH; + ma = pdo_max_current(pdo); + } else { + ma = max_ma; + } + break; + case PDO_TYPE_BATT: + if (pdo_type(matching_snk_pdo) == PDO_TYPE_BATT) + max_mw = pdo_max_power(matching_snk_pdo); + else + max_mw = pdo_max_current(matching_snk_pdo) * + pdo_min_voltage(matching_snk_pdo) / + 1000; + + if (max_mw > pdo_max_power(pdo)) { + flags |= RDO_CAP_MISMATCH; + mw = pdo_max_power(pdo); + } else { + mw = max_mw; + } + + ma = mw * 1000 / mv; + break; + default: + break; } tcpm_log(port, "cc=%d cc1=%d cc2=%d vbus=%d vconn=%s polarity=%d",
This patch is a further update for rdo based on [1], which removed max_snk_ma/mv/mw but kept operating_snk_mw. operating_snk_mw is only used to judge capability mismatch, per PD spec, we can achieve this via compare the selected source PDO and matching sink PDO, also after patch [1], we don't limit the PDO matching between the same type, so the rdo operating and max current/power calculation should be updated accordingly. [1]https://patchwork.kernel.org/patch/10342299/ Signed-off-by: Li Jun <jun.li@nxp.com> --- drivers/usb/typec/tcpm.c | 52 ++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 19 deletions(-)