From: Jack Pham <jackp@codeaurora.org>
To: Badhri Jagan Sridharan <badhri@google.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
USB <linux-usb@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Guenter Roeck <linux@roeck-us.net>
Subject: [2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V
Date: Fri, 14 Sep 2018 11:42:42 -0700 [thread overview]
Message-ID: <20180914184242.GC13306@jackp-linux.qualcomm.com> (raw)
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@codeaurora.org> 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
WARNING: multiple messages have this Message-ID (diff)
From: Jack Pham <jackp@codeaurora.org>
To: Badhri Jagan Sridharan <badhri@google.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
USB <linux-usb@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Guenter Roeck <linux@roeck-us.net>
Subject: Re: [PATCH 2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V
Date: Fri, 14 Sep 2018 11:42:42 -0700 [thread overview]
Message-ID: <20180914184242.GC13306@jackp-linux.qualcomm.com> (raw)
In-Reply-To: <CAPTae5+WyxADQfDJivzmCBpFbA8RqpCGKDys6tUpHwF8+vt8iQ@mail.gmail.com>
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@codeaurora.org> 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
next reply other threads:[~2018-09-14 18:42 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-14 18:42 Jack Pham [this message]
2018-09-14 18:42 ` [PATCH 2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V Jack Pham
-- strict thread matches above, loose matches on Subject: below --
2018-09-28 22:44 [2/2] " Badhri Jagan Sridharan
2018-09-28 22:44 ` [PATCH 2/2] " Badhri Jagan Sridharan
2018-09-14 9:54 [1/2] typec: tcpm: Do not disconnect link for self powered devices Heikki Krogerus
2018-09-14 9:54 ` [PATCH 1/2] " Heikki Krogerus
2018-09-13 23:38 [2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V Badhri Jagan Sridharan
2018-09-13 23:38 ` [PATCH 2/2] " Badhri Jagan Sridharan
2018-09-13 23:16 [1/2] typec: tcpm: Do not disconnect link for self powered devices Badhri Jagan Sridharan
2018-09-13 23:16 ` [PATCH 1/2] " Badhri Jagan Sridharan
2018-09-13 17:51 [1/2] " Guenter Roeck
2018-09-13 17:51 ` [PATCH 1/2] " Guenter Roeck
2018-09-13 17:07 [2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V Jack Pham
2018-09-13 17:07 ` [PATCH 2/2] " Jack Pham
2018-09-13 13:45 [2/2] " Badhri Jagan Sridharan
2018-09-13 13:45 ` [PATCH 2/2] " Badhri Jagan Sridharan
2018-09-13 13:43 [2/2] " Badhri Jagan Sridharan
2018-09-13 13:43 ` [PATCH 2/2] " Badhri Jagan Sridharan
2018-09-13 13:24 [1/2] typec: tcpm: Do not disconnect link for self powered devices Heikki Krogerus
2018-09-13 13:24 ` [PATCH 1/2] " Heikki Krogerus
2018-09-13 6:39 [2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V Jack Pham
2018-09-13 6:39 ` [PATCH 2/2] " Jack Pham
2018-09-13 2:11 [2/2] " Badhri Jagan Sridharan
2018-09-13 2:11 ` [PATCH 2/2] " Badhri Jagan Sridharan
2018-09-13 2:11 [1/2] typec: tcpm: Do not disconnect link for self powered devices Badhri Jagan Sridharan
2018-09-13 2:11 ` [PATCH 1/2] " Badhri Jagan Sridharan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180914184242.GC13306@jackp-linux.qualcomm.com \
--to=jackp@codeaurora.org \
--cc=badhri@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=heikki.krogerus@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=linux@roeck-us.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.