Please ignore the last email, I sent the wrong archive.

On Mon, Jan 6, 2025 at 11:33 AM Dmitry Baryshkov [off-list ref] wrote:

On Sun, Dec 29, 2024 at 05:05:47PM +0800, Pengyu Luo wrote:

quoted

On Sun, Dec 29, 2024 at 12:40 PM Dmitry Baryshkov [off-list ref] wrote:

quoted

On Sat, Dec 28, 2024 at 01:13:51AM +0800, Pengyu Luo wrote:

quoted

The Huawei Matebook E Go (sc8280xp) tablet provides implements UCSI
interface in the onboard EC. Add the glue driver to interface the
platform's UCSI implementation.

Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
 drivers/usb/typec/ucsi/Kconfig              |   9 +
 drivers/usb/typec/ucsi/Makefile             |   1 +
 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c | 481 ++++++++++++++++++++
 3 files changed, 491 insertions(+)
 create mode 100644 drivers/usb/typec/ucsi/ucsi_huawei_gaokun.c

diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
index 680e1b87b..0d0f07488 100644
--- a/drivers/usb/typec/ucsi/Kconfig
+++ b/drivers/usb/typec/ucsi/Kconfig

@@ -78,4 +78,13 @@ config UCSI_LENOVO_YOGA_C630
        To compile the driver as a module, choose M here: the module will be
        called ucsi_yoga_c630.

[...]

quoted

quoted

quoted

+
+     spin_lock_irqsave(&port->lock, flags);
+
+     port->ccx = FIELD_GET(GAOKUN_CCX_MASK, dcc);
+     port->mux = FIELD_GET(GAOKUN_MUX_MASK, dcc);
+     port->mode = FIELD_GET(GAOKUN_DPAM_MASK, ddi);
+     port->hpd_state = FIELD_GET(GAOKUN_HPD_STATE_MASK, ddi);
+     port->hpd_irq = FIELD_GET(GAOKUN_HPD_IRQ_MASK, ddi);
+
+     switch (port->mux) {
+     case USBC_MUX_NONE:
+             port->svid = 0;
+             break;
+     case USBC_MUX_USB_2L:
+             port->svid = USB_SID_PD;
+             break;
+     case USBC_MUX_DP_4L:
+     case USBC_MUX_USB_DP:
+             port->svid = USB_SID_DISPLAYPORT;
+             if (port->ccx == USBC_CCX_REVERSE)
+                     port->mode -= 6;

I'd prefer it this were more explicit about what is happening.

If orientation is reverse, then we should minus 6, EC's logic.
I will add a comment for it. Actually, this field is unused, I don't
find the mux yet, so I cannot set it with this field. But I don't want
to make things imcomplete, so keep it.

Which values are you expecting / getting there? The -6 is a pure magic.
Please replace this with a switch-case or something more obvious.

In v2, I have deduced their meaning, with a switch to map them.

quoted

Let me go off the topic, on my device, I can just use drm_aux_hpd_bridge_notify
to enable altmode, usb functions well after I pluged out, I don't need set mode
switch(orientation switch is required if orientation is reverse), which is quiet
similar to Acer aspire 1. Is mux controlled also by QMP combo phy(see [1])?

quoted

quoted

+             break;
+     default:
+             break;
+     }
+
+     spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static int gaokun_ucsi_refresh(struct gaokun_ucsi *uec)
+{
+     struct gaokun_ucsi_reg ureg;
+     int ret, idx;
+
+     ret = gaokun_ec_ucsi_get_reg(uec->ec, (u8 *)&ureg);
+     if (ret)
+             return -EIO;
+
+     uec->port_num = ureg.port_num;
+     idx = GET_IDX(ureg.port_updt);
+
+     if (idx >= 0 && idx < ureg.port_num)
+             gaokun_ucsi_port_update(&uec->ports[idx], ureg.port_data);
+
+     return idx;
+}
+
+static void gaokun_ucsi_handle_altmode(struct gaokun_ucsi_port *port)
+{
+     struct gaokun_ucsi *uec = port->ucsi;
+     int idx = port->idx;
+
+     if (idx >= uec->ucsi->cap.num_connectors || !uec->ucsi->connector) {
+             dev_warn(uec->ucsi->dev, "altmode port out of range: %d\n", idx);
+             return;
+     }
+
+     /* UCSI callback .connector_status() have set orientation */
+     if (port->bridge)
+             drm_aux_hpd_bridge_notify(&port->bridge->dev,
+                                       port->hpd_state ?
+                                       connector_status_connected :
+                                       connector_status_disconnected);

Does your platform report any altmodes? What do you see in
/sys/class/typec/port0/port0.*/ ?

/sys/class/typec/port0/port0.0:
active  mode  mode1  power  svid  uevent  vdo

/sys/class/typec/port0/port0.1:
active  mode  mode1  power  svid  uevent  vdo

/sys/class/typec/port0/port0.2:
active  mode  mode1  power  svid  uevent  vdo

/sys/class/typec/port0/port0.3:
active  mode  mode2  power  svid  uevent  vdo

/sys/class/typec/port0/port0.4:
active  mode  mode3  power  svid  uevent  vdo

please:

cat /sys/class/typec/port0/port0*/svid
cat /sys/class/typec/port0/port0*/vdo

svid:
8087
ff01
12d1
12d1
12d1

vdo:
0xff000001
0xff1c1c46
0xff000001
0xff000002
0xff000003

If DP is reported as one the altmodes, then it should be using the
DisplayPort AltMode driver, as suggested by Heikki.

But this paltform cannot access to the partner device, related API
requires a partner.

BTW, it is unnecessary that implementing/call a DP Altmode driver for
this platform. Currently, we can enter altmode with a HPD event notify.
This point is quiet similar to Acer aspire 1. I mentioned this when we
last talked about minus 6.

quoted

quoted

quoted

+
+     gaokun_ec_ucsi_pan_ack(uec->ec, port->idx);
+}
+
+static void gaokun_ucsi_altmode_notify_ind(struct gaokun_ucsi *uec)
+{
+     int idx;
+
+     idx = gaokun_ucsi_refresh(uec);
+     if (idx < 0)
+             gaokun_ec_ucsi_pan_ack(uec->ec, idx);
+     else
+             gaokun_ucsi_handle_altmode(&uec->ports[idx]);
+}
+
+/*
+ * USB event is necessary for enabling altmode, the event should follow
+ * UCSI event, if not after timeout(this notify may be disabled somehow),
+ * then force to enable altmode.
+ */
+static void gaokun_ucsi_handle_no_usb_event(struct gaokun_ucsi *uec, int idx)
+{
+     struct gaokun_ucsi_port *port;
+
+     port = &uec->ports[idx];
+     if (!wait_for_completion_timeout(&port->usb_ack, 2 * HZ)) {
+             dev_warn(uec->dev, "No USB EVENT, triggered by UCSI EVENT");
+             gaokun_ucsi_altmode_notify_ind(uec);
+     }
+}
+

[...]

quoted

quoted

quoted

+
+static void gaokun_ucsi_register_worker(struct work_struct *work)
+{
+     struct gaokun_ucsi *uec;
+     struct ucsi *ucsi;
+     int ret;
+
+     uec = container_of(work, struct gaokun_ucsi, work);
+     ucsi = uec->ucsi;
+
+     ucsi->quirks = UCSI_NO_PARTNER_PDOS | UCSI_DELAY_DEVICE_PDOS;

Does it crash in the same way as GLINK crashes (as you've set
UCSI_NO_PARTNER_PDOS)?

Yes, no partner can be detected, I checked. I think it is also handled by
the firmware As you said in [2]

quoted

In some obscure cases (Qualcomm PMIC Glink) altmode is completely
handled by the firmware. Linux does not get proper partner altmode info.

This is a separate topic. Those two flags were added for a very
particular reason:

- To workaround firmware crash on requesting PDOs for a partner
- To delay requeting PDOs for the device because in the unconnected
  state the GET_PDOS returns incorrect information

Are you sure that those two flags are necessary for your platform?

Alright, I think I got things mixed up. Actually PDO requires UCSI only,
not a partner device.

I think I will remove it in v3 if it works well during the time. Rencetly,
this platform works well without it. Thanks for pointing out.

quoted

quoted

quoted

+
+     ssleep(3); /* EC can't handle UCSI properly in the early stage */
+
+     ret = gaokun_ec_register_notify(uec->ec, &uec->nb);
+     if (ret) {
+             dev_err_probe(ucsi->dev, ret, "notifier register failed\n");
+             return;
+     }
+
+     ret = ucsi_register(ucsi);
+     if (ret)
+             dev_err_probe(ucsi->dev, ret, "ucsi register failed\n");
+}
+
+static int gaokun_ucsi_register(struct gaokun_ucsi *uec)

Please inline

I see.

Best wishes
Pengyu

[1] https://elixir.bootlin.com/linux/v6.12.5/source/drivers/phy/qualcomm/phy-qcom-qmp-combo.c#L2679
[2] https://lore.kernel.org/lkml/20240416-ucsi-glink-altmode-v1-0-890db00877ac@linaro.org (local)

Best Wishes,
Pengyu