From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756134AbdBQIkO (ORCPT ); Fri, 17 Feb 2017 03:40:14 -0500 Received: from mail.free-electrons.com ([62.4.15.54]:60971 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755755AbdBQIjK (ORCPT ); Fri, 17 Feb 2017 03:39:10 -0500 Date: Thu, 16 Feb 2017 20:05:24 +0100 From: Maxime Ripard To: Corentin Labbe Cc: peppe.cavallaro@st.com, robh+dt@kernel.org, mark.rutland@arm.com, wens@csie.org, linux@armlinux.org.uk, catalin.marinas@arm.com, will.deacon@arm.com, alexandre.torgue@st.com, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com Subject: Re: [PATCH 05/21] net-next: stmmac: Add dwmac-sun8i Message-ID: <20170216190524.l2ddzhwabzdpdphx@lukather> References: <20170216124859.14346-1-clabbe.montjoie@gmail.com> <20170216124859.14346-6-clabbe.montjoie@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="dtgarqvbtcga2mi7" Content-Disposition: inline In-Reply-To: <20170216124859.14346-6-clabbe.montjoie@gmail.com> User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --dtgarqvbtcga2mi7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Thu, Feb 16, 2017 at 01:48:43PM +0100, Corentin Labbe wrote: > The dwmac-sun8i is a heavy hacked version of stmmac hardware by > allwinner. > In fact the only common part is the descriptor management and the first > register function. >=20 > Signed-off-by: Corentin Labbe > --- > drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 + > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 892 +++++++++++++++= ++++++ > .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 3 + > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 27 +- > .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 9 +- > include/linux/stmmac.h | 1 + > 7 files changed, 941 insertions(+), 3 deletions(-) > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c >=20 > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/et= hernet/stmicro/stmmac/Kconfig > index cfbe363..85c0e41 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > @@ -145,6 +145,17 @@ config DWMAC_SUNXI > This selects Allwinner SoC glue layer support for the > stmmac device driver. This driver is used for A20/A31 > GMAC ethernet controller. > + > +config DWMAC_SUN8I > + tristate "Allwinner sun8i GMAC support" > + default ARCH_SUNXI > + depends on OF && (ARCH_SUNXI || COMPILE_TEST) > + ---help--- > + Support for Allwinner H3 A83T A64 EMAC ethernet controllers. > + > + This selects Allwinner SoC glue layer support for the > + stmmac device driver. This driver is used for H3/A83T/A64 > + EMAC ethernet controller. > endif > =20 > config STMMAC_PCI > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/e= thernet/stmicro/stmmac/Makefile > index 700c603..fd4937a 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile > @@ -16,6 +16,7 @@ obj-$(CONFIG_DWMAC_SOCFPGA) +=3D dwmac-altr-socfpga.o > obj-$(CONFIG_DWMAC_STI) +=3D dwmac-sti.o > obj-$(CONFIG_DWMAC_STM32) +=3D dwmac-stm32.o > obj-$(CONFIG_DWMAC_SUNXI) +=3D dwmac-sunxi.o > +obj-$(CONFIG_DWMAC_SUN8I) +=3D dwmac-sun8i.o > obj-$(CONFIG_DWMAC_DWC_QOS_ETH) +=3D dwmac-dwc-qos-eth.o > obj-$(CONFIG_DWMAC_GENERIC) +=3D dwmac-generic.o > stmmac-platform-objs:=3D stmmac_platform.o > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/= net/ethernet/stmicro/stmmac/dwmac-sun8i.c > new file mode 100644 > index 0000000..0951eb9 > --- /dev/null > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c > @@ -0,0 +1,892 @@ > +/* > + * dwmac-sun8i.c - Allwinner sun8i DWMAC specific glue layer > + * > + * Copyright (C) 2017 Corentin Labbe > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "stmmac.h" > +#include "stmmac_platform.h" > + > +struct emac_variant { > + u32 default_syscon_value; Why do you need a default value? Can't you read it from the syscon directly? > + int internal_phy; > + bool support_mii; > + bool support_rmii; > + bool support_rgmii; > +}; > + > +struct sunxi_priv_data { > + struct clk *tx_clk; > + struct clk *ephy_clk; > + struct regulator *regulator; > + struct reset_control *rst_ephy; > + const struct emac_variant *variant; > + bool use_internal_phy; > + struct regmap *regmap; > +}; > + > +static const struct emac_variant emac_variant_h3 =3D { > + .default_syscon_value =3D 0x58000, > + .internal_phy =3D PHY_INTERFACE_MODE_MII, > + .support_mii =3D true, > + .support_rmii =3D true, > + .support_rgmii =3D true > +}; > + > +static const struct emac_variant emac_variant_a83t =3D { > + .default_syscon_value =3D 0, > + .internal_phy =3D 0, > + .support_mii =3D true, > + .support_rgmii =3D true > +}; > + > +static const struct emac_variant emac_variant_a64 =3D { > + .default_syscon_value =3D 0, > + .internal_phy =3D 0, > + .support_mii =3D true, > + .support_rmii =3D true, > + .support_rgmii =3D true > +}; > + > +#define EMAC_BASIC_CTL0 0x00 > +#define EMAC_BASIC_CTL1 0x04 > +#define EMAC_INT_STA 0x08 > +#define EMAC_INT_EN 0x0C > +#define EMAC_TX_CTL0 0x10 > +#define EMAC_TX_CTL1 0x14 > +#define EMAC_TX_FLOW_CTL 0x1C > +#define EMAC_TX_DESC_LIST 0x20 > +#define EMAC_RX_CTL0 0x24 > +#define EMAC_RX_CTL1 0x28 > +#define EMAC_RX_DESC_LIST 0x34 > +#define EMAC_RX_FRM_FLT 0x38 > +#define EMAC_MDIO_CMD 0x48 > +#define EMAC_MDIO_DATA 0x4C > +#define EMAC_MACADDR_HI(reg) (0x50 + (reg) * 8) > +#define EMAC_MACADDR_LO(reg) (0x54 + (reg) * 8) > +#define EMAC_TX_DMA_STA 0xB0 > +#define EMAC_TX_CUR_DESC 0xB4 > +#define EMAC_TX_CUR_BUF 0xB8 > +#define EMAC_RX_DMA_STA 0xC0 > +#define EMAC_RX_CUR_DESC 0xC4 > +#define EMAC_RX_CUR_BUF 0xC8 > + > +/* Used in EMAC_RX_FRM_FLT */ > +#define EMAC_FRM_FLT_RXALL BIT(0) > +#define EMAC_FRM_FLT_CTL BIT(13) > +#define EMAC_FRM_FLT_MULTICAST BIT(16) > + > +/* Used in RX_CTL1*/ > +#define EMAC_RX_MD BIT(1) > +#define EMAC_RX_TH_MASK GENMASK(4, 5) > +#define EMAC_RX_TH_32 0 > +#define EMAC_RX_TH_64 (0x1 << 4) > +#define EMAC_RX_TH_96 (0x2 << 4) > +#define EMAC_RX_TH_128 (0x3 << 4) > +#define EMAC_RX_DMA_EN BIT(30) > +#define EMAC_RX_DMA_START BIT(31) > + > +/* Used in TX_CTL1*/ > +#define EMAC_TX_MD BIT(1) > +#define EMAC_TX_NEXT_FRM BIT(2) > +#define EMAC_TX_TH_MASK GENMASK(8, 10) > +#define EMAC_TX_TH_64 0 > +#define EMAC_TX_TH_128 (0x1 << 8) > +#define EMAC_TX_TH_192 (0x2 << 8) > +#define EMAC_TX_TH_256 (0x3 << 8) > +#define EMAC_TX_DMA_EN BIT(30) > +#define EMAC_TX_DMA_START BIT(31) > + > +/* Used in RX_CTL0 */ > +#define EMAC_RX_RECEIVER_EN BIT(31) > +#define EMAC_RX_DO_CRC BIT(27) > +#define EMAC_RX_FLOW_CTL_EN BIT(16) > + > +/* Used in TX_CTL0 */ > +#define EMAC_TX_TRANSMITTER_EN BIT(31) > + > +/* Used in EMAC_TX_FLOW_CTL */ > +#define EMAC_TX_FLOW_CTL_EN BIT(0) > + > +/* Used in EMAC_INT_STA */ > +#define EMAC_TX_INT BIT(0) > +#define EMAC_TX_DMA_STOP_INT BIT(1) > +#define EMAC_TX_BUF_UA_INT BIT(2) > +#define EMAC_TX_TIMEOUT_INT BIT(3) > +#define EMAC_TX_UNDERFLOW_INT BIT(4) > +#define EMAC_TX_EARLY_INT BIT(5) > +#define EMAC_RX_INT BIT(8) > +#define EMAC_RX_BUF_UA_INT BIT(9) > +#define EMAC_RX_DMA_STOP_INT BIT(10) > +#define EMAC_RX_TIMEOUT_INT BIT(11) > +#define EMAC_RX_OVERFLOW_INT BIT(12) > +#define EMAC_RX_EARLY_INT BIT(13) > +#define EMAC_RGMII_STA_INT BIT(16) > + > +#define MAC_ADDR_TYPE_DST BIT(31) > + > +/* H3 specific bits for EPHY */ > +#define H3_EPHY_ADDR_SHIFT 20 > +#define H3_EPHY_LED_POL BIT(17) /* 1: active low, 0: active high */ > +#define H3_EPHY_SHUTDOWN BIT(16) /* 1: shutdown, 0: power up */ > +#define H3_EPHY_SELECT BIT(15) /* 1: internal PHY, 0: external PHY */ > + > +/* H3/A64 specific bits */ > +#define SYSCON_RMII_EN BIT(13) /* 1: enable RMII (overrides EPIT) */ > + > +/* Generic system control EMAC_CLK bits */ > +#define SYSCON_ETXDC_MASK GENMASK(2, 0) > +#define SYSCON_ETXDC_SHIFT 10 > +#define SYSCON_ERXDC_MASK GENMASK(4, 0) > +#define SYSCON_ERXDC_SHIFT 5 > +/* EMAC PHY Interface Type */ > +#define SYSCON_EPIT BIT(2) /* 1: RGMII, 0: MII */ > +#define SYSCON_ETCS_MASK GENMASK(1, 0) > +#define SYSCON_ETCS_MII 0x0 > +#define SYSCON_ETCS_EXT_GMII 0x1 > +#define SYSCON_ETCS_INT_GMII 0x2 > +#define SYSCON_EMAC_REG 0x30 > + > +static int sun8i_dwmac_dma_reset(void __iomem *ioaddr) > +{ > + writel(0, ioaddr + EMAC_RX_CTL1); > + writel(0, ioaddr + EMAC_TX_CTL1); > + writel(0, ioaddr + EMAC_RX_FRM_FLT); > + writel(0, ioaddr + EMAC_RX_DESC_LIST); > + writel(0, ioaddr + EMAC_TX_DESC_LIST); > + writel(0, ioaddr + EMAC_INT_EN); > + writel(0x1FFFFFF, ioaddr + EMAC_INT_STA); > + return 0; > +} > + > +static void sun8i_dwmac_dma_init(void __iomem *ioaddr, > + struct stmmac_dma_cfg *dma_cfg, > + u32 dma_tx, u32 dma_rx, int atds) > +{ > + /* write tx and rx descs*/ > + writel(dma_rx, ioaddr + EMAC_RX_DESC_LIST); > + writel(dma_tx, ioaddr + EMAC_TX_DESC_LIST); > + > + writel(EMAC_RX_INT | EMAC_TX_INT, ioaddr + EMAC_INT_EN); > + writel(0x1FFFFFF, ioaddr + EMAC_INT_STA); > +} > + > +static void sun8i_dwmac_dump_regs(void __iomem *ioaddr) > +{ > + int i; > + > + pr_info(" DMA registers\n"); Logging this as pr_info is bad already... > + for (i =3D 0; i < 0xC8; i +=3D 4) { > + if (i =3D=3D 0x32 || i =3D=3D 0x3C) > + continue; > + pr_err("Reg 0x%x: 0x%08x\n", i, readl(ioaddr + i)); =2E.. But this is worse. Why do you need to do that? Can't you create a file in debugfs instead? > + } > +} > + > +static void sun8i_dwmac_dump_mac_regs(struct mac_device_info *hw) > +{ > + sun8i_dwmac_dump_regs(hw->pcsr); > +} > + > +static void sun8i_dwmac_enable_dma_irq(void __iomem *ioaddr) > +{ > + writel(EMAC_RX_INT | EMAC_TX_INT, ioaddr + EMAC_INT_EN); > +} > + > +static void sun8i_dwmac_disable_dma_irq(void __iomem *ioaddr) > +{ > + writel(0, ioaddr + EMAC_INT_EN); > +} > + > +static void sun8i_dwmac_dma_start_tx(void __iomem *ioaddr) > +{ > + u32 v; > + > + v =3D readl(ioaddr + EMAC_TX_CTL0); > + v |=3D EMAC_TX_TRANSMITTER_EN; > + writel(v, ioaddr + EMAC_TX_CTL0); > + > + v =3D readl(ioaddr + EMAC_TX_CTL1); > + v |=3D EMAC_TX_DMA_START; > + v |=3D EMAC_TX_DMA_EN; > + writel(v, ioaddr + EMAC_TX_CTL1); This is a bit worrying. There's not a single lock in your driver, while you have a significant number of read / modify / write. Where is the locking handled? > +} > + > +static void sun8i_dwmac_enable_dma_transmission(void __iomem *ioaddr) > +{ > + u32 v; > + > + v =3D readl(ioaddr + EMAC_TX_CTL1); > + v |=3D EMAC_TX_DMA_START; > + v |=3D EMAC_TX_DMA_EN; > + writel_relaxed(v, ioaddr + EMAC_TX_CTL1); > +} > + > +static void sun8i_dwmac_dma_stop_tx(void __iomem *ioaddr) > +{ > + u32 v; > + > + v =3D readl(ioaddr + EMAC_TX_CTL0); > + v &=3D ~EMAC_TX_TRANSMITTER_EN; > + writel(v, ioaddr + EMAC_TX_CTL0); > + > + v =3D readl(ioaddr + EMAC_TX_CTL1); > + v &=3D ~EMAC_TX_DMA_EN; > + writel(v, ioaddr + EMAC_TX_CTL1); > +} > + > +static void sun8i_dwmac_dma_start_rx(void __iomem *ioaddr) > +{ > + u32 v; > + > + v =3D readl(ioaddr + EMAC_RX_CTL0); > + v |=3D EMAC_RX_RECEIVER_EN; > + writel(v, ioaddr + EMAC_RX_CTL0); > + > + v =3D readl(ioaddr + EMAC_RX_CTL1); > + v |=3D EMAC_RX_DMA_START; > + v |=3D EMAC_RX_DMA_EN; > + writel(v, ioaddr + EMAC_RX_CTL1); > +} > + > +static void sun8i_dwmac_dma_stop_rx(void __iomem *ioaddr) > +{ > + u32 v; > + > + v =3D readl(ioaddr + EMAC_RX_CTL0); > + v &=3D ~EMAC_RX_RECEIVER_EN; > + writel(v, ioaddr + EMAC_RX_CTL0); > + > + v =3D readl(ioaddr + EMAC_RX_CTL1); > + v &=3D ~EMAC_RX_DMA_EN; > + writel(v, ioaddr + EMAC_RX_CTL1); > +} > + > +static int sun8i_dwmac_dma_interrupt(void __iomem *ioaddr, > + struct stmmac_extra_stats *x) > +{ > + u32 v; > + int ret =3D 0; > + > + v =3D readl(ioaddr + EMAC_INT_STA); > + > + if (v & EMAC_TX_INT) { > + ret |=3D handle_tx; > + x->tx_normal_irq_n++; > + } > + > + if (v & EMAC_TX_DMA_STOP_INT) > + x->tx_process_stopped_irq++; > + > + if (v & EMAC_TX_BUF_UA_INT) > + x->tx_process_stopped_irq++; > + > + if (v & EMAC_TX_TIMEOUT_INT) > + ret |=3D tx_hard_error; > + > + if (v & EMAC_TX_UNDERFLOW_INT) { > + ret |=3D tx_hard_error; > + x->tx_undeflow_irq++; > + } > + > + if (v & EMAC_TX_EARLY_INT) > + x->tx_early_irq++; > + > + if (v & EMAC_RX_INT) { > + ret |=3D handle_rx; > + x->rx_normal_irq_n++; > + } > + > + if (v & EMAC_RX_BUF_UA_INT) > + x->rx_buf_unav_irq++; > + > + if (v & EMAC_RX_DMA_STOP_INT) > + x->rx_process_stopped_irq++; > + > + if (v & EMAC_RX_TIMEOUT_INT) > + ret |=3D tx_hard_error; > + > + if (v & EMAC_RX_OVERFLOW_INT) { > + ret |=3D tx_hard_error; > + x->rx_overflow_irq++; > + } > + > + if (v & EMAC_RX_EARLY_INT) > + x->rx_early_irq++; > + > + if (v & EMAC_RGMII_STA_INT) > + x->irq_rgmii_n++; > + > + writel(v, ioaddr + EMAC_INT_STA); > + > + return ret; > +} > + > +static void sun8i_dwmac_dma_operation_mode(void __iomem *ioaddr, int txm= ode, > + int rxmode, int rxfifosz) > +{ > + u32 v; > + > + v =3D readl(ioaddr + EMAC_TX_CTL1); > + if (txmode =3D=3D SF_DMA_MODE) { > + v |=3D EMAC_TX_MD; > + /* Undocumented bit (called TX_NEXT_FRM in BSP), the original > + * comment is > + * "Operating on second frame increase the performance > + * especially when transmit store-and-forward is used." > + */ > + v |=3D EMAC_TX_NEXT_FRM; > + } else { > + v &=3D ~EMAC_TX_MD; > + v &=3D ~EMAC_TX_TH_MASK; > + if (txmode < 64) > + v |=3D EMAC_TX_TH_64; > + else if (txmode < 128) > + v |=3D EMAC_TX_TH_128; > + else if (txmode < 192) > + v |=3D EMAC_TX_TH_192; > + else if (txmode < 256) > + v |=3D EMAC_TX_TH_256; > + } > + writel(v, ioaddr + EMAC_TX_CTL1); > + > + v =3D readl(ioaddr + EMAC_RX_CTL1); > + if (rxmode =3D=3D SF_DMA_MODE) { > + v |=3D EMAC_RX_MD; > + } else { > + v &=3D ~EMAC_RX_MD; > + v &=3D ~EMAC_RX_TH_MASK; > + if (rxmode < 32) > + v |=3D EMAC_RX_TH_32; > + else if (rxmode < 64) > + v |=3D EMAC_RX_TH_64; > + else if (rxmode < 96) > + v |=3D EMAC_RX_TH_96; > + else if (rxmode < 128) > + v |=3D EMAC_RX_TH_128; > + } > + writel(v, ioaddr + EMAC_RX_CTL1); > +} > + > +static const struct stmmac_dma_ops sun8i_dwmac_dma_ops =3D { > + .reset =3D sun8i_dwmac_dma_reset, > + .init =3D sun8i_dwmac_dma_init, > + .dump_regs =3D sun8i_dwmac_dump_regs, > + .dma_mode =3D sun8i_dwmac_dma_operation_mode, > + .enable_dma_transmission =3D sun8i_dwmac_enable_dma_transmission, > + .enable_dma_irq =3D sun8i_dwmac_enable_dma_irq, > + .disable_dma_irq =3D sun8i_dwmac_disable_dma_irq, > + .start_tx =3D sun8i_dwmac_dma_start_tx, > + .stop_tx =3D sun8i_dwmac_dma_stop_tx, > + .start_rx =3D sun8i_dwmac_dma_start_rx, > + .stop_rx =3D sun8i_dwmac_dma_stop_rx, > + .dma_interrupt =3D sun8i_dwmac_dma_interrupt, > +}; > + > +static int sun8i_dwmac_init(struct platform_device *pdev, void *priv) > +{ > + struct sunxi_priv_data *gmac =3D priv; > + int ret; > + > + if (gmac->regulator) { > + ret =3D regulator_enable(gmac->regulator); > + if (ret) { > + dev_err(&pdev->dev, "Fail to enable regulator\n"); > + return ret; > + } > + } > + > + ret =3D clk_prepare_enable(gmac->tx_clk); > + if (ret) { > + dev_err(&pdev->dev, "Could not enable AHB clock\n"); If that call fails, you leave the regulator (if there was any) enabled. > + return ret; > + } > + > + return 0; > +} > + > +static void sun8i_dwmac_core_init(struct mac_device_info *hw, int mtu) > +{ > + void __iomem *ioaddr =3D hw->pcsr; > + u32 v; > + > + v =3D (8 << 24);/* burst len */ > + writel(v, ioaddr + EMAC_BASIC_CTL1); do you need an intermediate value? you should make a define for that too. > +} > + > +static void sun8i_dwmac_set_umac_addr(struct mac_device_info *hw, > + unsigned char *addr, > + unsigned int reg_n) > +{ > + void __iomem *ioaddr =3D hw->pcsr; > + u32 v; > + > + stmmac_set_mac_addr(ioaddr, addr, EMAC_MACADDR_HI(reg_n), > + EMAC_MACADDR_LO(reg_n)); > + if (reg_n > 0) { > + v =3D readl(ioaddr + EMAC_MACADDR_HI(reg_n)); > + v |=3D MAC_ADDR_TYPE_DST; > + writel(v, ioaddr + EMAC_MACADDR_HI(reg_n)); > + } > +} > + > +static void sun8i_dwmac_get_umac_addr(struct mac_device_info *hw, > + unsigned char *addr, > + unsigned int reg_n) > +{ > + void __iomem *ioaddr =3D hw->pcsr; > + > + stmmac_get_mac_addr(ioaddr, addr, EMAC_MACADDR_HI(reg_n), > + EMAC_MACADDR_LO(reg_n)); > +} > + > +/* caution this function must return non 0 to work */ > +static int sun8i_dwmac_rx_ipc_enable(struct mac_device_info *hw) > +{ > + void __iomem *ioaddr =3D hw->pcsr; > + u32 v; > + > + v =3D readl(ioaddr + EMAC_RX_CTL0); > + v |=3D EMAC_RX_DO_CRC; > + writel(v, ioaddr + EMAC_RX_CTL0); > + > + return 1; > +} > + > +static void sun8i_dwmac_set_filter(struct mac_device_info *hw, > + struct net_device *dev) > +{ > + void __iomem *ioaddr =3D hw->pcsr; > + u32 v; > + int i =3D 0; > + struct netdev_hw_addr *ha; > + > + v =3D readl(ioaddr + EMAC_RX_FRM_FLT); > + > + v |=3D EMAC_FRM_FLT_CTL; > + > + if (dev->flags & IFF_PROMISC) { > + v =3D EMAC_FRM_FLT_RXALL; > + } else if (dev->flags & IFF_ALLMULTI) { > + v =3D EMAC_FRM_FLT_MULTICAST; > + } else if (!netdev_mc_empty(dev)) { > + netdev_for_each_mc_addr(ha, dev) { > + i++; > + sun8i_dwmac_set_umac_addr(hw, ha->addr, i); > + } > + } > + > + if (netdev_uc_count(dev) + i > hw->unicast_filter_entries) { > + netdev_info(dev, "Too many address, switching to promiscuous\n"); > + v =3D EMAC_FRM_FLT_RXALL; > + } else { > + netdev_for_each_uc_addr(ha, dev) { > + i++; > + sun8i_dwmac_set_umac_addr(hw, ha->addr, i); > + } > + } > + writel(v, ioaddr + EMAC_RX_FRM_FLT); > +} > + > +static void sun8i_dwmac_flow_ctrl(struct mac_device_info *hw, > + unsigned int duplex, > + unsigned int fc, unsigned int pause_time) > +{ > + void __iomem *ioaddr =3D hw->pcsr; > + u32 v; > + > + v =3D readl(ioaddr + EMAC_RX_CTL0); > + if (fc =3D=3D FLOW_AUTO) > + v |=3D EMAC_RX_FLOW_CTL_EN; > + else > + v &=3D ~EMAC_RX_FLOW_CTL_EN; > + writel(v, ioaddr + EMAC_RX_CTL0); > + > + v =3D readl(ioaddr + EMAC_TX_FLOW_CTL); > + if (fc =3D=3D FLOW_AUTO) > + v |=3D EMAC_TX_FLOW_CTL_EN; > + else > + v &=3D ~EMAC_TX_FLOW_CTL_EN; > + writel(v, ioaddr + EMAC_TX_FLOW_CTL); > +} > + > +static int sun8i_dwmac_reset(struct stmmac_priv *priv) > +{ > + u32 v; > + int err; > + > + v =3D readl(priv->ioaddr + EMAC_BASIC_CTL1); > + writel(v | 0x01, priv->ioaddr + EMAC_BASIC_CTL1); > + > + err =3D readl_poll_timeout(priv->ioaddr + EMAC_BASIC_CTL1, v, > + !(v & 0x01), 100, 10000); > + > + if (err) { > + dev_err(priv->device, "EMAC reset timeout\n"); > + return -EFAULT; > + } > + return 0; > +} > + > +static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv) > +{ > + struct sunxi_priv_data *gmac =3D priv->plat->bsp_priv; > + struct device_node *node =3D priv->device->of_node; > + int ret; > + u32 reg, val; > + > + reg =3D gmac->variant->default_syscon_value; > + > + if (gmac->variant->internal_phy) { > + if (!gmac->use_internal_phy) { > + /* switch to external PHY interface */ > + reg &=3D ~H3_EPHY_SELECT; > + } else { > + reg |=3D H3_EPHY_SELECT; > + reg &=3D ~H3_EPHY_SHUTDOWN; > + dev_info(priv->device, "Select internal_phy %x\n", reg); The logging level is too high > + > + if (of_property_read_bool(priv->plat->phy_node, > + "allwinner,leds-active-low")) > + reg |=3D H3_EPHY_LED_POL; > + else > + reg &=3D ~H3_EPHY_LED_POL; > + > + ret =3D of_mdio_parse_addr(priv->device, > + priv->plat->phy_node); > + if (ret < 0) { > + dev_err(priv->device, "Could not parse MDIO addr\n"); > + return ret; > + } > + /* of_mdio_parse_addr returns a valid (0 ~ 31) PHY > + * address. No need to mask it again. > + */ > + reg |=3D ret << H3_EPHY_ADDR_SHIFT; > + } > + } > + > + if (!of_property_read_u32(node, "allwinner,tx-delay", &val)) { How do you compute it? Can't this be done through auto-training? > + dev_info(priv->device, "set tx-delay to %x\n", val); change the logging level here too. > + if (val <=3D SYSCON_ETXDC_MASK) { > + reg &=3D ~(SYSCON_ETXDC_MASK << SYSCON_ETXDC_SHIFT); > + reg |=3D (val << SYSCON_ETXDC_SHIFT); > + } else { > + dev_warn(priv->device, "Invalid TX clock delay: %d\n", > + val); If it's invalid, why don't you treat it as an error and return? > + } > + } > + > + if (!of_property_read_u32(node, "allwinner,rx-delay", &val)) { > + dev_info(priv->device, "set rx-delay to %x\n", val); > + if (val <=3D SYSCON_ERXDC_MASK) { > + reg &=3D ~(SYSCON_ERXDC_MASK << SYSCON_ERXDC_SHIFT); > + reg |=3D (val << SYSCON_ERXDC_SHIFT); > + } else { > + dev_warn(priv->device, "Invalid RX clock delay: %d\n", > + val); > + } > + } > + > + /* Clear interface mode bits */ > + reg &=3D ~(SYSCON_ETCS_MASK | SYSCON_EPIT); > + if (gmac->variant->support_rmii) > + reg &=3D ~SYSCON_RMII_EN; > + > + switch (priv->plat->interface) { > + case PHY_INTERFACE_MODE_MII: > + /* default */ > + break; > + case PHY_INTERFACE_MODE_RGMII: > + reg |=3D SYSCON_EPIT | SYSCON_ETCS_INT_GMII; > + break; > + case PHY_INTERFACE_MODE_RMII: > + reg |=3D SYSCON_RMII_EN | SYSCON_ETCS_EXT_GMII; > + break; > + default: > + dev_err(priv->device, "Unsupported interface mode: %s", > + phy_modes(priv->plat->interface)); > + return -EINVAL; > + } > + > + regmap_write(gmac->regmap, SYSCON_EMAC_REG, reg); > + > + return 0; > +} > + > +static void sun8i_dwmac_unset_syscon(struct sunxi_priv_data *gmac) > +{ > + u32 reg =3D gmac->variant->default_syscon_value; > + > + regmap_write(gmac->regmap, SYSCON_EMAC_REG, reg); > +} > + > +static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv) > +{ > + struct sunxi_priv_data *gmac =3D priv->plat->bsp_priv; > + int ret; > + > + if (gmac->ephy_clk) { > + ret =3D clk_prepare_enable(gmac->ephy_clk); > + if (ret) { > + dev_err(priv->device, "Cannot enable ephy\n"); > + return ret; > + } > + } > + > + if (gmac->rst_ephy) { > + ret =3D reset_control_deassert(gmac->rst_ephy); > + if (ret) { > + dev_err(priv->device, "Cannot deassert ephy\n"); > + clk_disable_unprepare(gmac->ephy_clk); > + return ret; > + } > + } > + > + return 0; > +} > + > +static int sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac) > +{ > + if (gmac->ephy_clk) > + clk_disable_unprepare(gmac->ephy_clk); > + if (gmac->rst_ephy) > + reset_control_assert(gmac->rst_ephy); > + return 0; > +} > + > +static int sun8i_power_phy(struct net_device *ndev) > +{ > + struct stmmac_priv *priv =3D netdev_priv(ndev); > + struct sunxi_priv_data *gmac =3D priv->plat->bsp_priv; > + int ret; > + > + ret =3D sun8i_dwmac_power_internal_phy(priv); > + if (ret) > + return ret; > + > + ret =3D sun8i_dwmac_set_syscon(priv); > + if (ret) > + goto error_phy; > + > + ret =3D sun8i_dwmac_reset(priv); > + if (ret) > + goto error_phy; > + return 0; > + > +error_phy: > + sun8i_dwmac_unset_syscon(gmac); > + sun8i_dwmac_unpower_internal_phy(gmac); > + return ret; > +} > + > +static void sun8i_unpower_phy(struct net_device *ndev) > +{ > + struct stmmac_priv *priv =3D netdev_priv(ndev); > + struct sunxi_priv_data *gmac =3D priv->plat->bsp_priv; > + > + sun8i_dwmac_unset_syscon(gmac); > + sun8i_dwmac_unpower_internal_phy(gmac); > +} > + > +static void sun8i_dwmac_exit(struct platform_device *pdev, void *priv) > +{ > + struct sunxi_priv_data *gmac =3D priv; > + > + clk_disable_unprepare(gmac->tx_clk); > + > + if (gmac->regulator) > + regulator_disable(gmac->regulator); > +} > + > +static const struct stmmac_ops sun8i_dwmac_ops =3D { > + .core_init =3D sun8i_dwmac_core_init, > + .dump_regs =3D sun8i_dwmac_dump_mac_regs, > + .rx_ipc =3D sun8i_dwmac_rx_ipc_enable, > + .set_filter =3D sun8i_dwmac_set_filter, > + .flow_ctrl =3D sun8i_dwmac_flow_ctrl, > + .set_umac_addr =3D sun8i_dwmac_set_umac_addr, > + .get_umac_addr =3D sun8i_dwmac_get_umac_addr, > + .init_phy =3D sun8i_power_phy, > + .uninit_phy =3D sun8i_unpower_phy, > +}; > + > +static struct mac_device_info *sun8i_dwmac_setup(void __iomem *ioaddr, > + int mcbins, > + int perfect_uc_entries, > + int *synopsys_id) > +{ > + struct mac_device_info *mac; > + > + mac =3D kzalloc(sizeof(const struct mac_device_info), GFP_KERNEL); > + if (!mac) > + return NULL; Do you ever free that memory? > + > + mac->pcsr =3D ioaddr; > + mac->mac =3D &sun8i_dwmac_ops; > + mac->dma =3D &sun8i_dwmac_dma_ops; > + > + mac->link.port =3D 0; > + mac->link.duplex =3D BIT(0); > + mac->link.speed =3D 1; > + mac->mii.addr =3D EMAC_MDIO_CMD; > + mac->mii.data =3D EMAC_MDIO_DATA; > + mac->mii.reg_shift =3D 4; > + mac->mii.reg_mask =3D GENMASK(8, 4); > + mac->mii.addr_shift =3D 12; > + mac->mii.addr_mask =3D GENMASK(16, 12); > + mac->mii.clk_csr_shift =3D 20; > + mac->mii.clk_csr_mask =3D GENMASK(22, 20); > + mac->unicast_filter_entries =3D 8; > + > + /* Synopsys Id is not available */ > + *synopsys_id =3D 0; > + > + return mac; > +} > + > +static int sun8i_dwmac_probe(struct platform_device *pdev) > +{ > + struct plat_stmmacenet_data *plat_dat; > + struct stmmac_resources stmmac_res; > + struct sunxi_priv_data *gmac; > + struct device *dev =3D &pdev->dev; > + int ret; > + > + ret =3D stmmac_get_platform_resources(pdev, &stmmac_res); > + if (ret) > + return ret; > + > + plat_dat =3D stmmac_probe_config_dt(pdev, &stmmac_res.mac); > + if (IS_ERR(plat_dat)) > + return PTR_ERR(plat_dat); > + > + gmac =3D devm_kzalloc(dev, sizeof(*gmac), GFP_KERNEL); > + if (!gmac) > + return -ENOMEM; > + > + gmac->variant =3D of_device_get_match_data(&pdev->dev); > + if (!gmac->variant) { > + dev_err(&pdev->dev, "Missing sun8i-emac variant\n"); > + return -EINVAL; > + } > + > + gmac->tx_clk =3D devm_clk_get(dev, "stmmaceth"); > + if (IS_ERR(gmac->tx_clk)) { > + dev_err(dev, "could not get tx clock\n"); > + return PTR_ERR(gmac->tx_clk); > + } > + > + /* Optional regulator for PHY */ > + gmac->regulator =3D devm_regulator_get_optional(dev, "phy"); > + if (IS_ERR(gmac->regulator)) { > + if (PTR_ERR(gmac->regulator) =3D=3D -EPROBE_DEFER) > + return -EPROBE_DEFER; > + dev_info(dev, "no regulator found\n"); > + gmac->regulator =3D NULL; > + } > + > + gmac->regmap =3D syscon_regmap_lookup_by_phandle(pdev->dev.of_node, > + "syscon"); > + if (IS_ERR(gmac->regmap)) { > + ret =3D PTR_ERR(gmac->regmap); > + dev_err(&pdev->dev, "unable to map SYSCON:%d\n", ret); > + return ret; > + } > + > + plat_dat->interface =3D of_get_phy_mode(dev->of_node); > + if (plat_dat->interface =3D=3D gmac->variant->internal_phy) { > + dev_info(&pdev->dev, "Will use internal PHY\n"); > + gmac->use_internal_phy =3D true; > + gmac->ephy_clk =3D of_clk_get(plat_dat->phy_node, 0); > + if (IS_ERR(gmac->ephy_clk)) { > + ret =3D PTR_ERR(gmac->ephy_clk); > + dev_err(&pdev->dev, "Cannot get EPHY clock err=3D%d\n", > + ret); > + return -EINVAL; > + } > + > + gmac->rst_ephy =3D of_reset_control_get(plat_dat->phy_node, NULL); > + if (IS_ERR(gmac->rst_ephy)) { > + ret =3D PTR_ERR(gmac->rst_ephy); > + if (ret =3D=3D -EPROBE_DEFER) > + return ret; > + dev_err(&pdev->dev, "No EPHY reset control found %d\n", > + ret); > + return -EINVAL; > + } > + } else { > + dev_info(&pdev->dev, "Will use external PHY\n"); > + gmac->use_internal_phy =3D false; > + } > + > + /* platform data specifying hardware features and callbacks. > + * hardware features were copied from Allwinner drivers. > + */ > + plat_dat->rx_coe =3D STMMAC_RX_COE_TYPE2; > + plat_dat->tx_coe =3D 1; > + plat_dat->has_sun8i =3D true; > + plat_dat->bsp_priv =3D gmac; > + plat_dat->init =3D sun8i_dwmac_init; > + plat_dat->exit =3D sun8i_dwmac_exit; > + plat_dat->setup =3D sun8i_dwmac_setup; > + > + ret =3D sun8i_dwmac_init(pdev, plat_dat->bsp_priv); > + if (ret) > + return ret; > + > + ret =3D stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res); > + if (ret) > + sun8i_dwmac_exit(pdev, plat_dat->bsp_priv); > + > + return ret; > +} > + > +static const struct of_device_id sun8i_dwmac_match[] =3D { > + { .compatible =3D "allwinner,sun8i-h3-emac", > + .data =3D &emac_variant_h3 }, > + { .compatible =3D "allwinner,sun8i-a83t-emac", > + .data =3D &emac_variant_a83t }, > + { .compatible =3D "allwinner,sun50i-a64-emac", > + .data =3D &emac_variant_a64 }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, sun8i_dwmac_match); > + > +static struct platform_driver sun8i_dwmac_driver =3D { > + .probe =3D sun8i_dwmac_probe, > + .remove =3D stmmac_pltfr_remove, > + .driver =3D { > + .name =3D "sun8i-dwmac", > + .pm =3D &stmmac_pltfr_pm_ops, > + .of_match_table =3D sun8i_dwmac_match, > + }, > +}; > +module_platform_driver(sun8i_dwmac_driver); > + > +MODULE_AUTHOR("Corentin Labbe "); > +MODULE_DESCRIPTION("Allwinner sun8i DWMAC specific glue layer"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drive= rs/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > index 5ff6bc4..11db658 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > @@ -450,6 +450,9 @@ static void stmmac_ethtool_gregs(struct net_device *d= ev, > for (i =3D 0; i < 22; i++) > reg_space[i + 55] =3D > readl(priv->ioaddr + (DMA_BUS_MODE + (i * 4))); > + } else if (priv->plat->has_sun8i) { Surely we don't want to add a new flag to the common structure for every new platform supported. Can't you base that on the compatible instead? Thanks a lot for your work, Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --dtgarqvbtcga2mi7 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJYpffwAAoJEBx+YmzsjxAgfCEP/j6vCcvtu4zVrqS07EQNlXEQ qUswDnvWUEFoP24rhlLs+hftRr8zpRU9+ksT9yNDQvg2ESpzUYp8lV4p2glx7Tde IcKogFbagh3GQF4lLTD+fY7rI0q1yxE/9LwTqEd1GiwdWbGafsxoDItvPBsQ9miO C7kVohKE/Y7FzHwLN4ejMKTZYzmme4fTeXWLY3sSXqYLns0bjPD+SC58Ik7Qtbbc bfmJDEsJcE7AWyuqfAihrloy9cvdIY4tYEqjjuUKF4c41XAeLp+6elm+LAIU8JdQ 6gZ2gDB8cDE8V8vAGGdQ7S7h7cK8enUtxgDufrKZ2IlPA56QyGHF/ZVqd0ybE6P1 WryUCay6MqVqCzDibynK6ryK9yNvt8uHjqJoOM/zl7Mzz/lbb9hvvgQ9s2OV+tub fl+v9xHYG3sQt1GkebQuW9FQcI7rjg/XCvqqqOfZynagU3a4IUCh+qbu/hmow/Sg lkLlWXYe1g2Pm9f0S3L8u3CFiTIZtkdukq3BKFUzPwVPHWNUGssb5Yy3rXvXVN8a xKneVtQKRY9BDvkS79gpHVt3gHZf1D6HoymDYr+1CapXGuHC6N2OUVBlnD3lb/W3 mg1obVU3kahlqzH+bq0RF/EgVmVPAZ/FAMdmZFha/VUiojKSojRnNyhT1b+xQS6G vy4WVWvYEZbrOsWDdXnW =ETvb -----END PGP SIGNATURE----- --dtgarqvbtcga2mi7--