linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers
@ 2016-03-11 10:55 Caesar Wang
  2016-03-11 10:55 ` [PATCH 1/6] net: arc_emac: make the rockchip emac document more compatible Caesar Wang
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Caesar Wang @ 2016-03-11 10:55 UTC (permalink / raw)
  To: Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, Caesar Wang, devicetree,
	Michael Turquette, Alexander Kochetkov, Russell King,
	Stephen Boyd, netdev, Kumar Gala, linux-kernel, Ian Campbell,
	zhengxing, Jiri Kosina, Pawel Moll, Mark Rutland, linux-clk,
	linux-arm-kernel


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

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

* [PATCH 1/6] net: arc_emac: make the rockchip emac document more compatible
  2016-03-11 10:55 [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Caesar Wang
@ 2016-03-11 10:55 ` Caesar Wang
  2016-03-11 10:55 ` [PATCH 2/6] net: arc_emac: add phy-reset-* are optional for device tree Caesar Wang
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Caesar Wang @ 2016-03-11 10:55 UTC (permalink / raw)
  To: Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, Caesar Wang, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, devicetree,
	linux-arm-kernel, linux-kernel

This patch adds the all kings of SoCs to compatible.

The rk3036 emac has been landed in mainline, so we should add rk3036 emac
for document.

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

 Documentation/devicetree/bindings/net/emac_rockchip.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/emac_rockchip.txt b/Documentation/devicetree/bindings/net/emac_rockchip.txt
index 8dc1c79..f705bb8 100644
--- a/Documentation/devicetree/bindings/net/emac_rockchip.txt
+++ b/Documentation/devicetree/bindings/net/emac_rockchip.txt
@@ -1,8 +1,10 @@
-* ARC EMAC 10/100 Ethernet platform driver for Rockchip Rk3066/RK3188 SoCs
+* ARC EMAC 10/100 Ethernet platform driver for Rockchip RK3036/Rk3066/RK3188 SoCs
 
 Required properties:
-- compatible: Should be "rockchip,rk3066-emac" or "rockchip,rk3188-emac"
-  according to the target SoC.
+- compatible: should be "rockchip,<name>-emac"
+   "rockchip,rk3036-emac": found on RK3036 SoCs
+   "rockchip,rk3066-emac": found on RK3066 SoCs
+   "rockchip,rockchip,rk3188-emac": found on RK3188 SoCs
 - reg: Address and length of the register set for the device
 - interrupts: Should contain the EMAC interrupts
 - rockchip,grf: phandle to the syscon grf used to control speed and mode
-- 
1.9.1

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

* [PATCH 2/6] net: arc_emac: add phy-reset-* are optional for device tree
  2016-03-11 10:55 [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Caesar Wang
  2016-03-11 10:55 ` [PATCH 1/6] net: arc_emac: make the rockchip emac document more compatible Caesar Wang
@ 2016-03-11 10:55 ` Caesar Wang
  2016-03-18 19:21   ` Rob Herring
  2016-03-11 10:55 ` [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Caesar Wang
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Caesar Wang @ 2016-03-11 10:55 UTC (permalink / raw)
  To: Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, Caesar Wang, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, devicetree, linux-kernel

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

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

* [PATCH 3/6] net: arc_emac: support the phy reset for emac driver
  2016-03-11 10:55 [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Caesar Wang
  2016-03-11 10:55 ` [PATCH 1/6] net: arc_emac: make the rockchip emac document more compatible Caesar Wang
  2016-03-11 10:55 ` [PATCH 2/6] net: arc_emac: add phy-reset-* are optional for device tree Caesar Wang
@ 2016-03-11 10:55 ` Caesar Wang
  2016-03-11 13:47   ` Sergei Shtylyov
                     ` (2 more replies)
  2016-03-11 10:55 ` [PATCH 4/6] net: arc: trivial: cleanup the " Caesar Wang
                   ` (3 subsequent siblings)
  6 siblings, 3 replies; 22+ messages in thread
From: Caesar Wang @ 2016-03-11 10:55 UTC (permalink / raw)
  To: Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, Caesar Wang,
	Alexander Kochetkov, netdev, linux-kernel

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

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

