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

[Joe Hershberger <joe.hershberger@ni.com>]

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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot