From owner-p4-projects@FreeBSD.ORG Sun Dec 14 21:33:26 2008 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id A37A91065676; Sun, 14 Dec 2008 21:33:26 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 65F81106564A; Sun, 14 Dec 2008 21:33:26 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 0A1DB8FC13; Sun, 14 Dec 2008 21:33:25 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.2/8.14.1) with ESMTP id mBELWuWD021936; Sun, 14 Dec 2008 14:32:56 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Sun, 14 Dec 2008 14:32:57 -0700 (MST) Message-Id: <20081214.143257.1645216129.imp@bsdimp.com> To: sam@freebsd.org From: "M. Warner Losh" In-Reply-To: <200812101754.mBAHsoEI024577@repoman.freebsd.org> References: <200812101754.mBAHsoEI024577@repoman.freebsd.org> X-Mailer: Mew version 5.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: perforce@freebsd.org Subject: Re: PERFORCE change 154450 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 14 Dec 2008 21:33:27 -0000 In message: <200812101754.mBAHsoEI024577@repoman.freebsd.org> Sam Leffler 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 : #include : #include : +#include : #include : #include : +#include : + : #include : -#include : #include : : #include : @@ -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 */ :