Hi, thank you for your review!

3/8/21 4:36 PM, Maxime Ripard пишет:
Hi,

On Sun, Mar 07, 2021 at 06:13:51AM +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 <bo...@wirenboard.com>
On which device was it tested?
It's custom-made Allwinner A40i device with two IP101GRI PHYs in MII mode.
---
  drivers/net/ethernet/allwinner/sun4i-emac.c | 21 ++++++++++++++++++++-
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c 
b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 5ed80d9a6b9f..c0ae06dd922c 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"
@@ -85,6 +86,7 @@ struct emac_board_info {
        unsigned int            link;
        unsigned int            speed;
        unsigned int            duplex;
+       struct reset_control *reset;
You should align this with the rest of the other fields


phy_interface_t phy_interface;
  };
@@ -791,6 +793,7 @@ static int emac_probe(struct platform_device *pdev)
        struct net_device *ndev;
        int ret = 0;
        const char *mac_addr;
+       struct reset_control *reset;
ndev = alloc_etherdev(sizeof(struct emac_board_info));
        if (!ndev) {
@@ -852,6 +855,19 @@ static int emac_probe(struct platform_device *pdev)
                goto out_release_sram;
        }
+ reset = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL);
+       if (IS_ERR(reset)) {
+               dev_err(&pdev->dev, "unable to request reset\n");
+               ret = -ENODEV;
+               goto out_release_sram;
+       }
Judging from your commit log, it's not really optional for the R40. The
way we usually deal with this is to have a structure associated with a
new compatible and have a flag tell if that compatible requires a reset
line or not.

The dt binding should also be amended to allow the reset property

got it
+       db->reset = reset;
+       ret = reset_control_deassert(db->reset);
+       if (ret) {
+               dev_err(&pdev->dev, "could not deassert EMAC reset\n");
+               goto out_release_sram;
+       }
+
The programming guidelines in the datasheet ask that the reset line must
be deasserted before the clock in enabled.
right, found it at section 3.3.2.6, thanks

Maxime

Reply via email to