Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 17 Nov 2020 10:19:18 -0800
From:   Prashant Malani <pmalani@chromium.org>
To:     Utkarsh Patel <utkarsh.h.patel@intel.com>
Cc:     linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        heikki.krogerus@linux.intel.com, enric.balletbo@collabora.com,
        rajmohan.mani@intel.com, azhar.shaikh@intel.com
Subject: Re: [PATCH v2 6/8] platform/chrome: cros_ec_typec: Use Thunderbolt 3
 cable discover mode VDO in USB4 mode
Message-ID: <20201117181918.GB1819103@google.com>
References: <20201113202503.6559-1-utkarsh.h.patel@intel.com>
 <20201113202503.6559-7-utkarsh.h.patel@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201113202503.6559-7-utkarsh.h.patel@intel.com>
Precedence: bulk

Hi Utkarsh,

On Fri, Nov 13, 2020 at 12:25:01PM -0800, Utkarsh Patel wrote:
> Configure Thunderbolt3/USB4 cable generation value by filing Thunderbolt 3
> cable discover mode VDO to support rounded and non-rounded Thunderbolt3/
> USB4 cables.
> While we are here use Thunderbolt 3 cable discover mode VDO to fill active
> cable plug link training value.
> 
> Suggested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
> 
> --
> Changes in v2:
> - No change.
> --
> ---
>  drivers/platform/chrome/cros_ec_typec.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 8111ed1fc574..b7416e82c3b3 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -514,8 +514,18 @@ static int cros_typec_enable_usb4(struct cros_typec_data *typec,
>  	else if (pd_ctrl->control_flags & USB_PD_CTRL_ACTIVE_CABLE)
>  		data.eudo |= EUDO_CABLE_TYPE_RE_TIMER << EUDO_CABLE_TYPE_SHIFT;
>  
> -	data.active_link_training = !!(pd_ctrl->control_flags &
> -				       USB_PD_CTRL_ACTIVE_LINK_UNIDIR);
> +	/*
> +	 * This driver does not have access to the identity information or
> +	 * capabilities of the cable, so we don't know is it a real USB4 or
> +	 * TBT3 cable. Therefore pretending that it's always TBT3 cable by
> +	 * filling the TBT3 Cable VDO.
> +	 */
> +	data.tbt_cable_vdo = TBT_MODE;

Is it safe to be making this assumption unconditionally? It might work for
Intel Mux agent but is it guaranteed to be safe for any other future
mux implementation? In other words, what if a "true" USB4 cable is
connected which doesn't have the Thunderbolt SVID alt mode?

(Pre-fetching some alternatives in case the answer is no)

You might want to check with the Cros EC team if you can repurpose a bit of
the "reserved" field for specifying whether the cable is TBT or not.

Either that or see if there is a way to determine from the pd_ctrl->cable_speed
whether the cable is actually TBT or not.

Failing all the above, perhaps you'll have to wait for the PD discovery stuff
to make it's way through review and use that (note that there may be
timing issues between the Mux update event and PD discovery complete
event reaching the port driver).

Best regards,

-Prashant