Re: [PATCH 05/21] net-next: stmmac: Add dwmac-sun8i

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@xxxxxxxxx>
> ---
>  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
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/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
>  
>  config STMMAC_PCI
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/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)	+= dwmac-altr-socfpga.o
>  obj-$(CONFIG_DWMAC_STI)		+= dwmac-sti.o
>  obj-$(CONFIG_DWMAC_STM32)	+= dwmac-stm32.o
>  obj-$(CONFIG_DWMAC_SUNXI)	+= dwmac-sunxi.o
> +obj-$(CONFIG_DWMAC_SUN8I)	+= dwmac-sun8i.o
>  obj-$(CONFIG_DWMAC_DWC_QOS_ETH)	+= dwmac-dwc-qos-eth.o
>  obj-$(CONFIG_DWMAC_GENERIC)	+= dwmac-generic.o
>  stmmac-platform-objs:= 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 <clabbe.montjoie@xxxxxxxxx>
> + *
> + * 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 <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_mdio.h>
> +#include <linux/of_net.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
> +#include <linux/stmmac.h>
> +
> +#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 = {
> +	.default_syscon_value = 0x58000,
> +	.internal_phy = PHY_INTERFACE_MODE_MII,
> +	.support_mii = true,
> +	.support_rmii = true,
> +	.support_rgmii = true
> +};
> +
> +static const struct emac_variant emac_variant_a83t = {
> +	.default_syscon_value = 0,
> +	.internal_phy = 0,
> +	.support_mii = true,
> +	.support_rgmii = true
> +};
> +
> +static const struct emac_variant emac_variant_a64 = {
> +	.default_syscon_value = 0,
> +	.internal_phy = 0,
> +	.support_mii = true,
> +	.support_rmii = true,
> +	.support_rgmii = 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 = 0; i < 0xC8; i += 4) {
> +		if (i == 0x32 || i == 0x3C)
> +			continue;
> +		pr_err("Reg 0x%x: 0x%08x\n", i, readl(ioaddr + i));

... 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 = readl(ioaddr + EMAC_TX_CTL0);
> +	v |= EMAC_TX_TRANSMITTER_EN;
> +	writel(v, ioaddr + EMAC_TX_CTL0);
> +
> +	v = readl(ioaddr + EMAC_TX_CTL1);
> +	v |= EMAC_TX_DMA_START;
> +	v |= 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 = readl(ioaddr + EMAC_TX_CTL1);
> +	v |= EMAC_TX_DMA_START;
> +	v |= EMAC_TX_DMA_EN;
> +	writel_relaxed(v, ioaddr + EMAC_TX_CTL1);
> +}
> +
> +static void sun8i_dwmac_dma_stop_tx(void __iomem *ioaddr)
> +{
> +	u32 v;
> +
> +	v = readl(ioaddr + EMAC_TX_CTL0);
> +	v &= ~EMAC_TX_TRANSMITTER_EN;
> +	writel(v, ioaddr + EMAC_TX_CTL0);
> +
> +	v = readl(ioaddr + EMAC_TX_CTL1);
> +	v &= ~EMAC_TX_DMA_EN;
> +	writel(v, ioaddr + EMAC_TX_CTL1);
> +}
> +
> +static void sun8i_dwmac_dma_start_rx(void __iomem *ioaddr)
> +{
> +	u32 v;
> +
> +	v = readl(ioaddr + EMAC_RX_CTL0);
> +	v |= EMAC_RX_RECEIVER_EN;
> +	writel(v, ioaddr + EMAC_RX_CTL0);
> +
> +	v = readl(ioaddr + EMAC_RX_CTL1);
> +	v |= EMAC_RX_DMA_START;
> +	v |= EMAC_RX_DMA_EN;
> +	writel(v, ioaddr + EMAC_RX_CTL1);
> +}
> +
> +static void sun8i_dwmac_dma_stop_rx(void __iomem *ioaddr)
> +{
> +	u32 v;
> +
> +	v = readl(ioaddr + EMAC_RX_CTL0);
> +	v &= ~EMAC_RX_RECEIVER_EN;
> +	writel(v, ioaddr + EMAC_RX_CTL0);
> +
> +	v = readl(ioaddr + EMAC_RX_CTL1);
> +	v &= ~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 = 0;
> +
> +	v = readl(ioaddr + EMAC_INT_STA);
> +
> +	if (v & EMAC_TX_INT) {
> +		ret |= 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 |= tx_hard_error;
> +
> +	if (v & EMAC_TX_UNDERFLOW_INT) {
> +		ret |= tx_hard_error;
> +		x->tx_undeflow_irq++;
> +	}
> +
> +	if (v & EMAC_TX_EARLY_INT)
> +		x->tx_early_irq++;
> +
> +	if (v & EMAC_RX_INT) {
> +		ret |= 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 |= tx_hard_error;
> +
> +	if (v & EMAC_RX_OVERFLOW_INT) {
> +		ret |= 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 txmode,
> +					   int rxmode, int rxfifosz)
> +{
> +	u32 v;
> +
> +	v = readl(ioaddr + EMAC_TX_CTL1);
> +	if (txmode == SF_DMA_MODE) {
> +		v |= 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 |= EMAC_TX_NEXT_FRM;
> +	} else {
> +		v &= ~EMAC_TX_MD;
> +		v &= ~EMAC_TX_TH_MASK;
> +		if (txmode < 64)
> +			v |= EMAC_TX_TH_64;
> +		else if (txmode < 128)
> +			v |= EMAC_TX_TH_128;
> +		else if (txmode < 192)
> +			v |= EMAC_TX_TH_192;
> +		else if (txmode < 256)
> +			v |= EMAC_TX_TH_256;
> +	}
> +	writel(v, ioaddr + EMAC_TX_CTL1);
> +
> +	v = readl(ioaddr + EMAC_RX_CTL1);
> +	if (rxmode == SF_DMA_MODE) {
> +		v |= EMAC_RX_MD;
> +	} else {
> +		v &= ~EMAC_RX_MD;
> +		v &= ~EMAC_RX_TH_MASK;
> +		if (rxmode < 32)
> +			v |= EMAC_RX_TH_32;
> +		else if (rxmode < 64)
> +			v |= EMAC_RX_TH_64;
> +		else if (rxmode < 96)
> +			v |= EMAC_RX_TH_96;
> +		else if (rxmode < 128)
> +			v |= EMAC_RX_TH_128;
> +	}
> +	writel(v, ioaddr + EMAC_RX_CTL1);
> +}
> +
> +static const struct stmmac_dma_ops sun8i_dwmac_dma_ops = {
> +	.reset = sun8i_dwmac_dma_reset,
> +	.init = sun8i_dwmac_dma_init,
> +	.dump_regs = sun8i_dwmac_dump_regs,
> +	.dma_mode = sun8i_dwmac_dma_operation_mode,
> +	.enable_dma_transmission = sun8i_dwmac_enable_dma_transmission,
> +	.enable_dma_irq = sun8i_dwmac_enable_dma_irq,
> +	.disable_dma_irq = sun8i_dwmac_disable_dma_irq,
> +	.start_tx = sun8i_dwmac_dma_start_tx,
> +	.stop_tx = sun8i_dwmac_dma_stop_tx,
> +	.start_rx = sun8i_dwmac_dma_start_rx,
> +	.stop_rx = sun8i_dwmac_dma_stop_rx,
> +	.dma_interrupt = sun8i_dwmac_dma_interrupt,
> +};
> +
> +static int sun8i_dwmac_init(struct platform_device *pdev, void *priv)
> +{
> +	struct sunxi_priv_data *gmac = priv;
> +	int ret;
> +
> +	if (gmac->regulator) {
> +		ret = regulator_enable(gmac->regulator);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Fail to enable regulator\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = 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 = hw->pcsr;
> +	u32 v;
> +
> +	v = (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 = 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 = readl(ioaddr + EMAC_MACADDR_HI(reg_n));
> +		v |= 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 = 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 = hw->pcsr;
> +	u32 v;
> +
> +	v = readl(ioaddr + EMAC_RX_CTL0);
> +	v |= 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 = hw->pcsr;
> +	u32 v;
> +	int i = 0;
> +	struct netdev_hw_addr *ha;
> +
> +	v = readl(ioaddr + EMAC_RX_FRM_FLT);
> +
> +	v |= EMAC_FRM_FLT_CTL;
> +
> +	if (dev->flags & IFF_PROMISC) {
> +		v = EMAC_FRM_FLT_RXALL;
> +	} else if (dev->flags & IFF_ALLMULTI) {
> +		v = 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 = 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 = hw->pcsr;
> +	u32 v;
> +
> +	v = readl(ioaddr + EMAC_RX_CTL0);
> +	if (fc == FLOW_AUTO)
> +		v |= EMAC_RX_FLOW_CTL_EN;
> +	else
> +		v &= ~EMAC_RX_FLOW_CTL_EN;
> +	writel(v, ioaddr + EMAC_RX_CTL0);
> +
> +	v = readl(ioaddr + EMAC_TX_FLOW_CTL);
> +	if (fc == FLOW_AUTO)
> +		v |= EMAC_TX_FLOW_CTL_EN;
> +	else
> +		v &= ~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 = readl(priv->ioaddr + EMAC_BASIC_CTL1);
> +	writel(v | 0x01, priv->ioaddr + EMAC_BASIC_CTL1);
> +
> +	err = 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 = priv->plat->bsp_priv;
> +	struct device_node *node = priv->device->of_node;
> +	int ret;
> +	u32 reg, val;
> +
> +	reg = gmac->variant->default_syscon_value;
> +
> +	if (gmac->variant->internal_phy) {
> +		if (!gmac->use_internal_phy) {
> +			/* switch to external PHY interface */
> +			reg &= ~H3_EPHY_SELECT;
> +		} else {
> +			reg |= H3_EPHY_SELECT;
> +			reg &= ~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 |= H3_EPHY_LED_POL;
> +			else
> +				reg &= ~H3_EPHY_LED_POL;
> +
> +			ret = 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 |= 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 <= SYSCON_ETXDC_MASK) {
> +			reg &= ~(SYSCON_ETXDC_MASK << SYSCON_ETXDC_SHIFT);
> +			reg |= (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 <= SYSCON_ERXDC_MASK) {
> +			reg &= ~(SYSCON_ERXDC_MASK << SYSCON_ERXDC_SHIFT);
> +			reg |= (val << SYSCON_ERXDC_SHIFT);
> +		} else {
> +			dev_warn(priv->device, "Invalid RX clock delay: %d\n",
> +				 val);
> +		}
> +	}
> +
> +	/* Clear interface mode bits */
> +	reg &= ~(SYSCON_ETCS_MASK | SYSCON_EPIT);
> +	if (gmac->variant->support_rmii)
> +		reg &= ~SYSCON_RMII_EN;
> +
> +	switch (priv->plat->interface) {
> +	case PHY_INTERFACE_MODE_MII:
> +		/* default */
> +		break;
> +	case PHY_INTERFACE_MODE_RGMII:
> +		reg |= SYSCON_EPIT | SYSCON_ETCS_INT_GMII;
> +		break;
> +	case PHY_INTERFACE_MODE_RMII:
> +		reg |= 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 = 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 = priv->plat->bsp_priv;
> +	int ret;
> +
> +	if (gmac->ephy_clk) {
> +		ret = clk_prepare_enable(gmac->ephy_clk);
> +		if (ret) {
> +			dev_err(priv->device, "Cannot enable ephy\n");
> +			return ret;
> +		}
> +	}
> +
> +	if (gmac->rst_ephy) {
> +		ret = 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 = netdev_priv(ndev);
> +	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> +	int ret;
> +
> +	ret = sun8i_dwmac_power_internal_phy(priv);
> +	if (ret)
> +		return ret;
> +
> +	ret = sun8i_dwmac_set_syscon(priv);
> +	if (ret)
> +		goto error_phy;
> +
> +	ret = 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 = netdev_priv(ndev);
> +	struct sunxi_priv_data *gmac = 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 = priv;
> +
> +	clk_disable_unprepare(gmac->tx_clk);
> +
> +	if (gmac->regulator)
> +		regulator_disable(gmac->regulator);
> +}
> +
> +static const struct stmmac_ops sun8i_dwmac_ops = {
> +	.core_init = sun8i_dwmac_core_init,
> +	.dump_regs = sun8i_dwmac_dump_mac_regs,
> +	.rx_ipc = sun8i_dwmac_rx_ipc_enable,
> +	.set_filter = sun8i_dwmac_set_filter,
> +	.flow_ctrl = sun8i_dwmac_flow_ctrl,
> +	.set_umac_addr = sun8i_dwmac_set_umac_addr,
> +	.get_umac_addr = sun8i_dwmac_get_umac_addr,
> +	.init_phy = sun8i_power_phy,
> +	.uninit_phy = 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 = kzalloc(sizeof(const struct mac_device_info), GFP_KERNEL);
> +	if (!mac)
> +		return NULL;

Do you ever free that memory?

> +
> +	mac->pcsr = ioaddr;
> +	mac->mac = &sun8i_dwmac_ops;
> +	mac->dma = &sun8i_dwmac_dma_ops;
> +
> +	mac->link.port = 0;
> +	mac->link.duplex = BIT(0);
> +	mac->link.speed = 1;
> +	mac->mii.addr = EMAC_MDIO_CMD;
> +	mac->mii.data = EMAC_MDIO_DATA;
> +	mac->mii.reg_shift = 4;
> +	mac->mii.reg_mask = GENMASK(8, 4);
> +	mac->mii.addr_shift = 12;
> +	mac->mii.addr_mask = GENMASK(16, 12);
> +	mac->mii.clk_csr_shift = 20;
> +	mac->mii.clk_csr_mask = GENMASK(22, 20);
> +	mac->unicast_filter_entries = 8;
> +
> +	/* Synopsys Id is not available */
> +	*synopsys_id = 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 = &pdev->dev;
> +	int ret;
> +
> +	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> +	if (ret)
> +		return ret;
> +
> +	plat_dat = stmmac_probe_config_dt(pdev, &stmmac_res.mac);
> +	if (IS_ERR(plat_dat))
> +		return PTR_ERR(plat_dat);
> +
> +	gmac = devm_kzalloc(dev, sizeof(*gmac), GFP_KERNEL);
> +	if (!gmac)
> +		return -ENOMEM;
> +
> +	gmac->variant = of_device_get_match_data(&pdev->dev);
> +	if (!gmac->variant) {
> +		dev_err(&pdev->dev, "Missing sun8i-emac variant\n");
> +		return -EINVAL;
> +	}
> +
> +	gmac->tx_clk = 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 = devm_regulator_get_optional(dev, "phy");
> +	if (IS_ERR(gmac->regulator)) {
> +		if (PTR_ERR(gmac->regulator) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		dev_info(dev, "no regulator found\n");
> +		gmac->regulator = NULL;
> +	}
> +
> +	gmac->regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +						       "syscon");
> +	if (IS_ERR(gmac->regmap)) {
> +		ret = PTR_ERR(gmac->regmap);
> +		dev_err(&pdev->dev, "unable to map SYSCON:%d\n", ret);
> +		return ret;
> +	}
> +
> +	plat_dat->interface = of_get_phy_mode(dev->of_node);
> +	if (plat_dat->interface == gmac->variant->internal_phy) {
> +		dev_info(&pdev->dev, "Will use internal PHY\n");
> +		gmac->use_internal_phy = true;
> +		gmac->ephy_clk = of_clk_get(plat_dat->phy_node, 0);
> +		if (IS_ERR(gmac->ephy_clk)) {
> +			ret = PTR_ERR(gmac->ephy_clk);
> +			dev_err(&pdev->dev, "Cannot get EPHY clock err=%d\n",
> +				ret);
> +			return -EINVAL;
> +		}
> +
> +		gmac->rst_ephy = of_reset_control_get(plat_dat->phy_node, NULL);
> +		if (IS_ERR(gmac->rst_ephy)) {
> +			ret = PTR_ERR(gmac->rst_ephy);
> +			if (ret == -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 = false;
> +	}
> +
> +	/* platform data specifying hardware features and callbacks.
> +	 * hardware features were copied from Allwinner drivers.
> +	 */
> +	plat_dat->rx_coe = STMMAC_RX_COE_TYPE2;
> +	plat_dat->tx_coe = 1;
> +	plat_dat->has_sun8i = true;
> +	plat_dat->bsp_priv = gmac;
> +	plat_dat->init = sun8i_dwmac_init;
> +	plat_dat->exit = sun8i_dwmac_exit;
> +	plat_dat->setup = sun8i_dwmac_setup;
> +
> +	ret = sun8i_dwmac_init(pdev, plat_dat->bsp_priv);
> +	if (ret)
> +		return ret;
> +
> +	ret = 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[] = {
> +	{ .compatible = "allwinner,sun8i-h3-emac",
> +		.data = &emac_variant_h3 },
> +	{ .compatible = "allwinner,sun8i-a83t-emac",
> +		.data = &emac_variant_a83t },
> +	{ .compatible = "allwinner,sun50i-a64-emac",
> +		.data = &emac_variant_a64 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
> +
> +static struct platform_driver sun8i_dwmac_driver = {
> +	.probe  = sun8i_dwmac_probe,
> +	.remove = stmmac_pltfr_remove,
> +	.driver = {
> +		.name           = "sun8i-dwmac",
> +		.pm		= &stmmac_pltfr_pm_ops,
> +		.of_match_table = sun8i_dwmac_match,
> +	},
> +};
> +module_platform_driver(sun8i_dwmac_driver);
> +
> +MODULE_AUTHOR("Corentin Labbe <clabbe.montjoie@xxxxxxxxx>");
> +MODULE_DESCRIPTION("Allwinner sun8i DWMAC specific glue layer");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/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 *dev,
>  		for (i = 0; i < 22; i++)
>  			reg_space[i + 55] =
>  			    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

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux