RE: [PATCH RFC] usb: typec: tcpm: remove operating_snk_mw for rdo

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Hans de Goede [mailto:hdegoede@xxxxxxxxxx]
> Sent: 2018年4月18日 19:40
> To: Jun Li <jun.li@xxxxxxx>; linux@xxxxxxxxxxxx;
> heikki.krogerus@xxxxxxxxxxxxxxx
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>
> 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@xxxxxxx>
> > ---
> >   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",
> >
��.n��������+%������w��{.n�����{���)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux