2021-03-09 01:23:50

by Evgeny Boger

[permalink] [raw]
Subject: [PATCH v2 1/2] net: allwinner: reset control support

R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP.
However, on R40 the EMAC is gated by default.

Signed-off-by: Evgeny Boger <[email protected]>
---
.../net/allwinner,sun4i-a10-emac.yaml | 11 +++-
drivers/net/ethernet/allwinner/sun4i-emac.c | 65 +++++++++++++++++--
2 files changed, 70 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
index 8d8560a67abf..27f99372d153 100644
--- a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
+++ b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
@@ -15,7 +15,12 @@ maintainers:

properties:
compatible:
- const: allwinner,sun4i-a10-emac
+ oneOf:
+ - const: allwinner,sun4i-a10-emac
+ - const: allwinner,sun4i-r40-emac
+ - items:
+ - const: allwinner,sun4i-r40-emac
+ - const: allwinner,sun4i-a10-emac

reg:
maxItems: 1
@@ -30,6 +35,9 @@ properties:
description: Phandle to the device SRAM
$ref: /schemas/types.yaml#/definitions/phandle-array

+ resets:
+ maxItems: 1
+
required:
- compatible
- reg
@@ -47,6 +55,7 @@ examples:
reg = <0x01c0b000 0x1000>;
interrupts = <55>;
clocks = <&ahb_gates 17>;
+ resets = <&ccu RST_BUS_EMAC>;
phy-handle = <&phy0>;
allwinner,sram = <&emac_sram 1>;
};
diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 5ed80d9a6b9f..b26913610a38 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -28,6 +28,7 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/phy.h>
+#include <linux/reset.h>
#include <linux/soc/sunxi/sunxi_sram.h>

#include "sun4i-emac.h"
@@ -68,6 +69,15 @@ MODULE_PARM_DESC(watchdog, "transmit timeout in milliseconds");
* devices, EMACA and EMACB.
*/

+/**
+ * struct emac_quirks - Differences between SoC variants.
+ *
+ * @has_reset: SoC needs reset deasserted.
+ */
+struct emac_quirks {
+ bool has_reset;
+};
+
struct emac_board_info {
struct clk *clk;
struct device *dev;
@@ -85,6 +95,7 @@ struct emac_board_info {
unsigned int link;
unsigned int speed;
unsigned int duplex;
+ struct reset_control *reset;

phy_interface_t phy_interface;
};
@@ -791,6 +802,7 @@ static int emac_probe(struct platform_device *pdev)
struct net_device *ndev;
int ret = 0;
const char *mac_addr;
+ const struct emac_quirks *quirks;

ndev = alloc_etherdev(sizeof(struct emac_board_info));
if (!ndev) {
@@ -809,6 +821,13 @@ static int emac_probe(struct platform_device *pdev)

spin_lock_init(&db->lock);

+ quirks = of_device_get_match_data(&pdev->dev);
+ if (!quirks) {
+ dev_err(&pdev->dev, "Failed to determine the quirks to use\n");
+ ret = -ENODEV;
+ goto out;
+ }
+
db->membase = of_iomap(np, 0);
if (!db->membase) {
dev_err(&pdev->dev, "failed to remap registers\n");
@@ -825,16 +844,31 @@ static int emac_probe(struct platform_device *pdev)
goto out_iounmap;
}

+ if (quirks->has_reset) {
+ db->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(db->reset)) {
+ dev_err(&pdev->dev, "unable to request reset\n");
+ ret = PTR_ERR(db->reset);
+ goto out_dispose_mapping;
+ }
+
+ ret = reset_control_deassert(db->reset);
+ if (ret) {
+ dev_err(&pdev->dev, "could not deassert EMAC reset\n");
+ goto out_dispose_mapping;
+ }
+ }
+
db->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(db->clk)) {
ret = PTR_ERR(db->clk);
- goto out_dispose_mapping;
+ goto out_assert_reset;
}

ret = clk_prepare_enable(db->clk);
if (ret) {
dev_err(&pdev->dev, "Error couldn't enable clock (%d)\n", ret);
- goto out_dispose_mapping;
+ goto out_assert_reset;
}

