[U-Boot] [PATCH 1/2] dt: net: add DWC EQoS binding

From: Stephen Warren swarren@nvidia.com
The Synopsys DWC EQoS is a configurable Ethernet MAC/DMA IP block which supports multiple options for bus type, clocking and reset structure, and feature list.
This patch imports the binding from the Linux kernel, including my V3 patch to extend the binding to cover the Tegra186, which is applied for next-20160912. So far, my changes have been acked by Lars Persson, the original author of the binding.
Signed-off-by: Stephen Warren swarren@nvidia.com --- .../net/snps,dwc-qos-ethernet.txt | 166 +++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 doc/device-tree-bindings/net/snps,dwc-qos-ethernet.txt
diff --git a/doc/device-tree-bindings/net/snps,dwc-qos-ethernet.txt b/doc/device-tree-bindings/net/snps,dwc-qos-ethernet.txt new file mode 100644 index 000000000000..d93f71ce8346 --- /dev/null +++ b/doc/device-tree-bindings/net/snps,dwc-qos-ethernet.txt @@ -0,0 +1,166 @@ +* Synopsys DWC Ethernet QoS IP version 4.10 driver (GMAC) + +This binding supports the Synopsys Designware Ethernet QoS (Quality Of Service) +IP block. The IP supports multiple options for bus type, clocking and reset +structure, and feature list. Consequently, a number of properties and list +entries in properties are marked as optional, or only required in specific HW +configurations. + +Required properties: +- compatible: One of: + - "axis,artpec6-eqos", "snps,dwc-qos-ethernet-4.10" + Represents the IP core when integrated into the Axis ARTPEC-6 SoC. + - "nvidia,tegra186-eqos", "snps,dwc-qos-ethernet-4.10" + Represents the IP core when integrated into the NVIDIA Tegra186 SoC. + - "snps,dwc-qos-ethernet-4.10" + This combination is deprecated. It should be treated as equivalent to + "axis,artpec6-eqos", "snps,dwc-qos-ethernet-4.10". It is supported to be + compatible with earlier revisions of this binding. +- reg: Address and length of the register set for the device +- clocks: Phandle and clock specifiers for each entry in clock-names, in the + same order. See ../clock/clock-bindings.txt. +- clock-names: May contain any/all of the following depending on the IP + configuration, in any order: + - "tx" + The EQOS transmit path clock. The HW signal name is clk_tx_i. + In some configurations (e.g. GMII/RGMII), this clock also drives the PHY TX + path. In other configurations, other clocks (such as tx_125, rmii) may + drive the PHY TX path. + - "rx" + The EQOS receive path clock. The HW signal name is clk_rx_i. + In some configurations (e.g. GMII/RGMII), this clock is derived from the + PHY's RX clock output. In other configurations, other clocks (such as + rx_125, rmii) may drive the EQOS RX path. + In cases where the PHY clock is directly fed into the EQOS receive path + without intervening logic, the DT need not represent this clock, since it + is assumed to be fully under the control of the PHY device/driver. In + cases where SoC integration adds additional logic to this path, such as a + SW-controlled clock gate, this clock should be represented in DT. + - "slave_bus" + The CPU/slave-bus (CSR) interface clock. This applies to any bus type; + APB, AHB, AXI, etc. The HW signal name is hclk_i (AHB) or clk_csr_i (other + buses). + - "master_bus" + The master bus interface clock. Only required in configurations that use a + separate clock for the master and slave bus interfaces. The HW signal name + is hclk_i (AHB) or aclk_i (AXI). + - "ptp_ref" + The PTP reference clock. The HW signal name is clk_ptp_ref_i. + - "phy_ref_clk" + This clock is deprecated and should not be used by new compatible values. + It is equivalent to "tx". + - "apb_pclk" + This clock is deprecated and should not be used by new compatible values. + It is equivalent to "slave_bus". + + Note: Support for additional IP configurations may require adding the + following clocks to this list in the future: clk_rx_125_i, clk_tx_125_i, + clk_pmarx_0_i, clk_pmarx1_i, clk_rmii_i, clk_revmii_rx_i, clk_revmii_tx_i. + Configurations exist where multiple similar clocks are used at once, e.g. all + of clk_rx_125_i, clk_pmarx_0_i, clk_pmarx1_i. For this reason it is best to + extend the binding with a separate clock-names entry for each of those RX + clocks, rather than repurposing the existing "rx" clock-names entry as a + generic/logical clock in a similar fashion to "master_bus" and "slave_bus". + This will allow easy support for configurations that support multiple PHY + interfaces using a mux, and hence need to have explicit control over + specific RX clocks. + + The following compatible values require the following set of clocks: + - "nvidia,tegra186-eqos", "snps,dwc-qos-ethernet-4.10": + - "slave_bus" + - "master_bus" + - "rx" + - "tx" + - "ptp_ref" + - "axis,artpec6-eqos", "snps,dwc-qos-ethernet-4.10": + - "slave_bus" + - "master_bus" + - "tx" + - "ptp_ref" + - "snps,dwc-qos-ethernet-4.10" (deprecated): + - "phy_ref_clk" + - "apb_clk" +- interrupt-parent: Should be the phandle for the interrupt controller + that services interrupts for this device +- interrupts: Should contain the core's combined interrupt signal +- phy-mode: See ethernet.txt file in the same directory +- resets: Phandle and reset specifiers for each entry in reset-names, in the + same order. See ../reset/reset.txt. +- reset-names: May contain any/all of the following depending on the IP + configuration, in any order: + - "eqos". The reset to the entire module. The HW signal name is hreset_n + (AHB) or aresetn_i (AXI). + + The following compatible values require the following set of resets: + (the reset properties may be omitted if empty) + - "nvidia,tegra186-eqos", "snps,dwc-qos-ethernet-4.10": + - "eqos". + - "axis,artpec6-eqos", "snps,dwc-qos-ethernet-4.10": + - None. + - "snps,dwc-qos-ethernet-4.10" (deprecated): + - None. + +Optional properties: +- dma-coherent: Present if dma operations are coherent +- mac-address: See ethernet.txt in the same directory +- local-mac-address: See ethernet.txt in the same directory +- phy-reset-gpios: Phandle and specifier for any GPIO used to reset the PHY. + See ../gpio/gpio.txt. +- snps,en-lpi: If present it enables use of the AXI low-power interface +- snps,write-requests: Number of write requests that the AXI port can issue. + It depends on the SoC configuration. +- snps,read-requests: Number of read requests that the AXI port can issue. + It depends on the SoC configuration. +- snps,burst-map: Bitmap of allowed AXI burst lengts, with the LSB + representing 4, then 8 etc. +- snps,txpbl: DMA Programmable burst length for the TX DMA +- snps,rxpbl: DMA Programmable burst length for the RX DMA +- snps,en-tx-lpi-clockgating: Enable gating of the MAC TX clock during + TX low-power mode. +- phy-handle: See ethernet.txt file in the same directory +- mdio device tree subnode: When the GMAC has a phy connected to its local + mdio, there must be device tree subnode with the following + required properties: + - compatible: Must be "snps,dwc-qos-ethernet-mdio". + - #address-cells: Must be <1>. + - #size-cells: Must be <0>. + + For each phy on the mdio bus, there must be a node with the following + fields: + + - reg: phy id used to communicate to phy. + - device_type: Must be "ethernet-phy". + - fixed-mode device tree subnode: see fixed-link.txt in the same directory + +Examples: +ethernet2@40010000 { + clock-names = "phy_ref_clk", "apb_pclk"; + clocks = <&clkc 17>, <&clkc 15>; + compatible = "snps,dwc-qos-ethernet-4.10"; + interrupt-parent = <&intc>; + interrupts = <0x0 0x1e 0x4>; + reg = <0x40010000 0x4000>; + phy-handle = <&phy2>; + phy-mode = "gmii"; + phy-reset-gpios = <&gpioctlr 43 GPIO_ACTIVE_LOW>; + + snps,en-tx-lpi-clockgating; + snps,en-lpi; + snps,write-requests = <2>; + snps,read-requests = <16>; + snps,burst-map = <0x7>; + snps,txpbl = <8>; + snps,rxpbl = <2>; + + dma-coherent; + + mdio { + #address-cells = <0x1>; + #size-cells = <0x0>; + phy2: phy@1 { + compatible = "ethernet-phy-ieee802.3-c22"; + device_type = "ethernet-phy"; + reg = <0x1>; + }; + }; +};

