diff mbox

[1/3] usb: dwc3: pci: Add GPIO lookup table on platforms without ACPI GPIO resources

Message ID 20180607103845.13515-1-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hans de Goede June 7, 2018, 10:38 a.m. UTC
Bay Trail / BYT SoCs do not have a builtin device-mode phy, instead
they require an external ULPI phy for device-mode.

Only some BYT devices have an external phy, but even on those devices
device-mode is not working because the dwc3 does not see the phy.

The problem is that the ACPI fwnode for the dwc3 does not contain the
expected GPIO resources for the GPIOs connected to the chip-select and
reset pins of the phy.

I've found the workaround which some Android x86 kernels use for this:
https://github.com/BORETS24/Kernel-for-Asus-Zenfone-2/blob/master/arch/x86/platform/intel-mid/device_libs/pci/platform_usb_otg.c
Which boils down to hardcoding the GPIOs for these devices.

This commit adds a gpiod_lookup_table adding the mappings from Android,
and installs this table on BYT system with their BIOS vendor DMI string
set to "INSYDE Corp.". This seems to indicate that a (mostly) unmodified
version of the reference design BIOS is used. 3 out of the 20 BYT tablets
which I have for testing have an external ULPI phy and all 3 models use
this vendor string.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/dwc3/dwc3-pci.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Andy Shevchenko June 7, 2018, 1:28 p.m. UTC | #1
On Thu, Jun 7, 2018 at 1:38 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Bay Trail / BYT SoCs do not have a builtin device-mode phy, instead
> they require an external ULPI phy for device-mode.
>
> Only some BYT devices have an external phy, but even on those devices
> device-mode is not working because the dwc3 does not see the phy.
>
> The problem is that the ACPI fwnode for the dwc3 does not contain the
> expected GPIO resources for the GPIOs connected to the chip-select and
> reset pins of the phy.
>
> I've found the workaround which some Android x86 kernels use for this:
> https://github.com/BORETS24/Kernel-for-Asus-Zenfone-2/blob/master/arch/x86/platform/intel-mid/device_libs/pci/platform_usb_otg.c
> Which boils down to hardcoding the GPIOs for these devices.
>
> This commit adds a gpiod_lookup_table adding the mappings from Android,
> and installs this table on BYT system with their BIOS vendor DMI string
> set to "INSYDE Corp.". This seems to indicate that a (mostly) unmodified
> version of the reference design BIOS is used. 3 out of the 20 BYT tablets
> which I have for testing have an external ULPI phy and all 3 models use
> this vendor string.

> +                       vendor = dmi_get_system_info(DMI_BIOS_VENDOR);
> +                       if (vendor && strcmp(vendor, "INSYDE Corp.") == 0) {
> +                               gpiod_add_lookup_table(&platform_bytcr_gpios);
> +                               dwc->gpio_mapping_added = true;
> +                       }

I understand that my proposal might sound a bit overkill, though, I
would rather go with normal dmi tables and call back there.

P.S. Though, I would be okay with this as long as Felipe fine.
Hans de Goede June 7, 2018, 1:35 p.m. UTC | #2
Hi,

