From owner-freebsd-arch@FreeBSD.ORG Sun Sep 2 12:37:35 2012 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id E3BFA106566C for ; Sun, 2 Sep 2012 12:37:35 +0000 (UTC) (envelope-from loos.br@gmail.com) Received: from mail-gg0-f182.google.com (mail-gg0-f182.google.com [209.85.161.182]) by mx1.freebsd.org (Postfix) with ESMTP id 931238FC12 for ; Sun, 2 Sep 2012 12:37:35 +0000 (UTC) Received: by ggnk4 with SMTP id k4so838175ggn.13 for ; Sun, 02 Sep 2012 05:37:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:content-type:subject:date:message-id:to:mime-version:x-mailer; bh=4mvb+U5UWfYIQu/rzEMTKbzrYFSACeI9Buwnw5eFRyI=; b=ueLz5yBQEXuA0tkBwUqqXcAiGrPtanq5uif1F6j44MC3r9FExV0G22kiGgC4JwWZag aWYVBBleswunG28pmBFxGU6zf+mQRuAEnu4zT46dsFjpSLbO/LL5nxmL8BNr/eWcKjoU qmnFuhmCUjZRY+M00P0MwUxvBXnZTOQN/nz8y3/2xPZJjceiQrOq3Sov9AwaLzPAv52G J225eX1l1MYwKczz0O2TOihVRJ/7+X8FFUCItL3be0RYnm1cTMr3pdZHwjdZSFupCtT0 Ed6g+C7VcoAVhUaNrhGRq8Y/kyqA0dd6WdobCVSV5o6BtoLGOtFjJ+mIwVsQkD1cCa0U uW8w== Received: by 10.236.115.138 with SMTP id e10mr12723618yhh.79.1346589448693; Sun, 02 Sep 2012 05:37:28 -0700 (PDT) Received: from [192.168.0.53] ([187.120.131.141]) by mx.google.com with ESMTPS id v8sm18930927yhi.15.2012.09.02.05.37.26 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 02 Sep 2012 05:37:27 -0700 (PDT) From: Luiz Otavio O Souza Content-Type: multipart/mixed; boundary=Apple-Mail-58-1062540785 Date: Sun, 2 Sep 2012 09:37:24 -0300 Message-Id: To: freebsd-arch@freebsd.org Mime-Version: 1.0 (Apple Message framework v1084) X-Mailer: Apple Mail (2.1084) Subject: spibus access serialization X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Sep 2012 12:37:36 -0000 --Apple-Mail-58-1062540785 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii Hi, I've some embedded systems with spi devices that share the same spibus = and because of that i'm working to get some kind of access serialization = on spibus and also protection to devices which need a series of spi = transfers to achieve some goal. The SPI usage (with all the patches applied) goes like this: /* Wait until the spibus is free. When free, acquire the bus and select = the device */ SPIBUS_ACQUIRE_BUS(spibus, device); /* Program the CPLD to read data from NAND */ SPIBUS_TRANSFER(spibus, device, &cmd); /* Read 'n' bytes from CPLD */ SPIBUS_TRANSFER(spibus, device, &cmd); /* Release the spibus and deselect the device */ SPIBUS_RELEASE_BUS(spibus, device); While today everything is done inside SPIBUS_TRANSFER(). The patch spibus-01.diff adds the bus methods and the default methods. The spibus-02-devices.diff adds the needed glue to all spi devices = (dev/flash/at45d.c, dev/flash/mx25l.c, arm/lpc/ssd1289.c, = mips/atheros/pcf2123_rtc.c). As the default methods are just nops, there = are no functional changes. On spibus-03-controllers.diff we finally add the serialization methods = to spi controllers (mips/atheros/ar71xx_spi.c and arm/lpc/lpc_spi.c) and = change the device CS to happen on bus acquire and release and not on = start and end of each transfer. The spi controller on arm/at91/at91_spi.c wasn't changed because looks = like it will be need to move the device CS control from the controller = and use it as a GPIO pin. I need to read more of at91 code before i can = suggest some change there. Until there it will work just as now (no = serialization and selecting/deselecting the device within each = transfer). Comments ? Thanks, Luiz --Apple-Mail-58-1062540785 Content-Disposition: attachment; filename=spibus-01.diff Content-Type: application/octet-stream; name="spibus-01.diff" Content-Transfer-Encoding: 7bit Index: dev/spibus/spibus.c =================================================================== --- dev/spibus/spibus.c (revision 239916) +++ dev/spibus/spibus.c (working copy) @@ -155,6 +155,20 @@ resource_int_value(dname, dunit, "cs", &devi->cs); } +static void +spibus_acquire_bus_impl(device_t dev, device_t child) +{ + + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), child); +} + +static void +spibus_release_bus_impl(device_t dev, device_t child) +{ + + SPIBUS_RELEASE_BUS(device_get_parent(dev), child); +} + static int spibus_transfer_impl(device_t dev, device_t child, struct spi_command *cmd) { @@ -180,6 +194,8 @@ DEVMETHOD(bus_hinted_child, spibus_hinted_child), /* spibus interface */ + DEVMETHOD(spibus_acquire_bus, spibus_acquire_bus_impl), + DEVMETHOD(spibus_release_bus, spibus_release_bus_impl), DEVMETHOD(spibus_transfer, spibus_transfer_impl), DEVMETHOD_END Index: dev/spibus/spibus_if.m =================================================================== --- dev/spibus/spibus_if.m (revision 239916) +++ dev/spibus/spibus_if.m (working copy) @@ -32,6 +32,37 @@ INTERFACE spibus; # +# Default implementation +# +CODE { + static void + null_acquire_bus(device_t dev, device_t child) + { + } + + static void + null_release_bus(device_t dev, device_t child) + { + } +}; + +# +# Acquire bus and select the device +# +METHOD void acquire_bus { + device_t dev; + device_t child; +} DEFAULT null_acquire_bus; + +# +# Release bus and deselect the device +# +METHOD void release_bus { + device_t dev; + device_t child; +} DEFAULT null_release_bus; + +# # Do a spi command # METHOD int transfer { --Apple-Mail-58-1062540785 Content-Disposition: attachment; filename=spibus-02-devices.diff Content-Type: application/octet-stream; name="spibus-02-devices.diff" Content-Transfer-Encoding: 7bit Index: dev/flash/at45d.c =================================================================== --- dev/flash/at45d.c (revision 239916) +++ dev/flash/at45d.c (working copy) @@ -132,7 +132,9 @@ cmd.tx_cmd = txBuf; cmd.rx_cmd = rxBuf; cmd.rx_cmd_sz = cmd.tx_cmd_sz = 2; + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev); err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd); + SPIBUS_RELEASE_BUS(device_get_parent(dev), dev); *status = rxBuf[1]; return (err); } @@ -152,7 +154,9 @@ cmd.tx_cmd = &txBuf; cmd.rx_cmd = &rxBuf; cmd.tx_cmd_sz = cmd.rx_cmd_sz = 5; + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev); err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd); + SPIBUS_RELEASE_BUS(device_get_parent(dev), dev); if (err) return (err); memcpy(resp, rxBuf + 1, 4); @@ -365,8 +369,10 @@ txBuf[2] = ((addr >> 8) & 0xff); txBuf[3] = 0; cmd.tx_data_sz = cmd.rx_data_sz = 0; + SPIBUS_ACQUIRE_BUS(pdev, dev); err = SPIBUS_TRANSFER(pdev, dev, &cmd); + SPIBUS_RELEASE_BUS(pdev, dev); if (err == 0) err = at45d_wait_ready(dev, &status); @@ -383,7 +389,9 @@ txBuf[2] = ((addr >> 8) & 0xff); txBuf[3] = (addr & 0xff); cmd.tx_data_sz = cmd.rx_data_sz = len; + SPIBUS_ACQUIRE_BUS(pdev, dev); err = SPIBUS_TRANSFER(pdev, dev, &cmd); + SPIBUS_RELEASE_BUS(pdev, dev); if (err == 0 && bp->bio_cmd != BIO_READ) err = at45d_wait_ready(dev, &status); if (err != 0) { @@ -397,7 +405,9 @@ txBuf[2] = ((addr >> 8) & 0xff); txBuf[3] = 0; cmd.tx_data_sz = cmd.rx_data_sz = 0; + SPIBUS_ACQUIRE_BUS(pdev, dev); err = SPIBUS_TRANSFER(pdev, dev, &cmd); + SPIBUS_RELEASE_BUS(pdev, dev); if (err == 0) err = at45d_wait_ready(dev, &status); if (err != 0 || (status & 0x40) != 0) { Index: dev/flash/mx25l.c =================================================================== --- dev/flash/mx25l.c (revision 239916) +++ dev/flash/mx25l.c (working copy) @@ -123,7 +123,9 @@ cmd.rx_cmd = rxBuf; cmd.rx_cmd_sz = 2; cmd.tx_cmd_sz = 2; + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev); err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd); + SPIBUS_RELEASE_BUS(device_get_parent(dev), dev); return (rxBuf[1]); } @@ -157,7 +159,9 @@ */ cmd.tx_cmd_sz = 4; cmd.rx_cmd_sz = 4; + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev); err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd); + SPIBUS_RELEASE_BUS(device_get_parent(dev), dev); if (err) return (NULL); @@ -192,7 +196,9 @@ cmd.rx_cmd = rxBuf; cmd.rx_cmd_sz = 1; cmd.tx_cmd_sz = 1; + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev); err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd); + SPIBUS_RELEASE_BUS(device_get_parent(dev), dev); } static void @@ -217,7 +223,9 @@ txBuf[1] = ((sector >> 16) & 0xff); txBuf[2] = ((sector >> 8) & 0xff); txBuf[3] = (sector & 0xff); + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev); err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd); + SPIBUS_RELEASE_BUS(device_get_parent(dev), dev); } static int @@ -290,7 +298,9 @@ mx25l_wait_for_device_ready(dev); mx25l_set_writable(dev, 1); + SPIBUS_ACQUIRE_BUS(pdev, dev); err = SPIBUS_TRANSFER(pdev, dev, &cmd); + SPIBUS_RELEASE_BUS(pdev, dev); if (err) break; @@ -339,7 +349,9 @@ cmd.rx_data = data; cmd.rx_data_sz = count; + SPIBUS_ACQUIRE_BUS(pdev, dev); err = SPIBUS_TRANSFER(pdev, dev, &cmd); + SPIBUS_RELEASE_BUS(pdev, dev); return (err); } @@ -480,7 +492,7 @@ DEVMETHOD(device_attach, mx25l_attach), DEVMETHOD(device_detach, mx25l_detach), - { 0, 0 } + DEVMETHOD_END }; static driver_t mx25l_driver = { Index: arm/lpc/ssd1289.c =================================================================== --- arm/lpc/ssd1289.c (revision 239916) +++ arm/lpc/ssd1289.c (working copy) @@ -179,6 +179,7 @@ { uint8_t buffer[2]; + SPIBUS_ACQUIRE_BUS(device_get_parent(sc->ss_dev), sc->ss_dev); ssd1289_set_dc(sc, 0); buffer[0] = 0x00; buffer[1] = addr & 0xff; @@ -188,6 +189,7 @@ buffer[0] = (value >> 8) & 0xff; buffer[1] = value & 0xff; ssd1289_spi_send(sc, buffer, 2); + SPIBUS_RELEASE_BUS(device_get_parent(sc->ss_dev), sc->ss_dev); } static device_method_t ssd1289_methods[] = { @@ -195,7 +197,7 @@ DEVMETHOD(device_probe, ssd1289_probe), DEVMETHOD(device_attach, ssd1289_attach), - { 0, 0 } + DEVMETHOD_END }; static devclass_t ssd1289_devclass; Index: mips/atheros/pcf2123_rtc.c =================================================================== --- mips/atheros/pcf2123_rtc.c (revision 239916) +++ mips/atheros/pcf2123_rtc.c (working copy) @@ -91,7 +91,9 @@ cmd.tx_cmd = txBuf; cmd.rx_cmd_sz = sizeof(rxBuf); cmd.tx_cmd_sz = sizeof(txBuf); + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev); err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd); + SPIBUS_RELEASE_BUS(device_get_parent(dev), dev); DELAY(PCF2123_DELAY); return (0); @@ -120,7 +122,9 @@ cmd.tx_cmd = txTimedate; cmd.rx_cmd_sz = sizeof(rxTimedate); cmd.tx_cmd_sz = sizeof(txTimedate); + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev); err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd); + SPIBUS_RELEASE_BUS(device_get_parent(dev), dev); DELAY(PCF2123_DELAY); ct.nsec = 0; @@ -178,7 +182,9 @@ txTimedate[6] = TOBCD(ct.mon); txTimedate[7] = TOBCD(ct.year - YEAR_BASE); + SPIBUS_ACQUIRE_BUS(device_get_parent(dev), dev); err = SPIBUS_TRANSFER(device_get_parent(dev), dev, &cmd); + SPIBUS_RELEASE_BUS(device_get_parent(dev), dev); DELAY(PCF2123_DELAY); return (err); @@ -191,7 +197,7 @@ DEVMETHOD(clock_gettime, pcf2123_rtc_gettime), DEVMETHOD(clock_settime, pcf2123_rtc_settime), - { 0, 0 }, + DEVMETHOD_END }; static driver_t pcf2123_rtc_driver = { --Apple-Mail-58-1062540785 Content-Disposition: attachment; filename=spibus-03-controllers.diff Content-Type: application/octet-stream; name="spibus-03-controllers.diff" Content-Transfer-Encoding: 7bit Index: mips/atheros/ar71xx_spi.c =================================================================== --- mips/atheros/ar71xx_spi.c (revision 239916) +++ mips/atheros/ar71xx_spi.c (working copy) @@ -35,7 +35,9 @@ #include #include #include +#include #include +#include #include #include @@ -76,10 +78,21 @@ struct ar71xx_spi_softc { device_t sc_dev; + device_t sc_owner; + struct mtx sc_mtx; struct resource *sc_mem_res; uint32_t sc_reg_ctrl; }; +#define AR71XX_SPI_LOCK(_sc) mtx_lock(&(_sc)->sc_mtx) +#define AR71XX_SPI_UNLOCK(_sc) mtx_unlock(&(_sc)->sc_mtx) +#define AR71XX_SPI_LOCK_INIT(_sc) \ + mtx_init(&_sc->sc_mtx, device_get_nameunit(_sc->sc_dev), \ + "ar71xx_spi", MTX_DEF) +#define AR71XX_SPI_LOCK_DESTROY(_sc) mtx_destroy(&_sc->sc_mtx) +#define AR71XX_SPI_ASSERT_LOCKED(_sc) mtx_assert(&_sc->sc_mtx, MA_OWNED) +#define AR71XX_SPI_ASSERT_UNLOCKED(_sc) mtx_assert(&_sc->sc_mtx, MA_NOTOWNED) + static int ar71xx_spi_probe(device_t dev) { @@ -102,6 +115,7 @@ return (ENXIO); } + AR71XX_SPI_LOCK_INIT(sc); SPI_WRITE(sc, AR71XX_SPI_FS, 1); sc->sc_reg_ctrl = SPI_READ(sc, AR71XX_SPI_CTRL); @@ -133,6 +147,49 @@ SPI_WRITE(sc, AR71XX_SPI_IO_CTRL, SPI_IO_CTRL_CSMASK); } +static void +ar71xx_spi_acquire_bus(device_t dev, device_t child) +{ + struct spibus_ivar *devi = SPIBUS_IVAR(child); + struct ar71xx_spi_softc *sc; + + sc = device_get_softc(dev); + AR71XX_SPI_ASSERT_UNLOCKED(sc); + + /* Wait until bus is free and then set the new bus owner. */ + AR71XX_SPI_LOCK(sc); + while (sc->sc_owner != NULL) { + mtx_sleep(sc, &sc->sc_mtx, 0, "ar71xx_spi", 0); + } + sc->sc_owner = child; + AR71XX_SPI_UNLOCK(sc); + + /* Select the SPI device. */ + ar71xx_spi_chip_activate(sc, devi->cs); +} + +static void +ar71xx_spi_release_bus(device_t dev, device_t child) +{ + struct spibus_ivar *devi = SPIBUS_IVAR(child); + struct ar71xx_spi_softc *sc; + + sc = device_get_softc(dev); + AR71XX_SPI_ASSERT_UNLOCKED(sc); + + /* Free the SPI BUS. */ + AR71XX_SPI_LOCK(sc); + if (sc->sc_owner == NULL) + panic("ar71xx_spi: releasing unowned bus."); + if (sc->sc_owner != child) + panic("ar71xx_spi: you don't own the bus. game over."); + sc->sc_owner = NULL; + AR71XX_SPI_UNLOCK(sc); + + /* Deselect the SPI device. */ + ar71xx_spi_chip_deactivate(sc, devi->cs); +} + static uint8_t ar71xx_spi_txrx(struct ar71xx_spi_softc *sc, int cs, uint8_t data) { @@ -173,7 +230,7 @@ sc = device_get_softc(dev); - ar71xx_spi_chip_activate(sc, devi->cs); + KASSERT(sc->sc_owner != NULL, ("SPI transfer on unowned bus")); KASSERT(cmd->tx_cmd_sz == cmd->rx_cmd_sz, ("TX/RX command sizes should be equal")); @@ -196,8 +253,6 @@ for (i = 0; i < cmd->tx_data_sz; i++) buf_in[i] = ar71xx_spi_txrx(sc, devi->cs, buf_out[i]); - ar71xx_spi_chip_deactivate(sc, devi->cs); - return (0); } @@ -209,6 +264,8 @@ SPI_WRITE(sc, AR71XX_SPI_CTRL, sc->sc_reg_ctrl); SPI_WRITE(sc, AR71XX_SPI_FS, 0); + AR71XX_SPI_LOCK_DESTROY(sc); + if (sc->sc_mem_res) bus_release_resource(dev, SYS_RES_MEMORY, 0, sc->sc_mem_res); @@ -221,9 +278,11 @@ DEVMETHOD(device_attach, ar71xx_spi_attach), DEVMETHOD(device_detach, ar71xx_spi_detach), + DEVMETHOD(spibus_acquire_bus, ar71xx_spi_acquire_bus), + DEVMETHOD(spibus_release_bus, ar71xx_spi_release_bus), DEVMETHOD(spibus_transfer, ar71xx_spi_transfer), - {0, 0} + DEVMETHOD_END }; static driver_t ar71xx_spi_driver = { Index: arm/lpc/lpc_spi.c =================================================================== --- arm/lpc/lpc_spi.c (revision 239916) +++ arm/lpc/lpc_spi.c (working copy) @@ -67,6 +67,8 @@ struct lpc_spi_softc { device_t ls_dev; + device_t ls_owner; + struct mtx ls_mtx; struct resource * ls_mem_res; struct resource * ls_irq_res; bus_space_tag_t ls_bst; @@ -83,6 +85,15 @@ #define lpc_spi_write_4(_sc, _reg, _val) \ bus_space_write_4(_sc->ls_bst, _sc->ls_bsh, _reg, _val) +#define LPC_SPI_LOCK(_sc) mtx_lock(&(_sc)->ls_mtx) +#define LPC_SPI_UNLOCK(_sc) mtx_unlock(&(_sc)->ls_mtx) +#define LPC_SPI_LOCK_INIT(_sc) \ + mtx_init(&_sc->sc_mtx, device_get_nameunit(_sc->ls_dev), \ + "lpc_spi", MTX_DEF) +#define LPC_SPI_LOCK_DESTROY(_sc) mtx_destroy(&_sc->ls_mtx) +#define LPC_SPI_ASSERT_LOCKED(_sc) mtx_assert(&_sc->ls_mtx, MA_OWNED) +#define LPC_SPI_ASSERT_UNLOCKED(_sc) mtx_assert(&_sc->ls_mtx, MA_NOTOWNED) + static int lpc_spi_probe(device_t dev) { @@ -126,6 +137,8 @@ lpc_spi_write_4(sc, LPC_SSP_CR1, LPC_SSP_CR1_SSE); lpc_spi_write_4(sc, LPC_SSP_CPSR, 128); + LPC_SPI_LOCK_INIT(sc); + device_add_child(dev, "spibus", 0); return (bus_generic_attach(dev)); } @@ -136,6 +149,49 @@ return (EBUSY); } +static void +lpc_spi_acquire_bus(device_t dev, device_t child) +{ + struct spibus_ivar *devi = SPIBUS_IVAR(child); + struct lpc_spi_softc *sc; + + sc = device_get_softc(dev); + LPC_SPI_ASSERT_UNLOCKED(sc); + + /* Wait until bus is free and then set the new bus owner. */ + LPC_SPI_LOCK(sc); + while (sc->ls_owner != NULL) { + mtx_sleep(sc, &sc->ls_mtx, 0, "lpc_spi", 0); + } + sc->ls_owner = child; + LPC_SPI_UNLOCK(sc); + + /* Set CS active */ + lpc_gpio_set_state(child, devi->cs, 0); +} + +static void +lpc_spi_release_bus(device_t dev, device_t child) +{ + struct spibus_ivar *devi = SPIBUS_IVAR(child); + struct ar71xx_spi_softc *sc; + + sc = device_get_softc(dev); + LPC_SPI_ASSERT_UNLOCKED(sc); + + /* Free the SPI BUS. */ + LPC_SPI_LOCK(sc); + if (sc->ls_owner == NULL) + panic("lpc_spi: releasing unowned bus."); + if (sc->ls_owner != child) + panic("lpc_spi: you don't own the bus. game over."); + sc->ls_owner = NULL; + LPC_SPI_UNLOCK(sc); + + /* Set CS inactive */ + lpc_gpio_set_state(child, devi->cs, 1); +} + static int lpc_spi_transfer(device_t dev, device_t child, struct spi_command *cmd) { @@ -144,8 +200,7 @@ uint8_t *in_buf, *out_buf; int i; - /* Set CS active */ - lpc_gpio_set_state(child, devi->cs, 0); + KASSERT(sc->ls_owner != NULL, ("SPI transfer on unowned bus")); /* Wait for FIFO to be ready */ while ((lpc_spi_read_4(sc, LPC_SSP_SR) & LPC_SSP_SR_TNF) == 0); @@ -166,9 +221,6 @@ in_buf[i] = lpc_spi_read_4(sc, LPC_SSP_DR); } - /* Set CS inactive */ - lpc_gpio_set_state(child, devi->cs, 1); - return (0); } @@ -179,9 +231,11 @@ DEVMETHOD(device_detach, lpc_spi_detach), /* SPI interface */ + DEVMETHOD(spibus_acquire_bus, lpc_spi_acquire_bus), + DEVMETHOD(spibus_release_bus, lpc_spi_release_bus), DEVMETHOD(spibus_transfer, lpc_spi_transfer), - { 0, 0 } + DEVMETHOD_END }; static devclass_t lpc_spi_devclass; --Apple-Mail-58-1062540785--