From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/net/Kconfig | 11 + drivers/net/Makefile | 1 + drivers/net/dwc_eth_qos.c | 1492 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1504 insertions(+) create mode 100644 drivers/net/dwc_eth_qos.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index be3ed73e5221..8b35415a04ce 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -64,6 +64,17 @@ config ALTERA_TSE Please find details on the "Triple-Speed Ethernet MegaCore Function Resource Center" of Altera.
+config DWC_ETH_QOS + bool "Synopsys DWC Ethernet QOS device support" + depends on DM_ETH + select PHYLIB + help + This driver supports the Synopsys Designware Ethernet QOS (Quality + Of Service) IP block. The IP supports many options for bus type, + clocking/reset structure, and feature list. This driver currently + supports the specific configuration used in NVIDIA's Tegra186 chip, + but should be extensible to other combinations quite easily. + config E1000 bool "Intel PRO/1000 Gigabit Ethernet support" help diff --git a/drivers/net/Makefile b/drivers/net/Makefile index a4485266d457..9a7bfc6d5b05 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -76,3 +76,4 @@ obj-$(CONFIG_FSL_MC_ENET) += ldpaa_eth/ obj-$(CONFIG_FSL_MEMAC) += fm/memac_phy.o obj-$(CONFIG_VSC9953) += vsc9953.o obj-$(CONFIG_PIC32_ETH) += pic32_mdio.o pic32_eth.o +obj-$(CONFIG_DWC_ETH_QOS) += dwc_eth_qos.o diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c new file mode 100644 index 000000000000..3be7a187d3e1 --- /dev/null +++ b/drivers/net/dwc_eth_qos.c @@ -0,0 +1,1492 @@ +/* + * Copyright (c) 2016, NVIDIA CORPORATION. + * + * SPDX-License-Identifier: GPL-2.0 + * + * Portions based on U-Boot's rtl8169.c. + */ + +/* + * This driver supports the Synopsys Designware Ethernet QOS (Quality Of + * Service) IP block. The IP supports multiple options for bus type, clocking/ + * reset structure, and feature list. This driver currently supports the + * specific configuration used in NVIDIA's Tegra186 chip. + * + * The driver is written such that generic core logic is kept separate from + * configuration-specific logic. Code that interacts with configuration- + * specific resources is split out into separate functions to avoid polluting + * common code. If/when this driver is enhanced to support multiple + * configurations, the core code should be adapted to call all configuration- + * specific functions through function pointers, with the definition of those + * function pointers being supplied by struct udevice_id eqos_ids[]'s .data + * field. + */ + +#include <common.h> +#include <clk.h> +#include <dm.h> +#include <errno.h> +#include <memalign.h> +#include <miiphy.h> +#include <net.h> +#include <netdev.h> +#include <phy.h> +#include <reset.h> +#include <asm/gpio.h> +#include <asm/io.h> + +/* Core registers */ + +#define EQOS_MAC_CONFIGURATION 0 +#define EQOS_MAC_CONFIGURATION_GPSLCE BIT(23) +#define EQOS_MAC_CONFIGURATION_CST BIT(21) +#define EQOS_MAC_CONFIGURATION_ACS BIT(20) +#define EQOS_MAC_CONFIGURATION_WD BIT(19) +#define EQOS_MAC_CONFIGURATION_JD BIT(17) +#define EQOS_MAC_CONFIGURATION_JE BIT(16) +#define EQOS_MAC_CONFIGURATION_PS BIT(15) +#define EQOS_MAC_CONFIGURATION_FES BIT(14) +#define EQOS_MAC_CONFIGURATION_DM BIT(13) +#define EQOS_MAC_CONFIGURATION_TE BIT(1) +#define EQOS_MAC_CONFIGURATION_RE BIT(0) + +#define EQOS_MAC_Q0_TX_FLOW_CTRL 0x70 +#define EQOS_MAC_Q0_TX_FLOW_CTRL_PT_SHIFT 16 +#define EQOS_MAC_Q0_TX_FLOW_CTRL_PT_MASK 0xffff +#define EQOS_MAC_Q0_TX_FLOW_CTRL_TFE BIT(1) + +#define EQOS_MAC_RX_FLOW_CTRL 0x90 +#define EQOS_MAC_RX_FLOW_CTRL_RFE BIT(0) + +#define EQOS_MAC_TXQ_PRTY_MAP0 0x98 +#define EQOS_MAC_TXQ_PRTY_MAP0_PSTQ0_SHIFT 0 +#define EQOS_MAC_TXQ_PRTY_MAP0_PSTQ0_MASK 0xff + +#define EQOS_MAC_RXQ_CTRL0 0xa0 +#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT 0 +#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_MASK 3 +#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_NOT_ENABLED 0 +#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_DCB 2 + +#define EQOS_MAC_RXQ_CTRL2 0xa8 +#define EQOS_MAC_RXQ_CTRL2_PSRQ0_SHIFT 0 +#define EQOS_MAC_RXQ_CTRL2_PSRQ0_MASK 0xff + +#define EQOS_MAC_1US_TIC_COUNTER 0xdc + +#define EQOS_MAC_HW_FEATURE0 0x11c + +#define EQOS_MAC_HW_FEATURE1 0x120 +#define EQOS_MAC_HW_FEATURE1_TXFIFOSIZE_SHIFT 6 +#define EQOS_MAC_HW_FEATURE1_TXFIFOSIZE_MASK 0x1f +#define EQOS_MAC_HW_FEATURE1_RXFIFOSIZE_SHIFT 0 +#define EQOS_MAC_HW_FEATURE1_RXFIFOSIZE_MASK 0x1f + +#define EQOS_MAC_HW_FEATURE2 0x124 + +#define EQOS_MAC_MDIO_ADDRESS 0x200 +#define EQOS_MAC_MDIO_ADDRESS_PA_SHIFT 21 +#define EQOS_MAC_MDIO_ADDRESS_RDA_SHIFT 16 +#define EQOS_MAC_MDIO_ADDRESS_CR_SHIFT 8 +#define EQOS_MAC_MDIO_ADDRESS_CR_20_35 2 +#define EQOS_MAC_MDIO_ADDRESS_SKAP BIT(4) +#define EQOS_MAC_MDIO_ADDRESS_GOC_SHIFT 2 +#define EQOS_MAC_MDIO_ADDRESS_GOC_READ 3 +#define EQOS_MAC_MDIO_ADDRESS_GOC_WRITE 1 +#define EQOS_MAC_MDIO_ADDRESS_C45E BIT(1) +#define EQOS_MAC_MDIO_ADDRESS_GB BIT(0) + +#define EQOS_MAC_MDIO_DATA 0x204 +#define EQOS_MAC_MDIO_DATA_GD_MASK 0xffff + +#define EQOS_MAC_ADDRESS0_HIGH 0x300 + +#define EQOS_MAC_ADDRESS0_LOW 0x304 + +#define EQOS_MTL_TXQ0_OPERATION_MODE 0xd00 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TQS_SHIFT 16 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TQS_MASK 0x1ff +#define EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_SHIFT 2 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_MASK 3 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_ENABLED 2 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TSF BIT(1) +#define EQOS_MTL_TXQ0_OPERATION_MODE_FTQ BIT(0) + +#define EQOS_MTL_TXQ0_DEBUG 0xd08 +#define EQOS_MTL_TXQ0_DEBUG_TXQSTS BIT(4) +#define EQOS_MTL_TXQ0_DEBUG_TRCSTS_SHIFT 1 +#define EQOS_MTL_TXQ0_DEBUG_TRCSTS_MASK 3 + +#define EQOS_MTL_TXQ0_QUANTUM_WEIGHT 0xd18 + +#define EQOS_MTL_RXQ0_OPERATION_MODE 0xd30 +#define EQOS_MTL_RXQ0_OPERATION_MODE_RQS_SHIFT 20 +#define EQOS_MTL_RXQ0_OPERATION_MODE_RQS_MASK 0x3ff +#define EQOS_MTL_RXQ0_OPERATION_MODE_RFD_SHIFT 14 +#define EQOS_MTL_RXQ0_OPERATION_MODE_RFD_MASK 0x3f +#define EQOS_MTL_RXQ0_OPERATION_MODE_RFA_SHIFT 8 +#define EQOS_MTL_RXQ0_OPERATION_MODE_RFA_MASK 0x3f +#define EQOS_MTL_RXQ0_OPERATION_MODE_EHFC BIT(7) +#define EQOS_MTL_RXQ0_OPERATION_MODE_RSF BIT(5) + +#define EQOS_MTL_RXQ0_DEBUG 0xd38 +#define EQOS_MTL_RXQ0_DEBUG_PRXQ_SHIFT 16 +#define EQOS_MTL_RXQ0_DEBUG_PRXQ_MASK 0x7fff +#define EQOS_MTL_RXQ0_DEBUG_RXQSTS_SHIFT 4 +#define EQOS_MTL_RXQ0_DEBUG_RXQSTS_MASK 3 + +#define EQOS_DMA_MODE 0x1000 +#define EQOS_DMA_MODE_SWR BIT(0) + +#define EQOS_DMA_SYSBUS_MODE 0x1004 +#define EQOS_DMA_SYSBUS_MODE_RD_OSR_LMT_SHIFT 16 +#define EQOS_DMA_SYSBUS_MODE_RD_OSR_LMT_MASK 0xf +#define EQOS_DMA_SYSBUS_MODE_EAME BIT(11) +#define EQOS_DMA_SYSBUS_MODE_BLEN16 BIT(3) +#define EQOS_DMA_SYSBUS_MODE_BLEN8 BIT(2) +#define EQOS_DMA_SYSBUS_MODE_BLEN4 BIT(1) + +#define EQOS_DMA_CH0_CONTROL 0x1100 +#define EQOS_DMA_CH0_CONTROL_PBLX8 BIT(16) + +#define EQOS_DMA_CH0_TX_CONTROL 0x1104 +#define EQOS_DMA_CH0_TX_CONTROL_TXPBL_SHIFT 16 +#define EQOS_DMA_CH0_TX_CONTROL_TXPBL_MASK 0x3f +#define EQOS_DMA_CH0_TX_CONTROL_OSP BIT(4) +#define EQOS_DMA_CH0_TX_CONTROL_ST BIT(0) + +#define EQOS_DMA_CH0_RX_CONTROL 0x1108 +#define EQOS_DMA_CH0_RX_CONTROL_RXPBL_SHIFT 16 +#define EQOS_DMA_CH0_RX_CONTROL_RXPBL_MASK 0x3f +#define EQOS_DMA_CH0_RX_CONTROL_RBSZ_SHIFT 1 +#define EQOS_DMA_CH0_RX_CONTROL_RBSZ_MASK 0x3fff +#define EQOS_DMA_CH0_RX_CONTROL_SR BIT(0) + +#define EQOS_DMA_CH0_TXDESC_LIST_HADDRESS 0x1110 + +#define EQOS_DMA_CH0_TXDESC_LIST_ADDRESS 0x1114 + +#define EQOS_DMA_CH0_RXDESC_LIST_HADDRESS 0x1118 + +#define EQOS_DMA_CH0_RXDESC_LIST_ADDRESS 0x111c + +#define EQOS_DMA_CH0_TXDESC_TAIL_POINTER 0x1120 + +#define EQOS_DMA_CH0_RXDESC_TAIL_POINTER 0x1128 + +#define EQOS_DMA_CH0_TXDESC_RING_LENGTH 0x112c + +#define EQOS_DMA_CH0_RXDESC_RING_LENGTH 0x1130 + +/* Registers located at 0x8000 and later are Tegra-specific */ + +#define EQOS_SDMEMCOMPPADCTRL 0x8800 +#define EQOS_SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD BIT(31) + +#define EQOS_AUTO_CAL_CONFIG 0x8804 +#define EQOS_AUTO_CAL_CONFIG_START BIT(31) +#define EQOS_AUTO_CAL_CONFIG_ENABLE BIT(29) + +#define EQOS_AUTO_CAL_STATUS 0x880c +#define EQOS_AUTO_CAL_STATUS_ACTIVE BIT(31) + +/* Descriptors */ + +#define EQOS_DESCRIPTOR_WORDS 4 +#define EQOS_DESCRIPTOR_SIZE (EQOS_DESCRIPTOR_WORDS * 4) +/* We assume ARCH_DMA_MINALIGN >= 16; 16 is the EQOS HW minimum */ +#define EQOS_DESCRIPTOR_ALIGN ARCH_DMA_MINALIGN +#define EQOS_DESCRIPTORS_TX 4 +#define EQOS_DESCRIPTORS_RX 4 +#define EQOS_DESCRIPTORS_NUM (EQOS_DESCRIPTORS_TX + EQOS_DESCRIPTORS_RX) +#define EQOS_DESCRIPTORS_SIZE ALIGN(EQOS_DESCRIPTORS_NUM * \ + EQOS_DESCRIPTOR_SIZE, ARCH_DMA_MINALIGN) +#define EQOS_BUFFER_ALIGN ARCH_DMA_MINALIGN +#define EQOS_MAX_PACKET_SIZE ALIGN(1568, ARCH_DMA_MINALIGN) +#define EQOS_RX_BUFFER_SIZE (EQOS_DESCRIPTORS_RX * EQOS_MAX_PACKET_SIZE) + +/* + * Warn if the cache-line size is larger than the descriptor size. In such + * cases the driver will likely fail because the CPU needs to flush the cache + * when requeuing RX buffers, therefore descriptors written by the hardware + * may be discarded. + * + * This can be fixed by defining CONFIG_SYS_NONCACHED_MEMORY which will cause + * the driver to allocate descriptors from a pool of non-cached memory. + */ +#if EQOS_DESCRIPTOR_SIZE < ARCH_DMA_MINALIGN +#if !defined(CONFIG_SYS_NONCACHED_MEMORY) && \ + !defined(CONFIG_SYS_DCACHE_OFF) && !defined(CONFIG_X86) +#warning Cache line size is larger than descriptor size +#endif +#endif + +#define EQOS_DESC3_OWN BIT(31) +#define EQOS_DESC3_FD BIT(29) +#define EQOS_DESC3_LD BIT(28) +#define EQOS_DESC3_BUF1V BIT(24) + +struct eqos_priv { + struct udevice *dev; + fdt_addr_t regs; + struct reset_ctl reset_ctl; + struct gpio_desc phy_reset_gpio; + struct clk clk_master_bus; + struct clk clk_rx; + struct clk clk_ptp_ref; + struct clk clk_tx; + struct clk clk_slave_bus; + struct mii_dev *mii; + struct phy_device *phy; + u32 *descs, *tx_descs, *rx_descs; + int tx_desc_idx, rx_desc_idx; + void *tx_dma_buf; + void *rx_dma_buf; + void *rx_pkt; + bool started; +}; + +/* + * TX and RX descriptors are 16 bytes. This causes problems with the cache + * maintenance on CPUs where the cache-line size exceeds the size of these + * descriptors. What will happen is that when the driver receives a packet + * it will be immediately requeued for the hardware to reuse. The CPU will + * therefore need to flush the cache-line containing the descriptor, which + * will cause all other descriptors in the same cache-line to be flushed + * along with it. If one of those descriptors had been written to by the + * device those changes (and the associated packet) will be lost. + * + * To work around this, we make use of non-cached memory if available. If + * descriptors are mapped uncached there's no need to manually flush them + * or invalidate them. + * + * Note that this only applies to descriptors. The packet data buffers do + * not have the same constraints since they are 1536 bytes large, so they + * are unlikely to share cache-lines. + */ +static u32 *eqos_alloc_descs(unsigned int num) +{ +#ifdef CONFIG_SYS_NONCACHED_MEMORY + return (u32 *)noncached_alloc(EQOS_DESCRIPTORS_SIZE, + EQOS_DESCRIPTOR_ALIGN); +#else + return memalign(EQOS_DESCRIPTOR_ALIGN, EQOS_DESCRIPTORS_SIZE); +#endif +} + +static void eqos_free_descs(u32 *descs) +{ +#ifdef CONFIG_SYS_NONCACHED_MEMORY + /* FIXME: noncached_alloc() has no opposite */ +#else + free(descs); +#endif +} + +static void eqos_inval_desc(u32 *desc) +{ +#ifndef CONFIG_SYS_NONCACHED_MEMORY + unsigned long start = (unsigned long)desc & ~(ARCH_DMA_MINALIGN - 1); + unsigned long end = ALIGN(start + EQOS_DESCRIPTOR_SIZE, + ARCH_DMA_MINALIGN); + + invalidate_dcache_range(start, end); +#endif +} + +static void eqos_flush_desc(u32 *desc) +{ +#ifndef CONFIG_SYS_NONCACHED_MEMORY + flush_cache((unsigned long)desc, EQOS_DESCRIPTOR_SIZE); +#endif +} + +static void eqos_inval_buffer(void *buf, size_t size) +{ + unsigned long start = (unsigned long)buf & ~(ARCH_DMA_MINALIGN - 1); + unsigned long end = ALIGN(start + size, ARCH_DMA_MINALIGN); + + invalidate_dcache_range(start, end); +} + +static void eqos_flush_buffer(void *buf, size_t size) +{ + flush_cache((unsigned long)buf, size); +} + +static int eqos_mdio_wait_idle(struct eqos_priv *eqos) +{ + int i; + u32 val; + + for (i = 0; i < 1000000; i++) { + if (i) + udelay(1); + val = readl(eqos->regs + EQOS_MAC_MDIO_ADDRESS); + if (!(val & EQOS_MAC_MDIO_ADDRESS_GB)) + return 0; + } + + return -ETIMEDOUT; +} + +static int eqos_mdio_read(struct mii_dev *bus, int mdio_addr, int mdio_devad, + int mdio_reg) +{ + struct eqos_priv *eqos = bus->priv; + u32 val; + int ret; + + debug("%s(dev=%p, addr=%x, reg=%d):\n", __func__, eqos->dev, mdio_addr, + mdio_reg); + + ret = eqos_mdio_wait_idle(eqos); + if (ret) { + error("MDIO not idle at entry"); + return ret; + } + + val = readl(eqos->regs + EQOS_MAC_MDIO_ADDRESS); + val &= EQOS_MAC_MDIO_ADDRESS_SKAP | + EQOS_MAC_MDIO_ADDRESS_C45E; + val |= (mdio_addr << EQOS_MAC_MDIO_ADDRESS_PA_SHIFT) | + (mdio_reg << EQOS_MAC_MDIO_ADDRESS_RDA_SHIFT) | + (EQOS_MAC_MDIO_ADDRESS_CR_20_35 << + EQOS_MAC_MDIO_ADDRESS_CR_SHIFT) | + (EQOS_MAC_MDIO_ADDRESS_GOC_READ << + EQOS_MAC_MDIO_ADDRESS_GOC_SHIFT) | + EQOS_MAC_MDIO_ADDRESS_GB; + writel(val, eqos->regs + EQOS_MAC_MDIO_ADDRESS); + + udelay(10); + + ret = eqos_mdio_wait_idle(eqos); + if (ret) { + error("MDIO read didn't complete"); + return ret; + } + + val = readl(eqos->regs + EQOS_MAC_MDIO_DATA); + val &= EQOS_MAC_MDIO_DATA_GD_MASK; + + debug("%s: val=%x\n", __func__, val); + + return val; +} + +static int eqos_mdio_write(struct mii_dev *bus, int mdio_addr, int mdio_devad, + int mdio_reg, u16 mdio_val) +{ + struct eqos_priv *eqos = bus->priv; + u32 val; + int ret; + + debug("%s(dev=%p, addr=%x, reg=%d, val=%x):\n", __func__, eqos->dev, + mdio_addr, mdio_reg, mdio_val); + + ret = eqos_mdio_wait_idle(eqos); + if (ret) { + error("MDIO not idle at entry"); + return ret; + } + + writel(mdio_val, eqos->regs + EQOS_MAC_MDIO_DATA); + + val = readl(eqos->regs + EQOS_MAC_MDIO_ADDRESS); + val &= EQOS_MAC_MDIO_ADDRESS_SKAP | + EQOS_MAC_MDIO_ADDRESS_C45E; + val |= (mdio_addr << EQOS_MAC_MDIO_ADDRESS_PA_SHIFT) | + (mdio_reg << EQOS_MAC_MDIO_ADDRESS_RDA_SHIFT) | + (EQOS_MAC_MDIO_ADDRESS_CR_20_35 << + EQOS_MAC_MDIO_ADDRESS_CR_SHIFT) | + (EQOS_MAC_MDIO_ADDRESS_GOC_WRITE << + EQOS_MAC_MDIO_ADDRESS_GOC_SHIFT) | + EQOS_MAC_MDIO_ADDRESS_GB; + writel(val, eqos->regs + EQOS_MAC_MDIO_ADDRESS); + + udelay(10); + + ret = eqos_mdio_wait_idle(eqos); + if (ret) { + error("MDIO read didn't complete"); + return ret; + } + + return 0; +} + +static int eqos_start_clks_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + int ret; + + debug("%s(dev=%p):\n", __func__, dev); + + ret = clk_enable(&eqos->clk_slave_bus); + if (ret < 0) { + error("clk_enable(clk_slave_bus) failed: %d", ret); + goto err; + } + + ret = clk_enable(&eqos->clk_master_bus); + if (ret < 0) { + error("clk_enable(clk_master_bus) failed: %d", ret); + goto err_disable_clk_slave_bus; + } + + ret = clk_enable(&eqos->clk_rx); + if (ret < 0) { + error("clk_enable(clk_rx) failed: %d", ret); + goto err_disable_clk_master_bus; + } + + ret = clk_enable(&eqos->clk_ptp_ref); + if (ret < 0) { + error("clk_enable(clk_ptp_ref) failed: %d", ret); + goto err_disable_clk_rx; + } + + ret = clk_set_rate(&eqos->clk_ptp_ref, 125 * 1000 * 1000); + if (ret < 0) { + error("clk_set_rate(clk_ptp_ref) failed: %d", ret); + goto err_disable_clk_ptp_ref; + } + + ret = clk_enable(&eqos->clk_tx); + if (ret < 0) { + error("clk_enable(clk_tx) failed: %d", ret); + goto err_disable_clk_ptp_ref; + } + + debug("%s: OK\n", __func__); + return 0; + +err_disable_clk_ptp_ref: + clk_disable(&eqos->clk_ptp_ref); +err_disable_clk_rx: + clk_disable(&eqos->clk_rx); +err_disable_clk_master_bus: + clk_disable(&eqos->clk_master_bus); +err_disable_clk_slave_bus: + clk_disable(&eqos->clk_slave_bus); +err: + debug("%s: FAILED: %d\n", __func__, ret); + return ret; +} + +void eqos_stop_clks_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + clk_disable(&eqos->clk_tx); + clk_disable(&eqos->clk_ptp_ref); + clk_disable(&eqos->clk_rx); + clk_disable(&eqos->clk_master_bus); + clk_disable(&eqos->clk_slave_bus); + + debug("%s: OK\n", __func__); +} + +static int eqos_start_resets_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + int ret; + + debug("%s(dev=%p):\n", __func__, dev); + + ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 1); + if (ret < 0) { + error("dm_gpio_set_value(phy_reset, assert) failed: %d", ret); + return ret; + } + + udelay(2); + + ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0); + if (ret < 0) { + error("dm_gpio_set_value(phy_reset, deassert) failed: %d", ret); + return ret; + } + + ret = reset_assert(&eqos->reset_ctl); + if (ret < 0) { + error("reset_assert() failed: %d", ret); + return ret; + } + + udelay(2); + + ret = reset_deassert(&eqos->reset_ctl); + if (ret < 0) { + error("reset_deassert() failed: %d", ret); + return ret; + } + + debug("%s: OK\n", __func__); + return 0; +} + +static int eqos_stop_resets_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + reset_assert(&eqos->reset_ctl); + dm_gpio_set_value(&eqos->phy_reset_gpio, 1); + + return 0; +} + +static int eqos_calibrate_pads_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + u32 val; + int i, ret; + + debug("%s(dev=%p):\n", __func__, dev); + + setbits_le32(eqos->regs + EQOS_SDMEMCOMPPADCTRL, + EQOS_SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD); + + udelay(1); + + setbits_le32(eqos->regs + EQOS_AUTO_CAL_CONFIG, + EQOS_AUTO_CAL_CONFIG_START | EQOS_AUTO_CAL_CONFIG_ENABLE); + + for (i = 0; i <= 10; i++) { + if (i) + udelay(1); + val = readl(eqos->regs + EQOS_AUTO_CAL_STATUS); + if (val & EQOS_AUTO_CAL_STATUS_ACTIVE) + break; + } + if (!(val & EQOS_AUTO_CAL_STATUS_ACTIVE)) { + error("calibrate didn't start"); + ret = -ETIMEDOUT; + goto failed; + } + + for (i = 0; i <= 10; i++) { + if (i) + udelay(20); + val = readl(eqos->regs + EQOS_AUTO_CAL_STATUS); + if (!(val & EQOS_AUTO_CAL_STATUS_ACTIVE)) + break; + } + if (val & EQOS_AUTO_CAL_STATUS_ACTIVE) { + error("calibrate didn't finish"); + ret = -ETIMEDOUT; + goto failed; + } + + ret = 0; + +failed: + clrbits_le32(eqos->regs + EQOS_SDMEMCOMPPADCTRL, + EQOS_SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD); + + debug("%s: returns %d\n", __func__, ret); + + return ret; +} + +static int eqos_disable_calibration_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + clrbits_le32(eqos->regs + EQOS_AUTO_CAL_CONFIG, + EQOS_AUTO_CAL_CONFIG_ENABLE); + + return 0; +} + +static ulong eqos_get_tick_clk_rate_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + return clk_get_rate(&eqos->clk_slave_bus); +} + +static int eqos_set_full_duplex(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + setbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION, + EQOS_MAC_CONFIGURATION_DM); + + return 0; +} + +static int eqos_set_half_duplex(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + clrbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION, + EQOS_MAC_CONFIGURATION_DM); + + /* WAR: Flush TX queue when switching to half-duplex */ + setbits_le32(eqos->regs + EQOS_MTL_TXQ0_OPERATION_MODE, + EQOS_MTL_TXQ0_OPERATION_MODE_FTQ); + + return 0; +} + +static int eqos_set_gmii_speed(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + clrbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION, + EQOS_MAC_CONFIGURATION_PS | EQOS_MAC_CONFIGURATION_FES); + + return 0; +} + +static int eqos_set_mii_speed_100(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + setbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION, + EQOS_MAC_CONFIGURATION_PS | EQOS_MAC_CONFIGURATION_FES); + + return 0; +} + +static int eqos_set_mii_speed_10(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + clrsetbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION, + EQOS_MAC_CONFIGURATION_FES, EQOS_MAC_CONFIGURATION_PS); + + return 0; +} + +static int eqos_set_tx_clk_speed_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + ulong rate; + int ret; + + debug("%s(dev=%p):\n", __func__, dev); + + switch (eqos->phy->speed) { + case SPEED_1000: + rate = 125 * 1000 * 1000; + break; + case SPEED_100: + rate = 25 * 1000 * 1000; + break; + case SPEED_10: + rate = 2.5 * 1000 * 1000; + break; + default: + error("invalid speed %d", eqos->phy->speed); + return -EINVAL; + } + + ret = clk_set_rate(&eqos->clk_tx, rate); + if (ret < 0) { + error("clk_set_rate(tx_clk, %lu) failed: %d", rate, ret); + return ret; + } + + return 0; +} + +static int eqos_adjust_link(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + int ret; + bool en_calibration; + + debug("%s(dev=%p):\n", __func__, dev); + + if (eqos->phy->duplex) + ret = eqos_set_full_duplex(dev); + else + ret = eqos_set_half_duplex(dev); + if (ret < 0) { + error("eqos_set_*_duplex() failed: %d", ret); + return ret; + } + + switch (eqos->phy->speed) { + case SPEED_1000: + en_calibration = true; + ret = eqos_set_gmii_speed(dev); + break; + case SPEED_100: + en_calibration = true; + ret = eqos_set_mii_speed_100(dev); + break; + case SPEED_10: + en_calibration = false; + ret = eqos_set_mii_speed_10(dev); + break; + default: + error("invalid speed %d", eqos->phy->speed); + return -EINVAL; + } + if (ret < 0) { + error("eqos_set_*mii_speed*() failed: %d", ret); + return ret; + } + + if (en_calibration) { + ret = eqos_calibrate_pads_tegra(dev); + if (ret < 0) { + error("eqos_calibrate_pads_tegra() failed: %d", ret); + return ret; + } + } else { + ret = eqos_disable_calibration_tegra(dev); + if (ret < 0) { + error("eqos_disable_calibration_tegra() failed: %d", + ret); + return ret; + } + } + + ret = eqos_set_tx_clk_speed_tegra(dev); + if (ret < 0) { + error("eqos_set_tx_clk_speed_tegra() failed: %d", ret); + return ret; + } + + return 0; +} + +static int eqos_start(struct udevice *dev) +{ + struct eth_pdata *plat = dev_get_platdata(dev); + struct eqos_priv *eqos = dev_get_priv(dev); + int ret, i; + ulong rate; + u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl; + ulong last_rx_desc; + + debug("%s(dev=%p):\n", __func__, dev); + + eqos->tx_desc_idx = 0; + eqos->rx_desc_idx = 0; + + ret = eqos_start_clks_tegra(dev); + if (ret < 0) { + error("eqos_start_clks_tegra() failed: %d", ret); + goto err; + } + + ret = eqos_start_resets_tegra(dev); + if (ret < 0) { + error("eqos_start_resets_tegra() failed: %d", ret); + goto err_stop_clks; + } + + udelay(10); + + for (i = 0; i < 1000000; i++) { + if (i) + udelay(1); + val = readl(eqos->regs + EQOS_DMA_MODE); + if (!(val & EQOS_DMA_MODE_SWR)) + break; + } + if (val & EQOS_DMA_MODE_SWR) { + error("EQOS_DMA_MODE_SWR stuck (0x%x)", val); + ret = -ETIMEDOUT; + goto err_stop_resets; + } + + ret = eqos_calibrate_pads_tegra(dev); + if (ret < 0) { + error("eqos_calibrate_pads_tegra() failed: %d", ret); + goto err_stop_resets; + } + + rate = eqos_get_tick_clk_rate_tegra(dev); + val = (rate / 1000000) - 1; + writel(val, eqos->regs + EQOS_MAC_1US_TIC_COUNTER); + + eqos->phy = phy_connect(eqos->mii, 0, dev, 0); + if (!eqos->phy) { + error("phy_connect() failed"); + goto err_stop_resets; + } + ret = phy_config(eqos->phy); + if (ret < 0) { + error("phy_config() failed: %d", ret); + goto err_shutdown_phy; + } + ret = phy_startup(eqos->phy); + if (ret < 0) { + error("phy_startup() failed: %d", ret); + goto err_shutdown_phy; + } + + if (!eqos->phy->link) { + error("No link"); + goto err_shutdown_phy; + } + + ret = eqos_adjust_link(dev); + if (ret < 0) { + error("eqos_adjust_link() failed: %d", ret); + goto err_shutdown_phy; + } + + /* Configure MTL */ + + /* Enable Store and Forward mode for TX */ + /* Program Tx operating mode */ + setbits_le32(eqos->regs + EQOS_MTL_TXQ0_OPERATION_MODE, + EQOS_MTL_TXQ0_OPERATION_MODE_TSF | + (EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_ENABLED << + EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_SHIFT)); + + /* Transmit Queue weight */ + writel(0x10, eqos->regs + EQOS_MTL_TXQ0_QUANTUM_WEIGHT); + + /* Enable Store and Forward mode for RX, since no jumbo frame */ + setbits_le32(eqos->regs + EQOS_MTL_RXQ0_OPERATION_MODE, + EQOS_MTL_RXQ0_OPERATION_MODE_RSF); + + /* Transmit/Receive queue fifo size; use all RAM for 1 queue */ + val = readl(eqos->regs + EQOS_MAC_HW_FEATURE1); + tx_fifo_sz = (val >> EQOS_MAC_HW_FEATURE1_TXFIFOSIZE_SHIFT) & + EQOS_MAC_HW_FEATURE1_TXFIFOSIZE_MASK; + rx_fifo_sz = (val >> EQOS_MAC_HW_FEATURE1_RXFIFOSIZE_SHIFT) & + EQOS_MAC_HW_FEATURE1_RXFIFOSIZE_MASK; + + /* + * r/tx_fifo_sz is encoded as log2(n / 128). Undo that by shifting. + * r/tqs is encoded as (n / 256) - 1. + */ + tqs = (128 << tx_fifo_sz) / 256 - 1; + rqs = (128 << rx_fifo_sz) / 256 - 1; + + clrsetbits_le32(eqos->regs + EQOS_MTL_TXQ0_OPERATION_MODE, + EQOS_MTL_TXQ0_OPERATION_MODE_TQS_MASK << + EQOS_MTL_TXQ0_OPERATION_MODE_TQS_SHIFT, + tqs << EQOS_MTL_TXQ0_OPERATION_MODE_TQS_SHIFT); + clrsetbits_le32(eqos->regs + EQOS_MTL_RXQ0_OPERATION_MODE, + EQOS_MTL_RXQ0_OPERATION_MODE_RQS_MASK << + EQOS_MTL_RXQ0_OPERATION_MODE_RQS_SHIFT, + rqs << EQOS_MTL_RXQ0_OPERATION_MODE_RQS_SHIFT); + + /* Flow control used only if each channel gets 4KB or more FIFO */ + if (rqs >= ((4096 / 256) - 1)) { + u32 rfd, rfa; + + setbits_le32(eqos->regs + EQOS_MTL_RXQ0_OPERATION_MODE, + EQOS_MTL_RXQ0_OPERATION_MODE_EHFC); + + /* + * Set Threshold for Activating Flow Contol space for min 2 + * frames ie, (1500 * 1) = 1500 bytes. + * + * Set Threshold for Deactivating Flow Contol for space of + * min 1 frame (frame size 1500bytes) in receive fifo + */ + if (rqs == ((4096 / 256) - 1)) { + /* + * This violates the above formula because of FIFO size + * limit therefore overflow may occur inspite of this. + */ + rfd = 0x3; /* Full-3K */ + rfa = 0x1; /* Full-1.5K */ + } else if (rqs == ((8192 / 256) - 1)) { + rfd = 0x6; /* Full-4K */ + rfa = 0xa; /* Full-6K */ + } else if (rqs == ((16384 / 256) - 1)) { + rfd = 0x6; /* Full-4K */ + rfa = 0x12; /* Full-10K */ + } else { + rfd = 0x6; /* Full-4K */ + rfa = 0x1E; /* Full-16K */ + } + + clrsetbits_le32(eqos->regs + EQOS_MTL_RXQ0_OPERATION_MODE, + (EQOS_MTL_RXQ0_OPERATION_MODE_RFD_MASK << + EQOS_MTL_RXQ0_OPERATION_MODE_RFD_SHIFT) | + (EQOS_MTL_RXQ0_OPERATION_MODE_RFA_MASK << + EQOS_MTL_RXQ0_OPERATION_MODE_RFA_SHIFT), + (rfd << + EQOS_MTL_RXQ0_OPERATION_MODE_RFD_SHIFT) | + (rfa << + EQOS_MTL_RXQ0_OPERATION_MODE_RFA_SHIFT)); + } + + /* Configure MAC */ + + clrsetbits_le32(eqos->regs + EQOS_MAC_RXQ_CTRL0, + EQOS_MAC_RXQ_CTRL0_RXQ0EN_MASK << + EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT, + EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_DCB << + EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT); + + /* Set TX flow control parameters */ + /* Set Pause Time */ + setbits_le32(eqos->regs + EQOS_MAC_Q0_TX_FLOW_CTRL, + 0xffff << EQOS_MAC_Q0_TX_FLOW_CTRL_PT_SHIFT); + /* Assign priority for TX flow control */ + clrbits_le32(eqos->regs + EQOS_MAC_TXQ_PRTY_MAP0, + EQOS_MAC_TXQ_PRTY_MAP0_PSTQ0_MASK << + EQOS_MAC_TXQ_PRTY_MAP0_PSTQ0_SHIFT); + /* Assign priority for RX flow control */ + clrbits_le32(eqos->regs + EQOS_MAC_RXQ_CTRL2, + EQOS_MAC_RXQ_CTRL2_PSRQ0_MASK << + EQOS_MAC_RXQ_CTRL2_PSRQ0_SHIFT); + /* Enable flow control */ + setbits_le32(eqos->regs + EQOS_MAC_Q0_TX_FLOW_CTRL, + EQOS_MAC_Q0_TX_FLOW_CTRL_TFE); + setbits_le32(eqos->regs + EQOS_MAC_RX_FLOW_CTRL, + EQOS_MAC_RX_FLOW_CTRL_RFE); + + clrsetbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION, + EQOS_MAC_CONFIGURATION_GPSLCE | + EQOS_MAC_CONFIGURATION_WD | + EQOS_MAC_CONFIGURATION_JD | + EQOS_MAC_CONFIGURATION_JE, + EQOS_MAC_CONFIGURATION_CST | + EQOS_MAC_CONFIGURATION_ACS); + + /* Update the MAC address */ + val = (plat->enetaddr[5] << 8) | + (plat->enetaddr[4]); + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH); + val = (plat->enetaddr[3] << 24) | + (plat->enetaddr[2] << 16) | + (plat->enetaddr[1] << 8) | + (plat->enetaddr[0]); + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW); + + /* Configure DMA */ + + /* Enable OSP mode */ + setbits_le32(eqos->regs + EQOS_DMA_CH0_TX_CONTROL, + EQOS_DMA_CH0_TX_CONTROL_OSP); + + /* RX buffer size. Must be a multiple of bus width */ + clrsetbits_le32(eqos->regs + EQOS_DMA_CH0_RX_CONTROL, + EQOS_DMA_CH0_RX_CONTROL_RBSZ_MASK << + EQOS_DMA_CH0_RX_CONTROL_RBSZ_SHIFT, + EQOS_MAX_PACKET_SIZE << + EQOS_DMA_CH0_RX_CONTROL_RBSZ_SHIFT); + + setbits_le32(eqos->regs + EQOS_DMA_CH0_CONTROL, + EQOS_DMA_CH0_CONTROL_PBLX8); + + /* + * Burst length must be < 1/2 FIFO size. + * FIFO size in tqs is encoded as (n / 256) - 1. + * Each burst is n * 8 (PBLX8) * 16 (AXI width) == 128 bytes. + * Half of n * 256 is n * 128, so pbl == tqs, modulo the -1. + */ + pbl = tqs + 1; + if (pbl > 32) + pbl = 32; + clrsetbits_le32(eqos->regs + EQOS_DMA_CH0_TX_CONTROL, + EQOS_DMA_CH0_TX_CONTROL_TXPBL_MASK << + EQOS_DMA_CH0_TX_CONTROL_TXPBL_SHIFT, + pbl << EQOS_DMA_CH0_TX_CONTROL_TXPBL_SHIFT); + + clrsetbits_le32(eqos->regs + EQOS_DMA_CH0_RX_CONTROL, + EQOS_DMA_CH0_RX_CONTROL_RXPBL_MASK << + EQOS_DMA_CH0_RX_CONTROL_RXPBL_SHIFT, + 8 << EQOS_DMA_CH0_RX_CONTROL_RXPBL_SHIFT); + + /* DMA performance configuration */ + val = (2 << EQOS_DMA_SYSBUS_MODE_RD_OSR_LMT_SHIFT) | + EQOS_DMA_SYSBUS_MODE_EAME | EQOS_DMA_SYSBUS_MODE_BLEN16 | + EQOS_DMA_SYSBUS_MODE_BLEN8 | EQOS_DMA_SYSBUS_MODE_BLEN4; + writel(val, eqos->regs + EQOS_DMA_SYSBUS_MODE); + + /* Set up descriptors */ + + memset(eqos->descs, 0, EQOS_DESCRIPTORS_SIZE); + for (i = 0; i < EQOS_DESCRIPTORS_RX; i++) { + u32 *rx_desc = &(eqos->rx_descs[i * EQOS_DESCRIPTOR_WORDS]); + rx_desc[0] = (u32)(ulong)(eqos->rx_dma_buf + + (i * EQOS_MAX_PACKET_SIZE)); + rx_desc[3] |= EQOS_DESC3_OWN | EQOS_DESC3_BUF1V; + } + flush_cache((unsigned long)eqos->descs, EQOS_DESCRIPTORS_SIZE); + + writel(0, eqos->regs + EQOS_DMA_CH0_TXDESC_LIST_HADDRESS); + writel((ulong)eqos->tx_descs, + eqos->regs + EQOS_DMA_CH0_TXDESC_LIST_ADDRESS); + writel(EQOS_DESCRIPTORS_TX - 1, + eqos->regs + EQOS_DMA_CH0_TXDESC_RING_LENGTH); + + writel(0, eqos->regs + EQOS_DMA_CH0_RXDESC_LIST_HADDRESS); + writel((ulong)eqos->rx_descs, + eqos->regs + EQOS_DMA_CH0_RXDESC_LIST_ADDRESS); + writel(EQOS_DESCRIPTORS_RX - 1, + eqos->regs + EQOS_DMA_CH0_RXDESC_RING_LENGTH); + + /* Enable everything */ + + setbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION, + EQOS_MAC_CONFIGURATION_TE | EQOS_MAC_CONFIGURATION_RE); + + setbits_le32(eqos->regs + EQOS_DMA_CH0_TX_CONTROL, + EQOS_DMA_CH0_TX_CONTROL_ST); + setbits_le32(eqos->regs + EQOS_DMA_CH0_RX_CONTROL, + EQOS_DMA_CH0_RX_CONTROL_SR); + + /* TX tail pointer not written until we need to TX a packet */ + /* + * Point RX tail pointer at last descriptor. Ideally, we'd point at the + * first descriptor, implying all descriptors were available. However, + * that's not distinguishable from none of the descriptors being + * available. + */ + last_rx_desc = (ulong)&(eqos->rx_descs[(EQOS_DESCRIPTORS_RX - 1) * + EQOS_DESCRIPTOR_WORDS]); + writel(last_rx_desc, eqos->regs + EQOS_DMA_CH0_RXDESC_TAIL_POINTER); + + eqos->started = true; + + debug("%s: OK\n", __func__); + return 0; + +err_shutdown_phy: + phy_shutdown(eqos->phy); + eqos->phy = NULL; +err_stop_resets: + eqos_stop_resets_tegra(dev); +err_stop_clks: + eqos_stop_clks_tegra(dev); +err: + error("FAILED: %d", ret); + return ret; +} + +void eqos_stop(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + int i; + + debug("%s(dev=%p):\n", __func__, dev); + + if (!eqos->started) + return; + eqos->started = false; + + /* Disable TX DMA */ + clrbits_le32(eqos->regs + EQOS_DMA_CH0_TX_CONTROL, + EQOS_DMA_CH0_TX_CONTROL_ST); + + /* Wait for TX all packets to drain out of MTL */ + for (i = 0; i < 1000000; i++) { + u32 val = readl(eqos->regs + EQOS_MTL_TXQ0_DEBUG); + u32 trcsts = (val >> EQOS_MTL_TXQ0_DEBUG_TRCSTS_SHIFT) & + EQOS_MTL_TXQ0_DEBUG_TRCSTS_MASK; + u32 txqsts = val & EQOS_MTL_TXQ0_DEBUG_TXQSTS; + if ((trcsts != 1) && (!txqsts)) + break; + } + + /* Turn off MAC TX and RX */ + clrbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION, + EQOS_MAC_CONFIGURATION_TE | EQOS_MAC_CONFIGURATION_RE); + + /* Wait for all RX packets to drain out of MTL */ + for (i = 0; i < 1000000; i++) { + u32 val = readl(eqos->regs + EQOS_MTL_RXQ0_DEBUG); + u32 prxq = (val >> EQOS_MTL_RXQ0_DEBUG_PRXQ_SHIFT) & + EQOS_MTL_RXQ0_DEBUG_PRXQ_MASK; + u32 rxqsts = (val >> EQOS_MTL_RXQ0_DEBUG_RXQSTS_SHIFT) & + EQOS_MTL_RXQ0_DEBUG_RXQSTS_MASK; + if ((!prxq) && (!rxqsts)) + break; + } + + /* Turn off RX DMA */ + clrbits_le32(eqos->regs + EQOS_DMA_CH0_RX_CONTROL, + EQOS_DMA_CH0_RX_CONTROL_SR); + + if (eqos->phy) { + phy_shutdown(eqos->phy); + eqos->phy = NULL; + } + eqos_stop_resets_tegra(dev); + eqos_stop_clks_tegra(dev); + + debug("%s: OK\n", __func__); +} + +int eqos_send(struct udevice *dev, void *packet, int length) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + u32 *tx_desc; + int i; + + debug("%s(dev=%p, packet=%p, length=%d):\n", __func__, dev, packet, + length); + + memcpy(eqos->tx_dma_buf, packet, length); + eqos_flush_buffer(eqos->tx_dma_buf, length); + + tx_desc = &(eqos->tx_descs[eqos->tx_desc_idx * EQOS_DESCRIPTOR_WORDS]); + eqos->tx_desc_idx++; + eqos->tx_desc_idx %= EQOS_DESCRIPTORS_TX; + + tx_desc[0] = (ulong)eqos->tx_dma_buf; + tx_desc[1] = 0; + tx_desc[2] = length; + /* + * Make sure that if HW sees the _OWN write below, it will see all the + * writes to the rest of the descriptor too. + */ + mb(); + tx_desc[3] = EQOS_DESC3_OWN | EQOS_DESC3_FD | EQOS_DESC3_LD | length; + eqos_flush_desc(tx_desc); + + writel((ulong)(tx_desc + 4), + eqos->regs + EQOS_DMA_CH0_TXDESC_TAIL_POINTER); + + for (i = 0; i < 1000000; i++) { + eqos_inval_desc(tx_desc); + if (!(readl(&tx_desc[3]) & EQOS_DESC3_OWN)) + return 0; + udelay(1); + } + + debug("%s: TX timeout\n", __func__); + + return -ETIMEDOUT; +} + +int eqos_recv(struct udevice *dev, int flags, uchar **packetp) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + u32 *rx_desc; + int length; + + debug("%s(dev=%p, flags=%x):\n", __func__, dev, flags); + + rx_desc = &(eqos->rx_descs[eqos->rx_desc_idx * EQOS_DESCRIPTOR_WORDS]); + if (rx_desc[3] & EQOS_DESC3_OWN) { + debug("%s: RX packet not available\n", __func__); + return -EAGAIN; + } + + *packetp = eqos->rx_dma_buf + + (eqos->rx_desc_idx * EQOS_MAX_PACKET_SIZE); + length = rx_desc[3] & 0x7fff; + debug("%s: *packetp=%p, length=%d\n", __func__, *packetp, length); + + eqos_inval_buffer(*packetp, length); + + return length; +} + +int eqos_free_pkt(struct udevice *dev, uchar *packet, int length) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + uchar *packet_expected; + u32 *rx_desc; + + debug("%s(packet=%p, length=%d)\n", __func__, packet, length); + + packet_expected = eqos->rx_dma_buf + + (eqos->rx_desc_idx * EQOS_MAX_PACKET_SIZE); + if (packet != packet_expected) { + debug("%s: Unexpected packet (expected %p)\n", __func__, + packet_expected); + return -EINVAL; + } + + rx_desc = &(eqos->rx_descs[eqos->rx_desc_idx * EQOS_DESCRIPTOR_WORDS]); + rx_desc[0] = (u32)(ulong)packet; + rx_desc[1] = 0; + rx_desc[2] = 0; + /* + * Make sure that if HW sees the _OWN write below, it will see all the + * writes to the rest of the descriptor too. + */ + mb(); + rx_desc[3] |= EQOS_DESC3_OWN | EQOS_DESC3_BUF1V; + eqos_flush_desc(rx_desc); + + writel((ulong)rx_desc, eqos->regs + EQOS_DMA_CH0_RXDESC_TAIL_POINTER); + + eqos->rx_desc_idx++; + eqos->rx_desc_idx %= EQOS_DESCRIPTORS_RX; + + return 0; +} + +static int eqos_probe_resources_core(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + int ret; + + debug("%s(dev=%p):\n", __func__, dev); + + eqos->descs = eqos_alloc_descs(EQOS_DESCRIPTORS_TX + + EQOS_DESCRIPTORS_RX); + if (!eqos->descs) { + debug("%s: eqos_alloc_descs() failed\n", __func__); + ret = -ENOMEM; + goto err; + } + eqos->tx_descs = eqos->descs; + eqos->rx_descs = eqos->tx_descs + + (EQOS_DESCRIPTORS_TX * EQOS_DESCRIPTOR_WORDS); + debug("%s: tx_descs=%p, rx_descs=%p\n", __func__, eqos->tx_descs, + eqos->rx_descs); + + eqos->tx_dma_buf = memalign(EQOS_BUFFER_ALIGN, EQOS_MAX_PACKET_SIZE); + if (!eqos->tx_dma_buf) { + debug("%s: memalign(tx_dma_buf) failed\n", __func__); + ret = -ENOMEM; + goto err_free_descs; + } + debug("%s: rx_dma_buf=%p\n", __func__, eqos->rx_dma_buf); + + eqos->rx_dma_buf = memalign(EQOS_BUFFER_ALIGN, EQOS_RX_BUFFER_SIZE); + if (!eqos->rx_dma_buf) { + debug("%s: memalign(rx_dma_buf) failed\n", __func__); + ret = -ENOMEM; + goto err_free_tx_dma_buf; + } + debug("%s: tx_dma_buf=%p\n", __func__, eqos->tx_dma_buf); + + eqos->rx_pkt = malloc(EQOS_MAX_PACKET_SIZE); + if (!eqos->rx_pkt) { + debug("%s: malloc(rx_pkt) failed\n", __func__); + ret = -ENOMEM; + goto err_free_rx_dma_buf; + } + debug("%s: rx_pkt=%p\n", __func__, eqos->rx_pkt); + + debug("%s: OK\n", __func__); + return 0; + +err_free_rx_dma_buf: + free(eqos->rx_dma_buf); +err_free_tx_dma_buf: + free(eqos->tx_dma_buf); +err_free_descs: + eqos_free_descs(eqos->descs); +err: + + debug("%s: returns %d\n", __func__, ret); + return ret; +} + +static int eqos_remove_resources_core(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + free(eqos->rx_pkt); + free(eqos->rx_dma_buf); + free(eqos->tx_dma_buf); + eqos_free_descs(eqos->descs); + + debug("%s: OK\n", __func__); + return 0; +} + +static int eqos_probe_resources_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + int ret; + + debug("%s(dev=%p):\n", __func__, dev); + + ret = reset_get_by_name(dev, "eqos", &eqos->reset_ctl); + if (ret) { + error("reset_get_by_name(rst) failed: %d", ret); + return ret; + } + + ret = gpio_request_by_name(dev, "phy-reset-gpios", 0, + &eqos->phy_reset_gpio, + GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE); + if (ret) { + error("gpio_request_by_name(phy reset) failed: %d", ret); + goto err_free_reset_eqos; + } + + ret = clk_get_by_name(dev, "slave_bus", &eqos->clk_slave_bus); + if (ret) { + error("clk_get_by_name(slave_bus) failed: %d", ret); + goto err_free_gpio_phy_reset; + } + + ret = clk_get_by_name(dev, "master_bus", &eqos->clk_master_bus); + if (ret) { + error("clk_get_by_name(master_bus) failed: %d", ret); + goto err_free_clk_slave_bus; + } + + ret = clk_get_by_name(dev, "rx", &eqos->clk_rx); + if (ret) { + error("clk_get_by_name(rx) failed: %d", ret); + goto err_free_clk_master_bus; + } + + ret = clk_get_by_name(dev, "ptp_ref", &eqos->clk_ptp_ref); + if (ret) { + error("clk_get_by_name(ptp_ref) failed: %d", ret); + goto err_free_clk_rx; + return ret; + } + + ret = clk_get_by_name(dev, "tx", &eqos->clk_tx); + if (ret) { + error("clk_get_by_name(tx) failed: %d", ret); + goto err_free_clk_ptp_ref; + } + + debug("%s: OK\n", __func__); + return 0; + +err_free_clk_ptp_ref: + clk_free(&eqos->clk_ptp_ref); +err_free_clk_rx: + clk_free(&eqos->clk_rx); +err_free_clk_master_bus: + clk_free(&eqos->clk_master_bus); +err_free_clk_slave_bus: + clk_free(&eqos->clk_slave_bus); +err_free_gpio_phy_reset: + dm_gpio_free(dev, &eqos->phy_reset_gpio); +err_free_reset_eqos: + reset_free(&eqos->reset_ctl); + + debug("%s: returns %d\n", __func__, ret); + return ret; +} + +static int eqos_remove_resources_tegra(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + clk_free(&eqos->clk_tx); + clk_free(&eqos->clk_ptp_ref); + clk_free(&eqos->clk_rx); + clk_free(&eqos->clk_slave_bus); + clk_free(&eqos->clk_master_bus); + dm_gpio_free(dev, &eqos->phy_reset_gpio); + reset_free(&eqos->reset_ctl); + + debug("%s: OK\n", __func__); + return 0; +} + +static int eqos_probe(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + int ret; + + debug("%s(dev=%p):\n", __func__, dev); + + eqos->dev = dev; + + eqos->regs = dev_get_addr(dev); + if (eqos->regs == FDT_ADDR_T_NONE) { + error("dev_get_addr() failed"); + return -ENODEV; + } + + ret = eqos_probe_resources_core(dev); + if (ret < 0) { + error("eqos_probe_resources_core() failed: %d", ret); + return ret; + } + + ret = eqos_probe_resources_tegra(dev); + if (ret < 0) { + error("eqos_probe_resources_tegra() failed: %d", ret); + goto err_remove_resources_core; + } + + eqos->mii = mdio_alloc(); + if (!eqos->mii) { + error("mdio_alloc() failed"); + goto err_remove_resources_tegra; + } + eqos->mii->read = eqos_mdio_read; + eqos->mii->write = eqos_mdio_write; + eqos->mii->priv = eqos; + strcpy(eqos->mii->name, dev->name); + + ret = mdio_register(eqos->mii); + if (ret < 0) { + error("mdio_register() failed: %d", ret); + goto err_free_mdio; + } + + debug("%s: OK\n", __func__); + return 0; + +err_free_mdio: + mdio_free(eqos->mii); +err_remove_resources_tegra: + eqos_remove_resources_tegra(dev); +err_remove_resources_core: + eqos_remove_resources_core(dev); + + debug("%s: returns %d\n", __func__, ret); + return ret; +} + +static int eqos_remove(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + mdio_unregister(eqos->mii); + mdio_free(eqos->mii); + eqos_remove_resources_tegra(dev); + eqos_probe_resources_core(dev); + + debug("%s: OK\n", __func__); + return 0; +} + +static const struct eth_ops eqos_ops = { + .start = eqos_start, + .stop = eqos_stop, + .send = eqos_send, + .recv = eqos_recv, + .free_pkt = eqos_free_pkt, +}; + +static const struct udevice_id eqos_ids[] = { + { .compatible = "nvidia,tegra186-eqos" }, + { } +}; + +U_BOOT_DRIVER(eth_eqos) = { + .name = "eth_eqos", + .id = UCLASS_ETH, + .of_match = eqos_ids, + .probe = eqos_probe, + .remove = eqos_remove, + .ops = &eqos_ops, + .priv_auto_alloc_size = sizeof(struct eqos_priv), + .platdata_auto_alloc_size = sizeof(struct eth_pdata), +};

