[PATCH 0/8] usb: typec: ucsi: glink: merge in altmode support

[PATCH 0/8] usb: typec: ucsi: glink: merge in altmode support

[Dmitry Baryshkov]

PMIC GLINK platforms handle the altmode support via OOB messages rather
than by fully following the UCSI standard. Currently altmode handling is
implemented in a separate driver, which has to duplicate significant
part of the USB-C stack to control USB-C muxes, switches and retimers.
Also this potentially introduces race conditions, since both UCSI and
pmic-glink-altmode will drive those components. Last but not least,
there is no connection betnween the altmode's aux-hpd-bridge and
corresponding typec_port instance.

Merge the pmic-glink-altmode driver into the ucsi-glink, streamling the
altmode support for Qualcomm platforms.

Depends: https://lore.kernel.org/linux-usb/20240411-ucsi-orient-aware-v2-0-d4b1cb22a33f@linaro.org/

Merge strategy: since the series involves both UCSI and soc/qcom
drivers, I'd kindly ask to ack merging the whole patchset through the
USB tree.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Dmitry Baryshkov (8):
      usb: typec: Handle retimers in typec_set_mode()
      usb: typec: altmode: add low level altmode configuration helper
      usb: typec: ucsi: glink: check message data sizes
      usb: typec: ucsi: glink: use le32 for message data
      usb: typec: ucsi: glink: simplify notification handling
      usb: typec: ucsi: add ucsi_registered() callback
      usb: typec: ucsi: glink: merge pmic_glink_altmode driver
      soc: qcom: pmic-glink: drop separate altmode driver support

 drivers/soc/qcom/Makefile             |   1 -
 drivers/soc/qcom/pmic_glink.c         |  15 +-
 drivers/soc/qcom/pmic_glink_altmode.c | 546 ------------------------------
 drivers/usb/typec/bus.c               |  34 ++
 drivers/usb/typec/class.c             |   9 +-
 drivers/usb/typec/ucsi/ucsi.c         |   3 +
 drivers/usb/typec/ucsi/ucsi.h         |   2 +
 drivers/usb/typec/ucsi/ucsi_glink.c   | 615 +++++++++++++++++++++++++++++++---
 include/linux/usb/typec_altmode.h     |   3 +
 9 files changed, 619 insertions(+), 609 deletions(-)
---
base-commit: 7f3fd687151a552038967f31993f1bc7e447b99e
change-id: 20240408-ucsi-glink-altmode-293cdbf3270c

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

[PATCH 1/8] usb: typec: Handle retimers in typec_set_mode()

[Dmitry Baryshkov]

Make typec_set_mode() also handle retimers in addition to muxes. Setting
the USB mode requires retimers to be configured in addition to just
switching the mux configuration.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/usb/typec/class.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 9610e647a8d4..28d395535bd1 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -2095,14 +2095,21 @@ EXPORT_SYMBOL_GPL(typec_get_orientation);
  * @mode: Accessory Mode, USB Operation or Safe State
  *
  * Configure @port for Accessory Mode @mode. This function will configure the
- * muxes needed for @mode.
+ * muxes and retimeres needed for @mode.
  */
 int typec_set_mode(struct typec_port *port, int mode)
 {
+	struct typec_retimer_state retimer_state = { };
 	struct typec_mux_state state = { };
+	int ret;
 
+	retimer_state.mode = mode;
 	state.mode = mode;
 
+	ret = typec_retimer_set(port->retimer, &retimer_state);
+	if (ret)
+		return ret;
+
 	return typec_mux_set(port->mux, &state);
 }
 EXPORT_SYMBOL_GPL(typec_set_mode);

-- 
2.39.2

Re: [PATCH 1/8] usb: typec: Handle retimers in typec_set_mode()

[Konrad Dybcio]

On 4/16/24 04:20, Dmitry Baryshkov wrote:
> Make typec_set_mode() also handle retimers in addition to muxes. Setting
> the USB mode requires retimers to be configured in addition to just
> switching the mux configuration.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH 1/8] usb: typec: Handle retimers in typec_set_mode()

[Neil Armstrong]

On 16/04/2024 04:20, Dmitry Baryshkov wrote:
> Make typec_set_mode() also handle retimers in addition to muxes. Setting
> the USB mode requires retimers to be configured in addition to just
> switching the mux configuration.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/usb/typec/class.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 9610e647a8d4..28d395535bd1 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -2095,14 +2095,21 @@ EXPORT_SYMBOL_GPL(typec_get_orientation);
>    * @mode: Accessory Mode, USB Operation or Safe State
>    *
>    * Configure @port for Accessory Mode @mode. This function will configure the
> - * muxes needed for @mode.
> + * muxes and retimeres needed for @mode.
>    */
>   int typec_set_mode(struct typec_port *port, int mode)
>   {
> +	struct typec_retimer_state retimer_state = { };
>   	struct typec_mux_state state = { };
> +	int ret;
>   
> +	retimer_state.mode = mode;
>   	state.mode = mode;
>   
> +	ret = typec_retimer_set(port->retimer, &retimer_state);
> +	if (ret)
> +		return ret;
> +
>   	return typec_mux_set(port->mux, &state);
>   }
>   EXPORT_SYMBOL_GPL(typec_set_mode);
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH 1/8] usb: typec: Handle retimers in typec_set_mode()

[Heikki Krogerus]

On Tue, Apr 16, 2024 at 05:20:50AM +0300, Dmitry Baryshkov wrote:
> Make typec_set_mode() also handle retimers in addition to muxes. Setting
> the USB mode requires retimers to be configured in addition to just
> switching the mux configuration.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

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

> ---
>  drivers/usb/typec/class.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 9610e647a8d4..28d395535bd1 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -2095,14 +2095,21 @@ EXPORT_SYMBOL_GPL(typec_get_orientation);
>   * @mode: Accessory Mode, USB Operation or Safe State
>   *
>   * Configure @port for Accessory Mode @mode. This function will configure the
> - * muxes needed for @mode.
> + * muxes and retimeres needed for @mode.
>   */
>  int typec_set_mode(struct typec_port *port, int mode)
>  {
> +	struct typec_retimer_state retimer_state = { };
>  	struct typec_mux_state state = { };
> +	int ret;
>  
> +	retimer_state.mode = mode;
>  	state.mode = mode;
>  
> +	ret = typec_retimer_set(port->retimer, &retimer_state);
> +	if (ret)
> +		return ret;
> +
>  	return typec_mux_set(port->mux, &state);
>  }
>  EXPORT_SYMBOL_GPL(typec_set_mode);
> 
> -- 
> 2.39.2

-- 
heikki

[PATCH 2/8] usb: typec: altmode: add low level altmode configuration helper

[Dmitry Baryshkov]

In some obscure cases (Qualcomm PMIC Glink) altmode is completely
handled by the firmware. Linux does not get proper partner altmode info.
Instead we get the notification once the altmode is negotiated and
entered (or left). However even in such a case the driver has to switch
board components (muxes, switches and retimers) according to the altmode
selected by the hardware.

We can not use existing typec_altmode_enter() / typec_altmode_exit() /
typec_altmode_notify() functions in such a case, since there is no
corresponding partner's altmode instance.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/usb/typec/bus.c           | 34 ++++++++++++++++++++++++++++++++++
 include/linux/usb/typec_altmode.h |  3 +++
 2 files changed, 37 insertions(+)

diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index 6ea103e1abae..68f3908401c6 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -67,6 +67,40 @@ static int typec_altmode_set_state(struct typec_altmode *adev,
 	return typec_altmode_set_switches(port_altmode, conf, data);
 }
 
+/**
+ * typec_altmode_set_port - set the altmode configuration
+ * @conf: Alternate mode specific configuration value
+ * @dVata: Alternate mode specific data
+ *
+ * This function allows configuring muxes and retimer for the selected altmode.
+ * This function may only be used by the special case drivers, that handle
+ * the altmode negotiation by the alternative means and thus have no
+ * corresponding typec_altmode instance for the parnter.
+ */
+int typec_altmode_set_port(struct typec_altmode *adev,
+			   unsigned long conf, void *data)
+{
+	bool is_port;
+	struct altmode *altmode;
+	int ret;
+
+	if (!adev)
+		return 0;
+
+	altmode = to_altmode(adev);
+	is_port = is_typec_port(adev->dev.parent);
+
+	if (altmode->partner || !is_port)
+		return -EINVAL;
+
+	ret = typec_altmode_set_switches(altmode, conf, data);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(typec_altmode_set_port);
+
 /* -------------------------------------------------------------------------- */
 /* Common API */
 
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index b3c0866ea70f..d78a9618bedf 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -77,6 +77,9 @@ int typec_altmode_notify(struct typec_altmode *altmode, unsigned long conf,
 const struct typec_altmode *
 typec_altmode_get_partner(struct typec_altmode *altmode);
 
+int typec_altmode_set_port(struct typec_altmode *altmode, unsigned long conf,
+			   void *data);
+
 /**
  * struct typec_cable_ops - Cable alternate mode operations vector
  * @enter: Operations to be executed with Enter Mode Command

-- 
2.39.2

Re: [PATCH 2/8] usb: typec: altmode: add low level altmode configuration helper

[Konrad Dybcio]

On 4/16/24 04:20, Dmitry Baryshkov wrote:
> In some obscure cases (Qualcomm PMIC Glink) altmode is completely
> handled by the firmware. Linux does not get proper partner altmode info.
> Instead we get the notification once the altmode is negotiated and
> entered (or left). However even in such a case the driver has to switch
> board components (muxes, switches and retimers) according to the altmode
> selected by the hardware.
> 
> We can not use existing typec_altmode_enter() / typec_altmode_exit() /
> typec_altmode_notify() functions in such a case, since there is no
> corresponding partner's altmode instance.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Should this now be called from e.g. typec_almode_notify to limit
duplication?

Konrad

Re: [PATCH 2/8] usb: typec: altmode: add low level altmode configuration helper

[Dmitry Baryshkov]

On Tue, 16 Apr 2024 at 17:32, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 4/16/24 04:20, Dmitry Baryshkov wrote:
> > In some obscure cases (Qualcomm PMIC Glink) altmode is completely
> > handled by the firmware. Linux does not get proper partner altmode info.
> > Instead we get the notification once the altmode is negotiated and
> > entered (or left). However even in such a case the driver has to switch
> > board components (muxes, switches and retimers) according to the altmode
> > selected by the hardware.
> >
> > We can not use existing typec_altmode_enter() / typec_altmode_exit() /
> > typec_altmode_notify() functions in such a case, since there is no
> > corresponding partner's altmode instance.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
>
> Should this now be called from e.g. typec_almode_notify to limit
> duplication?

typec_altmode_notify works only if there is an altmode->partner (which
we don't have with pmic-glink).


-- 
With best wishes
Dmitry

Re: [PATCH 2/8] usb: typec: altmode: add low level altmode configuration helper

[Konrad Dybcio]

On 4/16/24 16:48, Dmitry Baryshkov wrote:
> On Tue, 16 Apr 2024 at 17:32, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>>
>>
>> On 4/16/24 04:20, Dmitry Baryshkov wrote:
>>> In some obscure cases (Qualcomm PMIC Glink) altmode is completely
>>> handled by the firmware. Linux does not get proper partner altmode info.
>>> Instead we get the notification once the altmode is negotiated and
>>> entered (or left). However even in such a case the driver has to switch
>>> board components (muxes, switches and retimers) according to the altmode
>>> selected by the hardware.
>>>
>>> We can not use existing typec_altmode_enter() / typec_altmode_exit() /
>>> typec_altmode_notify() functions in such a case, since there is no
>>> corresponding partner's altmode instance.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>
>> Should this now be called from e.g. typec_almode_notify to limit
>> duplication?
> 
> typec_altmode_notify works only if there is an altmode->partner (which
> we don't have with pmic-glink).

Yes and this seems to be an excerpt from these functions, should they
now drop that code and call this function instead, so as not to have
it in the tree twice?

Konrad

Re: [PATCH 2/8] usb: typec: altmode: add low level altmode configuration helper

[Dmitry Baryshkov]

On Tue, 16 Apr 2024 at 17:57, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 4/16/24 16:48, Dmitry Baryshkov wrote:
> > On Tue, 16 Apr 2024 at 17:32, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>
> >>
> >>
> >> On 4/16/24 04:20, Dmitry Baryshkov wrote:
> >>> In some obscure cases (Qualcomm PMIC Glink) altmode is completely
> >>> handled by the firmware. Linux does not get proper partner altmode info.
> >>> Instead we get the notification once the altmode is negotiated and
> >>> entered (or left). However even in such a case the driver has to switch
> >>> board components (muxes, switches and retimers) according to the altmode
> >>> selected by the hardware.
> >>>
> >>> We can not use existing typec_altmode_enter() / typec_altmode_exit() /
> >>> typec_altmode_notify() functions in such a case, since there is no
> >>> corresponding partner's altmode instance.
> >>>
> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> ---
> >>
> >> Should this now be called from e.g. typec_almode_notify to limit
> >> duplication?
> >
> > typec_altmode_notify works only if there is an altmode->partner (which
> > we don't have with pmic-glink).
>
> Yes and this seems to be an excerpt from these functions, should they
> now drop that code and call this function instead, so as not to have
> it in the tree twice?

I thought about it, but then I abandoned this idea. The
typec_altmode_notify() has its own use case, normal altmode drivers.
It is an error to call it if there is no partner registered, etc. So I
wanted to preserve error checks in that function instead of dropping
them. The significant part of the code is shared anyway thanks to
typec_altmode_set_switches().

-- 
With best wishes
Dmitry

[PATCH 3/8] usb: typec: ucsi: glink: check message data sizes

[Dmitry Baryshkov]

The driver gets data from the DSP firmware. Sanitize data size before
reading corresponding message structures.

Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/usb/typec/ucsi/ucsi_glink.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index f7546bb488c3..6be9d89d4a28 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -223,9 +223,16 @@ static const struct ucsi_operations pmic_glink_ucsi_ops = {
 	.connector_status = pmic_glink_ucsi_connector_status,
 };
 
-static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, int len)
+static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, size_t len)
 {
-	const struct ucsi_read_buf_resp_msg *resp = data;
+	const struct ucsi_read_buf_resp_msg *resp;
+
+	if (len != sizeof (*resp)) {
+		dev_err_ratelimited(ucsi->dev, "Unexpected read response struct size %zd\n", len);
+		return;
+	}
+
+	resp = data;
 
 	if (resp->ret_code)
 		return;
@@ -234,9 +241,16 @@ static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *d
 	complete(&ucsi->read_ack);
 }
 
-static void pmic_glink_ucsi_write_ack(struct pmic_glink_ucsi *ucsi, const void *data, int len)
+static void pmic_glink_ucsi_write_ack(struct pmic_glink_ucsi *ucsi, const void *data, size_t len)
 {
-	const struct ucsi_write_buf_resp_msg *resp = data;
+	const struct ucsi_write_buf_resp_msg *resp;
+
+	if (len != sizeof (*resp)) {
+		dev_err_ratelimited(ucsi->dev, "Unexpected write ack struct size %zd\n", len);
+		return;
+	}
+
+	resp = data;
 
 	if (resp->ret_code)
 		return;

-- 
2.39.2

Re: [PATCH 3/8] usb: typec: ucsi: glink: check message data sizes

[Konrad Dybcio]

On 4/16/24 04:20, Dmitry Baryshkov wrote:
> The driver gets data from the DSP firmware. Sanitize data size before
> reading corresponding message structures.
> 
> Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

I think more backstory would be beneficial here.. Does this happen often?
What are the consequences? What are the causes? Can there be one-off invalid
messages, or does that mean the firwmare has entered some unstable state?

And I suppose, if answer to the last question is "unstable state", are we
doing something incorrectly in Linux that causes it to happen?

Konrad

Re: [PATCH 3/8] usb: typec: ucsi: glink: check message data sizes

[Dmitry Baryshkov]

On Tue, 16 Apr 2024 at 17:33, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 4/16/24 04:20, Dmitry Baryshkov wrote:
> > The driver gets data from the DSP firmware. Sanitize data size before
> > reading corresponding message structures.
> >
> > Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver")
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
>
> I think more backstory would be beneficial here.. Does this happen often?
> What are the consequences? What are the causes? Can there be one-off invalid
> messages, or does that mean the firwmare has entered some unstable state?
>
> And I suppose, if answer to the last question is "unstable state", are we
> doing something incorrectly in Linux that causes it to happen?

No. I haven't seen such cases. However as we are getting the data from
external entity which don't control, we should check that the messages
conform to what we are expecting.



-- 
With best wishes
Dmitry

[PATCH 4/8] usb: typec: ucsi: glink: use le32 for message data

[Dmitry Baryshkov]

The message structures as transferred by the PMIC_GLINK use le32 for
data encoding. Correct struct accessors to follow the lead of the main
pmic_glink.c driver.

Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/usb/typec/ucsi/ucsi_glink.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index 6be9d89d4a28..d029cc9d82e3 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -33,7 +33,7 @@ struct ucsi_read_buf_req_msg {
 struct ucsi_read_buf_resp_msg {
 	struct pmic_glink_hdr   hdr;
 	u8                      buf[UCSI_BUF_SIZE];
-	u32                     ret_code;
+	__le32                  ret_code;
 };
 
 struct ucsi_write_buf_req_msg {
@@ -44,13 +44,13 @@ struct ucsi_write_buf_req_msg {
 
 struct ucsi_write_buf_resp_msg {
 	struct pmic_glink_hdr   hdr;
-	u32                     ret_code;
+	__le32                  ret_code;
 };
 
 struct ucsi_notify_ind_msg {
 	struct pmic_glink_hdr   hdr;
-	u32                     notification;
-	u32                     receiver;
+	__le32                  notification;
+	__le32                  receiver;
 	u32                     reserved;
 };
 
@@ -255,7 +255,7 @@ static void pmic_glink_ucsi_write_ack(struct pmic_glink_ucsi *ucsi, const void *
 	if (resp->ret_code)
 		return;
 
-	ucsi->sync_val = resp->ret_code;
+	ucsi->sync_val = le32_to_cpu(resp->ret_code);
 	complete(&ucsi->write_ack);
 }
 

-- 
2.39.2

Re: [PATCH 4/8] usb: typec: ucsi: glink: use le32 for message data

[Konrad Dybcio]

On 4/16/24 04:20, Dmitry Baryshkov wrote:
> The message structures as transferred by the PMIC_GLINK use le32 for
> data encoding. Correct struct accessors to follow the lead of the main
> pmic_glink.c driver.
> 
> Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

There are a couple of if (resp->ret_code) occurences, but zero is zero
no matter the bit order, so that works out

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH 4/8] usb: typec: ucsi: glink: use le32 for message data

[Neil Armstrong]

On 16/04/2024 04:20, Dmitry Baryshkov wrote:
> The message structures as transferred by the PMIC_GLINK use le32 for
> data encoding. Correct struct accessors to follow the lead of the main
> pmic_glink.c driver.
> 
> Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/usb/typec/ucsi/ucsi_glink.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index 6be9d89d4a28..d029cc9d82e3 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -33,7 +33,7 @@ struct ucsi_read_buf_req_msg {
>   struct ucsi_read_buf_resp_msg {
>   	struct pmic_glink_hdr   hdr;
>   	u8                      buf[UCSI_BUF_SIZE];
> -	u32                     ret_code;
> +	__le32                  ret_code;
>   };
>   
>   struct ucsi_write_buf_req_msg {
> @@ -44,13 +44,13 @@ struct ucsi_write_buf_req_msg {
>   
>   struct ucsi_write_buf_resp_msg {
>   	struct pmic_glink_hdr   hdr;
> -	u32                     ret_code;
> +	__le32                  ret_code;
>   };
>   
>   struct ucsi_notify_ind_msg {
>   	struct pmic_glink_hdr   hdr;
> -	u32                     notification;
> -	u32                     receiver;
> +	__le32                  notification;
> +	__le32                  receiver;
>   	u32                     reserved;
>   };
>   
> @@ -255,7 +255,7 @@ static void pmic_glink_ucsi_write_ack(struct pmic_glink_ucsi *ucsi, const void *
>   	if (resp->ret_code)
>   		return;
>   
> -	ucsi->sync_val = resp->ret_code;
> +	ucsi->sync_val = le32_to_cpu(resp->ret_code);
>   	complete(&ucsi->write_ack);
>   }
>   
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH 4/8] usb: typec: ucsi: glink: use le32 for message data

[Heikki Krogerus]

On Tue, Apr 16, 2024 at 05:20:53AM +0300, Dmitry Baryshkov wrote:
> The message structures as transferred by the PMIC_GLINK use le32 for
> data encoding. Correct struct accessors to follow the lead of the main
> pmic_glink.c driver.
> 
> Fixes: 62b5412b1f4a ("usb: typec: ucsi: add PMIC Glink UCSI driver")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

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

> ---
>  drivers/usb/typec/ucsi/ucsi_glink.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index 6be9d89d4a28..d029cc9d82e3 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -33,7 +33,7 @@ struct ucsi_read_buf_req_msg {
>  struct ucsi_read_buf_resp_msg {
>  	struct pmic_glink_hdr   hdr;
>  	u8                      buf[UCSI_BUF_SIZE];
> -	u32                     ret_code;
> +	__le32                  ret_code;
>  };
>  
>  struct ucsi_write_buf_req_msg {
> @@ -44,13 +44,13 @@ struct ucsi_write_buf_req_msg {
>  
>  struct ucsi_write_buf_resp_msg {
>  	struct pmic_glink_hdr   hdr;
> -	u32                     ret_code;
> +	__le32                  ret_code;
>  };
>  
>  struct ucsi_notify_ind_msg {
>  	struct pmic_glink_hdr   hdr;
> -	u32                     notification;
> -	u32                     receiver;
> +	__le32                  notification;
> +	__le32                  receiver;
>  	u32                     reserved;
>  };
>  
> @@ -255,7 +255,7 @@ static void pmic_glink_ucsi_write_ack(struct pmic_glink_ucsi *ucsi, const void *
>  	if (resp->ret_code)
>  		return;
>  
> -	ucsi->sync_val = resp->ret_code;
> +	ucsi->sync_val = le32_to_cpu(resp->ret_code);
>  	complete(&ucsi->write_ack);
>  }
>  
> 
> -- 
> 2.39.2

-- 
heikki

[PATCH 5/8] usb: typec: ucsi: glink: simplify notification handling

[Dmitry Baryshkov]

All platforms except Qualcomm SC8180X pass CCI in the notification
message. Use it instead of going back and forth over RPMSG
interface to read CCI.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/usb/typec/ucsi/ucsi_glink.c | 90 +++++++++++++++++++++++++++++--------
 1 file changed, 71 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index d029cc9d82e3..40fcda055b05 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -6,6 +6,7 @@
 #include <linux/auxiliary_bus.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/property.h>
 #include <linux/soc/qcom/pdr.h>
@@ -70,7 +71,7 @@ struct pmic_glink_ucsi {
 
 	int sync_val;
 
-	struct work_struct notify_work;
+	struct work_struct notify_work_sc8180x;
 	struct work_struct register_work;
 
 	u8 read_buf[UCSI_BUF_SIZE];
@@ -223,6 +224,20 @@ static const struct ucsi_operations pmic_glink_ucsi_ops = {
 	.connector_status = pmic_glink_ucsi_connector_status,
 };
 
+static void pmic_glink_ucsi_notify_handle(struct pmic_glink_ucsi *ucsi, u32 cci)
+{
+	unsigned int con_num;
+
+	con_num = UCSI_CCI_CONNECTOR(cci);
+	if (con_num)
+		ucsi_connector_change(ucsi->ucsi, con_num);
+
+	if (ucsi->sync_pending &&
+		   (cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE))) {
+		complete(&ucsi->sync_ack);
+	}
+}
+
 static void pmic_glink_ucsi_read_ack(struct pmic_glink_ucsi *ucsi, const void *data, size_t len)
 {
 	const struct ucsi_read_buf_resp_msg *resp;
@@ -259,10 +274,24 @@ static void pmic_glink_ucsi_write_ack(struct pmic_glink_ucsi *ucsi, const void *
 	complete(&ucsi->write_ack);
 }
 
-static void pmic_glink_ucsi_notify(struct work_struct *work)
+/* used everywhere except sc8180x */
+static void pmic_glink_ucsi_notify_ind(struct pmic_glink_ucsi *ucsi, const void *data, size_t len)
 {
-	struct pmic_glink_ucsi *ucsi = container_of(work, struct pmic_glink_ucsi, notify_work);
-	unsigned int con_num;
+	const struct ucsi_notify_ind_msg *msg;
+
+	if (len != sizeof (*msg)) {
+		dev_err_ratelimited(ucsi->dev, "Unexpected notification struct size %zd\n", len);
+		return;
+	}
+
+	msg = data;
+
+	pmic_glink_ucsi_notify_handle(ucsi, le32_to_cpu(msg->notification));
+}
+
+static void pmic_glink_ucsi_notify_work_sc8180x(struct work_struct *work)
+{
+	struct pmic_glink_ucsi *ucsi = container_of(work, struct pmic_glink_ucsi, notify_work_sc8180x);
 	u32 cci;
 	int ret;
 
@@ -272,14 +301,7 @@ static void pmic_glink_ucsi_notify(struct work_struct *work)
 		return;
 	}
 
-	con_num = UCSI_CCI_CONNECTOR(cci);
-	if (con_num)
-		ucsi_connector_change(ucsi->ucsi, con_num);
-
-	if (ucsi->sync_pending &&
-		   (cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE))) {
-		complete(&ucsi->sync_ack);
-	}
+	pmic_glink_ucsi_notify_handle(ucsi, cci);
 }
 
 static void pmic_glink_ucsi_register(struct work_struct *work)
