[PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[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 are moved slightly, and add the clk_mac_speed for selecting
mac speed.

Signed-off-by: David Wu <david.wu@rock-chips.com>
---
Change in v2:
- Fix some error in commit title and message

 .../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..5e69dd0 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

Re: [PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[Heiko Stübner]

Hi David,

Am Donnerstag, 14. Juni 2018, 08:15:27 CEST schrieb 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 are moved slightly, and add the clk_mac_speed for selecting
> mac speed.
> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>

[...]

> @@ -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");
> +

I don't see that new clock documented in the dt-binding.
Also, which clock from the clock-controller does this connect to?


Thanks
Heiko

Re: [PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[David Wu]

Hi Heiko,

在 2018年06月14日 15:54, Heiko Stübner 写道:
> I don't see that new clock documented in the dt-binding.
> Also, which clock from the clock-controller does this connect to?

The clock is the "SCLK_GMAC_RMII" at the clock-controller, which could 
be set rate by the link speed.

Re: [PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[Heiko Stübner]

Am Donnerstag, 14. Juni 2018, 10:14:31 CEST schrieb David Wu:
> Hi Heiko,
> 
> 在 2018年06月14日 15:54, Heiko Stübner 写道:
> > I don't see that new clock documented in the dt-binding.
> > Also, which clock from the clock-controller does this connect to?
> 
> The clock is the "SCLK_GMAC_RMII" at the clock-controller, which could
> be set rate by the link speed.

Hmm, while these huge number of clocks are somewhat strange,
shouldn't it be named something with _rmii instead of _speed then?

Also, I don't see any clk_enable action for that new clock, so you could
end up with being off?

And someone could convert the driver to use the new clk-bulk APIs [0],
so the large number of clk_prepare_enable calls would be a bit
trimmed down.


Heiko

[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-bulk.c

Re: [PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[David Wu]

Hi Heiko,

在 2018年06月14日 16:30, Heiko Stübner 写道:
> Am Donnerstag, 14. Juni 2018, 10:14:31 CEST schrieb David Wu:
>> Hi Heiko,
>>
>> 在 2018年06月14日 15:54, Heiko Stübner 写道:
>>> I don't see that new clock documented in the dt-binding.
>>> Also, which clock from the clock-controller does this connect to?
>>
>> The clock is the "SCLK_GMAC_RMII" at the clock-controller, which could
>> be set rate by the link speed.
> 
> Hmm, while these huge number of clocks are somewhat strange,
> shouldn't it be named something with _rmii instead of _speed then?

Okay, it is better to be named _speed.

> 
> Also, I don't see any clk_enable action for that new clock, so you could
> end up with being off?

The new speed is the parent of the clk_tx_rx, to enable/disable 
clk_tx_rx, the new clock would be also enabled/disabled.

> 
> And someone could convert the driver to use the new clk-bulk APIs [0],
> so the large number of clk_prepare_enable calls would be a bit
> trimmed down.
> 
> 
> Heiko
> 
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-bulk.c
> 
> 
> 
> 
> 

Re: [PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[David Wu]

Hi Heiko,

在 2018年06月14日 16:30, Heiko Stübner 写道:
> And someone could convert the driver to use the new clk-bulk APIs [0],
> so the large number of clk_prepare_enable calls would be a bit
> trimmed down.

Some clocks need special treatment at special cases, may not know which 
index is we need at clk_bulk_data struct.
1. At rmii mode, need to use mac_ref, mac_refout; but at rgmii, they are 
not needed.
2. At rgmii mode, rx is coming in from external source, there is no 
gate, and it is coming from mac_ref_clk at rmii mode, there is gate.
3. clk_mac needs to be configured rate 50M or 125M.
4. mac_clk_speed needs to be configured at PX30 Soc and next Socs.

It looks like use the clk-bulk, will not be more flexible, and we still 
keep the present. What do you think?

Re: [PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[Heiko Stuebner]

Hi David,

Am Freitag, 22. Juni 2018, 09:22:35 CEST schrieb David Wu:
> 在 2018年06月14日 16:30, Heiko Stübner 写道:
> > And someone could convert the driver to use the new clk-bulk APIs [0],
> > so the large number of clk_prepare_enable calls would be a bit
> > trimmed down.
> 
> Some clocks need special treatment at special cases, may not know which 
> index is we need at clk_bulk_data struct.
> 1. At rmii mode, need to use mac_ref, mac_refout; but at rgmii, they are 
> not needed.
> 2. At rgmii mode, rx is coming in from external source, there is no 
> gate, and it is coming from mac_ref_clk at rmii mode, there is gate.
> 3. clk_mac needs to be configured rate 50M or 125M.
> 4. mac_clk_speed needs to be configured at PX30 Soc and next Socs.
> 
> It looks like use the clk-bulk, will not be more flexible, and we still 
> keep the present. What do you think?

yeah, you're probably right. I just saw all these clk_prepare_enable calls
and didn't think enough about the config side ;-) .


Heiko

Re: [PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[Heiko Stuebner]

Hi David,

Am Mittwoch, 20. Juni 2018, 04:40:35 CEST schrieb David Wu:
> 在 2018年06月14日 16:30, Heiko Stübner 写道:
> > Am Donnerstag, 14. Juni 2018, 10:14:31 CEST schrieb David Wu:
> >> Hi Heiko,
> >>
> >> 在 2018年06月14日 15:54, Heiko Stübner 写道:
> >>> I don't see that new clock documented in the dt-binding.
> >>> Also, which clock from the clock-controller does this connect to?
> >>
> >> The clock is the "SCLK_GMAC_RMII" at the clock-controller, which could
> >> be set rate by the link speed.
> > 
> > Hmm, while these huge number of clocks are somewhat strange,
> > shouldn't it be named something with _rmii instead of _speed then?
> 
> Okay, it is better to be named _speed.
> 
> > 
> > Also, I don't see any clk_enable action for that new clock, so you could
> > end up with being off?
> 
> The new speed is the parent of the clk_tx_rx, to enable/disable 
> clk_tx_rx, the new clock would be also enabled/disabled.

Still it is nicer to really enable it, so that the clock-framework can keep
track of usage counts.

Because also no-one hinders the chip-designer from putting a gate in
between in one of the next socs ;-)


Heiko

Re: [PATCH v2] net: ethernet: stmmac: dwmac-rk: Add GMAC support for PX30

[David Wu]

Hi Heiko,

在 2018年06月22日 15:30, Heiko Stuebner 写道:
> Hi David,
> 
> Am Mittwoch, 20. Juni 2018, 04:40:35 CEST schrieb David Wu:
>> 在 2018年06月14日 16:30, Heiko Stübner 写道:
>>> Am Donnerstag, 14. Juni 2018, 10:14:31 CEST schrieb David Wu:
>>>> Hi Heiko,
>>>>
>>>> 在 2018年06月14日 15:54, Heiko Stübner 写道:
>>>>> I don't see that new clock documented in the dt-binding.
>>>>> Also, which clock from the clock-controller does this connect to?
>>>>
>>>> The clock is the "SCLK_GMAC_RMII" at the clock-controller, which could
>>>> be set rate by the link speed.
>>>
>>> Hmm, while these huge number of clocks are somewhat strange,
>>> shouldn't it be named something with _rmii instead of _speed then?
>>
>> Okay, it is better to be named _speed.
>>
>>>
>>> Also, I don't see any clk_enable action for that new clock, so you could
>>> end up with being off?
>>
>> The new speed is the parent of the clk_tx_rx, to enable/disable
>> clk_tx_rx, the new clock would be also enabled/disabled.
> 
> Still it is nicer to really enable it, so that the clock-framework can keep
> track of usage counts.
> 
> Because also no-one hinders the chip-designer from putting a gate in
> between in one of the next socs ;-)
> 

Okay, i will add the enable/disable for clk_mac_speed.
  ;-)

> 
> Heiko
> 
> 
> 
> 
>