On 12 September 2016 at 11:48, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
Signed-off-by: Stephen Warren swarren@nvidia.com
drivers/net/Kconfig | 11 + drivers/net/Makefile | 1 + drivers/net/dwc_eth_qos.c | 1492 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1504 insertions(+) create mode 100644 drivers/net/dwc_eth_qos.c
Reviewed-by: Simon Glass sjg@chromium.org

Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
Signed-off-by: Stephen Warren swarren@nvidia.com
drivers/net/Kconfig | 11 + drivers/net/Makefile | 1 + drivers/net/dwc_eth_qos.c | 1492 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1504 insertions(+) create mode 100644 drivers/net/dwc_eth_qos.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index be3ed73e5221..8b35415a04ce 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -64,6 +64,17 @@ config ALTERA_TSE Please find details on the "Triple-Speed Ethernet MegaCore Function Resource Center" of Altera.
+config DWC_ETH_QOS
bool "Synopsys DWC Ethernet QOS device support"
depends on DM_ETH
select PHYLIB
help
This driver supports the Synopsys Designware Ethernet QOS (Quality
Of Service) IP block. The IP supports many options for bus type,
clocking/reset structure, and feature list. This driver currently
supports the specific configuration used in NVIDIA's Tegra186 chip,
but should be extensible to other combinations quite easily.
config E1000 bool "Intel PRO/1000 Gigabit Ethernet support" help diff --git a/drivers/net/Makefile b/drivers/net/Makefile index a4485266d457..9a7bfc6d5b05 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -76,3 +76,4 @@ obj-$(CONFIG_FSL_MC_ENET) += ldpaa_eth/ obj-$(CONFIG_FSL_MEMAC) += fm/memac_phy.o obj-$(CONFIG_VSC9953) += vsc9953.o obj-$(CONFIG_PIC32_ETH) += pic32_mdio.o pic32_eth.o +obj-$(CONFIG_DWC_ETH_QOS) += dwc_eth_qos.o diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c new file mode 100644 index 000000000000..3be7a187d3e1 --- /dev/null +++ b/drivers/net/dwc_eth_qos.c @@ -0,0 +1,1492 @@ +/*
- Copyright (c) 2016, NVIDIA CORPORATION.
- SPDX-License-Identifier: GPL-2.0
- Portions based on U-Boot's rtl8169.c.
- */
+/*
- This driver supports the Synopsys Designware Ethernet QOS (Quality Of
- Service) IP block. The IP supports multiple options for bus type, clocking/
- reset structure, and feature list. This driver currently supports the
- specific configuration used in NVIDIA's Tegra186 chip.
- The driver is written such that generic core logic is kept separate from
- configuration-specific logic. Code that interacts with configuration-
- specific resources is split out into separate functions to avoid polluting
- common code. If/when this driver is enhanced to support multiple
- configurations, the core code should be adapted to call all configuration-
- specific functions through function pointers, with the definition of those
- function pointers being supplied by struct udevice_id eqos_ids[]'s .data
- field.
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <errno.h> +#include <memalign.h> +#include <miiphy.h> +#include <net.h> +#include <netdev.h> +#include <phy.h> +#include <reset.h> +#include <asm/gpio.h> +#include <asm/io.h>
+/* Core registers */
Why are registers not defined as a struct? That is the common way that is accepted to represent registers in a driver.
+#define EQOS_MAC_CONFIGURATION 0 +#define EQOS_MAC_CONFIGURATION_GPSLCE BIT(23) +#define EQOS_MAC_CONFIGURATION_CST BIT(21) +#define EQOS_MAC_CONFIGURATION_ACS BIT(20) +#define EQOS_MAC_CONFIGURATION_WD BIT(19) +#define EQOS_MAC_CONFIGURATION_JD BIT(17) +#define EQOS_MAC_CONFIGURATION_JE BIT(16) +#define EQOS_MAC_CONFIGURATION_PS BIT(15) +#define EQOS_MAC_CONFIGURATION_FES BIT(14) +#define EQOS_MAC_CONFIGURATION_DM BIT(13) +#define EQOS_MAC_CONFIGURATION_TE BIT(1) +#define EQOS_MAC_CONFIGURATION_RE BIT(0)
+#define EQOS_MAC_Q0_TX_FLOW_CTRL 0x70 +#define EQOS_MAC_Q0_TX_FLOW_CTRL_PT_SHIFT 16 +#define EQOS_MAC_Q0_TX_FLOW_CTRL_PT_MASK 0xffff +#define EQOS_MAC_Q0_TX_FLOW_CTRL_TFE BIT(1)
+#define EQOS_MAC_RX_FLOW_CTRL 0x90 +#define EQOS_MAC_RX_FLOW_CTRL_RFE BIT(0)
+#define EQOS_MAC_TXQ_PRTY_MAP0 0x98 +#define EQOS_MAC_TXQ_PRTY_MAP0_PSTQ0_SHIFT 0 +#define EQOS_MAC_TXQ_PRTY_MAP0_PSTQ0_MASK 0xff
+#define EQOS_MAC_RXQ_CTRL0 0xa0 +#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT 0 +#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_MASK 3 +#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_NOT_ENABLED 0 +#define EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_DCB 2
+#define EQOS_MAC_RXQ_CTRL2 0xa8 +#define EQOS_MAC_RXQ_CTRL2_PSRQ0_SHIFT 0 +#define EQOS_MAC_RXQ_CTRL2_PSRQ0_MASK 0xff
+#define EQOS_MAC_1US_TIC_COUNTER 0xdc
+#define EQOS_MAC_HW_FEATURE0 0x11c
+#define EQOS_MAC_HW_FEATURE1 0x120 +#define EQOS_MAC_HW_FEATURE1_TXFIFOSIZE_SHIFT 6 +#define EQOS_MAC_HW_FEATURE1_TXFIFOSIZE_MASK 0x1f +#define EQOS_MAC_HW_FEATURE1_RXFIFOSIZE_SHIFT 0 +#define EQOS_MAC_HW_FEATURE1_RXFIFOSIZE_MASK 0x1f
+#define EQOS_MAC_HW_FEATURE2 0x124
+#define EQOS_MAC_MDIO_ADDRESS 0x200 +#define EQOS_MAC_MDIO_ADDRESS_PA_SHIFT 21 +#define EQOS_MAC_MDIO_ADDRESS_RDA_SHIFT 16 +#define EQOS_MAC_MDIO_ADDRESS_CR_SHIFT 8 +#define EQOS_MAC_MDIO_ADDRESS_CR_20_35 2 +#define EQOS_MAC_MDIO_ADDRESS_SKAP BIT(4) +#define EQOS_MAC_MDIO_ADDRESS_GOC_SHIFT 2 +#define EQOS_MAC_MDIO_ADDRESS_GOC_READ 3 +#define EQOS_MAC_MDIO_ADDRESS_GOC_WRITE 1 +#define EQOS_MAC_MDIO_ADDRESS_C45E BIT(1) +#define EQOS_MAC_MDIO_ADDRESS_GB BIT(0)
+#define EQOS_MAC_MDIO_DATA 0x204 +#define EQOS_MAC_MDIO_DATA_GD_MASK 0xffff
+#define EQOS_MAC_ADDRESS0_HIGH 0x300
+#define EQOS_MAC_ADDRESS0_LOW 0x304
+#define EQOS_MTL_TXQ0_OPERATION_MODE 0xd00 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TQS_SHIFT 16 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TQS_MASK 0x1ff +#define EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_SHIFT 2 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_MASK 3 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_ENABLED 2 +#define EQOS_MTL_TXQ0_OPERATION_MODE_TSF BIT(1) +#define EQOS_MTL_TXQ0_OPERATION_MODE_FTQ BIT(0)
+#define EQOS_MTL_TXQ0_DEBUG 0xd08 +#define EQOS_MTL_TXQ0_DEBUG_TXQSTS BIT(4) +#define EQOS_MTL_TXQ0_DEBUG_TRCSTS_SHIFT 1 +#define EQOS_MTL_TXQ0_DEBUG_TRCSTS_MASK 3
+#define EQOS_MTL_TXQ0_QUANTUM_WEIGHT 0xd18
+#define EQOS_MTL_RXQ0_OPERATION_MODE 0xd30 +#define EQOS_MTL_RXQ0_OPERATION_MODE_RQS_SHIFT 20 +#define EQOS_MTL_RXQ0_OPERATION_MODE_RQS_MASK 0x3ff +#define EQOS_MTL_RXQ0_OPERATION_MODE_RFD_SHIFT 14 +#define EQOS_MTL_RXQ0_OPERATION_MODE_RFD_MASK 0x3f +#define EQOS_MTL_RXQ0_OPERATION_MODE_RFA_SHIFT 8 +#define EQOS_MTL_RXQ0_OPERATION_MODE_RFA_MASK 0x3f +#define EQOS_MTL_RXQ0_OPERATION_MODE_EHFC BIT(7) +#define EQOS_MTL_RXQ0_OPERATION_MODE_RSF BIT(5)
+#define EQOS_MTL_RXQ0_DEBUG 0xd38 +#define EQOS_MTL_RXQ0_DEBUG_PRXQ_SHIFT 16 +#define EQOS_MTL_RXQ0_DEBUG_PRXQ_MASK 0x7fff +#define EQOS_MTL_RXQ0_DEBUG_RXQSTS_SHIFT 4 +#define EQOS_MTL_RXQ0_DEBUG_RXQSTS_MASK 3
+#define EQOS_DMA_MODE 0x1000 +#define EQOS_DMA_MODE_SWR BIT(0)
+#define EQOS_DMA_SYSBUS_MODE 0x1004 +#define EQOS_DMA_SYSBUS_MODE_RD_OSR_LMT_SHIFT 16 +#define EQOS_DMA_SYSBUS_MODE_RD_OSR_LMT_MASK 0xf +#define EQOS_DMA_SYSBUS_MODE_EAME BIT(11) +#define EQOS_DMA_SYSBUS_MODE_BLEN16 BIT(3) +#define EQOS_DMA_SYSBUS_MODE_BLEN8 BIT(2) +#define EQOS_DMA_SYSBUS_MODE_BLEN4 BIT(1)
+#define EQOS_DMA_CH0_CONTROL 0x1100 +#define EQOS_DMA_CH0_CONTROL_PBLX8 BIT(16)
+#define EQOS_DMA_CH0_TX_CONTROL 0x1104 +#define EQOS_DMA_CH0_TX_CONTROL_TXPBL_SHIFT 16 +#define EQOS_DMA_CH0_TX_CONTROL_TXPBL_MASK 0x3f +#define EQOS_DMA_CH0_TX_CONTROL_OSP BIT(4) +#define EQOS_DMA_CH0_TX_CONTROL_ST BIT(0)
+#define EQOS_DMA_CH0_RX_CONTROL 0x1108 +#define EQOS_DMA_CH0_RX_CONTROL_RXPBL_SHIFT 16 +#define EQOS_DMA_CH0_RX_CONTROL_RXPBL_MASK 0x3f +#define EQOS_DMA_CH0_RX_CONTROL_RBSZ_SHIFT 1 +#define EQOS_DMA_CH0_RX_CONTROL_RBSZ_MASK 0x3fff +#define EQOS_DMA_CH0_RX_CONTROL_SR BIT(0)
+#define EQOS_DMA_CH0_TXDESC_LIST_HADDRESS 0x1110
+#define EQOS_DMA_CH0_TXDESC_LIST_ADDRESS 0x1114
+#define EQOS_DMA_CH0_RXDESC_LIST_HADDRESS 0x1118
+#define EQOS_DMA_CH0_RXDESC_LIST_ADDRESS 0x111c
+#define EQOS_DMA_CH0_TXDESC_TAIL_POINTER 0x1120
+#define EQOS_DMA_CH0_RXDESC_TAIL_POINTER 0x1128
+#define EQOS_DMA_CH0_TXDESC_RING_LENGTH 0x112c
+#define EQOS_DMA_CH0_RXDESC_RING_LENGTH 0x1130
+/* Registers located at 0x8000 and later are Tegra-specific */
+#define EQOS_SDMEMCOMPPADCTRL 0x8800 +#define EQOS_SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD BIT(31)
+#define EQOS_AUTO_CAL_CONFIG 0x8804 +#define EQOS_AUTO_CAL_CONFIG_START BIT(31) +#define EQOS_AUTO_CAL_CONFIG_ENABLE BIT(29)
+#define EQOS_AUTO_CAL_STATUS 0x880c +#define EQOS_AUTO_CAL_STATUS_ACTIVE BIT(31)
+/* Descriptors */
+#define EQOS_DESCRIPTOR_WORDS 4 +#define EQOS_DESCRIPTOR_SIZE (EQOS_DESCRIPTOR_WORDS * 4) +/* We assume ARCH_DMA_MINALIGN >= 16; 16 is the EQOS HW minimum */ +#define EQOS_DESCRIPTOR_ALIGN ARCH_DMA_MINALIGN +#define EQOS_DESCRIPTORS_TX 4 +#define EQOS_DESCRIPTORS_RX 4 +#define EQOS_DESCRIPTORS_NUM (EQOS_DESCRIPTORS_TX + EQOS_DESCRIPTORS_RX) +#define EQOS_DESCRIPTORS_SIZE ALIGN(EQOS_DESCRIPTORS_NUM * \
EQOS_DESCRIPTOR_SIZE, ARCH_DMA_MINALIGN)
+#define EQOS_BUFFER_ALIGN ARCH_DMA_MINALIGN +#define EQOS_MAX_PACKET_SIZE ALIGN(1568, ARCH_DMA_MINALIGN) +#define EQOS_RX_BUFFER_SIZE (EQOS_DESCRIPTORS_RX * EQOS_MAX_PACKET_SIZE)
+/*
- Warn if the cache-line size is larger than the descriptor size. In such
- cases the driver will likely fail because the CPU needs to flush the cache
- when requeuing RX buffers, therefore descriptors written by the hardware
- may be discarded.
- This can be fixed by defining CONFIG_SYS_NONCACHED_MEMORY which will cause
- the driver to allocate descriptors from a pool of non-cached memory.
- */
+#if EQOS_DESCRIPTOR_SIZE < ARCH_DMA_MINALIGN +#if !defined(CONFIG_SYS_NONCACHED_MEMORY) && \
!defined(CONFIG_SYS_DCACHE_OFF) && !defined(CONFIG_X86)
Please include an explanation for why x86 is special.
+#warning Cache line size is larger than descriptor size +#endif +#endif
+#define EQOS_DESC3_OWN BIT(31) +#define EQOS_DESC3_FD BIT(29) +#define EQOS_DESC3_LD BIT(28) +#define EQOS_DESC3_BUF1V BIT(24)
+struct eqos_priv {
struct udevice *dev;
fdt_addr_t regs;
struct reset_ctl reset_ctl;
struct gpio_desc phy_reset_gpio;
struct clk clk_master_bus;
struct clk clk_rx;
struct clk clk_ptp_ref;
struct clk clk_tx;
struct clk clk_slave_bus;
struct mii_dev *mii;
struct phy_device *phy;
u32 *descs, *tx_descs, *rx_descs;
int tx_desc_idx, rx_desc_idx;
void *tx_dma_buf;
void *rx_dma_buf;
void *rx_pkt;
bool started;
+};
+/*
- TX and RX descriptors are 16 bytes. This causes problems with the cache
- maintenance on CPUs where the cache-line size exceeds the size of these
- descriptors. What will happen is that when the driver receives a packet
- it will be immediately requeued for the hardware to reuse. The CPU will
- therefore need to flush the cache-line containing the descriptor, which
- will cause all other descriptors in the same cache-line to be flushed
- along with it. If one of those descriptors had been written to by the
- device those changes (and the associated packet) will be lost.
- To work around this, we make use of non-cached memory if available. If
- descriptors are mapped uncached there's no need to manually flush them
- or invalidate them.
- Note that this only applies to descriptors. The packet data buffers do
- not have the same constraints since they are 1536 bytes large, so they
- are unlikely to share cache-lines.
- */
+static u32 *eqos_alloc_descs(unsigned int num) +{ +#ifdef CONFIG_SYS_NONCACHED_MEMORY
return (u32 *)noncached_alloc(EQOS_DESCRIPTORS_SIZE,
EQOS_DESCRIPTOR_ALIGN);
+#else
return memalign(EQOS_DESCRIPTOR_ALIGN, EQOS_DESCRIPTORS_SIZE);
+#endif +}
+static void eqos_free_descs(u32 *descs) +{ +#ifdef CONFIG_SYS_NONCACHED_MEMORY
/* FIXME: noncached_alloc() has no opposite */
+#else
free(descs);
+#endif +}
+static void eqos_inval_desc(u32 *desc) +{ +#ifndef CONFIG_SYS_NONCACHED_MEMORY
unsigned long start = (unsigned long)desc & ~(ARCH_DMA_MINALIGN - 1);
unsigned long end = ALIGN(start + EQOS_DESCRIPTOR_SIZE,
ARCH_DMA_MINALIGN);
invalidate_dcache_range(start, end);
+#endif +}
+static void eqos_flush_desc(u32 *desc) +{ +#ifndef CONFIG_SYS_NONCACHED_MEMORY
flush_cache((unsigned long)desc, EQOS_DESCRIPTOR_SIZE);
+#endif +}
+static void eqos_inval_buffer(void *buf, size_t size) +{
unsigned long start = (unsigned long)buf & ~(ARCH_DMA_MINALIGN - 1);
unsigned long end = ALIGN(start + size, ARCH_DMA_MINALIGN);
invalidate_dcache_range(start, end);
+}
+static void eqos_flush_buffer(void *buf, size_t size) +{
flush_cache((unsigned long)buf, size);
+}
+static int eqos_mdio_wait_idle(struct eqos_priv *eqos) +{
int i;
u32 val;
for (i = 0; i < 1000000; i++) {
if (i)
udelay(1);
val = readl(eqos->regs + EQOS_MAC_MDIO_ADDRESS);
if (!(val & EQOS_MAC_MDIO_ADDRESS_GB))
return 0;
Please use wait_bit here.
}
return -ETIMEDOUT;
+}
+static int eqos_mdio_read(struct mii_dev *bus, int mdio_addr, int mdio_devad,
int mdio_reg)
+{
struct eqos_priv *eqos = bus->priv;
u32 val;
int ret;
debug("%s(dev=%p, addr=%x, reg=%d):\n", __func__, eqos->dev, mdio_addr,
mdio_reg);
ret = eqos_mdio_wait_idle(eqos);
if (ret) {
error("MDIO not idle at entry");
return ret;
}
val = readl(eqos->regs + EQOS_MAC_MDIO_ADDRESS);
val &= EQOS_MAC_MDIO_ADDRESS_SKAP |
EQOS_MAC_MDIO_ADDRESS_C45E;
val |= (mdio_addr << EQOS_MAC_MDIO_ADDRESS_PA_SHIFT) |
(mdio_reg << EQOS_MAC_MDIO_ADDRESS_RDA_SHIFT) |
(EQOS_MAC_MDIO_ADDRESS_CR_20_35 <<
EQOS_MAC_MDIO_ADDRESS_CR_SHIFT) |
(EQOS_MAC_MDIO_ADDRESS_GOC_READ <<
EQOS_MAC_MDIO_ADDRESS_GOC_SHIFT) |
EQOS_MAC_MDIO_ADDRESS_GB;
writel(val, eqos->regs + EQOS_MAC_MDIO_ADDRESS);
udelay(10);
ret = eqos_mdio_wait_idle(eqos);
if (ret) {
error("MDIO read didn't complete");
return ret;
}
val = readl(eqos->regs + EQOS_MAC_MDIO_DATA);
val &= EQOS_MAC_MDIO_DATA_GD_MASK;
debug("%s: val=%x\n", __func__, val);
return val;
+}
+static int eqos_mdio_write(struct mii_dev *bus, int mdio_addr, int mdio_devad,
int mdio_reg, u16 mdio_val)
+{
struct eqos_priv *eqos = bus->priv;
u32 val;
int ret;
debug("%s(dev=%p, addr=%x, reg=%d, val=%x):\n", __func__, eqos->dev,
mdio_addr, mdio_reg, mdio_val);
ret = eqos_mdio_wait_idle(eqos);
if (ret) {
error("MDIO not idle at entry");
return ret;
}
writel(mdio_val, eqos->regs + EQOS_MAC_MDIO_DATA);
val = readl(eqos->regs + EQOS_MAC_MDIO_ADDRESS);
val &= EQOS_MAC_MDIO_ADDRESS_SKAP |
EQOS_MAC_MDIO_ADDRESS_C45E;
val |= (mdio_addr << EQOS_MAC_MDIO_ADDRESS_PA_SHIFT) |
(mdio_reg << EQOS_MAC_MDIO_ADDRESS_RDA_SHIFT) |
(EQOS_MAC_MDIO_ADDRESS_CR_20_35 <<
EQOS_MAC_MDIO_ADDRESS_CR_SHIFT) |
(EQOS_MAC_MDIO_ADDRESS_GOC_WRITE <<
EQOS_MAC_MDIO_ADDRESS_GOC_SHIFT) |
EQOS_MAC_MDIO_ADDRESS_GB;
writel(val, eqos->regs + EQOS_MAC_MDIO_ADDRESS);
udelay(10);
ret = eqos_mdio_wait_idle(eqos);
if (ret) {
error("MDIO read didn't complete");
return ret;
}
return 0;
+}
+static int eqos_start_clks_tegra(struct udevice *dev)
Why is this not in a board file and a separate patch?
+{
struct eqos_priv *eqos = dev_get_priv(dev);
int ret;
debug("%s(dev=%p):\n", __func__, dev);
ret = clk_enable(&eqos->clk_slave_bus);
if (ret < 0) {
error("clk_enable(clk_slave_bus) failed: %d", ret);
goto err;
}
ret = clk_enable(&eqos->clk_master_bus);
if (ret < 0) {
error("clk_enable(clk_master_bus) failed: %d", ret);
goto err_disable_clk_slave_bus;
}
ret = clk_enable(&eqos->clk_rx);
if (ret < 0) {
error("clk_enable(clk_rx) failed: %d", ret);
goto err_disable_clk_master_bus;
}
ret = clk_enable(&eqos->clk_ptp_ref);
if (ret < 0) {
error("clk_enable(clk_ptp_ref) failed: %d", ret);
goto err_disable_clk_rx;
}
ret = clk_set_rate(&eqos->clk_ptp_ref, 125 * 1000 * 1000);
if (ret < 0) {
error("clk_set_rate(clk_ptp_ref) failed: %d", ret);
goto err_disable_clk_ptp_ref;
}
ret = clk_enable(&eqos->clk_tx);
if (ret < 0) {
error("clk_enable(clk_tx) failed: %d", ret);
goto err_disable_clk_ptp_ref;
}
debug("%s: OK\n", __func__);
return 0;
+err_disable_clk_ptp_ref:
clk_disable(&eqos->clk_ptp_ref);
+err_disable_clk_rx:
clk_disable(&eqos->clk_rx);
+err_disable_clk_master_bus:
clk_disable(&eqos->clk_master_bus);
+err_disable_clk_slave_bus:
clk_disable(&eqos->clk_slave_bus);
+err:
debug("%s: FAILED: %d\n", __func__, ret);
return ret;
+}
+void eqos_stop_clks_tegra(struct udevice *dev)
Ditto for all *_tegra()
+{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
clk_disable(&eqos->clk_tx);
clk_disable(&eqos->clk_ptp_ref);
clk_disable(&eqos->clk_rx);
clk_disable(&eqos->clk_master_bus);
clk_disable(&eqos->clk_slave_bus);
debug("%s: OK\n", __func__);
+}
+static int eqos_start_resets_tegra(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
int ret;
debug("%s(dev=%p):\n", __func__, dev);
ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 1);
if (ret < 0) {
error("dm_gpio_set_value(phy_reset, assert) failed: %d", ret);
return ret;
}
udelay(2);
ret = dm_gpio_set_value(&eqos->phy_reset_gpio, 0);
if (ret < 0) {
error("dm_gpio_set_value(phy_reset, deassert) failed: %d", ret);
return ret;
}
ret = reset_assert(&eqos->reset_ctl);
if (ret < 0) {
error("reset_assert() failed: %d", ret);
return ret;
}
udelay(2);
ret = reset_deassert(&eqos->reset_ctl);
if (ret < 0) {
error("reset_deassert() failed: %d", ret);
return ret;
}
debug("%s: OK\n", __func__);
return 0;
+}
+static int eqos_stop_resets_tegra(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
reset_assert(&eqos->reset_ctl);
dm_gpio_set_value(&eqos->phy_reset_gpio, 1);
return 0;
+}
+static int eqos_calibrate_pads_tegra(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
u32 val;
int i, ret;
debug("%s(dev=%p):\n", __func__, dev);
setbits_le32(eqos->regs + EQOS_SDMEMCOMPPADCTRL,
EQOS_SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD);
udelay(1);
setbits_le32(eqos->regs + EQOS_AUTO_CAL_CONFIG,
EQOS_AUTO_CAL_CONFIG_START | EQOS_AUTO_CAL_CONFIG_ENABLE);
for (i = 0; i <= 10; i++) {
if (i)
udelay(1);
val = readl(eqos->regs + EQOS_AUTO_CAL_STATUS);
if (val & EQOS_AUTO_CAL_STATUS_ACTIVE)
break;
}
if (!(val & EQOS_AUTO_CAL_STATUS_ACTIVE)) {
error("calibrate didn't start");
ret = -ETIMEDOUT;
goto failed;
}
for (i = 0; i <= 10; i++) {
if (i)
udelay(20);
val = readl(eqos->regs + EQOS_AUTO_CAL_STATUS);
if (!(val & EQOS_AUTO_CAL_STATUS_ACTIVE))
break;
}
if (val & EQOS_AUTO_CAL_STATUS_ACTIVE) {
error("calibrate didn't finish");
ret = -ETIMEDOUT;
goto failed;
}
ret = 0;
+failed:
clrbits_le32(eqos->regs + EQOS_SDMEMCOMPPADCTRL,
EQOS_SDMEMCOMPPADCTRL_PAD_E_INPUT_OR_E_PWRD);
debug("%s: returns %d\n", __func__, ret);
return ret;
+}
+static int eqos_disable_calibration_tegra(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
clrbits_le32(eqos->regs + EQOS_AUTO_CAL_CONFIG,
EQOS_AUTO_CAL_CONFIG_ENABLE);
return 0;
+}
+static ulong eqos_get_tick_clk_rate_tegra(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
return clk_get_rate(&eqos->clk_slave_bus);
+}
+static int eqos_set_full_duplex(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
setbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION,
EQOS_MAC_CONFIGURATION_DM);
return 0;
+}
+static int eqos_set_half_duplex(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
clrbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION,
EQOS_MAC_CONFIGURATION_DM);
/* WAR: Flush TX queue when switching to half-duplex */
setbits_le32(eqos->regs + EQOS_MTL_TXQ0_OPERATION_MODE,
EQOS_MTL_TXQ0_OPERATION_MODE_FTQ);
return 0;
+}
+static int eqos_set_gmii_speed(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
clrbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION,
EQOS_MAC_CONFIGURATION_PS | EQOS_MAC_CONFIGURATION_FES);
return 0;
+}
+static int eqos_set_mii_speed_100(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
setbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION,
EQOS_MAC_CONFIGURATION_PS | EQOS_MAC_CONFIGURATION_FES);
return 0;
+}
+static int eqos_set_mii_speed_10(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
clrsetbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION,
EQOS_MAC_CONFIGURATION_FES, EQOS_MAC_CONFIGURATION_PS);
return 0;
+}
+static int eqos_set_tx_clk_speed_tegra(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
ulong rate;
int ret;
debug("%s(dev=%p):\n", __func__, dev);
switch (eqos->phy->speed) {
case SPEED_1000:
rate = 125 * 1000 * 1000;
break;
case SPEED_100:
rate = 25 * 1000 * 1000;
break;
case SPEED_10:
rate = 2.5 * 1000 * 1000;
break;
default:
error("invalid speed %d", eqos->phy->speed);
return -EINVAL;
}
ret = clk_set_rate(&eqos->clk_tx, rate);
if (ret < 0) {
error("clk_set_rate(tx_clk, %lu) failed: %d", rate, ret);
return ret;
}
return 0;
+}
+static int eqos_adjust_link(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
int ret;
bool en_calibration;
debug("%s(dev=%p):\n", __func__, dev);
if (eqos->phy->duplex)
ret = eqos_set_full_duplex(dev);
else
ret = eqos_set_half_duplex(dev);
if (ret < 0) {
error("eqos_set_*_duplex() failed: %d", ret);
return ret;
}
switch (eqos->phy->speed) {
case SPEED_1000:
en_calibration = true;
ret = eqos_set_gmii_speed(dev);
break;
case SPEED_100:
en_calibration = true;
ret = eqos_set_mii_speed_100(dev);
break;
case SPEED_10:
en_calibration = false;
ret = eqos_set_mii_speed_10(dev);
break;
default:
error("invalid speed %d", eqos->phy->speed);
return -EINVAL;
}
if (ret < 0) {
error("eqos_set_*mii_speed*() failed: %d", ret);
return ret;
}
if (en_calibration) {
ret = eqos_calibrate_pads_tegra(dev);
if (ret < 0) {
error("eqos_calibrate_pads_tegra() failed: %d", ret);
return ret;
}
} else {
ret = eqos_disable_calibration_tegra(dev);
if (ret < 0) {
error("eqos_disable_calibration_tegra() failed: %d",
ret);
return ret;
}
}
ret = eqos_set_tx_clk_speed_tegra(dev);
if (ret < 0) {
error("eqos_set_tx_clk_speed_tegra() failed: %d", ret);
return ret;
}
return 0;
+}
+static int eqos_start(struct udevice *dev) +{
struct eth_pdata *plat = dev_get_platdata(dev);
struct eqos_priv *eqos = dev_get_priv(dev);
int ret, i;
ulong rate;
u32 val, tx_fifo_sz, rx_fifo_sz, tqs, rqs, pbl;
ulong last_rx_desc;
debug("%s(dev=%p):\n", __func__, dev);
eqos->tx_desc_idx = 0;
eqos->rx_desc_idx = 0;
ret = eqos_start_clks_tegra(dev);
if (ret < 0) {
error("eqos_start_clks_tegra() failed: %d", ret);
goto err;
}
ret = eqos_start_resets_tegra(dev);
if (ret < 0) {
error("eqos_start_resets_tegra() failed: %d", ret);
goto err_stop_clks;
}
udelay(10);
for (i = 0; i < 1000000; i++) {
if (i)
udelay(1);
val = readl(eqos->regs + EQOS_DMA_MODE);
if (!(val & EQOS_DMA_MODE_SWR))
break;
}
Use wait_bit() here.
if (val & EQOS_DMA_MODE_SWR) {
error("EQOS_DMA_MODE_SWR stuck (0x%x)", val);
ret = -ETIMEDOUT;
goto err_stop_resets;
}
ret = eqos_calibrate_pads_tegra(dev);
if (ret < 0) {
error("eqos_calibrate_pads_tegra() failed: %d", ret);
goto err_stop_resets;
}
rate = eqos_get_tick_clk_rate_tegra(dev);
val = (rate / 1000000) - 1;
writel(val, eqos->regs + EQOS_MAC_1US_TIC_COUNTER);
eqos->phy = phy_connect(eqos->mii, 0, dev, 0);
if (!eqos->phy) {
error("phy_connect() failed");
goto err_stop_resets;
}
ret = phy_config(eqos->phy);
if (ret < 0) {
error("phy_config() failed: %d", ret);
goto err_shutdown_phy;
}
ret = phy_startup(eqos->phy);
if (ret < 0) {
error("phy_startup() failed: %d", ret);
goto err_shutdown_phy;
}
if (!eqos->phy->link) {
error("No link");
goto err_shutdown_phy;
}
ret = eqos_adjust_link(dev);
if (ret < 0) {
error("eqos_adjust_link() failed: %d", ret);
goto err_shutdown_phy;
}
/* Configure MTL */
/* Enable Store and Forward mode for TX */
/* Program Tx operating mode */
setbits_le32(eqos->regs + EQOS_MTL_TXQ0_OPERATION_MODE,
EQOS_MTL_TXQ0_OPERATION_MODE_TSF |
(EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_ENABLED <<
EQOS_MTL_TXQ0_OPERATION_MODE_TXQEN_SHIFT));
/* Transmit Queue weight */
writel(0x10, eqos->regs + EQOS_MTL_TXQ0_QUANTUM_WEIGHT);
/* Enable Store and Forward mode for RX, since no jumbo frame */
setbits_le32(eqos->regs + EQOS_MTL_RXQ0_OPERATION_MODE,
EQOS_MTL_RXQ0_OPERATION_MODE_RSF);
/* Transmit/Receive queue fifo size; use all RAM for 1 queue */
val = readl(eqos->regs + EQOS_MAC_HW_FEATURE1);
tx_fifo_sz = (val >> EQOS_MAC_HW_FEATURE1_TXFIFOSIZE_SHIFT) &
EQOS_MAC_HW_FEATURE1_TXFIFOSIZE_MASK;
rx_fifo_sz = (val >> EQOS_MAC_HW_FEATURE1_RXFIFOSIZE_SHIFT) &
EQOS_MAC_HW_FEATURE1_RXFIFOSIZE_MASK;
/*
* r/tx_fifo_sz is encoded as log2(n / 128). Undo that by shifting.
* r/tqs is encoded as (n / 256) - 1.
*/
tqs = (128 << tx_fifo_sz) / 256 - 1;
rqs = (128 << rx_fifo_sz) / 256 - 1;
clrsetbits_le32(eqos->regs + EQOS_MTL_TXQ0_OPERATION_MODE,
EQOS_MTL_TXQ0_OPERATION_MODE_TQS_MASK <<
EQOS_MTL_TXQ0_OPERATION_MODE_TQS_SHIFT,
tqs << EQOS_MTL_TXQ0_OPERATION_MODE_TQS_SHIFT);
clrsetbits_le32(eqos->regs + EQOS_MTL_RXQ0_OPERATION_MODE,
EQOS_MTL_RXQ0_OPERATION_MODE_RQS_MASK <<
EQOS_MTL_RXQ0_OPERATION_MODE_RQS_SHIFT,
rqs << EQOS_MTL_RXQ0_OPERATION_MODE_RQS_SHIFT);
/* Flow control used only if each channel gets 4KB or more FIFO */
if (rqs >= ((4096 / 256) - 1)) {
u32 rfd, rfa;
setbits_le32(eqos->regs + EQOS_MTL_RXQ0_OPERATION_MODE,
EQOS_MTL_RXQ0_OPERATION_MODE_EHFC);
/*
* Set Threshold for Activating Flow Contol space for min 2
* frames ie, (1500 * 1) = 1500 bytes.
*
* Set Threshold for Deactivating Flow Contol for space of
* min 1 frame (frame size 1500bytes) in receive fifo
*/
if (rqs == ((4096 / 256) - 1)) {
/*
* This violates the above formula because of FIFO size
* limit therefore overflow may occur inspite of this.
*/
rfd = 0x3; /* Full-3K */
rfa = 0x1; /* Full-1.5K */
} else if (rqs == ((8192 / 256) - 1)) {
rfd = 0x6; /* Full-4K */
rfa = 0xa; /* Full-6K */
} else if (rqs == ((16384 / 256) - 1)) {
rfd = 0x6; /* Full-4K */
rfa = 0x12; /* Full-10K */
} else {
rfd = 0x6; /* Full-4K */
rfa = 0x1E; /* Full-16K */
}
clrsetbits_le32(eqos->regs + EQOS_MTL_RXQ0_OPERATION_MODE,
(EQOS_MTL_RXQ0_OPERATION_MODE_RFD_MASK <<
EQOS_MTL_RXQ0_OPERATION_MODE_RFD_SHIFT) |
(EQOS_MTL_RXQ0_OPERATION_MODE_RFA_MASK <<
EQOS_MTL_RXQ0_OPERATION_MODE_RFA_SHIFT),
(rfd <<
EQOS_MTL_RXQ0_OPERATION_MODE_RFD_SHIFT) |
(rfa <<
EQOS_MTL_RXQ0_OPERATION_MODE_RFA_SHIFT));
}
/* Configure MAC */
clrsetbits_le32(eqos->regs + EQOS_MAC_RXQ_CTRL0,
EQOS_MAC_RXQ_CTRL0_RXQ0EN_MASK <<
EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT,
EQOS_MAC_RXQ_CTRL0_RXQ0EN_ENABLED_DCB <<
EQOS_MAC_RXQ_CTRL0_RXQ0EN_SHIFT);
/* Set TX flow control parameters */
/* Set Pause Time */
setbits_le32(eqos->regs + EQOS_MAC_Q0_TX_FLOW_CTRL,
0xffff << EQOS_MAC_Q0_TX_FLOW_CTRL_PT_SHIFT);
/* Assign priority for TX flow control */
clrbits_le32(eqos->regs + EQOS_MAC_TXQ_PRTY_MAP0,
EQOS_MAC_TXQ_PRTY_MAP0_PSTQ0_MASK <<
EQOS_MAC_TXQ_PRTY_MAP0_PSTQ0_SHIFT);
/* Assign priority for RX flow control */
clrbits_le32(eqos->regs + EQOS_MAC_RXQ_CTRL2,
EQOS_MAC_RXQ_CTRL2_PSRQ0_MASK <<
EQOS_MAC_RXQ_CTRL2_PSRQ0_SHIFT);
/* Enable flow control */
setbits_le32(eqos->regs + EQOS_MAC_Q0_TX_FLOW_CTRL,
EQOS_MAC_Q0_TX_FLOW_CTRL_TFE);
setbits_le32(eqos->regs + EQOS_MAC_RX_FLOW_CTRL,
EQOS_MAC_RX_FLOW_CTRL_RFE);
clrsetbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION,
EQOS_MAC_CONFIGURATION_GPSLCE |
EQOS_MAC_CONFIGURATION_WD |
EQOS_MAC_CONFIGURATION_JD |
EQOS_MAC_CONFIGURATION_JE,
EQOS_MAC_CONFIGURATION_CST |
EQOS_MAC_CONFIGURATION_ACS);
/* Update the MAC address */
val = (plat->enetaddr[5] << 8) |
(plat->enetaddr[4]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH);
val = (plat->enetaddr[3] << 24) |
(plat->enetaddr[2] << 16) |
(plat->enetaddr[1] << 8) |
(plat->enetaddr[0]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW);
This should be implemented in write_hwaddr() op.
/* Configure DMA */
/* Enable OSP mode */
setbits_le32(eqos->regs + EQOS_DMA_CH0_TX_CONTROL,
EQOS_DMA_CH0_TX_CONTROL_OSP);
/* RX buffer size. Must be a multiple of bus width */
clrsetbits_le32(eqos->regs + EQOS_DMA_CH0_RX_CONTROL,
EQOS_DMA_CH0_RX_CONTROL_RBSZ_MASK <<
EQOS_DMA_CH0_RX_CONTROL_RBSZ_SHIFT,
EQOS_MAX_PACKET_SIZE <<
EQOS_DMA_CH0_RX_CONTROL_RBSZ_SHIFT);
setbits_le32(eqos->regs + EQOS_DMA_CH0_CONTROL,
EQOS_DMA_CH0_CONTROL_PBLX8);
/*
* Burst length must be < 1/2 FIFO size.
* FIFO size in tqs is encoded as (n / 256) - 1.
* Each burst is n * 8 (PBLX8) * 16 (AXI width) == 128 bytes.
* Half of n * 256 is n * 128, so pbl == tqs, modulo the -1.
*/
pbl = tqs + 1;
if (pbl > 32)
pbl = 32;
clrsetbits_le32(eqos->regs + EQOS_DMA_CH0_TX_CONTROL,
EQOS_DMA_CH0_TX_CONTROL_TXPBL_MASK <<
EQOS_DMA_CH0_TX_CONTROL_TXPBL_SHIFT,
pbl << EQOS_DMA_CH0_TX_CONTROL_TXPBL_SHIFT);
clrsetbits_le32(eqos->regs + EQOS_DMA_CH0_RX_CONTROL,
EQOS_DMA_CH0_RX_CONTROL_RXPBL_MASK <<
EQOS_DMA_CH0_RX_CONTROL_RXPBL_SHIFT,
8 << EQOS_DMA_CH0_RX_CONTROL_RXPBL_SHIFT);
/* DMA performance configuration */
val = (2 << EQOS_DMA_SYSBUS_MODE_RD_OSR_LMT_SHIFT) |
EQOS_DMA_SYSBUS_MODE_EAME | EQOS_DMA_SYSBUS_MODE_BLEN16 |
EQOS_DMA_SYSBUS_MODE_BLEN8 | EQOS_DMA_SYSBUS_MODE_BLEN4;
writel(val, eqos->regs + EQOS_DMA_SYSBUS_MODE);
/* Set up descriptors */
memset(eqos->descs, 0, EQOS_DESCRIPTORS_SIZE);
for (i = 0; i < EQOS_DESCRIPTORS_RX; i++) {
u32 *rx_desc = &(eqos->rx_descs[i * EQOS_DESCRIPTOR_WORDS]);
rx_desc[0] = (u32)(ulong)(eqos->rx_dma_buf +
(i * EQOS_MAX_PACKET_SIZE));
rx_desc[3] |= EQOS_DESC3_OWN | EQOS_DESC3_BUF1V;
}
flush_cache((unsigned long)eqos->descs, EQOS_DESCRIPTORS_SIZE);
writel(0, eqos->regs + EQOS_DMA_CH0_TXDESC_LIST_HADDRESS);
writel((ulong)eqos->tx_descs,
eqos->regs + EQOS_DMA_CH0_TXDESC_LIST_ADDRESS);
writel(EQOS_DESCRIPTORS_TX - 1,
eqos->regs + EQOS_DMA_CH0_TXDESC_RING_LENGTH);
writel(0, eqos->regs + EQOS_DMA_CH0_RXDESC_LIST_HADDRESS);
writel((ulong)eqos->rx_descs,
eqos->regs + EQOS_DMA_CH0_RXDESC_LIST_ADDRESS);
writel(EQOS_DESCRIPTORS_RX - 1,
eqos->regs + EQOS_DMA_CH0_RXDESC_RING_LENGTH);
/* Enable everything */
setbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION,
EQOS_MAC_CONFIGURATION_TE | EQOS_MAC_CONFIGURATION_RE);
setbits_le32(eqos->regs + EQOS_DMA_CH0_TX_CONTROL,
EQOS_DMA_CH0_TX_CONTROL_ST);
setbits_le32(eqos->regs + EQOS_DMA_CH0_RX_CONTROL,
EQOS_DMA_CH0_RX_CONTROL_SR);
/* TX tail pointer not written until we need to TX a packet */
/*
* Point RX tail pointer at last descriptor. Ideally, we'd point at the
* first descriptor, implying all descriptors were available. However,
* that's not distinguishable from none of the descriptors being
* available.
*/
last_rx_desc = (ulong)&(eqos->rx_descs[(EQOS_DESCRIPTORS_RX - 1) *
EQOS_DESCRIPTOR_WORDS]);
writel(last_rx_desc, eqos->regs + EQOS_DMA_CH0_RXDESC_TAIL_POINTER);
eqos->started = true;
debug("%s: OK\n", __func__);
return 0;
+err_shutdown_phy:
phy_shutdown(eqos->phy);
eqos->phy = NULL;
+err_stop_resets:
eqos_stop_resets_tegra(dev);
+err_stop_clks:
eqos_stop_clks_tegra(dev);
+err:
error("FAILED: %d", ret);
return ret;
+}
+void eqos_stop(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
int i;
debug("%s(dev=%p):\n", __func__, dev);
if (!eqos->started)
return;
eqos->started = false;
/* Disable TX DMA */
clrbits_le32(eqos->regs + EQOS_DMA_CH0_TX_CONTROL,
EQOS_DMA_CH0_TX_CONTROL_ST);
/* Wait for TX all packets to drain out of MTL */
for (i = 0; i < 1000000; i++) {
u32 val = readl(eqos->regs + EQOS_MTL_TXQ0_DEBUG);
u32 trcsts = (val >> EQOS_MTL_TXQ0_DEBUG_TRCSTS_SHIFT) &
EQOS_MTL_TXQ0_DEBUG_TRCSTS_MASK;
u32 txqsts = val & EQOS_MTL_TXQ0_DEBUG_TXQSTS;
if ((trcsts != 1) && (!txqsts))
break;
}
/* Turn off MAC TX and RX */
clrbits_le32(eqos->regs + EQOS_MAC_CONFIGURATION,
EQOS_MAC_CONFIGURATION_TE | EQOS_MAC_CONFIGURATION_RE);
/* Wait for all RX packets to drain out of MTL */
for (i = 0; i < 1000000; i++) {
u32 val = readl(eqos->regs + EQOS_MTL_RXQ0_DEBUG);
u32 prxq = (val >> EQOS_MTL_RXQ0_DEBUG_PRXQ_SHIFT) &
EQOS_MTL_RXQ0_DEBUG_PRXQ_MASK;
u32 rxqsts = (val >> EQOS_MTL_RXQ0_DEBUG_RXQSTS_SHIFT) &
EQOS_MTL_RXQ0_DEBUG_RXQSTS_MASK;
if ((!prxq) && (!rxqsts))
break;
}
/* Turn off RX DMA */
clrbits_le32(eqos->regs + EQOS_DMA_CH0_RX_CONTROL,
EQOS_DMA_CH0_RX_CONTROL_SR);
if (eqos->phy) {
phy_shutdown(eqos->phy);
eqos->phy = NULL;
}
eqos_stop_resets_tegra(dev);
eqos_stop_clks_tegra(dev);
debug("%s: OK\n", __func__);
+}
+int eqos_send(struct udevice *dev, void *packet, int length) +{
struct eqos_priv *eqos = dev_get_priv(dev);
u32 *tx_desc;
Please make this a struct.
int i;
debug("%s(dev=%p, packet=%p, length=%d):\n", __func__, dev, packet,
length);
memcpy(eqos->tx_dma_buf, packet, length);
eqos_flush_buffer(eqos->tx_dma_buf, length);
tx_desc = &(eqos->tx_descs[eqos->tx_desc_idx * EQOS_DESCRIPTOR_WORDS]);
eqos->tx_desc_idx++;
eqos->tx_desc_idx %= EQOS_DESCRIPTORS_TX;
tx_desc[0] = (ulong)eqos->tx_dma_buf;
tx_desc[1] = 0;
tx_desc[2] = length;
/*
* Make sure that if HW sees the _OWN write below, it will see all the
* writes to the rest of the descriptor too.
*/
mb();
tx_desc[3] = EQOS_DESC3_OWN | EQOS_DESC3_FD | EQOS_DESC3_LD | length;
eqos_flush_desc(tx_desc);
writel((ulong)(tx_desc + 4),
eqos->regs + EQOS_DMA_CH0_TXDESC_TAIL_POINTER);
for (i = 0; i < 1000000; i++) {
eqos_inval_desc(tx_desc);
if (!(readl(&tx_desc[3]) & EQOS_DESC3_OWN))
return 0;
Use wait_bit() here.
udelay(1);
}
debug("%s: TX timeout\n", __func__);
return -ETIMEDOUT;
+}
+int eqos_recv(struct udevice *dev, int flags, uchar **packetp) +{
struct eqos_priv *eqos = dev_get_priv(dev);
u32 *rx_desc;
Please make this a struct.
int length;
debug("%s(dev=%p, flags=%x):\n", __func__, dev, flags);
rx_desc = &(eqos->rx_descs[eqos->rx_desc_idx * EQOS_DESCRIPTOR_WORDS]);
if (rx_desc[3] & EQOS_DESC3_OWN) {
debug("%s: RX packet not available\n", __func__);
return -EAGAIN;
}
*packetp = eqos->rx_dma_buf +
(eqos->rx_desc_idx * EQOS_MAX_PACKET_SIZE);
length = rx_desc[3] & 0x7fff;
debug("%s: *packetp=%p, length=%d\n", __func__, *packetp, length);
eqos_inval_buffer(*packetp, length);
return length;
+}
+int eqos_free_pkt(struct udevice *dev, uchar *packet, int length) +{
struct eqos_priv *eqos = dev_get_priv(dev);
uchar *packet_expected;
u32 *rx_desc;
Please make this a struct.
debug("%s(packet=%p, length=%d)\n", __func__, packet, length);
packet_expected = eqos->rx_dma_buf +
(eqos->rx_desc_idx * EQOS_MAX_PACKET_SIZE);
if (packet != packet_expected) {
debug("%s: Unexpected packet (expected %p)\n", __func__,
packet_expected);
return -EINVAL;
}
rx_desc = &(eqos->rx_descs[eqos->rx_desc_idx * EQOS_DESCRIPTOR_WORDS]);
rx_desc[0] = (u32)(ulong)packet;
rx_desc[1] = 0;
rx_desc[2] = 0;
/*
* Make sure that if HW sees the _OWN write below, it will see all the
* writes to the rest of the descriptor too.
*/
mb();
rx_desc[3] |= EQOS_DESC3_OWN | EQOS_DESC3_BUF1V;
eqos_flush_desc(rx_desc);
writel((ulong)rx_desc, eqos->regs + EQOS_DMA_CH0_RXDESC_TAIL_POINTER);
eqos->rx_desc_idx++;
eqos->rx_desc_idx %= EQOS_DESCRIPTORS_RX;
return 0;
+}
+static int eqos_probe_resources_core(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
int ret;
debug("%s(dev=%p):\n", __func__, dev);
eqos->descs = eqos_alloc_descs(EQOS_DESCRIPTORS_TX +
EQOS_DESCRIPTORS_RX);
if (!eqos->descs) {
debug("%s: eqos_alloc_descs() failed\n", __func__);
ret = -ENOMEM;
goto err;
}
eqos->tx_descs = eqos->descs;
eqos->rx_descs = eqos->tx_descs +
(EQOS_DESCRIPTORS_TX * EQOS_DESCRIPTOR_WORDS);
debug("%s: tx_descs=%p, rx_descs=%p\n", __func__, eqos->tx_descs,
eqos->rx_descs);
eqos->tx_dma_buf = memalign(EQOS_BUFFER_ALIGN, EQOS_MAX_PACKET_SIZE);
if (!eqos->tx_dma_buf) {
debug("%s: memalign(tx_dma_buf) failed\n", __func__);
ret = -ENOMEM;
goto err_free_descs;
}
debug("%s: rx_dma_buf=%p\n", __func__, eqos->rx_dma_buf);
eqos->rx_dma_buf = memalign(EQOS_BUFFER_ALIGN, EQOS_RX_BUFFER_SIZE);
if (!eqos->rx_dma_buf) {
debug("%s: memalign(rx_dma_buf) failed\n", __func__);
ret = -ENOMEM;
goto err_free_tx_dma_buf;
}
debug("%s: tx_dma_buf=%p\n", __func__, eqos->tx_dma_buf);
eqos->rx_pkt = malloc(EQOS_MAX_PACKET_SIZE);
if (!eqos->rx_pkt) {
debug("%s: malloc(rx_pkt) failed\n", __func__);
ret = -ENOMEM;
goto err_free_rx_dma_buf;
}
debug("%s: rx_pkt=%p\n", __func__, eqos->rx_pkt);
debug("%s: OK\n", __func__);
return 0;
+err_free_rx_dma_buf:
free(eqos->rx_dma_buf);
+err_free_tx_dma_buf:
free(eqos->tx_dma_buf);
+err_free_descs:
eqos_free_descs(eqos->descs);
+err:
debug("%s: returns %d\n", __func__, ret);
return ret;
+}
+static int eqos_remove_resources_core(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
free(eqos->rx_pkt);
free(eqos->rx_dma_buf);
free(eqos->tx_dma_buf);
eqos_free_descs(eqos->descs);
debug("%s: OK\n", __func__);
return 0;
+}
+static int eqos_probe_resources_tegra(struct udevice *dev)
Sure would be great to not have this in here.
+{
struct eqos_priv *eqos = dev_get_priv(dev);
int ret;
debug("%s(dev=%p):\n", __func__, dev);
ret = reset_get_by_name(dev, "eqos", &eqos->reset_ctl);
if (ret) {
error("reset_get_by_name(rst) failed: %d", ret);
return ret;
}
ret = gpio_request_by_name(dev, "phy-reset-gpios", 0,
&eqos->phy_reset_gpio,
GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
if (ret) {
error("gpio_request_by_name(phy reset) failed: %d", ret);
goto err_free_reset_eqos;
}
ret = clk_get_by_name(dev, "slave_bus", &eqos->clk_slave_bus);
if (ret) {
error("clk_get_by_name(slave_bus) failed: %d", ret);
goto err_free_gpio_phy_reset;
}
ret = clk_get_by_name(dev, "master_bus", &eqos->clk_master_bus);
if (ret) {
error("clk_get_by_name(master_bus) failed: %d", ret);
goto err_free_clk_slave_bus;
}
ret = clk_get_by_name(dev, "rx", &eqos->clk_rx);
if (ret) {
error("clk_get_by_name(rx) failed: %d", ret);
goto err_free_clk_master_bus;
}
ret = clk_get_by_name(dev, "ptp_ref", &eqos->clk_ptp_ref);
if (ret) {
error("clk_get_by_name(ptp_ref) failed: %d", ret);
goto err_free_clk_rx;
return ret;
}
ret = clk_get_by_name(dev, "tx", &eqos->clk_tx);
if (ret) {
error("clk_get_by_name(tx) failed: %d", ret);
goto err_free_clk_ptp_ref;
}
debug("%s: OK\n", __func__);
return 0;
+err_free_clk_ptp_ref:
clk_free(&eqos->clk_ptp_ref);
+err_free_clk_rx:
clk_free(&eqos->clk_rx);
+err_free_clk_master_bus:
clk_free(&eqos->clk_master_bus);
+err_free_clk_slave_bus:
clk_free(&eqos->clk_slave_bus);
+err_free_gpio_phy_reset:
dm_gpio_free(dev, &eqos->phy_reset_gpio);
+err_free_reset_eqos:
reset_free(&eqos->reset_ctl);
debug("%s: returns %d\n", __func__, ret);
return ret;
+}
+static int eqos_remove_resources_tegra(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
clk_free(&eqos->clk_tx);
clk_free(&eqos->clk_ptp_ref);
clk_free(&eqos->clk_rx);
clk_free(&eqos->clk_slave_bus);
clk_free(&eqos->clk_master_bus);
dm_gpio_free(dev, &eqos->phy_reset_gpio);
reset_free(&eqos->reset_ctl);
debug("%s: OK\n", __func__);
return 0;
+}
+static int eqos_probe(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
int ret;
debug("%s(dev=%p):\n", __func__, dev);
eqos->dev = dev;
eqos->regs = dev_get_addr(dev);
if (eqos->regs == FDT_ADDR_T_NONE) {
error("dev_get_addr() failed");
return -ENODEV;
}
ret = eqos_probe_resources_core(dev);
if (ret < 0) {
error("eqos_probe_resources_core() failed: %d", ret);
return ret;
}
ret = eqos_probe_resources_tegra(dev);
if (ret < 0) {
error("eqos_probe_resources_tegra() failed: %d", ret);
goto err_remove_resources_core;
}
eqos->mii = mdio_alloc();
if (!eqos->mii) {
error("mdio_alloc() failed");
goto err_remove_resources_tegra;
}
eqos->mii->read = eqos_mdio_read;
eqos->mii->write = eqos_mdio_write;
eqos->mii->priv = eqos;
strcpy(eqos->mii->name, dev->name);
ret = mdio_register(eqos->mii);
if (ret < 0) {
error("mdio_register() failed: %d", ret);
goto err_free_mdio;
}
debug("%s: OK\n", __func__);
return 0;
+err_free_mdio:
mdio_free(eqos->mii);
+err_remove_resources_tegra:
eqos_remove_resources_tegra(dev);
+err_remove_resources_core:
eqos_remove_resources_core(dev);
debug("%s: returns %d\n", __func__, ret);
return ret;
+}
+static int eqos_remove(struct udevice *dev) +{
struct eqos_priv *eqos = dev_get_priv(dev);
debug("%s(dev=%p):\n", __func__, dev);
mdio_unregister(eqos->mii);
mdio_free(eqos->mii);
eqos_remove_resources_tegra(dev);
eqos_probe_resources_core(dev);
debug("%s: OK\n", __func__);
return 0;
+}
+static const struct eth_ops eqos_ops = {
.start = eqos_start,
.stop = eqos_stop,
.send = eqos_send,
.recv = eqos_recv,
.free_pkt = eqos_free_pkt,
+};
+static const struct udevice_id eqos_ids[] = {
{ .compatible = "nvidia,tegra186-eqos" },
{ }
+};
+U_BOOT_DRIVER(eth_eqos) = {
.name = "eth_eqos",
.id = UCLASS_ETH,
.of_match = eqos_ids,
.probe = eqos_probe,
.remove = eqos_remove,
.ops = &eqos_ops,
.priv_auto_alloc_size = sizeof(struct eqos_priv),
.platdata_auto_alloc_size = sizeof(struct eth_pdata),
+};
2.9.3
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On 09/23/2016 03:49 PM, Joe Hershberger wrote:
Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
+/* Core registers */
Why are registers not defined as a struct? That is the common way that is accepted to represent registers in a driver.
It is common, but I find using structs has significant disadvantages, which outweigh any advantages, so I strongly prefer not to use them.
Disadvantages of structs:
* Drivers often have to deal with different but similar HW variants. Similar enough that it makes sense to use the same driver for them, but different enough that certain registers are placed differently in the memory map. With structs, there's little choice but to use ifdefs to pick a particular HW variant and bake that into the driver. This doesn't work well for drivers that must pick the variant at run-time rather than compile-time, e.g. drivers for USB or PCIe devices, or when you wish to build a single SW image that can run on multiple SoCs.
A common variant on this theme is device with different register layouts due to IP integration issues, such as a UART with byte-wide registers that appear at consecutive byte offsets in some SoCs, but at separate word addresses in other SoCs.
This issue is relevant here since the EQoS block is a generic IP block with many options that may have different SoC- or IP-version-specific differences between SoCs.
* The register space for the EQoS driver is quite sparse, and U-Boot uses a subset of the registers effectively making it even more sparse. Defining a struct to describe this will be a frustrating exercise in calculating the right amount of padding to place into the struct to get the correct offsets. Mistakes will be made and it will be annoying.
* Structs don't translate well to interactive debugging. It's common to read/write registers using md/mw U-Boot shell commands. With structs, you need to either (a) use some other data source for the register offsets, (b) manually add up field sizes (c) write a comment indicating the offset next to each field. (a) and (b) are both annoying, and by the time you've done (c) (which is quite complicated in the face of ifdefs) you may as well have simply used #defines in the first place.
+/*
- Warn if the cache-line size is larger than the descriptor size. In such
- cases the driver will likely fail because the CPU needs to flush the cache
- when requeuing RX buffers, therefore descriptors written by the hardware
- may be discarded.
- This can be fixed by defining CONFIG_SYS_NONCACHED_MEMORY which will cause
- the driver to allocate descriptors from a pool of non-cached memory.
- */
+#if EQOS_DESCRIPTOR_SIZE < ARCH_DMA_MINALIGN +#if !defined(CONFIG_SYS_NONCACHED_MEMORY) && \
!defined(CONFIG_SYS_DCACHE_OFF) && !defined(CONFIG_X86)
Please include an explanation for why x86 is special.
Sure. Do note though that this exact text already exists in other U-Boot drivers.
+static int eqos_start_clks_tegra(struct udevice *dev)
Why is this not in a board file and a separate patch?
The clocks (and other resources) that this driver operates on are directly related to the EQoS IP block itself. Hence, it's completely appropriate for this driver to deal with them.
Perhaps the name "tegra" is slightly misleading; it refers to "the configuration of the core EQoS block that Tegra happens to have chosen" rather than "something entirely Tegra-specific that is unrelated to this device/driver". However, I'm not sure what name would be better than "tegra" here; the correct name is something like axi_ahb_dma_mtl_core_single_phy_rgmii, but that's far too long for a symbol name. I attempted to describe this in the comment regarding IP configuration at the beginning of the file. Is there a specific shortcoming in that comment that could be changed to address this better?
+static int eqos_calibrate_pads_tegra(struct udevice *dev)
This function is the only part that's truly Tegra-specific, rather than specific to the standard IP configuration Tegra happens to use. However, I still believe it's entirely appropriate for this driver to include the logic to control these registers; they are part of the Ethernet IP block itself, even if NVIDIA added them as customer registers. The registers can't be controlled from any board/... file without very tight co-ordination with the driver; they aren't something that a board/... file could set up once at boot time in readiness for this driver to run later. The only way we could separate out this function from the driver would be to provide hook/callback functions from this driver to board logic. I don't believe that would make the code any cleaner. For one thing, it would spread related code across multiple locations making it hard to find. Another issue would be determining when to call the hook/callback functions for a specific device; they may only be applicable to a single instance of the device in a multi-device system. It's entirely possible someone could place an EQoS IP block into a PCI device, and then you'd potentially need a separate hook implementation for the in-SoC and the on-PCI instance. The driver can work that out reasonably easily itself based on how it's instantiated, but finding and distinguishing the different external implementations of the hook functions would be challenging.
+static int eqos_start(struct udevice *dev)
/* Update the MAC address */
val = (plat->enetaddr[5] << 8) |
(plat->enetaddr[4]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH);
val = (plat->enetaddr[3] << 24) |
(plat->enetaddr[2] << 16) |
(plat->enetaddr[1] << 8) |
(plat->enetaddr[0]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW);
This should be implemented in write_hwaddr() op.
Does the network core code guarantee that start() is called before write_hwaddr() (so that clocks are active) and write_hwaddr() is called before any packets are sent/received? If so, I can move this.
I note that there are other existing network drivers that set the MAC address in start(); see rtl8169.c for example.
+int eqos_send(struct udevice *dev, void *packet, int length) +{
struct eqos_priv *eqos = dev_get_priv(dev);
u32 *tx_desc;
Please make this a struct.
IIRC I'd avoided that because there are a variety of different formats based on the type, which will involve a mess of unions. I'll take a look at the details and convert them if it doesn't turn out massively horrible; they're small enough that it sounds reasonable to do so on the surface.

Hi Stephen,
On Tue, Sep 27, 2016 at 12:02 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 09/23/2016 03:49 PM, Joe Hershberger wrote:
Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
+/* Core registers */
Why are registers not defined as a struct? That is the common way that is accepted to represent registers in a driver.
It is common, but I find using structs has significant disadvantages, which outweigh any advantages, so I strongly prefer not to use them.
Disadvantages of structs:
- Drivers often have to deal with different but similar HW variants. Similar
enough that it makes sense to use the same driver for them, but different enough that certain registers are placed differently in the memory map. With structs, there's little choice but to use ifdefs to pick a particular HW variant and bake that into the driver. This doesn't work well for drivers that must pick the variant at run-time rather than compile-time, e.g. drivers for USB or PCIe devices, or when you wish to build a single SW image that can run on multiple SoCs.
Is that really the case here or are you just making a broad statement? Are registers really moving based on IP configuration? That sounds like broken IP / IP generator.
A common variant on this theme is device with different register layouts due to IP integration issues, such as a UART with byte-wide registers that appear at consecutive byte offsets in some SoCs, but at separate word addresses in other SoCs.
Again, this sounds like a generic argument that doesn't apply here.
This issue is relevant here since the EQoS block is a generic IP block with many options that may have different SoC- or IP-version-specific differences between SoCs.
But simply additive, no? Included features add registers or bitfields.
- The register space for the EQoS driver is quite sparse, and U-Boot uses a
subset of the registers effectively making it even more sparse. Defining a struct to describe this will be a frustrating exercise in calculating the right amount of padding to place into the struct to get the correct offsets. Mistakes will be made and it will be annoying.
It's also organized into a few blocks. It's preferable to group registers into a struct that represents each block instead of one huge struct. It also hurts nothing to have registers defined in the struct that apply to configurations that may not be enabled in a given instance... the switching can be done at the spot where the register is accessed. And the accesses don't have to be compile-time options so you can have your built-in and your PCIe variants.
- Structs don't translate well to interactive debugging. It's common to
read/write registers using md/mw U-Boot shell commands. With structs, you need to either (a) use some other data source for the register offsets, (b) manually add up field sizes (c) write a comment indicating the offset next to each field. (a) and (b) are both annoying, and by the time you've done (c) (which is quite complicated in the face of ifdefs) you may as well have simply used #defines in the first place.
(a) is not so bad, but not ideal. I agree that (b) is annoying. I don't agree that you have to ifdef it, see above. I think (c) is a bit nicer than (a), but either are fine. I think the resulting code that is accessing the registers looks more concise and readable than a defined constant that is going to need to contain all of the context / scope in its name for disambiguation in the global namespace. A register name in a struct is scoped to that block and a local variable can be much shorter such that you don't have to keep repeating the same text throughout the function.
+/*
- Warn if the cache-line size is larger than the descriptor size. In
such
- cases the driver will likely fail because the CPU needs to flush the
cache
- when requeuing RX buffers, therefore descriptors written by the
hardware
- may be discarded.
- This can be fixed by defining CONFIG_SYS_NONCACHED_MEMORY which will
cause
- the driver to allocate descriptors from a pool of non-cached memory.
- */
+#if EQOS_DESCRIPTOR_SIZE < ARCH_DMA_MINALIGN +#if !defined(CONFIG_SYS_NONCACHED_MEMORY) && \
!defined(CONFIG_SYS_DCACHE_OFF) && !defined(CONFIG_X86)
Please include an explanation for why x86 is special.
Sure. Do note though that this exact text already exists in other U-Boot drivers.
That's unfortunate. At least someone can find the explanation here after you add it.
+static int eqos_start_clks_tegra(struct udevice *dev)
Why is this not in a board file and a separate patch?
The clocks (and other resources) that this driver operates on are directly related to the EQoS IP block itself. Hence, it's completely appropriate for this driver to deal with them.
Perhaps the name "tegra" is slightly misleading; it refers to "the configuration of the core EQoS block that Tegra happens to have chosen" rather than "something entirely Tegra-specific that is unrelated to this device/driver". However, I'm not sure what name would be better than "tegra" here; the correct name is something like axi_ahb_dma_mtl_core_single_phy_rgmii, but that's far too long for a symbol name. I attempted to describe this in the comment regarding IP configuration at the beginning of the file. Is there a specific shortcoming in that comment that could be changed to address this better?
I think the comment should list the configuration with at least the level of detail that you used to create that long symbol name.
Do you anticipate that this set of configurations will with high confidence be used in Tegra going forward? Or would it be better to call it tegra186?
+static int eqos_calibrate_pads_tegra(struct udevice *dev)
This function is the only part that's truly Tegra-specific, rather than specific to the standard IP configuration Tegra happens to use. However, I still believe it's entirely appropriate for this driver to include the logic to control these registers; they are part of the Ethernet IP block itself, even if NVIDIA added them as customer registers. The registers can't be controlled from any board/... file without very tight co-ordination with the driver; they aren't something that a board/... file could set up once at boot time in readiness for this driver to run later. The only way we could separate out this function from the driver would be to provide hook/callback functions from this driver to board logic. I don't believe that would make the code any cleaner. For one thing, it would spread related code across multiple locations making it hard to find. Another issue would be determining when to call the hook/callback functions for a specific device; they may only be applicable to a single instance of the device in a multi-device system. It's entirely possible someone could place an EQoS IP block into a PCI device, and then you'd potentially need a separate hook implementation for the in-SoC and the on-PCI instance. The driver can work that out reasonably easily itself based on how it's instantiated, but finding and distinguishing the different external implementations of the hook functions would be challenging.
OK, I'm sold. The registers are part of this IP block, then the code should be in this driver. Thanks.
+static int eqos_start(struct udevice *dev)
/* Update the MAC address */
val = (plat->enetaddr[5] << 8) |
(plat->enetaddr[4]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH);
val = (plat->enetaddr[3] << 24) |
(plat->enetaddr[2] << 16) |
(plat->enetaddr[1] << 8) |
(plat->enetaddr[0]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW);
This should be implemented in write_hwaddr() op.
Does the network core code guarantee that start() is called before write_hwaddr() (so that clocks are active) and write_hwaddr() is called before any packets are sent/received? If so, I can move this.
start() is not called first, it's called after. But it can also be called other times later, so it makes sense to register it even if you call it again in start().
Maybe I need to add a flag that defers this until after start. However, for most boards, it is undesirable to require start. From net/eth-uclass.c:
/* * Devices need to write the hwaddr even if not started so that Linux * will have access to the hwaddr that u-boot stored for the device. * This is accomplished by attempting to probe each device and calling * their write_hwaddr() operation. */
It sounds like you have a peculiar situation where you aren't starting your peripheral clocks in SPL or some early board init, so you can't even access registers around probe time. Is that really required?
I note that there are other existing network drivers that set the MAC address in start(); see rtl8169.c for example.
I believe that the reset that is done in the rtl8169 driver clears the MAC address, so it needs to be set on each start, but that is not that common. The real question is why those drivers need to reset the hardware for every network operation instead of just in probe or so. Realistically, the network stack needs to be rearchitected to allow multiple active MACs simultaneously. Along with that would be the elimination of the nuclear option of resetting the IP before every operation.
Thanks, -Joe

On 10/11/2016 05:24 PM, Joe Hershberger wrote:
Hi Stephen,
On Tue, Sep 27, 2016 at 12:02 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 09/23/2016 03:49 PM, Joe Hershberger wrote:
Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
+/* Core registers */
Why are registers not defined as a struct? That is the common way that is accepted to represent registers in a driver.
It is common, but I find using structs has significant disadvantages, which outweigh any advantages, so I strongly prefer not to use them.
Disadvantages of structs:
- Drivers often have to deal with different but similar HW variants. Similar
enough that it makes sense to use the same driver for them, but different enough that certain registers are placed differently in the memory map. With structs, there's little choice but to use ifdefs to pick a particular HW variant and bake that into the driver. This doesn't work well for drivers that must pick the variant at run-time rather than compile-time, e.g. drivers for USB or PCIe devices, or when you wish to build a single SW image that can run on multiple SoCs.
Is that really the case here or are you just making a broad statement? Are registers really moving based on IP configuration? That sounds like broken IP / IP generator.
I think we should choose the best techniques and use them globally, even if a particular disadvantage of a non-optimal technique isn't relevant to a particular device. Thus, I don't think whether I'm making a broad statement is too relevant.
I know there are multiple versions of this HW block in existence. I don't know the details of the register compatibility between the different versions for sure.
There are many broken IPs and IP generators in existence, and I believe we need to accomodate that.
A common variant on this theme is device with different register layouts due to IP integration issues, such as a UART with byte-wide registers that appear at consecutive byte offsets in some SoCs, but at separate word addresses in other SoCs.
Again, this sounds like a generic argument that doesn't apply here.
I don't believe the distinction is appropriate.
This issue is relevant here since the EQoS block is a generic IP block with many options that may have different SoC- or IP-version-specific differences between SoCs.
But simply additive, no? Included features add registers or bitfields.
As far as I know, NVIDIA's register additions to this particular version of this particular HW block are purely additions in a well segregated register block.
This certainly isn't true for other HW blocks in Tegra though (e.g. consider the Synopsys USB2 controller Tegra uses), and I'm sure the same applies to many other HW blocks in many other SoCs. I wouldn't be at all surprised if some SoC vendor does the same thing to this IP block. There's no way to tell until someone appears to integrate driver support into U-Boot.
- The register space for the EQoS driver is quite sparse, and U-Boot uses a
subset of the registers effectively making it even more sparse. Defining a struct to describe this will be a frustrating exercise in calculating the right amount of padding to place into the struct to get the correct offsets. Mistakes will be made and it will be annoying.
It's also organized into a few blocks. It's preferable to group registers into a struct that represents each block instead of one huge struct.
That will certainly help.
It also hurts nothing to have registers defined in the struct that apply to configurations that may not be enabled in a given instance... the switching can be done at the spot where the register is accessed. And the accesses don't have to be compile-time options so you can have your built-in and your PCIe variants.
So long as there aren't conflicting register definitions, which is certainly not guaranteed in any way, what you say is true.
Anyway, I'll rewrite the driver using structs for now in the interests of moving it forward. If this causes issue down the road, whomever is adding the support for additional devices can worry about how to solve any issues that arise then.
+static int eqos_start_clks_tegra(struct udevice *dev)
Why is this not in a board file and a separate patch?
The clocks (and other resources) that this driver operates on are directly related to the EQoS IP block itself. Hence, it's completely appropriate for this driver to deal with them.
Perhaps the name "tegra" is slightly misleading; it refers to "the configuration of the core EQoS block that Tegra happens to have chosen" rather than "something entirely Tegra-specific that is unrelated to this device/driver". However, I'm not sure what name would be better than "tegra" here; the correct name is something like axi_ahb_dma_mtl_core_single_phy_rgmii, but that's far too long for a symbol name. I attempted to describe this in the comment regarding IP configuration at the beginning of the file. Is there a specific shortcoming in that comment that could be changed to address this better?
I think the comment should list the configuration with at least the level of detail that you used to create that long symbol name.
Do you anticipate that this set of configurations will with high confidence be used in Tegra going forward? Or would it be better to call it tegra186?
It's too early to tell. I hope we'll maintain the same configuration in any future SoCs, but who knows.
I can rename it all to tegra186 if you want; we can simply call the tegra186 functions when running on any future SoCs that are backwards-compatible, which would be conceptually similar to the way Device Tree compatible values are extended.

On 09/23/2016 03:49 PM, Joe Hershberger wrote:
Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
+static int eqos_start(struct udevice *dev)
/* Update the MAC address */
val = (plat->enetaddr[5] << 8) |
(plat->enetaddr[4]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH);
val = (plat->enetaddr[3] << 24) |
(plat->enetaddr[2] << 16) |
(plat->enetaddr[1] << 8) |
(plat->enetaddr[0]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW);
This should be implemented in write_hwaddr() op.
That op is never called because this driver is only instantiated by device tree. Since this code can't be skipped, it can't be moved to that op.
+int eqos_send(struct udevice *dev, void *packet, int length)
for (i = 0; i < 1000000; i++) {
eqos_inval_desc(tx_desc);
if (!(readl(&tx_desc[3]) & EQOS_DESC3_OWN))
return 0;
Use wait_bit() here.
That function won't work here due to the need to call eqos_inval_desc(tx_desc) inside the loop.

On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 09/23/2016 03:49 PM, Joe Hershberger wrote:
Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
+static int eqos_start(struct udevice *dev)
/* Update the MAC address */
val = (plat->enetaddr[5] << 8) |
(plat->enetaddr[4]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH);
val = (plat->enetaddr[3] << 24) |
(plat->enetaddr[2] << 16) |
(plat->enetaddr[1] << 8) |
(plat->enetaddr[0]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW);
This should be implemented in write_hwaddr() op.
That op is never called because this driver is only instantiated by device tree. Since this code can't be skipped, it can't be moved to that op.
I don't understand what you're saying here. That op is called in eth_initialize() on every device found in device tree.
+int eqos_send(struct udevice *dev, void *packet, int length)
for (i = 0; i < 1000000; i++) {
eqos_inval_desc(tx_desc);
if (!(readl(&tx_desc[3]) & EQOS_DESC3_OWN))
return 0;
Use wait_bit() here.
That function won't work here due to the need to call eqos_inval_desc(tx_desc) inside the loop.

On 10/11/2016 04:48 PM, Joe Hershberger wrote:
On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 09/23/2016 03:49 PM, Joe Hershberger wrote:
Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
+static int eqos_start(struct udevice *dev)
/* Update the MAC address */
val = (plat->enetaddr[5] << 8) |
(plat->enetaddr[4]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH);
val = (plat->enetaddr[3] << 24) |
(plat->enetaddr[2] << 16) |
(plat->enetaddr[1] << 8) |
(plat->enetaddr[0]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW);
This should be implemented in write_hwaddr() op.
That op is never called because this driver is only instantiated by device tree. Since this code can't be skipped, it can't be moved to that op.
I don't understand what you're saying here. That op is called in eth_initialize() on every device found in device tree.
Oh, so it is. I must have screwed up my tracing before.
Anyway, I still don't believe using write_hwaddr() is correct for this HW. It's marked optional in include/net.h; it would be implemented in cases where the MAC address should be passed to subsequent SW in Ethernet controller registers. That's not the case here. The master location for the MAC address is in an unrelated EEPROM that all drivers must read. Resetting the device (as any driver would do to guarantee correct operation no matter what SW ran before), or never initializing it in the first place (as is the case now without any driver in U-Boot) means that those registers cannot be assumed to contain valid data. Thus SW after U-Boot won't rely on the MAC address register values, and hence there's no need to implement .write_hwaddr() separately, since that's the only reason it exists separately according to the comments in include/net.h.

On Fri, Oct 14, 2016 at 1:35 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/11/2016 04:48 PM, Joe Hershberger wrote:
On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 09/23/2016 03:49 PM, Joe Hershberger wrote:
Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
+static int eqos_start(struct udevice *dev)
/* Update the MAC address */
val = (plat->enetaddr[5] << 8) |
(plat->enetaddr[4]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH);
val = (plat->enetaddr[3] << 24) |
(plat->enetaddr[2] << 16) |
(plat->enetaddr[1] << 8) |
(plat->enetaddr[0]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW);
This should be implemented in write_hwaddr() op.
That op is never called because this driver is only instantiated by device tree. Since this code can't be skipped, it can't be moved to that op.
I don't understand what you're saying here. That op is called in eth_initialize() on every device found in device tree.
Oh, so it is. I must have screwed up my tracing before.
Anyway, I still don't believe using write_hwaddr() is correct for this HW. It's marked optional in include/net.h; it would be implemented in cases where the MAC address should be passed to subsequent SW in Ethernet controller registers. That's not the case here. The master location for the MAC address is in an unrelated EEPROM that all drivers must read.
That sounds more like a NV storage location for a read_rom_hwaddr() op to get a default mac addr that can be overridden with the env. The write_hwaddr is about what the mac uses to filter for limiting packet ingress. One reason to support it as an op is so that when the env var for the mac address is changed, the mac filter in the hw is also updated.
Resetting the device (as any driver would do to guarantee correct operation no matter what SW ran before), or never initializing it in the first place (as is the case now without any driver in U-Boot) means that those registers cannot be assumed to contain valid data.
That's what the comment I quoted in my last mail is directly addressing. It does make that data valid.
Thus SW after U-Boot won't rely on the MAC address register values,
And yet it does in most cases, hence the comment I quoted in the last mail.
and hence there's no need to implement .write_hwaddr() separately, since that's the only reason it exists separately according to the comments in include/net.h.

On 10/13/2016 05:46 PM, Joe Hershberger wrote:
On Fri, Oct 14, 2016 at 1:35 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/11/2016 04:48 PM, Joe Hershberger wrote:
On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 09/23/2016 03:49 PM, Joe Hershberger wrote:
Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This driver supports the Synopsys Designware Ethernet QoS (Quality of Service) a/k/a eqos IP block, which is a different design than the HW supported by the existing designware.c driver. The IP supports many options for bus type, clocking/reset structure, and feature list. This driver currently supports the specific configuration used in NVIDIA's Tegra186 chip, but should be extensible to other combinations quite easily, as explained in the source.
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
+static int eqos_start(struct udevice *dev)
/* Update the MAC address */
val = (plat->enetaddr[5] << 8) |
(plat->enetaddr[4]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH);
val = (plat->enetaddr[3] << 24) |
(plat->enetaddr[2] << 16) |
(plat->enetaddr[1] << 8) |
(plat->enetaddr[0]);
writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW);
This should be implemented in write_hwaddr() op.
That op is never called because this driver is only instantiated by device tree. Since this code can't be skipped, it can't be moved to that op.
I don't understand what you're saying here. That op is called in eth_initialize() on every device found in device tree.
Oh, so it is. I must have screwed up my tracing before.
Anyway, I still don't believe using write_hwaddr() is correct for this HW. It's marked optional in include/net.h; it would be implemented in cases where the MAC address should be passed to subsequent SW in Ethernet controller registers. That's not the case here. The master location for the MAC address is in an unrelated EEPROM that all drivers must read.
That sounds more like a NV storage location for a read_rom_hwaddr() op to get a default mac addr that can be overridden with the env.
If the EQoS HW module contained the interface to this EEPROM, such that all instances of the HW module always accessed the EEPROM in the same way and the layout of data in the EEPROM was fixed by the HW module, then yes.
However, the EqoS HW module doesn't define any mechanism for non-volatile MAC address storage; only the runtime registers. So, we can't implement read_rom_hwaddr() inside the EQoS driver unfortunately.
The write_hwaddr is about what the mac uses to filter for limiting packet ingress. One reason to support it as an op is so that when the env var for the mac address is changed, the mac filter in the hw is also updated.
I believe that every time the Ethernet device is used, the start() op is called first, followed by packet transfer, followed by the stop() op. If start() always programs the MAC address, the driver will always end up using the value requested by the user.
Resetting the device (as any driver would do to guarantee correct operation no matter what SW ran before), or never initializing it in the first place (as is the case now without any driver in U-Boot) means that those registers cannot be assumed to contain valid data.
That's what the comment I quoted in my last mail is directly addressing. It does make that data valid.
Resetting the device zeros out the MAC address value in the runtime registers. Perhaps we mean different things by "make the data valid", but resetting the device certainly doesn't load/activate any useful MAC address.
Thus SW after U-Boot won't rely on the MAC address register values,
And yet it does in most cases, hence the comment I quoted in the last mail.
This certainly isn't true for this HW at least, and I believe aside from any platform-specific hacks is true fairly generally across other Ethernet devices too. I've checked:
a) The mainline kernel's EQoS driver.
b) The Synopsis-supplied EQoS driver as used in NVIDIA's downstream Tegra kernel.
In both cases, the driver retrieves the desired MAC address from sources other than the EQoS registers (i.e. device tree, or a system-specific user-space application which sets the MAC address before enabling the interface), and unconditionally programs that value into the EQoS runtime registers.
I also talked to the only user of the mainline Linux EQoS driver, and he also is of the opinion that we can't rely on transferring the MAC address between U-Boot (or any FW/...) and Linux using the EQoS registers.

Hi Stephen,
On Mon, Oct 17, 2016 at 1:32 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/13/2016 05:46 PM, Joe Hershberger wrote:
On Fri, Oct 14, 2016 at 1:35 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/11/2016 04:48 PM, Joe Hershberger wrote:
On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 09/23/2016 03:49 PM, Joe Hershberger wrote:
Hi Stephen,
Thanks for sending this! I have some comments below.
Cheers, -Joe
On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote: > > > > From: Stephen Warren swarren@nvidia.com > > This driver supports the Synopsys Designware Ethernet QoS (Quality of > Service) a/k/a eqos IP block, which is a different design than the HW > supported by the existing designware.c driver. The IP supports many > options for bus type, clocking/reset structure, and feature list. > This > driver currently supports the specific configuration used in NVIDIA's > Tegra186 chip, but should be extensible to other combinations quite > easily, as explained in the source.
> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
+static int eqos_start(struct udevice *dev)
> + /* Update the MAC address */ > + val = (plat->enetaddr[5] << 8) | > + (plat->enetaddr[4]); > + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH); > + val = (plat->enetaddr[3] << 24) | > + (plat->enetaddr[2] << 16) | > + (plat->enetaddr[1] << 8) | > + (plat->enetaddr[0]); > + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW);
This should be implemented in write_hwaddr() op.
That op is never called because this driver is only instantiated by device tree. Since this code can't be skipped, it can't be moved to that op.
I don't understand what you're saying here. That op is called in eth_initialize() on every device found in device tree.
Oh, so it is. I must have screwed up my tracing before.
Anyway, I still don't believe using write_hwaddr() is correct for this HW. It's marked optional in include/net.h; it would be implemented in cases where the MAC address should be passed to subsequent SW in Ethernet controller registers. That's not the case here. The master location for the MAC address is in an unrelated EEPROM that all drivers must read.
That sounds more like a NV storage location for a read_rom_hwaddr() op to get a default mac addr that can be overridden with the env.
If the EQoS HW module contained the interface to this EEPROM, such that all instances of the HW module always accessed the EEPROM in the same way and the layout of data in the EEPROM was fixed by the HW module, then yes.
However, the EqoS HW module doesn't define any mechanism for non-volatile MAC address storage; only the runtime registers. So, we can't implement read_rom_hwaddr() inside the EQoS driver unfortunately.
OK.
The write_hwaddr is about what the mac uses to filter for limiting packet ingress. One reason to support it as an op is so that when the env var for the mac address is changed, the mac filter in the hw is also updated.
I believe that every time the Ethernet device is used, the start() op is called first, followed by packet transfer, followed by the stop() op. If start() always programs the MAC address, the driver will always end up using the value requested by the user.
That may be. I still don't understand the reluctance to implement it. When the behavior of start is restructured, it will be one fewer place to be forced to retrofit.
Resetting the device (as any driver would do to guarantee correct operation no matter what SW ran before), or never initializing it in the first place (as is the case now without any driver in U-Boot) means that those registers cannot be assumed to contain valid data.
That's what the comment I quoted in my last mail is directly addressing. It does make that data valid.
Resetting the device zeros out the MAC address value in the runtime registers. Perhaps we mean different things by "make the data valid", but resetting the device certainly doesn't load/activate any useful MAC address.
The point that comment was making was to initialize those registers in all cases, so as to make them valid. Of course resetting undoes that. At least some of the devices I've worked with use those registers to communicate the MAC address from U-Boot to kernel. If that is not true of this device, so be it.
Thus SW after U-Boot won't rely on the MAC address register values,
And yet it does in most cases, hence the comment I quoted in the last mail.
This certainly isn't true for this HW at least, and I believe aside from any platform-specific hacks is true fairly generally across other Ethernet devices too. I've checked:
OK
a) The mainline kernel's EQoS driver.
b) The Synopsis-supplied EQoS driver as used in NVIDIA's downstream Tegra kernel.
Is this different from a) for some reason?
In both cases, the driver retrieves the desired MAC address from sources other than the EQoS registers (i.e. device tree, or a system-specific user-space application which sets the MAC address before enabling the interface), and unconditionally programs that value into the EQoS runtime registers.
I also talked to the only user of the mainline Linux EQoS driver, and he also is of the opinion that we can't rely on transferring the MAC address between U-Boot (or any FW/...) and Linux using the EQoS registers.
I think you can choose to rely on it and make it work. It's done by others, but does not have to be if you choose to go another way.
If you were to rely on it the way others do, then you would be able to isolate the knowledge about how to determine what the MAC is in a single location in U-Boot and be able to share that logic between U-Boot and Linux, since U-Boot is the only one that needs to know about the EEPROM (for instance).
-Joe