@@ -289,6 +311,24 @@ static void pmic_glink_ucsi_register(struct work_struct *work)
 	ucsi_register(ucsi->ucsi);
 }
 
+static void pmic_glink_ucsi_callback_sc8180x(const void *data, size_t len, void *priv)
+{
+	struct pmic_glink_ucsi *ucsi = priv;
+	const struct pmic_glink_hdr *hdr = data;
+
+	switch (le32_to_cpu(hdr->opcode)) {
+	case UC_UCSI_READ_BUF_REQ:
+		pmic_glink_ucsi_read_ack(ucsi, data, len);
+		break;
+	case UC_UCSI_WRITE_BUF_REQ:
+		pmic_glink_ucsi_write_ack(ucsi, data, len);
+		break;
+	case UC_UCSI_USBC_NOTIFY_IND:
+		schedule_work(&ucsi->notify_work_sc8180x);
+		break;
+	};
+}
+
 static void pmic_glink_ucsi_callback(const void *data, size_t len, void *priv)
 {
 	struct pmic_glink_ucsi *ucsi = priv;
@@ -302,7 +342,7 @@ static void pmic_glink_ucsi_callback(const void *data, size_t len, void *priv)
 		pmic_glink_ucsi_write_ack(ucsi, data, len);
 		break;
 	case UC_UCSI_USBC_NOTIFY_IND:
-		schedule_work(&ucsi->notify_work);
+		pmic_glink_ucsi_notify_ind(ucsi, data, len);
 		break;
 	};
 }
@@ -348,6 +388,7 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 	struct device *dev = &adev->dev;
 	const struct of_device_id *match;
 	struct fwnode_handle *fwnode;
+	bool sc8180x_glink;
 	int ret;
 
 	ucsi = devm_kzalloc(dev, sizeof(*ucsi), GFP_KERNEL);
@@ -357,7 +398,7 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 	ucsi->dev = dev;
 	dev_set_drvdata(dev, ucsi);
 
-	INIT_WORK(&ucsi->notify_work, pmic_glink_ucsi_notify);
+	INIT_WORK(&ucsi->notify_work_sc8180x, pmic_glink_ucsi_notify_work_sc8180x);
 	INIT_WORK(&ucsi->register_work, pmic_glink_ucsi_register);
 	init_completion(&ucsi->read_ack);
 	init_completion(&ucsi->write_ack);
@@ -406,11 +447,22 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 		ucsi->port_orientation[port] = desc;
 	}
 
-	ucsi->client = devm_pmic_glink_register_client(dev,
-						       PMIC_GLINK_OWNER_USBC,
-						       pmic_glink_ucsi_callback,
-						       pmic_glink_ucsi_pdr_notify,
-						       ucsi);
+	sc8180x_glink = of_device_is_compatible(dev->parent->of_node,
+						"qcom,sc8180x-pmic-glink");
+
+	if (sc8180x_glink) {
+		ucsi->client = devm_pmic_glink_register_client(dev,
+							       PMIC_GLINK_OWNER_USBC,
+							       pmic_glink_ucsi_callback_sc8180x,
+							       pmic_glink_ucsi_pdr_notify,
+							       ucsi);
+	} else {
+		ucsi->client = devm_pmic_glink_register_client(dev,
+							       PMIC_GLINK_OWNER_USBC,
+							       pmic_glink_ucsi_callback,
+							       pmic_glink_ucsi_pdr_notify,
+							       ucsi);
+	}
 	return PTR_ERR_OR_ZERO(ucsi->client);
 }
 

-- 
2.39.2

Re: [PATCH 5/8] usb: typec: ucsi: glink: simplify notification handling

[Konrad Dybcio]

On 4/16/24 04:20, Dmitry Baryshkov wrote:
> All platforms except Qualcomm SC8180X pass CCI in the notification
> message. Use it instead of going back and forth over RPMSG
> interface to read CCI.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Are we sure it's reeeeallly just 8180?

Konrad

Re: [PATCH 5/8] usb: typec: ucsi: glink: simplify notification handling

[Dmitry Baryshkov]

On Tue, 16 Apr 2024 at 17:36, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>
>
> On 4/16/24 04:20, Dmitry Baryshkov wrote:
> > All platforms except Qualcomm SC8180X pass CCI in the notification
> > message. Use it instead of going back and forth over RPMSG
> > interface to read CCI.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
>
> Are we sure it's reeeeallly just 8180?

More or less so. Previous platforms used either qcom-pmic-typec
(sm8150, sm8250) or laptop-specific EC (sdm850 / c630, sc7180 /
aspire1, CrOS). Next platforms (sc8280xp and sm8350, qcm6490) used
pmic-glink and the new way of sending events. I think sc8180x was
really the unfortunate one to use mixed events.

-- 
With best wishes
Dmitry

[PATCH 6/8] usb: typec: ucsi: add ucsi_registered() callback

[Dmitry Baryshkov]

As the registration of the UCSI device is performed from the scheduled
worker, the glue driver isn't notified when the UCSI registration
succeeds. The ucsi_glink driver needs this event to be able to manually
register DisplayPort altmodes.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/usb/typec/ucsi/ucsi.c | 3 +++
 drivers/usb/typec/ucsi/ucsi.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index cb52e7b0a2c5..ae89c4c8341d 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1755,6 +1755,9 @@ static int ucsi_init(struct ucsi *ucsi)
 	if (UCSI_CCI_CONNECTOR(cci))
 		ucsi_connector_change(ucsi, UCSI_CCI_CONNECTOR(cci));
 
+	if (ucsi->ops->ucsi_registered)
+		ucsi->ops->ucsi_registered(ucsi);
+
 	return 0;
 
 err_unregister:
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index c4d103db9d0f..37ee1b1d8c31 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -62,6 +62,7 @@ struct dentry;
  * @update_altmodes: Squashes duplicate DP altmodes
  * @update_connector: Update connector capabilities before registering
  * @connector_status: Updates connector status, called holding connector lock
+ * @ucsi_registered: notify host driver when the UCSI interface is registered
  *
  * 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
@@ -78,6 +79,7 @@ struct ucsi_operations {
 				struct ucsi_altmode *updated);
 	void (*update_connector)(struct ucsi_connector *con);
 	void (*connector_status)(struct ucsi_connector *con);
+	void (*ucsi_registered)(struct ucsi *ucsi);
 };
 
 struct ucsi *ucsi_create(struct device *dev, const struct ucsi_operations *ops);

-- 
2.39.2

[PATCH 7/8] usb: typec: ucsi: glink: merge pmic_glink_altmode driver

[Dmitry Baryshkov]

Move handling of USB Altmode to the ucsi_glink driver. This way the
altmode is properly registered in the Type-C framework, the altmode
handlers can use generic typec calls, the UCSI driver can use
orientation information from altmode messages and vice versa, the
altmode handlers can use GPIO-based orientation inormation from UCSI
GLINK driver.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/soc/qcom/Makefile             |   1 -
 drivers/soc/qcom/pmic_glink_altmode.c | 546 ----------------------------------
 drivers/usb/typec/ucsi/ucsi_glink.c   | 495 ++++++++++++++++++++++++++++--
 3 files changed, 475 insertions(+), 567 deletions(-)

diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index ca0bece0dfff..d43d2b444634 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -9,7 +9,6 @@ obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
 obj-$(CONFIG_QCOM_OCMEM)	+= ocmem.o
 obj-$(CONFIG_QCOM_PDR_HELPERS)	+= pdr_interface.o
 obj-$(CONFIG_QCOM_PMIC_GLINK)	+= pmic_glink.o
-obj-$(CONFIG_QCOM_PMIC_GLINK)	+= pmic_glink_altmode.o
 obj-$(CONFIG_QCOM_PMIC_PDCHARGER_ULOG)	+= pmic_pdcharger_ulog.o
 CFLAGS_pmic_pdcharger_ulog.o	:=  -I$(src)
 obj-$(CONFIG_QCOM_QMI_HELPERS)	+= qmi_helpers.o
diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
deleted file mode 100644
index b3808fc24c69..000000000000
--- a/drivers/soc/qcom/pmic_glink_altmode.c
+++ /dev/null
@@ -1,546 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved.
- * Copyright (c) 2022, Linaro Ltd
- */
-#include <linux/auxiliary_bus.h>
-#include <linux/bitfield.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
-#include <linux/mutex.h>
-#include <linux/property.h>
-#include <linux/soc/qcom/pdr.h>
-#include <drm/bridge/aux-bridge.h>
-
-#include <linux/usb/typec_altmode.h>
-#include <linux/usb/typec_dp.h>
-#include <linux/usb/typec_mux.h>
-#include <linux/usb/typec_retimer.h>
-
-#include <linux/soc/qcom/pmic_glink.h>
-
-#define PMIC_GLINK_MAX_PORTS	2
-
-#define USBC_SC8180X_NOTIFY_IND	0x13
-#define USBC_CMD_WRITE_REQ      0x15
-#define USBC_NOTIFY_IND		0x16
-
-#define ALTMODE_PAN_EN		0x10
-#define ALTMODE_PAN_ACK		0x11
-
-struct usbc_write_req {
-	struct pmic_glink_hdr   hdr;
-	__le32 cmd;
-	__le32 arg;
-	__le32 reserved;
-};
-
-#define NOTIFY_PAYLOAD_SIZE 16
-struct usbc_notify {
-	struct pmic_glink_hdr hdr;
-	char payload[NOTIFY_PAYLOAD_SIZE];
-	u32 reserved;
-};
-
-struct usbc_sc8180x_notify {
-	struct pmic_glink_hdr hdr;
-	__le32 notification;
-	__le32 reserved[2];
-};
-
-enum pmic_glink_altmode_pin_assignment {
-	DPAM_HPD_OUT,
-	DPAM_HPD_A,
-	DPAM_HPD_B,
-	DPAM_HPD_C,
-	DPAM_HPD_D,
-	DPAM_HPD_E,
-	DPAM_HPD_F,
-};
-
-struct pmic_glink_altmode;
-
-#define work_to_altmode_port(w) container_of((w), struct pmic_glink_altmode_port, work)
-
-struct pmic_glink_altmode_port {
-	struct pmic_glink_altmode *altmode;
-	unsigned int index;
-
-	struct typec_switch *typec_switch;
-	struct typec_mux *typec_mux;
-	struct typec_mux_state state;
-	struct typec_retimer *typec_retimer;
-	struct typec_retimer_state retimer_state;
-	struct typec_altmode dp_alt;
-
-	struct work_struct work;
-
-	struct auxiliary_device *bridge;
-
-	enum typec_orientation orientation;
-	u16 svid;
-	u8 dp_data;
-	u8 mode;
-	u8 hpd_state;
-	u8 hpd_irq;
-};
-
-#define work_to_altmode(w) container_of((w), struct pmic_glink_altmode, enable_work)
-
-struct pmic_glink_altmode {
-	struct device *dev;
-
-	unsigned int owner_id;
-
-	/* To synchronize WRITE_REQ acks */
-	struct mutex lock;
-
-	struct completion pan_ack;
-	struct pmic_glink_client *client;
-
-	struct work_struct enable_work;
-
-	struct pmic_glink_altmode_port ports[PMIC_GLINK_MAX_PORTS];
-};
-
-static int pmic_glink_altmode_request(struct pmic_glink_altmode *altmode, u32 cmd, u32 arg)
-{
-	struct usbc_write_req req = {};
-	unsigned long left;
-	int ret;
-
-	/*
-	 * The USBC_CMD_WRITE_REQ ack doesn't identify the request, so wait for
-	 * one ack at a time.
-	 */
-	mutex_lock(&altmode->lock);
-
-	req.hdr.owner = cpu_to_le32(altmode->owner_id);
-	req.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP);
-	req.hdr.opcode = cpu_to_le32(USBC_CMD_WRITE_REQ);
-	req.cmd = cpu_to_le32(cmd);
-	req.arg = cpu_to_le32(arg);
-
-	ret = pmic_glink_send(altmode->client, &req, sizeof(req));
-	if (ret) {
-		dev_err(altmode->dev, "failed to send altmode request: %#x (%d)\n", cmd, ret);
-		goto out_unlock;
-	}
-
-	left = wait_for_completion_timeout(&altmode->pan_ack, 5 * HZ);
-	if (!left) {
-		dev_err(altmode->dev, "timeout waiting for altmode request ack for: %#x\n", cmd);
-		ret = -ETIMEDOUT;
-	}
-
-out_unlock:
-	mutex_unlock(&altmode->lock);
-	return ret;
-}
-
-static void pmic_glink_altmode_enable_dp(struct pmic_glink_altmode *altmode,
-					 struct pmic_glink_altmode_port *port,
-					 u8 mode, bool hpd_state,
-					 bool hpd_irq)
-{
-	struct typec_displayport_data dp_data = {};
-	int ret;
-
-	dp_data.status = DP_STATUS_ENABLED;
-	if (hpd_state)
-		dp_data.status |= DP_STATUS_HPD_STATE;
-	if (hpd_irq)
-		dp_data.status |= DP_STATUS_IRQ_HPD;
-	dp_data.conf = DP_CONF_SET_PIN_ASSIGN(mode);
-
-	port->state.alt = &port->dp_alt;
-	port->state.data = &dp_data;
-	port->state.mode = TYPEC_MODAL_STATE(mode);
-
-	ret = typec_mux_set(port->typec_mux, &port->state);
-	if (ret)
-		dev_err(altmode->dev, "failed to switch mux to DP: %d\n", ret);
-
-	port->retimer_state.alt = &port->dp_alt;
-	port->retimer_state.data = &dp_data;
-	port->retimer_state.mode = TYPEC_MODAL_STATE(mode);
-
-	ret = typec_retimer_set(port->typec_retimer, &port->retimer_state);
-	if (ret)
-		dev_err(altmode->dev, "failed to setup retimer to DP: %d\n", ret);
-}
-
-static void pmic_glink_altmode_enable_usb(struct pmic_glink_altmode *altmode,
-					  struct pmic_glink_altmode_port *port)
-{
-	int ret;
-
-	port->state.alt = NULL;
-	port->state.data = NULL;
-	port->state.mode = TYPEC_STATE_USB;
-
-	ret = typec_mux_set(port->typec_mux, &port->state);
-	if (ret)
-		dev_err(altmode->dev, "failed to switch mux to USB: %d\n", ret);
-
-	port->retimer_state.alt = NULL;
-	port->retimer_state.data = NULL;
-	port->retimer_state.mode = TYPEC_STATE_USB;
-
-	ret = typec_retimer_set(port->typec_retimer, &port->retimer_state);
-	if (ret)
-		dev_err(altmode->dev, "failed to setup retimer to USB: %d\n", ret);
-}
-
-static void pmic_glink_altmode_safe(struct pmic_glink_altmode *altmode,
-				    struct pmic_glink_altmode_port *port)
-{
-	int ret;
-
-	port->state.alt = NULL;
-	port->state.data = NULL;
-	port->state.mode = TYPEC_STATE_SAFE;
-
-	ret = typec_mux_set(port->typec_mux, &port->state);
-	if (ret)
-		dev_err(altmode->dev, "failed to switch mux to safe mode: %d\n", ret);
-
-	port->retimer_state.alt = NULL;
-	port->retimer_state.data = NULL;
-	port->retimer_state.mode = TYPEC_STATE_SAFE;
-
-	ret = typec_retimer_set(port->typec_retimer, &port->retimer_state);
-	if (ret)
-		dev_err(altmode->dev, "failed to setup retimer to USB: %d\n", ret);
-}
-
-static void pmic_glink_altmode_worker(struct work_struct *work)
-{
-	struct pmic_glink_altmode_port *alt_port = work_to_altmode_port(work);
-	struct pmic_glink_altmode *altmode = alt_port->altmode;
-
-	typec_switch_set(alt_port->typec_switch, alt_port->orientation);
-
-	if (alt_port->svid == USB_TYPEC_DP_SID && alt_port->mode == 0xff)
-		pmic_glink_altmode_safe(altmode, alt_port);
-	else if (alt_port->svid == USB_TYPEC_DP_SID)
-		pmic_glink_altmode_enable_dp(altmode, alt_port, alt_port->mode,
-					     alt_port->hpd_state, alt_port->hpd_irq);
-	else
-		pmic_glink_altmode_enable_usb(altmode, alt_port);
-
-	drm_aux_hpd_bridge_notify(&alt_port->bridge->dev,
-				  alt_port->hpd_state ?
-				  connector_status_connected :
-				  connector_status_disconnected);
-
-	pmic_glink_altmode_request(altmode, ALTMODE_PAN_ACK, alt_port->index);
-}
-
-static enum typec_orientation pmic_glink_altmode_orientation(unsigned int orientation)
-{
-	if (orientation == 0)
-		return TYPEC_ORIENTATION_NORMAL;
-	else if (orientation == 1)
-		return TYPEC_ORIENTATION_REVERSE;
-	else
-		return TYPEC_ORIENTATION_NONE;
-}
-
-#define SC8180X_PORT_MASK		0x000000ff
-#define SC8180X_ORIENTATION_MASK	0x0000ff00
-#define SC8180X_MUX_MASK		0x00ff0000
-#define SC8180X_MODE_MASK		0x3f000000
-#define SC8180X_HPD_STATE_MASK		0x40000000
-#define SC8180X_HPD_IRQ_MASK		0x80000000
-
-static void pmic_glink_altmode_sc8180xp_notify(struct pmic_glink_altmode *altmode,
-					       const void *data, size_t len)
-{
-	struct pmic_glink_altmode_port *alt_port;
-	const struct usbc_sc8180x_notify *msg;
-	u32 notification;
-	u8 orientation;
-	u8 hpd_state;
-	u8 hpd_irq;
-	u16 svid;
-	u8 port;
-	u8 mode;
-	u8 mux;
-
-	if (len != sizeof(*msg)) {
-		dev_warn(altmode->dev, "invalid length of USBC_NOTIFY indication: %zd\n", len);
-		return;
-	}
-
-	msg = data;
-	notification = le32_to_cpu(msg->notification);
-	port = FIELD_GET(SC8180X_PORT_MASK, notification);
-	orientation = FIELD_GET(SC8180X_ORIENTATION_MASK, notification);
-	mux = FIELD_GET(SC8180X_MUX_MASK, notification);
-	mode = FIELD_GET(SC8180X_MODE_MASK, notification);
-	hpd_state = FIELD_GET(SC8180X_HPD_STATE_MASK, notification);
-	hpd_irq = FIELD_GET(SC8180X_HPD_IRQ_MASK, notification);
-
-	svid = mux == 2 ? USB_TYPEC_DP_SID : 0;
-
-	if (port >= ARRAY_SIZE(altmode->ports) || !altmode->ports[port].altmode) {
-		dev_dbg(altmode->dev, "notification on undefined port %d\n", port);
-		return;
-	}
-
-	alt_port = &altmode->ports[port];
-	alt_port->orientation = pmic_glink_altmode_orientation(orientation);
-	alt_port->svid = svid;
-	alt_port->mode = mode;
-	alt_port->hpd_state = hpd_state;
-	alt_port->hpd_irq = hpd_irq;
-	schedule_work(&alt_port->work);
-}
-
-#define SC8280XP_DPAM_MASK	0x3f
-#define SC8280XP_HPD_STATE_MASK BIT(6)
-#define SC8280XP_HPD_IRQ_MASK	BIT(7)
-
-static void pmic_glink_altmode_sc8280xp_notify(struct pmic_glink_altmode *altmode,
-					       u16 svid, const void *data, size_t len)
-{
-	struct pmic_glink_altmode_port *alt_port;
-	const struct usbc_notify *notify;
-	u8 orientation;
-	u8 hpd_state;
-	u8 hpd_irq;
-	u8 mode;
-	u8 port;
-
-	if (len != sizeof(*notify)) {
-		dev_warn(altmode->dev, "invalid length USBC_NOTIFY_IND: %zd\n",
-			 len);
-		return;
-	}
-
-	notify = data;
-
-	port = notify->payload[0];
-	orientation = notify->payload[1];
-	mode = FIELD_GET(SC8280XP_DPAM_MASK, notify->payload[8]) - DPAM_HPD_A;
-	hpd_state = FIELD_GET(SC8280XP_HPD_STATE_MASK, notify->payload[8]);
-	hpd_irq = FIELD_GET(SC8280XP_HPD_IRQ_MASK, notify->payload[8]);
-
-	if (port >= ARRAY_SIZE(altmode->ports) || !altmode->ports[port].altmode) {
-		dev_dbg(altmode->dev, "notification on undefined port %d\n", port);
-		return;
-	}
-
-	alt_port = &altmode->ports[port];
-	alt_port->orientation = pmic_glink_altmode_orientation(orientation);
-	alt_port->svid = svid;
-	alt_port->mode = mode;
-	alt_port->hpd_state = hpd_state;
-	alt_port->hpd_irq = hpd_irq;
-	schedule_work(&alt_port->work);
-}
-
-static void pmic_glink_altmode_callback(const void *data, size_t len, void *priv)
-{
-	struct pmic_glink_altmode *altmode = priv;
-	const struct pmic_glink_hdr *hdr = data;
-	u16 opcode;
-	u16 svid;
-
-	opcode = le32_to_cpu(hdr->opcode) & 0xff;
-	svid = le32_to_cpu(hdr->opcode) >> 16;
-
-	switch (opcode) {
-	case USBC_CMD_WRITE_REQ:
-		complete(&altmode->pan_ack);
-		break;
-	case USBC_NOTIFY_IND:
-		pmic_glink_altmode_sc8280xp_notify(altmode, svid, data, len);
-		break;
-	case USBC_SC8180X_NOTIFY_IND:
-		pmic_glink_altmode_sc8180xp_notify(altmode, data, len);
-		break;
-	}
-}
-
-static void pmic_glink_altmode_put_retimer(void *data)
-{
-	typec_retimer_put(data);
-}
-
-static void pmic_glink_altmode_put_mux(void *data)
-{
-	typec_mux_put(data);
-}
-
-static void pmic_glink_altmode_put_switch(void *data)
-{
-	typec_switch_put(data);
-}
-
-static void pmic_glink_altmode_enable_worker(struct work_struct *work)
-{
-	struct pmic_glink_altmode *altmode = work_to_altmode(work);
-	int ret;
-
-	ret = pmic_glink_altmode_request(altmode, ALTMODE_PAN_EN, 0);
-	if (ret)
-		dev_err(altmode->dev, "failed to request altmode notifications: %d\n", ret);
-}
-
-static void pmic_glink_altmode_pdr_notify(void *priv, int state)
-{
-	struct pmic_glink_altmode *altmode = priv;
-
-	if (state == SERVREG_SERVICE_STATE_UP)
-		schedule_work(&altmode->enable_work);
-}
-
-static const struct of_device_id pmic_glink_altmode_of_quirks[] = {
-	{ .compatible = "qcom,sc8180x-pmic-glink", .data = (void *)PMIC_GLINK_OWNER_USBC },
-	{}
-};
-
-static int pmic_glink_altmode_probe(struct auxiliary_device *adev,
-				    const struct auxiliary_device_id *id)
-{
-	struct pmic_glink_altmode_port *alt_port;
-	struct pmic_glink_altmode *altmode;
-	const struct of_device_id *match;
-	struct fwnode_handle *fwnode;
-	struct device *dev = &adev->dev;
-	u32 port;
-	int ret;
-
-	altmode = devm_kzalloc(dev, sizeof(*altmode), GFP_KERNEL);
-	if (!altmode)
-		return -ENOMEM;
-
-	altmode->dev = dev;
-
-	match = of_match_device(pmic_glink_altmode_of_quirks, dev->parent);
-	if (match)
-		altmode->owner_id = (unsigned long)match->data;
-	else
-		altmode->owner_id = PMIC_GLINK_OWNER_USBC_PAN;
-
-	INIT_WORK(&altmode->enable_work, pmic_glink_altmode_enable_worker);
-	init_completion(&altmode->pan_ack);
-	mutex_init(&altmode->lock);
-
-	device_for_each_child_node(dev, fwnode) {
-		ret = fwnode_property_read_u32(fwnode, "reg", &port);
-		if (ret < 0) {
-			dev_err(dev, "missing reg property of %pOFn\n", fwnode);
-			fwnode_handle_put(fwnode);
-			return ret;
-		}
-
-		if (port >= ARRAY_SIZE(altmode->ports)) {
-			dev_warn(dev, "invalid connector number, ignoring\n");
-			continue;
-		}
-
-		if (altmode->ports[port].altmode) {
-			dev_err(dev, "multiple connector definition for port %u\n", port);
-			fwnode_handle_put(fwnode);
-			return -EINVAL;
-		}
-
-		alt_port = &altmode->ports[port];
-		alt_port->altmode = altmode;
-		alt_port->index = port;
-		INIT_WORK(&alt_port->work, pmic_glink_altmode_worker);
-
-		alt_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev, to_of_node(fwnode));
-		if (IS_ERR(alt_port->bridge)) {
-			fwnode_handle_put(fwnode);
-			return PTR_ERR(alt_port->bridge);
-		}
-
-		alt_port->dp_alt.svid = USB_TYPEC_DP_SID;
-		alt_port->dp_alt.mode = USB_TYPEC_DP_MODE;
-		alt_port->dp_alt.active = 1;
-
-		alt_port->typec_mux = fwnode_typec_mux_get(fwnode);
-		if (IS_ERR(alt_port->typec_mux)) {
-			fwnode_handle_put(fwnode);
-			return dev_err_probe(dev, PTR_ERR(alt_port->typec_mux),
-					     "failed to acquire mode-switch for port: %d\n",
-					     port);
-		}
-
-		ret = devm_add_action_or_reset(dev, pmic_glink_altmode_put_mux,
-					       alt_port->typec_mux);
-		if (ret) {
-			fwnode_handle_put(fwnode);
-			return ret;
-		}
-
-		alt_port->typec_retimer = fwnode_typec_retimer_get(fwnode);
-		if (IS_ERR(alt_port->typec_retimer)) {
-			fwnode_handle_put(fwnode);
-			return dev_err_probe(dev, PTR_ERR(alt_port->typec_retimer),
-					     "failed to acquire retimer-switch for port: %d\n",
-					     port);
-		}
-
-		ret = devm_add_action_or_reset(dev, pmic_glink_altmode_put_retimer,
-					       alt_port->typec_retimer);
-		if (ret) {
-			fwnode_handle_put(fwnode);
-			return ret;
-		}
-
-		alt_port->typec_switch = fwnode_typec_switch_get(fwnode);
-		if (IS_ERR(alt_port->typec_switch)) {
-			fwnode_handle_put(fwnode);
-			return dev_err_probe(dev, PTR_ERR(alt_port->typec_switch),
-					     "failed to acquire orientation-switch for port: %d\n",
-					     port);
-		}
-
-		ret = devm_add_action_or_reset(dev, pmic_glink_altmode_put_switch,
-					       alt_port->typec_switch);
-		if (ret) {
-			fwnode_handle_put(fwnode);
-			return ret;
-		}
-	}
-
-	for (port = 0; port < ARRAY_SIZE(altmode->ports); port++) {
-		alt_port = &altmode->ports[port];
-		if (!alt_port->bridge)
-			continue;
-
-		ret = devm_drm_dp_hpd_bridge_add(dev, alt_port->bridge);
-		if (ret)
-			return ret;
-	}
-
-	altmode->client = devm_pmic_glink_register_client(dev,
-							  altmode->owner_id,
-							  pmic_glink_altmode_callback,
-							  pmic_glink_altmode_pdr_notify,
-							  altmode);
-	return PTR_ERR_OR_ZERO(altmode->client);
-}
-
-static const struct auxiliary_device_id pmic_glink_altmode_id_table[] = {
-	{ .name = "pmic_glink.altmode", },
-	{},
-};
-MODULE_DEVICE_TABLE(auxiliary, pmic_glink_altmode_id_table);
-
-static struct auxiliary_driver pmic_glink_altmode_driver = {
-	.name = "pmic_glink_altmode",
-	.probe = pmic_glink_altmode_probe,
-	.id_table = pmic_glink_altmode_id_table,
-};
-
-module_auxiliary_driver(pmic_glink_altmode_driver);
-
-MODULE_DESCRIPTION("Qualcomm PMIC GLINK Altmode driver");
-MODULE_LICENSE("GPL");
diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index 40fcda055b05..1ef638d5fd79 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -10,9 +10,14 @@
 #include <linux/of_device.h>
 #include <linux/property.h>
 #include <linux/soc/qcom/pdr.h>
+#include <linux/usb/pd_vdo.h>
+#include <linux/usb/typec_dp.h>
 #include <linux/usb/typec_mux.h>
 #include <linux/gpio/consumer.h>
 #include <linux/soc/qcom/pmic_glink.h>
+
+#include <drm/bridge/aux-bridge.h>
+
 #include "ucsi.h"
 
 #define PMIC_GLINK_MAX_PORTS	2
@@ -27,6 +32,16 @@
 #define UC_UCSI_WRITE_BUF_REQ           0x12
 #define UC_UCSI_USBC_NOTIFY_IND         0x13
 
+/*
+ * On sc8180x these requests use UCSI owner,
+ * on other platforms they use USBC_PAN.
+ */
+#define USBC_CMD_WRITE_REQ		0x15
+#define USBC_PAN_NOTIFY_IND		0x16
+
+#define ALTMODE_PAN_EN		0x10
+#define ALTMODE_PAN_ACK		0x11
+
 struct ucsi_read_buf_req_msg {
 	struct pmic_glink_hdr   hdr;
 };
@@ -55,17 +70,89 @@ struct ucsi_notify_ind_msg {
 	u32                     reserved;
 };
 
+struct usbc_write_req_msg {
+	struct pmic_glink_hdr   hdr;
+	__le32			cmd;
+	__le32			arg;
+	u32                     reserved;
+};
+
+#define USBC_NOTIFY_PAYLOAD_SIZE 16
+struct usbc_pan_notify_ind_msg {
+	struct pmic_glink_hdr	hdr;
+	char			payload[USBC_NOTIFY_PAYLOAD_SIZE];
+	u32			reserved;
+};
+
+enum pmic_glink_ucsi_orientation {
+	USBC_ORIENTATION_NORMAL,
+	USBC_ORIENTATION_REVERSE,
+	USBC_ORIENTATION_NONE,
+};
+
+enum pmic_glink_ucsi_mux {
+	USBC_MUX_NONE,
+	USBC_MUX_USB_2L,
+	USBC_MUX_DP_4L,
+	USBC_MUX_USB_DP,
+};
+
+enum pmic_glink_altmode_pin_assignment {
+	DPAM_HPD_OUT,
+	DPAM_HPD_A,
+	DPAM_HPD_B,
+	DPAM_HPD_C,
+	DPAM_HPD_D,
+	DPAM_HPD_E,
+	DPAM_HPD_F,
+};
+
+#define SC8180X_PORT_MASK		GENMASK(7, 0)
+#define SC8180X_ORIENTATION_MASK	GENMASK(15, 8)
+#define SC8180X_MUX_MASK		GENMASK(23, 16)
+#define SC8180X_MODE_MASK		GENMASK(29, 24)
+#define SC8180X_HPD_STATE_MASK		BIT(30)
+#define SC8180X_HPD_IRQ_MASK		BIT(31)
+
+#define SC8280XP_DPAM_MASK		GENMASK(5, 0)
+#define SC8280XP_HPD_STATE_MASK		BIT(6)
+#define SC8280XP_HPD_IRQ_MASK		BIT(7)
+
+struct pmic_glink_ucsi_port {
+	spinlock_t lock;
+
+	struct work_struct altmode_work;
+
+	struct pmic_glink_ucsi *ucsi;
+	struct gpio_desc *port_orientation;
+	struct auxiliary_device *bridge;
+
+	int idx;
+
+	enum typec_orientation orientation;
+
+	enum pmic_glink_ucsi_mux mux;
+	unsigned int mode;
+
+	u16 svid;
+	u8 hpd_state;
+	u8 hpd_irq;
+};
+
 struct pmic_glink_ucsi {
 	struct device *dev;
 
-	struct gpio_desc *port_orientation[PMIC_GLINK_MAX_PORTS];
+	struct pmic_glink_ucsi_port ports[PMIC_GLINK_MAX_PORTS];
 
+	unsigned int altmode_id;
+	struct pmic_glink_client *altmode_client;
 	struct pmic_glink_client *client;
 
 	struct ucsi *ucsi;
 	struct completion read_ack;
 	struct completion write_ack;
 	struct completion sync_ack;
+	struct completion pan_ack;
 	bool sync_pending;
 	struct mutex lock;	/* protects concurrent access to PMIC Glink interface */
 
@@ -193,27 +280,128 @@ static void pmic_glink_ucsi_update_connector(struct ucsi_connector *con)
 	int i;
 
 	for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) {
-		if (ucsi->port_orientation[i])
+		if (ucsi->ports[i].port_orientation)
 			con->typec_cap.orientation_aware = true;
 	}
 }
 