On 07-06-18 15:28, Andy Shevchenko wrote:
> On Thu, Jun 7, 2018 at 1:38 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Bay Trail / BYT SoCs do not have a builtin device-mode phy, instead
>> they require an external ULPI phy for device-mode.
>>
>> Only some BYT devices have an external phy, but even on those devices
>> device-mode is not working because the dwc3 does not see the phy.
>>
>> The problem is that the ACPI fwnode for the dwc3 does not contain the
>> expected GPIO resources for the GPIOs connected to the chip-select and
>> reset pins of the phy.
>>
>> I've found the workaround which some Android x86 kernels use for this:
>> https://github.com/BORETS24/Kernel-for-Asus-Zenfone-2/blob/master/arch/x86/platform/intel-mid/device_libs/pci/platform_usb_otg.c
>> Which boils down to hardcoding the GPIOs for these devices.
>>
>> This commit adds a gpiod_lookup_table adding the mappings from Android,
>> and installs this table on BYT system with their BIOS vendor DMI string
>> set to "INSYDE Corp.". This seems to indicate that a (mostly) unmodified
>> version of the reference design BIOS is used. 3 out of the 20 BYT tablets
>> which I have for testing have an external ULPI phy and all 3 models use
>> this vendor string.
> 
>> +                       vendor = dmi_get_system_info(DMI_BIOS_VENDOR);
>> +                       if (vendor && strcmp(vendor, "INSYDE Corp.") == 0) {
>> +                               gpiod_add_lookup_table(&platform_bytcr_gpios);
>> +                               dwc->gpio_mapping_added = true;
>> +                       }
> 
> I understand that my proposal might sound a bit overkill, though, I
> would rather go with normal dmi tables and call back there.

I was actually thinking about maybe going the other direction and
dropping the DMI check all together if it turns out to be too
narrow. If you look at:
https://github.com/BORETS24/Kernel-for-Asus-Zenfone-2/blob/master/arch/x86/platform/intel-mid/device_libs/pci/platform_usb_otg.c

Then all models that code knows except for the 1st gen
development board (Bay Lake CRB/RVP FAB 1) use these gpios.

And *all* FFRDs (Form Factor Reference Designs) use these pins.

Note that the gpiolib code used the lookup table as a fallback,
so if there are proper ACPI GPIO resources in the resource table
those will be used.

So having a hardcoded fallback to the GPIO pins which all
FFRDs use seems reasonable.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans de Goede June 10, 2018, 1:28 p.m. UTC | #3
Hi,

On 07-06-18 15:35, Hans de Goede wrote:
> Hi,
> 
> On 07-06-18 15:28, Andy Shevchenko wrote:
>> On Thu, Jun 7, 2018 at 1:38 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Bay Trail / BYT SoCs do not have a builtin device-mode phy, instead
>>> they require an external ULPI phy for device-mode.
>>>
>>> Only some BYT devices have an external phy, but even on those devices
>>> device-mode is not working because the dwc3 does not see the phy.
>>>
>>> The problem is that the ACPI fwnode for the dwc3 does not contain the
>>> expected GPIO resources for the GPIOs connected to the chip-select and
>>> reset pins of the phy.
>>>
>>> I've found the workaround which some Android x86 kernels use for this:
>>> https://github.com/BORETS24/Kernel-for-Asus-Zenfone-2/blob/master/arch/x86/platform/intel-mid/device_libs/pci/platform_usb_otg.c
>>> Which boils down to hardcoding the GPIOs for these devices.
>>>
>>> This commit adds a gpiod_lookup_table adding the mappings from Android,
>>> and installs this table on BYT system with their BIOS vendor DMI string
>>> set to "INSYDE Corp.". This seems to indicate that a (mostly) unmodified
>>> version of the reference design BIOS is used. 3 out of the 20 BYT tablets
>>> which I have for testing have an external ULPI phy and all 3 models use
>>> this vendor string.
>>
>>> +                       vendor = dmi_get_system_info(DMI_BIOS_VENDOR);
>>> +                       if (vendor && strcmp(vendor, "INSYDE Corp.") == 0) {
>>> +                               gpiod_add_lookup_table(&platform_bytcr_gpios);
>>> +                               dwc->gpio_mapping_added = true;
>>> +                       }
>>
>> I understand that my proposal might sound a bit overkill, though, I
>> would rather go with normal dmi tables and call back there.
> 
> I was actually thinking about maybe going the other direction and
> dropping the DMI check all together if it turns out to be too
> narrow. If you look at:
> https://github.com/BORETS24/Kernel-for-Asus-Zenfone-2/blob/master/arch/x86/platform/intel-mid/device_libs/pci/platform_usb_otg.c
> 
> Then all models that code knows except for the 1st gen
> development board (Bay Lake CRB/RVP FAB 1) use these gpios.
> 
> And *all* FFRDs (Form Factor Reference Designs) use these pins.
> 
> Note that the gpiolib code used the lookup table as a fallback,
> so if there are proper ACPI GPIO resources in the resource table
> those will be used.
> 
> So having a hardcoded fallback to the GPIO pins which all
> FFRDs use seems reasonable.

