[PATCH 0/4] ARM: sun7i: Convert A20 GMAC driver to CCU

[PATCH 0/4] ARM: sun7i: Convert A20 GMAC driver to CCU

[Priit Laes]

This serie converts Allwinner A20 (sun7i) GMAC driver to CCU
while still retaining compatibility with existing devicetrees.

First two patches contain preliminary work which convert
sun4i/sun7i clock drivers to platform devices and creates regmap
to access gmac register from the sun7i gmac driver.

Third patch implements syscon-based regmap to allow driver manage
its own clock source.

Fourth patch updates the devicetree and drops the unused clocks.

While testing the driver I noticed following bugs with the existing
sun7i gmac driver:
- driver relies on u-boot for initialization (fixed in this
  implementation)
- `systemctl restart networking` fails to bring the link up again.


Priit Laes (4):
  clk: sunxi-ng: a10/a20: rewrite init code to a platform driver
  clk: sunxi-ng: a20: export a regmap to access the GMAC register
  net: stmmac: dwmac-sunxi: Implement syscon-based clock handling
  ARM: dts: sun7i: Use syscon-based implementation for gmac

 arch/arm/boot/dts/sun7i-a20.dtsi              |  36 +----
 drivers/clk/sunxi-ng/ccu-sun4i-a10.c          | 108 ++++++++++++---
 .../net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 124 ++++++++++++++++--
 3 files changed, 206 insertions(+), 62 deletions(-)

-- 
2.25.2

[PATCH 1/4] clk: sunxi-ng: a10/a20: rewrite init code to a platform driver

[Priit Laes]

In order to register regmap for sun7i CCU, there needs to be
a device structure already bound to the CCU device node.

Convert the sun4i/sun7i CCU setup to platform driver to use
it later as platform device.

Signed-off-by: Priit Laes <plaes@plaes.org>
---
 drivers/clk/sunxi-ng/ccu-sun4i-a10.c | 77 ++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 21 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun4i-a10.c b/drivers/clk/sunxi-ng/ccu-sun4i-a10.c
index f32366d9336e..839e9d5a1cff 100644
--- a/drivers/clk/sunxi-ng/ccu-sun4i-a10.c
+++ b/drivers/clk/sunxi-ng/ccu-sun4i-a10.c
@@ -7,7 +7,8 @@
 
 #include <linux/clk-provider.h>
 #include <linux/io.h>
-#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
 
 #include "ccu_common.h"
 #include "ccu_reset.h"
@@ -1425,19 +1426,10 @@ static const struct sunxi_ccu_desc sun7i_a20_ccu_desc = {
 	.num_resets	= ARRAY_SIZE(sunxi_a10_a20_ccu_resets),
 };
 
-static void __init sun4i_ccu_init(struct device_node *node,
-				  const struct sunxi_ccu_desc *desc)
+static void bootstrap_clocks(void __iomem *reg)
 {
-	void __iomem *reg;
 	u32 val;
 
-	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
-	if (IS_ERR(reg)) {
-		pr_err("%s: Could not map the clock registers\n",
-		       of_node_full_name(node));
-		return;
-	}
-
 	val = readl(reg + SUN4I_PLL_AUDIO_REG);
 
 	/*
@@ -1463,20 +1455,63 @@ static void __init sun4i_ccu_init(struct device_node *node,
 	val = readl(reg + SUN4I_AHB_REG);
 	val &= ~GENMASK(7, 6);
 	writel(val | (2 << 6), reg + SUN4I_AHB_REG);
-
-	sunxi_ccu_probe(node, reg, desc);
 }
 
-static void __init sun4i_a10_ccu_setup(struct device_node *node)
+static int sun4i_a10_ccu_probe(struct platform_device *pdev)
 {
-	sun4i_ccu_init(node, &sun4i_a10_ccu_desc);
+	struct resource *res;
+	void __iomem *reg;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	bootstrap_clocks(reg);
+
+	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun4i_a10_ccu_desc);
 }
-CLK_OF_DECLARE(sun4i_a10_ccu, "allwinner,sun4i-a10-ccu",
-	       sun4i_a10_ccu_setup);
 
-static void __init sun7i_a20_ccu_setup(struct device_node *node)
+static int sun7i_a20_ccu_probe(struct platform_device *pdev)
 {
-	sun4i_ccu_init(node, &sun7i_a20_ccu_desc);
+	struct resource *res;
+	void __iomem *reg;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	reg = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	bootstrap_clocks(reg);
+
+	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun7i_a20_ccu_desc);
 }
-CLK_OF_DECLARE(sun7i_a20_ccu, "allwinner,sun7i-a20-ccu",
-	       sun7i_a20_ccu_setup);
+
+
+static const struct of_device_id sun4i_a10_ccu_ids[] = {
+	{ .compatible = "allwinner,sun4i-a10-ccu"},
+	{ }
+};
+
+static struct platform_driver sun4i_a10_ccu_driver = {
+	.probe = sun4i_a10_ccu_probe,
+	.driver = {
+		.name = "sun4i-a10-ccu",
+		.of_match_table = sun4i_a10_ccu_ids,
+	},
+};
+builtin_platform_driver(sun4i_a10_ccu_driver);
+
+static const struct of_device_id sun7i_a20_ccu_ids[] = {
+	{ .compatible = "allwinner,sun7i-a20-ccu"},
+	{ }
+};
+
+static struct platform_driver sun7i_a20_ccu_driver = {
+	.probe = sun7i_a20_ccu_probe,
+	.driver = {
+		.name = "sun7i-a20-ccu",
+		.of_match_table = sun7i_a20_ccu_ids,
+	},
+};
+builtin_platform_driver(sun7i_a20_ccu_driver);
-- 
2.25.2

Re: [PATCH 1/4] clk: sunxi-ng: a10/a20: rewrite init code to a platform driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]

On Sat, Apr 18, 2020 at 01:17:27AM +0300, Priit Laes wrote:
> In order to register regmap for sun7i CCU, there needs to be
> a device structure already bound to the CCU device node.
> 
> Convert the sun4i/sun7i CCU setup to platform driver to use
> it later as platform device.
> 
> Signed-off-by: Priit Laes <plaes@plaes.org>

You can't relly do that though. We have timers that need those clocks before the
device model is initialized.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/4] clk: sunxi-ng: a10/a20: rewrite init code to a platform driver

[Priit Laes]

On Mon, Apr 20, 2020 at 02:49:35PM +0200, Maxime Ripard wrote:
> On Sat, Apr 18, 2020 at 01:17:27AM +0300, Priit Laes wrote:
> > In order to register regmap for sun7i CCU, there needs to be
> > a device structure already bound to the CCU device node.
> > 
> > Convert the sun4i/sun7i CCU setup to platform driver to use
> > it later as platform device.
> > 
> > Signed-off-by: Priit Laes <plaes@plaes.org>
> 
> You can't relly do that though. We have timers that need those clocks before the
> device model is initialized.

Ok, I'm somewhat lost now... are these the affected timers on sun7i following:
- allwinner,sun4i-a10-timer (timer@1c20c00)
- allwinner,sun7i-a20-hstimer (hstimer@1c60000)

Any ideas on what approach I could actually use?

Also, similar timer dependency would affect then sun6i-a31 and sun9i-a80
platforms too...

> 
> Maxime

Re: [PATCH 1/4] clk: sunxi-ng: a10/a20: rewrite init code to a platform driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1114 bytes --]

Hi,

On Mon, Apr 20, 2020 at 08:32:28PM +0000, Priit Laes wrote:
> On Mon, Apr 20, 2020 at 02:49:35PM +0200, Maxime Ripard wrote:
> > On Sat, Apr 18, 2020 at 01:17:27AM +0300, Priit Laes wrote:
> > > In order to register regmap for sun7i CCU, there needs to be
> > > a device structure already bound to the CCU device node.
> > > 
> > > Convert the sun4i/sun7i CCU setup to platform driver to use
> > > it later as platform device.
> > > 
> > > Signed-off-by: Priit Laes <plaes@plaes.org>
> > 
> > You can't relly do that though. We have timers that need those clocks before the
> > device model is initialized.
> 
> Ok, I'm somewhat lost now... are these the affected timers on sun7i following:
> - allwinner,sun4i-a10-timer (timer@1c20c00)
> - allwinner,sun7i-a20-hstimer (hstimer@1c60000)

Yep

> Any ideas on what approach I could actually use?

I guess you could keep the CLK_OF_DECLARE registration, and then have a
platform_driver probe and register the regmap?

> Also, similar timer dependency would affect then sun6i-a31 and sun9i-a80
> platforms too...

Indeed.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/4] clk: sunxi-ng: a10/a20: rewrite init code to a platform driver

[Priit Laes]

On Wed, Apr 29, 2020 at 04:35:10PM +0200, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Apr 20, 2020 at 08:32:28PM +0000, Priit Laes wrote:
> > On Mon, Apr 20, 2020 at 02:49:35PM +0200, Maxime Ripard wrote:
> > > On Sat, Apr 18, 2020 at 01:17:27AM +0300, Priit Laes wrote:
> > > > In order to register regmap for sun7i CCU, there needs to be
> > > > a device structure already bound to the CCU device node.
> > > > 
> > > > Convert the sun4i/sun7i CCU setup to platform driver to use
> > > > it later as platform device.
> > > > 
> > > > Signed-off-by: Priit Laes <plaes@plaes.org>
> > > 
> > > You can't relly do that though. We have timers that need those clocks before the
> > > device model is initialized.
> > 
> > Ok, I'm somewhat lost now... are these the affected timers on sun7i following:
> > - allwinner,sun4i-a10-timer (timer@1c20c00)
> > - allwinner,sun7i-a20-hstimer (hstimer@1c60000)
> 
> Yep
> 
> > Any ideas on what approach I could actually use?
> 
> I guess you could keep the CLK_OF_DECLARE registration, and then have a
> platform_driver probe and register the regmap?
> 

Thanks this did the trick.

> > Also, similar timer dependency would affect then sun6i-a31 and sun9i-a80
> > platforms too...

I didn't check this before, but sun9i-a80 CCU is initialized currently via
platform device. Should it be converted first to clock driver (CLK_OF_DECLARE)?

I have sent out the v2 which contains sun7i/sun6i changes.

> 
> Indeed.
> 
> Maxime

Re: [PATCH 1/4] clk: sunxi-ng: a10/a20: rewrite init code to a platform driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1693 bytes --]

On Thu, Apr 30, 2020 at 06:21:37AM +0000, Priit Laes wrote:
> On Wed, Apr 29, 2020 at 04:35:10PM +0200, Maxime Ripard wrote:
> > On Mon, Apr 20, 2020 at 08:32:28PM +0000, Priit Laes wrote:
> > > On Mon, Apr 20, 2020 at 02:49:35PM +0200, Maxime Ripard wrote:
> > > > On Sat, Apr 18, 2020 at 01:17:27AM +0300, Priit Laes wrote:
> > > > > In order to register regmap for sun7i CCU, there needs to be
> > > > > a device structure already bound to the CCU device node.
> > > > > 
> > > > > Convert the sun4i/sun7i CCU setup to platform driver to use
> > > > > it later as platform device.
> > > > > 
> > > > > Signed-off-by: Priit Laes <plaes@plaes.org>
> > > > 
> > > > You can't relly do that though. We have timers that need those clocks before the
> > > > device model is initialized.
> > > 
> > > Ok, I'm somewhat lost now... are these the affected timers on sun7i following:
> > > - allwinner,sun4i-a10-timer (timer@1c20c00)
> > > - allwinner,sun7i-a20-hstimer (hstimer@1c60000)
> > 
> > Yep
> > 
> > > Any ideas on what approach I could actually use?
> > 
> > I guess you could keep the CLK_OF_DECLARE registration, and then have a
> > platform_driver probe and register the regmap?
> > 
> 
> Thanks this did the trick.
> 
> > > Also, similar timer dependency would affect then sun6i-a31 and sun9i-a80
> > > platforms too...
> 
> I didn't check this before, but sun9i-a80 CCU is initialized currently via
> platform device. Should it be converted first to clock driver (CLK_OF_DECLARE)?

I guess we could just remove the timer node on the A80. It has never been tested
and never worked if the clock driver is probed through a platform device.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/4] clk: sunxi-ng: a20: export a regmap to access the GMAC register

[Priit Laes]

Only GMAC register is allowed to be written, read access to registers
is not restricted.

Export a regmap of the CCU.

Signed-off-by: Priit Laes <plaes@plaes.org>
---
 drivers/clk/sunxi-ng/ccu-sun4i-a10.c | 31 ++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/clk/sunxi-ng/ccu-sun4i-a10.c b/drivers/clk/sunxi-ng/ccu-sun4i-a10.c
index 839e9d5a1cff..cba51c2c7eba 100644
--- a/drivers/clk/sunxi-ng/ccu-sun4i-a10.c
+++ b/drivers/clk/sunxi-ng/ccu-sun4i-a10.c
@@ -1426,6 +1426,30 @@ static const struct sunxi_ccu_desc sun7i_a20_ccu_desc = {
 	.num_resets	= ARRAY_SIZE(sunxi_a10_a20_ccu_resets),
 };
 
+/*
+ * Add regmap for the GMAC driver (dwmac-sun8i) to allow access to
+ * GMAC configuration register.
+ */
+
+#define SUN7I_A20_GMAC_CFG_REG 0x164
+static bool sun7i_a20_ccu_regmap_accessible_reg(struct device *dev,
+						unsigned int reg)
+{
+	if (reg == SUN7I_A20_GMAC_CFG_REG)
+		return true;
+	return false;
+}
+
+static struct regmap_config sun7i_a20_ccu_regmap_config = {
+	.reg_bits	= 32,
+	.val_bits	= 32,
+	.reg_stride	= 4,
+	.max_register	= 0x1f4, /* clk_out_b */
+
+	.readable_reg	= sun7i_a20_ccu_regmap_accessible_reg,
+	.writeable_reg	= sun7i_a20_ccu_regmap_accessible_reg,
+};
+
 static void bootstrap_clocks(void __iomem *reg)
 {
 	u32 val;
@@ -1474,6 +1498,7 @@ static int sun4i_a10_ccu_probe(struct platform_device *pdev)
 
 static int sun7i_a20_ccu_probe(struct platform_device *pdev)
 {
+	struct regmap *regmap;
 	struct resource *res;
 	void __iomem *reg;
 
@@ -1484,6 +1509,12 @@ static int sun7i_a20_ccu_probe(struct platform_device *pdev)
 
 	bootstrap_clocks(reg);
 
+	regmap = devm_regmap_init_mmio(&pdev->dev, reg,
+				       &sun7i_a20_ccu_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+
 	return sunxi_ccu_probe(pdev->dev.of_node, reg, &sun7i_a20_ccu_desc);
 }
 
-- 
2.25.2

Re: [PATCH 2/4] clk: sunxi-ng: a20: export a regmap to access the GMAC register

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1428 bytes --]

On Sat, Apr 18, 2020 at 01:17:28AM +0300, Priit Laes wrote:
> Only GMAC register is allowed to be written, read access to registers
> is not restricted.
> 
> Export a regmap of the CCU.
> 
> Signed-off-by: Priit Laes <plaes@plaes.org>
> ---
>  drivers/clk/sunxi-ng/ccu-sun4i-a10.c | 31 ++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun4i-a10.c b/drivers/clk/sunxi-ng/ccu-sun4i-a10.c
> index 839e9d5a1cff..cba51c2c7eba 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun4i-a10.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun4i-a10.c
> @@ -1426,6 +1426,30 @@ static const struct sunxi_ccu_desc sun7i_a20_ccu_desc = {
>  	.num_resets	= ARRAY_SIZE(sunxi_a10_a20_ccu_resets),
>  };
>  
> +/*
> + * Add regmap for the GMAC driver (dwmac-sun8i) to allow access to
> + * GMAC configuration register.
> + */
> +
> +#define SUN7I_A20_GMAC_CFG_REG 0x164
> +static bool sun7i_a20_ccu_regmap_accessible_reg(struct device *dev,
> +						unsigned int reg)
> +{
> +	if (reg == SUN7I_A20_GMAC_CFG_REG)
> +		return true;
> +	return false;
> +}
> +
> +static struct regmap_config sun7i_a20_ccu_regmap_config = {
> +	.reg_bits	= 32,
> +	.val_bits	= 32,
> +	.reg_stride	= 4,
> +	.max_register	= 0x1f4, /* clk_out_b */

As far as I know, clk_out_b is a register that is also modified through that
clock driver. How do you handle the concurrent accesses?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 3/4] net: stmmac: dwmac-sunxi: Implement syscon-based clock handling

[Priit Laes]

Convert the sun7i-gmac driver to use a regmap-based driver,
instead of relying on the custom clock implementation.

This allows to get rid of the last custom clock in the sun7i
device tree making the sun7i fully CCU-compatible.

Compatibility with existing devicetrees is retained.

Signed-off-by: Priit Laes <plaes@plaes.org>
---
 .../net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 124 ++++++++++++++++--
 1 file changed, 116 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c
index 0e1ca2cba3c7..3476920bc762 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c
@@ -12,8 +12,11 @@
 #include <linux/module.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
+#include <linux/of_device.h>
 #include <linux/of_net.h>
 #include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 
 #include "stmmac_platform.h"
 
@@ -22,10 +25,20 @@ struct sunxi_priv_data {
 	int clk_enabled;
 	struct clk *tx_clk;
 	struct regulator *regulator;
+	struct regmap_field *regmap_field;
+};
+
+/* EMAC clock register @ 0x164 in the CCU address range */
+static const struct reg_field ccu_reg_field = {
+	.reg = 0x164,
+	.lsb = 0,
+	.msb = 31,
 };
 
 #define SUN7I_GMAC_GMII_RGMII_RATE	125000000
 #define SUN7I_GMAC_MII_RATE		25000000
+#define SUN7I_A20_RGMII_CLK		((3 << 1) | (1 << 12))
+#define SUN7I_A20_MII_CLK		(1 << 12)
 
 static int sun7i_gmac_init(struct platform_device *pdev, void *priv)
 {
@@ -38,7 +51,20 @@ static int sun7i_gmac_init(struct platform_device *pdev, void *priv)
 			return ret;
 	}
 
-	/* Set GMAC interface port mode
+	if (gmac->regmap_field) {
+		if (phy_interface_mode_is_rgmii(gmac->interface)) {
+			regmap_field_write(gmac->regmap_field,
+					   SUN7I_A20_RGMII_CLK);
+			return clk_prepare_enable(gmac->tx_clk);
+		}
+		regmap_field_write(gmac->regmap_field, SUN7I_A20_MII_CLK);
+		return clk_enable(gmac->tx_clk);
+	}
+
+	/* Legacy devicetree support */
+
+	/*
+	 * Set GMAC interface port mode
 	 *
 	 * The GMAC TX clock lines are configured by setting the clock
 	 * rate, which then uses the auto-reparenting feature of the
@@ -62,9 +88,15 @@ static void sun7i_gmac_exit(struct platform_device *pdev, void *priv)
 {
 	struct sunxi_priv_data *gmac = priv;
 
-	if (gmac->clk_enabled) {
+	if (gmac->regmap_field) {
+		regmap_field_write(gmac->regmap_field, 0);
 		clk_disable(gmac->tx_clk);
-		gmac->clk_enabled = 0;
+	} else {
+		/* Legacy devicetree support */
+		if (gmac->clk_enabled) {
+			clk_disable(gmac->tx_clk);
+			gmac->clk_enabled = 0;
+		}
 	}
 	clk_unprepare(gmac->tx_clk);
 
@@ -72,10 +104,53 @@ static void sun7i_gmac_exit(struct platform_device *pdev, void *priv)
 		regulator_disable(gmac->regulator);
 }
 
+static struct regmap *sun7i_gmac_get_syscon_from_dev(struct device_node *node)
+{
+	struct device_node *syscon_node;
+	struct platform_device *syscon_pdev;
+	struct regmap *regmap = NULL;
+
+	syscon_node = of_parse_phandle(node, "syscon", 0);
+	if (!syscon_node)
+		return ERR_PTR(-ENODEV);
+
+	syscon_pdev = of_find_device_by_node(syscon_node);
+	if (!syscon_pdev) {
+		/* platform device might not be probed yet */
+		regmap = ERR_PTR(-EPROBE_DEFER);
+		goto out_put_node;
+	}
+
+	/* If no regmap is found then the other device driver is at fault */
+	regmap = dev_get_regmap(&syscon_pdev->dev, NULL);
+	if (!regmap)
+		regmap = ERR_PTR(-EINVAL);
+
+	platform_device_put(syscon_pdev);
+out_put_node:
+	of_node_put(syscon_node);
+	return regmap;
+}
+
 static void sun7i_fix_speed(void *priv, unsigned int speed)
 {
 	struct sunxi_priv_data *gmac = priv;
 
+	if (gmac->regmap_field) {
+		clk_disable(gmac->tx_clk);
+		clk_unprepare(gmac->tx_clk);
+		if (speed == 1000)
+			regmap_field_write(gmac->regmap_field,
+					   SUN7I_A20_RGMII_CLK);
+		else
+			regmap_field_write(gmac->regmap_field,
+					   SUN7I_A20_MII_CLK);
+		clk_prepare_enable(gmac->tx_clk);
+		return;
+	}
+
+	/* Legacy devicetree support... */
+
 	/* only GMII mode requires us to reconfigure the clock lines */
 	if (gmac->interface != PHY_INTERFACE_MODE_GMII)
 		return;
@@ -102,6 +177,8 @@ static int sun7i_gmac_probe(struct platform_device *pdev)
 	struct stmmac_resources stmmac_res;
 	struct sunxi_priv_data *gmac;
 	struct device *dev = &pdev->dev;
+	struct device_node *syscon_node;
+	struct regmap *regmap = NULL;
 	int ret;
 
 	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
@@ -124,11 +201,42 @@ static int sun7i_gmac_probe(struct platform_device *pdev)
 		goto err_remove_config_dt;
 	}
 
