[U-Boot] [U-Boot, 01/14] net: rockchip: Separate rmii and rgmii speed setup

[Philipp Tomsich]

On Sat, 3 Feb 2018, David Wu wrote:

> Some Socs both have rgmii and rmii interface, so we need to
> separate their speed setting.
>
> 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 | 62 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 43 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c
> index 683e820..4396ca1 100644
> --- a/drivers/net/gmac_rockchip.c
> +++ b/drivers/net/gmac_rockchip.c
> @@ -40,7 +40,10 @@ struct gmac_rockchip_platdata {
> };
>
> struct rk_gmac_ops {
> -	int (*fix_mac_speed)(struct dw_eth_dev *priv);
> +	int (*fix_rmii_speed)(struct gmac_rockchip_platdata *pdata,
> +			      struct dw_eth_dev *priv);
> +	int (*fix_rgmii_speed)(struct gmac_rockchip_platdata *pdata,
> +			       struct dw_eth_dev *priv);

Why can't this be a single fix_mac_speed function that does the right 
thing both for RMII and RGMII depending on the platdata?

> 	void (*set_to_rmii)(struct gmac_rockchip_platdata *pdata);
> 	void (*set_to_rgmii)(struct gmac_rockchip_platdata *pdata);
> };
> @@ -70,7 +73,8 @@ static int gmac_rockchip_ofdata_to_platdata(struct udevice *dev)
> 	return designware_eth_ofdata_to_platdata(dev);
> }
>
> -static int rk3228_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> +static int rk3228_gmac_fix_rgmii_speed(struct gmac_rockchip_platdata *pdata,
> +				       struct dw_eth_dev *priv)
> {
> 	struct rk322x_grf *grf;
> 	int clk;
> @@ -103,7 +107,8 @@ static int rk3228_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> 	return 0;
> }
>
> -static int rk3288_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> +static int rk3288_gmac_fix_rgmii_speed(struct gmac_rockchip_platdata *pdata,
> +				       struct dw_eth_dev *priv)
> {
> 	struct rk3288_grf *grf;
> 	int clk;
> @@ -129,7 +134,8 @@ static int rk3288_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> 	return 0;
> }
>
> -static int rk3328_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> +static int rk3328_gmac_fix_rgmii_speed(struct gmac_rockchip_platdata *pdata,
> +				       struct dw_eth_dev *priv)
> {
> 	struct rk3328_grf_regs *grf;
> 	int clk;
> @@ -162,7 +168,8 @@ static int rk3328_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> 	return 0;
> }
>
> -static int rk3368_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> +static int rk3368_gmac_fix_rgmii_speed(struct gmac_rockchip_platdata *pdata,
> +				       struct dw_eth_dev *priv)
> {
> 	struct rk3368_grf *grf;
> 	int clk;
> @@ -194,7 +201,8 @@ static int rk3368_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> 	return 0;
> }
>
> -static int rk3399_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> +static int rk3399_gmac_fix_rgmii_speed(struct gmac_rockchip_platdata *pdata,
> +				       struct dw_eth_dev *priv)
> {
> 	struct rk3399_grf_regs *grf;
> 	int clk;
> @@ -220,7 +228,8 @@ static int rk3399_gmac_fix_mac_speed(struct dw_eth_dev *priv)
> 	return 0;
> }
>
> -static int rv1108_set_rmii_speed(struct dw_eth_dev *priv)
> +static int rv1108_gmac_fix_rmii_speed(struct gmac_rockchip_platdata *pdata,
> +				      struct dw_eth_dev *priv)
> {
> 	struct rv1108_grf *grf;
> 	int clk, speed;
> @@ -489,7 +498,7 @@ static int gmac_rockchip_probe(struct udevice *dev)
>
> 		break;
> 	default:
> -		debug("NO interface defined!\n");
> +		debug("%s: NO interface defined!\n", __func__);
> 		return -ENXIO;
> 	}
>
> @@ -498,18 +507,33 @@ static int gmac_rockchip_probe(struct udevice *dev)
>
> static int gmac_rockchip_eth_start(struct udevice *dev)
> {
> -	struct eth_pdata *pdata = dev_get_platdata(dev);
> +	struct eth_pdata *eth_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_platdata *pdata = dev_get_platdata(dev);
> 	int ret;
>
> -	ret = designware_eth_init(priv, pdata->enetaddr);
> -	if (ret)
> -		return ret;
> -	ret = ops->fix_mac_speed(priv);
> +	ret = designware_eth_init(priv, eth_pdata->enetaddr);
> 	if (ret)
> 		return ret;
> +
> +	switch (eth_pdata->phy_interface) {
> +	case PHY_INTERFACE_MODE_RGMII:
> +		ret = ops->fix_rgmii_speed(pdata, priv);
> +		if (ret)
> +			return ret;
> +		break;
> +	case PHY_INTERFACE_MODE_RMII:
> +		ret = ops->fix_rmii_speed(pdata, priv);
> +		if (ret)
> +			return ret;
> +		break;

Looking at this, the fix_mac_speed()-function could look into pdata to 
determine what needs to be done... no need to complicate the common code 
path with this.

> +	default:
> +		debug("%s: NO interface defined!\n", __func__);
> +		return -ENXIO;
> +	}
> +
> 	ret = designware_eth_enable(priv);
> 	if (ret)
> 		return ret;
> @@ -527,32 +551,32 @@ const struct eth_ops gmac_rockchip_eth_ops = {
> };
>
> const struct rk_gmac_ops rk3228_gmac_ops = {
> -	.fix_mac_speed = rk3228_gmac_fix_mac_speed,
> +	.fix_rgmii_speed = rk3228_gmac_fix_rgmii_speed,
> 	.set_to_rgmii = rk3228_gmac_set_to_rgmii,
> };
>
> const struct rk_gmac_ops rk3288_gmac_ops = {
> -	.fix_mac_speed = rk3288_gmac_fix_mac_speed,
> +	.fix_rgmii_speed = rk3288_gmac_fix_rgmii_speed,
> 	.set_to_rgmii = rk3288_gmac_set_to_rgmii,
> };
>
> const struct rk_gmac_ops rk3328_gmac_ops = {
> -	.fix_mac_speed = rk3328_gmac_fix_mac_speed,
> +	.fix_rgmii_speed = rk3328_gmac_fix_rgmii_speed,
> 	.set_to_rgmii = rk3328_gmac_set_to_rgmii,
> };
>
> const struct rk_gmac_ops rk3368_gmac_ops = {
> -	.fix_mac_speed = rk3368_gmac_fix_mac_speed,
> +	.fix_rgmii_speed = rk3368_gmac_fix_rgmii_speed,
> 	.set_to_rgmii = rk3368_gmac_set_to_rgmii,
> };
>
> const struct rk_gmac_ops rk3399_gmac_ops = {
> -	.fix_mac_speed = rk3399_gmac_fix_mac_speed,
> +	.fix_rgmii_speed = rk3399_gmac_fix_rgmii_speed,
> 	.set_to_rgmii = rk3399_gmac_set_to_rgmii,
> };
>
> const struct rk_gmac_ops rv1108_gmac_ops = {
> -	.fix_mac_speed = rv1108_set_rmii_speed,
> +	.fix_rmii_speed = rv1108_gmac_fix_rmii_speed,
> 	.set_to_rmii = rv1108_gmac_set_to_rmii,
> };
>
>