From: Richard Leitner <d...@g0hl1n.net> Sent: Monday, November 20, 2017 4:34 PM
>The fec_reset_phy function allowed only one execution during probeing.
>To make it more usable move the dt parsing and gpio allocation to the probe
>function. The parameters of the phy reset are added to the fec_enet_private
>struct. As a result the fec_reset_phy function may be called anytime after
>probe.
>
>One checkpatch.pl warning (too long line) is ignored. This is due to the fact a
>string (dt property name) otherwise needs to be split over multiple lines,
>which is counterproductive for the readability.
>
>Signed-off-by: Richard Leitner <richard.leit...@skidata.com>
>---

It is better to convert to gpio descriptor, and use dts gpio flag as the gpio 
polarity instead of extra phy_reset_active_high variable.

Regards,
Andy

> drivers/net/ethernet/freescale/fec.h      |  4 ++
> drivers/net/ethernet/freescale/fec_main.c | 88 ++++++++++++++++----------
>-----
> 2 files changed, 50 insertions(+), 42 deletions(-)
>
>diff --git a/drivers/net/ethernet/freescale/fec.h
>b/drivers/net/ethernet/freescale/fec.h
>index 5385074b3b7d..401c4eabf08a 100644
>--- a/drivers/net/ethernet/freescale/fec.h
>+++ b/drivers/net/ethernet/freescale/fec.h
>@@ -539,6 +539,10 @@ struct fec_enet_private {
>       int     pause_flag;
>       int     wol_flag;
>       u32     quirks;
>+      int     phy_reset;
>+      int     phy_reset_duration;
>+      int     phy_reset_post_delay;
>+      bool    phy_reset_active_high;
>
>       struct  napi_struct napi;
>       int     csum_flags;
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index 610573855213..06a7caca0cee 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -3212,62 +3212,36 @@ static int fec_enet_init(struct net_device *ndev)  }
>
> #ifdef CONFIG_OF
>-static int fec_reset_phy(struct platform_device *pdev)
>+static int fec_reset_phy(struct net_device *ndev)
> {
>-      int err, phy_reset;
>-      bool active_high = false;
>-      int msec = 1, phy_post_delay = 0;
>-      struct device_node *np = pdev->dev.of_node;
>-
>-      if (!np)
>-              return 0;
>-
>-      err = of_property_read_u32(np, "phy-reset-duration", &msec);
>-      /* A sane reset duration should not be longer than 1s */
>-      if (!err && msec > 1000)
>-              msec = 1;
>+      struct fec_enet_private *fep = netdev_priv(ndev);
>
>-      phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
>-      if (phy_reset == -EPROBE_DEFER)
>-              return phy_reset;
>-      else if (!gpio_is_valid(phy_reset))
>+      if (!fep->phy_reset)
>               return 0;
>
>-      err = of_property_read_u32(np, "phy-reset-post-delay",
>&phy_post_delay);
>-      /* valid reset duration should be less than 1s */
>-      if (!err && phy_post_delay > 1000)
>-              return -EINVAL;
>-
>-      active_high = of_property_read_bool(np, "phy-reset-active-high");
>+      gpio_set_value_cansleep(fep->phy_reset, fep-
>>phy_reset_active_high);
>
>-      err = devm_gpio_request_one(&pdev->dev, phy_reset,
>-                      active_high ? GPIOF_OUT_INIT_HIGH :
>GPIOF_OUT_INIT_LOW,
>-                      "phy-reset");
>-      if (err) {
>-              dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n",
>err);
>-              return err;
>-      }
>-
>-      if (msec > 20)
>-              msleep(msec);
>+      if (fep->phy_reset_duration > 20)
>+              msleep(fep->phy_reset_duration);
>       else
>-              usleep_range(msec * 1000, msec * 1000 + 1000);
>+              usleep_range(fep->phy_reset_duration * 1000,
>+                           fep->phy_reset_duration * 1000 + 1000);
>
>-      gpio_set_value_cansleep(phy_reset, !active_high);
>+      gpio_set_value_cansleep(fep->phy_reset, !fep-
>>phy_reset_active_high);
>
>-      if (!phy_post_delay)
>+      if (!fep->phy_reset_post_delay)
>               return 0;
>
>-      if (phy_post_delay > 20)
>-              msleep(phy_post_delay);
>+      if (fep->phy_reset_post_delay > 20)
>+              msleep(fep->phy_reset_post_delay);
>       else
>-              usleep_range(phy_post_delay * 1000,
>-                           phy_post_delay * 1000 + 1000);
>+              usleep_range(fep->phy_reset_post_delay * 1000,
>+                           fep->phy_reset_post_delay * 1000 + 1000);
>
>       return 0;
> }
> #else /* CONFIG_OF */
>-static int fec_reset_phy(struct platform_device *pdev)
>+static int fec_reset_phy(struct net_device *ndev)
> {
>       /*
>        * In case of platform probe, the reset has been done @@ -3400,6
>+3374,36 @@ fec_probe(struct platform_device *pdev)
>       }
>       fep->phy_node = phy_node;
>
>+      fep->phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
>+      if (gpio_is_valid(fep->phy_reset)) {
>+              ret = of_property_read_u32(np, "phy-reset-duration",
>+                                         &fep->phy_reset_duration);
>+              /* A sane reset duration should not be longer than 1s */
>+              if (!ret && fep->phy_reset_post_delay > 1000)
>+                      fep->phy_reset_post_delay = 1;
>+
>+              ret = of_property_read_u32(np, "phy-reset-post-delay",
>+                                         &fep->phy_reset_post_delay);
>+              /* valid post reset delay should be less than 1s */
>+              if (!ret && fep->phy_reset_post_delay > 1000)
>+                      fep->phy_reset_post_delay = 1;
>+
>+              fep->phy_reset_active_high = of_property_read_bool(np,
>+"phy-reset-active-high");
>+
>+              ret = devm_gpio_request_one(&pdev->dev, fep->phy_reset,
>+                                          fep->phy_reset_active_high ?
>+                                          GPIOF_OUT_INIT_HIGH :
>+                                          GPIOF_OUT_INIT_LOW,
>+                                          "phy-reset");
>+              if (ret) {
>+                      dev_err(&pdev->dev, "failed to get reset-
>gpios: %d\n",
>+                              ret);
>+                      goto failed_phy;
>+              }
>+      } else {
>+              fep->phy_reset = 0;
>+      }
>+
>       ret = of_get_phy_mode(pdev->dev.of_node);
>       if (ret < 0) {
>               pdata = dev_get_platdata(&pdev->dev); @@ -3472,7 +3476,7
>@@ fec_probe(struct platform_device *pdev)
>       pm_runtime_set_active(&pdev->dev);
>       pm_runtime_enable(&pdev->dev);
>
>-      ret = fec_reset_phy(pdev);
>+      ret = fec_reset_phy(ndev);
>       if (ret)
>               goto failed_reset;
>
>--
>2.11.0

Reply via email to