Date: Sun, 14 Dec 2008 14:32:57 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: sam@freebsd.org Cc: perforce@freebsd.org Subject: Re: PERFORCE change 154450 for review Message-ID: <20081214.143257.1645216129.imp@bsdimp.com> In-Reply-To: <200812101754.mBAHsoEI024577@repoman.freebsd.org> References: <200812101754.mBAHsoEI024577@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <200812101754.mBAHsoEI024577@repoman.freebsd.org>
Sam Leffler <sam@freebsd.org> writes:
: http://perforce.freebsd.org/chv.cgi?CH=154450
:
: Change 154450 by sam@sam_ebb on 2008/12/10 17:53:56
:
: Remove EHCI_SCFLG_BIGENDIAN; this appears to be the wrong
: approach. Handle the 1- and 2-byte register ops by overriding
: the bus tag ops in the bus-shim. Also mark the controller with
: EHCI_SCFLG_FORCESPEED as we need to identify the device speed
: from the PortStatus following a port enable.
FYI: On the Alchemy USB device, one has to always to 4-byte operations
on the hardware for a similar flag to be effective. Thanks for
stubbing your toe on this limitation in our stack so I won't have to
stub mine nearly as badly...
Warner
: Affected files ...
:
: .. //depot/projects/vap/sys/arm/xscale/ixp425/ixp435_ehci.c#2 edit
: .. //depot/projects/vap/sys/dev/usb/ehci.c#16 edit
: .. //depot/projects/vap/sys/dev/usb/ehcivar.h#10 edit
:
: Differences ...
:
: ==== //depot/projects/vap/sys/arm/xscale/ixp425/ixp435_ehci.c#2 (text+ko) ====
:
: @@ -38,10 +38,12 @@
: #include <sys/lock.h>
: #include <sys/mutex.h>
: #include <sys/bus.h>
: +#include <sys/endian.h>
: #include <sys/queue.h>
: #include <sys/lockmgr.h>
: +#include <sys/rman.h>
: +
: #include <machine/bus.h>
: -#include <sys/rman.h>
: #include <machine/resource.h>
:
: #include <dev/usb/usb.h>
: @@ -58,11 +60,21 @@
: #define EHCI_VENDORID_IXP4XX 0x42fa05
: #define EHCI_HC_DEVSTR "IXP4XX Integrated USB 2.0 controller"
:
: -static device_attach_t ehci_ixp_attach;
: -static device_detach_t ehci_ixp_detach;
: -static device_shutdown_t ehci_ixp_shutdown;
: -static device_suspend_t ehci_ixp_suspend;
: -static device_resume_t ehci_ixp_resume;
: +struct ixp_ehci_softc {
: + ehci_softc_t base; /* storage for EHCI code */
: + bus_space_tag_t iot;
: + bus_space_handle_t ioh;
: + struct bus_space tag; /* tag for private bus space ops */
: +};
: +
: +static int ehci_ixp_detach(device_t self);
: +
: +static uint8_t ehci_bs_r_1(void *, bus_space_handle_t, bus_size_t);
: +static void ehci_bs_w_1(void *, bus_space_handle_t, bus_size_t, u_int8_t);
: +static uint16_t ehci_bs_r_2(void *, bus_space_handle_t, bus_size_t);
: +static void ehci_bs_w_2(void *, bus_space_handle_t, bus_size_t, uint16_t);
: +static uint32_t ehci_bs_r_4(void *, bus_space_handle_t, bus_size_t);
: +static void ehci_bs_w_4(void *, bus_space_handle_t, bus_size_t, uint32_t);
:
: static int
: ehci_ixp_suspend(device_t self)
: @@ -112,7 +124,8 @@
: static int
: ehci_ixp_attach(device_t self)
: {
: - ehci_softc_t *sc = device_get_softc(self);
: + struct ixp_ehci_softc *isc = device_get_softc(self);
: + ehci_softc_t *sc = &isc->base;
: int err, rid;
:
: sc->sc_bus.usbrev = USBREV_2_0;
: @@ -126,16 +139,27 @@
: device_printf(self, "Could not map memory\n");
: return ENXIO;
: }
: - sc->iot = rman_get_bustag(sc->io_res);
: +
: + /*
: + * Craft special resource for bus space ops that handle
: + * byte-alignment of non-word addresses. Also, since
: + * we're already intercepting bus space ops we handle
: + * the register window offset that could otherwise be
: + * done with bus_space_subregion.
: + */
: + isc->iot = rman_get_bustag(sc->io_res);
: + isc->tag.bs_cookie = isc->iot;
: + /* read single */
: + isc->tag.bs_r_1 = ehci_bs_r_1,
: + isc->tag.bs_r_2 = ehci_bs_r_2,
: + isc->tag.bs_r_4 = ehci_bs_r_4,
: + /* write (single) */
: + isc->tag.bs_w_1 = ehci_bs_w_1,
: + isc->tag.bs_w_2 = ehci_bs_w_2,
: + isc->tag.bs_w_4 = ehci_bs_w_4,
: +
: + sc->iot = &isc->tag;
: sc->ioh = rman_get_bushandle(sc->io_res);
: -
: - /* shift register window for EHCI driver */
: - if (bus_space_subregion(sc->iot, sc->ioh,
: - 0x100, IXP435_USB1_SIZE - 0x100, &sc->ioh) != 0) {
: - device_printf(self, "Could not setup subregion for USB host"
: - "registers\n");
: - return ENXIO;
: - }
: sc->sc_size = IXP435_USB1_SIZE - 0x100;
:
: rid = 0;
: @@ -197,10 +221,13 @@
: * Arrange to force Host mode, select big-endian byte alignment,
: * and arrange to not terminate reset operations (the adapter
: * will ignore it if we do but might as well save a reg write).
: + * Also, the controller has an embedded Transaction Translator
: + * which means port speed must be read from the Port Status
: + * register following a port enable.
: */
: sc->sc_flags |= EHCI_SCFLG_SETMODE
: - | EHCI_SCFLG_BIGENDIAN
: | EHCI_SCFLG_NORESTERM
: + | EHCI_SCFLG_FORCESPEED
: ;
: err = ehci_init(sc);
: if (!err) {
: @@ -219,7 +246,8 @@
: static int
: ehci_ixp_detach(device_t self)
: {
: - ehci_softc_t *sc = device_get_softc(self);
: + struct ixp_ehci_softc *isc = device_get_softc(self);
: + ehci_softc_t *sc = &isc->base;
: int err;
:
: if (sc->sc_flags & EHCI_SCFLG_DONEINIT) {
: @@ -262,6 +290,48 @@
: return 0;
: }
:
: +/*
: + * Bus space accessors for PIO operations.
: + */
: +
: +static uint8_t
: +ehci_bs_r_1(void *t, bus_space_handle_t h, bus_size_t o)
: +{
: + return bus_space_read_1((bus_space_tag_t) t, h,
: + 0x100 + (o &~ 3) + (3 - (o & 3)));
: +}
: +
: +static void
: +ehci_bs_w_1(void *t, bus_space_handle_t h, bus_size_t o, u_int8_t v)
: +{
: + panic("%s", __func__);
: +}
: +
: +static uint16_t
: +ehci_bs_r_2(void *t, bus_space_handle_t h, bus_size_t o)
: +{
: + return bus_space_read_2((bus_space_tag_t) t, h,
: + 0x100 + (o &~ 3) + (2 - (o & 3)));
: +}
: +
: +static void
: +ehci_bs_w_2(void *t, bus_space_handle_t h, bus_size_t o, uint16_t v)
: +{
: + panic("%s", __func__);
: +}
: +
: +static uint32_t
: +ehci_bs_r_4(void *t, bus_space_handle_t h, bus_size_t o)
: +{
: + return bus_space_read_4((bus_space_tag_t) t, h, 0x100 + o);
: +}
: +
: +static void
: +ehci_bs_w_4(void *t, bus_space_handle_t h, bus_size_t o, uint32_t v)
: +{
: + bus_space_write_4((bus_space_tag_t) t, h, 0x100 + o, v);
: +}
: +
: static device_method_t ehci_methods[] = {
: /* Device interface */
: DEVMETHOD(device_probe, ehci_ixp_probe),
: @@ -280,7 +350,7 @@
: static driver_t ehci_driver = {
: "ehci",
: ehci_methods,
: - sizeof(ehci_softc_t),
: + sizeof(struct ixp_ehci_softc),
: };
: static devclass_t ehci_devclass;
: DRIVER_MODULE(ehci, ixp, ehci_driver, ehci_devclass, 0, 0);
:
: ==== //depot/projects/vap/sys/dev/usb/ehci.c#16 (text+ko) ====
:
: @@ -351,12 +351,10 @@
: usb_delay_ms(&sc->sc_bus, 1);
: hcr = EOREAD4(sc, EHCI_USBCMD) & EHCI_CMD_HCRESET;
: if (!hcr) {
: - if (sc->sc_flags & (EHCI_SCFLG_SETMODE | EHCI_SCFLG_BIGENDIAN)) {
: + if (sc->sc_flags & EHCI_SCFLG_SETMODE) {
: /*
: * Force USBMODE as requested. Controllers
: - * may have multiple operating modes and on
: - * some platforms we need to force big-endian
: - * byte alignement of data structures.
: + * may have multiple operating modes.
: */
: uint32_t usbmode = EOREAD4(sc, EHCI_USBMODE);
: if (sc->sc_flags & EHCI_SCFLG_SETMODE) {
: @@ -364,11 +362,6 @@
: device_printf(sc->sc_bus.bdev,
: "set host controller mode\n");
: }
: - if (sc->sc_flags & EHCI_SCFLG_BIGENDIAN) {
: - usbmode |= EHCI_UM_ES_BE;
: - device_printf(sc->sc_bus.bdev,
: - "set big-endian byte alignment\n");
: - }
: EOWRITE4(sc, EHCI_USBMODE, usbmode);
: }
: return (USBD_NORMAL_COMPLETION);
: @@ -395,11 +388,9 @@
:
: /* NB: must handle byte-order manually before ehci_hcreset */
:
: - sc->sc_offs = EREAD1(sc, sc->sc_flags & EHCI_SCFLG_BIGENDIAN ?
: - 3-EHCI_CAPLENGTH : EHCI_CAPLENGTH);
: + sc->sc_offs = EREAD1(sc, EHCI_CAPLENGTH);
:
: - version = EREAD2(sc, sc->sc_flags & EHCI_SCFLG_BIGENDIAN ?
: - 2-EHCI_HCIVERSION : EHCI_HCIVERSION);
: + version = EREAD2(sc, EHCI_HCIVERSION);
: device_printf(sc->sc_bus.bdev, "EHCI version %x.%x\n",
: version >> 8, version & 0xff);
:
:
: ==== //depot/projects/vap/sys/dev/usb/ehcivar.h#10 (text+ko) ====
:
: @@ -125,7 +125,6 @@
: #define EHCI_SCFLG_SETMODE 0x0004 /* set bridge mode again after init (Marvell) */
: #define EHCI_SCFLG_FORCESPEED 0x0008 /* force speed (Marvell) */
: #define EHCI_SCFLG_NORESTERM 0x0010 /* don't terminate reset sequence (Marvell) */
: -#define EHCI_SCFLG_BIGENDIAN 0x0020 /* set big-endian select on reset */
:
: typedef struct ehci_softc {
: struct usbd_bus sc_bus; /* base device */
:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081214.143257.1645216129.imp>