And I've just hit my first tablet during testing which needs the GPIO
mapping and has a different BIOS vendor string.

I really don't want to play a game of whack-a-mole with adding more
and more DMI matches here.

So given that as mentioned above all boards seem to use the same
GPIOs and that this is only a fallback for when the GPIOs are
not listed in ACPI, I'm going to submit a v3 of this patch-set
which adds the mapping unconditionally.

Note that most BYT devices have the DWC3 controller disabled in the
BIOS, so this code will only get executed on the few devices which
enable it (because they actually have the external ULPI phy).

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index c961a94d136b..34b84d3bc7cf 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -8,6 +8,7 @@ 
  *	    Sebastian Andrzej Siewior <bigeasy@linutronix.de>
  */
 
+#include <linux/dmi.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -16,6 +17,7 @@ 
 #include <linux/pm_runtime.h>
 #include <linux/platform_device.h>
 #include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
 #include <linux/acpi.h>
 #include <linux/delay.h>
 
@@ -54,6 +56,7 @@  struct dwc3_pci {
 	guid_t guid;
 
 	unsigned int has_dsm_for_pm:1;
+	unsigned int gpio_mapping_added:1;
 	struct work_struct wakeup_work;
 };
 
@@ -66,6 +69,15 @@  static const struct acpi_gpio_mapping acpi_dwc3_byt_gpios[] = {
 	{ },
 };
 
+static struct gpiod_lookup_table platform_bytcr_gpios = {
+	.dev_id		= "0000:00:16.0",
+	.table		= {
+		GPIO_LOOKUP("INT33FC:00", 54, "reset", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("INT33FC:02", 14, "cs", GPIO_ACTIVE_HIGH),
+		{}
+	},
+};
+
 static int dwc3_pci_quirks(struct dwc3_pci *dwc)
 {
 	struct platform_device		*dwc3 = dwc->dwc3;
@@ -120,12 +132,24 @@  static int dwc3_pci_quirks(struct dwc3_pci *dwc)
 
 		if (pdev->device == PCI_DEVICE_ID_INTEL_BYT) {
 			struct gpio_desc *gpio;
+			const char *vendor;
 
 			ret = devm_acpi_dev_add_driver_gpios(&pdev->dev,
 					acpi_dwc3_byt_gpios);
 			if (ret)
 				dev_dbg(&pdev->dev, "failed to add mapping table\n");
 
+			/*
+			 * A lot of BYT devices lack ACPI resource entries for
+			 * the GPIOs, add a manual mapping on devices which use
+			 * an unmodified "INSYDE Corp." reference BIOS / design.
+			 */
+			vendor = dmi_get_system_info(DMI_BIOS_VENDOR);
+			if (vendor && strcmp(vendor, "INSYDE Corp.") == 0) {
+				gpiod_add_lookup_table(&platform_bytcr_gpios);
+				dwc->gpio_mapping_added = true;
+			}
+
 			/*
 			 * These GPIOs will turn on the USB2 PHY. Note that we have to
 			 * put the gpio descriptors again here because the phy driver
@@ -256,6 +280,9 @@  static void dwc3_pci_remove(struct pci_dev *pci)
 {
 	struct dwc3_pci		*dwc = pci_get_drvdata(pci);
 
+	if (dwc->gpio_mapping_added)
+		gpiod_remove_lookup_table(&platform_bytcr_gpios);
+
 #ifdef CONFIG_PM
 	cancel_work_sync(&dwc->wakeup_work);
 #endif