[PATCH 0/3] Load ASMedia XHCI controller firmware

newer
[PATCH 0/2] arm: dts: k3-am6: Sync...

Mark Kettenis

14 Jul 2023 14 Jul '23

10:43 p.m.

Some of the Apple desktop machine have a PCI ASMedia XHCI controller that needs firmware to be uploaded to function. This firmware is placed on the EFI system partition by the Asahi installer. So this series sets up a file system firmware loader that reads from that partition and ports the Linux driver that uploads this firmware to the controller. With this series, users can use USB devices (such as a keyboard) when they are connected to USB ports handled by the ASMedia XHCI controller. This includes the type-A ports on the M2 Mac mini for example.

Mark Kettenis (3): apple: Set up file system firmware loader iopoll: Add readb_poll_sleep_timeout usb: xhci-pci: Load ASMedia XHCI controller firmware

-- 2.41.0

Show replies by date

Mark Kettenis

14 Jul 14 Jul

10:43 p.m.

New subject: [PATCH 1/3] apple: Set up file system firmware loader

Find the appropriate EFI system partition on the internal NVMe storage and set the U-Boot environment variables such that the file system firmware loader can load firmware from it.

Signed-off-by: Mark Kettenis kettenis@openbsd.org --- arch/arm/mach-apple/board.c | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)

diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c index d501948118..7799a0f916 100644 --- a/arch/arm/mach-apple/board.c +++ b/arch/arm/mach-apple/board.c @@ -8,6 +8,8 @@ #include <dm/uclass-internal.h> #include <efi_loader.h> #include <lmb.h> +#include <nvme.h> +#include <part.h>

#include <asm/armv8/mmu.h> #include <asm/global_data.h> @@ -539,6 +541,60 @@ u64 get_page_table_size(void) return SZ_256K; }

+static char *asahi_esp_devpart(void) +{ + struct disk_partition info; + struct blk_desc *nvme_blk; + const char *uuid = NULL; + int devnum, len, p, part, ret; + static char devpart[64]; + struct udevice *dev; + ofnode node; + + if (devpart[0]) + return devpart; + + node = ofnode_path("/chosen"); + if (ofnode_valid(node)) { + uuid = ofnode_get_property(node, "asahi,efi-system-partition", + &len); + } + + nvme_scan_namespace(); + for (devnum = 0, part = 0;; devnum++) { + nvme_blk = blk_get_devnum_by_uclass_id(UCLASS_NVME, devnum); + if (nvme_blk == NULL) + break; + + dev = dev_get_parent(nvme_blk->bdev); + if (!device_is_compatible(dev, "apple,nvme-ans2")) + continue; + + for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) { + ret = part_get_info(nvme_blk, p, &info); + if (ret < 0) + break; + + if (info.bootable & PART_EFI_SYSTEM_PARTITION) { + if (uuid && strcasecmp(uuid, info.uuid) == 0) { + part = p; + break; + } + if (part == 0) + part = p; + } + } + + if (part > 0) + break; + } + + if (part > 0) + snprintf(devpart, sizeof(devpart), "%d:%d", devnum, part); + + return devpart; +} + #define KERNEL_COMP_SIZE SZ_128M

int board_late_init(void) @@ -546,6 +602,10 @@ int board_late_init(void) struct lmb lmb; u32 status = 0;

+ status |= env_set("storage_interface", + blk_get_uclass_name(UCLASS_NVME)); + status |= env_set("fw_dev_part", asahi_esp_devpart()); + lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);

