Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1355817 > unrolled thread

[PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Caesar Wang <wxt@rock-chips.com> - 2016-03-11 12:00 +0100
    [PATCH 2/6] net: arc_emac: add phy-reset-* are optional for device tree Caesar Wang <wxt@rock-chips.com> - 2016-03-11 12:00 +0100
    [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Caesar Wang <wxt@rock-chips.com> - 2016-03-11 12:10 +0100
      Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-03-11 14:50 +0100
        Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Heiko Stübner <heiko@sntech.de> - 2016-03-11 16:00 +0100
          Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Heiko Stübner <heiko@sntech.de> - 2016-03-11 16:00 +0100
      Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-03-11 19:40 +0100
        Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Caesar Wang <caesar.upstream@gmail.com> - 2016-03-13 05:00 +0100
      Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-03-11 20:30 +0100
    Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac  drivers Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-03-11 14:50 +0100
      Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac  drivers Caesar Wang <caesar.upstream@gmail.com> - 2016-03-11 15:50 +0100
        Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac  drivers Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-03-11 19:50 +0100
          Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac  drivers Caesar Wang <caesar.upstream@gmail.com> - 2016-03-13 05:10 +0100

#1355817 — [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers

This series patches are based on kernel 4.5-rc7+ version.
Linux version 4.5.0-rc7-next-20160310+ (wxt@nb) (...) #23 SMP Fri Mar 11 15:55:53

Verified on kylin board with my github.
https://github.com/Caesar-github/rockchip/tree/kylin/next

That's verified on kylin board with ubuntu os.

How to test and verify?

You can refer to the following wiki document.
http://rockchip.wikidot.com/linux-develop-guide

bootup log:
   1.268113] rockchip_emac 10200000.ethernet: no regulator found
   [    1.286682] rockchip_emac 10200000.ethernet: ARC EMAC detected with id: 0x7fd02
   [    1.294007] rockchip_emac 10200000.ethernet: IRQ is 29
   [    1.299453] rockchip_emac 10200000.ethernet: MAC address is now 1e:cd:18:78:90:25
   [    1.726564] rockchip_emac 10200000.ethernet: connected to Generic PHY phy with id 0x1cc816
   [    8.936862] rockchip_emac 10200000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off

root@localhost:/# busybox ping www.baidu.com
PING www.baidu.com (14.215.177.38): 56 data bytes
64 bytes from 14.215.177.38: seq=0 ttl=48 time=35.046 ms
64 bytes from 14.215.177.38: seq=1 ttl=48 time=35.095 ms
64 bytes from 14.215.177.38: seq=2 ttl=48 time=34.203 ms
64 bytes from 14.215.177.38: seq=3 ttl=48 time=38.516 ms
...
---

1) This series has 6 patches: (1--->6)
net: arc_emac: make the rockchip emac document more compatible
net: arc_emac: add phy-reset-* are optional for device tree
net: arc_emac: support the phy reset for emac driver
net: arc: trivial: cleanup the emac driver
clk: rockchip: rk3036: fix and add node id for emac clock
ARM: dts: rockchip: add support emac for RK3036

2) This series patches have the following decriptions:

Hi Rob, David:
PATCH[1/6-2/6]: ====>
net: arc_emac: make the rockchip emac document more compatible
net: arc_emac: add phy-reset-* are optional for device tree

The patches change the rockchip emac document for more compatible and
Add the phy-reset-* property for document.

This patch adds the following property for arc_emac.

phy-reset-* include the following:
1) phy-reset-gpios:
The phy-reset-gpios is an optional property for arc emac device tree boot.
Change the binding document to match the driver code.

2) phy-reset-duration:
Different boards may require different phy reset duration. Add property
phy-reset-duration for device tree probe, so that the boards that need
a longer reset duration can specify it in their device tree.

3) phy-reset-active-high:
We need that for a custom hardware that needs the reverse reset sequence.
---

Hi David
PATCH[3/6]: ====>
net: arc_emac: support the phy reset for emac driver

The emac didn't work on kylin board since in some case the clocks parent changed.
The kylin hardware connects the phy reset pin, we should use it with real world.

As the previous patch discuss on https://patchwork.kernel.org/patch/8186801/

Hi David
PATCH[4/6]: ====>
net: arc: trivial: cleanup the emac driver

The first time to look the emac drivers, I think that have to cleanup the drivers with scripts.
Although it's the trivial things, in order to be more read.
---