* [PATCH 4/6] net: arc: trivial: cleanup the emac driver
  2016-03-11 10:55 [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Caesar Wang
                   ` (2 preceding siblings ...)
  2016-03-11 10:55 ` [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Caesar Wang
@ 2016-03-11 10:55 ` Caesar Wang
  2016-03-11 11:28   ` kbuild test robot
  2016-03-11 10:55 ` [PATCH 5/6] clk: rockchip: rk3036: fix and add node id for emac clock Caesar Wang
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Caesar Wang @ 2016-03-11 10:55 UTC (permalink / raw)
  To: Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, Caesar Wang, Jiri Kosina,
	Alexander Kochetkov, Xing Zheng, netdev, linux-kernel,
	linux-arm-kernel

This patch will make the driver more readability

The emac has the error and warnings if you run
'scripts/checkpatch.pl -f --subjective xxx' to check.

Let's clean up such trivial details.

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

 drivers/net/ethernet/arc/emac.h          | 54 +++++++++++++++++---------------
 drivers/net/ethernet/arc/emac_main.c     | 35 ++++++++++-----------
 drivers/net/ethernet/arc/emac_mdio.c     |  2 +-
 drivers/net/ethernet/arc/emac_rockchip.c | 41 +++++++++++++++++-------
 4 files changed, 75 insertions(+), 57 deletions(-)

diff --git a/drivers/net/ethernet/arc/emac.h b/drivers/net/ethernet/arc/emac.h
index dae1ac3..63c2ba2 100644
--- a/drivers/net/ethernet/arc/emac.h
+++ b/drivers/net/ethernet/arc/emac.h
@@ -14,36 +14,36 @@
 #include <linux/clk.h>
 
 /* STATUS and ENABLE Register bit masks */
-#define TXINT_MASK	(1<<0)	/* Transmit interrupt */
-#define RXINT_MASK	(1<<1)	/* Receive interrupt */
-#define ERR_MASK	(1<<2)	/* Error interrupt */
-#define TXCH_MASK	(1<<3)	/* Transmit chaining error interrupt */
-#define MSER_MASK	(1<<4)	/* Missed packet counter error */
-#define RXCR_MASK	(1<<8)	/* RXCRCERR counter rolled over  */
-#define RXFR_MASK	(1<<9)	/* RXFRAMEERR counter rolled over */
-#define RXFL_MASK	(1<<10)	/* RXOFLOWERR counter rolled over */
-#define MDIO_MASK	(1<<12)	/* MDIO complete interrupt */
-#define TXPL_MASK	(1<<31)	/* Force polling of BD by EMAC */
+#define TXINT_MASK	BIT(0)	/* Transmit interrupt */
+#define RXINT_MASK	BIT(1)	/* Receive interrupt */
+#define ERR_MASK	BIT(2)	/* Error interrupt */
+#define TXCH_MASK	BIT(3)	/* Transmit chaining error interrupt */
+#define MSER_MASK	BIT(4)	/* Missed packet counter error */
+#define RXCR_MASK	BIT(8)	/* RXCRCERR counter rolled over  */
+#define RXFR_MASK	BIT(9)	/* RXFRAMEERR counter rolled over */
+#define RXFL_MASK	BIT(10)	/* RXOFLOWERR counter rolled over */
+#define MDIO_MASK	BIT(12)	/* MDIO complete interrupt */
+#define TXPL_MASK	BIT(31)	/* Force polling of BD by EMAC */
 
 /* CONTROL Register bit masks */
-#define EN_MASK		(1<<0)	/* VMAC enable */
-#define TXRN_MASK	(1<<3)	/* TX enable */
-#define RXRN_MASK	(1<<4)	/* RX enable */
-#define DSBC_MASK	(1<<8)	/* Disable receive broadcast */
-#define ENFL_MASK	(1<<10)	/* Enable Full-duplex */
-#define PROM_MASK	(1<<11)	/* Promiscuous mode */
+#define EN_MASK		BIT(0)	/* VMAC enable */
+#define TXRN_MASK	BIT(3)	/* TX enable */
+#define RXRN_MASK	BIT(4)	/* RX enable */
+#define DSBC_MASK	BIT(8)	/* Disable receive broadcast */
+#define ENFL_MASK	BIT(10)	/* Enable Full-duplex */
+#define PROM_MASK	BIT(11)	/* Promiscuous mode */
 
 /* Buffer descriptor INFO bit masks */
-#define OWN_MASK	(1<<31)	/* 0-CPU owns buffer, 1-EMAC owns buffer */
-#define FIRST_MASK	(1<<16)	/* First buffer in chain */
-#define LAST_MASK	(1<<17)	/* Last buffer in chain */
+#define OWN_MASK	BIT(31)	/* 0-CPU or 1-EMAC owns buffer */
+#define FIRST_MASK	BIT(16)	/* First buffer in chain */
+#define LAST_MASK	BIT(17)	/* Last buffer in chain */
 #define LEN_MASK	0x000007FF	/* last 11 bits */
-#define CRLS		(1<<21)
-#define DEFR		(1<<22)
-#define DROP		(1<<23)
-#define RTRY		(1<<24)
-#define LTCL		(1<<28)
-#define UFLO		(1<<29)
+#define CRLS		BIT(21)
+#define DEFR		BIT(22)
+#define DROP		BIT(23)
+#define RTRY		BIT(24)
+#define LTCL		BIT(28)
+#define UFLO		BIT(29)
 
 #define FOR_EMAC	OWN_MASK
 #define FOR_CPU		0
@@ -66,7 +66,7 @@ enum {
 	R_MDIO,
 };
 
-#define TX_TIMEOUT		(400*HZ/1000)	/* Transmission timeout */
+#define TX_TIMEOUT		(400 * HZ / 1000) /* Transmission timeout */
 
 #define ARC_EMAC_NAPI_WEIGHT	40		/* Workload for NAPI */
 
@@ -190,6 +190,7 @@ static inline unsigned int arc_reg_get(struct arc_emac_priv *priv, int reg)
 static inline void arc_reg_or(struct arc_emac_priv *priv, int reg, int mask)
 {
 	unsigned int value = arc_reg_get(priv, reg);
+
 	arc_reg_set(priv, reg, value | mask);
 }
 
@@ -205,6 +206,7 @@ static inline void arc_reg_or(struct arc_emac_priv *priv, int reg, int mask)
 static inline void arc_reg_clr(struct arc_emac_priv *priv, int reg, int mask)
 {
 	unsigned int value = arc_reg_get(priv, reg);
+
 	arc_reg_set(priv, reg, value & ~mask);
 }
 
diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
index 42384f6a..69760d4 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -26,7 +26,6 @@
 
 #include "emac.h"
 
-
 /**
  * arc_emac_tx_avail - Return the number of available slots in the tx ring.
  * @priv: Pointer to ARC EMAC private data structure.
@@ -66,7 +65,7 @@ static void arc_emac_adjust_link(struct net_device *ndev)
 	if (priv->duplex != phy_dev->duplex) {
 		reg = arc_reg_get(priv, R_CTRL);
 
-		if (DUPLEX_FULL == phy_dev->duplex)
+		if (phy_dev->duplex == DUPLEX_FULL)
 			reg |= ENFL_MASK;
 		else
 			reg &= ~ENFL_MASK;
@@ -466,9 +465,9 @@ static int arc_emac_open(struct net_device *ndev)
 
 	/* Set CONTROL */
 	arc_reg_set(priv, R_CTRL,
-		     (RX_BD_NUM << 24) |	/* RX BD table length */
-		     (TX_BD_NUM << 16) |	/* TX BD table length */
-		     TXRN_MASK | RXRN_MASK);
+		    (RX_BD_NUM << 24) |	/* RX BD table length */
+		    (TX_BD_NUM << 16) |	/* TX BD table length */
+		    TXRN_MASK | RXRN_MASK);
 
 	napi_enable(&priv->napi);
 
@@ -533,8 +532,10 @@ static void arc_free_tx_queue(struct net_device *ndev)
 		struct buffer_state *tx_buff = &priv->tx_buff[i];
 
 		if (tx_buff->skb) {
-			dma_unmap_single(&ndev->dev, dma_unmap_addr(tx_buff, addr),
-					 dma_unmap_len(tx_buff, len), DMA_TO_DEVICE);
+			dma_unmap_single(&ndev->dev,
+					 dma_unmap_addr(tx_buff, addr),
+					 dma_unmap_len(tx_buff, len),
+					 DMA_TO_DEVICE);
 
 			/* return the sk_buff to system */
 			dev_kfree_skb_irq(tx_buff->skb);
@@ -562,8 +563,10 @@ static void arc_free_rx_queue(struct net_device *ndev)
 		struct buffer_state *rx_buff = &priv->rx_buff[i];
 
 		if (rx_buff->skb) {
-			dma_unmap_single(&ndev->dev, dma_unmap_addr(rx_buff, addr),
-					dma_unmap_len(rx_buff, len), DMA_FROM_DEVICE);
+			dma_unmap_single(&ndev->dev,
+					 dma_unmap_addr(rx_buff, addr),
+					 dma_unmap_len(rx_buff, len),
+					 DMA_FROM_DEVICE);
 
 			/* return the sk_buff to system */
 			dev_kfree_skb_irq(rx_buff->skb);
@@ -717,8 +720,8 @@ static void arc_emac_set_address_internal(struct net_device *ndev)
 	struct arc_emac_priv *priv = netdev_priv(ndev);
 	unsigned int addr_low, addr_hi;
 
-	addr_low = le32_to_cpu(*(__le32 *) &ndev->dev_addr[0]);
-	addr_hi = le16_to_cpu(*(__le16 *) &ndev->dev_addr[4]);
+	addr_low = le32_to_cpu(*(__le32 *)&ndev->dev_addr[0]);
+	addr_hi = le16_to_cpu(*(__le16 *)&ndev->dev_addr[4]);
 
 	arc_reg_set(priv, R_ADDRL, addr_low);
 	arc_reg_set(priv, R_ADDRH, addr_hi);
@@ -813,7 +816,6 @@ int arc_emac_probe(struct net_device *ndev, int interface)
 	unsigned int id, clock_frequency, irq;
 	int err;
 
-
 	/* Get PHY from device tree */
 	phy_node = of_parse_phandle(dev->of_node, "phy", 0);
 	if (!phy_node) {
@@ -835,7 +837,6 @@ int arc_emac_probe(struct net_device *ndev, int interface)
 		return -ENODEV;
 	}
 
-
 	ndev->netdev_ops = &arc_emac_netdev_ops;
 	ndev->ethtool_ops = &arc_emac_ethtool_ops;
 	ndev->watchdog_timeo = TX_TIMEOUT;
@@ -848,9 +849,9 @@ int arc_emac_probe(struct net_device *ndev, int interface)
 	priv->dev = dev;
 
 	priv->regs = devm_ioremap_resource(dev, &res_regs);
-	if (IS_ERR(priv->regs)) {
+	if (IS_ERR(priv->regs))
 		return PTR_ERR(priv->regs);
-	}
+
 	dev_dbg(dev, "Registers base address is 0x%p\n", priv->regs);
 
 	if (priv->clk) {
@@ -971,10 +972,8 @@ int arc_emac_remove(struct net_device *ndev)
 	unregister_netdev(ndev);
 	netif_napi_del(&priv->napi);
 
-	if (!IS_ERR(priv->clk)) {
+	if (!IS_ERR(priv->clk))
 		clk_disable_unprepare(priv->clk);
-	}
-
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/arc/emac_mdio.c b/drivers/net/ethernet/arc/emac_mdio.c
index d5ee986..e312652 100644
--- a/drivers/net/ethernet/arc/emac_mdio.c
+++ b/drivers/net/ethernet/arc/emac_mdio.c
@@ -93,7 +93,7 @@ static int arc_mdio_write(struct mii_bus *bus, int phy_addr,
 		phy_addr, reg_num, value);
 
 	arc_reg_set(priv, R_MDIO,
-		     0x50020000 | (phy_addr << 23) | (reg_num << 18) | value);
+		    0x50020000 | (phy_addr << 23) | (reg_num << 18) | value);
 
 	return arc_mdio_complete_wait(priv);
 }
diff --git a/drivers/net/ethernet/arc/emac_rockchip.c b/drivers/net/ethernet/arc/emac_rockchip.c
index 85e821c..e278e3d 100644
--- a/drivers/net/ethernet/arc/emac_rockchip.c
+++ b/drivers/net/ethernet/arc/emac_rockchip.c
@@ -50,7 +50,7 @@ static void emac_rockchip_set_mac_speed(void *priv, unsigned int speed)
 	u32 data;
 	int err = 0;
 
-	switch(speed) {
+	switch (speed) {
 	case 10:
 		data = (1 << (speed_offset + 16)) | (0 << speed_offset);
 		break;
@@ -83,9 +83,18 @@ static const struct emac_rockchip_soc_data emac_rk3188_emac_data = {
 };
 
 static const struct of_device_id emac_rockchip_dt_ids[] = {
-	{ .compatible = "rockchip,rk3036-emac", .data = &emac_rk3036_emac_data },
-	{ .compatible = "rockchip,rk3066-emac", .data = &emac_rk3066_emac_data },
-	{ .compatible = "rockchip,rk3188-emac", .data = &emac_rk3188_emac_data },
+	{
+		.compatible = "rockchip,rk3036-emac",
+		.data = &emac_rk3036_emac_data,
+	},
+	{
+		.compatible = "rockchip,rk3066-emac",
+		.data = &emac_rk3066_emac_data,
+	},
+	{
+		.compatible = "rockchip,rk3188-emac",
+		.data = &emac_rk3188_emac_data,
+	},
 	{ /* Sentinel */ }
 };
 
@@ -123,9 +132,11 @@ static int emac_rockchip_probe(struct platform_device *pdev)
 		goto out_netdev;
 	}
 
-	priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,grf");
+	priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
+						    "rockchip,grf");
 	if (IS_ERR(priv->grf)) {
-		dev_err(dev, "failed to retrieve global register file (%ld)\n", PTR_ERR(priv->grf));
+		dev_err(dev, "failed to retrieve global register file (%ld)\n",
+			PTR_ERR(priv->grf));
 		err = PTR_ERR(priv->grf);
 		goto out_netdev;
 	}
@@ -135,14 +146,16 @@ static int emac_rockchip_probe(struct platform_device *pdev)
 
 	priv->emac.clk = devm_clk_get(dev, "hclk");
 	if (IS_ERR(priv->emac.clk)) {
-		dev_err(dev, "failed to retrieve host clock (%ld)\n", PTR_ERR(priv->emac.clk));
+		dev_err(dev, "failed to retrieve host clock (%ld)\n",
+			PTR_ERR(priv->emac.clk));
 		err = PTR_ERR(priv->emac.clk);
 		goto out_netdev;
 	}
 
 	priv->refclk = devm_clk_get(dev, "macref");
 	if (IS_ERR(priv->refclk)) {
-		dev_err(dev, "failed to retrieve reference clock (%ld)\n", PTR_ERR(priv->refclk));
+		dev_err(dev, "failed to retrieve reference clock (%ld)\n",
+			PTR_ERR(priv->refclk));
 		err = PTR_ERR(priv->refclk);
 		goto out_netdev;
 	}
@@ -179,19 +192,22 @@ static int emac_rockchip_probe(struct platform_device *pdev)
 
 	err = regmap_write(priv->grf, priv->soc_data->grf_offset, data);
 	if (err) {
-		dev_err(dev, "unable to apply initial settings to grf (%d)\n", err);
+		dev_err(dev, "unable to apply initial settings to grf (%d)\n",
+			err);
 		goto out_regulator_disable;
 	}
 
 	/* RMII interface needs always a rate of 50MHz */
 	err = clk_set_rate(priv->refclk, 50000000);
 	if (err)
-		dev_err(dev, "failed to change reference clock rate (%d)\n", err);
+		dev_err(dev,
+			"failed to change reference clock rate (%d)\n", err);
 
 	if (priv->soc_data->need_div_macclk) {
 		priv->macclk = devm_clk_get(dev, "macclk");
 		if (IS_ERR(priv->macclk)) {
-			dev_err(dev, "failed to retrieve mac clock (%ld)\n", PTR_ERR(priv->macclk));
+			dev_err(dev, "failed to retrieve mac clock (%ld)\n",
+				PTR_ERR(priv->macclk));
 			err = PTR_ERR(priv->macclk);
 			goto out_regulator_disable;
 		}
@@ -205,7 +221,8 @@ static int emac_rockchip_probe(struct platform_device *pdev)
 		/* RMII TX/RX needs always a rate of 25MHz */
 		err = clk_set_rate(priv->macclk, 25000000);
 		if (err)
-			dev_err(dev, "failed to change mac clock rate (%d)\n", err);
+			dev_err(dev,
+				"failed to change mac clock rate (%d)\n", err);
 	}
 
 	err = arc_emac_probe(ndev, interface);
-- 
1.9.1

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

* [PATCH 5/6] clk: rockchip: rk3036: fix and add node id for emac clock
  2016-03-11 10:55 [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Caesar Wang
                   ` (3 preceding siblings ...)
  2016-03-11 10:55 ` [PATCH 4/6] net: arc: trivial: cleanup the " Caesar Wang
@ 2016-03-11 10:55 ` Caesar Wang
  2016-03-11 11:15   ` Heiko Stübner
  2016-03-11 10:55 ` [PATCH 6/6] ARM: dts: rockchip: add support emac for RK3036 Caesar Wang
  2016-03-11 13:46 ` [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Sergei Shtylyov
  6 siblings, 1 reply; 22+ messages in thread
From: Caesar Wang @ 2016-03-11 10:55 UTC (permalink / raw)
  To: Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, zhengxing, Caesar Wang,
	Michael Turquette, Stephen Boyd, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, linux-clk, linux-arm-kernel,
	linux-kernel, devicetree

From: zhengxing <zhengxing@rock-chips.com>

In the emac driver, we need to refer HCLK_MAC since there are
only 3PLLs (APLL/GPLL/DPLL) on the rk3036, most clocks are under the
GPLL, and it is unable to provide the accurate rate for mac_ref which
need to 50MHz probability, we should let it under the DPLL and are
able to set the freq which integer multiples of 50MHz, so we add these
emac node for reference.

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

 drivers/clk/rockchip/clk-rk3036.c      | 9 ++++++---
 include/dt-bindings/clock/rk3036-cru.h | 2 ++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index 0703c8f..27c35fa 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -348,8 +348,11 @@ static struct rockchip_clk_branch rk3036_clk_branches[] __initdata = {
 			RK2928_CLKSEL_CON(16), 0, 2, MFLAGS, 2, 5, DFLAGS,
 			RK2928_CLKGATE_CON(10), 5, GFLAGS),
 
-	COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
-			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),
+	MUX(SCLK_MACPLL, "mac_pll_pre", mux_pll_src_3plls_p, 0,
+			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS),
+	DIV(0, "mac_pll_src", "mac_pll_pre", 0,
+			RK2928_CLKSEL_CON(21), 9, 5, DFLAGS),
+
 	MUX(SCLK_MACREF, "mac_clk_ref", mux_mac_p, CLK_SET_RATE_PARENT,
 			RK2928_CLKSEL_CON(21), 3, 1, MFLAGS),
 
@@ -408,7 +411,7 @@ static struct rockchip_clk_branch rk3036_clk_branches[] __initdata = {
 	GATE(HCLK_OTG1, "hclk_otg1", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(7), 3, GFLAGS),
 	GATE(HCLK_I2S, "hclk_i2s", "hclk_peri", 0, RK2928_CLKGATE_CON(7), 2, GFLAGS),
 	GATE(0, "hclk_sfc", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 14, GFLAGS),
-	GATE(0, "hclk_mac", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 15, GFLAGS),
+	GATE(HCLK_MAC, "hclk_mac", "hclk_peri", 0, RK2928_CLKGATE_CON(3), 5, GFLAGS),
 
 	/* pclk_peri gates */
 	GATE(0, "pclk_peri_matrix", "pclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(4), 1, GFLAGS),
diff --git a/include/dt-bindings/clock/rk3036-cru.h b/include/dt-bindings/clock/rk3036-cru.h
index ebc7a7b..de44109 100644
--- a/include/dt-bindings/clock/rk3036-cru.h
+++ b/include/dt-bindings/clock/rk3036-cru.h
@@ -54,6 +54,7 @@
 #define SCLK_PVTM_VIDEO		125
 #define SCLK_MAC		151
 #define SCLK_MACREF		152
+#define SCLK_MACPLL		153
 #define SCLK_SFC		160
 
 /* aclk gates */
@@ -92,6 +93,7 @@
 #define HCLK_SDMMC		456
 #define HCLK_SDIO		457
 #define HCLK_EMMC		459
+#define HCLK_MAC		460
 #define HCLK_I2S		462
 #define HCLK_LCDC		465
 #define HCLK_ROM		467
-- 
1.9.1

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

* [PATCH 6/6] ARM: dts: rockchip: add support emac for RK3036
  2016-03-11 10:55 [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Caesar Wang
                   ` (4 preceding siblings ...)
  2016-03-11 10:55 ` [PATCH 5/6] clk: rockchip: rk3036: fix and add node id for emac clock Caesar Wang
@ 2016-03-11 10:55 ` Caesar Wang
  2016-03-11 13:46 ` [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Sergei Shtylyov
  6 siblings, 0 replies; 22+ messages in thread
From: Caesar Wang @ 2016-03-11 10:55 UTC (permalink / raw)
  To: Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, zhengxing, Caesar Wang,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King,
	linux-arm-kernel, devicetree, linux-kernel

From: zhengxing <zhengxing@rock-chips.com>

This patch adds the emac device node for rk3036.
We need to let mac clock under the DPLL which is able to provide
the accurate 50MHz what mac_ref need, since that will cause some
unstable things if the cpufreq is working.

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

---

 arch/arm/boot/dts/rk3036-evb.dts   | 23 ++++++++++++++++++++++
 arch/arm/boot/dts/rk3036-kylin.dts | 20 +++++++++++++++++++
 arch/arm/boot/dts/rk3036.dtsi      | 39 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)

diff --git a/arch/arm/boot/dts/rk3036-evb.dts b/arch/arm/boot/dts/rk3036-evb.dts
index 28a0336..d7d3719 100644
--- a/arch/arm/boot/dts/rk3036-evb.dts
+++ b/arch/arm/boot/dts/rk3036-evb.dts
@@ -47,6 +47,17 @@
 	compatible = "rockchip,rk3036-evb", "rockchip,rk3036";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&emac_xfer>, <&emac_mdio>, <&rmii_rst>;
+	phy = <&phy0>;
+	status = "okay";
+
+	phy0: ethernet-phy@0 {
+		reg = <0>;
+	};
+};
+
 &i2c1 {
 	status = "okay";
 
@@ -62,3 +73,15 @@
 &uart2 {
 	status = "okay";
 };
+
+&pinctrl {
+	pcfg_output_high: pcfg-output-high {
+		output-high;
+	};
+
+	emac {
+		rmii_rst: rmii-rst {
+			rockchip,pins = <2 22 RK_FUNC_GPIO &pcfg_output_high>;
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/rk3036-kylin.dts b/arch/arm/boot/dts/rk3036-kylin.dts
index eb9c979..ab2744c 100644
--- a/arch/arm/boot/dts/rk3036-kylin.dts
+++ b/arch/arm/boot/dts/rk3036-kylin.dts
@@ -112,6 +112,20 @@
 	status = "okay";
 };
 
+&emac {
+	pinctrl-names = "default";
+	pinctrl-0 = <&emac_xfer>, <&emac_mdio>, <&rmii_rst>;
+	phy = <&phy0>;
+	phy-reset-gpios = <&gpio2 22 GPIO_ACTIVE_LOW>; /* PHY_RST */
+	phy-reset-duration = <10>; /* millisecond */
+
+	status = "okay";
+
+	phy0: ethernet-phy@0 {
+		reg = <0>;
+	};
+};
+
 &emmc {
 	status = "okay";
 };
@@ -383,6 +397,12 @@
 };
 
 &pinctrl {
+	emac {
+		rmii_rst: rmii-rst {
+			rockchip,pins = <2 22 RK_FUNC_GPIO &pcfg_pull_default>;
+		};
+	};
+
 	leds {
 		led_ctl: led-ctl {
 			rockchip,pins = <2 30 RK_FUNC_GPIO &pcfg_pull_none>;
diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index 90faa86..5175a2a 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -223,6 +223,27 @@
 		status = "disabled";
 	};
 
+	emac: ethernet@10200000 {
+		compatible = "rockchip,rk3036-emac", "snps,arc-emac";
+		reg = <0x10200000 0x4000>;
+		interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		rockchip,grf = <&grf>;
+		clocks = <&cru HCLK_MAC>, <&cru SCLK_MACREF>, <&cru SCLK_MAC>;
+		clock-names = "hclk", "macref", "macclk";
+		/*
+		 * Fix the emac parent clock is DPLL instead of APLL.
+		 * since that will cause some unstable things if the cpufreq
+		 * is working. (e.g: the accurate 50MHz what mac_ref need)
+		 */
+		assigned-clocks = <&cru SCLK_MACPLL>;
+		assigned-clock-parents = <&cru PLL_DPLL>;
+		max-speed = <100>;
+		phy-mode = "rmii";
+		status = "disabled";
+	};
+
 	sdmmc: dwmmc@10214000 {
 		compatible = "rockchip,rk3036-dw-mshc", "rockchip,rk3288-dw-mshc";
 		reg = <0x10214000 0x4000>;
@@ -628,6 +649,24 @@
 			};
 		};
 
+		emac {
+			emac_xfer: emac-xfer {
+				rockchip,pins = <2 10 RK_FUNC_1 &pcfg_pull_default>, /* crs_dvalid */
+						<2 13 RK_FUNC_1 &pcfg_pull_default>, /* tx_en */
+						<2 14 RK_FUNC_1 &pcfg_pull_default>, /* mac_clk */
+						<2 15 RK_FUNC_1 &pcfg_pull_default>, /* rx_err */
+						<2 16 RK_FUNC_1 &pcfg_pull_default>, /* rxd1 */
+						<2 17 RK_FUNC_1 &pcfg_pull_default>, /* rxd0 */
+						<2 18 RK_FUNC_1 &pcfg_pull_default>, /* txd1 */
+						<2 19 RK_FUNC_1 &pcfg_pull_default>; /* txd0 */
+			};
+
+			emac_mdio: emac-mdio {
+				rockchip,pins = <2 12 RK_FUNC_1 &pcfg_pull_default>, /* mac_md */
+						<2 25 RK_FUNC_1 &pcfg_pull_default>; /* mac_mdclk */
+			};
+		};
+
 		i2c0 {
 			i2c0_xfer: i2c0-xfer {
 				rockchip,pins = <0 0 RK_FUNC_1 &pcfg_pull_none>,
-- 
1.9.1

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

* Re: [PATCH 5/6] clk: rockchip: rk3036: fix and add node id for emac clock
  2016-03-11 10:55 ` [PATCH 5/6] clk: rockchip: rk3036: fix and add node id for emac clock Caesar Wang
@ 2016-03-11 11:15   ` Heiko Stübner
  2016-03-11 12:01     ` Caesar Wang
  0 siblings, 1 reply; 22+ messages in thread
From: Heiko Stübner @ 2016-03-11 11:15 UTC (permalink / raw)
  To: Caesar Wang
  Cc: David S. Miller, Rob Herring, linux-rockchip, keescook, leozwang,
	zhengxing, Michael Turquette, Stephen Boyd, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, linux-clk,
	linux-arm-kernel, linux-kernel, devicetree

Hi Caesar,

Am Freitag, 11. März 2016, 18:55:30 schrieb Caesar Wang:
> From: zhengxing <zhengxing@rock-chips.com>
> 
> In the emac driver, we need to refer HCLK_MAC since there are
> only 3PLLs (APLL/GPLL/DPLL) on the rk3036, most clocks are under the
> GPLL, and it is unable to provide the accurate rate for mac_ref which
> need to 50MHz probability, we should let it under the DPLL and are
> able to set the freq which integer multiples of 50MHz, so we add these
> emac node for reference.
> 
> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> Signed-off-by: Caesar Wang <wxt@rock-chips.com>

I think I mentioned it somewhere before, but I'd like to do this
differently, like in [0].

That should work in a similar way and at least in my tests the reported
clock rate seems to be correct. As I said as well I haven't been able to
make the emac detect a link on my kylin boards, so it would be cool
if you could test if this different approach works in practice as well.


Thanks
Heiko

------ 8< ---------
>From e83a8b19dbf95c40d2c908727c342fbc6b167ea1 Mon Sep 17 00:00:00 2001
From: Heiko Stuebner <heiko@sntech.de>
Date: Fri, 19 Feb 2016 21:31:43 +0100
Subject: [PATCH] clk: rockchip: associate SCLK_MAC_PLL and disable reparenting
 on rk3036

The emac needs constant and very specific rate but the possible PLL-sources
are very limited, so we expect the PLL source to be set manually on per
board and don't want it to get changed in an automatic way later.
So add the necessary clock-id and disable reparenting on set_rate calls.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/rockchip/clk-rk3036.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index 3c742bf..0084c57 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -348,7 +348,7 @@ static struct rockchip_clk_branch rk3036_clk_branches[] __initdata = {
 			RK2928_CLKSEL_CON(16), 0, 2, MFLAGS, 2, 5, DFLAGS,
 			RK2928_CLKGATE_CON(10), 5, GFLAGS),
 
-	COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
+	COMPOSITE_NOGATE(SCLK_MACPLL, "mac_pll_src", mux_pll_src_3plls_p, CLK_SET_RATE_NO_REPARENT,
 			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 4, 5, DFLAGS),
 	MUX(SCLK_MACREF, "mac_clk_ref", mux_mac_p, CLK_SET_RATE_PARENT,
 			RK2928_CLKSEL_CON(21), 3, 1, MFLAGS),

------ 8< ---------


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


> ---
> 
>  drivers/clk/rockchip/clk-rk3036.c      | 9 ++++++---
>  include/dt-bindings/clock/rk3036-cru.h | 2 ++
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3036.c
> b/drivers/clk/rockchip/clk-rk3036.c index 0703c8f..27c35fa 100644
> --- a/drivers/clk/rockchip/clk-rk3036.c
> +++ b/drivers/clk/rockchip/clk-rk3036.c
> @@ -348,8 +348,11 @@ static struct rockchip_clk_branch rk3036_clk_branches[]
> __initdata = { RK2928_CLKSEL_CON(16), 0, 2, MFLAGS, 2, 5, DFLAGS,
>  			RK2928_CLKGATE_CON(10), 5, GFLAGS),
> 
> -	COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
> -			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),
> +	MUX(SCLK_MACPLL, "mac_pll_pre", mux_pll_src_3plls_p, 0,
> +			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS),
> +	DIV(0, "mac_pll_src", "mac_pll_pre", 0,
> +			RK2928_CLKSEL_CON(21), 9, 5, DFLAGS),
> +
>  	MUX(SCLK_MACREF, "mac_clk_ref", mux_mac_p, CLK_SET_RATE_PARENT,
>  			RK2928_CLKSEL_CON(21), 3, 1, MFLAGS),
> 
> @@ -408,7 +411,7 @@ static struct rockchip_clk_branch rk3036_clk_branches[]
> __initdata = { GATE(HCLK_OTG1, "hclk_otg1", "hclk_peri", CLK_IGNORE_UNUSED,
> RK2928_CLKGATE_CON(7), 3, GFLAGS), GATE(HCLK_I2S, "hclk_i2s", "hclk_peri",
> 0, RK2928_CLKGATE_CON(7), 2, GFLAGS), GATE(0, "hclk_sfc", "hclk_peri",
> CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 14, GFLAGS), -	GATE(0,
> "hclk_mac", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 15,
> GFLAGS), +	GATE(HCLK_MAC, "hclk_mac", "hclk_peri", 0,
> RK2928_CLKGATE_CON(3), 5, GFLAGS),
> 
>  	/* pclk_peri gates */
>  	GATE(0, "pclk_peri_matrix", "pclk_peri", CLK_IGNORE_UNUSED,
> RK2928_CLKGATE_CON(4), 1, GFLAGS), diff --git
> a/include/dt-bindings/clock/rk3036-cru.h
> b/include/dt-bindings/clock/rk3036-cru.h index ebc7a7b..de44109 100644
> --- a/include/dt-bindings/clock/rk3036-cru.h
> +++ b/include/dt-bindings/clock/rk3036-cru.h
> @@ -54,6 +54,7 @@
>  #define SCLK_PVTM_VIDEO		125
>  #define SCLK_MAC		151
>  #define SCLK_MACREF		152
> +#define SCLK_MACPLL		153
>  #define SCLK_SFC		160
> 
>  /* aclk gates */
> @@ -92,6 +93,7 @@
>  #define HCLK_SDMMC		456
>  #define HCLK_SDIO		457
>  #define HCLK_EMMC		459
> +#define HCLK_MAC		460
>  #define HCLK_I2S		462
>  #define HCLK_LCDC		465
>  #define HCLK_ROM		467

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

* Re: [PATCH 4/6] net: arc: trivial: cleanup the emac driver
  2016-03-11 10:55 ` [PATCH 4/6] net: arc: trivial: cleanup the " Caesar Wang
@ 2016-03-11 11:28   ` kbuild test robot
  0 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2016-03-11 11:28 UTC (permalink / raw)
  To: Caesar Wang
  Cc: kbuild-all, Heiko Stuebner, David S. Miller, Rob Herring,
	linux-rockchip, keescook, leozwang, Caesar Wang, Jiri Kosina,
	Alexander Kochetkov, Xing Zheng, netdev, linux-kernel,
	linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 3246 bytes --]

Hi Caesar,

[auto build test WARNING on next-20160311]
[also build test WARNING on v4.5-rc7]
[cannot apply to rockchip/for-next net-next/master v4.5-rc7 v4.5-rc6 v4.5-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Caesar-Wang/net-arc_emac-make-the-rockchip-emac-document-more-compatible/20160311-190611
config: alpha-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=alpha 

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/arc/emac_main.c: In function 'arc_emac_open':
>> drivers/net/ethernet/arc/emac_main.c:468:7: warning: overflow in implicit constant conversion [-Woverflow]
          (RX_BD_NUM << 24) | /* RX BD table length */
          ^

vim +468 drivers/net/ethernet/arc/emac_main.c

e4f2379d Alexey Brodkin    2013-06-24  452  	/* Clean Tx BD's */
e4f2379d Alexey Brodkin    2013-06-24  453  	memset(priv->txbd, 0, TX_RING_SZ);
e4f2379d Alexey Brodkin    2013-06-24  454  
e4f2379d Alexey Brodkin    2013-06-24  455  	/* Initialize logical address filter */
e4f2379d Alexey Brodkin    2013-06-24  456  	arc_reg_set(priv, R_LAFL, 0);
e4f2379d Alexey Brodkin    2013-06-24  457  	arc_reg_set(priv, R_LAFH, 0);
e4f2379d Alexey Brodkin    2013-06-24  458  
e4f2379d Alexey Brodkin    2013-06-24  459  	/* Set BD ring pointers for device side */
e4f2379d Alexey Brodkin    2013-06-24  460  	arc_reg_set(priv, R_RX_RING, (unsigned int)priv->rxbd_dma);
e4f2379d Alexey Brodkin    2013-06-24  461  	arc_reg_set(priv, R_TX_RING, (unsigned int)priv->txbd_dma);
e4f2379d Alexey Brodkin    2013-06-24  462  
e4f2379d Alexey Brodkin    2013-06-24  463  	/* Enable interrupts */
7ce7679d Beniamino Galvani 2014-09-10  464  	arc_reg_set(priv, R_ENABLE, RXINT_MASK | TXINT_MASK | ERR_MASK);
e4f2379d Alexey Brodkin    2013-06-24  465  
e4f2379d Alexey Brodkin    2013-06-24  466  	/* Set CONTROL */
e4f2379d Alexey Brodkin    2013-06-24  467  	arc_reg_set(priv, R_CTRL,
e4f2379d Alexey Brodkin    2013-06-24 @468  		    (RX_BD_NUM << 24) |	/* RX BD table length */
e4f2379d Alexey Brodkin    2013-06-24  469  		    (TX_BD_NUM << 16) |	/* TX BD table length */
e4f2379d Alexey Brodkin    2013-06-24  470  		    TXRN_MASK | RXRN_MASK);
e4f2379d Alexey Brodkin    2013-06-24  471  
e4f2379d Alexey Brodkin    2013-06-24  472  	napi_enable(&priv->napi);
e4f2379d Alexey Brodkin    2013-06-24  473  
e4f2379d Alexey Brodkin    2013-06-24  474  	/* Enable EMAC */
e4f2379d Alexey Brodkin    2013-06-24  475  	arc_reg_or(priv, R_CTRL, EN_MASK);
e4f2379d Alexey Brodkin    2013-06-24  476  

:::::: The code at line 468 was first introduced by commit
:::::: e4f2379db6c6823c5d4a4c2c912df00c65de51d7 ethernet/arc/arc_emac - Add new driver

:::::: TO: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 45778 bytes --]

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

* Re: [PATCH 5/6] clk: rockchip: rk3036: fix and add node id for emac clock
  2016-03-11 11:15   ` Heiko Stübner
@ 2016-03-11 12:01     ` Caesar Wang
  2016-03-11 12:28       ` Heiko Stübner
  0 siblings, 1 reply; 22+ messages in thread
From: Caesar Wang @ 2016-03-11 12:01 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Caesar Wang, Mark Rutland, devicetree, Pawel Moll, zhengxing,
	Ian Campbell, Michael Turquette, Kumar Gala, Stephen Boyd,
	linux-kernel, linux-clk, linux-rockchip, Rob Herring,
	linux-arm-kernel, keescook, David S. Miller, leozwang

Hi Heiko,

The link [0] need a bit changes if we want the emac to be happy work.

-RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 4, 5, DFLAGS),

+RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),


I will need resend the series patches with your change in link[0], OK?

Since the Mr.rebot just notice the build error, I will check and resend 
with your emac changing.




在 2016年03月11日 19:15, Heiko Stübner 写道:
> Hi Caesar,
>
> Am Freitag, 11. März 2016, 18:55:30 schrieb Caesar Wang:
>> From: zhengxing <zhengxing@rock-chips.com>
>>
>> In the emac driver, we need to refer HCLK_MAC since there are
>> only 3PLLs (APLL/GPLL/DPLL) on the rk3036, most clocks are under the
>> GPLL, and it is unable to provide the accurate rate for mac_ref which
>> need to 50MHz probability, we should let it under the DPLL and are
>> able to set the freq which integer multiples of 50MHz, so we add these
>> emac node for reference.
>>
>> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> I think I mentioned it somewhere before, but I'd like to do this
> differently, like in [0].
>
> That should work in a similar way and at least in my tests the reported
> clock rate seems to be correct. As I said as well I haven't been able to
> make the emac detect a link on my kylin boards, so it would be cool
> if you could test if this different approach works in practice as well.

I fetch your branch patches, it doesn't work for me.
c467a5f clk: rockchip: associate SCLK_MAC_PLL and disable reparenting on 
rk3036
ae7ed09 clk: rockchip: add clock-id for rk3036 emac pll source clock
f876a7e clk: rockchip: associate the rk3036 HCLK_EMAC clock-id
5093371 clk: rockchip: add node-id for rk3036 emac hclk

f44eeee Revert "clk: rockchip: rk3036: fix and add node id for emac clock"
..

It works if the patch
c467a5f clk: rockchip: associate SCLK_MAC_PLL and disable reparenting on 
rk3036 to change as following

--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -348,8 +348,8 @@ static struct rockchip_clk_branch 
rk3036_clk_branches[] __initdata = {
                         RK2928_CLKSEL_CON(16), 0, 2, MFLAGS, 2, 5, DFLAGS,
                         RK2928_CLKGATE_CON(10), 5, GFLAGS),

-       COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
-                       RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),
+       COMPOSITE_NOGATE(SCLK_MACPLL, "mac_pll_src", 
mux_pll_src_3plls_p, CLK_SET_RATE_NO_REPARENT,
+                       RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),


>
>
> Thanks
> Heiko
>
> ------ 8< ---------
>  From e83a8b19dbf95c40d2c908727c342fbc6b167ea1 Mon Sep 17 00:00:00 2001
> From: Heiko Stuebner <heiko@sntech.de>
> Date: Fri, 19 Feb 2016 21:31:43 +0100
> Subject: [PATCH] clk: rockchip: associate SCLK_MAC_PLL and disable reparenting
>   on rk3036
>
> The emac needs constant and very specific rate but the possible PLL-sources
> are very limited, so we expect the PLL source to be set manually on per
> board and don't want it to get changed in an automatic way later.
> So add the necessary clock-id and disable reparenting on set_rate calls.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   drivers/clk/rockchip/clk-rk3036.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
> index 3c742bf..0084c57 100644
> --- a/drivers/clk/rockchip/clk-rk3036.c
> +++ b/drivers/clk/rockchip/clk-rk3036.c
> @@ -348,7 +348,7 @@ static struct rockchip_clk_branch rk3036_clk_branches[] __initdata = {
>   			RK2928_CLKSEL_CON(16), 0, 2, MFLAGS, 2, 5, DFLAGS,
>   			RK2928_CLKGATE_CON(10), 5, GFLAGS),
>   
> -	COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
> +	COMPOSITE_NOGATE(SCLK_MACPLL, "mac_pll_src", mux_pll_src_3plls_p, CLK_SET_RATE_NO_REPARENT,
>   			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 4, 5, DFLAGS),

-RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 4, 5, DFLAGS),

+RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),



>   	MUX(SCLK_MACREF, "mac_clk_ref", mux_mac_p, CLK_SET_RATE_PARENT,
>   			RK2928_CLKSEL_CON(21), 3, 1, MFLAGS),
>
> ------ 8< ---------
>
>
> [0] https://github.com/mmind/linux-rockchip/commit/e83a8b19dbf95c40d2c908727c342fbc6b167ea1
>
>
>> ---
>>
>>   drivers/clk/rockchip/clk-rk3036.c      | 9 ++++++---
>>   include/dt-bindings/clock/rk3036-cru.h | 2 ++
>>   2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/rockchip/clk-rk3036.c
>> b/drivers/clk/rockchip/clk-rk3036.c index 0703c8f..27c35fa 100644
>> --- a/drivers/clk/rockchip/clk-rk3036.c
>> +++ b/drivers/clk/rockchip/clk-rk3036.c
>> @@ -348,8 +348,11 @@ static struct rockchip_clk_branch rk3036_clk_branches[]
>> __initdata = { RK2928_CLKSEL_CON(16), 0, 2, MFLAGS, 2, 5, DFLAGS,
>>   			RK2928_CLKGATE_CON(10), 5, GFLAGS),
>>
>> -	COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
>> -			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),
>> +	MUX(SCLK_MACPLL, "mac_pll_pre", mux_pll_src_3plls_p, 0,
>> +			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS),
>> +	DIV(0, "mac_pll_src", "mac_pll_pre", 0,
>> +			RK2928_CLKSEL_CON(21), 9, 5, DFLAGS),
>> +
>>   	MUX(SCLK_MACREF, "mac_clk_ref", mux_mac_p, CLK_SET_RATE_PARENT,
>>   			RK2928_CLKSEL_CON(21), 3, 1, MFLAGS),
>>
>> @@ -408,7 +411,7 @@ static struct rockchip_clk_branch rk3036_clk_branches[]
>> __initdata = { GATE(HCLK_OTG1, "hclk_otg1", "hclk_peri", CLK_IGNORE_UNUSED,
>> RK2928_CLKGATE_CON(7), 3, GFLAGS), GATE(HCLK_I2S, "hclk_i2s", "hclk_peri",
>> 0, RK2928_CLKGATE_CON(7), 2, GFLAGS), GATE(0, "hclk_sfc", "hclk_peri",
>> CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 14, GFLAGS), -	GATE(0,
>> "hclk_mac", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 15,
>> GFLAGS), +	GATE(HCLK_MAC, "hclk_mac", "hclk_peri", 0,
>> RK2928_CLKGATE_CON(3), 5, GFLAGS),
>>
>>   	/* pclk_peri gates */
>>   	GATE(0, "pclk_peri_matrix", "pclk_peri", CLK_IGNORE_UNUSED,
>> RK2928_CLKGATE_CON(4), 1, GFLAGS), diff --git
>> a/include/dt-bindings/clock/rk3036-cru.h
>> b/include/dt-bindings/clock/rk3036-cru.h index ebc7a7b..de44109 100644
>> --- a/include/dt-bindings/clock/rk3036-cru.h
>> +++ b/include/dt-bindings/clock/rk3036-cru.h
>> @@ -54,6 +54,7 @@
>>   #define SCLK_PVTM_VIDEO		125
>>   #define SCLK_MAC		151
>>   #define SCLK_MACREF		152
>> +#define SCLK_MACPLL		153
>>   #define SCLK_SFC		160
>>
>>   /* aclk gates */
>> @@ -92,6 +93,7 @@
>>   #define HCLK_SDMMC		456
>>   #define HCLK_SDIO		457
>>   #define HCLK_EMMC		459
>> +#define HCLK_MAC		460
>>   #define HCLK_I2S		462
>>   #define HCLK_LCDC		465
>>   #define HCLK_ROM		467
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip


-- 
Thanks,
Caesar

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

* Re: [PATCH 5/6] clk: rockchip: rk3036: fix and add node id for emac clock
  2016-03-11 12:01     ` Caesar Wang
@ 2016-03-11 12:28       ` Heiko Stübner
  0 siblings, 0 replies; 22+ messages in thread
From: Heiko Stübner @ 2016-03-11 12:28 UTC (permalink / raw)
  To: Caesar Wang
  Cc: Caesar Wang, Mark Rutland, devicetree, Pawel Moll, zhengxing,
	Ian Campbell, Michael Turquette, Kumar Gala, Stephen Boyd,
	linux-kernel, linux-clk, linux-rockchip, Rob Herring,
	linux-arm-kernel, keescook, David S. Miller, leozwang

Hi Caesar,

Am Freitag, 11. März 2016, 20:01:10 schrieb Caesar Wang:
> The link [0] need a bit changes if we want the emac to be happy work.
> 
> -RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 4, 5, DFLAGS),
> 
> +RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),

ah, then that is probably the reason I don't get connectivity. Thanks for 
noticing this.


> I will need resend the series patches with your change in link[0], OK?
> 
> Since the Mr.rebot just notice the build error, I will check and resend
> with your emac changing.

ok, great :-D


Thanks
Heiko


> 在 2016年03月11日 19:15, Heiko Stübner 写道:
> > Hi Caesar,
> > 
> > Am Freitag, 11. März 2016, 18:55:30 schrieb Caesar Wang:
> >> From: zhengxing <zhengxing@rock-chips.com>
> >> 
> >> In the emac driver, we need to refer HCLK_MAC since there are
> >> only 3PLLs (APLL/GPLL/DPLL) on the rk3036, most clocks are under the
> >> GPLL, and it is unable to provide the accurate rate for mac_ref which
> >> need to 50MHz probability, we should let it under the DPLL and are
> >> able to set the freq which integer multiples of 50MHz, so we add these
> >> emac node for reference.
> >> 
> >> Signed-off-by: Xing Zheng <zhengxing@rock-chips.com>
> >> Signed-off-by: Caesar Wang <wxt@rock-chips.com>
> > 
> > I think I mentioned it somewhere before, but I'd like to do this
> > differently, like in [0].
> > 
> > That should work in a similar way and at least in my tests the reported
> > clock rate seems to be correct. As I said as well I haven't been able to
> > make the emac detect a link on my kylin boards, so it would be cool
> > if you could test if this different approach works in practice as well.
> 
> I fetch your branch patches, it doesn't work for me.
> c467a5f clk: rockchip: associate SCLK_MAC_PLL and disable reparenting on
> rk3036
> ae7ed09 clk: rockchip: add clock-id for rk3036 emac pll source clock
> f876a7e clk: rockchip: associate the rk3036 HCLK_EMAC clock-id
> 5093371 clk: rockchip: add node-id for rk3036 emac hclk
> 
> f44eeee Revert "clk: rockchip: rk3036: fix and add node id for emac clock"
> ..
> 
> It works if the patch
> c467a5f clk: rockchip: associate SCLK_MAC_PLL and disable reparenting on
> rk3036 to change as following
> 
> --- a/drivers/clk/rockchip/clk-rk3036.c
> +++ b/drivers/clk/rockchip/clk-rk3036.c
> @@ -348,8 +348,8 @@ static struct rockchip_clk_branch
> rk3036_clk_branches[] __initdata = {
>                          RK2928_CLKSEL_CON(16), 0, 2, MFLAGS, 2, 5, DFLAGS,
>                          RK2928_CLKGATE_CON(10), 5, GFLAGS),
> 
> -       COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
> -                       RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),
> +       COMPOSITE_NOGATE(SCLK_MACPLL, "mac_pll_src",
> mux_pll_src_3plls_p, CLK_SET_RATE_NO_REPARENT,
> +                       RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),
> 
> > Thanks
> > Heiko
> > 
> > ------ 8< ---------
> > 
> >  From e83a8b19dbf95c40d2c908727c342fbc6b167ea1 Mon Sep 17 00:00:00 2001
> > 
> > From: Heiko Stuebner <heiko@sntech.de>
> > Date: Fri, 19 Feb 2016 21:31:43 +0100
> > Subject: [PATCH] clk: rockchip: associate SCLK_MAC_PLL and disable
> > reparenting> 
> >   on rk3036
> > 
> > The emac needs constant and very specific rate but the possible
> > PLL-sources
> > are very limited, so we expect the PLL source to be set manually on per
> > board and don't want it to get changed in an automatic way later.
> > So add the necessary clock-id and disable reparenting on set_rate calls.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > 
> >   drivers/clk/rockchip/clk-rk3036.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/rockchip/clk-rk3036.c
> > b/drivers/clk/rockchip/clk-rk3036.c index 3c742bf..0084c57 100644
> > --- a/drivers/clk/rockchip/clk-rk3036.c
> > +++ b/drivers/clk/rockchip/clk-rk3036.c
> > @@ -348,7 +348,7 @@ static struct rockchip_clk_branch
> > rk3036_clk_branches[] __initdata = {> 
> >   			RK2928_CLKSEL_CON(16), 0, 2, MFLAGS, 2, 5, DFLAGS,
> >   			RK2928_CLKGATE_CON(10), 5, GFLAGS),
> > 
> > -	COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
> > +	COMPOSITE_NOGATE(SCLK_MACPLL, "mac_pll_src", mux_pll_src_3plls_p,
> > CLK_SET_RATE_NO_REPARENT,> 
> >   			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 4, 5, DFLAGS),
> 
> -RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 4, 5, DFLAGS),
> 
> +RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),
> 
> >   	MUX(SCLK_MACREF, "mac_clk_ref", mux_mac_p, CLK_SET_RATE_PARENT,
> >   	
> >   			RK2928_CLKSEL_CON(21), 3, 1, MFLAGS),
> > 
> > ------ 8< ---------
> > 
> > 
> > [0]
> > https://github.com/mmind/linux-rockchip/commit/e83a8b19dbf95c40d2c908727c
> > 342fbc6b167ea1> 
> >> ---
> >> 
> >>   drivers/clk/rockchip/clk-rk3036.c      | 9 ++++++---
> >>   include/dt-bindings/clock/rk3036-cru.h | 2 ++
> >>   2 files changed, 8 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/clk/rockchip/clk-rk3036.c
> >> b/drivers/clk/rockchip/clk-rk3036.c index 0703c8f..27c35fa 100644
> >> --- a/drivers/clk/rockchip/clk-rk3036.c
> >> +++ b/drivers/clk/rockchip/clk-rk3036.c
> >> @@ -348,8 +348,11 @@ static struct rockchip_clk_branch
> >> rk3036_clk_branches[] __initdata = { RK2928_CLKSEL_CON(16), 0, 2,
> >> MFLAGS, 2, 5, DFLAGS,
> >> 
> >>   			RK2928_CLKGATE_CON(10), 5, GFLAGS),
> >> 
> >> -	COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
> >> -			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 9, 5, DFLAGS),
> >> +	MUX(SCLK_MACPLL, "mac_pll_pre", mux_pll_src_3plls_p, 0,
> >> +			RK2928_CLKSEL_CON(21), 0, 2, MFLAGS),
> >> +	DIV(0, "mac_pll_src", "mac_pll_pre", 0,
> >> +			RK2928_CLKSEL_CON(21), 9, 5, DFLAGS),
> >> +
> >> 
> >>   	MUX(SCLK_MACREF, "mac_clk_ref", mux_mac_p, CLK_SET_RATE_PARENT,
> >>   	
> >>   			RK2928_CLKSEL_CON(21), 3, 1, MFLAGS),
> >> 
> >> @@ -408,7 +411,7 @@ static struct rockchip_clk_branch
> >> rk3036_clk_branches[]
> >> __initdata = { GATE(HCLK_OTG1, "hclk_otg1", "hclk_peri",
> >> CLK_IGNORE_UNUSED,
> >> RK2928_CLKGATE_CON(7), 3, GFLAGS), GATE(HCLK_I2S, "hclk_i2s",
> >> "hclk_peri",
> >> 0, RK2928_CLKGATE_CON(7), 2, GFLAGS), GATE(0, "hclk_sfc", "hclk_peri",
> >> CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 14, GFLAGS), -	GATE(0,
> >> "hclk_mac", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 15,
> >> GFLAGS), +	GATE(HCLK_MAC, "hclk_mac", "hclk_peri", 0,
> >> RK2928_CLKGATE_CON(3), 5, GFLAGS),
> >> 
> >>   	/* pclk_peri gates */
> >>   	GATE(0, "pclk_peri_matrix", "pclk_peri", CLK_IGNORE_UNUSED,
> >> 
> >> RK2928_CLKGATE_CON(4), 1, GFLAGS), diff --git
> >> a/include/dt-bindings/clock/rk3036-cru.h
> >> b/include/dt-bindings/clock/rk3036-cru.h index ebc7a7b..de44109 100644
> >> --- a/include/dt-bindings/clock/rk3036-cru.h
> >> +++ b/include/dt-bindings/clock/rk3036-cru.h
> >> @@ -54,6 +54,7 @@
> >> 
> >>   #define SCLK_PVTM_VIDEO		125
> >>   #define SCLK_MAC		151
> >>   #define SCLK_MACREF		152
> >> 
> >> +#define SCLK_MACPLL		153
> >> 
> >>   #define SCLK_SFC		160
> >>   
> >>   /* aclk gates */
> >> 
> >> @@ -92,6 +93,7 @@
> >> 
> >>   #define HCLK_SDMMC		456
> >>   #define HCLK_SDIO		457
> >>   #define HCLK_EMMC		459
> >> 
> >> +#define HCLK_MAC		460
> >> 
> >>   #define HCLK_I2S		462
> >>   #define HCLK_LCDC		465
> >>   #define HCLK_ROM		467
> > 
> > _______________________________________________
> > Linux-rockchip mailing list
> > Linux-rockchip@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers
  2016-03-11 10:55 [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Caesar Wang
                   ` (5 preceding siblings ...)
  2016-03-11 10:55 ` [PATCH 6/6] ARM: dts: rockchip: add support emac for RK3036 Caesar Wang
@ 2016-03-11 13:46 ` Sergei Shtylyov
  2016-03-11 14:48   ` Caesar Wang
  6 siblings, 1 reply; 22+ messages in thread
From: Sergei Shtylyov @ 2016-03-11 13:46 UTC (permalink / raw)
  To: Caesar Wang, Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, devicetree,
	Michael Turquette, Alexander Kochetkov, Russell King,
	Stephen Boyd, netdev, Kumar Gala, linux-kernel, Ian Campbell,
	zhengxing, Jiri Kosina, Pawel Moll, Mark Rutland, linux-clk,
	linux-arm-kernel

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

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

* Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver
  2016-03-11 10:55 ` [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Caesar Wang
@ 2016-03-11 13:47   ` Sergei Shtylyov
  2016-03-11 14:56     ` Heiko Stübner
  2016-03-11 18:35   ` Sergei Shtylyov
  2016-03-11 19:22   ` Sergei Shtylyov
  2 siblings, 1 reply; 22+ messages in thread
From: Sergei Shtylyov @ 2016-03-11 13:47 UTC (permalink / raw)
  To: Caesar Wang, Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, Alexander Kochetkov, netdev,
	linux-kernel

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

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

* Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers
  2016-03-11 13:46 ` [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Sergei Shtylyov
@ 2016-03-11 14:48   ` Caesar Wang
  2016-03-11 18:46     ` Sergei Shtylyov
  0 siblings, 1 reply; 22+ messages in thread
From: Caesar Wang @ 2016-03-11 14:48 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Caesar Wang, Heiko Stuebner, David S. Miller, Rob Herring,
	Mark Rutland, devicetree, Ian Campbell, Russell King, Pawel Moll,
	zhengxing, Alexander Kochetkov, netdev, Michael Turquette,
	Kumar Gala, Stephen Boyd, linux-kernel, linux-rockchip,
	linux-arm-kernel, keescook, Jiri Kosina, linux-clk, leozwang

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

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

* Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver
  2016-03-11 13:47   ` Sergei Shtylyov
@ 2016-03-11 14:56     ` Heiko Stübner
  2016-03-11 14:59       ` Heiko Stübner
  0 siblings, 1 reply; 22+ messages in thread
From: Heiko Stübner @ 2016-03-11 14:56 UTC (permalink / raw)
  To: Sergei Shtylyov, Caesar Wang
  Cc: David S. Miller, Rob Herring, linux-rockchip, keescook, leozwang,
	Alexander Kochetkov, netdev, linux-kernel

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

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

* Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver
  2016-03-11 14:56     ` Heiko Stübner
@ 2016-03-11 14:59       ` Heiko Stübner
  0 siblings, 0 replies; 22+ messages in thread
From: Heiko Stübner @ 2016-03-11 14:59 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Caesar Wang, David S. Miller, Rob Herring, linux-rockchip,
	keescook, leozwang, Alexander Kochetkov, netdev, linux-kernel

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

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

* Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver
  2016-03-11 10:55 ` [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Caesar Wang
  2016-03-11 13:47   ` Sergei Shtylyov
@ 2016-03-11 18:35   ` Sergei Shtylyov
  2016-03-13  3:57     ` Caesar Wang
  2016-03-11 19:22   ` Sergei Shtylyov
  2 siblings, 1 reply; 22+ messages in thread
From: Sergei Shtylyov @ 2016-03-11 18:35 UTC (permalink / raw)
  To: Caesar Wang, Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, Alexander Kochetkov, netdev,
	linux-kernel

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

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

* Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers
  2016-03-11 14:48   ` Caesar Wang
@ 2016-03-11 18:46     ` Sergei Shtylyov
  2016-03-13  4:04       ` Caesar Wang
  0 siblings, 1 reply; 22+ messages in thread
From: Sergei Shtylyov @ 2016-03-11 18:46 UTC (permalink / raw)
  To: Caesar Wang
  Cc: Caesar Wang, Heiko Stuebner, David S. Miller, Rob Herring,
	Mark Rutland, devicetree, Ian Campbell, Russell King, Pawel Moll,
	zhengxing, Alexander Kochetkov, netdev, Michael Turquette,
	Kumar Gala, Stephen Boyd, linux-kernel, linux-rockchip,
	linux-arm-kernel, keescook, Jiri Kosina, linux-clk, leozwang

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

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

* Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver
  2016-03-11 10:55 ` [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Caesar Wang
  2016-03-11 13:47   ` Sergei Shtylyov
  2016-03-11 18:35   ` Sergei Shtylyov
@ 2016-03-11 19:22   ` Sergei Shtylyov
  2 siblings, 0 replies; 22+ messages in thread
From: Sergei Shtylyov @ 2016-03-11 19:22 UTC (permalink / raw)
  To: Caesar Wang, Heiko Stuebner, David S. Miller, Rob Herring
  Cc: linux-rockchip, keescook, leozwang, Alexander Kochetkov, netdev,
	linux-kernel

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

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

* Re: [PATCH 3/6] net: arc_emac: support the phy reset for emac driver
  2016-03-11 18:35   ` Sergei Shtylyov
@ 2016-03-13  3:57     ` Caesar Wang
  0 siblings, 0 replies; 22+ messages in thread
From: Caesar Wang @ 2016-03-13  3:57 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Caesar Wang, Heiko Stuebner, David S. Miller, Rob Herring,
	Alexander Kochetkov, netdev, linux-kernel, linux-rockchip,
	keescook, leozwang

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

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

* Re: [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers
  2016-03-11 18:46     ` Sergei Shtylyov
@ 2016-03-13  4:04       ` Caesar Wang
  0 siblings, 0 replies; 22+ messages in thread
From: Caesar Wang @ 2016-03-13  4:04 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Caesar Wang, Mark Rutland, Heiko Stuebner, Alexander Kochetkov,
	Michael Turquette, linux-clk, Russell King, zhengxing,
	linux-rockchip, Caesar Wang, devicetree, Pawel Moll,
	Ian Campbell, Kumar Gala, Rob Herring, linux-arm-kernel,
	Jiri Kosina, netdev, Stephen Boyd, linux-kernel, keescook,
	David S. Miller, leozwang



在 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

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

* Re: [PATCH 2/6] net: arc_emac: add phy-reset-* are optional for device tree
  2016-03-11 10:55 ` [PATCH 2/6] net: arc_emac: add phy-reset-* are optional for device tree Caesar Wang
@ 2016-03-18 19:21   ` Rob Herring
  0 siblings, 0 replies; 22+ messages in thread
From: Rob Herring @ 2016-03-18 19:21 UTC (permalink / raw)
  To: Caesar Wang
  Cc: Heiko Stuebner, David S. Miller, linux-rockchip, keescook,
	leozwang, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree, linux-kernel

On Fri, Mar 11, 2016 at 06:55:27PM +0800, Caesar Wang wrote:
> 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

Append units please (-msec).

> +  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).

These are really all properties of the phy, not the mac, so they would 
be more appropriately be in the phy node even though it is the mac 
driver that wants to control the gpio.

Rob

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

end of thread, other threads:[~2016-03-18 19:21 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-11 10:55 [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Caesar Wang
2016-03-11 10:55 ` [PATCH 1/6] net: arc_emac: make the rockchip emac document more compatible Caesar Wang
2016-03-11 10:55 ` [PATCH 2/6] net: arc_emac: add phy-reset-* are optional for device tree Caesar Wang
2016-03-18 19:21   ` Rob Herring
2016-03-11 10:55 ` [PATCH 3/6] net: arc_emac: support the phy reset for emac driver Caesar Wang
2016-03-11 13:47   ` Sergei Shtylyov
2016-03-11 14:56     ` Heiko Stübner
2016-03-11 14:59       ` Heiko Stübner
2016-03-11 18:35   ` Sergei Shtylyov
2016-03-13  3:57     ` Caesar Wang
2016-03-11 19:22   ` Sergei Shtylyov
2016-03-11 10:55 ` [PATCH 4/6] net: arc: trivial: cleanup the " Caesar Wang
2016-03-11 11:28   ` kbuild test robot
2016-03-11 10:55 ` [PATCH 5/6] clk: rockchip: rk3036: fix and add node id for emac clock Caesar Wang
2016-03-11 11:15   ` Heiko Stübner
2016-03-11 12:01     ` Caesar Wang
2016-03-11 12:28       ` Heiko Stübner
2016-03-11 10:55 ` [PATCH 6/6] ARM: dts: rockchip: add support emac for RK3036 Caesar Wang
2016-03-11 13:46 ` [PATCH 0/6] arc_emac: fixes the emac issues oand cleanup emac drivers Sergei Shtylyov
2016-03-11 14:48   ` Caesar Wang
2016-03-11 18:46     ` Sergei Shtylyov
2016-03-13  4:04       ` Caesar Wang

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).