ret = sunxi_sram_claim(&pdev->dev);
@@ -852,6 +886,7 @@ static int emac_probe(struct platform_device *pdev)
goto out_release_sram;
}

+
/* Read MAC-address from DT */
mac_addr = of_get_mac_address(np);
if (!IS_ERR(mac_addr))
@@ -893,6 +928,8 @@ static int emac_probe(struct platform_device *pdev)
sunxi_sram_release(&pdev->dev);
out_clk_disable_unprepare:
clk_disable_unprepare(db->clk);
+out_assert_reset:
+ reset_control_assert(db->reset);
out_dispose_mapping:
irq_dispose_mapping(ndev->irq);
out_iounmap:
@@ -913,6 +950,7 @@ static int emac_remove(struct platform_device *pdev)
unregister_netdev(ndev);
sunxi_sram_release(&pdev->dev);
clk_disable_unprepare(db->clk);
+ reset_control_assert(db->reset);
irq_dispose_mapping(ndev->irq);
iounmap(db->membase);
free_netdev(ndev);
@@ -944,11 +982,28 @@ static int emac_resume(struct platform_device *dev)
return 0;
}

-static const struct of_device_id emac_of_match[] = {
- {.compatible = "allwinner,sun4i-a10-emac",},
+static const struct emac_quirks sun4i_a10_emac_quirks = {
+ .has_reset = false,
+};

+static const struct emac_quirks sun4i_r40_emac_quirks = {
+ .has_reset = true,
+};
+
+static const struct of_device_id emac_of_match[] = {
+ {
+ .compatible = "allwinner,sun4i-a10-emac",
+ .data = &sun4i_a10_emac_quirks
+ },
+ {
+ .compatible = "allwinner,sun4i-r40-emac",
+ .data = &sun4i_r40_emac_quirks
+ },
/* Deprecated */
- {.compatible = "allwinner,sun4i-emac",},
+ {
+ .compatible = "allwinner,sun4i-emac",
+ .data = &sun4i_a10_emac_quirks
+ },
{},
};

--
2.17.1


2021-03-09 17:20:50

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] net: allwinner: reset control support

On Tue, 09 Mar 2021 04:21:15 +0300, Evgeny Boger wrote:
> R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP.
> However, on R40 the EMAC is gated by default.
>
> Signed-off-by: Evgeny Boger <[email protected]>
> ---
> .../net/allwinner,sun4i-a10-emac.yaml | 11 +++-
> drivers/net/ethernet/allwinner/sun4i-emac.c | 65 +++++++++++++++++--
> 2 files changed, 70 insertions(+), 6 deletions(-)
>

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.example.dts:24.28-29 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.example.dt.yaml] Error 1
make: *** [Makefile:1380: dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1449498

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2021-03-10 08:41:52

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] net: allwinner: reset control support

Hi,

On Tue, Mar 09, 2021 at 04:21:15AM +0300, Evgeny Boger wrote:
> R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP.
> However, on R40 the EMAC is gated by default.
>
> Signed-off-by: Evgeny Boger <[email protected]>
> ---
> .../net/allwinner,sun4i-a10-emac.yaml | 11 +++-
> drivers/net/ethernet/allwinner/sun4i-emac.c | 65 +++++++++++++++++--
> 2 files changed, 70 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
> index 8d8560a67abf..27f99372d153 100644
> --- a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
> +++ b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
> @@ -15,7 +15,12 @@ maintainers:
>
> properties:
> compatible:
> - const: allwinner,sun4i-a10-emac
> + oneOf:
> + - const: allwinner,sun4i-a10-emac
> + - const: allwinner,sun4i-r40-emac
> + - items:
> + - const: allwinner,sun4i-r40-emac
> + - const: allwinner,sun4i-a10-emac

There's no need to handle the fallback case, it should have either one
of the two, not both.

The good news is that it simplifies the binding here too, since you can
use an enum

The DT binding modifications are usually in a separate patch too

>
> reg:
> maxItems: 1
> @@ -30,6 +35,9 @@ properties:
> description: Phandle to the device SRAM
> $ref: /schemas/types.yaml#/definitions/phandle-array
>
> + resets:
> + maxItems: 1
> +

You should make resets required for the R40 compatible too through an if
clause.

It looks good otherwise, thanks!
Maxime


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