LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] ethernet: stmmac: dwmac-rk: Add GMAC support for px30
@ 2018-05-16  3:38 David Wu
  2018-05-16  6:34 ` Shawn Lin
  0 siblings, 1 reply; 3+ messages in thread
From: David Wu @ 2018-05-16  3:38 UTC (permalink / raw)
  To: davem, heiko, robh+dt
  Cc: mark.rutland, huangtao, netdev, linux-arm-kernel, linux-rockchip,
	linux-kernel, David Wu

Add constants and callback functions for the dwmac on px30 soc.
The base structure is the same, but registers and the bits in
them moved slightly, and add the clk_mac_speed for the select
of mac speed.

Signed-off-by: David Wu <david.wu@rock-chips.com>
---
 .../devicetree/bindings/net/rockchip-dwmac.txt     |  1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c     | 64 ++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
index 9c16ee2..3b71da7 100644
--- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
+++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
@@ -4,6 +4,7 @@ The device node has following properties.
 
 Required properties:
  - compatible: should be "rockchip,<name>-gamc"
+   "rockchip,px30-gmac":   found on PX30 SoCs
    "rockchip,rk3128-gmac": found on RK312x SoCs
    "rockchip,rk3228-gmac": found on RK322x SoCs
    "rockchip,rk3288-gmac": found on RK3288 SoCs
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 13133b3..4b2ab71 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -61,6 +61,7 @@ struct rk_priv_data {
 	struct clk *mac_clk_tx;
 	struct clk *clk_mac_ref;
 	struct clk *clk_mac_refout;
+	struct clk *clk_mac_speed;
 	struct clk *aclk_mac;
 	struct clk *pclk_mac;
 	struct clk *clk_phy;
@@ -83,6 +84,64 @@ struct rk_priv_data {
 	(((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE : soc##_GMAC_TXCLK_DLY_DISABLE) | \
 	 ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE : soc##_GMAC_RXCLK_DLY_DISABLE))
 
+#define PX30_GRF_GMAC_CON1		0X0904
+
+/* PX30_GRF_GMAC_CON1 */
+#define PX30_GMAC_PHY_INTF_SEL_RMII	(GRF_CLR_BIT(4) | GRF_CLR_BIT(5) | \
+					GRF_BIT(6))
+#define PX30_GMAC_SPEED_10M		GRF_CLR_BIT(2)
+#define PX30_GMAC_SPEED_100M		GRF_BIT(2)
+
+static void px30_set_to_rmii(struct rk_priv_data *bsp_priv)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+
+	if (IS_ERR(bsp_priv->grf)) {
+		dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
+		     PX30_GMAC_PHY_INTF_SEL_RMII);
+}
+
+static void px30_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
+{
+	struct device *dev = &bsp_priv->pdev->dev;
+	int ret;
+
+	if (IS_ERR(bsp_priv->clk_mac_speed)) {
+		dev_err(dev, "%s: Missing clk_mac_speed clock\n", __func__);
+		return;
+	}
+
+	if (speed == 10) {
+		regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
+			     PX30_GMAC_SPEED_10M);
+
+		ret = clk_set_rate(bsp_priv->clk_mac_speed, 2500000);
+		if (ret)
+			dev_err(dev, "%s: set clk_mac_speed rate 2500000 failed: %d\n",
+				__func__, ret);
+	} else if (speed == 100) {
+		regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
+			     PX30_GMAC_SPEED_100M);
+
+		ret = clk_set_rate(bsp_priv->clk_mac_speed, 25000000);
+		if (ret)
+			dev_err(dev, "%s: set clk_mac_speed rate 25000000 failed: %d\n",
+				__func__, ret);
+
+	} else {
+		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	}
+}
+
+static const struct rk_gmac_ops px30_ops = {
+	.set_to_rmii = px30_set_to_rmii,
+	.set_rmii_speed = px30_set_rmii_speed,
+};
+
 #define RK3128_GRF_MAC_CON0	0x0168
 #define RK3128_GRF_MAC_CON1	0x016c
 
@@ -1042,6 +1101,10 @@ static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
 		}
 	}
 
+	bsp_priv->clk_mac_speed = devm_clk_get(dev, "clk_mac_speed");
+	if (IS_ERR(bsp_priv->clk_mac_speed))
+		dev_err(dev, "cannot get clock %s\n", "clk_mac_speed");
+
 	if (bsp_priv->clock_input) {
 		dev_info(dev, "clock input from PHY\n");
 	} else {
@@ -1424,6 +1487,7 @@ static int rk_gmac_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(rk_gmac_pm_ops, rk_gmac_suspend, rk_gmac_resume);
 
 static const struct of_device_id rk_gmac_dwmac_match[] = {
+	{ .compatible = "rockchip,px30-gmac",	.data = &px30_ops   },
 	{ .compatible = "rockchip,rk3128-gmac", .data = &rk3128_ops },
 	{ .compatible = "rockchip,rk3228-gmac", .data = &rk3228_ops },
 	{ .compatible = "rockchip,rk3288-gmac", .data = &rk3288_ops },
-- 
2.7.4

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ethernet: stmmac: dwmac-rk: Add GMAC support for px30
  2018-05-16  3:38 [PATCH] ethernet: stmmac: dwmac-rk: Add GMAC support for px30 David Wu
@ 2018-05-16  6:34 ` Shawn Lin
  2018-05-17 14:52   ` David Wu
  0 siblings, 1 reply; 3+ messages in thread
From: Shawn Lin @ 2018-05-16  6:34 UTC (permalink / raw)
  To: David Wu, robh+dt
  Cc: davem, heiko, shawn.lin, mark.rutland, huangtao, netdev,
	linux-kernel, linux-rockchip, linux-arm-kernel

Hi David,

On 2018/5/16 11:38, David Wu wrote:
> Add constants and callback functions for the dwmac on px30 soc.

s/soc/SoC

> The base structure is the same, but registers and the bits in
> them moved slightly, and add the clk_mac_speed for the select

s/moved/are moved

> of mac speed.

for selecting mas speed.

> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>

git log --oneline  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c

shows very inconsistent format wrt. commit title, so please
follow the exsiting exsamples as possible.

> ---
>   .../devicetree/bindings/net/rockchip-dwmac.txt     |  1 +
>   drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c     | 64 ++++++++++++++++++++++
>   2 files changed, 65 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> index 9c16ee2..3b71da7 100644
> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.txt
> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.txt

It'd be better to split doc changes into a separate patch.

> @@ -4,6 +4,7 @@ The device node has following properties.
>   
>   Required properties:
>    - compatible: should be "rockchip,<name>-gamc"
> +   "rockchip,px30-gmac":   found on PX30 SoCs
>      "rockchip,rk3128-gmac": found on RK312x SoCs
>      "rockchip,rk3228-gmac": found on RK322x SoCs
>      "rockchip,rk3288-gmac": found on RK3288 SoCs
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index 13133b3..4b2ab71 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -61,6 +61,7 @@ struct rk_priv_data {
>   	struct clk *mac_clk_tx;
>   	struct clk *clk_mac_ref;
>   	struct clk *clk_mac_refout;
> +	struct clk *clk_mac_speed;

No need to do anything now but it seems you could consider doing some
cleanup by using clk bulk APIs in the future.

>   	struct clk *aclk_mac;
>   	struct clk *pclk_mac;
>   	struct clk *clk_phy;
> @@ -83,6 +84,64 @@ struct rk_priv_data {
>   	(((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE : soc##_GMAC_TXCLK_DLY_DISABLE) | \
>   	 ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE : soc##_GMAC_RXCLK_DLY_DISABLE))
>   
> +#define PX30_GRF_GMAC_CON1		0X0904

s/0X0904/0x0904 , since the other constants in this file follow the
same format.

> +
> +/* PX30_GRF_GMAC_CON1 */
> +#define PX30_GMAC_PHY_INTF_SEL_RMII	(GRF_CLR_BIT(4) | GRF_CLR_BIT(5) | \
> +					GRF_BIT(6))
> +#define PX30_GMAC_SPEED_10M		GRF_CLR_BIT(2)
> +#define PX30_GMAC_SPEED_100M		GRF_BIT(2)
> +
> +static void px30_set_to_rmii(struct rk_priv_data *bsp_priv)
> +{
> +	struct device *dev = &bsp_priv->pdev->dev;
> +
> +	if (IS_ERR(bsp_priv->grf)) {
> +		dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
> +		return;
> +	}
> +
> +	regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
> +		     PX30_GMAC_PHY_INTF_SEL_RMII);
> +}
> +
> +static void px30_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
> +{
> +	struct device *dev = &bsp_priv->pdev->dev;
> +	int ret;
> +
> +	if (IS_ERR(bsp_priv->clk_mac_speed)) {
> +		dev_err(dev, "%s: Missing clk_mac_speed clock\n", __func__);
> +		return;
> +	}
> +
> +	if (speed == 10) {
> +		regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
> +			     PX30_GMAC_SPEED_10M);
> +
> +		ret = clk_set_rate(bsp_priv->clk_mac_speed, 2500000);
> +		if (ret)
> +			dev_err(dev, "%s: set clk_mac_speed rate 2500000 failed: %d\n",
> +				__func__, ret);
> +	} else if (speed == 100) {
> +		regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
> +			     PX30_GMAC_SPEED_100M);
> +
> +		ret = clk_set_rate(bsp_priv->clk_mac_speed, 25000000);
> +		if (ret)
> +			dev_err(dev, "%s: set clk_mac_speed rate 25000000 failed: %d\n",
> +				__func__, ret);

I know it follows the existing examples, but IMHO it duplicates
unnecessary code as all the difference is PX30_GMAC_SPEED_*

> +
> +	} else {
> +		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
> +	}
> +}
> +
> +static const struct rk_gmac_ops px30_ops = {
> +	.set_to_rmii = px30_set_to_rmii,
> +	.set_rmii_speed = px30_set_rmii_speed,
> +};
> +
>   #define RK3128_GRF_MAC_CON0	0x0168
>   #define RK3128_GRF_MAC_CON1	0x016c
>   
> @@ -1042,6 +1101,10 @@ static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
>   		}
>   	}
>   
> +	bsp_priv->clk_mac_speed = devm_clk_get(dev, "clk_mac_speed");

Mightbe it'd be better to use "mac-speed" in DT bindings.

> +	if (IS_ERR(bsp_priv->clk_mac_speed))
> +		dev_err(dev, "cannot get clock %s\n", "clk_mac_speed");
> +

Would you like to handle deferred probe?

>   	if (bsp_priv->clock_input) {
>   		dev_info(dev, "clock input from PHY\n");
>   	} else {
> @@ -1424,6 +1487,7 @@ static int rk_gmac_resume(struct device *dev)
>   static SIMPLE_DEV_PM_OPS(rk_gmac_pm_ops, rk_gmac_suspend, rk_gmac_resume);
>   
>   static const struct of_device_id rk_gmac_dwmac_match[] = {
> +	{ .compatible = "rockchip,px30-gmac",	.data = &px30_ops   },
>   	{ .compatible = "rockchip,rk3128-gmac", .data = &rk3128_ops },
>   	{ .compatible = "rockchip,rk3228-gmac", .data = &rk3228_ops },
>   	{ .compatible = "rockchip,rk3288-gmac", .data = &rk3288_ops },
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ethernet: stmmac: dwmac-rk: Add GMAC support for px30
  2018-05-16  6:34 ` Shawn Lin
@ 2018-05-17 14:52   ` David Wu
  0 siblings, 0 replies; 3+ messages in thread
From: David Wu @ 2018-05-17 14:52 UTC (permalink / raw)
  To: Shawn Lin, robh+dt
  Cc: davem, heiko, mark.rutland, huangtao, netdev, linux-kernel,
	linux-rockchip, linux-arm-kernel


Hi Shawn,

Thanks for the suggestion, the most is okay.

在 2018年05月16日 14:34, Shawn Lin 写道:
> Hi David,
> 
> On 2018/5/16 11:38, David Wu wrote:
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> index 13133b3..4b2ab71 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> @@ -61,6 +61,7 @@ struct rk_priv_data {
>>       struct clk *mac_clk_tx;
>>       struct clk *clk_mac_ref;
>>       struct clk *clk_mac_refout;
>> +    struct clk *clk_mac_speed;
> 
> No need to do anything now but it seems you could consider doing some
> cleanup by using clk bulk APIs in the future.

The use of this may seem to be less applicable because there are many 
scenarios using different clocks.

> 
>>       struct clk *aclk_mac;
>>       struct clk *pclk_mac;
>>       struct clk *clk_phy;
>> @@ -83,6 +84,64 @@ struct rk_priv_data {
>>       (((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE : 
>> soc##_GMAC_TXCLK_DLY_DISABLE) | \
>>        ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE : 
>> soc##_GMAC_RXCLK_DLY_DISABLE))
>> +#define PX30_GRF_GMAC_CON1        0X0904
> 
> s/0X0904/0x0904 , since the other constants in this file follow the
> same format.
> 
>> +
>> +/* PX30_GRF_GMAC_CON1 */
>> +#define PX30_GMAC_PHY_INTF_SEL_RMII    (GRF_CLR_BIT(4) | 
>> GRF_CLR_BIT(5) | \
>> +                    GRF_BIT(6))
>> +#define PX30_GMAC_SPEED_10M        GRF_CLR_BIT(2)
>> +#define PX30_GMAC_SPEED_100M        GRF_BIT(2)
>> +
>> +static void px30_set_to_rmii(struct rk_priv_data *bsp_priv)
>> +{
>> +    struct device *dev = &bsp_priv->pdev->dev;
>> +
>> +    if (IS_ERR(bsp_priv->grf)) {
>> +        dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
>> +        return;
>> +    }
>> +
>> +    regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
>> +             PX30_GMAC_PHY_INTF_SEL_RMII);
>> +}
>> +
>> +static void px30_set_rmii_speed(struct rk_priv_data *bsp_priv, int 
>> speed)
>> +{
>> +    struct device *dev = &bsp_priv->pdev->dev;
>> +    int ret;
>> +
>> +    if (IS_ERR(bsp_priv->clk_mac_speed)) {
>> +        dev_err(dev, "%s: Missing clk_mac_speed clock\n", __func__);
>> +        return;
>> +    }
>> +
>> +    if (speed == 10) {
>> +        regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
>> +                 PX30_GMAC_SPEED_10M);
>> +
>> +        ret = clk_set_rate(bsp_priv->clk_mac_speed, 2500000);
>> +        if (ret)
>> +            dev_err(dev, "%s: set clk_mac_speed rate 2500000 failed: 
>> %d\n",
>> +                __func__, ret);
>> +    } else if (speed == 100) {
>> +        regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
>> +                 PX30_GMAC_SPEED_100M);
>> +
>> +        ret = clk_set_rate(bsp_priv->clk_mac_speed, 25000000);
>> +        if (ret)
>> +            dev_err(dev, "%s: set clk_mac_speed rate 25000000 failed: 
>> %d\n",
>> +                __func__, ret);
> 
> I know it follows the existing examples, but IMHO it duplicates
> unnecessary code as all the difference is PX30_GMAC_SPEED_*
> 

i think the difference is the register offset and bits.

>> +
>> +    } else {
>> +        dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
>> +    }
>> +}
>> +
>> +static const struct rk_gmac_ops px30_ops = {
>> +    .set_to_rmii = px30_set_to_rmii,
>> +    .set_rmii_speed = px30_set_rmii_speed,
>> +};
>> +
>>   #define RK3128_GRF_MAC_CON0    0x0168
>>   #define RK3128_GRF_MAC_CON1    0x016c
>> @@ -1042,6 +1101,10 @@ static int rk_gmac_clk_init(struct 
>> plat_stmmacenet_data *plat)
>>           }
>>       }
>> +    bsp_priv->clk_mac_speed = devm_clk_get(dev, "clk_mac_speed");
> 
> Mightbe it'd be better to use "mac-speed" in DT bindings.
> 
>> +    if (IS_ERR(bsp_priv->clk_mac_speed))
>> +        dev_err(dev, "cannot get clock %s\n", "clk_mac_speed");
>> +
> 
> Would you like to handle deferred probe >

No,

>>       if (bsp_priv->clock_input) {
>>           dev_info(dev, "clock input from PHY\n");
>>       } else {
>> @@ -1424,6 +1487,7 @@ static int rk_gmac_resume(struct device *dev)
>>   static SIMPLE_DEV_PM_OPS(rk_gmac_pm_ops, rk_gmac_suspend, 
>> rk_gmac_resume);
>>   static const struct of_device_id rk_gmac_dwmac_match[] = {
>> +    { .compatible = "rockchip,px30-gmac",    .data = &px30_ops   },
>>       { .compatible = "rockchip,rk3128-gmac", .data = &rk3128_ops },
>>       { .compatible = "rockchip,rk3228-gmac", .data = &rk3228_ops },
>>       { .compatible = "rockchip,rk3288-gmac", .data = &rk3288_ops },
>>
> 
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-05-17 14:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-16  3:38 [PATCH] ethernet: stmmac: dwmac-rk: Add GMAC support for px30 David Wu
2018-05-16  6:34 ` Shawn Lin
2018-05-17 14:52   ` David Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).