-	gmac->tx_clk = devm_clk_get(dev, "allwinner_gmac_tx");
-	if (IS_ERR(gmac->tx_clk)) {
-		dev_err(dev, "could not get tx clock\n");
-		ret = PTR_ERR(gmac->tx_clk);
-		goto err_remove_config_dt;
+	/* Attempt to fetch syscon node... */
+	syscon_node = of_parse_phandle(dev->of_node, "syscon", 0);
+	if (syscon_node) {
+		gmac->tx_clk = devm_clk_get(dev, "stmmaceth");
+		if (IS_ERR(gmac->tx_clk)) {
+			dev_err(dev, "Could not get TX clock\n");
+			ret = PTR_ERR(gmac->tx_clk);
+			goto err_remove_config_dt;
+		}
+
+		regmap = sun7i_gmac_get_syscon_from_dev(pdev->dev.of_node);
+		if (IS_ERR(regmap))
+			regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+								 "syscon");
+		if (IS_ERR(regmap)) {
+			ret = PTR_ERR(regmap);
+			dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret);
+			goto err_remove_config_dt;
+		}
+
+		gmac->regmap_field = devm_regmap_field_alloc(dev, regmap, ccu_reg_field);
+
+		if (IS_ERR(gmac->regmap_field)) {
+			ret = PTR_ERR(gmac->regmap_field);
+			dev_err(dev, "Unable to map syscon register: %d\n", ret);
+			goto err_remove_config_dt;
+		}
+	/* ...or fall back to legacy clock setup */
+	} else {
+		dev_info(dev, "Falling back to legacy devicetree support!\n");
+		gmac->tx_clk = devm_clk_get(dev, "allwinner_gmac_tx");
+		if (IS_ERR(gmac->tx_clk)) {
+			dev_err(dev, "could not get tx clock\n");
+			ret = PTR_ERR(gmac->tx_clk);
+			goto err_remove_config_dt;
+		}
 	}
 
 	/* Optional regulator for PHY */
