Re: [PATCH 2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V

[Jack Pham]

Hi Badhri,

On Thu, Sep 13, 2018 at 04:38:10PM -0700, Badhri Jagan Sridharan wrote:
> On Thu, Sep 13, 2018 at 10:07 AM Jack Pham <jackp@xxxxxxxxxxxxxx> wrote:
> > On Thu, Sep 13, 2018 at 6:43 AM Badhri Jagan Sridharan
> > > Yeah thats for the source. But for sink, Say if the source isnt PD, then,
> > > sink initiated hard resets happen during the connection. Sink would hard reset
> > > couple of times before deeming that the partner is non PD. When connected
> > > to Type-A ports/non-pd partner, vbus is not going to likely drop so there isnt
> > > a reason to setcharge to false or drop the input current limit. Do you agree ?
> >
> > Sure that makes sense. In this case I wonder if TCPM even needs to call
> > set_charge(false) considering it does not yet know if the partner is PD
> > capable or not. For sure, if the partner is PD capable and contract had
> > been previously established, we'd definitely need to set_current_limit()
> > to default levels and/or turn off charging.
> >
> > But in the case of hard reset attempts to try to determine if the source
> > will send its capabilities (thereby being PD capable), wouldn't the
> > initial default current limits still be in place? I think this is the
> > point you're trying to make, that there is no need to disrupt charging
> > if a hard reset is not going to cause VBUS to reset.
> 
> Yes that's right ! I wasnt sure how to put that in words. Thanks for
> doing that !
> You do concur right ?

Yes.

> > To me it sounds like what you're trying to do makes sense only if you
> > can make a run-time determination of a partner's PD capability, and not
> > based on a config option.
> 
> Yes this should be possible. port->pd_capable already has that info.
> 
> To sum it up:
> if the partner is pd capable, set charge to false in SNK_HARD_RESET_SINK_OFF and
> readjust current limits to default in SNK_HARD_RESET_SINK_ON and turn
> on charging.
> 
> If partner is not pd capable, do not turn off charging nor adjust
> current limit as host port is
> not going to respond to hard reset.
> 
> Does it make sense ?

Right, although the "not pd capable" path could also mean the partner is
PD capable but it is not determined yet. For example: if a sink is
connected to a PD source, established a contract, and the sink reboots
and has to initialize TCPM again. Assuming the sink never disconnected
from CC when rebooting, the source won't automatically re-send its
Source Capabilities as it will still be in its previous state(*). The
sink TCPM however would send a hard reset in order to try to
(re)establish a contract, so here is where this path overlaps with the
not-pd-capable case. In this case I think it might be proper to readjust
current limits to default also unless it was already done earlier--I see
it is done in tcpm_reset_port() and in SNK_DISCOVERY, so you might be
covered here.

The question I have is whether you still need to consider calling
set_charge(false) for this example.

(*) Just thought of one more thing, what if the previous contract was
negotiated at greater than 5V VBUS? How does the sink TCPM handle
setting charging parameters after a reboot but prior to hard reset?

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project