Hi Heiko,Michael,Stephen:
PATCH[5/6]: ====>
clk: rockchip: rk3036: fix and add node id for emac clock

Add the emac needed clocks for rk3036 SOCs
---

Hi Heiko:
PATCH[6/6]: ====>
ARM: dts: rockchip: add support emac for RK3036

Add the emac node info for rk3036 dts/dtsi.
---

Thanks your reviewing! :)



Caesar Wang (4):
  net: arc_emac: make the rockchip emac document more compatible
  net: arc_emac: add phy-reset-* are optional for device tree
  net: arc_emac: support the phy reset for emac driver
  net: arc: trivial: cleanup the emac driver

zhengxing (2):
  clk: rockchip: rk3036: fix and add node id for emac clock
  ARM: dts: rockchip: add support emac for RK3036

 Documentation/devicetree/bindings/net/arc_emac.txt | 10 +++
 .../devicetree/bindings/net/emac_rockchip.txt      |  8 ++-
 arch/arm/boot/dts/rk3036-evb.dts                   | 23 +++++++
 arch/arm/boot/dts/rk3036-kylin.dts                 | 20 ++++++
 arch/arm/boot/dts/rk3036.dtsi                      | 39 +++++++++++
 drivers/clk/rockchip/clk-rk3036.c                  |  9 ++-
 drivers/net/ethernet/arc/emac.h                    | 54 +++++++--------
 drivers/net/ethernet/arc/emac_main.c               | 76 +++++++++++++++++-----
 drivers/net/ethernet/arc/emac_mdio.c               |  2 +-
 drivers/net/ethernet/arc/emac_rockchip.c           | 41 ++++++++----
 include/dt-bindings/clock/rk3036-cru.h             |  2 +
 11 files changed, 221 insertions(+), 63 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1355822 — [PATCH 2/6] net: arc_emac: add phy-reset-* are optional for device tree

This patch adds the following property for arc_emac.

1) phy-reset-gpios:
The phy-reset-gpios is an optional property for arc emac device tree boot.
Change the binding document to match the driver code.

2) phy-reset-duration:
Different boards may require different phy reset duration. Add property
phy-reset-duration for device tree probe, so that the boards that need
a longer reset duration can specify it in their device tree.

3) phy-reset-active-high:
We need that for a custom hardware that needs the reverse reset
sequence.

Anyway, we can add the above property for arc emac.

Signed-off-by: Caesar Wang <wxt@rock-chips.com>
---

 Documentation/devicetree/bindings/net/arc_emac.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/arc_emac.txt b/Documentation/devicetree/bindings/net/arc_emac.txt
index a1d71eb..6389b00 100644
--- a/Documentation/devicetree/bindings/net/arc_emac.txt
+++ b/Documentation/devicetree/bindings/net/arc_emac.txt
@@ -7,6 +7,16 @@ Required properties:
 - max-speed: see ethernet.txt file in the same directory.
 - phy: see ethernet.txt file in the same directory.
 
+Optional properties:
+- phy-reset-gpios : Should specify the gpio for phy reset
+- phy-reset-duration : Reset duration in milliseconds.  Should present
+  only if property "phy-reset-gpios" is available.  Missing the property
+  will have the duration be 1 millisecond.  Numbers greater than 1000 are
+  invalid and 1 millisecond will be used instead.
+- phy-reset-active-high : If present then the reset sequence using the GPIO
+  specified in the "phy-reset-gpios" property is reversed (H=reset state,
+  L=operation state).
+
 Clock handling:
 The clock frequency is needed to calculate and set polling period of EMAC.
 It must be provided by one of:
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1355825 — [PATCH 3/6] net: arc_emac: support the phy reset for emac driver

This patch adds to support the emac phy reset.

1) phy-reset-gpios:
The phy-reset-gpios is an optional property for arc emac device tree boot.
Change the binding document to match the driver code.

2) phy-reset-duration:
Different boards may require different phy reset duration. Add property
phy-reset-duration for device tree probe, so that the boards that need
a longer reset duration can specify it in their device tree.

3) phy-reset-active-high:
We need that for a custom hardware that needs the reverse reset sequence.

Of course, this patch will fix the issue on
https://patchwork.kernel.org/patch/8186801/.

In some cases, the emac couldn't work if you don't have reset the phy.
Let's add it to happy work.

Signed-off-by: Caesar Wang <wxt@rock-chips.com>
---

 drivers/net/ethernet/arc/emac_main.c | 41 ++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
