netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] net: fec: optionally reset PHY via a reset-controller
@ 2019-07-15 21:05 Sven Van Asbroeck
  2019-07-16  2:02 ` [EXT] " Andy Duan
  0 siblings, 1 reply; 6+ messages in thread
From: Sven Van Asbroeck @ 2019-07-15 21:05 UTC (permalink / raw)
  To: Fugang Duan; +Cc: David S . Miller, netdev, linux-kernel

The current fec driver allows the PHY to be reset via a gpio,
specified in the devicetree. However, some PHYs need to be reset
in a more complex way.

To accommodate such PHYs, allow an optional reset controller
in the fec devicetree. If no reset controller is found, the
fec will fall back to the legacy reset behaviour.

Example:
&fec {
	phy-mode = "rgmii";
	resets = <&phy_reset>;
	reset-names = "phy";
	status = "okay";
};

Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---

Will send a Documentation patch if this receives a positive review.

 drivers/net/ethernet/freescale/fec_main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 38f10f7dcbc3..5a5f3ed6f16d 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -61,6 +61,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/if_vlan.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/reset.h>
 #include <linux/prefetch.h>
 #include <soc/imx/cpuidle.h>
 
@@ -3335,6 +3336,7 @@ static int fec_enet_get_irq_cnt(struct platform_device *pdev)
 static int
 fec_probe(struct platform_device *pdev)
 {
+	struct reset_control *phy_reset;
 	struct fec_enet_private *fep;
 	struct fec_platform_data *pdata;
 	struct net_device *ndev;
@@ -3490,7 +3492,9 @@ fec_probe(struct platform_device *pdev)
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
-	ret = fec_reset_phy(pdev);
+	phy_reset = devm_reset_control_get_exclusive(&pdev->dev, "phy");
+	ret = IS_ERR(phy_reset) ? fec_reset_phy(pdev) :
+			reset_control_reset(phy_reset);
 	if (ret)
 		goto failed_reset;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [EXT] [PATCH v1] net: fec: optionally reset PHY via a reset-controller
  2019-07-15 21:05 [PATCH v1] net: fec: optionally reset PHY via a reset-controller Sven Van Asbroeck
@ 2019-07-16  2:02 ` Andy Duan
  2019-07-16 13:19   ` Sven Van Asbroeck
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Duan @ 2019-07-16  2:02 UTC (permalink / raw)
  To: Sven Van Asbroeck; +Cc: David S . Miller, netdev, linux-kernel

From: Sven Van Asbroeck <thesven73@gmail.com> Sent: Tuesday, July 16, 2019 5:05 AM
> The current fec driver allows the PHY to be reset via a gpio, specified in the
> devicetree. However, some PHYs need to be reset in a more complex way.
> 
> To accommodate such PHYs, allow an optional reset controller in the fec
> devicetree. If no reset controller is found, the fec will fall back to the legacy
> reset behaviour.
> 
> Example:
> &fec {
>         phy-mode = "rgmii";
>         resets = <&phy_reset>;
>         reset-names = "phy";
>         status = "okay";
> };
> 
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---
> 
> Will send a Documentation patch if this receives a positive review.
> 
>  drivers/net/ethernet/freescale/fec_main.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 38f10f7dcbc3..5a5f3ed6f16d 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -61,6 +61,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/if_vlan.h>
>  #include <linux/pinctrl/consumer.h>
> +#include <linux/reset.h>
>  #include <linux/prefetch.h>
>  #include <soc/imx/cpuidle.h>
> 
> @@ -3335,6 +3336,7 @@ static int fec_enet_get_irq_cnt(struct
> platform_device *pdev)  static int  fec_probe(struct platform_device *pdev)
> {
> +       struct reset_control *phy_reset;
>         struct fec_enet_private *fep;
>         struct fec_platform_data *pdata;
>         struct net_device *ndev;
> @@ -3490,7 +3492,9 @@ fec_probe(struct platform_device *pdev)
>         pm_runtime_set_active(&pdev->dev);
>         pm_runtime_enable(&pdev->dev);
> 
> -       ret = fec_reset_phy(pdev);
> +       phy_reset = devm_reset_control_get_exclusive(&pdev->dev,
> "phy");
> +       ret = IS_ERR(phy_reset) ? fec_reset_phy(pdev) :
> +                       reset_control_reset(phy_reset);
>         if (ret)
>                 goto failed_reset;

The patch looks fine.
But the reset mechanism in the driver should be abandoned since
the phylib already can handle mii bus reset and phy device reset like
below piece code:

of_mdiobus_register()
    of_mdiobus_register_phy()
        phy_device_register()
            mdiobus_register_device()
                /* Assert the reset signal */
                mdio_device_reset(mdiodev, 1);
            /* Deassert the reset signal */
            mdio_device_reset(&phydev->mdio, 0);

dts:
        ethernet-phy@0 {
            compatible = "ethernet-phy-id0141.0e90", "ethernet-phy-ieee802.3-c45";
            interrupt-parent = <&PIC>;
            interrupts = <35 1>;
            reg = <0>;

            resets = <&rst 8>;
            reset-names = "phy";
            reset-gpios = <&gpio1 4 1>;
            reset-assert-us = <1000>;
            reset-deassert-us = <2000>;
        };
    };
            
Please refer to doc:
Documentation/devicetree/bindings/net/ethernet-phy.yaml
> 
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [EXT] [PATCH v1] net: fec: optionally reset PHY via a reset-controller
  2019-07-16  2:02 ` [EXT] " Andy Duan