On 10/19/2016 12:29 PM, Joe Hershberger wrote:
Hi Stephen,
On Mon, Oct 17, 2016 at 1:32 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/13/2016 05:46 PM, Joe Hershberger wrote:
On Fri, Oct 14, 2016 at 1:35 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/11/2016 04:48 PM, Joe Hershberger wrote:
On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 09/23/2016 03:49 PM, Joe Hershberger wrote: > On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren swarren@wwwdotorg.org wrote: >> This driver supports the Synopsys Designware Ethernet QoS (Quality of >> Service) a/k/a eqos IP block, which is a different design than the HW >> supported by the existing designware.c driver. The IP supports many >> options for bus type, clocking/reset structure, and feature list. >> This >> driver currently supports the specific configuration used in NVIDIA's >> Tegra186 chip, but should be extensible to other combinations quite >> easily, as explained in the source.
...
>> +static int eqos_start(struct udevice *dev)
...
>> + /* Update the MAC address */ >> + val = (plat->enetaddr[5] << 8) | >> + (plat->enetaddr[4]); >> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH); >> + val = (plat->enetaddr[3] << 24) | >> + (plat->enetaddr[2] << 16) | >> + (plat->enetaddr[1] << 8) | >> + (plat->enetaddr[0]); >> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW); > > This should be implemented in write_hwaddr() op.
...
Anyway, I still don't believe using write_hwaddr() is correct for this HW. It's marked optional in include/net.h; it would be implemented in cases where the MAC address should be passed to subsequent SW in Ethernet controller registers. That's not the case here. The master location for the MAC address is in an unrelated EEPROM that all drivers must read.
That sounds more like a NV storage location for a read_rom_hwaddr() op to get a default mac addr that can be overridden with the env.
If the EQoS HW module contained the interface to this EEPROM, such that all instances of the HW module always accessed the EEPROM in the same way and the layout of data in the EEPROM was fixed by the HW module, then yes.
However, the EqoS HW module doesn't define any mechanism for non-volatile MAC address storage; only the runtime registers. So, we can't implement read_rom_hwaddr() inside the EQoS driver unfortunately.
OK.
The write_hwaddr is about what the mac uses to filter for limiting packet ingress. One reason to support it as an op is so that when the env var for the mac address is changed, the mac filter in the hw is also updated.
I believe that every time the Ethernet device is used, the start() op is called first, followed by packet transfer, followed by the stop() op. If start() always programs the MAC address, the driver will always end up using the value requested by the user.
That may be. I still don't understand the reluctance to implement it.
I don't want to implement it because it can't work.
write_hwaddr() is called before start() is called. At that point, clocks to the EQoS HW are not running and the EQoS HW is in reset, and hence it cannot accept any register accesses; attempting any accesses will hang the bus and CPU.
These are the possible solutions:
a) Don't implement write_hwaddr()
b) Make write_hwaddr() turn on the clock and clear the reset, program the register, then reset the device and assert the reset. Re-asserting reset is required so that setting the MAC address doesn't leave the clock running even when the device isn't in use.This is pointless since the written value will not last beyond the end of the function.
c) Make probe() start the clock and clear the reset. Then write_hwaddr() can successfully write the MAC address registers at any time. This would waste power running the clock to the device when it's not in use. Also, Simon Glass continually asks that U-Boot not initialize HW that the user hasn't attempted to use. I believe turning on the clocks in probe() violates this.
d) Modify the network core to only call write_hwaddr() between the device's start() and stop() functions. I haven't looked into this at all, but I imagine it will have a fairly significant impact across many parts of the network core and other drivers.
When the behavior of start is restructured, it will be one fewer place to be forced to retrofit.
I believe that the extra work required to refactor write_hwaddr() will be miniscule compared to splitting probe(), start(), and stop() up into separate functions anyway.
...
a) The mainline kernel's EQoS driver.
b) The Synopsis-supplied EQoS driver as used in NVIDIA's downstream Tegra kernel.
Is this different from a) for some reason?
Yes. Synopsis supplies a non-mainlined Linux driver for the EQoS HW. For whatever reason, we're using that in our downstream kernel.
In both cases, the driver retrieves the desired MAC address from sources other than the EQoS registers (i.e. device tree, or a system-specific user-space application which sets the MAC address before enabling the interface), and unconditionally programs that value into the EQoS runtime registers.
I also talked to the only user of the mainline Linux EQoS driver, and he also is of the opinion that we can't rely on transferring the MAC address between U-Boot (or any FW/...) and Linux using the EQoS registers.
I think you can choose to rely on it and make it work. It's done by others, but does not have to be if you choose to go another way.
If you were to rely on it the way others do, then you would be able to isolate the knowledge about how to determine what the MAC is in a single location in U-Boot and be able to share that logic between U-Boot and Linux, since U-Boot is the only one that needs to know about the EEPROM (for instance).
The knowledge is already isolated; the mainline Linux kernel will take the MAC address from device tree. This is the standard mechanism (for systems that boot using DT) that works across all Ethernet devices.