index 6446af1..42384f6a 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -764,6 +764,45 @@ static const struct net_device_ops arc_emac_netdev_ops = {
 #endif
 };
 
+#ifdef CONFIG_OF
+static void emac_reset_phy(struct net_device *pdev)
+{
+	int err, phy_reset;
+	bool active_high = false;
+	int msec = 10;
+	struct device *dev = pdev->dev.parent;
+	struct device_node *np = dev->of_node;
+
+	if (!np)
+		return;
+
+	of_property_read_u32(np, "phy-reset-duration", &msec);
+	/* A sane reset duration should not be longer than 1s */
+	if (msec > 1000)
+		msec = 1;
+
+	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
+	if (!gpio_is_valid(phy_reset))
+		return;
+
+	active_high = of_property_read_bool(np, "phy-reset-active-high");
+
+	err = devm_gpio_request_one(dev, phy_reset, active_high ?
+				    GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
+				    "phy-reset");
+	if (err) {
+		dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);
+		return;
+	}
+	msleep(msec);
+	gpio_set_value_cansleep(phy_reset, !active_high);
+}
+#else
+static void emac_reset_phy(struct platform_device *pdev)
+{
+}
+#endif /* CONFIG_OF */
+
 int arc_emac_probe(struct net_device *ndev, int interface)
 {
 	struct device *dev = ndev->dev.parent;
@@ -803,6 +842,8 @@ int arc_emac_probe(struct net_device *ndev, int interface)
 	/* FIXME :: no multicast support yet */
 	ndev->flags &= ~IFF_MULTICAST;
 
+	emac_reset_phy(ndev);
+
 	priv = netdev_priv(ndev);
 	priv->dev = dev;
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1355909 — Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver

Hello.

On 3/11/2016 1:55 PM, Caesar Wang wrote:

> This patch adds to support the emac phy reset.
>
> 1) phy-reset-gpios:
> The phy-reset-gpios is an optional property for arc emac device tree boot.
> Change the binding document to match the driver code.
>
> 2) phy-reset-duration:
> Different boards may require different phy reset duration. Add property
> phy-reset-duration for device tree probe, so that the boards that need
> a longer reset duration can specify it in their device tree.
>
> 3) phy-reset-active-high:
> We need that for a custom hardware that needs the reverse reset sequence.
>
> Of course, this patch will fix the issue on
> https://patchwork.kernel.org/patch/8186801/.
>
> In some cases, the emac couldn't work if you don't have reset the phy.
> Let's add it to happy work.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> ---
>
>   drivers/net/ethernet/arc/emac_main.c | 41 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
>
> diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
> index 6446af1..42384f6a 100644
> --- a/drivers/net/ethernet/arc/emac_main.c
> +++ b/drivers/net/ethernet/arc/emac_main.c
> @@ -764,6 +764,45 @@ static const struct net_device_ops arc_emac_netdev_ops = {
>   #endif
>   };
>
> +#ifdef CONFIG_OF
> +static void emac_reset_phy(struct net_device *pdev)
> +{
> +	int err, phy_reset;
> +	bool active_high = false;
> +	int msec = 10;
> +	struct device *dev = pdev->dev.parent;
> +	struct device_node *np = dev->of_node;
> +
> +	if (!np)
> +		return;
> +
> +	of_property_read_u32(np, "phy-reset-duration", &msec);
> +	/* A sane reset duration should not be longer than 1s */
> +	if (msec > 1000)
> +		msec = 1;
> +
> +	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> +	if (!gpio_is_valid(phy_reset))
> +		return;
> +
> +	active_high = of_property_read_bool(np, "phy-reset-active-high");
> +
> +	err = devm_gpio_request_one(dev, phy_reset, active_high ?
> +				    GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
> +				    "phy-reset");
> +	if (err) {
> +		dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);
> +		return;
> +	}
> +	msleep(msec);
> +	gpio_set_value_cansleep(phy_reset, !active_high);
> +}
[...]

    Why not make it the mii_bus::reset() method? It gets called before the 
MDIO bus scan.

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1355960 — Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver

Hi Caesar,

