Re: [PATCHv2 net-next] net: fec: Ensure clocks are enabled while using mdio bus

2015-06-21 Thread Andrew Lunn 
On Sat, Jun 20, 2015 at 06:01:33PM -0700, Florian Fainelli wrote:
 Le 06/20/15 09:15, Andrew Lunn a écrit :
  When a switch is attached to the mdio bus, the mdio bus can be used
  while the interface is not open. If the IPG clock are not enabled,
  MDIO reads/writes will simply time out. So enable the clock before
  starting a transaction, and disable it afterwards. The CCF performs
  reference counting so the clock will only be disabled if there are no
  other users.
  
  Signed-off-by: Andrew Lunn and...@lunn.ch
  ---
  v2:
Only enable/disable the IPG clock.
  
  drivers/net/ethernet/freescale/fec_main.c | 21 +++--
   1 file changed, 19 insertions(+), 2 deletions(-)
  
  diff --git a/drivers/net/ethernet/freescale/fec_main.c 
  b/drivers/net/ethernet/freescale/fec_main.c
  index bf4cf3fbb5f2..2b8a043a573c 100644
  --- a/drivers/net/ethernet/freescale/fec_main.c
  +++ b/drivers/net/ethernet/freescale/fec_main.c
  @@ -65,6 +65,7 @@
   
   static void set_multicast_list(struct net_device *ndev);
   static void fec_enet_itr_coal_init(struct net_device *ndev);
  +static int fec_enet_clk_enable(struct net_device *ndev, bool enable);
 
 You do not seem to be using this, unrelated change?

Hi Florian

Left over from v1, where i did call this function to enable/disable
the clocks. It can be removed now that i use the common clock
framework directly to enable only a single clock.

  Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in

[PATCHv2 net-next] net: fec: Ensure clocks are enabled while using mdio bus

2015-06-20 Thread Andrew Lunn 
When a switch is attached to the mdio bus, the mdio bus can be used
while the interface is not open. If the IPG clock are not enabled,
MDIO reads/writes will simply time out. So enable the clock before
starting a transaction, and disable it afterwards. The CCF performs
reference counting so the clock will only be disabled if there are no
other users.

Signed-off-by: Andrew Lunn and...@lunn.ch
---
v2:
  Only enable/disable the IPG clock.

drivers/net/ethernet/freescale/fec_main.c | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index bf4cf3fbb5f2..2b8a043a573c 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -65,6 +65,7 @@
 
 static void set_multicast_list(struct net_device *ndev);
 static void fec_enet_itr_coal_init(struct net_device *ndev);
+static int fec_enet_clk_enable(struct net_device *ndev, bool enable);
 
 #define DRIVER_NAMEfec
 
@@ -1764,6 +1765,11 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int 
mii_id, int regnum)
 {
struct fec_enet_private *fep = bus-priv;
unsigned long time_left;
+   int ret;
+
+   ret = clk_prepare_enable(fep-clk_ipg);
+   if (ret)
+   return 0x;
 
fep-mii_timeout = 0;
init_completion(fep-mdio_done);
@@ -1779,11 +1785,14 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int 
mii_id, int regnum)
if (time_left == 0) {
fep-mii_timeout = 1;
netdev_err(fep-netdev, MDIO read timeout\n);
+   clk_disable_unprepare(fep-clk_ipg);
return -ETIMEDOUT;
}
 
-   /* return value */
-   return FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
+   ret = FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
+   clk_disable_unprepare(fep-clk_ipg);
+
+   return ret;
 }
 
 static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
@@ -1791,10 +1800,15 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int 
mii_id, int regnum,
 {
struct fec_enet_private *fep = bus-priv;
unsigned long time_left;
+   int ret;
 
fep-mii_timeout = 0;
init_completion(fep-mdio_done);
 
+   ret = clk_prepare_enable(fep-clk_ipg);
+   if (ret)
+   return ret;
+
/* start a write op */
writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE |
FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) |
@@ -1807,9 +1821,12 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int 
mii_id, int regnum,
if (time_left == 0) {
fep-mii_timeout = 1;
netdev_err(fep-netdev, MDIO write timeout\n);
+   clk_disable_unprepare(fep-clk_ipg);
return -ETIMEDOUT;
}
 
+   clk_disable_unprepare(fep-clk_ipg);
+
return 0;
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe netdev in

Re: [PATCHv2 net-next] net: fec: Ensure clocks are enabled while using mdio bus

2015-06-20 Thread Fabio Estevam 
On Sat, Jun 20, 2015 at 1:15 PM, Andrew Lunn and...@lunn.ch wrote:

 @@ -1764,6 +1765,11 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int 
 mii_id, int regnum)
  {
 struct fec_enet_private *fep = bus-priv;
 unsigned long time_left;
 +   int ret;
 +
 +   ret = clk_prepare_enable(fep-clk_ipg);
 +   if (ret)
 +   return 0x;

Why don`t you return ret instead?
--
To unsubscribe from this list: send the line unsubscribe netdev in

Re: [PATCHv2 net-next] net: fec: Ensure clocks are enabled while using mdio bus

2015-06-20 Thread Andrew Lunn 
On Sat, Jun 20, 2015 at 02:47:20PM -0300, Fabio Estevam wrote:
 On Sat, Jun 20, 2015 at 1:15 PM, Andrew Lunn and...@lunn.ch wrote:
 
  @@ -1764,6 +1765,11 @@ static int fec_enet_mdio_read(struct mii_bus *bus, 
  int mii_id, int regnum)
   {
  struct fec_enet_private *fep = bus-priv;
  unsigned long time_left;
  +   int ret;
  +
  +   ret = clk_prepare_enable(fep-clk_ipg);
  +   if (ret)
  +   return 0x;
 
 Why don`t you return ret instead?

ret would also work.

v3 to follow soon.

   Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in

Re: [PATCHv2 net-next] net: fec: Ensure clocks are enabled while using mdio bus

2015-06-20 Thread Florian Fainelli 
Le 06/20/15 09:15, Andrew Lunn a écrit :
 When a switch is attached to the mdio bus, the mdio bus can be used
 while the interface is not open. If the IPG clock are not enabled,
 MDIO reads/writes will simply time out. So enable the clock before
 starting a transaction, and disable it afterwards. The CCF performs
 reference counting so the clock will only be disabled if there are no
 other users.
 
 Signed-off-by: Andrew Lunn and...@lunn.ch
 ---
 v2:
   Only enable/disable the IPG clock.
 
 drivers/net/ethernet/freescale/fec_main.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/net/ethernet/freescale/fec_main.c 
 b/drivers/net/ethernet/freescale/fec_main.c
 index bf4cf3fbb5f2..2b8a043a573c 100644
 --- a/drivers/net/ethernet/freescale/fec_main.c
 +++ b/drivers/net/ethernet/freescale/fec_main.c
 @@ -65,6 +65,7 @@
  
  static void set_multicast_list(struct net_device *ndev);
  static void fec_enet_itr_coal_init(struct net_device *ndev);
 +static int fec_enet_clk_enable(struct net_device *ndev, bool enable);

You do not seem to be using this, unrelated change?
-- 
Florian
--
To unsubscribe from this list: send the line unsubscribe netdev in