-- 
2.25.2

Re: [PATCH 3/4] net: stmmac: dwmac-sunxi: Implement syscon-based clock handling

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 6991 bytes --]

On Sat, Apr 18, 2020 at 01:17:29AM +0300, Priit Laes wrote:
> Convert the sun7i-gmac driver to use a regmap-based driver,
> instead of relying on the custom clock implementation.
> 
> This allows to get rid of the last custom clock in the sun7i
> device tree making the sun7i fully CCU-compatible.
> 
> Compatibility with existing devicetrees is retained.
> 
> Signed-off-by: Priit Laes <plaes@plaes.org>
> ---
>  .../net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 124 ++++++++++++++++--
>  1 file changed, 116 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c
> index 0e1ca2cba3c7..3476920bc762 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c
> @@ -12,8 +12,11 @@
>  #include <linux/module.h>
>  #include <linux/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/of_device.h>
>  #include <linux/of_net.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
>  
>  #include "stmmac_platform.h"
>  
> @@ -22,10 +25,20 @@ struct sunxi_priv_data {
>  	int clk_enabled;
>  	struct clk *tx_clk;
>  	struct regulator *regulator;
> +	struct regmap_field *regmap_field;
> +};
> +
> +/* EMAC clock register @ 0x164 in the CCU address range */
> +static const struct reg_field ccu_reg_field = {
> +	.reg = 0x164,
> +	.lsb = 0,
> +	.msb = 31,
>  };
>  
>  #define SUN7I_GMAC_GMII_RGMII_RATE	125000000
>  #define SUN7I_GMAC_MII_RATE		25000000
> +#define SUN7I_A20_RGMII_CLK		((3 << 1) | (1 << 12))
> +#define SUN7I_A20_MII_CLK		(1 << 12)
>  
>  static int sun7i_gmac_init(struct platform_device *pdev, void *priv)
>  {
> @@ -38,7 +51,20 @@ static int sun7i_gmac_init(struct platform_device *pdev, void *priv)
>  			return ret;
>  	}
>  
> -	/* Set GMAC interface port mode
> +	if (gmac->regmap_field) {
> +		if (phy_interface_mode_is_rgmii(gmac->interface)) {
> +			regmap_field_write(gmac->regmap_field,
> +					   SUN7I_A20_RGMII_CLK);
> +			return clk_prepare_enable(gmac->tx_clk);
> +		}
> +		regmap_field_write(gmac->regmap_field, SUN7I_A20_MII_CLK);
> +		return clk_enable(gmac->tx_clk);
> +	}
> +
> +	/* Legacy devicetree support */
> +