Am Freitag, 11. März 2016, 16:47:59 schrieb Sergei Shtylyov:
> On 3/11/2016 1:55 PM, Caesar Wang wrote:
> > This patch adds to support the emac phy reset.
> > 
> > 1) phy-reset-gpios:
> > The phy-reset-gpios is an optional property for arc emac device tree boot.
> > Change the binding document to match the driver code.
> > 
> > 2) phy-reset-duration:
> > Different boards may require different phy reset duration. Add property
> > phy-reset-duration for device tree probe, so that the boards that need
> > a longer reset duration can specify it in their device tree.
> > 
> > 3) phy-reset-active-high:
> > We need that for a custom hardware that needs the reverse reset sequence.
> > 
> > Of course, this patch will fix the issue on
> > https://patchwork.kernel.org/patch/8186801/.
> > 
> > In some cases, the emac couldn't work if you don't have reset the phy.
> > Let's add it to happy work.
> > 
> > Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> > ---
> > 
> >   drivers/net/ethernet/arc/emac_main.c | 41
> >   ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/arc/emac_main.c
> > b/drivers/net/ethernet/arc/emac_main.c index 6446af1..42384f6a 100644
> > --- a/drivers/net/ethernet/arc/emac_main.c
> > +++ b/drivers/net/ethernet/arc/emac_main.c
> > @@ -764,6 +764,45 @@ static const struct net_device_ops
> > arc_emac_netdev_ops = {> 
> >   #endif
> >   };
> > 
> > +#ifdef CONFIG_OF
> > +static void emac_reset_phy(struct net_device *pdev)
> > +{
> > +	int err, phy_reset;
> > +	bool active_high = false;
> > +	int msec = 10;
> > +	struct device *dev = pdev->dev.parent;
> > +	struct device_node *np = dev->of_node;
> > +
> > +	if (!np)
> > +		return;
> > +
> > +	of_property_read_u32(np, "phy-reset-duration", &msec);
> > +	/* A sane reset duration should not be longer than 1s */
> > +	if (msec > 1000)
> > +		msec = 1;
> > +
> > +	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> > +	if (!gpio_is_valid(phy_reset))
> > +		return;
> > +
> > +	active_high = of_property_read_bool(np, "phy-reset-active-high");
> > +
> > +	err = devm_gpio_request_one(dev, phy_reset, active_high ?
> > +				    GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
> > +				    "phy-reset");
> > +	if (err) {
> > +		dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);
> > +		return;
> > +	}
> > +	msleep(msec);
> > +	gpio_set_value_cansleep(phy_reset, !active_high);
> > +}
> 
> [...]
> 
>     Why not make it the mii_bus::reset() method? It gets called before the
> MDIO bus scan.

while meddling with the emac, I've built some sort of preliminary variant of
Sergei's suggestion at [0] - maybe you could take a look for some sort of
inspiration ;-) 

The code is lifted from the designware gmac driver, so the binding also is
similar.


Heiko


[0] https://github.com/mmind/linux-rockchip/commit/a943c588783438ff1c508dfa8c79f1709aa5775e

[toc] | [prev] | [next] | [standalone]

#1355970 — Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver

Am Freitag, 11. März 2016, 15:56:04 schrieb Heiko Stübner:
> Hi Caesar,
> 
> Am Freitag, 11. März 2016, 16:47:59 schrieb Sergei Shtylyov:
> > On 3/11/2016 1:55 PM, Caesar Wang wrote:
> > > This patch adds to support the emac phy reset.
> > > 
> > > 1) phy-reset-gpios:
> > > The phy-reset-gpios is an optional property for arc emac device tree
> > > boot.
> > > Change the binding document to match the driver code.
> > > 
> > > 2) phy-reset-duration:
> > > Different boards may require different phy reset duration. Add property
> > > phy-reset-duration for device tree probe, so that the boards that need
> > > a longer reset duration can specify it in their device tree.
> > > 
> > > 3) phy-reset-active-high:
> > > We need that for a custom hardware that needs the reverse reset
> > > sequence.
> > > 
> > > Of course, this patch will fix the issue on
> > > https://patchwork.kernel.org/patch/8186801/.
> > > 
> > > In some cases, the emac couldn't work if you don't have reset the phy.
> > > Let's add it to happy work.
> > > 
> > > Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> > > ---
> > > 
> > >   drivers/net/ethernet/arc/emac_main.c | 41
> > >   ++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/arc/emac_main.c
> > > b/drivers/net/ethernet/arc/emac_main.c index 6446af1..42384f6a 100644
> > > --- a/drivers/net/ethernet/arc/emac_main.c
> > > +++ b/drivers/net/ethernet/arc/emac_main.c
> > > @@ -764,6 +764,45 @@ static const struct net_device_ops
> > > arc_emac_netdev_ops = {>
> > > 
> > >   #endif
> > >   };
> > > 
> > > +#ifdef CONFIG_OF
> > > +static void emac_reset_phy(struct net_device *pdev)
> > > +{
> > > +	int err, phy_reset;
> > > +	bool active_high = false;
> > > +	int msec = 10;
> > > +	struct device *dev = pdev->dev.parent;
> > > +	struct device_node *np = dev->of_node;
> > > +
> > > +	if (!np)
> > > +		return;
> > > +
> > > +	of_property_read_u32(np, "phy-reset-duration", &msec);
> > > +	/* A sane reset duration should not be longer than 1s */
> > > +	if (msec > 1000)
> > > +		msec = 1;
> > > +
> > > +	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> > > +	if (!gpio_is_valid(phy_reset))
> > > +		return;
> > > +
> > > +	active_high = of_property_read_bool(np, "phy-reset-active-high");
> > > +
> > > +	err = devm_gpio_request_one(dev, phy_reset, active_high ?
> > > +				    GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
> > > +				    "phy-reset");
> > > +	if (err) {
> > > +		dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);
> > > +		return;
> > > +	}
> > > +	msleep(msec);
> > > +	gpio_set_value_cansleep(phy_reset, !active_high);
> > > +}
> > 
> > [...]
> > 
> >     Why not make it the mii_bus::reset() method? It gets called before the
> > 
> > MDIO bus scan.
> 
> while meddling with the emac, I've built some sort of preliminary variant of
> Sergei's suggestion at [0] - maybe you could take a look for some sort of
> inspiration ;-)

