2020-04-17 22:30:31

by Priit Laes

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

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


2020-04-17 22:30:33

by Priit Laes

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

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 <[email protected]>
---
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

2020-04-17 22:30:40

by Priit Laes

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

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 <[email protected]>
---
.../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

2020-04-20 13:01:09

by Maxime Ripard

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

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 <[email protected]>
> ---
> .../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
>


Attachments:
(No filename) (6.83 kB)
signature.asc (235.00 B)
Download all attachments

2020-04-20 13:01:52

by Maxime Ripard

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

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 <[email protected]>
> ---
> 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


Attachments:
(No filename) (2.32 kB)
signature.asc (235.00 B)
Download all attachments

2020-04-20 13:47:32

by Priit Laes

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

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
>

2020-04-20 16:37:06

by Priit Laes

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

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 <[email protected]>
> > ---
> > 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