/* somewhat based on the Linux Kernel boot requirements:

-- 2.41.0

Marek Vasut

11:27 p.m.

New subject: [PATCH 1/3] apple: Set up file system firmware loader

On 7/14/23 22:43, Mark Kettenis wrote:

...

Find the appropriate EFI system partition on the internal NVMe storage and set the U-Boot environment variables such that the file system firmware loader can load firmware from it.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/mach-apple/board.c | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)

diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c index d501948118..7799a0f916 100644 --- a/arch/arm/mach-apple/board.c +++ b/arch/arm/mach-apple/board.c @@ -8,6 +8,8 @@ #include <dm/uclass-internal.h> #include <efi_loader.h> #include <lmb.h> +#include <nvme.h> +#include <part.h>

#include <asm/armv8/mmu.h> #include <asm/global_data.h> @@ -539,6 +541,60 @@ u64 get_page_table_size(void) return SZ_256K; }

+static char *asahi_esp_devpart(void) +{
struct disk_partition info;

struct blk_desc *nvme_blk;

const char *uuid = NULL;

int devnum, len, p, part, ret;

static char devpart[64];

struct udevice *dev;

ofnode node;

if (devpart[0])
return devpart;
node = ofnode_path("/chosen");

if (ofnode_valid(node)) {
uuid = ofnode_get_property(node, "asahi,efi-system-partition",
			   &len);
}

nvme_scan_namespace();

for (devnum = 0, part = 0;; devnum++) {
nvme_blk = blk_get_devnum_by_uclass_id(UCLASS_NVME, devnum);
if (nvme_blk == NULL)
	break;
dev = dev_get_parent(nvme_blk->bdev);
if (!device_is_compatible(dev, "apple,nvme-ans2"))

Can we somehow use ofnode_for_each_compatible_node() here ? That might simplify this code.

...

	continue;
for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
	ret = part_get_info(nvme_blk, p, &info);
	if (ret < 0)
		break;
	if (info.bootable & PART_EFI_SYSTEM_PARTITION) {
		if (uuid && strcasecmp(uuid, info.uuid) == 0) {
			part = p;
			break;
		}
		if (part == 0)
			part = p;
	}
}
if (part > 0)
	break;
}

if (part > 0)
snprintf(devpart, sizeof(devpart), "%d:%d", devnum, part);
return devpart;
+}

#define KERNEL_COMP_SIZE SZ_128M

int board_late_init(void)

@@ -546,6 +602,10 @@ int board_late_init(void) struct lmb lmb; u32 status = 0;
status |= env_set("storage_interface",
	  blk_get_uclass_name(UCLASS_NVME));
status |= env_set("fw_dev_part", asahi_esp_devpart());

I think env_set() returns integer (and this could be negative too), so you might want to check the return value instead of casting it to unsigned integer.

Mark Kettenis

15 Jul 15 Jul

2:47 p.m.

New subject: [PATCH 1/3] apple: Set up file system firmware loader

...

Date: Fri, 14 Jul 2023 23:27:42 +0200 From: Marek Vasut marex@denx.de

On 7/14/23 22:43, Mark Kettenis wrote:

...
Find the appropriate EFI system partition on the internal NVMe storage and set the U-Boot environment variables such that the file system firmware loader can load firmware from it.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/mach-apple/board.c | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)

diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c index d501948118..7799a0f916 100644 --- a/arch/arm/mach-apple/board.c +++ b/arch/arm/mach-apple/board.c @@ -8,6 +8,8 @@ #include <dm/uclass-internal.h> #include <efi_loader.h> #include <lmb.h> +#include <nvme.h> +#include <part.h>

#include <asm/armv8/mmu.h> #include <asm/global_data.h> @@ -539,6 +541,60 @@ u64 get_page_table_size(void) return SZ_256K; }

+static char *asahi_esp_devpart(void) +{
struct disk_partition info;

struct blk_desc *nvme_blk;

const char *uuid = NULL;

int devnum, len, p, part, ret;

static char devpart[64];

struct udevice *dev;

ofnode node;

if (devpart[0])
return devpart;
node = ofnode_path("/chosen");

if (ofnode_valid(node)) {
uuid = ofnode_get_property(node, "asahi,efi-system-partition",
			   &len);
}

nvme_scan_namespace();

for (devnum = 0, part = 0;; devnum++) {
nvme_blk = blk_get_devnum_by_uclass_id(UCLASS_NVME, devnum);
if (nvme_blk == NULL)
	break;
dev = dev_get_parent(nvme_blk->bdev);
if (!device_is_compatible(dev, "apple,nvme-ans2"))
Can we somehow use ofnode_for_each_compatible_node() here ? That might simplify this code.

I don't really see how that would simplify things. I'm iterating over all NVMe devices here and then checking the compatible of the parent to make sure I pick the on-board one. I could do the inverse and lookup the node first and then use that to find the NVMe block device, but it will still involve a loop and several function calls.

...

...
	continue;
for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
	ret = part_get_info(nvme_blk, p, &info);
	if (ret < 0)
		break;
	if (info.bootable & PART_EFI_SYSTEM_PARTITION) {
		if (uuid && strcasecmp(uuid, info.uuid) == 0) {
			part = p;
			break;
		}
		if (part == 0)
			part = p;
	}
}
if (part > 0)
	break;
}

if (part > 0)
snprintf(devpart, sizeof(devpart), "%d:%d", devnum, part);
return devpart;
+}

#define KERNEL_COMP_SIZE SZ_128M

int board_late_init(void)

@@ -546,6 +602,10 @@ int board_late_init(void) struct lmb lmb; u32 status = 0;
status |= env_set("storage_interface",
	  blk_get_uclass_name(UCLASS_NVME));
status |= env_set("fw_dev_part", asahi_esp_devpart());
I think env_set() returns integer (and this could be negative too), so you might want to check the return value instead of casting it to unsigned integer.

I'm just using the existing idiom. But maybe I should just check the return value and throw a warning instead? Not having the firmware loader available isn't fatal. It just means some of the USB ports won't work.

Marek Vasut

18 Jul 18 Jul

1:09 a.m.

New subject: [PATCH 1/3] apple: Set up file system firmware loader

On 7/15/23 14:47, Mark Kettenis wrote:

...

...
Date: Fri, 14 Jul 2023 23:27:42 +0200 From: Marek Vasut marex@denx.de

On 7/14/23 22:43, Mark Kettenis wrote:

...
Find the appropriate EFI system partition on the internal NVMe storage and set the U-Boot environment variables such that the file system firmware loader can load firmware from it.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

arch/arm/mach-apple/board.c | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)

diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c index d501948118..7799a0f916 100644 --- a/arch/arm/mach-apple/board.c +++ b/arch/arm/mach-apple/board.c @@ -8,6 +8,8 @@ #include <dm/uclass-internal.h> #include <efi_loader.h> #include <lmb.h> +#include <nvme.h> +#include <part.h>

#include <asm/armv8/mmu.h> #include <asm/global_data.h> @@ -539,6 +541,60 @@ u64 get_page_table_size(void) return SZ_256K; }

+static char *asahi_esp_devpart(void) +{
struct disk_partition info;

struct blk_desc *nvme_blk;

const char *uuid = NULL;

int devnum, len, p, part, ret;

static char devpart[64];

struct udevice *dev;

ofnode node;

if (devpart[0])
return devpart;
node = ofnode_path("/chosen");

if (ofnode_valid(node)) {
uuid = ofnode_get_property(node, "asahi,efi-system-partition",
			   &len);
}

nvme_scan_namespace();

for (devnum = 0, part = 0;; devnum++) {
nvme_blk = blk_get_devnum_by_uclass_id(UCLASS_NVME, devnum);
if (nvme_blk == NULL)
	break;
dev = dev_get_parent(nvme_blk->bdev);
if (!device_is_compatible(dev, "apple,nvme-ans2"))
Can we somehow use ofnode_for_each_compatible_node() here ? That might simplify this code.
I don't really see how that would simplify things. I'm iterating over all NVMe devices here and then checking the compatible of the parent to make sure I pick the on-board one. I could do the inverse and lookup the node first and then use that to find the NVMe block device, but it will still involve a loop and several function calls.

What about:

" struct blk_desc *desc;

ofnode_for_each_compatible_node(node, "apple,nvme-ans2") { uclass_get_device_by_ofnode(UCLASS_NVME, node, &blk_dev); desc = dev_get_uclass_plat(blk_dev); for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) { part_get_info(desc, part, &info); ... } } "

I'm _not_ sure anymore whether this is actually easier though. What do you think ?

...

...
...
	continue;
for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
	ret = part_get_info(nvme_blk, p, &info);
	if (ret < 0)
		break;
	if (info.bootable & PART_EFI_SYSTEM_PARTITION) {
		if (uuid && strcasecmp(uuid, info.uuid) == 0) {
			part = p;
			break;
		}
		if (part == 0)
			part = p;
	}
}
if (part > 0)
	break;
}

if (part > 0)
snprintf(devpart, sizeof(devpart), "%d:%d", devnum, part);
return devpart;
+}

#define KERNEL_COMP_SIZE SZ_128M

int board_late_init(void)

@@ -546,6 +602,10 @@ int board_late_init(void) struct lmb lmb; u32 status = 0;
status |= env_set("storage_interface",
	  blk_get_uclass_name(UCLASS_NVME));
status |= env_set("fw_dev_part", asahi_esp_devpart());
I think env_set() returns integer (and this could be negative too), so you might want to check the return value instead of casting it to unsigned integer.
I'm just using the existing idiom. But maybe I should just check the return value and throw a warning instead? Not having the firmware loader available isn't fatal. It just means some of the USB ports won't work.

That's better, yeah. I don't think you can safely do bitwise operations on signed types.

Mark Kettenis

14 Jul 14 Jul

10:43 p.m.

New subject: [PATCH 2/3] iopoll: Add readb_poll_sleep_timeout

Add a variant of readl_poll_sleep_timeout that reads a single byte to match the readb_poll_timeout API that Linux has.

Signed-off-by: Mark Kettenis kettenis@openbsd.org --- include/linux/iopoll.h | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h index 0ee2bddaa8..89990e95e9 100644 --- a/include/linux/iopoll.h +++ b/include/linux/iopoll.h @@ -49,6 +49,9 @@ #define readl_poll_sleep_timeout(addr, val, cond, sleep_us, timeout_us) \ readx_poll_sleep_timeout(readl, addr, val, cond, sleep_us, timeout_us)

+#define readb_poll_sleep_timeout(addr, val, cond, sleep_us, timeout_us) \ + readx_poll_sleep_timeout(readb, addr, val, cond, sleep_us, timeout_us) + #define readx_poll_timeout(op, addr, val, cond, timeout_us) \ read_poll_timeout(op, val, cond, false, timeout_us, addr)

-- 2.41.0

Marek Vasut

11:22 p.m.

New subject: [PATCH 2/3] iopoll: Add readb_poll_sleep_timeout

On 7/14/23 22:43, Mark Kettenis wrote:

...

Add a variant of readl_poll_sleep_timeout that reads a single byte to match the readb_poll_timeout API that Linux has.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

include/linux/iopoll.h | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h index 0ee2bddaa8..89990e95e9 100644 --- a/include/linux/iopoll.h +++ b/include/linux/iopoll.h @@ -49,6 +49,9 @@ #define readl_poll_sleep_timeout(addr, val, cond, sleep_us, timeout_us) \ readx_poll_sleep_timeout(readl, addr, val, cond, sleep_us, timeout_us)

+#define readb_poll_sleep_timeout(addr, val, cond, sleep_us, timeout_us) \

readx_poll_sleep_timeout(readb, addr, val, cond, sleep_us, timeout_us)

readb variant should go before readl variant I think.

Mark Kettenis

15 Jul 15 Jul

2:30 p.m.

New subject: [PATCH 2/3] iopoll: Add readb_poll_sleep_timeout

...

Date: Fri, 14 Jul 2023 23:22:00 +0200 From: Marek Vasut marex@denx.de

On 7/14/23 22:43, Mark Kettenis wrote:

...
Add a variant of readl_poll_sleep_timeout that reads a single byte to match the readb_poll_timeout API that Linux has.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

include/linux/iopoll.h | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h index 0ee2bddaa8..89990e95e9 100644 --- a/include/linux/iopoll.h +++ b/include/linux/iopoll.h @@ -49,6 +49,9 @@ #define readl_poll_sleep_timeout(addr, val, cond, sleep_us, timeout_us) \ readx_poll_sleep_timeout(readl, addr, val, cond, sleep_us, timeout_us)

+#define readb_poll_sleep_timeout(addr, val, cond, sleep_us, timeout_us) \

readx_poll_sleep_timeout(readb, addr, val, cond, sleep_us, timeout_us)

readb variant should go before readl variant I think.

That makes to order consistent, so that makes sense. I'll fix that in the next revision.

Mark Kettenis

14 Jul 14 Jul

10:43 p.m.

New subject: [PATCH 3/3] usb: xhci-pci: Load ASMedia XHCI controller firmware

The ASMedia XHCI controller found on some of the Apple Silicon machines needs firmware to operate. Use the file system firmware loader interface to read the firmware and load it onto the controller. This allows keyboards connected to the type-A ports on these machines to function in U-Boot.

Signed-off-by: Mark Kettenis kettenis@openbsd.org --- MAINTAINERS | 1 + drivers/usb/host/Kconfig | 11 + drivers/usb/host/Makefile | 1 + drivers/usb/host/xhci-pci-asmedia.c | 394 ++++++++++++++++++++++++++++ drivers/usb/host/xhci-pci.c | 9 + include/usb/xhci.h | 3 + 6 files changed, 419 insertions(+) create mode 100644 drivers/usb/host/xhci-pci-asmedia.c

diff --git a/MAINTAINERS b/MAINTAINERS index 2477923a52..0acf04fff8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -124,6 +124,7 @@ F: drivers/iommu/apple_dart.c F: drivers/nvme/nvme_apple.c F: drivers/pci/pcie_apple.c F: drivers/pinctrl/pinctrl-apple.c +F: drivers/usb/host/xhci-pci-asmedia.c F: drivers/watchdog/apple_wdt.c F: include/configs/apple.h

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 1a883babf4..ad8dfabeeb 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -95,6 +95,17 @@ config USB_XHCI_PCI help Enables support for the PCI-based xHCI controller.

+config USB_XHCI_PCI_ASMEDIA + bool "Support for ASMedia xHCI USB controller" + depends on USB_XHCI_PCI + default y if ARCH_APPLE + imply FS_LOADER + help + Enables support for loading firmware for the ASMedia xHCI + controller. This is needed on Apple Silicon machines that + contain ASMedia xHCI controllers without the full USB + firmware. + config USB_XHCI_RCAR bool "Renesas RCar USB 3.0 support" default y diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index 8dad36f936..2aa2d80263 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -52,6 +52,7 @@ obj-$(CONFIG_USB_XHCI_MTK) += xhci-mtk.o obj-$(CONFIG_USB_XHCI_MVEBU) += xhci-mvebu.o obj-$(CONFIG_USB_XHCI_OMAP) += xhci-omap.o obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o +obj-$(CONFIG_USB_XHCI_PCI_ASMEDIA) += xhci-pci-asmedia.o obj-$(CONFIG_USB_XHCI_RCAR) += xhci-rcar.o obj-$(CONFIG_USB_XHCI_STI) += dwc3-sti-glue.o obj-$(CONFIG_USB_XHCI_OCTEON) += dwc3-octeon-glue.o diff --git a/drivers/usb/host/xhci-pci-asmedia.c b/drivers/usb/host/xhci-pci-asmedia.c new file mode 100644 index 0000000000..cc1d25bde2 --- /dev/null +++ b/drivers/usb/host/xhci-pci-asmedia.c @@ -0,0 +1,394 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT +/* + * ASMedia xHCI firmware loader + * Copyright (C) The Asahi Linux Contributors + */ + +#include <common.h> +#include <dm.h> +#include <dm/device_compat.h> +#include <fs_loader.h> +#include <linux/iopoll.h> +#include <pci.h> +#include <usb.h> +#include <usb/xhci.h> + +/* Configuration space registers */ +#define ASMT_CFG_CONTROL 0xe0 +#define ASMT_CFG_CONTROL_WRITE BIT(1) +#define ASMT_CFG_CONTROL_READ BIT(0) + +#define ASMT_CFG_SRAM_ADDR 0xe2 + +#define ASMT_CFG_SRAM_ACCESS 0xef +#define ASMT_CFG_SRAM_ACCESS_READ BIT(6) +#define ASMT_CFG_SRAM_ACCESS_ENABLE BIT(7) + +#define ASMT_CFG_DATA_READ0 0xf0 +#define ASMT_CFG_DATA_READ1 0xf4 + +#define ASMT_CFG_DATA_WRITE0 0xf8 +#define ASMT_CFG_DATA_WRITE1 0xfc + +#define ASMT_CMD_GET_FWVER 0x8000060840 +#define ASMT_FWVER_ROM 0x010250090816 + +/* BAR0 registers */ +#define ASMT_REG_ADDR 0x3000 + +#define ASMT_REG_WDATA 0x3004 +#define ASMT_REG_RDATA 0x3008 + +#define ASMT_REG_STATUS 0x3009 +#define ASMT_REG_STATUS_BUSY BIT(7) + +#define ASMT_REG_CODE_WDATA 0x3010 +#define ASMT_REG_CODE_RDATA 0x3018 + +#define ASMT_MMIO_CPU_MISC 0x500e +#define ASMT_MMIO_CPU_MISC_CODE_RAM_WR BIT(0) + +#define ASMT_MMIO_CPU_MODE_NEXT 0x5040 +#define ASMT_MMIO_CPU_MODE_CUR 0x5041 + +#define ASMT_MMIO_CPU_MODE_RAM BIT(0) +#define ASMT_MMIO_CPU_MODE_HALFSPEED BIT(1) + +#define ASMT_MMIO_CPU_EXEC_CTRL 0x5042 +#define ASMT_MMIO_CPU_EXEC_CTRL_RESET BIT(0) +#define ASMT_MMIO_CPU_EXEC_CTRL_HALT BIT(1) + +#define TIMEOUT_USEC 10000 +#define RESET_TIMEOUT_USEC 500000 + +static int asmedia_mbox_tx(struct udevice *pdev, u64 data) +{ + u8 op; + int i; + + for (i = 0; i < TIMEOUT_USEC; i++) { + dm_pci_read_config8(pdev, ASMT_CFG_CONTROL, &op); + if (!(op & ASMT_CFG_CONTROL_WRITE)) + break; + udelay(1); + } + + if (op & ASMT_CFG_CONTROL_WRITE) { + dev_err(pdev, + "Timed out on mailbox tx: 0x%llx\n", + data); + return -ETIMEDOUT; + } + + dm_pci_write_config32(pdev, ASMT_CFG_DATA_WRITE0, data); + dm_pci_write_config32(pdev, ASMT_CFG_DATA_WRITE1, data >> 32); + dm_pci_write_config8(pdev, ASMT_CFG_CONTROL, + ASMT_CFG_CONTROL_WRITE); + + return 0; +} + +static int asmedia_mbox_rx(struct udevice *pdev, u64 *data) +{ + u8 op; + u32 low, high; + int i; + + for (i = 0; i < TIMEOUT_USEC; i++) { + dm_pci_read_config8(pdev, ASMT_CFG_CONTROL, &op); + if (op & ASMT_CFG_CONTROL_READ) + break; + udelay(1); + } + + if (!(op & ASMT_CFG_CONTROL_READ)) { + dev_err(pdev, "Timed out on mailbox rx\n"); + return -ETIMEDOUT; + } + + dm_pci_read_config32(pdev, ASMT_CFG_DATA_READ0, &low); + dm_pci_read_config32(pdev, ASMT_CFG_DATA_READ1, &high); + dm_pci_write_config8(pdev, ASMT_CFG_CONTROL, + ASMT_CFG_CONTROL_READ); + + *data = ((u64)high << 32) | low; + return 0; +} + +static int asmedia_get_fw_version(struct udevice *pdev, u64 *version) +{ + int err = 0; + u64 cmd; + + err = asmedia_mbox_tx(pdev, ASMT_CMD_GET_FWVER); + if (err) + return err; + err = asmedia_mbox_tx(pdev, 0); + if (err) + return err; + + err = asmedia_mbox_rx(pdev, &cmd); + if (err) + return err; + err = asmedia_mbox_rx(pdev, version); + if (err) + return err; + + if (cmd != ASMT_CMD_GET_FWVER) { + dev_err(pdev, "Unexpected reply command 0x%llx\n", cmd); + return -EIO; + } + + return 0; +} + +static bool asmedia_check_firmware(struct udevice *pdev) +{ + u64 fwver; + int ret; + + ret = asmedia_get_fw_version(pdev, &fwver); + if (ret) + return ret; + + dev_info(pdev, "Firmware version: 0x%llx\n", fwver); + + return fwver != ASMT_FWVER_ROM; +} + +static int asmedia_wait_reset(struct udevice *pdev, struct xhci_hcor *hcor) +{ + u32 val; + int ret; + + ret = readl_poll_sleep_timeout(&hcor->or_usbcmd, + val, !(val & CMD_RESET), + 1000, RESET_TIMEOUT_USEC); + + if (!ret) + return 0; + + dev_err(pdev, "Reset timed out, trying to kick it\n"); + + dm_pci_write_config8(pdev, ASMT_CFG_SRAM_ACCESS, + ASMT_CFG_SRAM_ACCESS_ENABLE); + + dm_pci_write_config8(pdev, ASMT_CFG_SRAM_ACCESS, 0); + + ret = readl_poll_sleep_timeout(&hcor->or_usbcmd, + val, !(val & CMD_RESET), + 1000, RESET_TIMEOUT_USEC); + + if (ret) + dev_err(pdev, "Reset timed out, giving up\n"); + + return ret; +} + +static u8 asmedia_read_reg(struct udevice *pdev, struct xhci_hccr *hccr, + u16 addr) +{ + void __iomem *regs = (void *)hccr; + u8 status; + int ret; + + ret = readb_poll_sleep_timeout(regs + ASMT_REG_STATUS, status, + !(status & ASMT_REG_STATUS_BUSY), + 1000, TIMEOUT_USEC); + + if (ret) { + dev_err(pdev, + "Read reg wait timed out ([%04x])\n", addr); + return ~0; + } + + writew_relaxed(addr, regs + ASMT_REG_ADDR); + + ret = readb_poll_sleep_timeout(regs + ASMT_REG_STATUS, status, + !(status & ASMT_REG_STATUS_BUSY), + 1000, TIMEOUT_USEC); + + if (ret) { + dev_err(pdev, + "Read reg addr timed out ([%04x])\n", addr); + return ~0; + } + + return readb_relaxed(regs + ASMT_REG_RDATA); +} + +static void asmedia_write_reg(struct udevice *pdev, struct xhci_hccr *hccr, + u16 addr, u8 data, bool wait) +{ + void __iomem *regs = (void *)hccr; + u8 status; + int ret, i; + + writew_relaxed(addr, regs + ASMT_REG_ADDR); + + ret = readb_poll_sleep_timeout(regs + ASMT_REG_STATUS, status, + !(status & ASMT_REG_STATUS_BUSY), + 1000, TIMEOUT_USEC); + + if (ret) + dev_err(pdev, + "Write reg addr timed out ([%04x] = %02x)\n", + addr, data); + + writeb_relaxed(data, regs + ASMT_REG_WDATA); + + ret = readb_poll_sleep_timeout(regs + ASMT_REG_STATUS, status, + !(status & ASMT_REG_STATUS_BUSY), + 1000, TIMEOUT_USEC); + + if (ret) + dev_err(pdev, + "Write reg data timed out ([%04x] = %02x)\n", + addr, data); + + if (!wait) + return; + + for (i = 0; i < TIMEOUT_USEC; i++) { + if (asmedia_read_reg(pdev, hccr, addr) == data) + break; + } + + if (i >= TIMEOUT_USEC) { + dev_err(pdev, + "Verify register timed out ([%04x] = %02x)\n", + addr, data); + } +} + +static int asmedia_load_fw(struct udevice *pdev, struct xhci_hccr *hccr, + struct xhci_hcor *hcor, void *fwdata, size_t fwsize) +{ + void __iomem *regs = (void *)hccr; + const u16 *fw_data = (const u16 *)fwdata; + u16 raddr; + u32 data; + size_t index = 0, addr = 0; + size_t words = fwsize >> 1; + int ret, i; + + asmedia_write_reg(pdev, hccr, ASMT_MMIO_CPU_MODE_NEXT, + ASMT_MMIO_CPU_MODE_HALFSPEED, false); + + asmedia_write_reg(pdev, hccr, ASMT_MMIO_CPU_EXEC_CTRL, + ASMT_MMIO_CPU_EXEC_CTRL_RESET, false); + + ret = asmedia_wait_reset(pdev, hcor); + if (ret) { + dev_err(pdev, "Failed pre-upload reset\n"); + return ret; + } + + asmedia_write_reg(pdev, hccr, ASMT_MMIO_CPU_EXEC_CTRL, + ASMT_MMIO_CPU_EXEC_CTRL_HALT, false); + + asmedia_write_reg(pdev, hccr, ASMT_MMIO_CPU_MISC, + ASMT_MMIO_CPU_MISC_CODE_RAM_WR, true); + + dm_pci_write_config8(pdev, ASMT_CFG_SRAM_ACCESS, + ASMT_CFG_SRAM_ACCESS_ENABLE); + + /* The firmware upload is interleaved in 0x4000 word blocks */ + addr = index = 0; + while (index < words) { + data = fw_data[index]; + if ((index | 0x4000) < words) + data |= fw_data[index | 0x4000] << 16; + + dm_pci_write_config16(pdev, ASMT_CFG_SRAM_ADDR, + addr); + + writel_relaxed(data, regs + ASMT_REG_CODE_WDATA); + + for (i = 0; i < TIMEOUT_USEC; i++) { + dm_pci_read_config16(pdev, ASMT_CFG_SRAM_ADDR, &raddr); + if (raddr != addr) + break; + udelay(1); + } + + if (raddr == addr) { + dev_err(pdev, "Word write timed out\n"); + return -ETIMEDOUT; + } + + if (++index & 0x4000) + index += 0x4000; + addr += 2; + } + + dm_pci_write_config8(pdev, ASMT_CFG_SRAM_ACCESS, 0); + + asmedia_write_reg(pdev, hccr, ASMT_MMIO_CPU_MISC, 0, true); + + asmedia_write_reg(pdev, hccr, ASMT_MMIO_CPU_MODE_NEXT, + ASMT_MMIO_CPU_MODE_RAM | + ASMT_MMIO_CPU_MODE_HALFSPEED, false); + + asmedia_write_reg(pdev, hccr, ASMT_MMIO_CPU_EXEC_CTRL, 0, false); + + ret = asmedia_wait_reset(pdev, hcor); + if (ret) { + dev_err(pdev, "Failed post-upload reset\n"); + return ret; + } + + return 0; +} + +int asmedia_xhci_check_request_fw(struct udevice *pdev, + struct xhci_hccr *hccr, + struct xhci_hcor *hcor) +{ + const char *fw_name = "vendorfw/u-boot/asmedia/asm2214a-apple.bin"; + struct udevice *fsdev; + void *data; + int ret; + + /* Check if device has firmware, if so skip everything */ + ret = asmedia_check_firmware(pdev); + if (ret < 0) + return ret; + else if (ret == 1) + return 0; + + ret = get_fs_loader(&fsdev); + if (ret) { + dev_err(pdev, "Could not load firmware %s: %d\n", + fw_name, ret); + return ret; + } + + data = malloc(SZ_256K); + ret = request_firmware_into_buf(fsdev, fw_name, data, SZ_256K, 0); + if (ret < 0) { + dev_err(pdev, "Could not load firmware %s: %d\n", + fw_name, ret); + return ret; + } + + ret = asmedia_load_fw(pdev, hccr, hcor, data, ret); + if (ret) { + dev_err(pdev, "Firmware upload failed: %d\n", ret); + goto err; + } + + ret = asmedia_check_firmware(pdev); + if (ret < 0) { + goto err; + } else if (ret != 1) { + dev_err(pdev, "Firmware version is too old after upload\n"); + ret = -EIO; + } else { + ret = 0; + } + +err: + free(data); + return ret; +} diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 11f1c02000..ca0f1fbcef 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -53,6 +53,7 @@ static int xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr, static int xhci_pci_probe(struct udevice *dev) { struct xhci_pci_plat *plat = dev_get_plat(dev); + struct pci_child_plat *pplat = dev_get_parent_plat(dev); struct xhci_hccr *hccr; struct xhci_hcor *hcor; int ret; @@ -77,6 +78,14 @@ static int xhci_pci_probe(struct udevice *dev) if (ret) goto err_reset;