forgot to add ... of course the newer gpiod_* infrastructure should be used 
instead of the old integer-based gpio numbers

> 
> The code is lifted from the designware gmac driver, so the binding also is
> similar.
> 
> 
> Heiko
> 
> 
> [0]
> https://github.com/mmind/linux-rockchip/commit/a943c588783438ff1c508dfa8c79
> f1709aa5775e

[toc] | [prev] | [next] | [standalone]

#1356123 — Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver

On 03/11/2016 01:55 PM, Caesar Wang wrote:

> This patch adds to support the emac phy reset.
>
> 1) phy-reset-gpios:
> The phy-reset-gpios is an optional property for arc emac device tree boot.
> Change the binding document to match the driver code.

    The binding document is apparently changed in another patch. Not sure what 
you wanted to say here...

> 2) phy-reset-duration:
> Different boards may require different phy reset duration. Add property
> phy-reset-duration for device tree probe, so that the boards that need
> a longer reset duration can specify it in their device tree.
>
> 3) phy-reset-active-high:
> We need that for a custom hardware that needs the reverse reset sequence.

    No, we don't really need that, "phy-reset-gpio" prop can contain this data.

> Of course, this patch will fix the issue on
> https://patchwork.kernel.org/patch/8186801/.
>
> In some cases, the emac couldn't work if you don't have reset the phy.
> Let's add it to happy work.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> ---
>
>   drivers/net/ethernet/arc/emac_main.c | 41 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
>
> diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
> index 6446af1..42384f6a 100644
> --- a/drivers/net/ethernet/arc/emac_main.c
> +++ b/drivers/net/ethernet/arc/emac_main.c
> @@ -764,6 +764,45 @@ static const struct net_device_ops arc_emac_netdev_ops = {
>   #endif
>   };
>
> +#ifdef CONFIG_OF
> +static void emac_reset_phy(struct net_device *pdev)
> +{
> +	int err, phy_reset;
> +	bool active_high = false;
> +	int msec = 10;
> +	struct device *dev = pdev->dev.parent;
> +	struct device_node *np = dev->of_node;
> +
> +	if (!np)
> +		return;
> +
> +	of_property_read_u32(np, "phy-reset-duration", &msec);
> +	/* A sane reset duration should not be longer than 1s */
> +	if (msec > 1000)
> +		msec = 1;
> +
> +	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);

    Forgot to say that these old integer-base GPIO APIs shouldn't be used by 
the new code, there's new 'struct gpio_desc' based APIs, like devm_gpiod_get() 
etc.

> +	if (!gpio_is_valid(phy_reset))
> +		return;
> +
> +	active_high = of_property_read_bool(np, "phy-reset-active-high");

    Well, I still don't understand why this prop is needed, while the GPIO 
sense is transparently handled by the gpiolib (at least when using DT).

[...]

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1356608 — Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver

Hi Sergei,

