[PATCH 3/3] usb: xhci-pci: Load ASMedia XHCI controller firmware
Mark Kettenis
mark.kettenis at xs4all.nl
Sat Jul 15 14:51:43 CEST 2023
> Date: Fri, 14 Jul 2023 23:30:43 +0200
> From: Marek Vasut <marex at 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-asmedia.c
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 at 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.
>
> [...]
>
More information about the U-Boot
mailing list