> Date: Wed, 29 Dec 2021 17:16:36 +0100
> From: Patrick Wildt <[email protected]>
>
> On Wed, Dec 29, 2021 at 05:03:50PM +0100, Patrick Wildt wrote:
> > Hi,
> >
> > a device I have still has oHCI and eHCI controllers, which are needed to
> > be able to use the second USB port.
> >
> > ehci(4) at acpi(4) is plain simple, as it attaches to PNP0D20. I have
> > copied the xhci(4) at acpi(4) file and added the functional changes that
> > exist between xhci(4) and ehci(4) at fdt(4).
> >
> > ohci(4) is very similar, I have also added the functional changes that
> > way. There's a difference in how they attach: It uses a PRP0001 ID,
> > which basically is a device-tree namespace link. That means we match
> > on that link *and* have to check the 'compatible' property.
> >
> > With these changes, my second USB port works just fine.
> >
> > Patrick
>
> Actually, there's a better way to match ohci(4). We can match on _CLS,
> like we do for ahci(4) at acpi(4). Updated diff below.
Yes, that is cleaner...
> diff --git a/sys/arch/arm64/conf/GENERIC b/sys/arch/arm64/conf/GENERIC
> index 19fd6e03c4f..879d024503f 100644
> --- a/sys/arch/arm64/conf/GENERIC
> +++ b/sys/arch/arm64/conf/GENERIC
> @@ -54,7 +54,9 @@ com* at acpi?
> dwgpio* at acpi?
> dwiic* at acpi?
> iic* at dwiic?
> +ehci* at acpi?
> ipmi* at acpi?
> +ohci* at acpi?
> pluart* at acpi?
> sdhc* at acpi?
> xhci* at acpi?
> diff --git a/sys/arch/arm64/conf/RAMDISK b/sys/arch/arm64/conf/RAMDISK
> index b505e37b399..caf2eea44b8 100644
> --- a/sys/arch/arm64/conf/RAMDISK
> +++ b/sys/arch/arm64/conf/RAMDISK
> @@ -44,6 +44,8 @@ pci* at acpipci?
> ahci* at acpi?
> ccp* at acpi? # AMD Cryptographic Co-processor
> com* at acpi?
> +ehci* at acpi?
> +ohci* at acpi?
> pluart* at acpi?
> sdhc* at acpi?
> xhci* at acpi?
> diff --git a/sys/dev/acpi/ehci_acpi.c b/sys/dev/acpi/ehci_acpi.c
> new file mode 100644
> index 00000000000..cd1777f26e4
> --- /dev/null
> +++ b/sys/dev/acpi/ehci_acpi.c
> @@ -0,0 +1,125 @@
> +/* $OpenBSD: ehci_acpi.c,v 1.4 2021/12/21 20:53:46 kettenis Exp $ */
> +/*
> + * Copyright (c) 2018 Mark Kettenis
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <sys/param.h>
> +#include <sys/systm.h>
> +#include <sys/device.h>
> +
> +#include <machine/bus.h>
> +#include <machine/intr.h>
> +
> +#include <dev/acpi/acpireg.h>
> +#include <dev/acpi/acpivar.h>
> +#include <dev/acpi/acpidev.h>
> +#include <dev/acpi/amltypes.h>
> +#include <dev/acpi/dsdt.h>
> +
> +#include <dev/usb/usb.h>
> +#include <dev/usb/usbdi.h>
> +#include <dev/usb/usbdivar.h>
> +#include <dev/usb/usb_mem.h>
> +
> +#include <dev/usb/ehcireg.h>
> +#include <dev/usb/ehcivar.h>
> +
> +struct ehci_acpi_softc {
> + struct ehci_softc sc;
> + struct acpi_softc *sc_acpi;
> + struct aml_node *sc_node;
> + void *sc_ih;
> +};
> +
> +int ehci_acpi_match(struct device *, void *, void *);
> +void ehci_acpi_attach(struct device *, struct device *, void *);
> +
> +struct cfattach ehci_acpi_ca = {
> + sizeof(struct ehci_acpi_softc), ehci_acpi_match, ehci_acpi_attach
> +};
> +
> +const char *ehci_hids[] = {
> + "PNP0D20",
> + NULL
> +};
> +
> +int
> +ehci_acpi_match(struct device *parent, void *match, void *aux)
> +{
> + struct acpi_attach_args *aaa = aux;
> + struct cfdata *cf = match;
> +
> + if (aaa->aaa_naddr < 1 || aaa->aaa_nirq < 1)
> + return 0;
> + return acpi_matchhids(aaa, ehci_hids, cf->cf_driver->cd_name);
> +}
> +
> +void
> +ehci_acpi_attach(struct device *parent, struct device *self, void *aux)
> +{
> + struct ehci_acpi_softc *sc = (struct ehci_acpi_softc *)self;
> + struct acpi_attach_args *aaa = aux;
> + int error;
> +
> + sc->sc_acpi = (struct acpi_softc *)parent;
> + sc->sc_node = aaa->aaa_node;
> + printf(" %s", sc->sc_node->name);
> +
> + printf(" addr 0x%llx/0x%llx", aaa->aaa_addr[0], aaa->aaa_size[0]);
> + printf(" irq %d", aaa->aaa_irq[0]);
> +
> + sc->sc.iot = aaa->aaa_bst[0];
> + sc->sc.sc_size = aaa->aaa_size[0];
> + sc->sc.sc_bus.dmatag = aaa->aaa_dmat;
> +
> + if (bus_space_map(sc->sc.iot, aaa->aaa_addr[0], aaa->aaa_size[0],
> + 0, &sc->sc.ioh)) {
> + printf(": can't map registers\n");
> + return;
> + }
> +
> + /* Disable interrupts, so we don't get any spurious ones. */
> + sc->sc.sc_offs = EREAD1(&sc->sc, EHCI_CAPLENGTH);
> + EOWRITE2(&sc->sc, EHCI_USBINTR, 0);
> +
> + sc->sc_ih = acpi_intr_establish(aaa->aaa_irq[0], aaa->aaa_irq_flags[0],
> + IPL_USB, ehci_intr, sc, sc->sc.sc_bus.bdev.dv_xname);
> + if (sc->sc_ih == NULL) {
> + printf(": can't establish interrupt\n");
> + goto unmap;
> + }
> +
> + printf("\n");
> +
> + strlcpy(sc->sc.sc_vendor, "Generic", sizeof(sc->sc.sc_vendor));
> + if ((error = ehci_init(&sc->sc)) != 0) {
> + printf("%s: init failed, error=%d\n",
> + sc->sc.sc_bus.bdev.dv_xname, error);
> + goto disestablish_ret;
> + }
> +
> + /* Attach usb device. */
> + config_found(self, &sc->sc.sc_bus, usbctlprint);
> +
> + return;
> +
> +disestablish_ret:
> +#ifdef notyet
> + acpi_intr_disestablish(sc->sc_ih);
We have acpi_intr_disestablish() on arm64 now...
> +#endif
> +unmap:
> + bus_space_unmap(sc->sc.iot, sc->sc.ioh, sc->sc.sc_size);
> + return;
> +}
> diff --git a/sys/dev/acpi/files.acpi b/sys/dev/acpi/files.acpi
> index 95df4ffe448..da8e38d8a79 100644
> --- a/sys/dev/acpi/files.acpi
> +++ b/sys/dev/acpi/files.acpi
> @@ -187,6 +187,14 @@ file dev/acpi/pluart_acpi.c pluart_acpi
> attach sdhc at acpi with sdhc_acpi
> file dev/acpi/sdhc_acpi.c sdhc_acpi
>
> +# OHCI
> +attach ohci at acpi with ohci_acpi
> +file dev/acpi/ohci_acpi.c ohci_acpi
> +
> +# EHCI
> +attach ehci at acpi with ehci_acpi
> +file dev/acpi/ehci_acpi.c ehci_acpi
> +
> # XHCI
> attach xhci at acpi with xhci_acpi
> file dev/acpi/xhci_acpi.c xhci_acpi
> diff --git a/sys/dev/acpi/ohci_acpi.c b/sys/dev/acpi/ohci_acpi.c
> new file mode 100644
> index 00000000000..f596305378a
> --- /dev/null
> +++ b/sys/dev/acpi/ohci_acpi.c
> @@ -0,0 +1,172 @@
> +/* $OpenBSD: ohci_acpi.c,v 1.4 2021/12/21 20:53:46 kettenis Exp $ */
> +/*
> + * Copyright (c) 2018 Mark Kettenis
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <sys/param.h>
> +#include <sys/systm.h>
> +#include <sys/device.h>
> +
> +#include <machine/bus.h>
> +#include <machine/intr.h>
> +
> +#include <dev/acpi/acpireg.h>
> +#include <dev/acpi/acpivar.h>
> +#include <dev/acpi/acpidev.h>
> +#include <dev/acpi/amltypes.h>
> +#include <dev/acpi/dsdt.h>
> +
> +#include <dev/usb/usb.h>
> +#include <dev/usb/usbdi.h>
> +#include <dev/usb/usbdivar.h>
> +#include <dev/usb/usb_mem.h>
> +
> +#include <dev/usb/ohcireg.h>
> +#include <dev/usb/ohcivar.h>
> +
> +struct ohci_acpi_softc {
> + struct ohci_softc sc;
> + struct acpi_softc *sc_acpi;
> + struct aml_node *sc_node;
> + void *sc_ih;
> +};
> +
> +int ohci_acpi_match(struct device *, void *, void *);
> +void ohci_acpi_attach(struct device *, struct device *, void *);
> +
> +struct cfattach ohci_acpi_ca = {
> + sizeof(struct ohci_acpi_softc), ohci_acpi_match, ohci_acpi_attach
> +};
> +
> +void ohci_acpi_attach_deferred(struct device *);
> +
> +int
> +ohci_acpi_match(struct device *parent, void *match, void *aux)
> +{
> + struct acpi_attach_args *aaa = aux;
> +
> + return acpi_matchcls(aaa, PCI_CLASS_SERIALBUS,
> + PCI_SUBCLASS_SERIALBUS_USB, PCI_INTERFACE_OHCI);
> +}
> +
> +void
> +ohci_acpi_attach(struct device *parent, struct device *self, void *aux)
> +{
> + struct ohci_acpi_softc *sc = (struct ohci_acpi_softc *)self;
> + struct acpi_attach_args *aaa = aux;
> + int error;
> +
> + sc->sc_acpi = (struct acpi_softc *)parent;
> + sc->sc_node = aaa->aaa_node;
> + printf(" %s", sc->sc_node->name);
> +
> + printf(" addr 0x%llx/0x%llx", aaa->aaa_addr[0], aaa->aaa_size[0]);
> + printf(" irq %d", aaa->aaa_irq[0]);
> +
> + sc->sc.iot = aaa->aaa_bst[0];
> + sc->sc.sc_size = aaa->aaa_size[0];
> + sc->sc.sc_bus.dmatag = aaa->aaa_dmat;
> +
> + if (bus_space_map(sc->sc.iot, aaa->aaa_addr[0], aaa->aaa_size[0],
> + 0, &sc->sc.ioh)) {
> + printf(": can't map registers\n");
> + return;
> + }
> +
> + /* Record what interrupts were enabled by SMM/BIOS. */
> + sc->sc.sc_intre = bus_space_read_4(sc->sc.iot, sc->sc.ioh,
> + OHCI_INTERRUPT_ENABLE);
> +
> + /* Disable interrupts, so we don't get any spurious ones. */
> + bus_space_write_4(sc->sc.iot, sc->sc.ioh, OHCI_INTERRUPT_DISABLE,
> + OHCI_MIE);
> +
> + bus_space_barrier(sc->sc.iot, sc->sc.ioh, 0, sc->sc.sc_size,
> + BUS_SPACE_BARRIER_READ|BUS_SPACE_BARRIER_WRITE);
> + bus_space_write_4(sc->sc.iot, sc->sc.ioh,
> + OHCI_INTERRUPT_DISABLE, OHCI_MIE);
> +
> + /* Map and establish the interrupt. */
> + splassert(IPL_USB);
I don't think that splassert is very useful.
> + sc->sc_ih = acpi_intr_establish(aaa->aaa_irq[0], aaa->aaa_irq_flags[0],
> + IPL_USB, ohci_intr, sc, sc->sc.sc_bus.bdev.dv_xname);
> + if (sc->sc_ih == NULL) {
> + printf(": can't establish interrupt\n");
> + goto unmap;
> + }
> +
> + printf(": ");
> +
> + strlcpy(sc->sc.sc_vendor, "Generic", sizeof(sc->sc.sc_vendor));
> +
> + /* Display revision. Legacy handover isn't needed as there's no smm */
That is not necessarily true with ACPI. But I suppose as long as we
only enable this on arm64...
> + if (ohci_checkrev(&sc->sc) != USBD_NORMAL_COMPLETION) {
> + goto disestablish_ret;
> + }
Don't need those curly braces.
> +
> + /* Ignore interrupts for now */
> + sc->sc.sc_bus.dying = 1;
> +
> + config_defer(self, ohci_acpi_attach_deferred);
> +
> + if ((error = ohci_init(&sc->sc)) != 0) {
I hate assignments in if statements.
> + printf("%s: init failed, error=%d\n",
> + sc->sc.sc_bus.bdev.dv_xname, error);
> + goto disestablish_ret;
> + }
> +
> + /* Attach usb device. */
> + config_found(self, &sc->sc.sc_bus, usbctlprint);
> +
> + return;
> +
> +disestablish_ret:
> +#ifdef notyet
> + acpi_intr_disestablish(sc->sc_ih);
We have acpi_intr_disestablish() on arm64 now...
> +#endif
> +unmap:
> + bus_space_unmap(sc->sc.iot, sc->sc.ioh, sc->sc.sc_size);
> + return;
> +}
> +
> +void
> +ohci_acpi_attach_deferred(struct device *self)
> +{
> + struct ohci_acpi_softc *sc = (struct ohci_acpi_softc *)self;
> + usbd_status r;
> + int s;
> +
> + s = splusb();
> +
> + sc->sc.sc_bus.dying = 0;
> +
> + r = ohci_init(&sc->sc);
> +
> + splx(s);
Formatting is a bit weird here. I would remove the blank lines above.
> + if (r != USBD_NORMAL_COMPLETION) {
> + printf("%s: init failed, error=%d\n",
> + sc->sc.sc_bus.bdev.dv_xname, r);
> +#ifdef notyet
> + acpi_intr_disestablish(sc->sc_ih);
> +#endif
> + sc->sc_ih = NULL;
> + bus_space_unmap(sc->sc.iot, sc->sc.ioh, sc->sc.sc_size);
> + sc->sc.sc_size = 0;
> + return;
> + }
> +
> + /* Attach usb device. */
> + config_found(self, &sc->sc.sc_bus, usbctlprint);
> +}
>
>