在 2016年03月12日 02:35, Sergei Shtylyov 写道:
> On 03/11/2016 01:55 PM, Caesar Wang wrote:
>
>> This patch adds to support the emac phy reset.
>>
>> 1) phy-reset-gpios:
>> The phy-reset-gpios is an optional property for arc emac device tree 
>> boot.
>> Change the binding document to match the driver code.
>
>    The binding document is apparently changed in another patch. Not 
> sure what you wanted to say here...
>
>> 2) phy-reset-duration:
>> Different boards may require different phy reset duration. Add property
>> phy-reset-duration for device tree probe, so that the boards that need
>> a longer reset duration can specify it in their device tree.
>>
>> 3) phy-reset-active-high:
>> We need that for a custom hardware that needs the reverse reset 
>> sequence.
>
>    No, we don't really need that, "phy-reset-gpio" prop can contain 
> this data.
>
>> Of course, this patch will fix the issue on
>> https://patchwork.kernel.org/patch/8186801/.
>>
>> In some cases, the emac couldn't work if you don't have reset the phy.
>> Let's add it to happy work.
>>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
>> ---
>>
>>   drivers/net/ethernet/arc/emac_main.c | 41 
>> ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 41 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/arc/emac_main.c 
>> b/drivers/net/ethernet/arc/emac_main.c
>> index 6446af1..42384f6a 100644
>> --- a/drivers/net/ethernet/arc/emac_main.c
>> +++ b/drivers/net/ethernet/arc/emac_main.c
>> @@ -764,6 +764,45 @@ static const struct net_device_ops 
>> arc_emac_netdev_ops = {
>>   #endif
>>   };
>>
>> +#ifdef CONFIG_OF
>> +static void emac_reset_phy(struct net_device *pdev)
>> +{
>> +    int err, phy_reset;
>> +    bool active_high = false;
>> +    int msec = 10;
>> +    struct device *dev = pdev->dev.parent;
>> +    struct device_node *np = dev->of_node;
>> +
>> +    if (!np)
>> +        return;
>> +
>> +    of_property_read_u32(np, "phy-reset-duration", &msec);
>> +    /* A sane reset duration should not be longer than 1s */
>> +    if (msec > 1000)
>> +        msec = 1;
>> +
>> +    phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
>
>    Forgot to say that these old integer-base GPIO APIs shouldn't be 
> used by the new code, there's new 'struct gpio_desc' based APIs, like 
> devm_gpiod_get() etc.
>
>> +    if (!gpio_is_valid(phy_reset))
>> +        return;
>> +
>> +    active_high = of_property_read_bool(np, "phy-reset-active-high");
>
>    Well, I still don't understand why this prop is needed, while the 
> GPIO sense is transparently handled by the gpiolib (at least when 
> using DT).

Okay, I have to admit that reference the exist way to do...


wxt@nb:~/kernel/drivers/net/ethernet$ ag reset-gpios
micrel/ks8851.c
1427:    gpio = of_get_named_gpio_flags(spi->dev.of_node, "reset-gpios",

arc/emac_main.c
787:    phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
797:        dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);

arc/emac_main.c~
784:    phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
794:        dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);

davicom/dm9000.c
1451:    reset_gpios = of_get_named_gpio_flags(dev->of_node, 
"reset-gpios", 0,

freescale/fec_main.c
3206:    phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
3216:        dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", 
err);

cadence/macb.c
2958:        int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0);

...

Yep, the gpiolib that can include the prop to work with the same ways.



>
> [...]
>
> MBR, Sergei
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


-- 
Thanks,
Caesar

[toc] | [prev] | [next] | [standalone]

#1356148 — Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver

On 03/11/2016 01:55 PM, Caesar Wang wrote:

> This patch adds to support the emac phy reset.
>
> 1) phy-reset-gpios:
> The phy-reset-gpios is an optional property for arc emac device tree boot.
> Change the binding document to match the driver code.
>
> 2) phy-reset-duration:
> Different boards may require different phy reset duration. Add property
> phy-reset-duration for device tree probe, so that the boards that need
> a longer reset duration can specify it in their device tree.
>
> 3) phy-reset-active-high:
> We need that for a custom hardware that needs the reverse reset sequence.

    What concerns me the most about the existing (and suggested) PHY reset 
related props is that they are located in the MAC device node while not having 
*anything* to do with the MAC at all! These props actually belong to the PHY 
nodes, and I'm currently looking into how to handle them there, where they 
belong...