Hi,
On 19 October 2016 at 15:14, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/19/2016 12:29 PM, Joe Hershberger wrote:
Hi Stephen,
On Mon, Oct 17, 2016 at 1:32 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/13/2016 05:46 PM, Joe Hershberger wrote:
On Fri, Oct 14, 2016 at 1:35 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/11/2016 04:48 PM, Joe Hershberger wrote:
On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren swarren@wwwdotorg.org wrote: > > On 09/23/2016 03:49 PM, Joe Hershberger wrote: >> >> On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren >> swarren@wwwdotorg.org wrote: >>> >>> This driver supports the Synopsys Designware Ethernet QoS (Quality >>> of >>> Service) a/k/a eqos IP block, which is a different design than the >>> HW >>> supported by the existing designware.c driver. The IP supports many >>> options for bus type, clocking/reset structure, and feature list. >>> This >>> driver currently supports the specific configuration used in >>> NVIDIA's >>> Tegra186 chip, but should be extensible to other combinations quite >>> easily, as explained in the source.
...
>>> >>> +static int eqos_start(struct udevice *dev)
...
>>> >>> + /* Update the MAC address */ >>> + val = (plat->enetaddr[5] << 8) | >>> + (plat->enetaddr[4]); >>> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH); >>> + val = (plat->enetaddr[3] << 24) | >>> + (plat->enetaddr[2] << 16) | >>> + (plat->enetaddr[1] << 8) | >>> + (plat->enetaddr[0]); >>> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW); >> >> >> This should be implemented in write_hwaddr() op.
...
Anyway, I still don't believe using write_hwaddr() is correct for this HW. It's marked optional in include/net.h; it would be implemented in cases where the MAC address should be passed to subsequent SW in Ethernet controller registers. That's not the case here. The master location for the MAC address is in an unrelated EEPROM that all drivers must read.
That sounds more like a NV storage location for a read_rom_hwaddr() op to get a default mac addr that can be overridden with the env.
If the EQoS HW module contained the interface to this EEPROM, such that all instances of the HW module always accessed the EEPROM in the same way and the layout of data in the EEPROM was fixed by the HW module, then yes.
However, the EqoS HW module doesn't define any mechanism for non-volatile MAC address storage; only the runtime registers. So, we can't implement read_rom_hwaddr() inside the EQoS driver unfortunately.
OK.
The write_hwaddr is about what the mac uses to filter for limiting packet ingress. One reason to support it as an op is so that when the env var for the mac address is changed, the mac filter in the hw is also updated.
I believe that every time the Ethernet device is used, the start() op is called first, followed by packet transfer, followed by the stop() op. If start() always programs the MAC address, the driver will always end up using the value requested by the user.
That may be. I still don't understand the reluctance to implement it.
I don't want to implement it because it can't work.
write_hwaddr() is called before start() is called. At that point, clocks to the EQoS HW are not running and the EQoS HW is in reset, and hence it cannot accept any register accesses; attempting any accesses will hang the bus and CPU.
These are the possible solutions:
a) Don't implement write_hwaddr()
b) Make write_hwaddr() turn on the clock and clear the reset, program the register, then reset the device and assert the reset. Re-asserting reset is required so that setting the MAC address doesn't leave the clock running even when the device isn't in use.This is pointless since the written value will not last beyond the end of the function.
c) Make probe() start the clock and clear the reset. Then write_hwaddr() can successfully write the MAC address registers at any time. This would waste power running the clock to the device when it's not in use. Also, Simon Glass continually asks that U-Boot not initialize HW that the user hasn't attempted to use. I believe turning on the clocks in probe() violates this.
Not quite...or at least if I did I was mistaken. Of course we should limit init in probe() to what is necessary, but it is the bind() method which must not touch hardware.
It is fine to turn clocks on in probe if you want to.
However I am wondering whether we have something wrong in the Ethernet uclass interface. Should we move the MAC setting to after start(), or similar?
d) Modify the network core to only call write_hwaddr() between the device's start() and stop() functions. I haven't looked into this at all, but I imagine it will have a fairly significant impact across many parts of the network core and other drivers.
When the behavior of start is restructured, it will be one fewer place to be forced to retrofit.
I believe that the extra work required to refactor write_hwaddr() will be miniscule compared to splitting probe(), start(), and stop() up into separate functions anyway.
...
a) The mainline kernel's EQoS driver.
b) The Synopsis-supplied EQoS driver as used in NVIDIA's downstream Tegra kernel.
Is this different from a) for some reason?
Yes. Synopsis supplies a non-mainlined Linux driver for the EQoS HW. For whatever reason, we're using that in our downstream kernel.
In both cases, the driver retrieves the desired MAC address from sources other than the EQoS registers (i.e. device tree, or a system-specific user-space application which sets the MAC address before enabling the interface), and unconditionally programs that value into the EQoS runtime registers.
I also talked to the only user of the mainline Linux EQoS driver, and he also is of the opinion that we can't rely on transferring the MAC address between U-Boot (or any FW/...) and Linux using the EQoS registers.
I think you can choose to rely on it and make it work. It's done by others, but does not have to be if you choose to go another way.
If you were to rely on it the way others do, then you would be able to isolate the knowledge about how to determine what the MAC is in a single location in U-Boot and be able to share that logic between U-Boot and Linux, since U-Boot is the only one that needs to know about the EEPROM (for instance).
The knowledge is already isolated; the mainline Linux kernel will take the MAC address from device tree. This is the standard mechanism (for systems that boot using DT) that works across all Ethernet devices.
Regards, Simon

