Re: [U-Boot] [PATCH v3 2/5] net: sun8i-emac: support R40 GMAC

On Mon, Apr 23, 2018 at 9:57 AM, Lothar Felten lothar.felten@gmail.com wrote:

Add support for the GMAC found in the Allwinner R40/V40 SoC.

The R40 GMAC interface is not controlled by the syscon register but has a separate configuration register in the CCU. The clock gate and reset bits are in a different register compared to the other SoCs supported by this driver. The diver uses the -gmac suffix for the R40 because the R40 also

diver -> driver

has a different 100 MBit MAC (EMAC).

Signed-off-by: Lothar Felten lothar.felten@gmail.com

drivers/net/sun8i_emac.c | 69 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 22 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index b6e5dafe83..83844a1d40 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -68,6 +68,8 @@

#if defined(CONFIG_MACH_SUNXI_H3_H5) #define SUN8I_GPD8_GMAC 2 +#elif defined(CONFIG_MACH_SUN8I_R40) +#define SUN8I_GPD8_GMAC 5 #else #define SUN8I_GPD8_GMAC 4 #endif @@ -99,6 +101,7 @@ DECLARE_GLOBAL_DATA_PTR; enum emac_variant { A83T_EMAC = 1, H3_EMAC,

•   R40_GMAC,
  

It probably make sense to not insert this in the list... please put it at the end.

    A64_EMAC,

};

@@ -279,6 +282,9 @@ static int sun8i_emac_set_syscon(struct emac_eth_dev *priv) int ret; u32 reg;

•   if (priv->variant == R40_GMAC)
  

•           return 0;
  

•   reg = readl(priv->sysctl_reg + 0x30);
  
    if (priv->variant == H3_EMAC) {
  

@@ -630,11 +636,25 @@ static void sun8i_emac_board_setup(struct emac_eth_dev *priv) } #endif

+#ifdef CONFIG_MACH_SUN8I_R40

This should check variant instead of using a CPP guard.

•   /* Set clock gating for emac */
  

•   setbits_le32(&ccm->ahb_reset1_cfg, BIT(AHB_RESET_OFFSET_GMAC));
  

•   /* De-assert EMAC */
  

•   setbits_le32(&ccm->ahb_gate1, BIT(AHB_GATE_OFFSET_GMAC));
  

•   /* Select RGMII for R40 */
  

•   setbits_le32(&ccm->gmac_clk_cfg, CCM_GMAC_CTRL_TX_CLK_SRC_INT_RGMII |
  

•           CCM_GMAC_CTRL_GPIT_RGMII);
  

•   setbits_le32(&ccm->gmac_clk_cfg,
  

•                CCM_GMAC_CTRL_TX_CLK_DELAY(CONFIG_GMAC_TX_DELAY));
  

+#else /* Set clock gating for emac */ setbits_le32(&ccm->ahb_gate0, BIT(AHB_GATE_OFFSET_GMAC));

    /* De-assert EMAC */
    setbits_le32(&ccm->ahb_reset0_cfg, BIT(AHB_RESET_OFFSET_GMAC));

+#endif }

#if defined(CONFIG_DM_GPIO) @@ -801,22 +821,33 @@ static int sun8i_emac_eth_ofdata_to_platdata(struct udevice *dev) return -EINVAL; }

•   offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "syscon");
  

•   if (offset < 0) {
  

•           debug("%s: cannot find syscon node\n", __func__);
  

•           return -EINVAL;
  

•   }
  

•   reg = fdt_getprop(gd->fdt_blob, offset, "reg", NULL);
  

•   if (!reg) {
  

•           debug("%s: cannot find reg property in syscon node\n",
  

•                 __func__);
  

•   priv->variant = dev_get_driver_data(dev);
  

•   if (!priv->variant) {
  

•           printf("%s: Missing variant '%s'\n", __func__,
  

•                  (char *)priv->variant);
  

This makes no sense. The if statement above verified that the value was 0. You shouldn't cast that to a char * and try to print NULL.

            return -EINVAL;
    }

•   priv->sysctl_reg = fdt_translate_address((void *)gd->fdt_blob,
  

•                                            offset, reg);
  

•   if (priv->sysctl_reg == FDT_ADDR_T_NONE) {
  

•           debug("%s: Cannot find syscon base address\n", __func__);
  

•           return -EINVAL;
  

•   if (priv->variant != R40_GMAC) {
  

•           offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "syscon");
  

•           if (offset < 0) {
  

•                   debug("%s: cannot find syscon node\n", __func__);
  

•                   return -EINVAL;
  

•           }
  

•           reg = fdt_getprop(gd->fdt_blob, offset, "reg", NULL);
  

•           if (!reg) {
  

•                   debug("%s: cannot find reg property in syscon node\n",
  

•                         __func__);
  

•                   return -EINVAL;
  

•           }
  

•           priv->sysctl_reg = fdt_translate_address((void *)gd->fdt_blob,
  

•                                                    offset, reg);
  

•           if (priv->sysctl_reg == FDT_ADDR_T_NONE) {
  

•                   debug("%s: Cannot find syscon base address\n",
  

•                         __func__);
  

•                   return -EINVAL;
  

•           }
    }
  
    pdata->phy_interface = -1;
  

@@ -841,14 +872,6 @@ static int sun8i_emac_eth_ofdata_to_platdata(struct udevice *dev) return -EINVAL; }

•   priv->variant = dev_get_driver_data(dev);
  

•   if (!priv->variant) {
  

•           printf("%s: Missing variant '%s'\n", __func__,
  

•                  (char *)priv->variant);
  

Ah, you are just moving this from here... Please add a patch before this one that fixes this.

•           return -EINVAL;
  

•   }
  

•   if (priv->variant == H3_EMAC) {
            int parent = fdt_parent_offset(gd->fdt_blob, offset);
  
  

@@ -885,6 +908,8 @@ static int sun8i_emac_eth_ofdata_to_platdata(struct udevice *dev)

static const struct udevice_id sun8i_emac_eth_ids[] = { {.compatible = "allwinner,sun8i-h3-emac", .data = (uintptr_t)H3_EMAC },

•   {.compatible = "allwinner,sun8i-r40-gmac",
  

•           .data = (uintptr_t)R40_GMAC },
    {.compatible = "allwinner,sun50i-a64-emac",
            .data = (uintptr_t)A64_EMAC },
    {.compatible = "allwinner,sun8i-a83t-emac",
  

-- 2.14.1

---

U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot