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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180913170746.GB13306@jackp-linux.qualcomm.com>
Date:   Thu, 13 Sep 2018 10:07:46 -0700
From:   Jack Pham <jackp@...eaurora.org>
To:     Badhri Jagan Sridharan <badhri@...gle.com>
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        USB <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH 2/2] typec: tcpm: Add option to maintain current limit at
 Vsafe5V

On Thu, Sep 13, 2018 at 6:43 AM Badhri Jagan Sridharan
<badhri@...gle.com> wrote:
>
> On Wed, Sep 12, 2018 at 11:39 PM Jack Pham <jackp@...eaurora.org> 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.

Ok, you are talking about sink current draw limits but vSafe{0,5}V are
voltage definitions so these are orthogonal. Again, the sink can't
directly dictate the voltage that's being sourced so I don't see how it
has a choice here. If a PD contract was negotiated for greater than 5V
and a hard reset happens then yes the voltage would fall to 0V and then
rise back to 5V and during this time sink needs to draw appropriate
current.

> > > 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 ?

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.

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.

Thanks,
Jack

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