On Thu, 21 Sep 2017, David Wu wrote:

If we include both the rk3288_grf.h and rv1108_grf.h, there is a
number of compiling error for redefinition. So we define the reg
structs of mac_grf at gmac_rockchip.c. Remove the rk**_grf.h files,
give them own grf offset for their use.

The reg offset should not be open-coded in gmac_rockchip.c.

The issue of GRF-header having conflicting definitions was already
discussed on the list, when I initially submitted the RK3368 support.
The decision back then was as follows:
1/ The GRF files should not contain definitions that are private to
   the IOMUX (e.g. these should go into the pinctrl-driver), etc.
2/ As an intermediate step, we move some of this (i.e. the GMAC_CLK_SEL
   definitions into gmac_rockchip.c.
3/ The long-term solution will be to either create misc-devices that
   handle the 'set-to-rgmii' functionality and the 'GMAC_CLK_SEL'
   bits (although on those, modelling it via the clk-framework might
   be more appropriate).

Please clean up the affected GRF files that cause the conflicts (e.g.
the RV1108) and extend the current implementation w/o open-coding a
grf-offet.

Additional requested changes below.


Signed-off-by: David Wu <[email protected]>
Acked-by: Philipp Tomsich <[email protected]>
---

drivers/net/gmac_rockchip.c | 144 +++++++++++++++++++++++++++++++++++---------
1 file changed, 116 insertions(+), 28 deletions(-)

diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c
index 586ccbf..5f8f0cd 100644
--- a/drivers/net/gmac_rockchip.c
+++ b/drivers/net/gmac_rockchip.c
@@ -15,9 +15,6 @@
#include <asm/arch/periph.h>
#include <asm/arch/clock.h>
#include <asm/arch/hardware.h>
-#include <asm/arch/grf_rk3288.h>
-#include <asm/arch/grf_rk3368.h>
-#include <asm/arch/grf_rk3399.h>
#include <dm/pinctrl.h>
#include <dt-bindings/clock/rk3288-cru.h>
#include "designware.h"
@@ -31,15 +28,37 @@ DECLARE_GLOBAL_DATA_PTR;
 */
struct gmac_rockchip_platdata {
        struct dw_eth_pdata dw_eth_pdata;
+       void *grf;
        int tx_delay;
        int rx_delay;
};

struct rk_gmac_ops {
-       int (*fix_mac_speed)(struct dw_eth_dev *priv);
+       int (*fix_mac_speed)(struct gmac_rockchip_platdata *pdata,
+                            struct dw_eth_dev *priv);
        void (*set_to_rgmii)(struct gmac_rockchip_platdata *pdata);
};

+struct gmac_rockchip_driver_data {
+       const struct rk_gmac_ops *ops;
+       unsigned int grf_offset;
+};
+
+struct rk3288_mac_grf {
+       u32 soc_con1;
+       u32 reserved;
+       u32 soc_con3;
+};
+
+struct rk3368_mac_grf {
+       u32 soc_con15;
+       u32 soc_con16;
+};
+
+struct rk3399_mac_grf {
+       u32 soc_con5;
+       u32 soc_con6;
+};

We really can't pollute the GMAC driver with these definitions.
The actual values need to come out of the central GRF structure definition.


static int gmac_rockchip_ofdata_to_platdata(struct udevice *dev)
{
@@ -58,10 +77,18 @@ static int gmac_rockchip_ofdata_to_platdata(struct udevice 
*dev)
        return designware_eth_ofdata_to_platdata(dev);
}

-static int rk3288_gmac_fix_mac_speed(struct dw_eth_dev *priv)
+static int rk3288_gmac_fix_mac_speed(struct gmac_rockchip_platdata *pdata,
+                                    struct dw_eth_dev *priv)
{
-       struct rk3288_grf *grf;
+       struct rk3288_mac_grf *grf = (struct rk3288_mac_grf *)pdata->grf;
        int clk;
+       enum {
+               RK3288_GMAC_CLK_SEL_SHIFT = 12,
+               RK3288_GMAC_CLK_SEL_MASK  = GENMASK(13, 12),
+               RK3288_GMAC_CLK_SEL_125M  = 0 << 12,
+               RK3288_GMAC_CLK_SEL_25M   = 3 << 12,
+               RK3288_GMAC_CLK_SEL_2_5M  = 2 << 12,
+       };

        switch (priv->phydev->speed) {
        case 10:
@@ -78,15 +105,15 @@ static int rk3288_gmac_fix_mac_speed(struct dw_eth_dev 
*priv)
                return -EINVAL;
        }

-       grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
        rk_clrsetreg(&grf->soc_con1, RK3288_GMAC_CLK_SEL_MASK, clk);

        return 0;
}

-static int rk3368_gmac_fix_mac_speed(struct dw_eth_dev *priv)
+static int rk3368_gmac_fix_mac_speed(struct gmac_rockchip_platdata *pdata,
+                                    struct dw_eth_dev *priv)
{
-       struct rk3368_grf *grf;
+       struct rk3368_mac_grf *grf = (struct rk3368_mac_grf *)pdata->grf;
        int clk;
        enum {
                RK3368_GMAC_CLK_SEL_2_5M = 2 << 4,
@@ -110,16 +137,22 @@ static int rk3368_gmac_fix_mac_speed(struct dw_eth_dev 
*priv)
                return -EINVAL;
        }

-       grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
        rk_clrsetreg(&grf->soc_con15, RK3368_GMAC_CLK_SEL_MASK, clk);

        return 0;
}

-static int rk3399_gmac_fix_mac_speed(struct dw_eth_dev *priv)
+static int rk3399_gmac_fix_mac_speed(struct gmac_rockchip_platdata *pdata,
+                                    struct dw_eth_dev *priv)
{
-       struct rk3399_grf_regs *grf;
+       struct rk3399_mac_grf *grf = (struct rk3399_mac_grf *)pdata->grf;
        int clk;
+       enum {
+               RK3399_GMAC_CLK_SEL_MASK  = GENMASK(6, 4),
+               RK3399_GMAC_CLK_SEL_125M  = 0 << 4,
+               RK3399_GMAC_CLK_SEL_25M   = 3 << 4,
+               RK3399_GMAC_CLK_SEL_2_5M  = 2 << 4,
+       };

        switch (priv->phydev->speed) {
        case 10:
@@ -136,7 +169,6 @@ static int rk3399_gmac_fix_mac_speed(struct dw_eth_dev 
*priv)
                return -EINVAL;
        }

-       grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
        rk_clrsetreg(&grf->soc_con5, RK3399_GMAC_CLK_SEL_MASK, clk);

        return 0;
@@ -144,9 +176,31 @@ static int rk3399_gmac_fix_mac_speed(struct dw_eth_dev 
*priv)

static void rk3288_gmac_set_to_rgmii(struct gmac_rockchip_platdata *pdata)
{
-       struct rk3288_grf *grf;
+       struct rk3288_mac_grf *grf = (struct rk3288_mac_grf *)pdata->grf;
+       enum {
+               RK3288_RMII_MODE_SHIFT = 14,
+               RK3288_RMII_MODE_MASK  = BIT(14),
+
+               RK3288_GMAC_PHY_INTF_SEL_SHIFT = 6,
+               RK3288_GMAC_PHY_INTF_SEL_MASK  = GENMASK(8, 6),
+               RK3288_GMAC_PHY_INTF_SEL_RGMII = BIT(6),
+       };
+       enum {
+               RK3288_RXCLK_DLY_ENA_GMAC_MASK = BIT(15),
+               RK3288_RXCLK_DLY_ENA_GMAC_DISABLE = 0,
+               RK3288_RXCLK_DLY_ENA_GMAC_ENABLE = BIT(15),
+
+               RK3288_TXCLK_DLY_ENA_GMAC_MASK = BIT(14),
+               RK3288_TXCLK_DLY_ENA_GMAC_DISABLE = 0,
+               RK3288_TXCLK_DLY_ENA_GMAC_ENABLE = BIT(14),
+
+               RK3288_CLK_RX_DL_CFG_GMAC_SHIFT = 0x7,
+               RK3288_CLK_RX_DL_CFG_GMAC_MASK = GENMASK(13, 7),
+
+               RK3288_CLK_TX_DL_CFG_GMAC_SHIFT = 0x0,
+               RK3288_CLK_TX_DL_CFG_GMAC_MASK = GENMASK(6, 0),
+       };

-       grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
        rk_clrsetreg(&grf->soc_con1,
                     RK3288_RMII_MODE_MASK | RK3288_GMAC_PHY_INTF_SEL_MASK,
                     RK3288_GMAC_PHY_INTF_SEL_RGMII);
@@ -164,7 +218,7 @@ static void rk3288_gmac_set_to_rgmii(struct 
gmac_rockchip_platdata *pdata)

static void rk3368_gmac_set_to_rgmii(struct gmac_rockchip_platdata *pdata)
{
-       struct rk3368_grf *grf;
+       struct rk3368_mac_grf *grf = (struct rk3368_mac_grf *)pdata->grf;
        enum {
                RK3368_GMAC_PHY_INTF_SEL_RGMII = 1 << 9,
                RK3368_GMAC_PHY_INTF_SEL_MASK = GENMASK(11, 9),
@@ -184,7 +238,6 @@ static void rk3368_gmac_set_to_rgmii(struct 
gmac_rockchip_platdata *pdata)
                RK3368_CLK_TX_DL_CFG_GMAC_MASK = GENMASK(6, 0),
        };

-       grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
        rk_clrsetreg(&grf->soc_con15,
                     RK3368_RMII_MODE_MASK | RK3368_GMAC_PHY_INTF_SEL_MASK,
                     RK3368_GMAC_PHY_INTF_SEL_RGMII);
@@ -202,9 +255,24 @@ static void rk3368_gmac_set_to_rgmii(struct 
gmac_rockchip_platdata *pdata)

static void rk3399_gmac_set_to_rgmii(struct gmac_rockchip_platdata *pdata)
{
-       struct rk3399_grf_regs *grf;
-
-       grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
+       struct rk3399_mac_grf *grf = (struct rk3399_mac_grf *)pdata->grf;
+       enum {
+               RK3399_GMAC_PHY_INTF_SEL_MASK  = GENMASK(11, 9),
+               RK3399_GMAC_PHY_INTF_SEL_RGMII = 1 << 9,
+               RK3399_GMAC_PHY_INTF_SEL_RMII  = 4 << 9,
+       };
+       enum {
+               RK3399_RXCLK_DLY_ENA_GMAC_MASK = BIT(15),
+               RK3399_RXCLK_DLY_ENA_GMAC_DISABLE = 0,
+               RK3399_RXCLK_DLY_ENA_GMAC_ENABLE = BIT(15),
+               RK3399_TXCLK_DLY_ENA_GMAC_MASK = BIT(7),
+               RK3399_TXCLK_DLY_ENA_GMAC_DISABLE = 0,
+               RK3399_TXCLK_DLY_ENA_GMAC_ENABLE = BIT(7),
+               RK3399_CLK_RX_DL_CFG_GMAC_SHIFT = 8,
+               RK3399_CLK_RX_DL_CFG_GMAC_MASK = GENMASK(14, 8),
+               RK3399_CLK_TX_DL_CFG_GMAC_SHIFT = 0,
+               RK3399_CLK_TX_DL_CFG_GMAC_MASK = GENMASK(6, 0),
+       };

        rk_clrsetreg(&grf->soc_con5,
                     RK3399_GMAC_PHY_INTF_SEL_MASK,
@@ -224,8 +292,9 @@ static void rk3399_gmac_set_to_rgmii(struct 
gmac_rockchip_platdata *pdata)
static int gmac_rockchip_probe(struct udevice *dev)
{
        struct gmac_rockchip_platdata *pdata = dev_get_platdata(dev);
-       struct rk_gmac_ops *ops =
-               (struct rk_gmac_ops *)dev_get_driver_data(dev);
+       struct gmac_rockchip_driver_data *data =
+               (struct gmac_rockchip_driver_data *)dev_get_driver_data(dev);
+       const struct rk_gmac_ops *ops = data->ops;
        struct clk clk;
        int ret;

@@ -238,6 +307,9 @@ static int gmac_rockchip_probe(struct udevice *dev)
        if (ret)
                return ret;

+       pdata->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF) +
+                    data->grf_offset;
+
        /* Set to RGMII mode */
        ops->set_to_rgmii(pdata);

@@ -248,14 +320,15 @@ static int gmac_rockchip_eth_start(struct udevice *dev)
{
        struct eth_pdata *pdata = dev_get_platdata(dev);
        struct dw_eth_dev *priv = dev_get_priv(dev);
-       struct rk_gmac_ops *ops =
-               (struct rk_gmac_ops *)dev_get_driver_data(dev);
+       struct gmac_rockchip_driver_data *data =
+              (struct gmac_rockchip_driver_data *)dev_get_driver_data(dev);
+       const struct rk_gmac_ops *ops = data->ops;
        int ret;

        ret = designware_eth_init(priv, pdata->enetaddr);
        if (ret)
                return ret;
-       ret = ops->fix_mac_speed(priv);
+       ret = ops->fix_mac_speed((struct gmac_rockchip_platdata *)pdata, priv);
        if (ret)
                return ret;
        ret = designware_eth_enable(priv);
@@ -279,23 +352,38 @@ const struct rk_gmac_ops rk3288_gmac_ops = {
        .set_to_rgmii = rk3288_gmac_set_to_rgmii,
};

+const struct gmac_rockchip_driver_data rk3288_gmac_data = {
+       .ops            = &rk3288_gmac_ops,
+       .grf_offset     = 0x248,
+};
+
const struct rk_gmac_ops rk3368_gmac_ops = {
        .fix_mac_speed = rk3368_gmac_fix_mac_speed,
        .set_to_rgmii = rk3368_gmac_set_to_rgmii,
};

+const struct gmac_rockchip_driver_data rk3368_gmac_data = {
+       .ops            = &rk3368_gmac_ops,
+       .grf_offset     = 0x43c,
+};
+
const struct rk_gmac_ops rk3399_gmac_ops = {
        .fix_mac_speed = rk3399_gmac_fix_mac_speed,
        .set_to_rgmii = rk3399_gmac_set_to_rgmii,
};

+const struct gmac_rockchip_driver_data rk3399_gmac_data = {
+       .ops            = &rk3399_gmac_ops,
+       .grf_offset     = 0xc214,

The grf_offset really can not be hardcoded here.

+};
+
static const struct udevice_id rockchip_gmac_ids[] = {
        { .compatible = "rockchip,rk3288-gmac",
-         .data = (ulong)&rk3288_gmac_ops },
+         .data = (ulong)&rk3288_gmac_data },
        { .compatible = "rockchip,rk3368-gmac",
-         .data = (ulong)&rk3368_gmac_ops },
+         .data = (ulong)&rk3368_gmac_data },
        { .compatible = "rockchip,rk3399-gmac",
-         .data = (ulong)&rk3399_gmac_ops },
+         .data = (ulong)&rk3399_gmac_data },
        { }
};


_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot

Reply via email to