linux-kernel - [PATCH] usb: dwc3: pci: skip BYT GPIO lookup table for hardwired phy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230717170552.466914-1-gratian.crisan@ni.com>
Date:   Mon, 17 Jul 2023 12:05:52 -0500
From:   Gratian Crisan <gratian.crisan@...com>
To:     Thinh.Nguyen@...opsys.com, gregkh@...uxfoundation.org,
        hdegoede@...hat.com, felipe.balbi@...ux.intel.com
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        gratian@...il.com, Gratian Crisan <gratian.crisan@...com>
Subject: [PATCH] usb: dwc3: pci: skip BYT GPIO lookup table for hardwired phy

Hardware based on the Bay Trail / BYT SoCs require an external ULPI phy for
USB device-mode. The phy chip usually has its 'reset' and 'chip select'
lines connected to GPIOs described by ACPI fwnodes in the DSDT table.

Because of hardware with missing ACPI resources for the 'reset' and 'chip
select' GPIOs commit 5741022cbdf3 ("usb: dwc3: pci: Add GPIO lookup table
on platforms without ACPI GPIO resources") introduced a fallback
gpiod_lookup_table with hard-coded mappings for Bay Trail devices.

However there are existing Bay Trail based devices, like the National
Instruments cRIO-903x series, where the phy chip has its 'reset' and
'chip-select' lines always asserted in hardware via resistor pull-ups. On
this hardware the phy chip is always enabled and the ACPI dsdt table is
missing information not only for the 'chip-select' and 'reset' lines but
also for the BYT GPIO controller itself "INT33FC".

With the introduction of the gpiod_lookup_table initializing the USB
device-mode on these hardware now errors out. The error comes from the
gpiod_get_optional() calls in dwc3_pci_quirks() which will now return an
-ENOENT error due to the missing ACPI entry for the INT33FC gpio controller
used in the aforementioned table.

This hardware used to work before because gpiod_get_optional() will return
NULL instead of -ENOENT if no GPIO has been assigned to the requested
function. The dwc3_pci_quirks() code for setting the 'cs' and 'reset' GPIOs
was then skipped (due to the NULL return). This is the correct behavior in
cases where the phy chip is hardwired and there are no GPIOs to control.

Since the gpiod_lookup_table relies on the presence of INT33FC fwnode
in ACPI tables only add the table if we know the entry for the INT33FC
gpio controller is present. Additionally check the 'cs' gpio handle is
not NULL before using it (like we do for the 'reset' line). This
allows Bay Trail based devices with hardwired dwc3 ULPI phys to
continue working.

Fixes: 5741022cbdf3 ("usb: dwc3: pci: Add GPIO lookup table on platforms without ACPI GPIO resources")
Signed-off-by: Gratian Crisan <gratian.crisan@...com>
---
 drivers/usb/dwc3/dwc3-pci.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 44a04c9b2073..780984b07437 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -233,10 +233,12 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc,

 			/*
 			 * A lot of BYT devices lack ACPI resource entries for
-			 * the GPIOs, add a fallback mapping to the reference
+			 * the GPIOs. If the ACPI entry for the GPIO controller
+			 * is present add a fallback mapping to the reference
 			 * design GPIOs which all boards seem to use.
 			 */
-			gpiod_add_lookup_table(&platform_bytcr_gpios);
+			if (acpi_dev_present("INT33FC", NULL, -1))
+				gpiod_add_lookup_table(&platform_bytcr_gpios);

 			/*
 			 * These GPIOs will turn on the USB2 PHY. Note that we have to
@@ -247,8 +249,10 @@ static int dwc3_pci_quirks(struct dwc3_pci *dwc,
 			if (IS_ERR(gpio))
 				return PTR_ERR(gpio);

-			gpiod_set_value_cansleep(gpio, 1);
-			gpiod_put(gpio);
+			if (gpio) {
+				gpiod_set_value_cansleep(gpio, 1);
+				gpiod_put(gpio);
+			}

 			gpio = gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
 			if (IS_ERR(gpio))
-- 
2.41.0