Saying why exactly this is legacy would be great. Otherwise, we might end up
with a legacy2 devicetree support somewhere down the line and no idea what each
actually mean.

> +	/*
> +	 * Set GMAC interface port mode

Comments in net start on the first line

>  	 *
>  	 * The GMAC TX clock lines are configured by setting the clock
>  	 * rate, which then uses the auto-reparenting feature of the
> @@ -62,9 +88,15 @@ static void sun7i_gmac_exit(struct platform_device *pdev, void *priv)
>  {
>  	struct sunxi_priv_data *gmac = priv;
>  
> -	if (gmac->clk_enabled) {
> +	if (gmac->regmap_field) {
> +		regmap_field_write(gmac->regmap_field, 0);
>  		clk_disable(gmac->tx_clk);
> -		gmac->clk_enabled = 0;
> +	} else {
> +		/* Legacy devicetree support */
> +		if (gmac->clk_enabled) {
> +			clk_disable(gmac->tx_clk);
> +			gmac->clk_enabled = 0;
> +		}
>  	}
>  	clk_unprepare(gmac->tx_clk);
>  
> @@ -72,10 +104,53 @@ static void sun7i_gmac_exit(struct platform_device *pdev, void *priv)
>  		regulator_disable(gmac->regulator);
>  }
>  
> +static struct regmap *sun7i_gmac_get_syscon_from_dev(struct device_node *node)
> +{
> +	struct device_node *syscon_node;
> +	struct platform_device *syscon_pdev;
> +	struct regmap *regmap = NULL;
> +
> +	syscon_node = of_parse_phandle(node, "syscon", 0);
> +	if (!syscon_node)
> +		return ERR_PTR(-ENODEV);
> +
> +	syscon_pdev = of_find_device_by_node(syscon_node);
> +	if (!syscon_pdev) {
> +		/* platform device might not be probed yet */
> +		regmap = ERR_PTR(-EPROBE_DEFER);
> +		goto out_put_node;
> +	}
> +
> +	/* If no regmap is found then the other device driver is at fault */
> +	regmap = dev_get_regmap(&syscon_pdev->dev, NULL);
> +	if (!regmap)
> +		regmap = ERR_PTR(-EINVAL);
> +
> +	platform_device_put(syscon_pdev);
> +out_put_node:
> +	of_node_put(syscon_node);
> +	return regmap;
> +}
> +
>  static void sun7i_fix_speed(void *priv, unsigned int speed)
>  {
>  	struct sunxi_priv_data *gmac = priv;
>  
> +	if (gmac->regmap_field) {
> +		clk_disable(gmac->tx_clk);
> +		clk_unprepare(gmac->tx_clk);
> +		if (speed == 1000)
> +			regmap_field_write(gmac->regmap_field,
> +					   SUN7I_A20_RGMII_CLK);
> +		else
> +			regmap_field_write(gmac->regmap_field,
> +					   SUN7I_A20_MII_CLK);
> +		clk_prepare_enable(gmac->tx_clk);
> +		return;
> +	}
> +
> +	/* Legacy devicetree support... */
> +
>  	/* only GMII mode requires us to reconfigure the clock lines */
>  	if (gmac->interface != PHY_INTERFACE_MODE_GMII)
>  		return;
> @@ -102,6 +177,8 @@ static int sun7i_gmac_probe(struct platform_device *pdev)
>  	struct stmmac_resources stmmac_res;
>  	struct sunxi_priv_data *gmac;
>  	struct device *dev = &pdev->dev;
> +	struct device_node *syscon_node;
> +	struct regmap *regmap = NULL;
>  	int ret;
>  
>  	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> @@ -124,11 +201,42 @@ static int sun7i_gmac_probe(struct platform_device *pdev)
>  		goto err_remove_config_dt;
>  	}
>  
> -	gmac->tx_clk = devm_clk_get(dev, "allwinner_gmac_tx");
> -	if (IS_ERR(gmac->tx_clk)) {
> -		dev_err(dev, "could not get tx clock\n");
> -		ret = PTR_ERR(gmac->tx_clk);
> -		goto err_remove_config_dt;
> +	/* Attempt to fetch syscon node... */
> +	syscon_node = of_parse_phandle(dev->of_node, "syscon", 0);
> +	if (syscon_node) {
> +		gmac->tx_clk = devm_clk_get(dev, "stmmaceth");
> +		if (IS_ERR(gmac->tx_clk)) {
> +			dev_err(dev, "Could not get TX clock\n");
> +			ret = PTR_ERR(gmac->tx_clk);
> +			goto err_remove_config_dt;
> +		}
> +
> +		regmap = sun7i_gmac_get_syscon_from_dev(pdev->dev.of_node);
> +		if (IS_ERR(regmap))
> +			regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +								 "syscon");
> +		if (IS_ERR(regmap)) {
> +			ret = PTR_ERR(regmap);
> +			dev_err(&pdev->dev, "Unable to map syscon: %d\n", ret);
> +			goto err_remove_config_dt;
> +		}
> +
> +		gmac->regmap_field = devm_regmap_field_alloc(dev, regmap, ccu_reg_field);
> +
> +		if (IS_ERR(gmac->regmap_field)) {
> +			ret = PTR_ERR(gmac->regmap_field);
> +			dev_err(dev, "Unable to map syscon register: %d\n", ret);
> +			goto err_remove_config_dt;
> +		}
> +	/* ...or fall back to legacy clock setup */
> +	} else {
> +		dev_info(dev, "Falling back to legacy devicetree support!\n");
> +		gmac->tx_clk = devm_clk_get(dev, "allwinner_gmac_tx");
> +		if (IS_ERR(gmac->tx_clk)) {
> +			dev_err(dev, "could not get tx clock\n");
> +			ret = PTR_ERR(gmac->tx_clk);
> +			goto err_remove_config_dt;
> +		}
>  	}
>  
>  	/* Optional regulator for PHY */
> -- 
> 2.25.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 4/4] ARM: dts: sun7i: Use syscon-based implementation for gmac