> Of course, this patch will fix the issue on
> https://patchwork.kernel.org/patch/8186801/.
>
> In some cases, the emac couldn't work if you don't have reset the phy.
> Let's add it to happy work.
>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>

[...]

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1355908 — Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers

Hello.

On 3/11/2016 1:55 PM, Caesar Wang wrote:

> This series patches are based on kernel 4.5-rc7+ version.
> Linux version 4.5.0-rc7-next-20160310+ (wxt@nb) (...) #23 SMP Fri Mar 11 15:55:53

[...]

> 1) This series has 6 patches: (1--->6)
> net: arc_emac: make the rockchip emac document more compatible
> net: arc_emac: add phy-reset-* are optional for device tree

    I'm not seeing these patches on netdev...

> net: arc_emac: support the phy reset for emac driver
> net: arc: trivial: cleanup the emac driver
> clk: rockchip: rk3036: fix and add node id for emac clock
> ARM: dts: rockchip: add support emac for RK3036
>
> 2) This series patches have the following decriptions:

    Descriptions.

> Hi Rob, David:
> PATCH[1/6-2/6]: ====>
> net: arc_emac: make the rockchip emac document more compatible
> net: arc_emac: add phy-reset-* are optional for device tree
>
> The patches change the rockchip emac document for more compatible and
> Add the phy-reset-* property for document.
>
> This patch adds the following property for arc_emac.
>
> phy-reset-* include the following:
> 1) phy-reset-gpios:
> The phy-reset-gpios is an optional property for arc emac device tree boot.
> Change the binding document to match the driver code.
>
> 2) phy-reset-duration:
> Different boards may require different phy reset duration. Add property
> phy-reset-duration for device tree probe, so that the boards that need
> a longer reset duration can specify it in their device tree.
>
> 3) phy-reset-active-high:
> We need that for a custom hardware that needs the reverse reset sequence.

    Why not infer this from the "phy-reset-gpios" prop?

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1355954 — Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers

Hi Sergei,

在 2016年03月11日 21:46, Sergei Shtylyov 写道:
> Hello.
>
> On 3/11/2016 1:55 PM, Caesar Wang wrote:
>
>> This series patches are based on kernel 4.5-rc7+ version.
>> Linux version 4.5.0-rc7-next-20160310+ (wxt@nb) (...) #23 SMP Fri Mar 
>> 11 15:55:53
>
> [...]
>
>> 1) This series has 6 patches: (1--->6)
>> net: arc_emac: make the rockchip emac document more compatible
>> net: arc_emac: add phy-reset-* are optional for device tree
>
>    I'm not seeing these patches on netdev...

Sent by the patman tool.

LKML:
https://patchwork.kernel.org/patch/8564501/
https://patchwork.kernel.org/patch/8564511/

>
>> net: arc_emac: support the phy reset for emac driver
>> net: arc: trivial: cleanup the emac driver
>> clk: rockchip: rk3036: fix and add node id for emac clock
>> ARM: dts: rockchip: add support emac for RK3036
>>
>> 2) This series patches have the following decriptions:
>
>    Descriptions.
>
>> Hi Rob, David:
>> PATCH[1/6-2/6]: ====>
>> net: arc_emac: make the rockchip emac document more compatible
>> net: arc_emac: add phy-reset-* are optional for device tree
>>
>> The patches change the rockchip emac document for more compatible and
>> Add the phy-reset-* property for document.
>>
>> This patch adds the following property for arc_emac.
>>
>> phy-reset-* include the following:
>> 1) phy-reset-gpios:
>> The phy-reset-gpios is an optional property for arc emac device tree 
>> boot.
>> Change the binding document to match the driver code.
>>
>> 2) phy-reset-duration:
>> Different boards may require different phy reset duration. Add property
>> phy-reset-duration for device tree probe, so that the boards that need
>> a longer reset duration can specify it in their device tree.
>>
>> 3) phy-reset-active-high:
>> We need that for a custom hardware that needs the reverse reset 
>> sequence.
>
>    Why not infer this from the "phy-reset-gpios" prop?

See:
https://patchwork.kernel.org/patch/8564511/

phy-reset-active-high : If present then the reset sequence using the GPIO
  specified in the "phy-reset-gpios" property is reversed (H=reset state,
  L=operation state).


Thanks,

Caesar

>
> MBR, Sergei
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

[toc] | [prev] | [next] | [standalone]

#1356129 — Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers

Hello.

On 03/11/2016 05:48 PM, Caesar Wang wrote:

