LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Icenowy Zheng <icenowy@aosc.io>
To: Kyle Tso <kyletso@google.com>, Guenter Roeck <linux@roeck-us.net>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	Badhri Jagan Sridharan <badhri@google.com>
Subject: Re: [PATCH] usb: typec: tcpm: always rediscover when swapping DR
Date: Wed, 18 Aug 2021 17:27:04 +0800	[thread overview]
Message-ID: <ADFCCA37-D216-407A-BDC6-01DB7F14548B@aosc.io> (raw)
In-Reply-To: <CAGZ6i=1s9X58tOwoiGAxMkMVBTyGTjysOSe9bP8Q4WosmCtymw@mail.gmail.com>



于 2021年8月18日 GMT+08:00 下午4:02:24, Kyle Tso <kyletso@google.com> 写到:
>On Tue, Aug 17, 2021 at 11:13 PM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 8/17/21 2:36 AM, Heikki Krogerus wrote:
>> > On Fri, Aug 13, 2021 at 12:31:31PM +0800, Icenowy Zheng wrote:
>> >> Currently, TCPM code omits discover when swapping to gadget, and assume
>> >> that no altmodes are available when swapping from gadget. However, we do
>> >> send discover when we get attached as gadget -- this leads to modes to be
>> >> discovered twice when attached as gadget and then swap to host.
>> >>
>> >> Always re-send discover when swapping DR, regardless of what change is
>> >> being made; and because of this, the assumption that no altmodes are
>> >> registered with gadget role is broken, and altmodes de-registeration is
>> >> always needed now.
>> >>
>> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> >> ---
>> >>   drivers/usb/typec/tcpm/tcpm.c | 9 ++++-----
>> >>   1 file changed, 4 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
>> >> index b9bb63d749ec..ab6d0d51ee1c 100644
>> >> --- a/drivers/usb/typec/tcpm/tcpm.c
>> >> +++ b/drivers/usb/typec/tcpm/tcpm.c
>> >> @@ -4495,15 +4495,14 @@ static void run_state_machine(struct tcpm_port *port)
>> >>              tcpm_set_state(port, ready_state(port), 0);
>> >>              break;
>> >>      case DR_SWAP_CHANGE_DR:
>> >> -            if (port->data_role == TYPEC_HOST) {
>> >> -                    tcpm_unregister_altmodes(port);
>> >> +            tcpm_unregister_altmodes(port);
>> >> +            if (port->data_role == TYPEC_HOST)
>> >>                      tcpm_set_roles(port, true, port->pwr_role,
>> >>                                     TYPEC_DEVICE);
>> >> -            } else {
>> >> +            else
>> >>                      tcpm_set_roles(port, true, port->pwr_role,
>> >>                                     TYPEC_HOST);
>> >> -                    port->send_discover = true;
>> >> -            }
>> >> +            port->send_discover = true;
>> >>              tcpm_ams_finish(port);
>> >>              tcpm_set_state(port, ready_state(port), 0);
>> >>              break;
>> >
>> > Why is it necessary to do discovery with data role swap in general?
>> >
>> > thanks,
>> >
>>
>> Additional question: There are two patches pending related to DR_SWAP
>> and discovery. Are they both needed, or do they both solve the same
>> problem ?
>>
>> Thanks,
>> Guenter
>
>Hi, I just noticed this patch.
>
>Part of this patch and part of my patch
>https://lore.kernel.org/r/20210816075449.2236547-1-kyletso@google.com
>are to solve the same problem that Discover_Identity is not sent in a
>case where the port becomes UFP after DR_SWAP while in PD3.
>
>The difference (for the DR_SWAP part) is that my patch does not set
>the flag "send_discover" if the port becomes UFP after PD2 DR_SWAP.
>That is because in PD2 Spec, UFP is not allowed to be the SVDM
>Initiator.
>
>This patch indeed solves another problem where
>tcpm_unregister_altmodes should be called during PD3 DR_SWAP because
>the port partner may return mode data in the latest Discover_Mode. For
>the PD2 case, I don't think it needs to be called because PD2 DFP will
>always return NAK for Discover_Mode. However it is fine because it is

It sounds like my dock is doing something against the specification...

It sends altmodes rather than NAK when my board gets attached as
UFP and send bogus discovers (because of bugs of current TCPM
code). These altmodes are then registered in TCPM and never get
cleaned up within it, which leads to conflict when the dock is
removed and then re-inserted.

(BTW is it neceesary for data role and power role to be the same
when attaching? My board now gets attached as UFP to drain
power, and then do DR_SWAP to become USB host.)

>safe to call tcpm_unregister_altmodes even if there is no mode data.
>
>In fact, when I was tracing the code I found another bug. PD2 UFP is
>not allowed to send Discover_Identity and Discover_Mode. I can send
>another patch to address this problem.
>
>thanks,
>Kyle

  parent reply	other threads:[~2021-08-18  9:27 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-13  4:31 Icenowy Zheng
2021-08-17  9:36 ` Heikki Krogerus
2021-08-17  9:41   ` Icenowy Zheng
2021-08-17 11:00     ` Heikki Krogerus
2021-08-18 13:56       ` Greg Kroah-Hartman
2021-08-18 14:02         ` Icenowy Zheng
2021-08-18 14:40           ` Greg Kroah-Hartman
2021-08-18 16:13             ` Kyle Tso
2021-08-18 16:17               ` Icenowy Zheng
2021-08-18 16:24               ` Icenowy Zheng
2021-08-17 15:13   ` Guenter Roeck
2021-08-18  8:02     ` Kyle Tso
2021-08-18  8:16       ` Kyle Tso
2021-08-18  9:20       ` Icenowy Zheng
2021-08-18  9:27       ` Icenowy Zheng [this message]
2021-08-18 10:47         ` Kyle Tso

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=ADFCCA37-D216-407A-BDC6-01DB7F14548B@aosc.io \
    --to=icenowy@aosc.io \
    --cc=badhri@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=kyletso@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --subject='Re: [PATCH] usb: typec: tcpm: always rediscover when swapping DR' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).