[Priit Laes]

Use syscon-based approach to access gmac clock configuration
register, instead of relying on a custom clock driver.

As a bonus, we can now drop the custom clock implementation
and dummy clocks making sun7i fully CCU-compatible.

Signed-off-by: Priit Laes <plaes@plaes.org>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 36 +++-----------------------------
 1 file changed, 3 insertions(+), 33 deletions(-)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index ffe1d10a1a84..750962a94fad 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -219,37 +219,6 @@ osc32k: clk-32k {
 			clock-frequency = <32768>;
 			clock-output-names = "osc32k";
 		};
-
-		/*
-		 * The following two are dummy clocks, placeholders
-		 * used in the gmac_tx clock. The gmac driver will
-		 * choose one parent depending on the PHY interface
-		 * mode, using clk_set_rate auto-reparenting.
-		 *
-		 * The actual TX clock rate is not controlled by the
-		 * gmac_tx clock.
-		 */
-		mii_phy_tx_clk: clk-mii-phy-tx {
-			#clock-cells = <0>;
-			compatible = "fixed-clock";
-			clock-frequency = <25000000>;
-			clock-output-names = "mii_phy_tx";
-		};
-
-		gmac_int_tx_clk: clk-gmac-int-tx {
-			#clock-cells = <0>;
-			compatible = "fixed-clock";
-			clock-frequency = <125000000>;
-			clock-output-names = "gmac_int_tx";
-		};
-
-		gmac_tx_clk: clk@1c20164 {
-			#clock-cells = <0>;
-			compatible = "allwinner,sun7i-a20-gmac-clk";
-			reg = <0x01c20164 0x4>;
-			clocks = <&mii_phy_tx_clk>, <&gmac_int_tx_clk>;
-			clock-output-names = "gmac_tx";
-		};
 	};
 
 