On 10/20/2016 12:48 PM, Simon Glass wrote:
Hi,
On 19 October 2016 at 15:14, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/19/2016 12:29 PM, Joe Hershberger wrote:
Hi Stephen,
On Mon, Oct 17, 2016 at 1:32 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/13/2016 05:46 PM, Joe Hershberger wrote:
On Fri, Oct 14, 2016 at 1:35 AM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/11/2016 04:48 PM, Joe Hershberger wrote: > > On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren > swarren@wwwdotorg.org wrote: >> >> On 09/23/2016 03:49 PM, Joe Hershberger wrote: >>> >>> On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren >>> swarren@wwwdotorg.org wrote: >>>> >>>> This driver supports the Synopsys Designware Ethernet QoS (Quality >>>> of >>>> Service) a/k/a eqos IP block, which is a different design than the >>>> HW >>>> supported by the existing designware.c driver. The IP supports many >>>> options for bus type, clocking/reset structure, and feature list. >>>> This >>>> driver currently supports the specific configuration used in >>>> NVIDIA's >>>> Tegra186 chip, but should be extensible to other combinations quite >>>> easily, as explained in the source.
...
>>>> >>>> +static int eqos_start(struct udevice *dev)
...
>>>> >>>> + /* Update the MAC address */ >>>> + val = (plat->enetaddr[5] << 8) | >>>> + (plat->enetaddr[4]); >>>> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH); >>>> + val = (plat->enetaddr[3] << 24) | >>>> + (plat->enetaddr[2] << 16) | >>>> + (plat->enetaddr[1] << 8) | >>>> + (plat->enetaddr[0]); >>>> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW); >>> >>> >>> This should be implemented in write_hwaddr() op.
...
Anyway, I still don't believe using write_hwaddr() is correct for this HW. It's marked optional in include/net.h; it would be implemented in cases where the MAC address should be passed to subsequent SW in Ethernet controller registers. That's not the case here. The master location for the MAC address is in an unrelated EEPROM that all drivers must read.
That sounds more like a NV storage location for a read_rom_hwaddr() op to get a default mac addr that can be overridden with the env.
If the EQoS HW module contained the interface to this EEPROM, such that all instances of the HW module always accessed the EEPROM in the same way and the layout of data in the EEPROM was fixed by the HW module, then yes.
However, the EqoS HW module doesn't define any mechanism for non-volatile MAC address storage; only the runtime registers. So, we can't implement read_rom_hwaddr() inside the EQoS driver unfortunately.
OK.
The write_hwaddr is about what the mac uses to filter for limiting packet ingress. One reason to support it as an op is so that when the env var for the mac address is changed, the mac filter in the hw is also updated.
I believe that every time the Ethernet device is used, the start() op is called first, followed by packet transfer, followed by the stop() op. If start() always programs the MAC address, the driver will always end up using the value requested by the user.
That may be. I still don't understand the reluctance to implement it.
I don't want to implement it because it can't work.
write_hwaddr() is called before start() is called. At that point, clocks to the EQoS HW are not running and the EQoS HW is in reset, and hence it cannot accept any register accesses; attempting any accesses will hang the bus and CPU.
These are the possible solutions:
a) Don't implement write_hwaddr()
b) Make write_hwaddr() turn on the clock and clear the reset, program the register, then reset the device and assert the reset. Re-asserting reset is required so that setting the MAC address doesn't leave the clock running even when the device isn't in use.This is pointless since the written value will not last beyond the end of the function.
c) Make probe() start the clock and clear the reset. Then write_hwaddr() can successfully write the MAC address registers at any time. This would waste power running the clock to the device when it's not in use. Also, Simon Glass continually asks that U-Boot not initialize HW that the user hasn't attempted to use. I believe turning on the clocks in probe() violates this.
Not quite...or at least if I did I was mistaken. Of course we should limit init in probe() to what is necessary, but it is the bind() method which must not touch hardware.
It is fine to turn clocks on in probe if you want to.
Even for a device that the user never ultimately makes use of? If so, this might be a reasonable solution. It feels like U-Boot should turn off the clocks before booting an OS though so that the overall system state isn't any different between the cases where this driver is present and enabled vs not. With the clocks manipulated by start()/stop() we already do this. If the clocks are enabled in probe() instead, this won't be the case.
However I am wondering whether we have something wrong in the Ethernet uclass interface. Should we move the MAC setting to after start(), or similar?
That's option d right below.
Joe's objection to this is that for some hardware, downstream OSs expect the MAC address to be programmed into controller registers before the OS starts. This required write_hwaddr() to be called even if the user doesn't actually make use of the Ethernet device, and hence in cases where start()/stop() are never called. This is probably more common for e.g. PCI devices where the bus imposes a certain system structure including the ability to access device registers immediately after boot without manual clock programming, rather than SoC-based designs where all bets are off regarding consistency, and system configuration data is more typically passed by either out-of-band configuration data structures like DT, or via entirely custom mechanisms.
d) Modify the network core to only call write_hwaddr() between the device's start() and stop() functions. I haven't looked into this at all, but I imagine it will have a fairly significant impact across many parts of the network core and other drivers.
When the behavior of start is restructured, it will be one fewer place to be forced to retrofit.
I believe that the extra work required to refactor write_hwaddr() will be miniscule compared to splitting probe(), start(), and stop() up into separate functions anyway.
...
a) The mainline kernel's EQoS driver.
b) The Synopsis-supplied EQoS driver as used in NVIDIA's downstream Tegra kernel.
Is this different from a) for some reason?
Yes. Synopsis supplies a non-mainlined Linux driver for the EQoS HW. For whatever reason, we're using that in our downstream kernel.
In both cases, the driver retrieves the desired MAC address from sources other than the EQoS registers (i.e. device tree, or a system-specific user-space application which sets the MAC address before enabling the interface), and unconditionally programs that value into the EQoS runtime registers.
I also talked to the only user of the mainline Linux EQoS driver, and he also is of the opinion that we can't rely on transferring the MAC address between U-Boot (or any FW/...) and Linux using the EQoS registers.
I think you can choose to rely on it and make it work. It's done by others, but does not have to be if you choose to go another way.
If you were to rely on it the way others do, then you would be able to isolate the knowledge about how to determine what the MAC is in a single location in U-Boot and be able to share that logic between U-Boot and Linux, since U-Boot is the only one that needs to know about the EEPROM (for instance).
The knowledge is already isolated; the mainline Linux kernel will take the MAC address from device tree. This is the standard mechanism (for systems that boot using DT) that works across all Ethernet devices.
Regards, Simon

