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