Re: [PATCH v2 3/4] net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC

[Martin Blumenstingl]

Hi Joachim,

On Sat, Aug 20, 2016 at 11:29 PM, Joachim  Eastwood  wrote:
>> +   platform_set_drvdata(pdev, dwmac);
>
> This will not work. The main stmmac driver already uses the driver_data field.
> See: 
> http://lxr.free-electrons.com/source/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L3218
>
>
>> +   return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
>
> So calling stmmac_dvr_probe here will overwrite the driver_data field.
>
>
>> +}
>> +
>> +static int meson8b_dwmac_remove(struct platform_device *pdev)
>> +{
>> +   struct meson8b_dwmac *dwmac = platform_get_drvdata(pdev);
>> +
>> +   clk_disable_unprepare(dwmac->m25_div_clk);
>
> Did you test this code? I am pretty sure it will blow up given that
> driver_data is not set to what you expect.
I guess I should've taken a closer look at my kernel config:
CONFIG_DWMAC_MESON=y
Thanks for spotting this issue!

> To get your meson8b_dwmac struct you must retrieve it from plat_dat->bsp_priv.
>
>
> I have some code for a helper to retrieve bsp_priv that I have meant
> to sent to the ML for a while now.
> See: 
> https://github.com/manabian/linux-lpc/commit/c3e155a6e38b9634e4e61aa4eeb4602ede7e44a6
>
> Feel free to add it to your patch set if you want.
>
> Alternatively take a look at the remove function from dwmac-stm32 here:
> https://patchwork.ozlabs.org/patch/619816/
excellent, thanks!

I will send a fixed version as soon as possible


Regards,
Martin

Re: [PATCH v2 3/4] net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC

[Joachim Eastwood]

Hi Martin,

On 20 August 2016 at 11:35, Martin Blumenstingl
 wrote:
> The Ethernet controller available in Meson8b and GXBB SoCs is a Synopsys
> DesignWare MAC IP core which is already supported by the stmmac driver.
>
> In addition to the standard stmmac driver some Meson8b / GXBB specific
> registers have to be configured for the PHY clocks. These SoC specific
> registers are called PRG_ETHERNET_ADDR0 and PRG_ETHERNET_ADDR1 in the
> datasheet.
> These registers are not backwards compatible with those on Meson 6b,
> which is why a new glue driver is introduced. This worked for many
> boards because the bootloader programs the PRG_ETHERNET registers
> correctly. Additionally the meson6-dwmac driver only sets bit 1 of
> PRG_ETHERNET_ADDR0 which (according to the datasheet) is only used
> during reset.
>
> Currently all configuration values can be determined automatically,
> based on the configured phy-mode (which is mandatory for the stmmac
> driver). If required the tx-delay and the mux clock (so it supports
> the MPLL2 clock as well) can be made configurable in the future.
>
> Signed-off-by: Martin Blumenstingl 
> Tested-by: Kevin Hilman 
> ---
>  drivers/net/ethernet/stmicro/stmmac/Makefile   |   2 +-
>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 329 
> +
>  2 files changed, 330 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c



> +static int meson8b_dwmac_probe(struct platform_device *pdev)
> +{
> +   struct plat_stmmacenet_data *plat_dat;
> +   struct stmmac_resources stmmac_res;
> +   struct resource *res;
> +   struct meson8b_dwmac *dwmac;
> +   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);
> +
> +   dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> +   if (!dwmac)
> +   return -ENOMEM;
> +
> +   res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +   if (!res)
> +   return -ENODEV;
> +
> +   dwmac->regs = devm_ioremap_resource(&pdev->dev, res);
> +   if (IS_ERR(dwmac->regs))
> +   return PTR_ERR(dwmac->regs);
> +
> +   dwmac->pdev = pdev;
> +   dwmac->phy_mode = of_get_phy_mode(pdev->dev.of_node);
> +   if (dwmac->phy_mode < 0) {
> +   dev_err(&pdev->dev, "missing phy-mode property\n");
> +   return -EINVAL;
> +   }
> +
> +   ret = meson8b_init_clk(dwmac);
> +   if (ret)
> +   return ret;
> +
> +   ret = meson8b_init_prg_eth(dwmac);
> +   if (ret)
> +   return ret;
> +
> +   plat_dat->bsp_priv = dwmac;
> +
> +   platform_set_drvdata(pdev, dwmac);

This will not work. The main stmmac driver already uses the driver_data field.
See: 
http://lxr.free-electrons.com/source/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c#L3218


> +   return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);

So calling stmmac_dvr_probe here will overwrite the driver_data field.


> +}
> +
> +static int meson8b_dwmac_remove(struct platform_device *pdev)
> +{
> +   struct meson8b_dwmac *dwmac = platform_get_drvdata(pdev);
> +
> +   clk_disable_unprepare(dwmac->m25_div_clk);

Did you test this code? I am pretty sure it will blow up given that
driver_data is not set to what you expect.

To get your meson8b_dwmac struct you must retrieve it from plat_dat->bsp_priv.


I have some code for a helper to retrieve bsp_priv that I have meant
to sent to the ML for a while now.
See: 
https://github.com/manabian/linux-lpc/commit/c3e155a6e38b9634e4e61aa4eeb4602ede7e44a6

Feel free to add it to your patch set if you want.

Alternatively take a look at the remove function from dwmac-stm32 here:
https://patchwork.ozlabs.org/patch/619816/


> +
> +   return stmmac_pltfr_remove(pdev);
> +}


