Received-SPF: pass (google.com: domain of linux-kernel+bounces-129597-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 3 Apr 2024 13:40:45 +0300
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: "Christian A. Ehrhardt" <lk@c--e.de>
Cc: linux-kernel@vger.kernel.org,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Prashant Malani <pmalani@chromium.org>,
	Jameson Thies <jthies@google.com>,
	Abhishek Pandit-Subedi <abhishekpandit@chromium.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= <u.kleine-koenig@pengutronix.de>,
	Samuel =?utf-8?B?xIxhdm9q?= <samuel@cavoj.net>,
	linux-usb@vger.kernel.org
Subject: Re: [PATCH 3/3] usb: typec: ucsi_acpi: Remove Dell quirk
Message-ID: <Zg0yLXfVa0wmA2GZ@kuha.fi.intel.com>
References: <20240327224554.1772525-1-lk@c--e.de>
 <20240327224554.1772525-4-lk@c--e.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240327224554.1772525-4-lk@c--e.de>

On Wed, Mar 27, 2024 at 11:45:54PM +0100, Christian A. Ehrhardt wrote:
> The Dell quirk from ucsi_acpi.c. The quirk is no longer
> necessary as we no longer send lone connector change acks.
> 
> Signed-off-by: Christian A. Ehrhardt <lk@c--e.de>

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

> ---
>  drivers/usb/typec/ucsi/ucsi_acpi.c | 53 +-----------------------------
>  1 file changed, 1 insertion(+), 52 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
> index cc03a49c589c..8d112c3edae5 100644
> --- a/drivers/usb/typec/ucsi/ucsi_acpi.c
> +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
> @@ -23,7 +23,6 @@ struct ucsi_acpi {
>  	void *base;
>  	struct completion complete;
>  	unsigned long flags;
> -#define UCSI_ACPI_SUPPRESS_EVENT	0
>  #define UCSI_ACPI_COMMAND_PENDING	1
>  #define UCSI_ACPI_ACK_PENDING		2
>  	guid_t guid;
> @@ -129,49 +128,6 @@ static const struct ucsi_operations ucsi_zenbook_ops = {
>  	.async_write = ucsi_acpi_async_write
>  };
>  
> -/*
> - * Some Dell laptops don't like ACK commands with the
> - * UCSI_ACK_CONNECTOR_CHANGE but not the UCSI_ACK_COMMAND_COMPLETE
> - * bit set. To work around this send a dummy command and bundle the
> - * UCSI_ACK_CONNECTOR_CHANGE with the UCSI_ACK_COMMAND_COMPLETE
> - * for the dummy command.
> - */
> -static int
> -ucsi_dell_sync_write(struct ucsi *ucsi, unsigned int offset,
> -		     const void *val, size_t val_len)
> -{
> -	struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
> -	u64 cmd = *(u64 *)val;
> -	u64 dummycmd = UCSI_GET_CAPABILITY;
> -	int ret;
> -
> -	if (cmd == (UCSI_ACK_CC_CI | UCSI_ACK_CONNECTOR_CHANGE)) {
> -		cmd |= UCSI_ACK_COMMAND_COMPLETE;
> -
> -		/*
> -		 * The UCSI core thinks it is sending a connector change ack
> -		 * and will accept new connector change events. We don't want
> -		 * this to happen for the dummy command as its response will
> -		 * still report the very event that the core is trying to clear.
> -		 */
> -		set_bit(UCSI_ACPI_SUPPRESS_EVENT, &ua->flags);
> -		ret = ucsi_acpi_sync_write(ucsi, UCSI_CONTROL, &dummycmd,
> -					   sizeof(dummycmd));
> -		clear_bit(UCSI_ACPI_SUPPRESS_EVENT, &ua->flags);
> -
> -		if (ret < 0)
> -			return ret;
> -	}
> -
> -	return ucsi_acpi_sync_write(ucsi, UCSI_CONTROL, &cmd, sizeof(cmd));
> -}
> -
> -static const struct ucsi_operations ucsi_dell_ops = {
> -	.read = ucsi_acpi_read,
> -	.sync_write = ucsi_dell_sync_write,
> -	.async_write = ucsi_acpi_async_write
> -};
> -
>  static const struct dmi_system_id ucsi_acpi_quirks[] = {
>  	{
>  		.matches = {
> @@ -180,12 +136,6 @@ static const struct dmi_system_id ucsi_acpi_quirks[] = {
>  		},
>  		.driver_data = (void *)&ucsi_zenbook_ops,
>  	},
> -	{
> -		.matches = {
> -			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> -		},
> -		.driver_data = (void *)&ucsi_dell_ops,
> -	},
>  	{ }
>  };
>  
> @@ -199,8 +149,7 @@ static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
>  	if (ret)
>  		return;
>  
> -	if (UCSI_CCI_CONNECTOR(cci) &&
> -	    !test_bit(UCSI_ACPI_SUPPRESS_EVENT, &ua->flags))
> +	if (UCSI_CCI_CONNECTOR(cci))
>  		ucsi_connector_change(ua->ucsi, UCSI_CCI_CONNECTOR(cci));
>  
>  	if (cci & UCSI_CCI_ACK_COMPLETE &&
> -- 
> 2.40.1

-- 
heikki