+static int pmic_glink_altmode_request(struct pmic_glink_ucsi *ucsi, u32 cmd, u32 arg)
+{
+	struct usbc_write_req_msg req = {};
+	unsigned long left;
+	int ret;
+
+	/*
+	 * The USBC_CMD_WRITE_REQ ack doesn't identify the request, so wait for
+	 * one ack at a time.
+	 */
+	mutex_lock(&ucsi->lock);
+
+	req.hdr.owner = cpu_to_le32(ucsi->altmode_id);
+	req.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP);
+	req.hdr.opcode = cpu_to_le32(USBC_CMD_WRITE_REQ);
+	req.cmd = cpu_to_le32(cmd);
+	req.arg = cpu_to_le32(arg);
+
+	reinit_completion(&ucsi->pan_ack);
+	ret = pmic_glink_send(ucsi->altmode_client, &req, sizeof(req));
+	if (ret) {
+		dev_err(ucsi->dev, "failed to send altmode request: %#x (%d)\n", cmd, ret);
+		goto out_unlock;
+	}
+
+	left = wait_for_completion_timeout(&ucsi->pan_ack, 5 * HZ);
+	if (!left) {
+		dev_err(ucsi->dev, "timeout waiting for altmode request ack for: %#x\n", cmd);
+		ret = -ETIMEDOUT;
+	}
+
+out_unlock:
+	mutex_unlock(&ucsi->lock);
+	return ret;
+}
+
+static void pmic_glink_ucsi_set_orientation(struct ucsi_connector *con,
+					    struct pmic_glink_ucsi_port *port)
+{
+	enum typec_orientation orientation;
+	unsigned long flags;
+
+	spin_lock_irqsave(&port->lock, flags);
+	orientation = port->orientation;
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	if (port->port_orientation) {
+		int val = gpiod_get_value(port->port_orientation);
+		if (val >= 0)
+			orientation = val ?
+				TYPEC_ORIENTATION_REVERSE :
+				TYPEC_ORIENTATION_NORMAL;
+	}
+
+	typec_set_orientation(con->port, orientation);
+}
+
 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])
+	if (con->num >= PMIC_GLINK_MAX_PORTS)
 		return;
 
-	orientation = gpiod_get_value(ucsi->port_orientation[con->num - 1]);
-	if (orientation >= 0) {
-		typec_set_orientation(con->port,
-				      orientation ?
-				      TYPEC_ORIENTATION_REVERSE :
-				      TYPEC_ORIENTATION_NORMAL);
-	}
+	pmic_glink_ucsi_set_orientation(con, &ucsi->ports[con->num - 1]);
+}
+
+static void pmic_glink_ucsi_register_altmode(struct ucsi_connector *con)
+{
+	static const u8 all_assignments = BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D) |
+			     BIT(DP_PIN_ASSIGN_E);
+	struct typec_altmode_desc desc;
+	struct typec_altmode *alt;
+
+	mutex_lock(&con->lock);
+
+	if (con->port_altmode[0])
+		goto out;
+
+	memset(&desc, 0, sizeof(desc));
+	desc.svid = USB_TYPEC_DP_SID;
+	desc.mode = USB_TYPEC_DP_MODE;
+
+	desc.vdo = DP_CAP_CAPABILITY(DP_CAP_DFP_D);
+
+	/* We can't rely on the firmware with the capabilities. */
+	desc.vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
+
+	/* Claiming that we support all pin assignments */
+	desc.vdo |= all_assignments << 8;
+	desc.vdo |= all_assignments << 16;
+
+	alt = typec_port_register_altmode(con->port, &desc);
+	if (IS_ERR(alt))
+		goto out;
+
+        alt->desc = "DisplayPort";
+
+	con->port_altmode[0] = alt;
+
+out:
+	mutex_unlock(&con->lock);
+}
+
+static void pmic_glink_ucsi_registered(struct ucsi *ucsi)
+{
+	int i, ret;
+
+	if (!ucsi->connector)
+		return;
+
+	for (i = 0; i < ucsi->cap.num_connectors; i++)
+		pmic_glink_ucsi_register_altmode(&ucsi->connector[i]);
+
+	ret = pmic_glink_altmode_request(ucsi_get_drvdata(ucsi), ALTMODE_PAN_EN, 0);
+	if (ret)
+		dev_err(ucsi->dev, "failed to request altmode notification: %d\n", ret);
 }
 
 static const struct ucsi_operations pmic_glink_ucsi_ops = {
@@ -222,6 +410,7 @@ static const struct ucsi_operations pmic_glink_ucsi_ops = {
 	.async_write = pmic_glink_ucsi_async_write,
 	.update_connector = pmic_glink_ucsi_update_connector,
 	.connector_status = pmic_glink_ucsi_connector_status,
+	.ucsi_registered = pmic_glink_ucsi_registered,
 };
 
 static void pmic_glink_ucsi_notify_handle(struct pmic_glink_ucsi *ucsi, u32 cci)
@@ -289,6 +478,206 @@ static void pmic_glink_ucsi_notify_ind(struct pmic_glink_ucsi *ucsi, const void
 	pmic_glink_ucsi_notify_handle(ucsi, le32_to_cpu(msg->notification));
 }
 
+static enum typec_orientation pmic_glink_altmode_orientation(unsigned int orientation)
+{
+	if (orientation == USBC_ORIENTATION_NORMAL)
+		return TYPEC_ORIENTATION_NORMAL;
+	else if (orientation == USBC_ORIENTATION_REVERSE)
+		return TYPEC_ORIENTATION_REVERSE;
+
+	WARN_ON(orientation != USBC_ORIENTATION_NONE);
+	return TYPEC_ORIENTATION_NONE;
+}
+
+static void pmic_glink_ucsi_notify_ind_sc8180x(struct pmic_glink_ucsi *ucsi, const void *data, size_t len)
+{
+	const struct ucsi_notify_ind_msg *msg;
+	struct pmic_glink_ucsi_port *port;
+	unsigned long flags;
+	u32 notification;
+	unsigned int idx;
+	unsigned int orientation;
+
+	if (len != sizeof (*msg)) {
+		dev_err_ratelimited(ucsi->dev, "Unexpected altmode notify struct size %zd\n", len);
+		return;
+	}
+
+	msg = data;
+
+	notification = le32_to_cpu(msg->notification);
+
+	idx = FIELD_GET(SC8180X_PORT_MASK, notification);
+	if (idx == 0x80) {
+		schedule_work(&ucsi->notify_work_sc8180x);
+		return;
+	}
+
+	if (idx > ARRAY_SIZE(ucsi->ports)) {
+		dev_err_ratelimited(ucsi->dev, "notification port index out of range (%d)\n", idx);
+		return;
+	}
+
+	port = &ucsi->ports[idx];
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	orientation = FIELD_GET(SC8180X_ORIENTATION_MASK, notification);
+	port->orientation = pmic_glink_altmode_orientation(orientation);
+	port->mux = FIELD_GET(SC8180X_MUX_MASK, notification);
+	port->mode = FIELD_GET(SC8180X_MODE_MASK, notification);
+	port->hpd_state = FIELD_GET(SC8180X_HPD_STATE_MASK, notification);
+	port->hpd_irq = FIELD_GET(SC8180X_HPD_IRQ_MASK, notification);
+
+	if (port->mux == USBC_MUX_DP_4L ||
+	    port->mux == USBC_MUX_USB_DP)
+		port->svid = USB_TYPEC_DP_SID;
+	else
+		port->svid = USB_SID_PD;
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	schedule_work(&port->altmode_work);
+}
+
+/* used everywhere except sc8180x */
+static void pmic_glink_ucsi_altmode_notify_ind(struct pmic_glink_ucsi *ucsi, u16 svid, const void *data, size_t len)
+{
+	const struct usbc_pan_notify_ind_msg *msg;
+	struct pmic_glink_ucsi_port *port;
+	unsigned long flags;
+	unsigned int idx;
+
+	if (len != sizeof (*msg)) {
+		dev_err_ratelimited(ucsi->dev, "Unexpected altmode notify struct size %zd\n", len);
+		return;
+	}
+
+	msg = data;
+
+	idx = msg->payload[0];
+	if (idx > ARRAY_SIZE(ucsi->ports)) {
+		dev_err_ratelimited(ucsi->dev, "notification port index out of range (%d)\n", idx);
+		return;
+	}
+
+	port = &ucsi->ports[idx];
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	port->orientation = pmic_glink_altmode_orientation(msg->payload[1]);
+	port->mux = msg->payload[2];
+	port->svid = svid;
+	port->mode = FIELD_GET(SC8280XP_DPAM_MASK, msg->payload[8]);
+	port->hpd_state = FIELD_GET(SC8280XP_HPD_STATE_MASK, msg->payload[8]);
+	port->hpd_irq = FIELD_GET(SC8280XP_HPD_IRQ_MASK, msg->payload[8]);
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	schedule_work(&port->altmode_work);
+}
+
+static struct typec_altmode *find_altmode(struct ucsi_connector *con,
+					  u16 svid)
+{
+	int i;
+
+	for (i = 0; con->port_altmode[i]; i++) {
+		if (con->port_altmode[i]->svid == svid)
+			return con->port_altmode[i];
+	}
+
+	return NULL;
+}
+
+static void pmic_glink_ucsi_set_state(struct ucsi_connector *con,
+				      struct pmic_glink_ucsi_port *port)
+{
+	struct typec_displayport_data dp_data = {};
+	struct typec_altmode *altmode = NULL;
+	unsigned long flags;
+	void *data = NULL;
+	int mode;
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	if (port->svid == USB_SID_PD) {
+		mode = TYPEC_STATE_USB;
+	} else if (port->svid == USB_TYPEC_DP_SID && port->mode == DPAM_HPD_OUT) {
+		mode = TYPEC_STATE_SAFE;
+	} else if (port->svid == USB_TYPEC_DP_SID) {
+		altmode = find_altmode(con, port->svid);
+		if (!altmode) {
+			dev_err(con->ucsi->dev, "altmode woth SVID 0x%04x not found\n",
+				port->svid);
+			spin_unlock_irqrestore(&port->lock, flags);
+			return;
+		}
+
+		mode = TYPEC_MODAL_STATE(port->mode - DPAM_HPD_A);
+
+		dp_data.status = DP_STATUS_ENABLED;
+		dp_data.status |= DP_STATUS_CON_DFP_D;
+		if (port->hpd_state)
+			dp_data.status |= DP_STATUS_HPD_STATE;
+		if (port->hpd_irq)
+			dp_data.status |= DP_STATUS_IRQ_HPD;
+		dp_data.conf = DP_CONF_SET_PIN_ASSIGN(port->mode - DPAM_HPD_A);
+
+		data = &dp_data;
+	} else {
+		dev_err(con->ucsi->dev, "Unsupported SVID 0x%04x\n", port->svid);
+		spin_unlock_irqrestore(&port->lock, flags);
+		return;
+	}
+
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	if (altmode)
+		typec_altmode_set_port(altmode, mode, data);
+	else
+		typec_set_mode(con->port, mode);
+
+	if (port->bridge)
+		drm_aux_hpd_bridge_notify(&port->bridge->dev,
+					  port->hpd_state ?
+					  connector_status_connected :
+					  connector_status_disconnected);
+
+}
+
+static void pmic_glink_ucsi_handle_altmode(struct pmic_glink_ucsi_port *port)
+{
+	struct pmic_glink_ucsi *ucsi = port->ucsi;
+	struct ucsi_connector *con;
+	int idx = port->idx;
+
+	if (idx > ucsi->ucsi->cap.num_connectors) {
+		dev_warn_ratelimited(ucsi->ucsi->dev, "altmode port out of range: %d\n", idx);
+		goto out;
+	}
+
+	con = &ucsi->ucsi->connector[idx];
+
+	mutex_lock(&con->lock);
+
+	pmic_glink_ucsi_set_orientation(con, port);
+
+	pmic_glink_ucsi_set_state(con, port);
+
+	mutex_unlock(&con->lock);
+
+out:
+	pmic_glink_altmode_request(ucsi, ALTMODE_PAN_ACK, idx);
+}
+
+static void pmic_glink_ucsi_altmode_work(struct work_struct *work)
+{
+	struct pmic_glink_ucsi_port *port = container_of(work, struct pmic_glink_ucsi_port, altmode_work);
+
+	pmic_glink_ucsi_handle_altmode(port);
+}
+
 static void pmic_glink_ucsi_notify_work_sc8180x(struct work_struct *work)
 {
 	struct pmic_glink_ucsi *ucsi = container_of(work, struct pmic_glink_ucsi, notify_work_sc8180x);
@@ -324,7 +713,10 @@ static void pmic_glink_ucsi_callback_sc8180x(const void *data, size_t len, void
 		pmic_glink_ucsi_write_ack(ucsi, data, len);
 		break;
 	case UC_UCSI_USBC_NOTIFY_IND:
-		schedule_work(&ucsi->notify_work_sc8180x);
+		pmic_glink_ucsi_notify_ind_sc8180x(ucsi, data, len);
+		break;
+	case USBC_CMD_WRITE_REQ:
+		complete(&ucsi->pan_ack);
 		break;
 	};
 }
@@ -347,6 +739,26 @@ static void pmic_glink_ucsi_callback(const void *data, size_t len, void *priv)
 	};
 }
 
+static void pmic_glink_ucsi_altmode_callback(const void *data, size_t len, void *priv)
+{
+	struct pmic_glink_ucsi *ucsi = priv;
+	const struct pmic_glink_hdr *hdr = data;
+	u16 opcode;
+	u16 svid;
+
+	opcode = le32_to_cpu(hdr->opcode) & 0xff;
+	svid = le32_to_cpu(hdr->opcode) >> 16;
+
+	switch (opcode) {
+	case USBC_CMD_WRITE_REQ:
+		complete(&ucsi->pan_ack);
+		break;
+	case USBC_PAN_NOTIFY_IND:
+		pmic_glink_ucsi_altmode_notify_ind(ucsi, svid, data, len);
+		break;
+	};
+}
+
 static void pmic_glink_ucsi_pdr_notify(void *priv, int state)
 {
 	struct pmic_glink_ucsi *ucsi = priv;
@@ -357,6 +769,11 @@ static void pmic_glink_ucsi_pdr_notify(void *priv, int state)
 		ucsi_unregister(ucsi->ucsi);
 }
 
+static void pmic_glink_ucsi_altmode_pdr_notify(void *priv, int state)
+{
+	/* do nothing */
+}
+
 static void pmic_glink_ucsi_destroy(void *data)
 {
 	struct pmic_glink_ucsi *ucsi = data;
@@ -389,7 +806,7 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 	const struct of_device_id *match;
 	struct fwnode_handle *fwnode;
 	bool sc8180x_glink;
-	int ret;
+	int i, ret;
 
 	ucsi = devm_kzalloc(dev, sizeof(*ucsi), GFP_KERNEL);
 	if (!ucsi)
@@ -403,6 +820,7 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 	init_completion(&ucsi->read_ack);
 	init_completion(&ucsi->write_ack);
 	init_completion(&ucsi->sync_ack);
+	init_completion(&ucsi->pan_ack);
 	mutex_init(&ucsi->lock);
 
 	ucsi->ucsi = ucsi_create(dev, &pmic_glink_ucsi_ops);
@@ -436,27 +854,64 @@ static int pmic_glink_ucsi_probe(struct auxiliary_device *adev,
 		}
 
 		desc = devm_gpiod_get_index_optional(&adev->dev, "orientation", port, GPIOD_IN);
-
-		/* If GPIO isn't found, continue */
-		if (!desc)
-			continue;
-
 		if (IS_ERR(desc))
 			return dev_err_probe(dev, PTR_ERR(desc),
 					     "unable to acquire orientation gpio\n");
-		ucsi->port_orientation[port] = desc;
+		ucsi->ports[port].port_orientation = desc;
+
+		ucsi->ports[port].bridge = devm_drm_dp_hpd_bridge_alloc(ucsi->dev,
+									to_of_node(fwnode));
+		if (IS_ERR(ucsi->ports[port].bridge)) {
+			fwnode_handle_put(fwnode);
+			return PTR_ERR(ucsi->ports[port].bridge);
+		}
+	}
+
+	for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) {
+		if (!ucsi->ports[i].bridge)
+			continue;
+
+		ret = devm_drm_dp_hpd_bridge_add(dev, ucsi->ports[i].bridge);
+		if (ret)
+			return ret;
 	}
 
 	sc8180x_glink = of_device_is_compatible(dev->parent->of_node,
 						"qcom,sc8180x-pmic-glink");
 