On Thu, Oct 20, 2016 at 2:19 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/20/2016 12:48 PM, Simon Glass wrote:
Hi,
On 19 October 2016 at 15:14, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/19/2016 12:29 PM, Joe Hershberger wrote:
Hi Stephen,
On Mon, Oct 17, 2016 at 1:32 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/13/2016 05:46 PM, Joe Hershberger wrote:
On Fri, Oct 14, 2016 at 1:35 AM, Stephen Warren swarren@wwwdotorg.org wrote: > > > On 10/11/2016 04:48 PM, Joe Hershberger wrote: >> >> >> On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren >> swarren@wwwdotorg.org wrote: >>> >>> >>> On 09/23/2016 03:49 PM, Joe Hershberger wrote: >>>> >>>> >>>> On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren >>>> swarren@wwwdotorg.org wrote: >>>>> >>>>> >>>>> This driver supports the Synopsys Designware Ethernet QoS >>>>> (Quality >>>>> of >>>>> Service) a/k/a eqos IP block, which is a different design than >>>>> the >>>>> HW >>>>> supported by the existing designware.c driver. The IP supports >>>>> many >>>>> options for bus type, clocking/reset structure, and feature list. >>>>> This >>>>> driver currently supports the specific configuration used in >>>>> NVIDIA's >>>>> Tegra186 chip, but should be extensible to other combinations >>>>> quite >>>>> easily, as explained in the source.
...
>>>>> >>>>> >>>>> +static int eqos_start(struct udevice *dev)
...
>>>>> >>>>> >>>>> + /* Update the MAC address */ >>>>> + val = (plat->enetaddr[5] << 8) | >>>>> + (plat->enetaddr[4]); >>>>> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH); >>>>> + val = (plat->enetaddr[3] << 24) | >>>>> + (plat->enetaddr[2] << 16) | >>>>> + (plat->enetaddr[1] << 8) | >>>>> + (plat->enetaddr[0]); >>>>> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW); >>>> >>>> >>>> >>>> This should be implemented in write_hwaddr() op.
...
> > > Anyway, I still don't believe using write_hwaddr() is correct for > this > HW. > It's marked optional in include/net.h; it would be implemented in > cases > where the MAC address should be passed to subsequent SW in Ethernet > controller registers. That's not the case here. The master location > for > the MAC address is in an unrelated EEPROM that all drivers must read.
That sounds more like a NV storage location for a read_rom_hwaddr() op to get a default mac addr that can be overridden with the env.
If the EQoS HW module contained the interface to this EEPROM, such that all instances of the HW module always accessed the EEPROM in the same way and the layout of data in the EEPROM was fixed by the HW module, then yes.
However, the EqoS HW module doesn't define any mechanism for non-volatile MAC address storage; only the runtime registers. So, we can't implement read_rom_hwaddr() inside the EQoS driver unfortunately.
OK.
The write_hwaddr is about what the mac uses to filter for limiting packet ingress. One reason to support it as an op is so that when the env var for the mac address is changed, the mac filter in the hw is also updated.
I believe that every time the Ethernet device is used, the start() op is called first, followed by packet transfer, followed by the stop() op. If start() always programs the MAC address, the driver will always end up using the value requested by the user.
That may be. I still don't understand the reluctance to implement it.
I don't want to implement it because it can't work.
write_hwaddr() is called before start() is called. At that point, clocks to the EQoS HW are not running and the EQoS HW is in reset, and hence it cannot accept any register accesses; attempting any accesses will hang the bus and CPU.
These are the possible solutions:
a) Don't implement write_hwaddr()
b) Make write_hwaddr() turn on the clock and clear the reset, program the register, then reset the device and assert the reset. Re-asserting reset is required so that setting the MAC address doesn't leave the clock running even when the device isn't in use.This is pointless since the written value will not last beyond the end of the function.
c) Make probe() start the clock and clear the reset. Then write_hwaddr() can successfully write the MAC address registers at any time. This would waste power running the clock to the device when it's not in use. Also, Simon Glass continually asks that U-Boot not initialize HW that the user hasn't attempted to use. I believe turning on the clocks in probe() violates this.
Not quite...or at least if I did I was mistaken. Of course we should limit init in probe() to what is necessary, but it is the bind() method which must not touch hardware.
It is fine to turn clocks on in probe if you want to.
Even for a device that the user never ultimately makes use of? If so, this might be a reasonable solution. It feels like U-Boot should turn off the clocks before booting an OS though so that the overall system state isn't any different between the cases where this driver is present and enabled vs not. With the clocks manipulated by start()/stop() we already do this. If the clocks are enabled in probe() instead, this won't be the case.
However I am wondering whether we have something wrong in the Ethernet uclass interface. Should we move the MAC setting to after start(), or similar?
That's option d right below.
Joe's objection to this is that for some hardware, downstream OSs expect the MAC address to be programmed into controller registers before the OS starts. This required write_hwaddr() to be called even if the user doesn't actually make use of the Ethernet device, and hence in cases where start()/stop() are never called. This is probably more common for e.g. PCI devices where the bus imposes a certain system structure including the ability to access device registers immediately after boot without manual clock programming, rather than SoC-based designs where all bets are off regarding consistency, and system configuration data is more typically passed by either out-of-band configuration data structures like DT, or via entirely custom mechanisms.
Maybe we should just make it a config option to select between the two behaviors. Or is there already something that says "This system is structured" vs "This system is a SoC / all bets are off"?
d) Modify the network core to only call write_hwaddr() between the device's start() and stop() functions. I haven't looked into this at all, but I imagine it will have a fairly significant impact across many parts of the network core and other drivers.

