Received-SPF: pass (google.com: domain of linux-kernel+bounces-99994-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 12 Mar 2024 11:16:02 +0200
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Krishna Kurapati <quic_kriskura@quicinc.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Johan Hovold <johan+linaro@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Min-Hua Chen <minhuadotchen@gmail.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	quic_ppratap@quicinc.com, quic_jackp@quicinc.com,
	quic_wcheng@quicinc.com, quic_subbaram@quicinc.com,
	stable@vger.kernel.org
Subject: Re: [PATCH] usb: typec: ucsi: Fix race between typec_switch and
 role_switch
Message-ID: <ZfAdUhbsVpMVFmYJ@kuha.fi.intel.com>
References: <20240301040914.458492-1-quic_kriskura@quicinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240301040914.458492-1-quic_kriskura@quicinc.com>

On Fri, Mar 01, 2024 at 09:39:14AM +0530, Krishna Kurapati wrote:
> When orientation switch is enabled in ucsi glink, there is a xhci
> probe failure seen when booting up in host mode in reverse
> orientation.
> 
> During bootup the following things happen in multiple drivers:
> 
> a) DWC3 controller driver initializes the core in device mode when the
> dr_mode is set to DRD. It relies on role_switch call to change role to
> host.
> 
> b) QMP driver initializes the lanes to TYPEC_ORIENTATION_NORMAL as a
> normal routine. It relies on the typec_switch_set call to get notified
> of orientation changes.
> 
> c) UCSI core reads the UCSI_GET_CONNECTOR_STATUS via the glink and
> provides initial role switch to dwc3 controller.
> 
> When booting up in host mode with orientation TYPEC_ORIENTATION_REVERSE,
> then we see the following things happening in order:
> 
> a) UCSI gives initial role as host to dwc3 controller ucsi_register_port.
> Upon receiving this notification, the dwc3 core needs to program GCTL from
> PRTCAP_DEVICE to PRTCAP_HOST and as part of this change, it asserts GCTL
> Core soft reset and waits for it to be  completed before shifting it to
> host. Only after the reset is done will the dwc3_host_init be invoked and
> xhci is probed. DWC3 controller expects that the usb phy's are stable
> during this process i.e., the phy init is already done.
> 
> b) During the 100ms wait for GCTL core soft reset, the actual notification
> from PPM is received by ucsi_glink via pmic glink for changing role to
> host. The pmic_glink_ucsi_notify routine first sends the orientation
> change to QMP and then sends role to dwc3 via ucsi framework. This is
> happening exactly at the time GCTL core soft reset is being processed.
> 
> c) When QMP driver receives typec switch to TYPEC_ORIENTATION_REVERSE, it
> then re-programs the phy at the instant GCTL core soft reset has been
> asserted by dwc3 controller due to which the QMP PLL lock fails in
> qmp_combo_usb_power_on.
> 
> d) After the 100ms of GCTL core soft reset is completed, the dwc3 core
> goes for initializing the host mode and invokes xhci probe. But at this
> point the QMP is non-responsive and as a result, the xhci plat probe fails
> during xhci_reset.
> 
> Fix this by passing orientation switch to available ucsi instances if
> their gpio configuration is available before ucsi_register is invoked so
> that by the time, the pmic_glink_ucsi_notify provides typec_switch to QMP,
> the lane is already configured and the call would be a NOP thus not racing
> with role switch.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: c6165ed2f425 ("usb: ucsi: glink: use the connector orientation GPIO to provide switch events")
> Suggested-by: Wesley Cheng <quic_wcheng@quicinc.com>
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/ucsi/ucsi_glink.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index 0bd3f6dee678..466df7b9f953 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -255,6 +255,20 @@ static void pmic_glink_ucsi_notify(struct work_struct *work)
>  static void pmic_glink_ucsi_register(struct work_struct *work)
>  {
>  	struct pmic_glink_ucsi *ucsi = container_of(work, struct pmic_glink_ucsi, register_work);
> +	int orientation;
> +	int i;
> +
> +	for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) {
> +		if (!ucsi->port_orientation[i])
> +			continue;
> +		orientation = gpiod_get_value(ucsi->port_orientation[i]);
> +
> +		if (orientation >= 0) {
> +			typec_switch_set(ucsi->port_switch[i],
> +					 orientation ? TYPEC_ORIENTATION_REVERSE
> +					     : TYPEC_ORIENTATION_NORMAL);
> +		}
> +	}
>  
>  	ucsi_register(ucsi->ucsi);
>  }
> -- 
> 2.34.1

-- 
heikki