+	for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) {
+		ucsi->ports[i].idx = i;
+		ucsi->ports[i].ucsi = ucsi;
+		spin_lock_init(&ucsi->ports[i].lock);
+		INIT_WORK(&ucsi->ports[i].altmode_work,
+			  pmic_glink_ucsi_altmode_work);
+	}
+
 	if (sc8180x_glink) {
+		ucsi->altmode_id = PMIC_GLINK_OWNER_USBC;
+
+		/*
+		 * We don't need another instance of USBC client, both altmode
+		 * and UCSI are handled via the same client.
+		 */
 		ucsi->client = devm_pmic_glink_register_client(dev,
 							       PMIC_GLINK_OWNER_USBC,
 							       pmic_glink_ucsi_callback_sc8180x,
 							       pmic_glink_ucsi_pdr_notify,
 							       ucsi);
+		ucsi->altmode_client = ucsi->client;
 	} else {
+		ucsi->altmode_id = PMIC_GLINK_OWNER_USBC_PAN;
+
+		ucsi->altmode_client =
+			devm_pmic_glink_register_client(dev,
+							ucsi->altmode_id,
+							pmic_glink_ucsi_altmode_callback,
+							pmic_glink_ucsi_altmode_pdr_notify,
+							ucsi);
+		if (IS_ERR(ucsi->altmode_client))
+			return PTR_ERR(ucsi->altmode_client);
+
 		ucsi->client = devm_pmic_glink_register_client(dev,
 							       PMIC_GLINK_OWNER_USBC,
 							       pmic_glink_ucsi_callback,

-- 
2.39.2

Re: [PATCH 7/8] usb: typec: ucsi: glink: merge pmic_glink_altmode driver

[Heikki Krogerus]

Hi Dmitry,

On Tue, Apr 16, 2024 at 05:20:56AM +0300, Dmitry Baryshkov wrote:
> Move handling of USB Altmode to the ucsi_glink driver. This way the
> altmode is properly registered in the Type-C framework, the altmode
> handlers can use generic typec calls, the UCSI driver can use
> orientation information from altmode messages and vice versa, the
> altmode handlers can use GPIO-based orientation inormation from UCSI
> GLINK driver.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/soc/qcom/Makefile             |   1 -
>  drivers/soc/qcom/pmic_glink_altmode.c | 546 ----------------------------------
>  drivers/usb/typec/ucsi/ucsi_glink.c   | 495 ++++++++++++++++++++++++++++--
>  3 files changed, 475 insertions(+), 567 deletions(-)
> 
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index ca0bece0dfff..d43d2b444634 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -9,7 +9,6 @@ obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
>  obj-$(CONFIG_QCOM_OCMEM)	+= ocmem.o
>  obj-$(CONFIG_QCOM_PDR_HELPERS)	+= pdr_interface.o
>  obj-$(CONFIG_QCOM_PMIC_GLINK)	+= pmic_glink.o
> -obj-$(CONFIG_QCOM_PMIC_GLINK)	+= pmic_glink_altmode.o
>  obj-$(CONFIG_QCOM_PMIC_PDCHARGER_ULOG)	+= pmic_pdcharger_ulog.o
>  CFLAGS_pmic_pdcharger_ulog.o	:=  -I$(src)
>  obj-$(CONFIG_QCOM_QMI_HELPERS)	+= qmi_helpers.o
> diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
> deleted file mode 100644
> index b3808fc24c69..000000000000
> --- a/drivers/soc/qcom/pmic_glink_altmode.c
> +++ /dev/null
> @@ -1,546 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Copyright (c) 2019-2020, The Linux Foundation. All rights reserved.
> - * Copyright (c) 2022, Linaro Ltd
> - */
> -#include <linux/auxiliary_bus.h>
> -#include <linux/bitfield.h>
> -#include <linux/module.h>
> -#include <linux/of.h>
> -#include <linux/of_device.h>
> -#include <linux/mutex.h>
> -#include <linux/property.h>
> -#include <linux/soc/qcom/pdr.h>
> -#include <drm/bridge/aux-bridge.h>
> -
> -#include <linux/usb/typec_altmode.h>
> -#include <linux/usb/typec_dp.h>
> -#include <linux/usb/typec_mux.h>
> -#include <linux/usb/typec_retimer.h>
> -
> -#include <linux/soc/qcom/pmic_glink.h>
> -
> -#define PMIC_GLINK_MAX_PORTS	2
> -
> -#define USBC_SC8180X_NOTIFY_IND	0x13
> -#define USBC_CMD_WRITE_REQ      0x15
> -#define USBC_NOTIFY_IND		0x16
> -
> -#define ALTMODE_PAN_EN		0x10
> -#define ALTMODE_PAN_ACK		0x11
> -
> -struct usbc_write_req {
> -	struct pmic_glink_hdr   hdr;
> -	__le32 cmd;
> -	__le32 arg;
> -	__le32 reserved;
> -};
> -
> -#define NOTIFY_PAYLOAD_SIZE 16
> -struct usbc_notify {
> -	struct pmic_glink_hdr hdr;
> -	char payload[NOTIFY_PAYLOAD_SIZE];
> -	u32 reserved;
> -};
> -
> -struct usbc_sc8180x_notify {
> -	struct pmic_glink_hdr hdr;
> -	__le32 notification;
> -	__le32 reserved[2];
> -};
> -
> -enum pmic_glink_altmode_pin_assignment {
> -	DPAM_HPD_OUT,
> -	DPAM_HPD_A,
> -	DPAM_HPD_B,
> -	DPAM_HPD_C,
> -	DPAM_HPD_D,
> -	DPAM_HPD_E,
> -	DPAM_HPD_F,
> -};
> -
> -struct pmic_glink_altmode;
> -
> -#define work_to_altmode_port(w) container_of((w), struct pmic_glink_altmode_port, work)
> -
> -struct pmic_glink_altmode_port {
> -	struct pmic_glink_altmode *altmode;
> -	unsigned int index;
> -
> -	struct typec_switch *typec_switch;
> -	struct typec_mux *typec_mux;
> -	struct typec_mux_state state;
> -	struct typec_retimer *typec_retimer;
> -	struct typec_retimer_state retimer_state;
> -	struct typec_altmode dp_alt;
> -
> -	struct work_struct work;
> -
> -	struct auxiliary_device *bridge;
> -
> -	enum typec_orientation orientation;
> -	u16 svid;
> -	u8 dp_data;
> -	u8 mode;
> -	u8 hpd_state;
> -	u8 hpd_irq;
> -};
> -
> -#define work_to_altmode(w) container_of((w), struct pmic_glink_altmode, enable_work)
> -
> -struct pmic_glink_altmode {
> -	struct device *dev;
> -
> -	unsigned int owner_id;
> -
> -	/* To synchronize WRITE_REQ acks */
> -	struct mutex lock;
> -
> -	struct completion pan_ack;
> -	struct pmic_glink_client *client;
> -
> -	struct work_struct enable_work;
> -
> -	struct pmic_glink_altmode_port ports[PMIC_GLINK_MAX_PORTS];
> -};
> -
> -static int pmic_glink_altmode_request(struct pmic_glink_altmode *altmode, u32 cmd, u32 arg)
> -{
> -	struct usbc_write_req req = {};
> -	unsigned long left;
> -	int ret;
> -
> -	/*
> -	 * The USBC_CMD_WRITE_REQ ack doesn't identify the request, so wait for
> -	 * one ack at a time.
> -	 */
> -	mutex_lock(&altmode->lock);
> -
> -	req.hdr.owner = cpu_to_le32(altmode->owner_id);
> -	req.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP);
> -	req.hdr.opcode = cpu_to_le32(USBC_CMD_WRITE_REQ);
> -	req.cmd = cpu_to_le32(cmd);
> -	req.arg = cpu_to_le32(arg);
> -
> -	ret = pmic_glink_send(altmode->client, &req, sizeof(req));
> -	if (ret) {
> -		dev_err(altmode->dev, "failed to send altmode request: %#x (%d)\n", cmd, ret);
> -		goto out_unlock;
> -	}
> -
> -	left = wait_for_completion_timeout(&altmode->pan_ack, 5 * HZ);
> -	if (!left) {
> -		dev_err(altmode->dev, "timeout waiting for altmode request ack for: %#x\n", cmd);
> -		ret = -ETIMEDOUT;
> -	}
> -
> -out_unlock:
> -	mutex_unlock(&altmode->lock);
> -	return ret;
> -}
> -
> -static void pmic_glink_altmode_enable_dp(struct pmic_glink_altmode *altmode,
> -					 struct pmic_glink_altmode_port *port,
> -					 u8 mode, bool hpd_state,
> -					 bool hpd_irq)
> -{
> -	struct typec_displayport_data dp_data = {};
> -	int ret;
> -
> -	dp_data.status = DP_STATUS_ENABLED;
> -	if (hpd_state)
> -		dp_data.status |= DP_STATUS_HPD_STATE;
> -	if (hpd_irq)
> -		dp_data.status |= DP_STATUS_IRQ_HPD;
> -	dp_data.conf = DP_CONF_SET_PIN_ASSIGN(mode);
> -
> -	port->state.alt = &port->dp_alt;
> -	port->state.data = &dp_data;
> -	port->state.mode = TYPEC_MODAL_STATE(mode);
> -
> -	ret = typec_mux_set(port->typec_mux, &port->state);
> -	if (ret)
> -		dev_err(altmode->dev, "failed to switch mux to DP: %d\n", ret);
> -
> -	port->retimer_state.alt = &port->dp_alt;
> -	port->retimer_state.data = &dp_data;
> -	port->retimer_state.mode = TYPEC_MODAL_STATE(mode);
> -
> -	ret = typec_retimer_set(port->typec_retimer, &port->retimer_state);
> -	if (ret)
> -		dev_err(altmode->dev, "failed to setup retimer to DP: %d\n", ret);
> -}
> -
> -static void pmic_glink_altmode_enable_usb(struct pmic_glink_altmode *altmode,
> -					  struct pmic_glink_altmode_port *port)
> -{
> -	int ret;
> -
> -	port->state.alt = NULL;
> -	port->state.data = NULL;
> -	port->state.mode = TYPEC_STATE_USB;
> -
> -	ret = typec_mux_set(port->typec_mux, &port->state);
> -	if (ret)
> -		dev_err(altmode->dev, "failed to switch mux to USB: %d\n", ret);
> -
> -	port->retimer_state.alt = NULL;
> -	port->retimer_state.data = NULL;
> -	port->retimer_state.mode = TYPEC_STATE_USB;
> -
> -	ret = typec_retimer_set(port->typec_retimer, &port->retimer_state);
> -	if (ret)
> -		dev_err(altmode->dev, "failed to setup retimer to USB: %d\n", ret);
> -}
> -
> -static void pmic_glink_altmode_safe(struct pmic_glink_altmode *altmode,
> -				    struct pmic_glink_altmode_port *port)
> -{
> -	int ret;
> -
> -	port->state.alt = NULL;
> -	port->state.data = NULL;
> -	port->state.mode = TYPEC_STATE_SAFE;
> -
> -	ret = typec_mux_set(port->typec_mux, &port->state);
> -	if (ret)
> -		dev_err(altmode->dev, "failed to switch mux to safe mode: %d\n", ret);
> -
> -	port->retimer_state.alt = NULL;
> -	port->retimer_state.data = NULL;
> -	port->retimer_state.mode = TYPEC_STATE_SAFE;
> -
> -	ret = typec_retimer_set(port->typec_retimer, &port->retimer_state);
> -	if (ret)
> -		dev_err(altmode->dev, "failed to setup retimer to USB: %d\n", ret);
> -}
> -
> -static void pmic_glink_altmode_worker(struct work_struct *work)
> -{
> -	struct pmic_glink_altmode_port *alt_port = work_to_altmode_port(work);
> -	struct pmic_glink_altmode *altmode = alt_port->altmode;
> -
> -	typec_switch_set(alt_port->typec_switch, alt_port->orientation);
> -
> -	if (alt_port->svid == USB_TYPEC_DP_SID && alt_port->mode == 0xff)
> -		pmic_glink_altmode_safe(altmode, alt_port);
> -	else if (alt_port->svid == USB_TYPEC_DP_SID)
> -		pmic_glink_altmode_enable_dp(altmode, alt_port, alt_port->mode,
> -					     alt_port->hpd_state, alt_port->hpd_irq);
> -	else
> -		pmic_glink_altmode_enable_usb(altmode, alt_port);
> -
> -	drm_aux_hpd_bridge_notify(&alt_port->bridge->dev,
> -				  alt_port->hpd_state ?
> -				  connector_status_connected :
> -				  connector_status_disconnected);
> -
> -	pmic_glink_altmode_request(altmode, ALTMODE_PAN_ACK, alt_port->index);
> -}
> -
> -static enum typec_orientation pmic_glink_altmode_orientation(unsigned int orientation)
> -{
> -	if (orientation == 0)
> -		return TYPEC_ORIENTATION_NORMAL;
> -	else if (orientation == 1)
> -		return TYPEC_ORIENTATION_REVERSE;
> -	else
> -		return TYPEC_ORIENTATION_NONE;
> -}
> -
> -#define SC8180X_PORT_MASK		0x000000ff
> -#define SC8180X_ORIENTATION_MASK	0x0000ff00
> -#define SC8180X_MUX_MASK		0x00ff0000
> -#define SC8180X_MODE_MASK		0x3f000000
> -#define SC8180X_HPD_STATE_MASK		0x40000000
> -#define SC8180X_HPD_IRQ_MASK		0x80000000
> -
> -static void pmic_glink_altmode_sc8180xp_notify(struct pmic_glink_altmode *altmode,
> -					       const void *data, size_t len)
> -{
> -	struct pmic_glink_altmode_port *alt_port;
> -	const struct usbc_sc8180x_notify *msg;
> -	u32 notification;
> -	u8 orientation;
> -	u8 hpd_state;
> -	u8 hpd_irq;
> -	u16 svid;
> -	u8 port;
> -	u8 mode;
> -	u8 mux;
> -
> -	if (len != sizeof(*msg)) {
> -		dev_warn(altmode->dev, "invalid length of USBC_NOTIFY indication: %zd\n", len);
> -		return;
> -	}
> -
> -	msg = data;
> -	notification = le32_to_cpu(msg->notification);
> -	port = FIELD_GET(SC8180X_PORT_MASK, notification);
> -	orientation = FIELD_GET(SC8180X_ORIENTATION_MASK, notification);
> -	mux = FIELD_GET(SC8180X_MUX_MASK, notification);
> -	mode = FIELD_GET(SC8180X_MODE_MASK, notification);
> -	hpd_state = FIELD_GET(SC8180X_HPD_STATE_MASK, notification);
> -	hpd_irq = FIELD_GET(SC8180X_HPD_IRQ_MASK, notification);
> -
> -	svid = mux == 2 ? USB_TYPEC_DP_SID : 0;
> -
> -	if (port >= ARRAY_SIZE(altmode->ports) || !altmode->ports[port].altmode) {
> -		dev_dbg(altmode->dev, "notification on undefined port %d\n", port);
> -		return;
> -	}
> -
> -	alt_port = &altmode->ports[port];
> -	alt_port->orientation = pmic_glink_altmode_orientation(orientation);
> -	alt_port->svid = svid;
> -	alt_port->mode = mode;
> -	alt_port->hpd_state = hpd_state;
> -	alt_port->hpd_irq = hpd_irq;
> -	schedule_work(&alt_port->work);
> -}
> -
> -#define SC8280XP_DPAM_MASK	0x3f
> -#define SC8280XP_HPD_STATE_MASK BIT(6)
> -#define SC8280XP_HPD_IRQ_MASK	BIT(7)
> -
> -static void pmic_glink_altmode_sc8280xp_notify(struct pmic_glink_altmode *altmode,
> -					       u16 svid, const void *data, size_t len)
> -{
> -	struct pmic_glink_altmode_port *alt_port;
> -	const struct usbc_notify *notify;
> -	u8 orientation;
> -	u8 hpd_state;
> -	u8 hpd_irq;
> -	u8 mode;
> -	u8 port;
> -
> -	if (len != sizeof(*notify)) {
> -		dev_warn(altmode->dev, "invalid length USBC_NOTIFY_IND: %zd\n",
> -			 len);
> -		return;
> -	}
> -
> -	notify = data;
> -
> -	port = notify->payload[0];
> -	orientation = notify->payload[1];
> -	mode = FIELD_GET(SC8280XP_DPAM_MASK, notify->payload[8]) - DPAM_HPD_A;
> -	hpd_state = FIELD_GET(SC8280XP_HPD_STATE_MASK, notify->payload[8]);
> -	hpd_irq = FIELD_GET(SC8280XP_HPD_IRQ_MASK, notify->payload[8]);
> -
> -	if (port >= ARRAY_SIZE(altmode->ports) || !altmode->ports[port].altmode) {
> -		dev_dbg(altmode->dev, "notification on undefined port %d\n", port);
> -		return;
> -	}
> -
> -	alt_port = &altmode->ports[port];
> -	alt_port->orientation = pmic_glink_altmode_orientation(orientation);
> -	alt_port->svid = svid;
> -	alt_port->mode = mode;
> -	alt_port->hpd_state = hpd_state;
> -	alt_port->hpd_irq = hpd_irq;
> -	schedule_work(&alt_port->work);
> -}
> -
> -static void pmic_glink_altmode_callback(const void *data, size_t len, void *priv)
> -{
> -	struct pmic_glink_altmode *altmode = priv;
> -	const struct pmic_glink_hdr *hdr = data;
> -	u16 opcode;
> -	u16 svid;
> -
> -	opcode = le32_to_cpu(hdr->opcode) & 0xff;
> -	svid = le32_to_cpu(hdr->opcode) >> 16;
> -
> -	switch (opcode) {
> -	case USBC_CMD_WRITE_REQ:
> -		complete(&altmode->pan_ack);
> -		break;
> -	case USBC_NOTIFY_IND:
> -		pmic_glink_altmode_sc8280xp_notify(altmode, svid, data, len);
> -		break;
> -	case USBC_SC8180X_NOTIFY_IND:
> -		pmic_glink_altmode_sc8180xp_notify(altmode, data, len);
> -		break;
> -	}
> -}
> -
> -static void pmic_glink_altmode_put_retimer(void *data)
> -{
> -	typec_retimer_put(data);
> -}
> -
> -static void pmic_glink_altmode_put_mux(void *data)
> -{
> -	typec_mux_put(data);
> -}
> -
> -static void pmic_glink_altmode_put_switch(void *data)
> -{
> -	typec_switch_put(data);
> -}
> -
> -static void pmic_glink_altmode_enable_worker(struct work_struct *work)
> -{
> -	struct pmic_glink_altmode *altmode = work_to_altmode(work);
> -	int ret;
> -
> -	ret = pmic_glink_altmode_request(altmode, ALTMODE_PAN_EN, 0);
> -	if (ret)
> -		dev_err(altmode->dev, "failed to request altmode notifications: %d\n", ret);
> -}
> -
> -static void pmic_glink_altmode_pdr_notify(void *priv, int state)
> -{
> -	struct pmic_glink_altmode *altmode = priv;
> -
> -	if (state == SERVREG_SERVICE_STATE_UP)
> -		schedule_work(&altmode->enable_work);
> -}
> -
> -static const struct of_device_id pmic_glink_altmode_of_quirks[] = {
> -	{ .compatible = "qcom,sc8180x-pmic-glink", .data = (void *)PMIC_GLINK_OWNER_USBC },
> -	{}
> -};
> -
> -static int pmic_glink_altmode_probe(struct auxiliary_device *adev,
> -				    const struct auxiliary_device_id *id)
> -{
> -	struct pmic_glink_altmode_port *alt_port;
> -	struct pmic_glink_altmode *altmode;
> -	const struct of_device_id *match;
> -	struct fwnode_handle *fwnode;
> -	struct device *dev = &adev->dev;
> -	u32 port;
> -	int ret;
> -
> -	altmode = devm_kzalloc(dev, sizeof(*altmode), GFP_KERNEL);
> -	if (!altmode)
> -		return -ENOMEM;
> -
> -	altmode->dev = dev;
> -
> -	match = of_match_device(pmic_glink_altmode_of_quirks, dev->parent);
> -	if (match)
> -		altmode->owner_id = (unsigned long)match->data;
> -	else
> -		altmode->owner_id = PMIC_GLINK_OWNER_USBC_PAN;
> -
> -	INIT_WORK(&altmode->enable_work, pmic_glink_altmode_enable_worker);
> -	init_completion(&altmode->pan_ack);
> -	mutex_init(&altmode->lock);
> -
> -	device_for_each_child_node(dev, fwnode) {
> -		ret = fwnode_property_read_u32(fwnode, "reg", &port);
> -		if (ret < 0) {
> -			dev_err(dev, "missing reg property of %pOFn\n", fwnode);
> -			fwnode_handle_put(fwnode);
> -			return ret;
> -		}
> -
> -		if (port >= ARRAY_SIZE(altmode->ports)) {
> -			dev_warn(dev, "invalid connector number, ignoring\n");
> -			continue;
> -		}
> -
> -		if (altmode->ports[port].altmode) {
> -			dev_err(dev, "multiple connector definition for port %u\n", port);
> -			fwnode_handle_put(fwnode);
> -			return -EINVAL;
> -		}
> -
> -		alt_port = &altmode->ports[port];
> -		alt_port->altmode = altmode;
> -		alt_port->index = port;
> -		INIT_WORK(&alt_port->work, pmic_glink_altmode_worker);
> -
> -		alt_port->bridge = devm_drm_dp_hpd_bridge_alloc(dev, to_of_node(fwnode));
> -		if (IS_ERR(alt_port->bridge)) {
> -			fwnode_handle_put(fwnode);
> -			return PTR_ERR(alt_port->bridge);
> -		}
> -
> -		alt_port->dp_alt.svid = USB_TYPEC_DP_SID;
> -		alt_port->dp_alt.mode = USB_TYPEC_DP_MODE;
> -		alt_port->dp_alt.active = 1;
> -
> -		alt_port->typec_mux = fwnode_typec_mux_get(fwnode);
> -		if (IS_ERR(alt_port->typec_mux)) {
> -			fwnode_handle_put(fwnode);
> -			return dev_err_probe(dev, PTR_ERR(alt_port->typec_mux),
> -					     "failed to acquire mode-switch for port: %d\n",
> -					     port);
> -		}
> -
> -		ret = devm_add_action_or_reset(dev, pmic_glink_altmode_put_mux,
> -					       alt_port->typec_mux);
> -		if (ret) {
> -			fwnode_handle_put(fwnode);
> -			return ret;
> -		}
> -
> -		alt_port->typec_retimer = fwnode_typec_retimer_get(fwnode);
> -		if (IS_ERR(alt_port->typec_retimer)) {
> -			fwnode_handle_put(fwnode);
> -			return dev_err_probe(dev, PTR_ERR(alt_port->typec_retimer),
> -					     "failed to acquire retimer-switch for port: %d\n",
> -					     port);
> -		}
> -
> -		ret = devm_add_action_or_reset(dev, pmic_glink_altmode_put_retimer,
> -					       alt_port->typec_retimer);
> -		if (ret) {
> -			fwnode_handle_put(fwnode);
> -			return ret;
> -		}
> -
> -		alt_port->typec_switch = fwnode_typec_switch_get(fwnode);
> -		if (IS_ERR(alt_port->typec_switch)) {
> -			fwnode_handle_put(fwnode);
> -			return dev_err_probe(dev, PTR_ERR(alt_port->typec_switch),
> -					     "failed to acquire orientation-switch for port: %d\n",
> -					     port);
> -		}
> -
> -		ret = devm_add_action_or_reset(dev, pmic_glink_altmode_put_switch,
> -					       alt_port->typec_switch);
> -		if (ret) {
> -			fwnode_handle_put(fwnode);
> -			return ret;
> -		}
> -	}
> -
> -	for (port = 0; port < ARRAY_SIZE(altmode->ports); port++) {
> -		alt_port = &altmode->ports[port];
> -		if (!alt_port->bridge)
> -			continue;
> -
> -		ret = devm_drm_dp_hpd_bridge_add(dev, alt_port->bridge);
> -		if (ret)
> -			return ret;
> -	}
> -
> -	altmode->client = devm_pmic_glink_register_client(dev,
> -							  altmode->owner_id,
> -							  pmic_glink_altmode_callback,
> -							  pmic_glink_altmode_pdr_notify,
> -							  altmode);
> -	return PTR_ERR_OR_ZERO(altmode->client);
> -}
> -
> -static const struct auxiliary_device_id pmic_glink_altmode_id_table[] = {
> -	{ .name = "pmic_glink.altmode", },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(auxiliary, pmic_glink_altmode_id_table);
> -
> -static struct auxiliary_driver pmic_glink_altmode_driver = {
> -	.name = "pmic_glink_altmode",
> -	.probe = pmic_glink_altmode_probe,
> -	.id_table = pmic_glink_altmode_id_table,
> -};
> -
> -module_auxiliary_driver(pmic_glink_altmode_driver);
> -
> -MODULE_DESCRIPTION("Qualcomm PMIC GLINK Altmode driver");
> -MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
> index 40fcda055b05..1ef638d5fd79 100644
> --- a/drivers/usb/typec/ucsi/ucsi_glink.c
> +++ b/drivers/usb/typec/ucsi/ucsi_glink.c
> @@ -10,9 +10,14 @@
>  #include <linux/of_device.h>
>  #include <linux/property.h>
>  #include <linux/soc/qcom/pdr.h>
> +#include <linux/usb/pd_vdo.h>
> +#include <linux/usb/typec_dp.h>
>  #include <linux/usb/typec_mux.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/soc/qcom/pmic_glink.h>
> +
> +#include <drm/bridge/aux-bridge.h>
> +
>  #include "ucsi.h"
>  
>  #define PMIC_GLINK_MAX_PORTS	2
> @@ -27,6 +32,16 @@
>  #define UC_UCSI_WRITE_BUF_REQ           0x12
>  #define UC_UCSI_USBC_NOTIFY_IND         0x13
>  
> +/*
> + * On sc8180x these requests use UCSI owner,
> + * on other platforms they use USBC_PAN.
> + */
> +#define USBC_CMD_WRITE_REQ		0x15
> +#define USBC_PAN_NOTIFY_IND		0x16
> +
> +#define ALTMODE_PAN_EN		0x10
> +#define ALTMODE_PAN_ACK		0x11
> +
>  struct ucsi_read_buf_req_msg {
>  	struct pmic_glink_hdr   hdr;
>  };
> @@ -55,17 +70,89 @@ struct ucsi_notify_ind_msg {
>  	u32                     reserved;
>  };
>  
> +struct usbc_write_req_msg {
> +	struct pmic_glink_hdr   hdr;
> +	__le32			cmd;
> +	__le32			arg;
> +	u32                     reserved;
> +};
> +
> +#define USBC_NOTIFY_PAYLOAD_SIZE 16
> +struct usbc_pan_notify_ind_msg {
> +	struct pmic_glink_hdr	hdr;
> +	char			payload[USBC_NOTIFY_PAYLOAD_SIZE];
> +	u32			reserved;
> +};
> +
> +enum pmic_glink_ucsi_orientation {
> +	USBC_ORIENTATION_NORMAL,
> +	USBC_ORIENTATION_REVERSE,
> +	USBC_ORIENTATION_NONE,
> +};
> +
> +enum pmic_glink_ucsi_mux {
> +	USBC_MUX_NONE,
> +	USBC_MUX_USB_2L,
> +	USBC_MUX_DP_4L,
> +	USBC_MUX_USB_DP,
> +};
> +
> +enum pmic_glink_altmode_pin_assignment {
> +	DPAM_HPD_OUT,
> +	DPAM_HPD_A,
> +	DPAM_HPD_B,
> +	DPAM_HPD_C,
> +	DPAM_HPD_D,
> +	DPAM_HPD_E,
> +	DPAM_HPD_F,
> +};
> +
> +#define SC8180X_PORT_MASK		GENMASK(7, 0)
> +#define SC8180X_ORIENTATION_MASK	GENMASK(15, 8)
> +#define SC8180X_MUX_MASK		GENMASK(23, 16)
> +#define SC8180X_MODE_MASK		GENMASK(29, 24)
> +#define SC8180X_HPD_STATE_MASK		BIT(30)
> +#define SC8180X_HPD_IRQ_MASK		BIT(31)
> +
> +#define SC8280XP_DPAM_MASK		GENMASK(5, 0)
> +#define SC8280XP_HPD_STATE_MASK		BIT(6)
> +#define SC8280XP_HPD_IRQ_MASK		BIT(7)
> +
> +struct pmic_glink_ucsi_port {
> +	spinlock_t lock;
> +
> +	struct work_struct altmode_work;
> +
> +	struct pmic_glink_ucsi *ucsi;
> +	struct gpio_desc *port_orientation;
> +	struct auxiliary_device *bridge;
> +
> +	int idx;
> +
> +	enum typec_orientation orientation;
> +
> +	enum pmic_glink_ucsi_mux mux;
> +	unsigned int mode;
> +
> +	u16 svid;
> +	u8 hpd_state;
> +	u8 hpd_irq;
> +};
> +
>  struct pmic_glink_ucsi {
>  	struct device *dev;
>  
> -	struct gpio_desc *port_orientation[PMIC_GLINK_MAX_PORTS];
> +	struct pmic_glink_ucsi_port ports[PMIC_GLINK_MAX_PORTS];
>  
> +	unsigned int altmode_id;
> +	struct pmic_glink_client *altmode_client;
>  	struct pmic_glink_client *client;
>  
>  	struct ucsi *ucsi;
>  	struct completion read_ack;
>  	struct completion write_ack;
>  	struct completion sync_ack;
> +	struct completion pan_ack;
>  	bool sync_pending;
>  	struct mutex lock;	/* protects concurrent access to PMIC Glink interface */
>  
> @@ -193,27 +280,128 @@ static void pmic_glink_ucsi_update_connector(struct ucsi_connector *con)
>  	int i;
>  
>  	for (i = 0; i < PMIC_GLINK_MAX_PORTS; i++) {
> -		if (ucsi->port_orientation[i])
> +		if (ucsi->ports[i].port_orientation)
>  			con->typec_cap.orientation_aware = true;
>  	}
>  }
>  
> +static int pmic_glink_altmode_request(struct pmic_glink_ucsi *ucsi, u32 cmd, u32 arg)
> +{
> +	struct usbc_write_req_msg req = {};
> +	unsigned long left;
> +	int ret;
> +
> +	/*
> +	 * The USBC_CMD_WRITE_REQ ack doesn't identify the request, so wait for
> +	 * one ack at a time.
> +	 */
> +	mutex_lock(&ucsi->lock);
> +
> +	req.hdr.owner = cpu_to_le32(ucsi->altmode_id);
> +	req.hdr.type = cpu_to_le32(PMIC_GLINK_REQ_RESP);
> +	req.hdr.opcode = cpu_to_le32(USBC_CMD_WRITE_REQ);
> +	req.cmd = cpu_to_le32(cmd);
> +	req.arg = cpu_to_le32(arg);
> +
> +	reinit_completion(&ucsi->pan_ack);
> +	ret = pmic_glink_send(ucsi->altmode_client, &req, sizeof(req));
> +	if (ret) {
> +		dev_err(ucsi->dev, "failed to send altmode request: %#x (%d)\n", cmd, ret);
> +		goto out_unlock;
> +	}
> +
> +	left = wait_for_completion_timeout(&ucsi->pan_ack, 5 * HZ);
> +	if (!left) {
> +		dev_err(ucsi->dev, "timeout waiting for altmode request ack for: %#x\n", cmd);
> +		ret = -ETIMEDOUT;
> +	}
> +
> +out_unlock:
> +	mutex_unlock(&ucsi->lock);
> +	return ret;
> +}
> +
> +static void pmic_glink_ucsi_set_orientation(struct ucsi_connector *con,
> +					    struct pmic_glink_ucsi_port *port)
> +{
> +	enum typec_orientation orientation;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +	orientation = port->orientation;
> +	spin_unlock_irqrestore(&port->lock, flags);
> +
> +	if (port->port_orientation) {
> +		int val = gpiod_get_value(port->port_orientation);
> +		if (val >= 0)
> +			orientation = val ?
> +				TYPEC_ORIENTATION_REVERSE :
> +				TYPEC_ORIENTATION_NORMAL;
> +	}
> +
> +	typec_set_orientation(con->port, orientation);
> +}
> +
>  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])
> +	if (con->num >= PMIC_GLINK_MAX_PORTS)
>  		return;
>  
> -	orientation = gpiod_get_value(ucsi->port_orientation[con->num - 1]);
> -	if (orientation >= 0) {
> -		typec_set_orientation(con->port,
> -				      orientation ?
> -				      TYPEC_ORIENTATION_REVERSE :
> -				      TYPEC_ORIENTATION_NORMAL);
> -	}
> +	pmic_glink_ucsi_set_orientation(con, &ucsi->ports[con->num - 1]);
> +}
> +
> +static void pmic_glink_ucsi_register_altmode(struct ucsi_connector *con)
> +{
> +	static const u8 all_assignments = BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D) |
> +			     BIT(DP_PIN_ASSIGN_E);
> +	struct typec_altmode_desc desc;
> +	struct typec_altmode *alt;
> +
> +	mutex_lock(&con->lock);
> +
> +	if (con->port_altmode[0])
> +		goto out;
> +
> +	memset(&desc, 0, sizeof(desc));
> +	desc.svid = USB_TYPEC_DP_SID;
> +	desc.mode = USB_TYPEC_DP_MODE;
> +
> +	desc.vdo = DP_CAP_CAPABILITY(DP_CAP_DFP_D);
> +
> +	/* We can't rely on the firmware with the capabilities. */
> +	desc.vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> +
> +	/* Claiming that we support all pin assignments */
> +	desc.vdo |= all_assignments << 8;
> +	desc.vdo |= all_assignments << 16;
> +
> +	alt = typec_port_register_altmode(con->port, &desc);

        alt = ucsi_register_displayport(con, 0, 0, &desc);

You need to export that function, but that should not be a problem:

diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index d9d3c91125ca..f2754d7b5876 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -315,11 +315,13 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
        struct ucsi_dp *dp;
 
        /* We can't rely on the firmware with the capabilities. */
-       desc->vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
+       if (!desc->vdo) {
+               desc->vdo = DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
 
-       /* Claiming that we support all pin assignments */
-       desc->vdo |= all_assignments << 8;
-       desc->vdo |= all_assignments << 16;
+               /* Claiming that we support all pin assignments */
+               desc->vdo |= all_assignments << 8;
+               desc->vdo |= all_assignments << 16;
+       }
 
        alt = typec_port_register_altmode(con->port, desc);
        if (IS_ERR(alt))
@@ -342,3 +344,4 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
 
        return alt;
 }
+EXPORT_SYMBOL_GPL(ucsi_register_displayport);

<snip>

> +static void pmic_glink_ucsi_set_state(struct ucsi_connector *con,
> +				      struct pmic_glink_ucsi_port *port)
> +{
> +	struct typec_displayport_data dp_data = {};
> +	struct typec_altmode *altmode = NULL;
> +	unsigned long flags;
> +	void *data = NULL;
> +	int mode;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +
> +	if (port->svid == USB_SID_PD) {
> +		mode = TYPEC_STATE_USB;
> +	} else if (port->svid == USB_TYPEC_DP_SID && port->mode == DPAM_HPD_OUT) {
> +		mode = TYPEC_STATE_SAFE;
> +	} else if (port->svid == USB_TYPEC_DP_SID) {
> +		altmode = find_altmode(con, port->svid);
> +		if (!altmode) {
> +			dev_err(con->ucsi->dev, "altmode woth SVID 0x%04x not found\n",
> +				port->svid);
> +			spin_unlock_irqrestore(&port->lock, flags);
> +			return;
> +		}
> +
> +		mode = TYPEC_MODAL_STATE(port->mode - DPAM_HPD_A);
> +
> +		dp_data.status = DP_STATUS_ENABLED;
> +		dp_data.status |= DP_STATUS_CON_DFP_D;
> +		if (port->hpd_state)
> +			dp_data.status |= DP_STATUS_HPD_STATE;
> +		if (port->hpd_irq)
> +			dp_data.status |= DP_STATUS_IRQ_HPD;
> +		dp_data.conf = DP_CONF_SET_PIN_ASSIGN(port->mode - DPAM_HPD_A);
> +
> +		data = &dp_data;
> +	} else {
> +		dev_err(con->ucsi->dev, "Unsupported SVID 0x%04x\n", port->svid);
> +		spin_unlock_irqrestore(&port->lock, flags);
> +		return;
> +	}
> +
> +	spin_unlock_irqrestore(&port->lock, flags);
> +
> +	if (altmode)
> +		typec_altmode_set_port(altmode, mode, data);

So if the port altmode is using the ucsi_displayport_ops, you can
simply register the partner altmode here instead. That should
guarantee that it'll bind to the DP altmode driver which will take
care of typec_altmode_enter() etc.

This will also allow the user space to see the altmode normally in
sysfs.

> +	else
> +		typec_set_mode(con->port, mode);
> +
> +	if (port->bridge)
> +		drm_aux_hpd_bridge_notify(&port->bridge->dev,
> +					  port->hpd_state ?
> +					  connector_status_connected :
> +					  connector_status_disconnected);
> +
> +}

thanks,

-- 
heikki

Re: [PATCH 7/8] usb: typec: ucsi: glink: merge pmic_glink_altmode driver

[Dmitry Baryshkov]

On Mon, Apr 22, 2024 at 01:59:10PM +0300, Heikki Krogerus wrote:
> Hi Dmitry,
> 
> On Tue, Apr 16, 2024 at 05:20:56AM +0300, Dmitry Baryshkov wrote:
> > Move handling of USB Altmode to the ucsi_glink driver. This way the
> > altmode is properly registered in the Type-C framework, the altmode
> > handlers can use generic typec calls, the UCSI driver can use
> > orientation information from altmode messages and vice versa, the
> > altmode handlers can use GPIO-based orientation inormation from UCSI
> > GLINK driver.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/soc/qcom/Makefile             |   1 -
> >  drivers/soc/qcom/pmic_glink_altmode.c | 546 ----------------------------------
> >  drivers/usb/typec/ucsi/ucsi_glink.c   | 495 ++++++++++++++++++++++++++++--
> >  3 files changed, 475 insertions(+), 567 deletions(-)
> > 

[skipped the patch]

> > +
> > +static void pmic_glink_ucsi_register_altmode(struct ucsi_connector *con)
> > +{
> > +	static const u8 all_assignments = BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D) |
> > +			     BIT(DP_PIN_ASSIGN_E);
> > +	struct typec_altmode_desc desc;
> > +	struct typec_altmode *alt;
> > +
> > +	mutex_lock(&con->lock);
> > +
> > +	if (con->port_altmode[0])
> > +		goto out;
> > +
> > +	memset(&desc, 0, sizeof(desc));
> > +	desc.svid = USB_TYPEC_DP_SID;
> > +	desc.mode = USB_TYPEC_DP_MODE;
> > +
> > +	desc.vdo = DP_CAP_CAPABILITY(DP_CAP_DFP_D);
> > +
> > +	/* We can't rely on the firmware with the capabilities. */
> > +	desc.vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> > +
> > +	/* Claiming that we support all pin assignments */
> > +	desc.vdo |= all_assignments << 8;
> > +	desc.vdo |= all_assignments << 16;
> > +
> > +	alt = typec_port_register_altmode(con->port, &desc);
> 
>         alt = ucsi_register_displayport(con, 0, 0, &desc);

Note, the existing UCSI displayport AltMode driver depends on the UCSI
actually handling the altomode. It needs a partner, etc.

