Date: Sat, 6 Jan 2018 21:19:52 +0000 (UTC) From: Ian Lepore <ian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r327638 - in stable/11/sys/dev: ffec sdhci Message-ID: <201801062119.w06LJqLi050559@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ian Date: Sat Jan 6 21:19:52 2018 New Revision: 327638 URL: https://svnweb.freebsd.org/changeset/base/327638 Log: MFC r325045, r325054-r325056, r325061, r325063, r325065 r325045: Actually release resources in detach() rather than just returning EBUSY. This will enable use of 'devctl disable', allow creation of a module, etc. r325054: Increase the alignment of the rx/tx descriptor ring buffers to 64 bytes. 16 was the correct alignment for older hardware, but the imx7 requires 64-byte alignment, which is a fine value to use on all systems. PR: 222634 Submitted by: sebastian.huber@embedded-brains.de r325055: Add FECFLAG_AVB variant flag to support new features on imx7. This flag is analogous to the Linux driver FEC_QUIRK_HAS_AVB. It indicates an FEC with support for Audio Video Bridging (AVB). This indicator is used for various other parts in the Linux driver (drivers/net/ethernet/freescale/fec_main.c). Use it to customize the receive/transmit buffer alignment. The receive buffer alignment increased to 64-bytes on the i.MX 6SoloX and i.MX 7Dual. There are no hard alignment restrictions for transmit buffers on these chips. Fix the ffec_softc::fectype type to provide enough storage for the feature flags. PR: 222634 Submitted by: sebastian.huber@embedded-brains.de r325056: Avoid AXI bus issues due to a MAC reset on imx6sx and imx7. When the FEC is connected to the AXI bus (indicated by AVB flag), a MAC reset while a bus transaction is pending can hang the bus. Instead of resetting, turn off the ENABLE bit, which allows the hardware to complete any in-progress transfers (appending a bad CRC to any partial packet) and release the AXI bus. This could probably be done unconditionally for all hardware variants, but that hasn't been tested. PR: 222634 Submitted by: sebastian.huber@embedded-brains.de r325061: Support up to 3 IRQs in the ffec driver. Newer hardware splits the interrupts onto 3 different irq lines, but the docs barely mention that there are multiple interrupts, and do not detail how they're split up. The code now supports 1-3 irqs, and uses the same interrupt service routine to handle all of them. I modified the submitted changes to use bus_alloc_resources() instead of using loops to allocate each irq separately. Thus, blame any bugs on me (I can't actually test on imx7 hardware). PR: 222634 Submitted by: sebastian.huber@embedded-brains.de r325063: Use the 16-bit receive shift feature in ffec hardware that supports it. When available, enabling this feature causes the hardware to write data to the receive buffer starting at a 16-bit offset from the start address. This eliminates the need to copy the data after receiving to re-align the protocol headers to a 32-bit boundary. PR: 222634 Submitted by: sebastian.huber@embedded-brains.de r325065: Split the hardware type enum and the hw feature flags bits into separate fields in the softc; they're ORed together in the ofw_compat_data. I already caught myself doing 'sc->fectype == <enum val>' without masking out the feature bits in one place, and that's sure to happen again. Glomming them together is convenient for storing them in the ofw_compat_data array, but there's no reason to keep them together in the softc. Modified: stable/11/sys/dev/ffec/if_ffec.c stable/11/sys/dev/ffec/if_ffecreg.h stable/11/sys/dev/sdhci/fsl_sdhci.c Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/dev/ffec/if_ffec.c ============================================================================== --- stable/11/sys/dev/ffec/if_ffec.c Sat Jan 6 20:52:30 2018 (r327637) +++ stable/11/sys/dev/ffec/if_ffec.c Sat Jan 6 21:19:52 2018 (r327638) @@ -97,16 +97,21 @@ enum { FECTYPE_NONE, FECTYPE_GENERIC, FECTYPE_IMX53, - FECTYPE_IMX6, + FECTYPE_IMX6, /* imx6 and imx7 */ FECTYPE_MVF, }; /* * Flags that describe general differences between the FEC hardware in various - * SoCs. These are ORed into the FECTYPE enum values. + * SoCs. These are ORed into the FECTYPE enum values in the ofw_compat_data, so + * the low 8 bits are reserved for the type enum. In the softc, the type and + * flags are put into separate members, so that you don't need to mask the flags + * out of the type to compare it. */ -#define FECTYPE_MASK 0x0000ffff -#define FECFLAG_GBE (0x0001 << 16) +#define FECTYPE_MASK 0x000000ff +#define FECFLAG_GBE (1 << 8) +#define FECFLAG_AVB (1 << 9) +#define FECFLAG_RACC (1 << 10) /* * Table of supported FDT compat strings and their associated FECTYPE values. @@ -114,9 +119,11 @@ enum { static struct ofw_compat_data compat_data[] = { {"fsl,imx51-fec", FECTYPE_GENERIC}, {"fsl,imx53-fec", FECTYPE_IMX53}, - {"fsl,imx6q-fec", FECTYPE_IMX6 | FECFLAG_GBE}, - {"fsl,imx6ul-fec", FECTYPE_IMX6}, - {"fsl,mvf600-fec", FECTYPE_MVF}, + {"fsl,imx6q-fec", FECTYPE_IMX6 | FECFLAG_RACC | FECFLAG_GBE }, + {"fsl,imx6ul-fec", FECTYPE_IMX6 | FECFLAG_RACC }, + {"fsl,imx7d-fec", FECTYPE_IMX6 | FECFLAG_RACC | FECFLAG_GBE | + FECFLAG_AVB }, + {"fsl,mvf600-fec", FECTYPE_MVF | FECFLAG_RACC }, {"fsl,mvf-fec", FECTYPE_MVF}, {NULL, FECTYPE_NONE}, }; @@ -131,6 +138,8 @@ static struct ofw_compat_data compat_data[] = { #define WATCHDOG_TIMEOUT_SECS 5 +#define MAX_IRQ_COUNT 3 + struct ffec_bufmap { struct mbuf *mbuf; bus_dmamap_t map; @@ -143,16 +152,19 @@ struct ffec_softc { struct ifnet *ifp; int if_flags; struct mtx mtx; - struct resource *irq_res; + struct resource *irq_res[MAX_IRQ_COUNT]; struct resource *mem_res; - void * intr_cookie; + void * intr_cookie[MAX_IRQ_COUNT]; struct callout ffec_callout; mii_contype_t phy_conn_type; + uint32_t fecflags; uint8_t fectype; boolean_t link_is_up; boolean_t is_attached; boolean_t is_detaching; int tx_watchdog_count; + int rxbuf_align; + int txbuf_align; bus_dma_tag_t rxdesc_tag; bus_dmamap_t rxdesc_map; @@ -173,6 +185,13 @@ struct ffec_softc { int txcount; }; +static struct resource_spec irq_res_spec[MAX_IRQ_COUNT + 1] = { + { SYS_RES_IRQ, 0, RF_ACTIVE }, + { SYS_RES_IRQ, 1, RF_ACTIVE | RF_OPTIONAL }, + { SYS_RES_IRQ, 2, RF_ACTIVE | RF_OPTIONAL }, + RESOURCE_SPEC_END +}; + #define FFEC_LOCK(sc) mtx_lock(&(sc)->mtx) #define FFEC_UNLOCK(sc) mtx_unlock(&(sc)->mtx) #define FFEC_LOCK_INIT(sc) mtx_init(&(sc)->mtx, \ @@ -246,7 +265,7 @@ ffec_miigasket_setup(struct ffec_softc *sc) * We only need the gasket for MII and RMII connections on certain SoCs. */ - switch (sc->fectype & FECTYPE_MASK) + switch (sc->fectype) { case FECTYPE_IMX53: break; @@ -747,14 +766,17 @@ ffec_setup_rxbuf(struct ffec_softc *sc, int idx, struc int error, nsegs; struct bus_dma_segment seg; - /* - * We need to leave at least ETHER_ALIGN bytes free at the beginning of - * the buffer to allow the data to be re-aligned after receiving it (by - * copying it backwards ETHER_ALIGN bytes in the same buffer). We also - * have to ensure that the beginning of the buffer is aligned to the - * hardware's requirements. - */ - m_adj(m, roundup(ETHER_ALIGN, FEC_RXBUF_ALIGN)); + if (!(sc->fecflags & FECFLAG_RACC)) { + /* + * The RACC[SHIFT16] feature is not available. So, we need to + * leave at least ETHER_ALIGN bytes free at the beginning of the + * buffer to allow the data to be re-aligned after receiving it + * (by copying it backwards ETHER_ALIGN bytes in the same + * buffer). We also have to ensure that the beginning of the + * buffer is aligned to the hardware's requirements. + */ + m_adj(m, roundup(ETHER_ALIGN, sc->rxbuf_align)); + } error = bus_dmamap_load_mbuf_sg(sc->rxbuf_tag, sc->rxbuf_map[idx].map, m, &seg, &nsegs, 0); @@ -802,23 +824,6 @@ ffec_rxfinish_onebuf(struct ffec_softc *sc, int len) return; } - /* - * Unfortunately, the protocol headers need to be aligned on a 32-bit - * boundary for the upper layers. The hardware requires receive - * buffers to be 16-byte aligned. The ethernet header is 14 bytes, - * leaving the protocol header unaligned. We used m_adj() after - * allocating the buffer to leave empty space at the start of the - * buffer, now we'll use the alignment agnostic bcopy() routine to - * shuffle all the data backwards 2 bytes and adjust m_data. - * - * XXX imx6 hardware is able to do this 2-byte alignment by setting the - * SHIFT16 bit in the RACC register. Older hardware doesn't have that - * feature, but for them could we speed this up by copying just the - * protocol headers into their own small mbuf then chaining the cluster - * to it? That way we'd only need to copy like 64 bytes or whatever - * the biggest header is, instead of the whole 1530ish-byte frame. - */ - FFEC_UNLOCK(sc); bmap = &sc->rxbuf_map[sc->rx_idx]; @@ -831,10 +836,24 @@ ffec_rxfinish_onebuf(struct ffec_softc *sc, int len) m->m_pkthdr.len = len; m->m_pkthdr.rcvif = sc->ifp; - src = mtod(m, uint8_t*); - dst = src - ETHER_ALIGN; - bcopy(src, dst, len); - m->m_data = dst; + /* + * Align the protocol headers in the receive buffer on a 32-bit + * boundary. Newer hardware does the alignment for us. On hardware + * that doesn't support this feature, we have to copy-align the data. + * + * XXX for older hardware, could we speed this up by copying just the + * protocol headers into their own small mbuf then chaining the cluster + * to it? That way we'd only need to copy like 64 bytes or whatever the + * biggest header is, instead of the whole 1530ish-byte frame. + */ + if (sc->fecflags & FECFLAG_RACC) { + m->m_data = mtod(m, uint8_t *) + 2; + } else { + src = mtod(m, uint8_t*); + dst = src - ETHER_ALIGN; + bcopy(src, dst, len); + m->m_data = dst; + } sc->ifp->if_input(sc->ifp, m); FFEC_LOCK(sc); @@ -1098,7 +1117,7 @@ ffec_init_locked(struct ffec_softc *sc) * when we support jumbo frames and receiving fragments of them into * separate buffers. */ - maxbuf = MCLBYTES - roundup(ETHER_ALIGN, FEC_RXBUF_ALIGN); + maxbuf = MCLBYTES - roundup(ETHER_ALIGN, sc->rxbuf_align); maxfl = min(maxbuf, 0x7ff); if (ifp->if_drv_flags & IFF_DRV_RUNNING) @@ -1208,6 +1227,14 @@ ffec_init_locked(struct ffec_softc *sc) ffec_clear_stats(sc); WR4(sc, FEC_MIBC_REG, regval & ~FEC_MIBC_DIS); + if (sc->fecflags & FECFLAG_RACC) { + /* + * RACC - Receive Accelerator Function Configuration. + */ + regval = RD4(sc, FEC_RACC_REG); + WR4(sc, FEC_RACC_REG, regval | FEC_RACC_SHIFT16); + } + /* * ECR - Ethernet control register. * @@ -1360,7 +1387,7 @@ ffec_detach(device_t dev) { struct ffec_softc *sc; bus_dmamap_t map; - int idx; + int idx, irq; /* * NB: This function can be called internally to unwind a failure to @@ -1411,15 +1438,17 @@ ffec_detach(device_t dev) bus_dmamap_destroy(sc->txdesc_tag, sc->txdesc_map); } if (sc->txdesc_tag != NULL) - bus_dma_tag_destroy(sc->txdesc_tag); + bus_dma_tag_destroy(sc->txdesc_tag); /* Release bus resources. */ - if (sc->intr_cookie) - bus_teardown_intr(dev, sc->irq_res, sc->intr_cookie); + for (irq = 0; irq < MAX_IRQ_COUNT; ++irq) { + if (sc->intr_cookie[irq] != NULL) { + bus_teardown_intr(dev, sc->irq_res[irq], + sc->intr_cookie[irq]); + } + } + bus_release_resources(dev, irq_res_spec, sc->irq_res); - if (sc->irq_res != NULL) - bus_release_resource(dev, SYS_RES_IRQ, 0, sc->irq_res); - if (sc->mem_res != NULL) bus_release_resource(dev, SYS_RES_MEMORY, 0, sc->mem_res); @@ -1434,10 +1463,11 @@ ffec_attach(device_t dev) struct ifnet *ifp = NULL; struct mbuf *m; void *dummy; + uintptr_t typeflags; phandle_t ofw_node; - int error, phynum, rid; - uint8_t eaddr[ETHER_ADDR_LEN]; uint32_t idx, mscr; + int error, phynum, rid, irq; + uint8_t eaddr[ETHER_ADDR_LEN]; sc = device_get_softc(dev); sc->dev = dev; @@ -1448,8 +1478,18 @@ ffec_attach(device_t dev) * There are differences in the implementation and features of the FEC * hardware on different SoCs, so figure out what type we are. */ - sc->fectype = ofw_bus_search_compatible(dev, compat_data)->ocd_data; + typeflags = ofw_bus_search_compatible(dev, compat_data)->ocd_data; + sc->fectype = (uint8_t)(typeflags & FECTYPE_MASK); + sc->fecflags = (uint32_t)(typeflags & ~FECTYPE_MASK); + if (sc->fecflags & FECFLAG_AVB) { + sc->rxbuf_align = 64; + sc->txbuf_align = 1; + } else { + sc->rxbuf_align = 16; + sc->txbuf_align = 16; + } + /* * We have to be told what kind of electrical connection exists between * the MAC and PHY or we can't operate correctly. @@ -1478,12 +1518,10 @@ ffec_attach(device_t dev) error = ENOMEM; goto out; } - rid = 0; - sc->irq_res = bus_alloc_resource_any(dev, SYS_RES_IRQ, &rid, - RF_ACTIVE); - if (sc->irq_res == NULL) { - device_printf(dev, "could not allocate interrupt resources.\n"); - error = ENOMEM; + + error = bus_alloc_resources(dev, irq_res_spec, sc->irq_res); + if (error != 0) { + device_printf(dev, "could not allocate interrupt resources\n"); goto out; } @@ -1525,7 +1563,7 @@ ffec_attach(device_t dev) error = bus_dma_tag_create( bus_get_dma_tag(dev), /* Parent tag. */ - FEC_TXBUF_ALIGN, 0, /* alignment, boundary */ + sc->txbuf_align, 0, /* alignment, boundary */ BUS_SPACE_MAXADDR_32BIT, /* lowaddr */ BUS_SPACE_MAXADDR, /* highaddr */ NULL, NULL, /* filter, filterarg */ @@ -1627,15 +1665,34 @@ ffec_attach(device_t dev) /* Try to get the MAC address from the hardware before resetting it. */ ffec_get_hwaddr(sc, eaddr); - /* Reset the hardware. Disables all interrupts. */ - WR4(sc, FEC_ECR_REG, FEC_ECR_RESET); + /* + * Reset the hardware. Disables all interrupts. + * + * When the FEC is connected to the AXI bus (indicated by AVB flag), a + * MAC reset while a bus transaction is pending can hang the bus. + * Instead of resetting, turn off the ENABLE bit, which allows the + * hardware to complete any in-progress transfers (appending a bad CRC + * to any partial packet) and release the AXI bus. This could probably + * be done unconditionally for all hardware variants, but that hasn't + * been tested. + */ + if (sc->fecflags & FECFLAG_AVB) + WR4(sc, FEC_ECR_REG, 0); + else + WR4(sc, FEC_ECR_REG, FEC_ECR_RESET); /* Setup interrupt handler. */ - error = bus_setup_intr(dev, sc->irq_res, INTR_TYPE_NET | INTR_MPSAFE, - NULL, ffec_intr, sc, &sc->intr_cookie); - if (error != 0) { - device_printf(dev, "could not setup interrupt handler.\n"); - goto out; + for (irq = 0; irq < MAX_IRQ_COUNT; ++irq) { + if (sc->irq_res[irq] != NULL) { + error = bus_setup_intr(dev, sc->irq_res[irq], + INTR_TYPE_NET | INTR_MPSAFE, NULL, ffec_intr, sc, + &sc->intr_cookie[irq]); + if (error != 0) { + device_printf(dev, + "could not setup interrupt handler.\n"); + goto out; + } + } } /* @@ -1701,7 +1758,7 @@ ffec_attach(device_t dev) } error = mii_attach(dev, &sc->miibus, ifp, ffec_media_change, ffec_media_status, BMSR_DEFCAPMASK, phynum, MII_OFFSET_ANY, - (sc->fectype & FECTYPE_MVF) ? MIIF_FORCEANEG : 0); + (sc->fecflags & FECTYPE_MVF) ? MIIF_FORCEANEG : 0); if (error != 0) { device_printf(dev, "PHY attach failed\n"); goto out; Modified: stable/11/sys/dev/ffec/if_ffecreg.h ============================================================================== --- stable/11/sys/dev/ffec/if_ffecreg.h Sat Jan 6 20:52:30 2018 (r327637) +++ stable/11/sys/dev/ffec/if_ffecreg.h Sat Jan 6 21:19:52 2018 (r327638) @@ -317,8 +317,6 @@ struct ffec_hwdesc * The hardware imposes alignment restrictions on various objects involved in * DMA transfers. These values are expressed in bytes (not bits). */ -#define FEC_DESC_RING_ALIGN 16 -#define FEC_RXBUF_ALIGN 16 -#define FEC_TXBUF_ALIGN 16 +#define FEC_DESC_RING_ALIGN 64 #endif /* IF_FFECREG_H */ Modified: stable/11/sys/dev/sdhci/fsl_sdhci.c ============================================================================== --- stable/11/sys/dev/sdhci/fsl_sdhci.c Sat Jan 6 20:52:30 2018 (r327637) +++ stable/11/sys/dev/sdhci/fsl_sdhci.c Sat Jan 6 21:19:52 2018 (r327638) @@ -807,9 +807,26 @@ fsl_sdhci_get_platform_clock(device_t dev) static int fsl_sdhci_detach(device_t dev) { + struct fsl_sdhci_softc *sc = device_get_softc(dev); - /* sdhci_fdt_gpio_teardown(sc->gpio); */ - return (EBUSY); + if (sc->gpio != NULL) + sdhci_fdt_gpio_teardown(sc->gpio); + + callout_drain(&sc->r1bfix_callout); + + if (sc->intr_cookie != NULL) + bus_teardown_intr(dev, sc->irq_res, sc->intr_cookie); + if (sc->irq_res != NULL) + bus_release_resource(dev, SYS_RES_IRQ, + rman_get_rid(sc->irq_res), sc->irq_res); + + if (sc->mem_res != NULL) { + sdhci_cleanup_slot(&sc->slot); + bus_release_resource(dev, SYS_RES_MEMORY, + rman_get_rid(sc->mem_res), sc->mem_res); + } + + return (0); } static int @@ -922,13 +939,7 @@ fsl_sdhci_attach(device_t dev) return (0); fail: - if (sc->intr_cookie) - bus_teardown_intr(dev, sc->irq_res, sc->intr_cookie); - if (sc->irq_res) - bus_release_resource(dev, SYS_RES_IRQ, 0, sc->irq_res); - if (sc->mem_res) - bus_release_resource(dev, SYS_RES_MEMORY, 0, sc->mem_res); - + fsl_sdhci_detach(dev); return (err); } @@ -936,7 +947,7 @@ static int fsl_sdhci_probe(device_t dev) { - if (!ofw_bus_status_okay(dev)) + if (!ofw_bus_status_okay(dev)) return (ENXIO); switch (ofw_bus_search_compatible(dev, compat_data)->ocd_data) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201801062119.w06LJqLi050559>