regards,
Joachim Eastwood

[PATCH v2 3/4] net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC

[Martin Blumenstingl]

The Ethernet controller available in Meson8b and GXBB SoCs is a Synopsys
DesignWare MAC IP core which is already supported by the stmmac driver.

In addition to the standard stmmac driver some Meson8b / GXBB specific
registers have to be configured for the PHY clocks. These SoC specific
registers are called PRG_ETHERNET_ADDR0 and PRG_ETHERNET_ADDR1 in the
datasheet.
These registers are not backwards compatible with those on Meson 6b,
which is why a new glue driver is introduced. This worked for many
boards because the bootloader programs the PRG_ETHERNET registers
correctly. Additionally the meson6-dwmac driver only sets bit 1 of
PRG_ETHERNET_ADDR0 which (according to the datasheet) is only used
during reset.

Currently all configuration values can be determined automatically,
based on the configured phy-mode (which is mandatory for the stmmac
driver). If required the tx-delay and the mux clock (so it supports
the MPLL2 clock as well) can be made configurable in the future.

Signed-off-by: Martin Blumenstingl 
Tested-by: Kevin Hilman 
---
 drivers/net/ethernet/stmicro/stmmac/Makefile   |   2 +-
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 329 +
 2 files changed, 330 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c

diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile 
b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 44b630c..f77edb9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -9,7 +9,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o 
ring_mode.o  \
 obj-$(CONFIG_STMMAC_PLATFORM)  += stmmac-platform.o
 obj-$(CONFIG_DWMAC_IPQ806X)+= dwmac-ipq806x.o
 obj-$(CONFIG_DWMAC_LPC18XX)+= dwmac-lpc18xx.o
-obj-$(CONFIG_DWMAC_MESON)  += dwmac-meson.o
+obj-$(CONFIG_DWMAC_MESON)  += dwmac-meson.o dwmac-meson8b.o
 obj-$(CONFIG_DWMAC_ROCKCHIP)   += dwmac-rk.o
 obj-$(CONFIG_DWMAC_SOCFPGA)+= dwmac-altr-socfpga.o
 obj-$(CONFIG_DWMAC_STI)+= dwmac-sti.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
new file mode 100644
index 000..d080512
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -0,0 +1,329 @@
+/*
+ * Amlogic Meson S805/S905 DWMAC glue layer
+ *
+ * Copyright (C) 20016 Martin Blumenstingl 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "stmmac_platform.h"
+
+#define PRG_ETH0   0x0
+
+#define PRG_ETH0_RGMII_MODEBIT(0)
+
+/* mux to choose between fclk_div2 (bit unset) and mpll2 (bit set) */
+#define PRG_ETH0_CLK_M250_SEL_SHIFT4
+#define PRG_ETH0_CLK_M250_SEL_MASK GENMASK(4, 4)
+
+#define PRG_ETH0_TXDLY_SHIFT   5
+#define PRG_ETH0_TXDLY_MASKGENMASK(6, 5)
+#define PRG_ETH0_TXDLY_OFF (0x0 << PRG_ETH0_TXDLY_SHIFT)
+#define PRG_ETH0_TXDLY_QUARTER (0x1 << PRG_ETH0_TXDLY_SHIFT)
+#define PRG_ETH0_TXDLY_HALF(0x2 << PRG_ETH0_TXDLY_SHIFT)
+#define PRG_ETH0_TXDLY_THREE_QUARTERS  (0x3 << PRG_ETH0_TXDLY_SHIFT)
+
+/* divider for the result of m250_sel */
+#define PRG_ETH0_CLK_M250_DIV_SHIFT7
+#define PRG_ETH0_CLK_M250_DIV_WIDTH3
+
+/* divides the result of m25_sel by either 5 (bit unset) or 10 (bit set) */
+#define PRG_ETH0_CLK_M25_DIV_SHIFT 10
+#define PRG_ETH0_CLK_M25_DIV_WIDTH 1
+
+#define PRG_ETH0_INVERTED_RMII_CLK BIT(11)
+#define PRG_ETH0_TX_AND_PHY_REF_CLKBIT(12)
+
+#define MUX_CLK_NUM_PARENTS2
+
+struct meson8b_dwmac {
+   struct platform_device  *pdev;
+
+   void __iomem*regs;
+
+   phy_interface_t phy_mode;
+
+   struct clk_mux  m250_mux;
+   struct clk  *m250_mux_clk;
+   struct clk  *m250_mux_parent[MUX_CLK_NUM_PARENTS];
+
+   struct clk_divider  m250_div;
+   struct clk  *m250_div_clk;
+
+   struct clk_divider  m25_div;
+   struct clk  *m25_div_clk;
+};
+
+static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
+   u32 mask, u32 value)
+{
+   u32 data;
+
+   data = readl(dwmac->regs + reg);
+   data &= ~mask;
+   data |= (value & mask);
+
+   writel(data, dwmac->regs + reg);
+}
+
+static int meson8b_init_clk(struct meson8b_dwmac *dwmac)
+{
+   struct clk_init_data init;
+   int i, ret;
+   struct device *dev = &dwmac->pdev->dev;
+   char clk_name[32];
+