> 
> You need to export that function, but that should not be a problem:
> 
> diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
> index d9d3c91125ca..f2754d7b5876 100644
> --- a/drivers/usb/typec/ucsi/displayport.c
> +++ b/drivers/usb/typec/ucsi/displayport.c
> @@ -315,11 +315,13 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
>         struct ucsi_dp *dp;
>  
>         /* We can't rely on the firmware with the capabilities. */
> -       desc->vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> +       if (!desc->vdo) {
> +               desc->vdo = DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
>  
> -       /* Claiming that we support all pin assignments */
> -       desc->vdo |= all_assignments << 8;
> -       desc->vdo |= all_assignments << 16;
> +               /* Claiming that we support all pin assignments */
> +               desc->vdo |= all_assignments << 8;
> +               desc->vdo |= all_assignments << 16;
> +       }
>  
>         alt = typec_port_register_altmode(con->port, desc);
>         if (IS_ERR(alt))
> @@ -342,3 +344,4 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
>  
>         return alt;
>  }
> +EXPORT_SYMBOL_GPL(ucsi_register_displayport);
> 
> <snip>
> 
> > +static void pmic_glink_ucsi_set_state(struct ucsi_connector *con,
> > +				      struct pmic_glink_ucsi_port *port)
> > +{
> > +	struct typec_displayport_data dp_data = {};
> > +	struct typec_altmode *altmode = NULL;
> > +	unsigned long flags;
> > +	void *data = NULL;
> > +	int mode;
> > +
> > +	spin_lock_irqsave(&port->lock, flags);
> > +
> > +	if (port->svid == USB_SID_PD) {
> > +		mode = TYPEC_STATE_USB;
> > +	} else if (port->svid == USB_TYPEC_DP_SID && port->mode == DPAM_HPD_OUT) {
> > +		mode = TYPEC_STATE_SAFE;
> > +	} else if (port->svid == USB_TYPEC_DP_SID) {
> > +		altmode = find_altmode(con, port->svid);
> > +		if (!altmode) {
> > +			dev_err(con->ucsi->dev, "altmode woth SVID 0x%04x not found\n",
> > +				port->svid);
> > +			spin_unlock_irqrestore(&port->lock, flags);
> > +			return;
> > +		}
> > +
> > +		mode = TYPEC_MODAL_STATE(port->mode - DPAM_HPD_A);
> > +
> > +		dp_data.status = DP_STATUS_ENABLED;
> > +		dp_data.status |= DP_STATUS_CON_DFP_D;
> > +		if (port->hpd_state)
> > +			dp_data.status |= DP_STATUS_HPD_STATE;
> > +		if (port->hpd_irq)
> > +			dp_data.status |= DP_STATUS_IRQ_HPD;
> > +		dp_data.conf = DP_CONF_SET_PIN_ASSIGN(port->mode - DPAM_HPD_A);
> > +
> > +		data = &dp_data;
> > +	} else {
> > +		dev_err(con->ucsi->dev, "Unsupported SVID 0x%04x\n", port->svid);
> > +		spin_unlock_irqrestore(&port->lock, flags);
> > +		return;
> > +	}
> > +
> > +	spin_unlock_irqrestore(&port->lock, flags);
> > +
> > +	if (altmode)
> > +		typec_altmode_set_port(altmode, mode, data);
> 
> So if the port altmode is using the ucsi_displayport_ops, you can
> simply register the partner altmode here instead. That should
> guarantee that it'll bind to the DP altmode driver which will take
> care of typec_altmode_enter() etc.

In our case the altmode is unfortunately completely hidden inside the
firmware. It is not exported via the native UCSI interface. Even if I
plug the DP dongle, there is no partner / altmode being reported by the
PPM. All DP events are reported via additional GLINK messages.

The goal is to use the core Type-C altmode handling, while keeping UCSI
out of the altmode business.

This allows the core to handle switches / muxes / retimers, report the
altmode to the userspace via sysfs, keep the link between the DP part of
the stack and the typec port, but at the same time we don't get errors
from UCSI because of the PPM reporting unsupported commands, etc.

> 
> This will also allow the user space to see the altmode normally in
> sysfs.
> 
> > +	else
> > +		typec_set_mode(con->port, mode);
> > +
> > +	if (port->bridge)
> > +		drm_aux_hpd_bridge_notify(&port->bridge->dev,
> > +					  port->hpd_state ?
> > +					  connector_status_connected :
> > +					  connector_status_disconnected);
> > +
> > +}
> 
> thanks,
> 
> -- 
> heikki

-- 
With best wishes
Dmitry

Re: [PATCH 7/8] usb: typec: ucsi: glink: merge pmic_glink_altmode driver

[Heikki Krogerus]

Hi Dmitry,

On Mon, Apr 22, 2024 at 03:45:22PM +0300, Dmitry Baryshkov wrote:
> On Mon, Apr 22, 2024 at 01:59:10PM +0300, Heikki Krogerus wrote:
> > Hi Dmitry,
> > 
> > On Tue, Apr 16, 2024 at 05:20:56AM +0300, Dmitry Baryshkov wrote:
> > > Move handling of USB Altmode to the ucsi_glink driver. This way the
> > > altmode is properly registered in the Type-C framework, the altmode
> > > handlers can use generic typec calls, the UCSI driver can use
> > > orientation information from altmode messages and vice versa, the
> > > altmode handlers can use GPIO-based orientation inormation from UCSI
> > > GLINK driver.
> > > 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  drivers/soc/qcom/Makefile             |   1 -
> > >  drivers/soc/qcom/pmic_glink_altmode.c | 546 ----------------------------------
> > >  drivers/usb/typec/ucsi/ucsi_glink.c   | 495 ++++++++++++++++++++++++++++--
> > >  3 files changed, 475 insertions(+), 567 deletions(-)
> > > 
> 
> [skipped the patch]
> 
> > > +
> > > +static void pmic_glink_ucsi_register_altmode(struct ucsi_connector *con)
> > > +{
> > > +	static const u8 all_assignments = BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D) |
> > > +			     BIT(DP_PIN_ASSIGN_E);
> > > +	struct typec_altmode_desc desc;
> > > +	struct typec_altmode *alt;
> > > +
> > > +	mutex_lock(&con->lock);
> > > +
> > > +	if (con->port_altmode[0])
> > > +		goto out;
> > > +
> > > +	memset(&desc, 0, sizeof(desc));
> > > +	desc.svid = USB_TYPEC_DP_SID;
> > > +	desc.mode = USB_TYPEC_DP_MODE;
> > > +
> > > +	desc.vdo = DP_CAP_CAPABILITY(DP_CAP_DFP_D);
> > > +
> > > +	/* We can't rely on the firmware with the capabilities. */
> > > +	desc.vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> > > +
> > > +	/* Claiming that we support all pin assignments */
> > > +	desc.vdo |= all_assignments << 8;
> > > +	desc.vdo |= all_assignments << 16;
> > > +
> > > +	alt = typec_port_register_altmode(con->port, &desc);
> > 
> >         alt = ucsi_register_displayport(con, 0, 0, &desc);
> 
> Note, the existing UCSI displayport AltMode driver depends on the UCSI
> actually handling the altomode. It needs a partner, etc.
> 
> > You need to export that function, but that should not be a problem:
> > 
> > diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
> > index d9d3c91125ca..f2754d7b5876 100644
> > --- a/drivers/usb/typec/ucsi/displayport.c
> > +++ b/drivers/usb/typec/ucsi/displayport.c
> > @@ -315,11 +315,13 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
> >         struct ucsi_dp *dp;
> >  
> >         /* We can't rely on the firmware with the capabilities. */
> > -       desc->vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> > +       if (!desc->vdo) {
> > +               desc->vdo = DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> >  
> > -       /* Claiming that we support all pin assignments */
> > -       desc->vdo |= all_assignments << 8;
> > -       desc->vdo |= all_assignments << 16;
> > +               /* Claiming that we support all pin assignments */
> > +               desc->vdo |= all_assignments << 8;
> > +               desc->vdo |= all_assignments << 16;
> > +       }
> >  
> >         alt = typec_port_register_altmode(con->port, desc);
> >         if (IS_ERR(alt))
> > @@ -342,3 +344,4 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
> >  
> >         return alt;
> >  }
> > +EXPORT_SYMBOL_GPL(ucsi_register_displayport);
> > 
> > <snip>
> > 
> > > +static void pmic_glink_ucsi_set_state(struct ucsi_connector *con,
> > > +				      struct pmic_glink_ucsi_port *port)
> > > +{
> > > +	struct typec_displayport_data dp_data = {};
> > > +	struct typec_altmode *altmode = NULL;
> > > +	unsigned long flags;
> > > +	void *data = NULL;
> > > +	int mode;
> > > +
> > > +	spin_lock_irqsave(&port->lock, flags);
> > > +
> > > +	if (port->svid == USB_SID_PD) {
> > > +		mode = TYPEC_STATE_USB;
> > > +	} else if (port->svid == USB_TYPEC_DP_SID && port->mode == DPAM_HPD_OUT) {
> > > +		mode = TYPEC_STATE_SAFE;
> > > +	} else if (port->svid == USB_TYPEC_DP_SID) {
> > > +		altmode = find_altmode(con, port->svid);
> > > +		if (!altmode) {
> > > +			dev_err(con->ucsi->dev, "altmode woth SVID 0x%04x not found\n",
> > > +				port->svid);
> > > +			spin_unlock_irqrestore(&port->lock, flags);
> > > +			return;
> > > +		}
> > > +
> > > +		mode = TYPEC_MODAL_STATE(port->mode - DPAM_HPD_A);
> > > +
> > > +		dp_data.status = DP_STATUS_ENABLED;
> > > +		dp_data.status |= DP_STATUS_CON_DFP_D;
> > > +		if (port->hpd_state)
> > > +			dp_data.status |= DP_STATUS_HPD_STATE;
> > > +		if (port->hpd_irq)
> > > +			dp_data.status |= DP_STATUS_IRQ_HPD;
> > > +		dp_data.conf = DP_CONF_SET_PIN_ASSIGN(port->mode - DPAM_HPD_A);
> > > +
> > > +		data = &dp_data;
> > > +	} else {
> > > +		dev_err(con->ucsi->dev, "Unsupported SVID 0x%04x\n", port->svid);
> > > +		spin_unlock_irqrestore(&port->lock, flags);
> > > +		return;
> > > +	}
> > > +
> > > +	spin_unlock_irqrestore(&port->lock, flags);
> > > +
> > > +	if (altmode)
> > > +		typec_altmode_set_port(altmode, mode, data);
> > 
> > So if the port altmode is using the ucsi_displayport_ops, you can
> > simply register the partner altmode here instead. That should
> > guarantee that it'll bind to the DP altmode driver which will take
> > care of typec_altmode_enter() etc.
> 
> In our case the altmode is unfortunately completely hidden inside the
> firmware. It is not exported via the native UCSI interface. Even if I
> plug the DP dongle, there is no partner / altmode being reported by the
> PPM. All DP events are reported via additional GLINK messages.

I understand that there is no alt mode being reported, but I assumed
that there is a notification about connections.

If that's not the case, then you need to use this code path to
register the partner device as well I think. The partner really has to
be registered somehow.

> The goal is to use the core Type-C altmode handling, while keeping UCSI
> out of the altmode business.
> 
> This allows the core to handle switches / muxes / retimers, report the
> altmode to the userspace via sysfs, keep the link between the DP part of
> the stack and the typec port, but at the same time we don't get errors
> from UCSI because of the PPM reporting unsupported commands, etc.

I understand, and just to be clear, I don't have a problem with
bypassing UCSI. But that does not mean you can skip the alt mode
registration.

The primary purpose of drivers/usb/typec/ucsi/displayport.c is to
emulate the partner DP alt mode device a little so that the actual DP
alt mode driver drivers/usb/typec/altmodes/displayport.c is happy. The
altmode driver will then make sure that all the muxes, switches and
what have you, are configured as they should, and more importantly,
make sure the DP alt mode is exposed to the user space exactly the
same way as it's exposed on all the other systems.

There are a couple of UCSI commands that are being used there yes, but
by modifying it so that those UCSI commands are executed conditionally
- by checking the ALT_MODE_DETAILS feature - you should be able to use
it also in this case.

You really need to register the partner alt mode(s) one way or the
other in any case, and the partner device itself you absolutely must
register. The user space interface needs to be consistent.

thanks,

-- 
heikki

Re: [PATCH 7/8] usb: typec: ucsi: glink: merge pmic_glink_altmode driver

[Dmitry Baryshkov]

On Mon, 22 Apr 2024 at 18:02, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi Dmitry,
>
> On Mon, Apr 22, 2024 at 03:45:22PM +0300, Dmitry Baryshkov wrote:
> > On Mon, Apr 22, 2024 at 01:59:10PM +0300, Heikki Krogerus wrote:
> > > Hi Dmitry,
> > >
> > > On Tue, Apr 16, 2024 at 05:20:56AM +0300, Dmitry Baryshkov wrote:
> > > > Move handling of USB Altmode to the ucsi_glink driver. This way the
> > > > altmode is properly registered in the Type-C framework, the altmode
> > > > handlers can use generic typec calls, the UCSI driver can use
> > > > orientation information from altmode messages and vice versa, the
> > > > altmode handlers can use GPIO-based orientation inormation from UCSI
> > > > GLINK driver.
> > > >
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > ---
> > > >  drivers/soc/qcom/Makefile             |   1 -
> > > >  drivers/soc/qcom/pmic_glink_altmode.c | 546 ----------------------------------
> > > >  drivers/usb/typec/ucsi/ucsi_glink.c   | 495 ++++++++++++++++++++++++++++--
> > > >  3 files changed, 475 insertions(+), 567 deletions(-)
> > > >
> >
> > [skipped the patch]
> >
> > > > +
> > > > +static void pmic_glink_ucsi_register_altmode(struct ucsi_connector *con)
> > > > +{
> > > > + static const u8 all_assignments = BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D) |
> > > > +                      BIT(DP_PIN_ASSIGN_E);
> > > > + struct typec_altmode_desc desc;
> > > > + struct typec_altmode *alt;
> > > > +
> > > > + mutex_lock(&con->lock);
> > > > +
> > > > + if (con->port_altmode[0])
> > > > +         goto out;
> > > > +
> > > > + memset(&desc, 0, sizeof(desc));
> > > > + desc.svid = USB_TYPEC_DP_SID;
> > > > + desc.mode = USB_TYPEC_DP_MODE;
> > > > +
> > > > + desc.vdo = DP_CAP_CAPABILITY(DP_CAP_DFP_D);
> > > > +
> > > > + /* We can't rely on the firmware with the capabilities. */
> > > > + desc.vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> > > > +
> > > > + /* Claiming that we support all pin assignments */
> > > > + desc.vdo |= all_assignments << 8;
> > > > + desc.vdo |= all_assignments << 16;
> > > > +
> > > > + alt = typec_port_register_altmode(con->port, &desc);
> > >
> > >         alt = ucsi_register_displayport(con, 0, 0, &desc);
> >
> > Note, the existing UCSI displayport AltMode driver depends on the UCSI
> > actually handling the altomode. It needs a partner, etc.
> >
> > > You need to export that function, but that should not be a problem:
> > >
> > > diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
> > > index d9d3c91125ca..f2754d7b5876 100644
> > > --- a/drivers/usb/typec/ucsi/displayport.c
> > > +++ b/drivers/usb/typec/ucsi/displayport.c
> > > @@ -315,11 +315,13 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
> > >         struct ucsi_dp *dp;
> > >
> > >         /* We can't rely on the firmware with the capabilities. */
> > > -       desc->vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> > > +       if (!desc->vdo) {
> > > +               desc->vdo = DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> > >
> > > -       /* Claiming that we support all pin assignments */
> > > -       desc->vdo |= all_assignments << 8;
> > > -       desc->vdo |= all_assignments << 16;
> > > +               /* Claiming that we support all pin assignments */
> > > +               desc->vdo |= all_assignments << 8;
> > > +               desc->vdo |= all_assignments << 16;
> > > +       }
> > >
> > >         alt = typec_port_register_altmode(con->port, desc);
> > >         if (IS_ERR(alt))
> > > @@ -342,3 +344,4 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
> > >
> > >         return alt;
> > >  }
> > > +EXPORT_SYMBOL_GPL(ucsi_register_displayport);
> > >
> > > <snip>
> > >
> > > > +static void pmic_glink_ucsi_set_state(struct ucsi_connector *con,
> > > > +                               struct pmic_glink_ucsi_port *port)
> > > > +{
> > > > + struct typec_displayport_data dp_data = {};
> > > > + struct typec_altmode *altmode = NULL;
> > > > + unsigned long flags;
> > > > + void *data = NULL;
> > > > + int mode;
> > > > +
> > > > + spin_lock_irqsave(&port->lock, flags);
> > > > +
> > > > + if (port->svid == USB_SID_PD) {
> > > > +         mode = TYPEC_STATE_USB;
> > > > + } else if (port->svid == USB_TYPEC_DP_SID && port->mode == DPAM_HPD_OUT) {
> > > > +         mode = TYPEC_STATE_SAFE;
> > > > + } else if (port->svid == USB_TYPEC_DP_SID) {
> > > > +         altmode = find_altmode(con, port->svid);
> > > > +         if (!altmode) {
> > > > +                 dev_err(con->ucsi->dev, "altmode woth SVID 0x%04x not found\n",
> > > > +                         port->svid);
> > > > +                 spin_unlock_irqrestore(&port->lock, flags);
> > > > +                 return;
> > > > +         }
> > > > +
> > > > +         mode = TYPEC_MODAL_STATE(port->mode - DPAM_HPD_A);
> > > > +
> > > > +         dp_data.status = DP_STATUS_ENABLED;
> > > > +         dp_data.status |= DP_STATUS_CON_DFP_D;
> > > > +         if (port->hpd_state)
> > > > +                 dp_data.status |= DP_STATUS_HPD_STATE;
> > > > +         if (port->hpd_irq)
> > > > +                 dp_data.status |= DP_STATUS_IRQ_HPD;
> > > > +         dp_data.conf = DP_CONF_SET_PIN_ASSIGN(port->mode - DPAM_HPD_A);
> > > > +
> > > > +         data = &dp_data;
> > > > + } else {
> > > > +         dev_err(con->ucsi->dev, "Unsupported SVID 0x%04x\n", port->svid);
> > > > +         spin_unlock_irqrestore(&port->lock, flags);
> > > > +         return;
> > > > + }
> > > > +
> > > > + spin_unlock_irqrestore(&port->lock, flags);
> > > > +
> > > > + if (altmode)
> > > > +         typec_altmode_set_port(altmode, mode, data);
> > >
> > > So if the port altmode is using the ucsi_displayport_ops, you can
> > > simply register the partner altmode here instead. That should
> > > guarantee that it'll bind to the DP altmode driver which will take
> > > care of typec_altmode_enter() etc.
> >
> > In our case the altmode is unfortunately completely hidden inside the
> > firmware. It is not exported via the native UCSI interface. Even if I
> > plug the DP dongle, there is no partner / altmode being reported by the
> > PPM. All DP events are reported via additional GLINK messages.
>
> I understand that there is no alt mode being reported, but I assumed
> that there is a notification about connections.

Yes, there is a notification.

>
> If that's not the case, then you need to use this code path to
> register the partner device as well I think. The partner really has to
> be registered somehow.
>
> > The goal is to use the core Type-C altmode handling, while keeping UCSI
> > out of the altmode business.
> >
> > This allows the core to handle switches / muxes / retimers, report the
> > altmode to the userspace via sysfs, keep the link between the DP part of
> > the stack and the typec port, but at the same time we don't get errors
> > from UCSI because of the PPM reporting unsupported commands, etc.
>
> I understand, and just to be clear, I don't have a problem with
> bypassing UCSI. But that does not mean you can skip the alt mode
> registration.
>
> The primary purpose of drivers/usb/typec/ucsi/displayport.c is to
> emulate the partner DP alt mode device a little so that the actual DP
> alt mode driver drivers/usb/typec/altmodes/displayport.c is happy. The
> altmode driver will then make sure that all the muxes, switches and
> what have you, are configured as they should, and more importantly,
> make sure the DP alt mode is exposed to the user space exactly the
> same way as it's exposed on all the other systems.

Ack. I'll take a look at implementing it this way. If it works, then
it becomes even easier.

A bit of justification from my side. I was comparing this
implementation with the Lenovo p53s laptop. Running 6.7 kernel, I see
two Type-C ports. They register altmodes, etc. However for the DP
partner (Lenovo USB-C dock) I only get the partner device, there are
no altmodes of the partner. /sys/bus/typec/devices/ is empty. The DP
works perfectly despite not having the typec device. But maybe it's
just some i915's extension or platform hack.

> There are a couple of UCSI commands that are being used there yes, but
> by modifying it so that those UCSI commands are executed conditionally
> - by checking the ALT_MODE_DETAILS feature - you should be able to use
> it also in this case.
>
> You really need to register the partner alt mode(s) one way or the
> other in any case, and the partner device itself you absolutely must
> register. The user space interface needs to be consistent.

Ack

-- 
With best wishes
Dmitry

Re: [PATCH 7/8] usb: typec: ucsi: glink: merge pmic_glink_altmode driver

[Dmitry Baryshkov]

On Mon, 22 Apr 2024 at 18:02, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi Dmitry,
>
> On Mon, Apr 22, 2024 at 03:45:22PM +0300, Dmitry Baryshkov wrote:
> > On Mon, Apr 22, 2024 at 01:59:10PM +0300, Heikki Krogerus wrote:
> > > Hi Dmitry,
> > >
> > > On Tue, Apr 16, 2024 at 05:20:56AM +0300, Dmitry Baryshkov wrote:
> > > > Move handling of USB Altmode to the ucsi_glink driver. This way the
> > > > altmode is properly registered in the Type-C framework, the altmode
> > > > handlers can use generic typec calls, the UCSI driver can use
> > > > orientation information from altmode messages and vice versa, the
> > > > altmode handlers can use GPIO-based orientation inormation from UCSI
> > > > GLINK driver.
> > > >
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > ---
> > > >  drivers/soc/qcom/Makefile             |   1 -
> > > >  drivers/soc/qcom/pmic_glink_altmode.c | 546 ----------------------------------
> > > >  drivers/usb/typec/ucsi/ucsi_glink.c   | 495 ++++++++++++++++++++++++++++--
> > > >  3 files changed, 475 insertions(+), 567 deletions(-)
> > > >
> >
> > [skipped the patch]
> >
> > > > +
> > > > +static void pmic_glink_ucsi_register_altmode(struct ucsi_connector *con)
> > > > +{
> > > > + static const u8 all_assignments = BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D) |
> > > > +                      BIT(DP_PIN_ASSIGN_E);
> > > > + struct typec_altmode_desc desc;
> > > > + struct typec_altmode *alt;
> > > > +
> > > > + mutex_lock(&con->lock);
> > > > +
> > > > + if (con->port_altmode[0])
> > > > +         goto out;
> > > > +
> > > > + memset(&desc, 0, sizeof(desc));
> > > > + desc.svid = USB_TYPEC_DP_SID;
> > > > + desc.mode = USB_TYPEC_DP_MODE;
> > > > +
> > > > + desc.vdo = DP_CAP_CAPABILITY(DP_CAP_DFP_D);
> > > > +
> > > > + /* We can't rely on the firmware with the capabilities. */
> > > > + desc.vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> > > > +
> > > > + /* Claiming that we support all pin assignments */
> > > > + desc.vdo |= all_assignments << 8;
> > > > + desc.vdo |= all_assignments << 16;
> > > > +
> > > > + alt = typec_port_register_altmode(con->port, &desc);
> > >
> > >         alt = ucsi_register_displayport(con, 0, 0, &desc);
> >
> > Note, the existing UCSI displayport AltMode driver depends on the UCSI
> > actually handling the altomode. It needs a partner, etc.
> >
> > > You need to export that function, but that should not be a problem:
> > >
> > > diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
> > > index d9d3c91125ca..f2754d7b5876 100644
> > > --- a/drivers/usb/typec/ucsi/displayport.c
> > > +++ b/drivers/usb/typec/ucsi/displayport.c
> > > @@ -315,11 +315,13 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
> > >         struct ucsi_dp *dp;
> > >
> > >         /* We can't rely on the firmware with the capabilities. */
> > > -       desc->vdo |= DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> > > +       if (!desc->vdo) {
> > > +               desc->vdo = DP_CAP_DP_SIGNALLING(0) | DP_CAP_RECEPTACLE;
> > >
> > > -       /* Claiming that we support all pin assignments */
> > > -       desc->vdo |= all_assignments << 8;
> > > -       desc->vdo |= all_assignments << 16;
> > > +               /* Claiming that we support all pin assignments */
> > > +               desc->vdo |= all_assignments << 8;
> > > +               desc->vdo |= all_assignments << 16;
> > > +       }
> > >
> > >         alt = typec_port_register_altmode(con->port, desc);
> > >         if (IS_ERR(alt))
> > > @@ -342,3 +344,4 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con,
> > >
> > >         return alt;
> > >  }
> > > +EXPORT_SYMBOL_GPL(ucsi_register_displayport);
> > >
> > > <snip>
> > >
> > > > +static void pmic_glink_ucsi_set_state(struct ucsi_connector *con,
> > > > +                               struct pmic_glink_ucsi_port *port)
> > > > +{
> > > > + struct typec_displayport_data dp_data = {};
> > > > + struct typec_altmode *altmode = NULL;
> > > > + unsigned long flags;
> > > > + void *data = NULL;
> > > > + int mode;
> > > > +
> > > > + spin_lock_irqsave(&port->lock, flags);
> > > > +
> > > > + if (port->svid == USB_SID_PD) {
> > > > +         mode = TYPEC_STATE_USB;
> > > > + } else if (port->svid == USB_TYPEC_DP_SID && port->mode == DPAM_HPD_OUT) {
> > > > +         mode = TYPEC_STATE_SAFE;
> > > > + } else if (port->svid == USB_TYPEC_DP_SID) {
> > > > +         altmode = find_altmode(con, port->svid);
> > > > +         if (!altmode) {
> > > > +                 dev_err(con->ucsi->dev, "altmode woth SVID 0x%04x not found\n",
> > > > +                         port->svid);
> > > > +                 spin_unlock_irqrestore(&port->lock, flags);
> > > > +                 return;
> > > > +         }
> > > > +
> > > > +         mode = TYPEC_MODAL_STATE(port->mode - DPAM_HPD_A);
> > > > +
> > > > +         dp_data.status = DP_STATUS_ENABLED;
> > > > +         dp_data.status |= DP_STATUS_CON_DFP_D;
> > > > +         if (port->hpd_state)
> > > > +                 dp_data.status |= DP_STATUS_HPD_STATE;
> > > > +         if (port->hpd_irq)
> > > > +                 dp_data.status |= DP_STATUS_IRQ_HPD;
> > > > +         dp_data.conf = DP_CONF_SET_PIN_ASSIGN(port->mode - DPAM_HPD_A);
> > > > +
> > > > +         data = &dp_data;
> > > > + } else {
> > > > +         dev_err(con->ucsi->dev, "Unsupported SVID 0x%04x\n", port->svid);
> > > > +         spin_unlock_irqrestore(&port->lock, flags);
> > > > +         return;
> > > > + }
> > > > +
> > > > + spin_unlock_irqrestore(&port->lock, flags);
> > > > +
> > > > + if (altmode)
> > > > +         typec_altmode_set_port(altmode, mode, data);
> > >
> > > So if the port altmode is using the ucsi_displayport_ops, you can
> > > simply register the partner altmode here instead. That should
> > > guarantee that it'll bind to the DP altmode driver which will take
> > > care of typec_altmode_enter() etc.
> >
> > In our case the altmode is unfortunately completely hidden inside the
> > firmware. It is not exported via the native UCSI interface. Even if I
> > plug the DP dongle, there is no partner / altmode being reported by the
> > PPM. All DP events are reported via additional GLINK messages.
>
> I understand that there is no alt mode being reported, but I assumed
> that there is a notification about connections.
>
> If that's not the case, then you need to use this code path to
> register the partner device as well I think. The partner really has to
> be registered somehow.
>
> > The goal is to use the core Type-C altmode handling, while keeping UCSI
> > out of the altmode business.
> >
> > This allows the core to handle switches / muxes / retimers, report the
> > altmode to the userspace via sysfs, keep the link between the DP part of
> > the stack and the typec port, but at the same time we don't get errors
> > from UCSI because of the PPM reporting unsupported commands, etc.
>
> I understand, and just to be clear, I don't have a problem with
> bypassing UCSI. But that does not mean you can skip the alt mode
> registration.
>
> The primary purpose of drivers/usb/typec/ucsi/displayport.c is to
> emulate the partner DP alt mode device a little so that the actual DP
> alt mode driver drivers/usb/typec/altmodes/displayport.c is happy. The
> altmode driver will then make sure that all the muxes, switches and
> what have you, are configured as they should, and more importantly,
> make sure the DP alt mode is exposed to the user space exactly the
> same way as it's exposed on all the other systems.
>
> There are a couple of UCSI commands that are being used there yes, but
> by modifying it so that those UCSI commands are executed conditionally
> - by checking the ALT_MODE_DETAILS feature - you should be able to use
> it also in this case.

I have played with the DP AltMode driver. I got it somewhat working,
but I think I'm facing a control issue.
Basically, the altmode driver wants to control pin assignment on its
own. It works with the software TCPM, as we control it.
It works with the normal UCSI, because it still can configure pin
config. However with PMIC GLINK implementation there is no way to
control pin assignment from the Linux side. The firmware does that for
us.
What would be the recommended way to handle it? Is it okay to override
status_update to return just the selected pin config? Or is there any
other (better) way to handle such an issue?

>
> You really need to register the partner alt mode(s) one way or the
> other in any case, and the partner device itself you absolutely must
> register. The user space interface needs to be consistent.

For reference, the partner is being reported and registered by the
UCSI firmware. It's only the altmode itself where I'm facing the
issue.

-- 
With best wishes
Dmitry

Re: [PATCH 7/8] usb: typec: ucsi: glink: merge pmic_glink_altmode driver

[Dmitry Baryshkov]

Hi Heikki,


On Sat, May 04, 2024 at 09:49:42AM +0300, Dmitry Baryshkov wrote:
> On Mon, 22 Apr 2024 at 18:02, Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > Hi Dmitry,
> >
> > On Mon, Apr 22, 2024 at 03:45:22PM +0300, Dmitry Baryshkov wrote:
> > > On Mon, Apr 22, 2024 at 01:59:10PM +0300, Heikki Krogerus wrote:
> > > > Hi Dmitry,
> > > >
> > > > On Tue, Apr 16, 2024 at 05:20:56AM +0300, Dmitry Baryshkov wrote:
> > > > > Move handling of USB Altmode to the ucsi_glink driver. This way the
> > > > > altmode is properly registered in the Type-C framework, the altmode
> > > > > handlers can use generic typec calls, the UCSI driver can use
> > > > > orientation information from altmode messages and vice versa, the
> > > > > altmode handlers can use GPIO-based orientation inormation from UCSI
> > > > > GLINK driver.
> > > > >

[skipped]

> > > Note, the existing UCSI displayport AltMode driver depends on the UCSI
> > > actually handling the altomode. It needs a partner, etc.
> > >

[skipped the patch]

> > > > > +static void pmic_glink_ucsi_set_state(struct ucsi_connector *con,
> > > > > +                               struct pmic_glink_ucsi_port *port)
> > > > > +{
> > > > > + struct typec_displayport_data dp_data = {};
> > > > > + struct typec_altmode *altmode = NULL;
> > > > > + unsigned long flags;
> > > > > + void *data = NULL;
> > > > > + int mode;
> > > > > +
> > > > > + spin_lock_irqsave(&port->lock, flags);
> > > > > +
> > > > > + if (port->svid == USB_SID_PD) {
> > > > > +         mode = TYPEC_STATE_USB;
> > > > > + } else if (port->svid == USB_TYPEC_DP_SID && port->mode == DPAM_HPD_OUT) {
> > > > > +         mode = TYPEC_STATE_SAFE;
> > > > > + } else if (port->svid == USB_TYPEC_DP_SID) {
> > > > > +         altmode = find_altmode(con, port->svid);
> > > > > +         if (!altmode) {
> > > > > +                 dev_err(con->ucsi->dev, "altmode woth SVID 0x%04x not found\n",
> > > > > +                         port->svid);
> > > > > +                 spin_unlock_irqrestore(&port->lock, flags);
> > > > > +                 return;
> > > > > +         }
> > > > > +
> > > > > +         mode = TYPEC_MODAL_STATE(port->mode - DPAM_HPD_A);
> > > > > +
> > > > > +         dp_data.status = DP_STATUS_ENABLED;
> > > > > +         dp_data.status |= DP_STATUS_CON_DFP_D;
> > > > > +         if (port->hpd_state)
> > > > > +                 dp_data.status |= DP_STATUS_HPD_STATE;
> > > > > +         if (port->hpd_irq)
> > > > > +                 dp_data.status |= DP_STATUS_IRQ_HPD;
> > > > > +         dp_data.conf = DP_CONF_SET_PIN_ASSIGN(port->mode - DPAM_HPD_A);
> > > > > +
> > > > > +         data = &dp_data;
> > > > > + } else {
> > > > > +         dev_err(con->ucsi->dev, "Unsupported SVID 0x%04x\n", port->svid);
> > > > > +         spin_unlock_irqrestore(&port->lock, flags);
> > > > > +         return;
> > > > > + }
> > > > > +
> > > > > + spin_unlock_irqrestore(&port->lock, flags);
> > > > > +
> > > > > + if (altmode)
> > > > > +         typec_altmode_set_port(altmode, mode, data);
> > > >
> > > > So if the port altmode is using the ucsi_displayport_ops, you can
> > > > simply register the partner altmode here instead. That should
> > > > guarantee that it'll bind to the DP altmode driver which will take
> > > > care of typec_altmode_enter() etc.
> > >
> > > In our case the altmode is unfortunately completely hidden inside the
> > > firmware. It is not exported via the native UCSI interface. Even if I
> > > plug the DP dongle, there is no partner / altmode being reported by the
> > > PPM. All DP events are reported via additional GLINK messages.
> >
> > I understand that there is no alt mode being reported, but I assumed
> > that there is a notification about connections.
> >
> > If that's not the case, then you need to use this code path to
> > register the partner device as well I think. The partner really has to
> > be registered somehow.
> >
> > > The goal is to use the core Type-C altmode handling, while keeping UCSI
> > > out of the altmode business.
> > >
> > > This allows the core to handle switches / muxes / retimers, report the
> > > altmode to the userspace via sysfs, keep the link between the DP part of
> > > the stack and the typec port, but at the same time we don't get errors
> > > from UCSI because of the PPM reporting unsupported commands, etc.
> >
> > I understand, and just to be clear, I don't have a problem with
> > bypassing UCSI. But that does not mean you can skip the alt mode
> > registration.
> >
> > The primary purpose of drivers/usb/typec/ucsi/displayport.c is to
> > emulate the partner DP alt mode device a little so that the actual DP
> > alt mode driver drivers/usb/typec/altmodes/displayport.c is happy. The
> > altmode driver will then make sure that all the muxes, switches and
> > what have you, are configured as they should, and more importantly,
> > make sure the DP alt mode is exposed to the user space exactly the
> > same way as it's exposed on all the other systems.
> >
> > There are a couple of UCSI commands that are being used there yes, but
> > by modifying it so that those UCSI commands are executed conditionally
> > - by checking the ALT_MODE_DETAILS feature - you should be able to use
> > it also in this case.
> 
> I have played with the DP AltMode driver. I got it somewhat working,
> but I think I'm facing a control issue.
> Basically, the altmode driver wants to control pin assignment on its
> own. It works with the software TCPM, as we control it.
> It works with the normal UCSI, because it still can configure pin
> config. However with PMIC GLINK implementation there is no way to
> control pin assignment from the Linux side. The firmware does that for
> us.
> What would be the recommended way to handle it? Is it okay to override
> status_update to return just the selected pin config? Or is there any
> other (better) way to handle such an issue?

Any suggestions or further comments? Is it better to extend the
DisplayPort Altmode driver with the 'forced' transitions? Or it would be
fine to just register a partner device, emulate the userspace events,
but completely ignore the existing displayport driver?

> 
> >
> > You really need to register the partner alt mode(s) one way or the
> > other in any case, and the partner device itself you absolutely must
> > register. The user space interface needs to be consistent.
> 
> For reference, the partner is being reported and registered by the
> UCSI firmware. It's only the altmode itself where I'm facing the
> issue.

-- 
With best wishes
Dmitry

Re: [PATCH 7/8] usb: typec: ucsi: glink: merge pmic_glink_altmode driver

[Heikki Krogerus]

Hi Dmitry,

On Wed, May 15, 2024 at 06:01:48PM +0300, Dmitry Baryshkov wrote:
> > I have played with the DP AltMode driver. I got it somewhat working,
> > but I think I'm facing a control issue.
> > Basically, the altmode driver wants to control pin assignment on its
> > own. It works with the software TCPM, as we control it.
> > It works with the normal UCSI, because it still can configure pin
> > config. However with PMIC GLINK implementation there is no way to
> > control pin assignment from the Linux side. The firmware does that for
> > us.
> > What would be the recommended way to handle it? Is it okay to override
> > status_update to return just the selected pin config? Or is there any
> > other (better) way to handle such an issue?
> 
> Any suggestions or further comments? Is it better to extend the
> DisplayPort Altmode driver with the 'forced' transitions? Or it would be
> fine to just register a partner device, emulate the userspace events,
> but completely ignore the existing displayport driver?

I may have to look at the DP alt mode with the TI host interface
(drivers/usb/typec/tipd/) at some point, because on some systems the
firmware does not seem to automatically enter the mode, but the forced
transition option would probable work there as well...

I would prefer that we don't ignore the DP alt mode driver, but just
to be clear, I'm also not completely against it if you feel that would
be the most reasonable option in your case.

br,

-- 
heikki

[PATCH 8/8] soc: qcom: pmic-glink: drop separate altmode driver support

[Dmitry Baryshkov]

As the separate pmic_glink_altmode driver has been merged to the
UCSI glink glue driver and the UCSI is now enabled on all platforms,
drop separate altmode device support in the pmic_glink driver.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/soc/qcom/pmic_glink.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c
index dcea33f93fae..680954334b7f 100644
--- a/drivers/soc/qcom/pmic_glink.c
+++ b/drivers/soc/qcom/pmic_glink.c
@@ -14,7 +14,6 @@
 
 enum {
 	PMIC_GLINK_CLIENT_BATT = 0,
-	PMIC_GLINK_CLIENT_ALTMODE,
 	PMIC_GLINK_CLIENT_UCSI,
 };
 
@@ -26,7 +25,6 @@ struct pmic_glink {
 
 	unsigned long client_mask;
 
-	struct auxiliary_device altmode_aux;
 	struct auxiliary_device ps_aux;
 	struct auxiliary_device ucsi_aux;
 
@@ -294,15 +292,10 @@ static int pmic_glink_probe(struct platform_device *pdev)
 		if (ret)
 			goto out_release_pdr_handle;
 	}
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE)) {
-		ret = pmic_glink_add_aux_device(pg, &pg->altmode_aux, "altmode");
-		if (ret)
-			goto out_release_ucsi_aux;
-	}
 	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT)) {
 		ret = pmic_glink_add_aux_device(pg, &pg->ps_aux, "power-supply");
 		if (ret)
-			goto out_release_altmode_aux;
+			goto out_release_ucsi_aux;
 	}
 
 	service = pdr_add_lookup(pg->pdr, "tms/servreg", "msm/adsp/charger_pd");
@@ -321,9 +314,6 @@ static int pmic_glink_probe(struct platform_device *pdev)
 out_release_aux_devices:
 	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
 		pmic_glink_del_aux_device(pg, &pg->ps_aux);
-out_release_altmode_aux:
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE))
-		pmic_glink_del_aux_device(pg, &pg->altmode_aux);
 out_release_ucsi_aux:
 	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
 		pmic_glink_del_aux_device(pg, &pg->ucsi_aux);
@@ -341,8 +331,6 @@ static void pmic_glink_remove(struct platform_device *pdev)
 
 	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_BATT))
 		pmic_glink_del_aux_device(pg, &pg->ps_aux);
-	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_ALTMODE))
-		pmic_glink_del_aux_device(pg, &pg->altmode_aux);
 	if (pg->client_mask & BIT(PMIC_GLINK_CLIENT_UCSI))
 		pmic_glink_del_aux_device(pg, &pg->ucsi_aux);
 
@@ -352,7 +340,6 @@ static void pmic_glink_remove(struct platform_device *pdev)
 }
 
 static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) |
-							   BIT(PMIC_GLINK_CLIENT_ALTMODE) |
 							   BIT(PMIC_GLINK_CLIENT_UCSI);
 
 static const struct of_device_id pmic_glink_of_match[] = {

-- 
2.39.2