[...]

>>> Hi Rob, David:
>>> PATCH[1/6-2/6]: ====>
>>> net: arc_emac: make the rockchip emac document more compatible
>>> net: arc_emac: add phy-reset-* are optional for device tree
>>>
>>> The patches change the rockchip emac document for more compatible and
>>> Add the phy-reset-* property for document.
>>>
>>> This patch adds the following property for arc_emac.
>>>
>>> phy-reset-* include the following:
>>> 1) phy-reset-gpios:
>>> The phy-reset-gpios is an optional property for arc emac device tree boot.
>>> Change the binding document to match the driver code.
>>>
>>> 2) phy-reset-duration:
>>> Different boards may require different phy reset duration. Add property
>>> phy-reset-duration for device tree probe, so that the boards that need
>>> a longer reset duration can specify it in their device tree.
>>>
>>> 3) phy-reset-active-high:
>>> We need that for a custom hardware that needs the reverse reset sequence.
>>
>>    Why not infer this from the "phy-reset-gpios" prop?
>
> See:
> https://patchwork.kernel.org/patch/8564511/
 >
> phy-reset-active-high : If present then the reset sequence using the GPIO
>   specified in the "phy-reset-gpios" property is reversed (H=reset state,
>   L=operation state).

    Referring to your own suggested bindings isn't an answer. If the driver 
that you're copying from here (fec) had a reason to handle the GPIO sense with 
the help of an extra prop (legacy code), it doesn't mean your new driver needs 
to mimic this as well, AFAIU...

> Thanks,
>
> Caesar

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1356609 — Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers

在 2016年03月12日 02:46, Sergei Shtylyov 写道:
> Hello.
>
> On 03/11/2016 05:48 PM, Caesar Wang wrote:
>
> [...]
>
>>>> Hi Rob, David:
>>>> PATCH[1/6-2/6]: ====>
>>>> net: arc_emac: make the rockchip emac document more compatible
>>>> net: arc_emac: add phy-reset-* are optional for device tree
>>>>
>>>> The patches change the rockchip emac document for more compatible and
>>>> Add the phy-reset-* property for document.
>>>>
>>>> This patch adds the following property for arc_emac.
>>>>
>>>> phy-reset-* include the following:
>>>> 1) phy-reset-gpios:
>>>> The phy-reset-gpios is an optional property for arc emac device 
>>>> tree boot.
>>>> Change the binding document to match the driver code.
>>>>
>>>> 2) phy-reset-duration:
>>>> Different boards may require different phy reset duration. Add 
>>>> property
>>>> phy-reset-duration for device tree probe, so that the boards that need
>>>> a longer reset duration can specify it in their device tree.
>>>>
>>>> 3) phy-reset-active-high:
>>>> We need that for a custom hardware that needs the reverse reset 
>>>> sequence.
>>>
>>>    Why not infer this from the "phy-reset-gpios" prop?
>>
>> See:
>> https://patchwork.kernel.org/patch/8564511/
> >
>> phy-reset-active-high : If present then the reset sequence using the 
>> GPIO
>>   specified in the "phy-reset-gpios" property is reversed (H=reset 
>> state,
>>   L=operation state).
>
>    Referring to your own suggested bindings isn't an answer. If the 
> driver that you're copying from here (fec) had a reason to handle the 
> GPIO sense with the help of an extra prop (legacy code), it doesn't 
> mean your new driver needs to mimic this as well, AFAIU...

I know your suggestion is a fair request.

Oh, that copy from the 'freescale/fec_main.c' ....

So, The exist way was old and unwise in mainline. :(

wxt@nb:~/kernel/drivers/net/ethernet$ ag reset-gpios
micrel/ks8851.c
1427:    gpio = of_get_named_gpio_flags(spi->dev.of_node, "reset-gpios",

arc/emac_main.c
787:    phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
797:        dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);

arc/emac_main.c~
784:    phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
794:        dev_err(dev, "failed to get phy-reset-gpios: %d\n", err);

davicom/dm9000.c
1451:    reset_gpios = of_get_named_gpio_flags(dev->of_node, 
"reset-gpios", 0,

freescale/fec_main.c
3206:    phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
3216:        dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", 
err);

cadence/macb.c
2958:        int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0);

...


Anyway, I will update it with your suggestion.

Thanks,


Caesar





>
>> Thanks,
>>
>> Caesar
>
> MBR, Sergei
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


-- 
Thanks,
Caesar

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web