@ 2019-07-16 13:19   ` Sven Van Asbroeck
  2019-07-17  1:32     ` Andy Duan
  0 siblings, 1 reply; 6+ messages in thread
From: Sven Van Asbroeck @ 2019-07-16 13:19 UTC (permalink / raw)
  To: Andy Duan; +Cc: David S . Miller, netdev, linux-kernel

Hi Andy,

On Mon, Jul 15, 2019 at 10:02 PM Andy Duan <fugang.duan@nxp.com> wrote:
>
> the phylib already can handle mii bus reset and phy device reset

That's a great suggestion, thank you !! I completely overlooked that code.
What will happen to the legacy phy reset code in fec? Are there many users left?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [EXT] [PATCH v1] net: fec: optionally reset PHY via a reset-controller
  2019-07-16 13:19   ` Sven Van Asbroeck
@ 2019-07-17  1:32     ` Andy Duan
  2019-07-17 12:48       ` Sven Van Asbroeck
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Duan @ 2019-07-17  1:32 UTC (permalink / raw)
  To: Sven Van Asbroeck; +Cc: David S . Miller, netdev, linux-kernel

From: Sven Van Asbroeck <thesven73@gmail.com> Sent: Tuesday, July 16, 2019 9:19 PM
> Hi Andy,
> 
> On Mon, Jul 15, 2019 at 10:02 PM Andy Duan <fugang.duan@nxp.com>
> wrote:
> >
> > the phylib already can handle mii bus reset and phy device reset
> 
> That's a great suggestion, thank you !! I completely overlooked that code.
> What will happen to the legacy phy reset code in fec? Are there many users
> left?

Yes, so the old legacy code is kept there. But it is better to clean up all if 
there have enough boards to verify them.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [EXT] [PATCH v1] net: fec: optionally reset PHY via a reset-controller
  2019-07-17  1:32     ` Andy Duan
@ 2019-07-17 12:48       ` Sven Van Asbroeck
  2019-07-18  1:24         ` Andy Duan
  0 siblings, 1 reply; 6+ messages in thread
From: Sven Van Asbroeck @ 2019-07-17 12:48 UTC (permalink / raw)
  To: Andy Duan; +Cc: David S . Miller, netdev, linux-kernel

On Tue, Jul 16, 2019 at 9:32 PM Andy Duan <fugang.duan@nxp.com> wrote:
>
> Yes, so the old legacy code is kept there. But it is better to clean up all if
> there have enough boards to verify them.

Would it make sense to print a warning message to the log whenever
someone tries to use the legacy phy reset on the fec?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [EXT] [PATCH v1] net: fec: optionally reset PHY via a reset-controller
  2019-07-17 12:48       ` Sven Van Asbroeck
@ 2019-07-18  1:24         ` Andy Duan
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Duan @ 2019-07-18  1:24 UTC (permalink / raw)
  To: Sven Van Asbroeck; +Cc: David S . Miller, netdev, linux-kernel

From: Sven Van Asbroeck <thesven73@gmail.com> Sent: Wednesday, July 17, 2019 8:48 PM
> On Tue, Jul 16, 2019 at 9:32 PM Andy Duan <fugang.duan@nxp.com> wrote:
> >
> > Yes, so the old legacy code is kept there. But it is better to clean
> > up all if there have enough boards to verify them.
> 
> Would it make sense to print a warning message to the log whenever
> someone tries to use the legacy phy reset on the fec?

It is feasible, just as reminder to drop such usage.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-07-18  1:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-15 21:05 [PATCH v1] net: fec: optionally reset PHY via a reset-controller Sven Van Asbroeck
2019-07-16  2:02 ` [EXT] " Andy Duan
2019-07-16 13:19   ` Sven Van Asbroeck
2019-07-17  1:32     ` Andy Duan
2019-07-17 12:48       ` Sven Van Asbroeck
2019-07-18  1:24         ` Andy Duan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).