+ if (IS_ENABLED(CONFIG_USB_XHCI_PCI_ASMEDIA) && + pplat->vendor == PCI_VENDOR_ID_ASMEDIA && + pplat->device == 0x2142) { + ret = asmedia_xhci_check_request_fw(dev, hccr, hcor); + if (ret) + goto err_reset; + } + ret = xhci_register(dev, hccr, hcor); if (ret) goto err_reset; diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 4a4ac10229..16c9311b5b 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -1312,4 +1312,7 @@ static inline void xhci_dma_unmap(struct xhci_ctrl *ctrl, dma_addr_t addr, #endif }

+int asmedia_xhci_check_request_fw(struct udevice *, struct xhci_hccr *, + struct xhci_hcor *); + #endif /* HOST_XHCI_H_ */

-- 2.41.0

Marek Vasut

11:30 p.m.

New subject: [PATCH 3/3] usb: xhci-pci: Load ASMedia XHCI controller firmware

On 7/14/23 22:43, Mark Kettenis wrote:

...

The ASMedia XHCI controller found on some of the Apple Silicon machines needs firmware to operate. Use the file system firmware loader interface to read the firmware and load it onto the controller. This allows keyboards connected to the type-A ports on these machines to function in U-Boot.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

MAINTAINERS | 1 + drivers/usb/host/Kconfig | 11 + drivers/usb/host/Makefile | 1 + drivers/usb/host/xhci-pci-asmedia.c | 394 ++++++++++++++++++++++++++++ drivers/usb/host/xhci-pci.c | 9 + include/usb/xhci.h | 3 + 6 files changed, 419 insertions(+) create mode 100644 drivers/usb/host/xhci-pci-asmedia.c

diff --git a/MAINTAINERS b/MAINTAINERS index 2477923a52..0acf04fff8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -124,6 +124,7 @@ F: drivers/iommu/apple_dart.c F: drivers/nvme/nvme_apple.c F: drivers/pci/pcie_apple.c F: drivers/pinctrl/pinctrl-apple.c +F: drivers/usb/host/xhci-pci-asmedia.c F: drivers/watchdog/apple_wdt.c F: include/configs/apple.h

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 1a883babf4..ad8dfabeeb 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -95,6 +95,17 @@ config USB_XHCI_PCI help Enables support for the PCI-based xHCI controller.

+config USB_XHCI_PCI_ASMEDIA
bool "Support for ASMedia xHCI USB controller"

depends on USB_XHCI_PCI

default y if ARCH_APPLE

imply FS_LOADER

help
 Enables support for loading firmware for the ASMedia xHCI
 controller.  This is needed on Apple Silicon machines that
 contain ASMedia xHCI controllers without the full USB
 firmware.
config USB_XHCI_RCAR bool "Renesas RCar USB 3.0 support" default y
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index 8dad36f936..2aa2d80263 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -52,6 +52,7 @@ obj-$(CONFIG_USB_XHCI_MTK) += xhci-mtk.o obj-$(CONFIG_USB_XHCI_MVEBU) += xhci-mvebu.o obj-$(CONFIG_USB_XHCI_OMAP) += xhci-omap.o obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o +obj-$(CONFIG_USB_XHCI_PCI_ASMEDIA) += xhci-pci-asmedia.o obj-$(CONFIG_USB_XHCI_RCAR) += xhci-rcar.o obj-$(CONFIG_USB_XHCI_STI) += dwc3-sti-glue.o obj-$(CONFIG_USB_XHCI_OCTEON) += dwc3-octeon-glue.o diff --git a/drivers/usb/host/xhci-pci-asmedia.c b/drivers/usb/host/xhci-pci-asmedia.c new file mode 100644 index 0000000000..cc1d25bde2 --- /dev/null +++ b/drivers/usb/host/xhci-pci-asmedia.c @@ -0,0 +1,394 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT +/*

ASMedia xHCI firmware loader

Copyright (C) The Asahi Linux Contributors

*/

+#include <common.h> +#include <dm.h> +#include <dm/device_compat.h> +#include <fs_loader.h> +#include <linux/iopoll.h> +#include <pci.h> +#include <usb.h> +#include <usb/xhci.h>

+/* Configuration space registers */ +#define ASMT_CFG_CONTROL 0xe0 +#define ASMT_CFG_CONTROL_WRITE BIT(1) +#define ASMT_CFG_CONTROL_READ BIT(0)

+#define ASMT_CFG_SRAM_ADDR 0xe2

+#define ASMT_CFG_SRAM_ACCESS 0xef +#define ASMT_CFG_SRAM_ACCESS_READ BIT(6) +#define ASMT_CFG_SRAM_ACCESS_ENABLE BIT(7)

+#define ASMT_CFG_DATA_READ0 0xf0 +#define ASMT_CFG_DATA_READ1 0xf4

+#define ASMT_CFG_DATA_WRITE0 0xf8 +#define ASMT_CFG_DATA_WRITE1 0xfc

+#define ASMT_CMD_GET_FWVER 0x8000060840 +#define ASMT_FWVER_ROM 0x010250090816

+/* BAR0 registers */ +#define ASMT_REG_ADDR 0x3000

+#define ASMT_REG_WDATA 0x3004 +#define ASMT_REG_RDATA 0x3008

+#define ASMT_REG_STATUS 0x3009 +#define ASMT_REG_STATUS_BUSY BIT(7)

+#define ASMT_REG_CODE_WDATA 0x3010 +#define ASMT_REG_CODE_RDATA 0x3018

+#define ASMT_MMIO_CPU_MISC 0x500e +#define ASMT_MMIO_CPU_MISC_CODE_RAM_WR BIT(0)

+#define ASMT_MMIO_CPU_MODE_NEXT 0x5040 +#define ASMT_MMIO_CPU_MODE_CUR 0x5041

+#define ASMT_MMIO_CPU_MODE_RAM BIT(0) +#define ASMT_MMIO_CPU_MODE_HALFSPEED BIT(1)

+#define ASMT_MMIO_CPU_EXEC_CTRL 0x5042 +#define ASMT_MMIO_CPU_EXEC_CTRL_RESET BIT(0) +#define ASMT_MMIO_CPU_EXEC_CTRL_HALT BIT(1)

+#define TIMEOUT_USEC 10000 +#define RESET_TIMEOUT_USEC 500000

+static int asmedia_mbox_tx(struct udevice *pdev, u64 data) +{
u8 op;

int i;

for (i = 0; i < TIMEOUT_USEC; i++) {
dm_pci_read_config8(pdev, ASMT_CFG_CONTROL, &op);
if (!(op & ASMT_CFG_CONTROL_WRITE))
	break;
udelay(1);

Would it be possible to use the wait_for_bit*() here ? Or read*poll*timeout() ?

...

}

if (op & ASMT_CFG_CONTROL_WRITE) {
dev_err(pdev,
	"Timed out on mailbox tx: 0x%llx\n",
	data);
return -ETIMEDOUT;
}

dm_pci_write_config32(pdev, ASMT_CFG_DATA_WRITE0, data);

dm_pci_write_config32(pdev, ASMT_CFG_DATA_WRITE1, data >> 32);

dm_pci_write_config8(pdev, ASMT_CFG_CONTROL,
	     ASMT_CFG_CONTROL_WRITE);

[...]

...

+static u8 asmedia_read_reg(struct udevice *pdev, struct xhci_hccr *hccr,
	   u16 addr)
+{
void __iomem *regs = (void *)hccr;

u8 status;

int ret;

ret = readb_poll_sleep_timeout(regs + ASMT_REG_STATUS, status,
		       !(status & ASMT_REG_STATUS_BUSY),
		       1000, TIMEOUT_USEC);
if (ret) {
dev_err(pdev,
	"Read reg wait timed out ([%04x])\n", addr);
return ~0;

It might make sense to return the value as parameter, and make return value of this function into integer, so error codes can be returned that way.

[...]

Mark Kettenis

15 Jul 15 Jul

2:51 p.m.

New subject: [PATCH 3/3] usb: xhci-pci: Load ASMedia XHCI controller firmware

...

Date: Fri, 14 Jul 2023 23:30:43 +0200 From: Marek Vasut marex@denx.de

Hi Marek,

Your suggestions are reasonable, but would make the driver deviate further from its Linux equivalent:

https://github.com/AsahiLinux/linux/blob/asahi/drivers/usb/host/xhci-pci-asm...

Right now I can actually easily diff the two drivers to pick up improvements. Changing things the way you suggest would make that harder.

Cheers,

Mark

...

On 7/14/23 22:43, Mark Kettenis wrote:

...
The ASMedia XHCI controller found on some of the Apple Silicon machines needs firmware to operate. Use the file system firmware loader interface to read the firmware and load it onto the controller. This allows keyboards connected to the type-A ports on these machines to function in U-Boot.

Signed-off-by: Mark Kettenis kettenis@openbsd.org

MAINTAINERS | 1 + drivers/usb/host/Kconfig | 11 + drivers/usb/host/Makefile | 1 + drivers/usb/host/xhci-pci-asmedia.c | 394 ++++++++++++++++++++++++++++ drivers/usb/host/xhci-pci.c | 9 + include/usb/xhci.h | 3 + 6 files changed, 419 insertions(+) create mode 100644 drivers/usb/host/xhci-pci-asmedia.c

diff --git a/MAINTAINERS b/MAINTAINERS index 2477923a52..0acf04fff8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -124,6 +124,7 @@ F: drivers/iommu/apple_dart.c F: drivers/nvme/nvme_apple.c F: drivers/pci/pcie_apple.c F: drivers/pinctrl/pinctrl-apple.c +F: drivers/usb/host/xhci-pci-asmedia.c F: drivers/watchdog/apple_wdt.c F: include/configs/apple.h

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 1a883babf4..ad8dfabeeb 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -95,6 +95,17 @@ config USB_XHCI_PCI help Enables support for the PCI-based xHCI controller.

+config USB_XHCI_PCI_ASMEDIA
bool "Support for ASMedia xHCI USB controller"

depends on USB_XHCI_PCI

default y if ARCH_APPLE

imply FS_LOADER

help
 Enables support for loading firmware for the ASMedia xHCI
 controller.  This is needed on Apple Silicon machines that
 contain ASMedia xHCI controllers without the full USB
 firmware.
config USB_XHCI_RCAR bool "Renesas RCar USB 3.0 support" default y
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index 8dad36f936..2aa2d80263 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -52,6 +52,7 @@ obj-$(CONFIG_USB_XHCI_MTK) += xhci-mtk.o obj-$(CONFIG_USB_XHCI_MVEBU) += xhci-mvebu.o obj-$(CONFIG_USB_XHCI_OMAP) += xhci-omap.o obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o +obj-$(CONFIG_USB_XHCI_PCI_ASMEDIA) += xhci-pci-asmedia.o obj-$(CONFIG_USB_XHCI_RCAR) += xhci-rcar.o obj-$(CONFIG_USB_XHCI_STI) += dwc3-sti-glue.o obj-$(CONFIG_USB_XHCI_OCTEON) += dwc3-octeon-glue.o diff --git a/drivers/usb/host/xhci-pci-asmedia.c b/drivers/usb/host/xhci-pci-asmedia.c new file mode 100644 index 0000000000..cc1d25bde2 --- /dev/null +++ b/drivers/usb/host/xhci-pci-asmedia.c @@ -0,0 +1,394 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT +/*

ASMedia xHCI firmware loader

Copyright (C) The Asahi Linux Contributors

*/

+#include <common.h> +#include <dm.h> +#include <dm/device_compat.h> +#include <fs_loader.h> +#include <linux/iopoll.h> +#include <pci.h> +#include <usb.h> +#include <usb/xhci.h>

+/* Configuration space registers */ +#define ASMT_CFG_CONTROL 0xe0 +#define ASMT_CFG_CONTROL_WRITE BIT(1) +#define ASMT_CFG_CONTROL_READ BIT(0)

+#define ASMT_CFG_SRAM_ADDR 0xe2

+#define ASMT_CFG_SRAM_ACCESS 0xef +#define ASMT_CFG_SRAM_ACCESS_READ BIT(6) +#define ASMT_CFG_SRAM_ACCESS_ENABLE BIT(7)

+#define ASMT_CFG_DATA_READ0 0xf0 +#define ASMT_CFG_DATA_READ1 0xf4

+#define ASMT_CFG_DATA_WRITE0 0xf8 +#define ASMT_CFG_DATA_WRITE1 0xfc

+#define ASMT_CMD_GET_FWVER 0x8000060840 +#define ASMT_FWVER_ROM 0x010250090816

+/* BAR0 registers */ +#define ASMT_REG_ADDR 0x3000

+#define ASMT_REG_WDATA 0x3004 +#define ASMT_REG_RDATA 0x3008

+#define ASMT_REG_STATUS 0x3009 +#define ASMT_REG_STATUS_BUSY BIT(7)

+#define ASMT_REG_CODE_WDATA 0x3010 +#define ASMT_REG_CODE_RDATA 0x3018

+#define ASMT_MMIO_CPU_MISC 0x500e +#define ASMT_MMIO_CPU_MISC_CODE_RAM_WR BIT(0)

+#define ASMT_MMIO_CPU_MODE_NEXT 0x5040 +#define ASMT_MMIO_CPU_MODE_CUR 0x5041

+#define ASMT_MMIO_CPU_MODE_RAM BIT(0) +#define ASMT_MMIO_CPU_MODE_HALFSPEED BIT(1)

+#define ASMT_MMIO_CPU_EXEC_CTRL 0x5042 +#define ASMT_MMIO_CPU_EXEC_CTRL_RESET BIT(0) +#define ASMT_MMIO_CPU_EXEC_CTRL_HALT BIT(1)

+#define TIMEOUT_USEC 10000 +#define RESET_TIMEOUT_USEC 500000

+static int asmedia_mbox_tx(struct udevice *pdev, u64 data) +{
u8 op;

int i;

for (i = 0; i < TIMEOUT_USEC; i++) {
dm_pci_read_config8(pdev, ASMT_CFG_CONTROL, &op);
if (!(op & ASMT_CFG_CONTROL_WRITE))
	break;
udelay(1);
Would it be possible to use the wait_for_bit*() here ? Or read*poll*timeout() ?

...
}

if (op & ASMT_CFG_CONTROL_WRITE) {
dev_err(pdev,
	"Timed out on mailbox tx: 0x%llx\n",
	data);
return -ETIMEDOUT;
}

dm_pci_write_config32(pdev, ASMT_CFG_DATA_WRITE0, data);

dm_pci_write_config32(pdev, ASMT_CFG_DATA_WRITE1, data >> 32);

dm_pci_write_config8(pdev, ASMT_CFG_CONTROL,
	     ASMT_CFG_CONTROL_WRITE);
[...]

...
+static u8 asmedia_read_reg(struct udevice *pdev, struct xhci_hccr *hccr,
	   u16 addr)
+{
void __iomem *regs = (void *)hccr;

u8 status;

int ret;

ret = readb_poll_sleep_timeout(regs + ASMT_REG_STATUS, status,
		       !(status & ASMT_REG_STATUS_BUSY),
		       1000, TIMEOUT_USEC);
if (ret) {
dev_err(pdev,
	"Read reg wait timed out ([%04x])\n", addr);
return ~0;
It might make sense to return the value as parameter, and make return value of this function into integer, so error codes can be returned that way.

[...]

Marek Vasut

18 Jul 18 Jul

1:10 a.m.

New subject: [PATCH 3/3] usb: xhci-pci: Load ASMedia XHCI controller firmware

On 7/15/23 14:51, Mark Kettenis wrote:

...

...
Date: Fri, 14 Jul 2023 23:30:43 +0200 From: Marek Vasut marex@denx.de

Hi Marek,

Hi,

...

Your suggestions are reasonable, but would make the driver deviate further from its Linux equivalent:

https://github.com/AsahiLinux/linux/blob/asahi/drivers/usb/host/xhci-pci-asm...

Can the changes be upstreamed to Linux, if they sound reasonable ?

...

Right now I can actually easily diff the two drivers to pick up improvements. Changing things the way you suggest would make that harder.

[...]

665

Age (days ago)

668

Last active (days ago)

List overview

Download

11 comments

3 participants

tags (0)

participants (3)

Marek Vasut
Mark Kettenis
Mark Kettenis