Received-SPF: pass (google.com: domain of linux-kernel+bounces-136331-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 9 Apr 2024 09:58:30 +0300
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	linux-usb@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] usb: typec: ucsi: add callback for connector status
 updates
Message-ID: <ZhTnFq4yuUHK83eM@kuha.fi.intel.com>
References: <20240408-ucsi-orient-aware-v1-0-95a74a163a10@linaro.org>
 <20240408-ucsi-orient-aware-v1-1-95a74a163a10@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240408-ucsi-orient-aware-v1-1-95a74a163a10@linaro.org>

On Mon, Apr 08, 2024 at 07:30:49AM +0300, Dmitry Baryshkov wrote:
> Allow UCSI glue driver to perform addtional work to update connector
> status. For example, it might check the cable orientation.  This call is
> performed after reading new connector statatus, so the platform driver
> can peek at new connection status bits.
> 
> The callback is called both when registering the port and when the
> connector change event is being handled.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 6 ++++++
>  drivers/usb/typec/ucsi/ucsi.h | 3 +++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 3106e69050cd..7ad544c968e4 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1199,6 +1199,9 @@ static void ucsi_handle_connector_change(struct work_struct *work)
>  
>  	trace_ucsi_connector_change(con->num, &con->status);
>  
> +	if (ucsi->ops->connector_status)
> +		ucsi->ops->connector_status(con);
> +
>  	role = !!(con->status.flags & UCSI_CONSTAT_PWR_DIR);
>  
>  	if (con->status.change & UCSI_CONSTAT_POWER_DIR_CHANGE) {
> @@ -1588,6 +1591,9 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
>  	}
>  	ret = 0; /* ucsi_send_command() returns length on success */
>  
> +	if (ucsi->ops->connector_status)
> +		ucsi->ops->connector_status(con);
> +
>  	switch (UCSI_CONSTAT_PARTNER_TYPE(con->status.flags)) {
>  	case UCSI_CONSTAT_PARTNER_TYPE_UFP:
>  	case UCSI_CONSTAT_PARTNER_TYPE_CABLE_AND_UFP:
> diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> index 2caf2969668c..6599fbd09bee 100644
> --- a/drivers/usb/typec/ucsi/ucsi.h
> +++ b/drivers/usb/typec/ucsi/ucsi.h
> @@ -53,12 +53,14 @@ struct dentry;
>  #define UCSI_CCI_ERROR			BIT(30)
>  #define UCSI_CCI_COMMAND_COMPLETE	BIT(31)
>  
> +struct ucsi_connector;

Let's keep the forward declarations in one place. Please move that to
the beginning of the file where the other forward declarations are.

thanks,

>  /**
>   * struct ucsi_operations - UCSI I/O operations
>   * @read: Read operation
>   * @sync_write: Blocking write operation
>   * @async_write: Non-blocking write operation
>   * @update_altmodes: Squashes duplicate DP altmodes
> + * @connector_status: Updates connector status, called holding connector lock
>   *
>   * Read and write routines for UCSI interface. @sync_write must wait for the
>   * Command Completion Event from the PPM before returning, and @async_write must
> @@ -73,6 +75,7 @@ struct ucsi_operations {
>  			   const void *val, size_t val_len);
>  	bool (*update_altmodes)(struct ucsi *ucsi, struct ucsi_altmode *orig,
>  				struct ucsi_altmode *updated);
> +	void (*connector_status)(struct ucsi_connector *con);
>  };
>  
>  struct ucsi *ucsi_create(struct device *dev, const struct ucsi_operations *ops);

-- 
heikki