Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

From: Kyle Tso <kyletso@google.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Guenter Roeck <linux@roeck-us.net>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Badhri Jagan Sridharan <badhri@google.com>,
	Adam Thomson <Adam.Thomson.Opensource@diasemi.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance
Date: Thu, 3 Oct 2019 18:04:48 +0800	[thread overview]
Message-ID: <CAGZ6i=1mid0Cq2EtWTJHwRzPxXZJnLtTWwYL2QS0vZHNR9mJqQ@mail.gmail.com> (raw)
In-Reply-To: <bd03390e-35fb-2885-d026-b8df58f02283@redhat.com>

Hi Hans

Could you append the TCPM log?

On Thu, Oct 3, 2019 at 5:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Kyle,
>
> On 20-09-2019 05:24, Kyle Tso wrote:
> > *** BLURB HERE ***
> >
> > Kyle Tso (2):
> >    usb: typec: tcpm: AMS and Collision Avoidance
> >    usb: typec: tcpm: AMS for PD2.0
>
> I've finally gotten a chance to test this on one of my own devices
> which uses the tcpm framework for its Type-c port.
>
> I am afraid that this series breaks DP altmode support,
> specifically, the dp_altmode_configure() function no longer
> seems to get called, leading to no pin-assignment being
> selected by default and DP thus not working.
>
> So sorry, but I have to NACK this series since it causes
> regressions.
>
> It might be easiest if you can get yourself some hardware
> which supports DP altmode and uses the fusb302 Type-C
> controller (which unlike your controller is actually
> supported by the mainline kernel).
>
> 2 devices which have this are the original (version 1)
> of the GPD win and the GPD pocket. Since the version
> is not always clearly marked, make sure you get one which
> has a X7-Z8750 CPU, those are the version 1 models, you
> can still get these e.g. here:
>
> https://www.geekbuying.com/item/GPD-Pocket-7-Inch-Tablet-PC-Intel-Atom-X7-Z8750-8GB-128GB-375711.html
> https://www.geekbuying.com/item/GPD-Win-Intel-Z8750-Windows-10-4GB-64GB-Gamepad-Tablet-PC---Black-378018.html
>
> These 2 devices still need 2 minor patches to hookup the DP
> support, I have just finished these 2 patches up and I'm
> submitting them upstream today, I will Cc you on these.
>
> If you combine these with one of the many DP-charging pass-through
> + USB-3 out + HDMI out dongles, e.g.:
> https://www.aliexpress.com/item/32953320909.html
>
> And then after plugging in do:
>
> cat /sys/class/typec/port0-partner/port0-partner.0/displayport/pin_assignment
>
> This should print:
>
> C [D]
>
> But when I add your patches into the mix it prints just:
>
> C D
>
> And these debug pr_err calls never happen:
>
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index 7845df030b72..d14f94078dd9 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -106,6 +106,7 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
>                 break;
>         }
>
> +       pr_err("dp_altmode_configure pin_assign %08x conf %08x\n", pin_assign, DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
>         /* Determining the initial pin assignment. */
>         if (!DP_CONF_GET_PIN_ASSIGN(dp->data.conf)) {
>                 /* Is USB together with DP preferred */
> @@ -115,6 +116,8 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
>                 else if (pin_assign & DP_PIN_ASSIGN_DP_ONLY_MASK)
>                         pin_assign &= DP_PIN_ASSIGN_DP_ONLY_MASK;
>
> +               pr_err("dp_altmode_configure masked pin_assign %08x\n", pin_assign);
> +
>                 if (!pin_assign)
>                         return -EINVAL;
>
>
> Regards,
>
> Hans
>