diff mbox series

[RFC,v2,1/2] usb: typec: ucsi: Helper for Get Connector Status command

Message ID 20240903145342.3449969-2-heikki.krogerus@linux.intel.com (mailing list archive)
State New
Headers show
Series usb: typec: ucsi: New way to handle GET_CONNECTOR_STATUS | expand

Commit Message

Heikki Krogerus Sept. 3, 2024, 2:53 p.m. UTC 
That command is executed from several locations in the
driver, so providing a dedicated helper function for it.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/ucsi/ucsi.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Comments

Abhishek Pandit-Subedi Sept. 4, 2024, 5:17 a.m. UTC | #1 
On Tue, Sep 3, 2024 at 7:53 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> That command is executed from several locations in the
> driver, so providing a dedicated helper function for it.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 35dce4057c25..71cf64a424d9 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -646,6 +646,16 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
>         }
>  }
>
> +static int ucsi_get_connector_status(struct ucsi_connector *con, bool conn_ack)
> +{
> +       u64 command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
> +       struct ucsi *ucsi = con->ucsi;
> +       int ret;
> +
> +       ret = ucsi_send_command_common(ucsi, command, &con->status, sizeof(con->status), conn_ack);
> +       return ret < 0 ? ret : 0;
> +}
> +
>  static int ucsi_read_pdos(struct ucsi_connector *con,
>                           enum typec_role role, int is_partner,
>                           u32 *pdos, int offset, int num_pdos)
> @@ -1123,12 +1133,10 @@ static void ucsi_partner_change(struct ucsi_connector *con)
>  static int ucsi_check_connection(struct ucsi_connector *con)
>  {
>         u8 prev_flags = con->status.flags;
> -       u64 command;
>         int ret;
>
> -       command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
> -       ret = ucsi_send_command(con->ucsi, command, &con->status, sizeof(con->status));
> -       if (ret < 0) {
> +       ret = ucsi_get_connector_status(con, false);
> +       if (ret) {
>                 dev_err(con->ucsi->dev, "GET_CONNECTOR_STATUS failed (%d)\n", ret);
>                 return ret;
>         }
> @@ -1192,16 +1200,12 @@ static void ucsi_handle_connector_change(struct work_struct *work)
>                                                   work);
>         struct ucsi *ucsi = con->ucsi;
>         enum typec_role role;
> -       u64 command;
>         int ret;
>
>         mutex_lock(&con->lock);
>
> -       command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
> -
> -       ret = ucsi_send_command_common(ucsi, command, &con->status,
> -                                      sizeof(con->status), true);
> -       if (ret < 0) {
> +       ret = ucsi_get_connector_status(con, true);
> +       if (ret) {
>                 dev_err(ucsi->dev, "%s: GET_CONNECTOR_STATUS failed (%d)\n",
>                         __func__, ret);
>                 clear_bit(EVENT_PENDING, &con->ucsi->flags);
> @@ -1611,14 +1615,11 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
>         }
>
>         /* Get the status */
> -       command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
> -       ret = ucsi_send_command(ucsi, command, &con->status, sizeof(con->status));
> -       if (ret < 0) {
> +       ret = ucsi_get_connector_status(con, false);
> +       if (ret) {
>                 dev_err(ucsi->dev, "con%d: failed to get status\n", con->num);
> -               ret = 0;
>                 goto out;
>         }
> -       ret = 0; /* ucsi_send_command() returns length on success */
>
>         if (ucsi->ops->connector_status)
>                 ucsi->ops->connector_status(con);
> --
> 2.45.2
>

Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Dmitry Baryshkov Sept. 12, 2024, 10:08 a.m. UTC | #2 
On Tue, Sep 03, 2024 at 05:53:41PM GMT, Heikki Krogerus wrote:
> That command is executed from several locations in the
> driver, so providing a dedicated helper function for it.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/usb/typec/ucsi/ucsi.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
diff mbox series

Patch

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 35dce4057c25..71cf64a424d9 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -646,6 +646,16 @@  static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
 	}
 }
 
+static int ucsi_get_connector_status(struct ucsi_connector *con, bool conn_ack)
+{
+	u64 command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
+	struct ucsi *ucsi = con->ucsi;
+	int ret;
+
+	ret = ucsi_send_command_common(ucsi, command, &con->status, sizeof(con->status), conn_ack);
+	return ret < 0 ? ret : 0;
+}
+
 static int ucsi_read_pdos(struct ucsi_connector *con,
 			  enum typec_role role, int is_partner,
 			  u32 *pdos, int offset, int num_pdos)
@@ -1123,12 +1133,10 @@  static void ucsi_partner_change(struct ucsi_connector *con)
 static int ucsi_check_connection(struct ucsi_connector *con)
 {
 	u8 prev_flags = con->status.flags;
-	u64 command;
 	int ret;
 
-	command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
-	ret = ucsi_send_command(con->ucsi, command, &con->status, sizeof(con->status));
-	if (ret < 0) {
+	ret = ucsi_get_connector_status(con, false);
+	if (ret) {
 		dev_err(con->ucsi->dev, "GET_CONNECTOR_STATUS failed (%d)\n", ret);
 		return ret;
 	}
@@ -1192,16 +1200,12 @@  static void ucsi_handle_connector_change(struct work_struct *work)
 						  work);
 	struct ucsi *ucsi = con->ucsi;
 	enum typec_role role;
-	u64 command;
 	int ret;
 
 	mutex_lock(&con->lock);
 
-	command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
-
-	ret = ucsi_send_command_common(ucsi, command, &con->status,
-				       sizeof(con->status), true);
-	if (ret < 0) {
+	ret = ucsi_get_connector_status(con, true);
+	if (ret) {
 		dev_err(ucsi->dev, "%s: GET_CONNECTOR_STATUS failed (%d)\n",
 			__func__, ret);
 		clear_bit(EVENT_PENDING, &con->ucsi->flags);
@@ -1611,14 +1615,11 @@  static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
 	}
 
 	/* Get the status */
-	command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
-	ret = ucsi_send_command(ucsi, command, &con->status, sizeof(con->status));
-	if (ret < 0) {
+	ret = ucsi_get_connector_status(con, false);
+	if (ret) {
 		dev_err(ucsi->dev, "con%d: failed to get status\n", con->num);
-		ret = 0;
 		goto out;
 	}
-	ret = 0; /* ucsi_send_command() returns length on success */
 
 	if (ucsi->ops->connector_status)
 		ucsi->ops->connector_status(con);