Re: [RFC PATCH] usb: typec: ucsi: ack connector change after all tasks finish

From: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
To: Christian Ehrhardt <lk@c--e.de>
Cc: heikki.krogerus@linux.intel.com, gregkh@linuxfoundation.org,
	 neil.armstrong@linaro.org, quic_prashk@quicinc.com,
	dmitry.baryshkov@linaro.org,  fabrice.gasnier@foss.st.com,
	saranya.gopal@intel.com, linux-usb@vger.kernel.org,
	 diogo.ivo@tecnico.ulisboa.pt
Subject: Re: [RFC PATCH] usb: typec: ucsi: ack connector change after all tasks finish
Date: Thu, 28 Mar 2024 16:04:43 +0000	[thread overview]
Message-ID: <nbvsbechvtakxdtsmabtcb4zf2sojmcucv3lhyxe32dgg3z5cy@ljru526jospx> (raw)
In-Reply-To: <ZgRMhB96pVgxnMq4@cae.in-ulm.de>

On Wed, Mar 27, 2024 at 05:42:44PM +0100, Christian Ehrhardt wrote:
> 
> Hi,
> 
> On Wed, Mar 27, 2024 at 12:39:04PM +0000, Diogo Ivo wrote:
> > 				...
> > +static void ucsi_handle_ack_connector_change(struct ucsi_connector *con)
> > +{
> > +	struct ucsi *ucsi = con->ucsi;
> > +	int ret;
> > +
> > +	if (list_empty(&con->partner_tasks)) {
> > +		mutex_lock(&ucsi->ppm_lock);
> > +		ret = ucsi_acknowledge_connector_change(ucsi);
> > +		mutex_unlock(&ucsi->ppm_lock);
> > +
> > +		if (ret)
> > +			dev_err(ucsi->dev, "%s: ACK failed (%d)", __func__, ret);
> 
> What if a real async connector change event happens here? It can because
> you just cleared the connector change condition. But it will be ignored
> because EVENT_PENDING is still set. In practive the new event might even
> be reported in the CCI along with the completion of the ACK command
> above (without an additional async event).

This patch was more to see how this idea would be received so I didn't
think too much about problems with timings, this would come after but
yes, you are correct in pointing this out.

> What UCSI backend (ACPI, CCG, ...) is this?

It is ACPI.

Best regards,

Diogo