[PATCH v3 0/2] usb: typec: intel_pmc_mux: Port status from IOM microcontroller

[PATCH v3 0/2] usb: typec: intel_pmc_mux: Port status from IOM microcontroller

[Heikki Krogerus]

Hi,

This is the third version of the series. The original two were send by
Rajmohan. There is now no separate IOM driver. Instead the mux driver
simply reads the IOM registers directly.

This is link to the v2 of the series:
https://lore.kernel.org/linux-usb/20200822040508.23510-1-rajmohan.mani@intel.com/

thanks,

Heikki Krogerus (2):
  usb: typec: intel_pmc_mux: Check the port status before connect
  usb: typec: intel_pmc_mux: Support for device role (UFP)

 drivers/usb/typec/mux/intel_pmc_mux.c | 202 +++++++++++++++++++++-----
 1 file changed, 167 insertions(+), 35 deletions(-)

-- 
2.28.0

[PATCH v3 1/2] usb: typec: intel_pmc_mux: Check the port status before connect

[Heikki Krogerus]

The PMC microcontroller that we use for configuration, does
not supply any status information back. For port status we
need to talk to another controller on the board called IOM
(I/O manager).

By checking the port status before configuring the muxes, we
can make sure that we do not reconfigure the port after
bootup when the system firmware (for example BIOS) has
already configured it.

Using the status information also to check if DisplayPort
HPD is still asserted when the cable plug is disconnected,
and clearing it if it is.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Rajmohan Mani <rajmohan.mani@intel.com>
Reviewed-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
Tested-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
---
 drivers/usb/typec/mux/intel_pmc_mux.c | 152 ++++++++++++++++++++++++--
 1 file changed, 141 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
index 3bc08847fb7f0..4ee14423a2e3d 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -80,10 +80,48 @@ enum {
 #define PMC_USB_DP_HPD_LVL		BIT(4)
 #define PMC_USB_DP_HPD_IRQ		BIT(5)
 
+/*
+ * Input Output Manager (IOM) PORT STATUS
+ */
+#define IOM_PORT_STATUS_OFFSET				0x560
+
+#define IOM_PORT_STATUS_ACTIVITY_TYPE_MASK		GENMASK(9, 6)
+#define IOM_PORT_STATUS_ACTIVITY_TYPE_SHIFT		6
+#define IOM_PORT_STATUS_ACTIVITY_TYPE_USB		0x03
+/* activity type: Safe Mode */
+#define IOM_PORT_STATUS_ACTIVITY_TYPE_SAFE_MODE		0x04
+/* activity type: Display Port */
+#define IOM_PORT_STATUS_ACTIVITY_TYPE_DP		0x05
+/* activity type: Display Port Multi Function Device */
+#define IOM_PORT_STATUS_ACTIVITY_TYPE_DP_MFD		0x06
+/* activity type: Thunderbolt */
+#define IOM_PORT_STATUS_ACTIVITY_TYPE_TBT		0x07
+#define IOM_PORT_STATUS_ACTIVITY_TYPE_ALT_MODE_USB	0x0c
+#define IOM_PORT_STATUS_ACTIVITY_TYPE_ALT_MODE_TBT_USB	0x0d
+/* Upstream Facing Port Information */
+#define IOM_PORT_STATUS_UFP				BIT(10)
+/* Display Port Hot Plug Detect status */
+#define IOM_PORT_STATUS_DHPD_HPD_STATUS_MASK		GENMASK(13, 12)
+#define IOM_PORT_STATUS_DHPD_HPD_STATUS_SHIFT		12
+#define IOM_PORT_STATUS_DHPD_HPD_STATUS_ASSERT		0x01
+#define IOM_PORT_STATUS_DHPD_HPD_SOURCE_TBT		BIT(14)
+#define IOM_PORT_STATUS_CONNECTED			BIT(31)
+
+#define IOM_PORT_ACTIVITY_IS(_status_, _type_)				\
+	((((_status_) & IOM_PORT_STATUS_ACTIVITY_TYPE_MASK) >>		\
+	  IOM_PORT_STATUS_ACTIVITY_TYPE_SHIFT) ==			\
+	 (IOM_PORT_STATUS_ACTIVITY_TYPE_##_type_))
+
+#define IOM_PORT_HPD_ASSERTED(_status_)					\
+	((((_status_) & IOM_PORT_STATUS_DHPD_HPD_STATUS_MASK) >>	\
+	  IOM_PORT_STATUS_DHPD_HPD_STATUS_SHIFT) &			\
+	 IOM_PORT_STATUS_DHPD_HPD_STATUS_ASSERT)
+
 struct pmc_usb;
 
 struct pmc_usb_port {
 	int num;
+	u32 iom_status;
 	struct pmc_usb *pmc;
 	struct typec_mux *typec_mux;
 	struct typec_switch *typec_sw;
@@ -104,8 +142,16 @@ struct pmc_usb {
 	struct device *dev;
 	struct intel_scu_ipc_dev *ipc;
 	struct pmc_usb_port *port;
+	struct acpi_device *iom_adev;
+	void __iomem *iom_base;
 };
 
+static void update_port_status(struct pmc_usb_port *port)
+{
+	port->iom_status = readl(port->pmc->iom_base + IOM_PORT_STATUS_OFFSET +
+				 port->usb3_port * sizeof(u32));
+}
+
 static int sbu_orientation(struct pmc_usb_port *port)
 {
 	if (port->sbu_orientation)
@@ -142,18 +188,17 @@ static int pmc_usb_command(struct pmc_usb_port *port, u8 *msg, u32 len)
 }
 
 static int
-pmc_usb_mux_dp_hpd(struct pmc_usb_port *port, struct typec_mux_state *state)
+pmc_usb_mux_dp_hpd(struct pmc_usb_port *port, struct typec_displayport_data *dp)
 {
-	struct typec_displayport_data *data = state->data;
 	u8 msg[2] = { };
 
 	msg[0] = PMC_USB_DP_HPD;
 	msg[0] |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 
-	if (data->status & DP_STATUS_IRQ_HPD)
+	if (dp->status & DP_STATUS_IRQ_HPD)
 		msg[1] = PMC_USB_DP_HPD_IRQ;
 
-	if (data->status & DP_STATUS_HPD_STATE)
+	if (dp->status & DP_STATUS_HPD_STATE)
 		msg[1] |= PMC_USB_DP_HPD_LVL;
 
 	return pmc_usb_command(port, msg, sizeof(msg));
@@ -166,8 +211,18 @@ pmc_usb_mux_dp(struct pmc_usb_port *port, struct typec_mux_state *state)
 	struct altmode_req req = { };
 	int ret;
 
+	if (IOM_PORT_ACTIVITY_IS(port->iom_status, DP) ||
+	    IOM_PORT_ACTIVITY_IS(port->iom_status, DP_MFD)) {
+		if (IOM_PORT_HPD_ASSERTED(port->iom_status) &&
+		    (!(data->status & DP_STATUS_IRQ_HPD) &&
+		     data->status & DP_STATUS_HPD_STATE))
+			return 0;
+
+		return pmc_usb_mux_dp_hpd(port, state->data);
+	}
+
 	if (data->status & DP_STATUS_IRQ_HPD)
-		return pmc_usb_mux_dp_hpd(port, state);
+		return pmc_usb_mux_dp_hpd(port, state->data);
 
 	req.usage = PMC_USB_ALT_MODE;
 	req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
@@ -183,8 +238,8 @@ pmc_usb_mux_dp(struct pmc_usb_port *port, struct typec_mux_state *state)
 	if (ret)
 		return ret;
 
-	if (data->status & DP_STATUS_HPD_STATE)
-		return pmc_usb_mux_dp_hpd(port, state);
+	if (data->status & (DP_STATUS_IRQ_HPD | DP_STATUS_HPD_STATE))
+		return pmc_usb_mux_dp_hpd(port, state->data);
 
 	return 0;
 }
@@ -196,6 +251,10 @@ pmc_usb_mux_tbt(struct pmc_usb_port *port, struct typec_mux_state *state)
 	u8 cable_speed = TBT_CABLE_SPEED(data->cable_mode);
 	struct altmode_req req = { };
 
+	if (IOM_PORT_ACTIVITY_IS(port->iom_status, TBT) ||
+	    IOM_PORT_ACTIVITY_IS(port->iom_status, ALT_MODE_TBT_USB))
+		return 0;
+
 	req.usage = PMC_USB_ALT_MODE;
 	req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 	req.mode_type = PMC_USB_MODE_TYPE_TBT << PMC_USB_MODE_TYPE_SHIFT;
@@ -227,6 +286,10 @@ pmc_usb_mux_usb4(struct pmc_usb_port *port, struct typec_mux_state *state)
 	struct altmode_req req = { };
 	u8 cable_speed;
 
+	if (IOM_PORT_ACTIVITY_IS(port->iom_status, TBT) ||
+	    IOM_PORT_ACTIVITY_IS(port->iom_status, ALT_MODE_TBT_USB))
+		return 0;
+
 	req.usage = PMC_USB_ALT_MODE;
 	req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 	req.mode_type = PMC_USB_MODE_TYPE_TBT << PMC_USB_MODE_TYPE_SHIFT;
@@ -261,6 +324,9 @@ static int pmc_usb_mux_safe_state(struct pmc_usb_port *port)
 {
 	u8 msg;
 
+	if (IOM_PORT_ACTIVITY_IS(port->iom_status, SAFE_MODE))
+		return 0;
+
 	msg = PMC_USB_SAFE_MODE;
 	msg |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 
@@ -271,6 +337,9 @@ static int pmc_usb_connect(struct pmc_usb_port *port)
 {
 	u8 msg[2];
 
+	if (port->iom_status & IOM_PORT_STATUS_CONNECTED)
+		return 0;
+
 	msg[0] = PMC_USB_CONNECT;
 	msg[0] |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 
@@ -283,8 +352,16 @@ static int pmc_usb_connect(struct pmc_usb_port *port)
 
 static int pmc_usb_disconnect(struct pmc_usb_port *port)
 {
+	struct typec_displayport_data data = { };
 	u8 msg[2];
 
+	if (!(port->iom_status & IOM_PORT_STATUS_CONNECTED))
+		return 0;
+
+	/* Clear DisplayPort HPD if it's still asserted. */
+	if (IOM_PORT_HPD_ASSERTED(port->iom_status))
+		pmc_usb_mux_dp_hpd(port, &data);
+
 	msg[0] = PMC_USB_DISCONNECT;
 	msg[0] |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 
@@ -298,6 +375,8 @@ pmc_usb_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
 {
 	struct pmc_usb_port *port = typec_mux_get_drvdata(mux);
 
+	update_port_status(port);
+
 	if (port->orientation == TYPEC_ORIENTATION_NONE || port->role == USB_ROLE_NONE)
 		return 0;
 
@@ -333,8 +412,7 @@ static int pmc_usb_set_orientation(struct typec_switch *sw,
 {
 	struct pmc_usb_port *port = typec_switch_get_drvdata(sw);
 
-	if (port->orientation == orientation)
-		return 0;
+	update_port_status(port);
 
 	port->orientation = orientation;
 
@@ -352,8 +430,7 @@ static int pmc_usb_set_role(struct usb_role_switch *sw, enum usb_role role)
 {
 	struct pmc_usb_port *port = usb_role_switch_get_drvdata(sw);
 
-	if (port->role == role)
-		return 0;
+	update_port_status(port);
 
 	port->role = role;
 
@@ -438,6 +515,45 @@ static int pmc_usb_register_port(struct pmc_usb *pmc, int index,
 	return ret;
 }
 
+static int is_memory(struct acpi_resource *res, void *data)
+{
+	struct resource r;
+
+	return !acpi_dev_resource_memory(res, &r);
+}
+
+static int pmc_usb_probe_iom(struct pmc_usb *pmc)
+{
+	struct list_head resource_list;
+	struct resource_entry *rentry;
+	struct acpi_device *adev;
+	int ret;
+
+	adev = acpi_dev_get_first_match_dev("INTC1072", NULL, -1);
+	if (!adev)
+		return -ENODEV;
+
+	INIT_LIST_HEAD(&resource_list);
+	ret = acpi_dev_get_resources(adev, &resource_list, is_memory, NULL);
+	if (ret < 0)
+		return ret;
+
+	rentry = list_first_entry_or_null(&resource_list, struct resource_entry, node);
+	if (rentry)
+		pmc->iom_base = devm_ioremap_resource(pmc->dev, rentry->res);
+
+	acpi_dev_free_resource_list(&resource_list);
+
+	if (!pmc->iom_base) {
+		put_device(&adev->dev);
+		return -ENOMEM;
+	}
+
+	pmc->iom_adev = adev;
+
+	return 0;
+}
+
 static int pmc_usb_probe(struct platform_device *pdev)
 {
 	struct fwnode_handle *fwnode = NULL;
@@ -452,6 +568,12 @@ static int pmc_usb_probe(struct platform_device *pdev)
 	device_for_each_child_node(&pdev->dev, fwnode)
 		pmc->num_ports++;
 
+	/* The IOM microcontroller has a limitation of max 4 ports. */
+	if (pmc->num_ports > 4) {
+		dev_err(&pdev->dev, "driver limited to 4 ports\n");
+		return -ERANGE;
+	}
+
 	pmc->port = devm_kcalloc(&pdev->dev, pmc->num_ports,
 				 sizeof(struct pmc_usb_port), GFP_KERNEL);
 	if (!pmc->port)
@@ -463,6 +585,10 @@ static int pmc_usb_probe(struct platform_device *pdev)
 
 	pmc->dev = &pdev->dev;
 
+	ret = pmc_usb_probe_iom(pmc);
+	if (ret)
+		return ret;
+
 	/*
 	 * For every physical USB connector (USB2 and USB3 combo) there is a
 	 * child ACPI device node under the PMC mux ACPI device object.
@@ -487,6 +613,8 @@ static int pmc_usb_probe(struct platform_device *pdev)
 		typec_mux_unregister(pmc->port[i].typec_mux);
 	}
 
+	put_device(&pmc->iom_adev->dev);
+
 	return ret;
 }
 
@@ -500,6 +628,8 @@ static int pmc_usb_remove(struct platform_device *pdev)
 		typec_mux_unregister(pmc->port[i].typec_mux);
 	}
 
+	put_device(&pmc->iom_adev->dev);
+
 	return 0;
 }
 
-- 
2.28.0

[PATCH v3 2/2] usb: typec: intel_pmc_mux: Support for device role (UFP)

[Heikki Krogerus]

This adds support for device data role, and data role
swapping. The driver no longer relies on the cached role, as
it may not be valid (for example after bootup). Instead, the
role is always checked by readding the port status from IOM.

Note. After this, the orientation is always only cached, so
the driver does not support scenario where the role is set
before orientation. It means the typec drivers must always
set the orientation first before role.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Rajmohan Mani <rajmohan.mani@intel.com>
Reviewed-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
Tested-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
---
 drivers/usb/typec/mux/intel_pmc_mux.c | 68 ++++++++++++++-------------
 1 file changed, 35 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
index 4ee14423a2e3d..067313af79708 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -221,9 +221,6 @@ pmc_usb_mux_dp(struct pmc_usb_port *port, struct typec_mux_state *state)
 		return pmc_usb_mux_dp_hpd(port, state->data);
 	}
 
-	if (data->status & DP_STATUS_IRQ_HPD)
-		return pmc_usb_mux_dp_hpd(port, state->data);
-
 	req.usage = PMC_USB_ALT_MODE;
 	req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 	req.mode_type = PMC_USB_MODE_TYPE_DP << PMC_USB_MODE_TYPE_SHIFT;
@@ -333,39 +330,52 @@ static int pmc_usb_mux_safe_state(struct pmc_usb_port *port)
 	return pmc_usb_command(port, &msg, sizeof(msg));
 }
 
-static int pmc_usb_connect(struct pmc_usb_port *port)
+static int pmc_usb_disconnect(struct pmc_usb_port *port)
 {
+	struct typec_displayport_data data = { };
 	u8 msg[2];
 
-	if (port->iom_status & IOM_PORT_STATUS_CONNECTED)
+	if (!(port->iom_status & IOM_PORT_STATUS_CONNECTED))
 		return 0;
 
-	msg[0] = PMC_USB_CONNECT;
+	/* Clear DisplayPort HPD if it's still asserted. */
+	if (IOM_PORT_HPD_ASSERTED(port->iom_status))
+		pmc_usb_mux_dp_hpd(port, &data);
+
+	msg[0] = PMC_USB_DISCONNECT;
 	msg[0] |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 
 	msg[1] = port->usb2_port << PMC_USB_MSG_USB2_PORT_SHIFT;
-	msg[1] |= hsl_orientation(port) << PMC_USB_MSG_ORI_HSL_SHIFT;
-	msg[1] |= sbu_orientation(port) << PMC_USB_MSG_ORI_AUX_SHIFT;
 
 	return pmc_usb_command(port, msg, sizeof(msg));
 }
 
-static int pmc_usb_disconnect(struct pmc_usb_port *port)
+static int pmc_usb_connect(struct pmc_usb_port *port, enum usb_role role)
 {
-	struct typec_displayport_data data = { };
+	u8 ufp = role == USB_ROLE_DEVICE ? 1 : 0;
 	u8 msg[2];
+	int ret;
 
-	if (!(port->iom_status & IOM_PORT_STATUS_CONNECTED))
-		return 0;
+	if (port->orientation == TYPEC_ORIENTATION_NONE)
+		return -EINVAL;
 
-	/* Clear DisplayPort HPD if it's still asserted. */
-	if (IOM_PORT_HPD_ASSERTED(port->iom_status))
-		pmc_usb_mux_dp_hpd(port, &data);
+	if (port->iom_status & IOM_PORT_STATUS_CONNECTED) {
+		if (port->role == role || port->role == USB_ROLE_NONE)
+			return 0;
 
-	msg[0] = PMC_USB_DISCONNECT;
+		/* Role swap */
+		ret = pmc_usb_disconnect(port);
+		if (ret)
+			return ret;
+	}
+
+	msg[0] = PMC_USB_CONNECT;
 	msg[0] |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
 
 	msg[1] = port->usb2_port << PMC_USB_MSG_USB2_PORT_SHIFT;
+	msg[1] |= ufp << PMC_USB_MSG_UFP_SHIFT;
+	msg[1] |= hsl_orientation(port) << PMC_USB_MSG_ORI_HSL_SHIFT;
+	msg[1] |= sbu_orientation(port) << PMC_USB_MSG_ORI_AUX_SHIFT;
 
 	return pmc_usb_command(port, msg, sizeof(msg));
 }
@@ -383,7 +393,7 @@ pmc_usb_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
 	if (state->mode == TYPEC_STATE_SAFE)
 		return pmc_usb_mux_safe_state(port);
 	if (state->mode == TYPEC_STATE_USB)
-		return pmc_usb_connect(port);
+		return pmc_usb_connect(port, port->role);
 
 	if (state->alt) {
 		switch (state->alt->svid) {
@@ -398,7 +408,7 @@ pmc_usb_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
 			/* REVISIT: Try with usb3_port set to 0? */
 			break;
 		case TYPEC_MODE_USB3:
-			return pmc_usb_connect(port);
+			return pmc_usb_connect(port, port->role);
 		case TYPEC_MODE_USB4:
 			return pmc_usb_mux_usb4(port, state);
 		}
@@ -416,32 +426,24 @@ static int pmc_usb_set_orientation(struct typec_switch *sw,
 
 	port->orientation = orientation;
 
-	if (port->role) {
-		if (orientation == TYPEC_ORIENTATION_NONE)
-			return pmc_usb_disconnect(port);
-		else
-			return pmc_usb_connect(port);
-	}
-
 	return 0;
 }
 
 static int pmc_usb_set_role(struct usb_role_switch *sw, enum usb_role role)
 {
 	struct pmc_usb_port *port = usb_role_switch_get_drvdata(sw);
+	int ret;
 
 	update_port_status(port);
 
-	port->role = role;
+	if (role == USB_ROLE_NONE)
+		ret = pmc_usb_disconnect(port);
+	else
+		ret = pmc_usb_connect(port, role);
 
-	if (port->orientation) {
-		if (role == USB_ROLE_NONE)
-			return pmc_usb_disconnect(port);
-		else
-			return pmc_usb_connect(port);
-	}
+	port->role = role;
 
-	return 0;
+	return ret;
 }
 
 static int pmc_usb_register_port(struct pmc_usb *pmc, int index,
-- 
2.28.0

Re: [PATCH v3 0/2] usb: typec: intel_pmc_mux: Port status from IOM microcontroller

[Greg Kroah-Hartman]

On Mon, Sep 07, 2020 at 05:24:26PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> This is the third version of the series. The original two were send by
> Rajmohan. There is now no separate IOM driver. Instead the mux driver
> simply reads the IOM registers directly.

Much cleaner, or at the very least, self-contained :)

thanks for the changes,

greg k-h