Received-SPF: pass (google.com: domain of linux-kernel+bounces-135467-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <22d7abd1-139d-405b-984e-e4b57cc989cc@quicinc.com>
Date: Mon, 8 Apr 2024 19:16:46 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Krishna Kurapati PSSNV <quic_kriskura@quicinc.com>
Subject: Re: [PATCH 2/5] usb: typec: ucsi: glink: move GPIO reading into
 connector_status callback
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Heikki Krogerus
	<heikki.krogerus@linux.intel.com>,
        Greg Kroah-Hartman
	<gregkh@linuxfoundation.org>
CC: Neil Armstrong <neil.armstrong@linaro.org>, <linux-usb@vger.kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20240408-ucsi-orient-aware-v1-0-95a74a163a10@linaro.org>
 <20240408-ucsi-orient-aware-v1-2-95a74a163a10@linaro.org>
Content-Language: en-US
In-Reply-To: <20240408-ucsi-orient-aware-v1-2-95a74a163a10@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 4/8/2024 10:00 AM, Dmitry Baryshkov wrote:
> To simplify the platform code move Type-C orientation handling into the
> connector_status callback. As it is called both during connector
> registration and on connector change events, duplicated code from
> pmic_glink_ucsi_register() can be dropped.
> 
> Also this moves operations that can sleep into a worker thread,
> removing the only sleeping operation from pmic_glink_ucsi_notify().
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Verified working of device mode (ADB) in SS in both orientations on 
QCS6490 RB3Gen2.

As an experiment, removed the connector status call being done in 
ucsi_register_port to see if the issue fixed by [1] would be 
reproducible or not and it is. So this patch is taking care of proper 
enumeration in bootup in host mode in reverse orientation as well.

Tested-by: Krishna Kurapati <quic_kriskura@quicinc.com>

[1]: 
https://lore.kernel.org/all/20240301040914.458492-1-quic_kriskura@quicinc.com/

> ---
>   drivers/usb/typec/ucsi/ucsi_glink.c | 48 ++++++++++++++++---------------------
>   1 file changed, 20 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index b91d2d15d7d9..d21f8cd2fe35 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -187,10 +187,28 @@ static int pmic_glink_ucsi_sync_write(struct ucsi *__ucsi, unsigned int offset,
>   	return ret;
>   }
>   
> +static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
> +{
> +	struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
> +	int orientation;
> +
> +	if (con->num >= PMIC_GLINK_MAX_PORTS ||
> +	    !ucsi->port_orientation[con->num - 1])
> +		return;
> +
> +	orientation = gpiod_get_value(ucsi->port_orientation[con->num - 1]);
> +	if (orientation >= 0) {
> +		typec_switch_set(ucsi->port_switch[con->num - 1],
> +				 orientation ? TYPEC_ORIENTATION_REVERSE
> +				 : TYPEC_ORIENTATION_NORMAL);
> +	}
> +}
> +
>   static const struct ucsi_operations pmic_glink_ucsi_ops = {
>   	.read = pmic_glink_ucsi_read,
>   	.sync_write = pmic_glink_ucsi_sync_write,
> -	.async_write = pmic_glink_ucsi_async_write
> +	.async_write = pmic_glink_ucsi_async_write,
> +	.connector_status = pmic_glink_ucsi_connector_status,
>   };
>   
>   static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, int len)
> @@ -229,20 +247,8 @@ static void pmic_glink_ucsi_notify(struct work_struct *work)
>   	}
>   
>   	con_num = UCSI_CCI_CONNECTOR(cci);
> -	if (con_num) {
> -		if (con_num <= PMIC_GLINK_MAX_PORTS &&
> -		    ucsi->port_orientation[con_num - 1]) {
> -			int orientation = gpiod_get_value(ucsi->port_orientation[con_num - 1]);
> -
> -			if (orientation >= 0) {
> -				typec_switch_set(ucsi->port_switch[con_num - 1],
> -						 orientation ? TYPEC_ORIENTATION_REVERSE
> -							     : TYPEC_ORIENTATION_NORMAL);
> -			}
> -		}
> -
> +	if (con_num)
>   		ucsi_connector_change(ucsi->ucsi, con_num);
> -	}
>   
>   	if (ucsi->sync_pending &&
>   		   (cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE))) {
> @@ -253,20 +259,6 @@ 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);
>   }
>