On 10/20/2016 01:30 PM, Joe Hershberger wrote:
On Thu, Oct 20, 2016 at 2:19 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/20/2016 12:48 PM, Simon Glass wrote:
Hi,
On 19 October 2016 at 15:14, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/19/2016 12:29 PM, Joe Hershberger wrote:
Hi Stephen,
On Mon, Oct 17, 2016 at 1:32 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/13/2016 05:46 PM, Joe Hershberger wrote: > > > On Fri, Oct 14, 2016 at 1:35 AM, Stephen Warren > swarren@wwwdotorg.org > wrote: >> >> >> On 10/11/2016 04:48 PM, Joe Hershberger wrote: >>> >>> >>> On Tue, Oct 4, 2016 at 12:13 AM, Stephen Warren >>> swarren@wwwdotorg.org wrote: >>>> >>>> >>>> On 09/23/2016 03:49 PM, Joe Hershberger wrote: >>>>> >>>>> >>>>> On Mon, Sep 12, 2016 at 12:48 PM, Stephen Warren >>>>> swarren@wwwdotorg.org wrote: >>>>>> >>>>>> >>>>>> This driver supports the Synopsys Designware Ethernet QoS >>>>>> (Quality >>>>>> of >>>>>> Service) a/k/a eqos IP block, which is a different design than >>>>>> the >>>>>> HW >>>>>> supported by the existing designware.c driver. The IP supports >>>>>> many >>>>>> options for bus type, clocking/reset structure, and feature list. >>>>>> This >>>>>> driver currently supports the specific configuration used in >>>>>> NVIDIA's >>>>>> Tegra186 chip, but should be extensible to other combinations >>>>>> quite >>>>>> easily, as explained in the source.
...
>>>>>> >>>>>> >>>>>> +static int eqos_start(struct udevice *dev)
...
>>>>>> >>>>>> >>>>>> + /* Update the MAC address */ >>>>>> + val = (plat->enetaddr[5] << 8) | >>>>>> + (plat->enetaddr[4]); >>>>>> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_HIGH); >>>>>> + val = (plat->enetaddr[3] << 24) | >>>>>> + (plat->enetaddr[2] << 16) | >>>>>> + (plat->enetaddr[1] << 8) | >>>>>> + (plat->enetaddr[0]); >>>>>> + writel(val, eqos->regs + EQOS_MAC_ADDRESS0_LOW); >>>>> >>>>> >>>>> >>>>> This should be implemented in write_hwaddr() op.
...
>> >> >> Anyway, I still don't believe using write_hwaddr() is correct for >> this >> HW. >> It's marked optional in include/net.h; it would be implemented in >> cases >> where the MAC address should be passed to subsequent SW in Ethernet >> controller registers. That's not the case here. The master location >> for >> the MAC address is in an unrelated EEPROM that all drivers must read. > > > > That sounds more like a NV storage location for a read_rom_hwaddr() op > to get a default mac addr that can be overridden with the env.
If the EQoS HW module contained the interface to this EEPROM, such that all instances of the HW module always accessed the EEPROM in the same way and the layout of data in the EEPROM was fixed by the HW module, then yes.
However, the EqoS HW module doesn't define any mechanism for non-volatile MAC address storage; only the runtime registers. So, we can't implement read_rom_hwaddr() inside the EQoS driver unfortunately.
OK.
> The > write_hwaddr is about what the mac uses to filter for limiting packet > ingress. One reason to support it as an op is so that when the env var > for the mac address is changed, the mac filter in the hw is also > updated.
I believe that every time the Ethernet device is used, the start() op is called first, followed by packet transfer, followed by the stop() op. If start() always programs the MAC address, the driver will always end up using the value requested by the user.
That may be. I still don't understand the reluctance to implement it.
I don't want to implement it because it can't work.
write_hwaddr() is called before start() is called. At that point, clocks to the EQoS HW are not running and the EQoS HW is in reset, and hence it cannot accept any register accesses; attempting any accesses will hang the bus and CPU.
These are the possible solutions:
a) Don't implement write_hwaddr()
b) Make write_hwaddr() turn on the clock and clear the reset, program the register, then reset the device and assert the reset. Re-asserting reset is required so that setting the MAC address doesn't leave the clock running even when the device isn't in use.This is pointless since the written value will not last beyond the end of the function.
c) Make probe() start the clock and clear the reset. Then write_hwaddr() can successfully write the MAC address registers at any time. This would waste power running the clock to the device when it's not in use. Also, Simon Glass continually asks that U-Boot not initialize HW that the user hasn't attempted to use. I believe turning on the clocks in probe() violates this.
Not quite...or at least if I did I was mistaken. Of course we should limit init in probe() to what is necessary, but it is the bind() method which must not touch hardware.
It is fine to turn clocks on in probe if you want to.
Even for a device that the user never ultimately makes use of? If so, this might be a reasonable solution. It feels like U-Boot should turn off the clocks before booting an OS though so that the overall system state isn't any different between the cases where this driver is present and enabled vs not. With the clocks manipulated by start()/stop() we already do this. If the clocks are enabled in probe() instead, this won't be the case.
However I am wondering whether we have something wrong in the Ethernet uclass interface. Should we move the MAC setting to after start(), or similar?
That's option d right below.
Joe's objection to this is that for some hardware, downstream OSs expect the MAC address to be programmed into controller registers before the OS starts. This required write_hwaddr() to be called even if the user doesn't actually make use of the Ethernet device, and hence in cases where start()/stop() are never called. This is probably more common for e.g. PCI devices where the bus imposes a certain system structure including the ability to access device registers immediately after boot without manual clock programming, rather than SoC-based designs where all bets are off regarding consistency, and system configuration data is more typically passed by either out-of-band configuration data structures like DT, or via entirely custom mechanisms.
Maybe we should just make it a config option to select between the two behaviors. Or is there already something that says "This system is structured" vs "This system is a SoC / all bets are off"?
Perhaps we should make a run-time decision rather than compile-time. This will allow a single system to contain multiple instances of the EQoS HW block with different behaviour, e.g. 1 embedded in the SoC, and 1 plug-in PCIe device.
We can assume that for any PCIe device, we can implement write_hwaddr() without issue.
For devices instantiated from DT, the SoC-level integration details will drive whether there is SW control over the HW block's clock/reset signals. We can use struct udevice_id's .data field to provide parameters for each different DT compatible value that the driver supports, which can indicate how write_hwaddr() should work.
Then, we can implement roughly the following:
write_hwaddr: if (!dev->reg_access_ok && !dev->reg_access_always_ok) return 0; write MAC address registers
That should also allow write_hwaddr to operate correctly if start()/stop() are drivers are refactored not to start()/stop() for every separate network transaction, since presumably the code that sets reg_access_ok will be refactored as part of that.

On 12 September 2016 at 11:48, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
The Synopsys DWC EQoS is a configurable Ethernet MAC/DMA IP block which supports multiple options for bus type, clocking and reset structure, and feature list.
This patch imports the binding from the Linux kernel, including my V3 patch to extend the binding to cover the Tegra186, which is applied for next-20160912. So far, my changes have been acked by Lars Persson, the original author of the binding.
Signed-off-by: Stephen Warren swarren@nvidia.com
.../net/snps,dwc-qos-ethernet.txt | 166 +++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 doc/device-tree-bindings/net/snps,dwc-qos-ethernet.txt
Reviewed-by: Simon Glass sjg@chromium.org
participants (3)
-
Joe Hershberger
-
Simon Glass
-
Stephen Warren