+ Guenter On Thu, Sep 13, 2018 at 6:43 AM Badhri Jagan Sridharan <badhri@xxxxxxxxxx> wrote: > > On Wed, Sep 12, 2018 at 11:39 PM Jack Pham <jackp@xxxxxxxxxxxxxx> wrote: > > > > Hello Badhri, > > > > On Wed, Sep 12, 2018 at 07:11:13PM -0700, Badhri Jagan Sridharan wrote: > > > During hard reset, TCPM turns off the charging path. > > > The spec provides an option for Sink to either drop to vSafe5V or vSafe0V. > > > > This doesn't make sense. By definition the sink isn't sourcing VBUS, so > > how can it control whether to allow the voltage to be 5V or 0V? > > The way I understand it, this is for the current limits that can be > set on the Sink side. > During hard reset, sink has to fallback to VSafe5V or VSafe0V if > higher pd contract was negotiated. > > > > > > > > From USB_PD_R3_0 > > > 2.6.2 Sink Operation > > > .. > > > Serious errors are handled by Hard Reset Signaling issued by either Port > > > Partner. A Hard Reset: > > > resets protocol as for a Soft Reset but also returns the power supply to > > > USB Default Operation (vSafe0V or vSafe5V output) in order to protect the > > > Sink. > > > > I can see how the wording here "vSafe0V *or* vSafe5V" is misleading, as > > I think it actually is both. In later parts of the spec, the source's > > VBUS behavior is well defined in that it must first drop to vSafe0V > > and then return to vSafe5V. Please refer to section 7.1.5. > > > 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 ? > > > > > > > > Add a config option to tcpc_dev and let the device specific driver decide > > > what needs to be done. > > > > > > Signed-off-by: Badhri Jagan Sridharan <Badhri@xxxxxxxxxx> > > > --- > > > drivers/usb/typec/tcpm.c | 7 ++++++- > > > include/linux/usb/tcpm.h | 1 + > > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c > > > index a4e0c027a2a9..350d1a7c4543 100644 > > > --- a/drivers/usb/typec/tcpm.c > > > +++ b/drivers/usb/typec/tcpm.c > > > @@ -3269,7 +3269,12 @@ static void run_state_machine(struct tcpm_port *port) > > > case SNK_HARD_RESET_SINK_OFF: > > > memset(&port->pps_data, 0, sizeof(port->pps_data)); > > > tcpm_set_vconn(port, false); > > > - tcpm_set_charge(port, false); > > > + if (port->tcpc->config->vsafe_5v_hard_reset) > > > > Therefore I think this config option doesn't make sense. > > > > > + tcpm_set_current_limit(port, > > > + tcpm_get_current_limit(port), > > > + 5000); > > > > But I do think this might still be useful at least in ensuring the sink > > returns to drawing only default power during the transition before > > re-establishing a contract. Given that the sink can't control when > > exactly VBUS will go to 0V, is it ok to call set_current_limit() even if > > VBUS is momentarily 0V, so at least it is in preparation for when VBUS > > turns back on? Or would it be safer to do it during the > > SNK_HARD_RESET_SINK_ON state after we know VBUS is back to vSafe5V? > > IMHO Doing it in SNK_HARD_RESET_SINK_ON makes more sense when > vsafe_5v_hard_reset > is not set. > > > > > > > > + else > > > + tcpm_set_charge(port, false); > > > > Regards, > > Jack > > -- > > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > > a Linux Foundation Collaborative Project