@@ -1511,11 +1480,12 @@ mali: gpu@1c40000 {
 
 		gmac: ethernet@1c50000 {
 			compatible = "allwinner,sun7i-a20-gmac";
+			syscon = <&ccu>;
 			reg = <0x01c50000 0x10000>;
 			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "macirq";
-			clocks = <&ccu CLK_AHB_GMAC>, <&gmac_tx_clk>;
-			clock-names = "stmmaceth", "allwinner_gmac_tx";
+			clocks = <&ccu CLK_AHB_GMAC>;
+			clock-names = "stmmaceth";
 			snps,pbl = <2>;
 			snps,fixed-burst;
 			snps,force_sf_dma_mode;
-- 
2.25.2

Re: [PATCH 4/4] ARM: dts: sun7i: Use syscon-based implementation for gmac

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]

On Sat, Apr 18, 2020 at 01:17:30AM +0300, Priit Laes wrote:
> Use syscon-based approach to access gmac clock configuration
> register, instead of relying on a custom clock driver.
> 
> As a bonus, we can now drop the custom clock implementation
> and dummy clocks making sun7i fully CCU-compatible.
> 
> Signed-off-by: Priit Laes <plaes@plaes.org>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi | 36 +++-----------------------------
>  1 file changed, 3 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index ffe1d10a1a84..750962a94fad 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -219,37 +219,6 @@ osc32k: clk-32k {
>  			clock-frequency = <32768>;
>  			clock-output-names = "osc32k";
>  		};
> -
> -		/*
> -		 * The following two are dummy clocks, placeholders
> -		 * used in the gmac_tx clock. The gmac driver will
> -		 * choose one parent depending on the PHY interface
> -		 * mode, using clk_set_rate auto-reparenting.
> -		 *
> -		 * The actual TX clock rate is not controlled by the
> -		 * gmac_tx clock.
> -		 */
> -		mii_phy_tx_clk: clk-mii-phy-tx {
> -			#clock-cells = <0>;
> -			compatible = "fixed-clock";
> -			clock-frequency = <25000000>;
> -			clock-output-names = "mii_phy_tx";
> -		};
> -
> -		gmac_int_tx_clk: clk-gmac-int-tx {
> -			#clock-cells = <0>;
> -			compatible = "fixed-clock";
> -			clock-frequency = <125000000>;
> -			clock-output-names = "gmac_int_tx";
> -		};
> -
> -		gmac_tx_clk: clk@1c20164 {
> -			#clock-cells = <0>;
> -			compatible = "allwinner,sun7i-a20-gmac-clk";
> -			reg = <0x01c20164 0x4>;
> -			clocks = <&mii_phy_tx_clk>, <&gmac_int_tx_clk>;
> -			clock-output-names = "gmac_tx";
> -		};
>  	};
>  
>  
> @@ -1511,11 +1480,12 @@ mali: gpu@1c40000 {
>  
>  		gmac: ethernet@1c50000 {
>  			compatible = "allwinner,sun7i-a20-gmac";
> +			syscon = <&ccu>;
>  			reg = <0x01c50000 0x10000>;
>  			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "macirq";
> -			clocks = <&ccu CLK_AHB_GMAC>, <&gmac_tx_clk>;
> -			clock-names = "stmmaceth", "allwinner_gmac_tx";
> +			clocks = <&ccu CLK_AHB_GMAC>;
> +			clock-names = "stmmaceth";

I guess you also need to update the binding so that it considers it valid?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/4] ARM: dts: sun7i: Use syscon-based implementation for gmac

[Priit Laes]

On Mon, Apr 20, 2020 at 02:59:19PM +0200, Maxime Ripard wrote:
> On Sat, Apr 18, 2020 at 01:17:30AM +0300, Priit Laes wrote:
> > Use syscon-based approach to access gmac clock configuration
> > register, instead of relying on a custom clock driver.
> > 
> > As a bonus, we can now drop the custom clock implementation
> > and dummy clocks making sun7i fully CCU-compatible.
> > 
> > Signed-off-by: Priit Laes <plaes@plaes.org>
> > ---
> >  arch/arm/boot/dts/sun7i-a20.dtsi | 36 +++-----------------------------
> >  1 file changed, 3 insertions(+), 33 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> > index ffe1d10a1a84..750962a94fad 100644
> > --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> > @@ -219,37 +219,6 @@ osc32k: clk-32k {
> >  			clock-frequency = <32768>;
> >  			clock-output-names = "osc32k";
> >  		};
> > -
> > -		/*
> > -		 * The following two are dummy clocks, placeholders
> > -		 * used in the gmac_tx clock. The gmac driver will
> > -		 * choose one parent depending on the PHY interface
> > -		 * mode, using clk_set_rate auto-reparenting.
> > -		 *
> > -		 * The actual TX clock rate is not controlled by the
> > -		 * gmac_tx clock.
> > -		 */
> > -		mii_phy_tx_clk: clk-mii-phy-tx {
> > -			#clock-cells = <0>;
> > -			compatible = "fixed-clock";
> > -			clock-frequency = <25000000>;
> > -			clock-output-names = "mii_phy_tx";
> > -		};
> > -
> > -		gmac_int_tx_clk: clk-gmac-int-tx {
> > -			#clock-cells = <0>;
> > -			compatible = "fixed-clock";
> > -			clock-frequency = <125000000>;
> > -			clock-output-names = "gmac_int_tx";
> > -		};
> > -
> > -		gmac_tx_clk: clk@1c20164 {
> > -			#clock-cells = <0>;
> > -			compatible = "allwinner,sun7i-a20-gmac-clk";
> > -			reg = <0x01c20164 0x4>;
> > -			clocks = <&mii_phy_tx_clk>, <&gmac_int_tx_clk>;
> > -			clock-output-names = "gmac_tx";
> > -		};
> >  	};
> >  
> >  
> > @@ -1511,11 +1480,12 @@ mali: gpu@1c40000 {
> >  
> >  		gmac: ethernet@1c50000 {
> >  			compatible = "allwinner,sun7i-a20-gmac";
> > +			syscon = <&ccu>;
> >  			reg = <0x01c50000 0x10000>;
> >  			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
> >  			interrupt-names = "macirq";
> > -			clocks = <&ccu CLK_AHB_GMAC>, <&gmac_tx_clk>;
> > -			clock-names = "stmmaceth", "allwinner_gmac_tx";
> > +			clocks = <&ccu CLK_AHB_GMAC>;
> > +			clock-names = "stmmaceth";
> 
> I guess you also need to update the binding so that it considers it valid?

Yes, will do it in the next round.

> 
> Maxime

Re: [PATCH 0/4] ARM: sun7i: Convert A20 GMAC driver to CCU

[Priit Laes]

On Sat, Apr 18, 2020 at 01:17:26AM +0300, Priit Laes wrote:
> This serie converts Allwinner A20 (sun7i) GMAC driver to CCU
> while still retaining compatibility with existing devicetrees.
> 
> First two patches contain preliminary work which convert
> sun4i/sun7i clock drivers to platform devices and creates regmap
> to access gmac register from the sun7i gmac driver.
> 
> Third patch implements syscon-based regmap to allow driver manage
> its own clock source.
> 
> Fourth patch updates the devicetree and drops the unused clocks.
> 
> While testing the driver I noticed following bugs with the existing
> sun7i gmac driver:
> - driver relies on u-boot for initialization (fixed in this
>   implementation)

Scratch that.. this is actually due to unhandled rx and tx delays,
which I "accidentally" fixed by copying the value BIT(12) from the
u-boot..

> - `systemctl restart networking` fails to bring the link up again.
> 
> 
> Priit Laes (4):
>   clk: sunxi-ng: a10/a20: rewrite init code to a platform driver
>   clk: sunxi-ng: a20: export a regmap to access the GMAC register
>   net: stmmac: dwmac-sunxi: Implement syscon-based clock handling
>   ARM: dts: sun7i: Use syscon-based implementation for gmac
> 
>  arch/arm/boot/dts/sun7i-a20.dtsi              |  36 +----
>  drivers/clk/sunxi-ng/ccu-sun4i-a10.c          | 108 ++++++++++++---
>  .../net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 124 ++++++++++++++++--
>  3 files changed, 206 insertions(+), 62 deletions(-)
> 
> -- 
> 2.25.2
>