[U-Boot] [U-Boot, 4/6] net: gmac_rockchip: Define the gmac grf register struct at gmac_rockchip.c

David.Wu david.wu at rock-chips.com
Tue Oct 17 11:52:56 UTC 2017


Hi Philipp,

在 2017/10/6 18:06, Philipp Tomsich 写道:
> 
> 
> 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.

I'll try to move all the iomux definitions at GRF-header into 
pinctrl-driver, use pinctrl_request* interface to set iomux in some 
board.c file.

> 2/ As an intermediate step, we move some of this (i.e. the GMAC_CLK_SEL
>     definitions into gmac_rockchip.c.

I still think the 'GMAC_CLK_SEL' bit set should be implemented at clk 
driver.

> 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).

Could you give me a example code for misc-devices?

> 
> 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 <david.wu at rock-chips.com>
>> Acked-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
>> ---
>>
>> 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 },
>>     { }
>> };
>>
>>
> 
> 
> 



More information about the U-Boot mailing list