[PATCH 0/5] USB Type-C fixes

[PATCH 0/5] USB Type-C fixes

[Heikki Krogerus]

Hi,

There are two fixes for the UCSI drivers that address issues related
to the alternate modes. The other three fixes are for the PMC mux
driver. We were missing dependency on ACPI and the port number that
was used with the IOM was incorrect.

Let me know if you want anything to be changed.

thanks,

Azhar Shaikh (1):
  usb: typec: intel_pmc_mux: Pass correct USB Type-C port number to SoC

Heikki Krogerus (3):
  usb: typec: ucsi: acpi: Increase command completion timeout value
  usb: typec: ucsi: Prevent mode overrun
  usb: typec: intel_pmc_mux: Add dependency on ACPI

Madhusudanarao Amara (1):
  usb: typec: intel_pmc_mux: Handle SCU IPC error conditions

 drivers/usb/typec/mux/Kconfig         |  1 +
 drivers/usb/typec/mux/intel_pmc_mux.c | 19 +++++++++++++++----
 drivers/usb/typec/ucsi/ucsi.c         | 22 ++++++++++++++++------
 drivers/usb/typec/ucsi/ucsi_acpi.c    |  2 +-
 4 files changed, 33 insertions(+), 11 deletions(-)

-- 
2.28.0

[PATCH 1/5] usb: typec: ucsi: acpi: Increase command completion timeout value

[Heikki Krogerus]

UCSI specification quite clearly states that if a command
can't be completed in 10ms, the firmware must notify
about BUSY condition. Unfortunately almost none of the
platforms (the firmware on them) generate the BUSY
notification even if a command can't be completed in time.

The driver already considered that, and used a timeout
value of 5 seconds, but processing especially the alternate
mode discovery commands takes often considerable amount of
time from the firmware, much more than the 5 seconds. That
happens especially after bootup when devices are already
connected to the USB Type-C connector. For now on those
platforms the alternate mode discovery has simply failed
because of the timeout.

To improve the situation, increasing the timeout value for
the command completion to 1 minute. That should give enough
time for even the slowest firmware to process the commands.

Fixes: f56de278e8ec ("usb: typec: ucsi: acpi: Move to the new API")
Cc: stable@vger.kernel.org
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/ucsi/ucsi_acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index c0aca2f0f23f0..fbfe8f5933af8 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -78,7 +78,7 @@ static int ucsi_acpi_sync_write(struct ucsi *ucsi, unsigned int offset,
 	if (ret)
 		goto out_clear_bit;
 
-	if (!wait_for_completion_timeout(&ua->complete, msecs_to_jiffies(5000)))
+	if (!wait_for_completion_timeout(&ua->complete, 60 * HZ))
 		ret = -ETIMEDOUT;
 
 out_clear_bit:
-- 
2.28.0

[PATCH 2/5] usb: typec: ucsi: Prevent mode overrun

[Heikki Krogerus]

Sometimes the embedded controller firmware does not
terminate the list of alternate modes that the partner
supports in its response to the GET_ALTERNATE_MODES command.
Instead the firmware returns the supported alternate modes
over and over again until the driver stops requesting them.

If that happens, the number of modes for each alternate mode
will exceed the maximum 6 that is defined in the USB Power
Delivery specification. Making sure that can't happen by
adding a check for it.

This fixes NULL pointer dereference that is caused by the
overrun.

Fixes: ad74b8649beaf ("usb: typec: ucsi: Preliminary support for alternate modes")
Cc: stable@vger.kernel.org
Reported-by: Zwane Mwaikambo <zwanem@gmail.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/ucsi/ucsi.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index e680fcfdee609..758b988ac518a 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -216,14 +216,18 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
 					    con->partner_altmode[i] == altmode);
 }
 
-static u8 ucsi_altmode_next_mode(struct typec_altmode **alt, u16 svid)
+static int ucsi_altmode_next_mode(struct typec_altmode **alt, u16 svid)
 {
 	u8 mode = 1;
 	int i;
 
-	for (i = 0; alt[i]; i++)
+	for (i = 0; alt[i]; i++) {
+		if (i > MODE_DISCOVERY_MAX)
+			return -ERANGE;
+
 		if (alt[i]->svid == svid)
 			mode++;
+	}
 
 	return mode;
 }
@@ -258,8 +262,11 @@ static int ucsi_register_altmode(struct ucsi_connector *con,
 			goto err;
 		}
 
-		desc->mode = ucsi_altmode_next_mode(con->port_altmode,
-						    desc->svid);
+		ret = ucsi_altmode_next_mode(con->port_altmode, desc->svid);
+		if (ret < 0)
+			return ret;
+
+		desc->mode = ret;
 
 		switch (desc->svid) {
 		case USB_TYPEC_DP_SID:
@@ -292,8 +299,11 @@ static int ucsi_register_altmode(struct ucsi_connector *con,
 			goto err;
 		}
 
-		desc->mode = ucsi_altmode_next_mode(con->partner_altmode,
-						    desc->svid);
+		ret = ucsi_altmode_next_mode(con->partner_altmode, desc->svid);
+		if (ret < 0)
+			return ret;
+
+		desc->mode = ret;
 
 		alt = typec_partner_register_altmode(con->partner, desc);
 		if (IS_ERR(alt)) {
-- 
2.28.0

[PATCH 3/5] usb: typec: intel_pmc_mux: Add dependency on ACPI

[Heikki Krogerus]

Since the driver now needs to find the IOM ACPI node, the
driver depends on ACPI. Without the dependency set, the
driver will only fail to compile when ACPI is not enabled.

Fixes: 43d596e32276 ("usb: typec: intel_pmc_mux: Check the port status before connect")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/mux/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
index a4dbd11f8ee26..edead555835e2 100644
--- a/drivers/usb/typec/mux/Kconfig
+++ b/drivers/usb/typec/mux/Kconfig
@@ -11,6 +11,7 @@ config TYPEC_MUX_PI3USB30532
 
 config TYPEC_MUX_INTEL_PMC
 	tristate "Intel PMC mux control"
+	depends on ACPI
 	depends on INTEL_SCU_IPC
 	select USB_ROLE_SWITCH
 	help
-- 
2.28.0

[PATCH 4/5] usb: typec: intel_pmc_mux: Pass correct USB Type-C port number to SoC

[Heikki Krogerus]

From: Azhar Shaikh <azhar.shaikh@intel.com>

The SoC expects the USB Type-C ports numbers to be starting with 0.
If the port number is passed as it is, the IOM status will not be
updated. The IOM port status check fails which will eventually
lead to PMC IPC communication failure.

Fixes: 43d596e32276 ("usb: typec: intel_pmc_mux: Check the port status before connect")
Suggested-by: Utkarsh Patel <utkarsh.h.patel@intel.com>
Signed-off-by: Azhar Shaikh <azhar.shaikh@intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/mux/intel_pmc_mux.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
index 307830b374ec7..109c1a796e844 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -148,8 +148,13 @@ struct pmc_usb {
 
 static void update_port_status(struct pmc_usb_port *port)
 {
+	u8 port_num;
+
+	/* SoC expects the USB Type-C port numbers to start with 0 */
+	port_num = port->usb3_port - 1;
+
 	port->iom_status = readl(port->pmc->iom_base + IOM_PORT_STATUS_OFFSET +
-				 port->usb3_port * sizeof(u32));
+				 port_num * sizeof(u32));
 }
 
 static int sbu_orientation(struct pmc_usb_port *port)
-- 
2.28.0

[PATCH 5/5] usb: typec: intel_pmc_mux: Handle SCU IPC error conditions

[Heikki Krogerus]

From: Madhusudanarao Amara <madhusudanarao.amara@intel.com>

Check and return if there are errors. The response bits are valid
only on no errors.

Fixes: b7404a29cd3d ("usb: typec: intel_pmc_mux: Definitions for response status bits")
Signed-off-by: Madhusudanarao Amara <madhusudanarao.amara@intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/mux/intel_pmc_mux.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
index 109c1a796e844..d7f63b74c6b14 100644
--- a/drivers/usb/typec/mux/intel_pmc_mux.c
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -176,13 +176,19 @@ static int hsl_orientation(struct pmc_usb_port *port)
 static int pmc_usb_command(struct pmc_usb_port *port, u8 *msg, u32 len)
 {
 	u8 response[4];
+	int ret;
 
 	/*
 	 * Error bit will always be 0 with the USBC command.
-	 * Status can be checked from the response message.
+	 * Status can be checked from the response message if the
+	 * function intel_scu_ipc_dev_command succeeds.
 	 */
-	intel_scu_ipc_dev_command(port->pmc->ipc, PMC_USBC_CMD, 0, msg, len,
-				  response, sizeof(response));
+	ret = intel_scu_ipc_dev_command(port->pmc->ipc, PMC_USBC_CMD, 0, msg,
+					len, response, sizeof(response));
+
+	if (ret)
+		return ret;
+
 	if (response[2] & PMC_USB_RESP_STATUS_FAILURE) {
 		if (response[2] & PMC_USB_RESP_STATUS_FATAL)
 			return -EIO;
-- 
2.28.0

Re: [PATCH 0/5] USB Type-C fixes

[Greg Kroah-Hartman]

On Wed, Sep 16, 2020 at 11:16:12AM +0300, Heikki Krogerus wrote:
> Hi,
> 
> There are two fixes for the UCSI drivers that address issues related
> to the alternate modes. The other three fixes are for the PMC mux
> driver. We were missing dependency on ACPI and the port number that
> was used with the IOM was incorrect.
> 
> Let me know if you want anything to be changed.

This seems to be a mix of patches to be sent to Linus for 5.9-final and
some for 5.10-rc1, right?

Can you resend these as two series so I know which one is which?

thanks,

greg k-h

Re: [PATCH 0/5] USB Type-C fixes

[Heikki Krogerus]

On Wed, Sep 16, 2020 at 10:37:09AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 16, 2020 at 11:16:12AM +0300, Heikki Krogerus wrote:
> > Hi,
> > 
> > There are two fixes for the UCSI drivers that address issues related
> > to the alternate modes. The other three fixes are for the PMC mux
> > driver. We were missing dependency on ACPI and the port number that
> > was used with the IOM was incorrect.
> > 
> > Let me know if you want anything to be changed.
> 
> This seems to be a mix of patches to be sent to Linus for 5.9-final and
> some for 5.10-rc1, right?
> 
> Can you resend these as two series so I know which one is which?

OK.

-- 
heikki