Re: [PATCH v5 6/8] net: can: c_can: Disable pins when CAN interface is down

2014-11-27 Thread Linus Walleij
On Fri, Nov 14, 2014 at 2:43 PM, Roger Quadros rog...@ti.com wrote:
 On 11/13/2014 06:03 PM, Marc Kleine-Budde wrote:
 On 11/13/2014 04:23 PM, Roger Quadros wrote:

 I just stumbled over pinctrl_pm_select_sleep_state(), is it possible to
 integrate this into runtime pm?

 http://lxr.free-electrons.com/source/drivers/pinctrl/core.c#L1282

 I think those functions are there for the same reason but not sure why aren't
 they used in runtime pm core.

 Linus W. any hints?

It is not used from PM core because there are cases where
you may want to put pins to sleep for completely PM-core
unrelated things.

Things like turning off a serial port from userspace,
for example. That should put the pins to sleep.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 6/8] net: can: c_can: Disable pins when CAN interface is down

2014-11-14 Thread Roger Quadros
On 11/13/2014 06:03 PM, Marc Kleine-Budde wrote:
 On 11/13/2014 04:23 PM, Roger Quadros wrote:
 DRA7 CAN IP suffers from a problem which causes it to be prevented
 from fully turning OFF (i.e. stuck in transition) if the module was
 disabled while there was traffic on the CAN_RX line.

 To work around this issue we select the SLEEP pin state by default
 on probe and use the DEFAULT pin state on CAN up and back to the
 SLEEP pin state on CAN down.

 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  drivers/net/can/c_can/c_can.c | 37 +
  drivers/net/can/c_can/c_can.h |  1 +
  2 files changed, 38 insertions(+)

 diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
 index 8e78bb4..c80cb3d 100644
 --- a/drivers/net/can/c_can/c_can.c
 +++ b/drivers/net/can/c_can/c_can.c
 @@ -35,6 +35,7 @@
  #include linux/list.h
  #include linux/io.h
  #include linux/pm_runtime.h
 +#include linux/pinctrl/consumer.h
  
  #include linux/can.h
  #include linux/can/dev.h
 @@ -587,6 +588,21 @@ static int c_can_chip_config(struct net_device *dev)
  return c_can_set_bittiming(dev);
  }
  
 +/*
 + * Selects the pinctrl state specified in the name.
 + */
 +static void c_can_pinctrl_select_state(struct c_can_priv *priv,
 +  const char *name)
 +{
 +if (!IS_ERR(priv-pinctrl)) {
 +struct pinctrl_state *s;
 +
 +s = pinctrl_lookup_state(priv-pinctrl, name);
 +if (!IS_ERR(s))
 +pinctrl_select_state(priv-pinctrl, s);
 +}
 +}
 +
  static int c_can_start(struct net_device *dev)
  {
  struct c_can_priv *priv = netdev_priv(dev);
 @@ -603,6 +619,8 @@ static int c_can_start(struct net_device *dev)
  
  priv-can.state = CAN_STATE_ERROR_ACTIVE;
  
 +/* activate pins */
 +c_can_pinctrl_select_state(priv, PINCTRL_STATE_DEFAULT);
  return 0;
  }
  
 @@ -611,6 +629,9 @@ static void c_can_stop(struct net_device *dev)
  struct c_can_priv *priv = netdev_priv(dev);
  
  c_can_irq_control(priv, false);
 +
 +/* deactivate pins */
 +c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP);
  priv-can.state = CAN_STATE_STOPPED;
  }
  
 @@ -1244,6 +1265,22 @@ int register_c_can_dev(struct net_device *dev)
  struct c_can_priv *priv = netdev_priv(dev);
  int err;
  
 +priv-pinctrl = devm_pinctrl_get(dev-dev.parent);
 
 What's dev-dev.parent?
 Ah, this is set by SET_NETDEV_DEV(dev, pdev-dev); Good work!
 
 I thought we had to set priv-pinctrl in the platform.c.
 
 +if (!IS_ERR(priv-pinctrl)) {
 +struct pinctrl_state *s;
 +
 +/* Deactivate pins to prevent DRA7 DCAN IP from being
 + * stuck in transition when module is disabled.
 + * Pins are activated in c_can_start() and deactivated
 + * in c_can_stop()
 + */
 +s = pinctrl_lookup_state(priv-pinctrl, PINCTRL_STATE_SLEEP);
 +if (!IS_ERR(s))
 +pinctrl_select_state(priv-pinctrl, s);
 +} else {
 +netdev_dbg(dev, failed to get pinctrl\n);
 +}
 +
 The above can be replace by this?
 
   c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP)

Yes, indeed.

 
 Then we don't have the worrying looking error message if there isn't any
 pinctrl, e.g. for the PCI driver with pinctrl enabled in the Kernel.
 
 I just stumbled over pinctrl_pm_select_sleep_state(), is it possible to
 integrate this into runtime pm?
 
 http://lxr.free-electrons.com/source/drivers/pinctrl/core.c#L1282

I think those functions are there for the same reason but not sure why aren't 
they used in runtime pm core.

Linus W. any hints?

cheers,
-roger

 
  c_can_pm_runtime_enable(priv);
  
  dev-flags |= IFF_ECHO; /* we support local echo */
 diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
 index c6715ca..3cedf48 100644
 --- a/drivers/net/can/c_can/c_can.h
 +++ b/drivers/net/can/c_can/c_can.h
 @@ -210,6 +210,7 @@ struct c_can_priv {
  u32 comm_rcv_high;
  u32 rxmasked;
  u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
 +struct pinctrl *pinctrl;
  };
  
  struct net_device *alloc_c_can_dev(void);

 
 Marc
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 6/8] net: can: c_can: Disable pins when CAN interface is down

2014-11-13 Thread Marc Kleine-Budde
On 11/12/2014 03:16 PM, Roger Quadros wrote:
 DRA7 CAN IP suffers from a problem which causes it to be prevented
 from fully turning OFF (i.e. stuck in transition) if the module was
 disabled while there was traffic on the CAN_RX line.
 
 To work around this issue we select the SLEEP pin state by default
 on probe and use the DEFAULT pin state on CAN up and back to the
 SLEEP pin state on CAN down.
 
 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  drivers/net/can/c_can/c_can.c  | 21 +
  drivers/net/can/c_can/c_can.h  |  1 +
  drivers/net/can/c_can/c_can_platform.c | 20 
  3 files changed, 42 insertions(+)
 
 diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
 index 8e78bb4..a950eea 100644
 --- a/drivers/net/can/c_can/c_can.c
 +++ b/drivers/net/can/c_can/c_can.c
 @@ -35,6 +35,7 @@
  #include linux/list.h
  #include linux/io.h
  #include linux/pm_runtime.h
 +#include linux/pinctrl/consumer.h
  
  #include linux/can.h
  #include linux/can/dev.h
 @@ -587,6 +588,21 @@ static int c_can_chip_config(struct net_device *dev)
   return c_can_set_bittiming(dev);
  }
  
 +/*
 + * Selects the pinctrl state specified in the name.
 + */
 +static void c_can_pinctrl_select_state(struct c_can_priv *priv,
 +   const char *name)
 +{
 + if (!IS_ERR(priv-pinctrl)) {

What happens if priv-pinctrl is NULL? This is probably the case with
the c_can_pci driver.

 + struct pinctrl_state *s;
 +
 + s = pinctrl_lookup_state(priv-pinctrl, name);
 + if (!IS_ERR(s))
 + pinctrl_select_state(priv-pinctrl, s);
 + }
 +}
 +
  static int c_can_start(struct net_device *dev)
  {
   struct c_can_priv *priv = netdev_priv(dev);
 @@ -603,6 +619,8 @@ static int c_can_start(struct net_device *dev)
  
   priv-can.state = CAN_STATE_ERROR_ACTIVE;
  
 + /* activate pins */
 + c_can_pinctrl_select_state(priv, PINCTRL_STATE_DEFAULT);
   return 0;
  }
  
 @@ -611,6 +629,9 @@ static void c_can_stop(struct net_device *dev)
   struct c_can_priv *priv = netdev_priv(dev);
  
   c_can_irq_control(priv, false);
 +
 + /* deactivate pins */
 + c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP);
   priv-can.state = CAN_STATE_STOPPED;
  }
  
 diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
 index c6715ca..3cedf48 100644
 --- a/drivers/net/can/c_can/c_can.h
 +++ b/drivers/net/can/c_can/c_can.h
 @@ -210,6 +210,7 @@ struct c_can_priv {
   u32 comm_rcv_high;
   u32 rxmasked;
   u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
 + struct pinctrl *pinctrl;
  };
  
  struct net_device *alloc_c_can_dev(void);
 diff --git a/drivers/net/can/c_can/c_can_platform.c 
 b/drivers/net/can/c_can/c_can_platform.c
 index b838c6b..71b9063 100644
 --- a/drivers/net/can/c_can/c_can_platform.c
 +++ b/drivers/net/can/c_can/c_can_platform.c
 @@ -34,6 +34,7 @@
  #include linux/of_device.h
  #include linux/mfd/syscon.h
  #include linux/regmap.h
 +#include linux/pinctrl/consumer.h
  
  #include linux/can/dev.h
  
 @@ -230,6 +231,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
   struct clk *clk;
   const struct c_can_driver_data *drvdata;
   struct device_node *np = pdev-dev.of_node;
 + struct pinctrl *pinctrl;
  
   match = of_match_device(c_can_of_table, pdev-dev);
   if (match) {
 @@ -241,6 +243,23 @@ static int c_can_plat_probe(struct platform_device *pdev)
   return -ENODEV;
   }
  
 + pinctrl = devm_pinctrl_get(pdev-dev);
 + if (!IS_ERR(pinctrl)) {
 + struct pinctrl_state *s;
 +
 + /* Deactivate pins to prevent DRA7 DCAN IP from being
 +  * stuck in transition when module is disabled.
 +  * Pins are activated in c_can_start() and deactivated
 +  * in c_can_stop()
 +  */
 + s = pinctrl_lookup_state(pinctrl, PINCTRL_STATE_SLEEP);
 + if (!IS_ERR(s))
 + pinctrl_select_state(pinctrl, s);
 + } else {
 + dev_warn(pdev-dev,
 +  failed to get pinctrl\n);
 + }

Can you move the initial setting into c_can.c, register_c_can_dev()
should be a good candidate.

 +
   /* get the appropriate clk */
   clk = devm_clk_get(pdev-dev, NULL);
   if (IS_ERR(clk)) {
 @@ -270,6 +289,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
   }
  
   priv = netdev_priv(dev);
 + priv-pinctrl = pinctrl;
  
   switch (drvdata-id) {
   case BOSCH_C_CAN:
 

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v5 6/8] net: can: c_can: Disable pins when CAN interface is down

2014-11-13 Thread Roger Quadros
On 11/13/2014 02:56 PM, Marc Kleine-Budde wrote:
 On 11/12/2014 03:16 PM, Roger Quadros wrote:
 DRA7 CAN IP suffers from a problem which causes it to be prevented
 from fully turning OFF (i.e. stuck in transition) if the module was
 disabled while there was traffic on the CAN_RX line.

 To work around this issue we select the SLEEP pin state by default
 on probe and use the DEFAULT pin state on CAN up and back to the
 SLEEP pin state on CAN down.

 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  drivers/net/can/c_can/c_can.c  | 21 +
  drivers/net/can/c_can/c_can.h  |  1 +
  drivers/net/can/c_can/c_can_platform.c | 20 
  3 files changed, 42 insertions(+)

 diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
 index 8e78bb4..a950eea 100644
 --- a/drivers/net/can/c_can/c_can.c
 +++ b/drivers/net/can/c_can/c_can.c
 @@ -35,6 +35,7 @@
  #include linux/list.h
  #include linux/io.h
  #include linux/pm_runtime.h
 +#include linux/pinctrl/consumer.h
  
  #include linux/can.h
  #include linux/can/dev.h
 @@ -587,6 +588,21 @@ static int c_can_chip_config(struct net_device *dev)
  return c_can_set_bittiming(dev);
  }
  
 +/*
 + * Selects the pinctrl state specified in the name.
 + */
 +static void c_can_pinctrl_select_state(struct c_can_priv *priv,
 +  const char *name)
 +{
 +if (!IS_ERR(priv-pinctrl)) {
 
 What happens if priv-pinctrl is NULL? This is probably the case with
 the c_can_pci driver.

There will be a NULL pointer dereference error. :(

 
 +struct pinctrl_state *s;
 +
 +s = pinctrl_lookup_state(priv-pinctrl, name);
 +if (!IS_ERR(s))
 +pinctrl_select_state(priv-pinctrl, s);
 +}
 +}
 +
  static int c_can_start(struct net_device *dev)
  {
  struct c_can_priv *priv = netdev_priv(dev);
 @@ -603,6 +619,8 @@ static int c_can_start(struct net_device *dev)
  
  priv-can.state = CAN_STATE_ERROR_ACTIVE;
  
 +/* activate pins */
 +c_can_pinctrl_select_state(priv, PINCTRL_STATE_DEFAULT);
  return 0;
  }
  
 @@ -611,6 +629,9 @@ static void c_can_stop(struct net_device *dev)
  struct c_can_priv *priv = netdev_priv(dev);
  
  c_can_irq_control(priv, false);
 +
 +/* deactivate pins */
 +c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP);
  priv-can.state = CAN_STATE_STOPPED;
  }
  
 diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
 index c6715ca..3cedf48 100644
 --- a/drivers/net/can/c_can/c_can.h
 +++ b/drivers/net/can/c_can/c_can.h
 @@ -210,6 +210,7 @@ struct c_can_priv {
  u32 comm_rcv_high;
  u32 rxmasked;
  u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
 +struct pinctrl *pinctrl;
  };
  
  struct net_device *alloc_c_can_dev(void);
 diff --git a/drivers/net/can/c_can/c_can_platform.c 
 b/drivers/net/can/c_can/c_can_platform.c
 index b838c6b..71b9063 100644
 --- a/drivers/net/can/c_can/c_can_platform.c
 +++ b/drivers/net/can/c_can/c_can_platform.c
 @@ -34,6 +34,7 @@
  #include linux/of_device.h
  #include linux/mfd/syscon.h
  #include linux/regmap.h
 +#include linux/pinctrl/consumer.h
  
  #include linux/can/dev.h
  
 @@ -230,6 +231,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
  struct clk *clk;
  const struct c_can_driver_data *drvdata;
  struct device_node *np = pdev-dev.of_node;
 +struct pinctrl *pinctrl;
  
  match = of_match_device(c_can_of_table, pdev-dev);
  if (match) {
 @@ -241,6 +243,23 @@ static int c_can_plat_probe(struct platform_device 
 *pdev)
  return -ENODEV;
  }
  
 +pinctrl = devm_pinctrl_get(pdev-dev);
 +if (!IS_ERR(pinctrl)) {
 +struct pinctrl_state *s;
 +
 +/* Deactivate pins to prevent DRA7 DCAN IP from being
 + * stuck in transition when module is disabled.
 + * Pins are activated in c_can_start() and deactivated
 + * in c_can_stop()
 + */
 +s = pinctrl_lookup_state(pinctrl, PINCTRL_STATE_SLEEP);
 +if (!IS_ERR(s))
 +pinctrl_select_state(pinctrl, s);
 +} else {
 +dev_warn(pdev-dev,
 + failed to get pinctrl\n);
 +}
 
 Can you move the initial setting into c_can.c, register_c_can_dev()
 should be a good candidate.

OK. This should also address the NULL pointer issue you pointed out earlier 
with c_can_pci.

cheers,
-roger


 
 +
  /* get the appropriate clk */
  clk = devm_clk_get(pdev-dev, NULL);
  if (IS_ERR(clk)) {
 @@ -270,6 +289,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
  }
  
  priv = netdev_priv(dev);
 +priv-pinctrl = pinctrl;
  
  switch (drvdata-id) {
  case BOSCH_C_CAN:

 
 Marc
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v5 6/8] net: can: c_can: Disable pins when CAN interface is down

2014-11-13 Thread Roger Quadros
DRA7 CAN IP suffers from a problem which causes it to be prevented
from fully turning OFF (i.e. stuck in transition) if the module was
disabled while there was traffic on the CAN_RX line.

To work around this issue we select the SLEEP pin state by default
on probe and use the DEFAULT pin state on CAN up and back to the
SLEEP pin state on CAN down.

Signed-off-by: Roger Quadros rog...@ti.com
---
 drivers/net/can/c_can/c_can.c | 37 +
 drivers/net/can/c_can/c_can.h |  1 +
 2 files changed, 38 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 8e78bb4..c80cb3d 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -35,6 +35,7 @@
 #include linux/list.h
 #include linux/io.h
 #include linux/pm_runtime.h
+#include linux/pinctrl/consumer.h
 
 #include linux/can.h
 #include linux/can/dev.h
@@ -587,6 +588,21 @@ static int c_can_chip_config(struct net_device *dev)
return c_can_set_bittiming(dev);
 }
 
+/*
+ * Selects the pinctrl state specified in the name.
+ */
+static void c_can_pinctrl_select_state(struct c_can_priv *priv,
+ const char *name)
+{
+   if (!IS_ERR(priv-pinctrl)) {
+   struct pinctrl_state *s;
+
+   s = pinctrl_lookup_state(priv-pinctrl, name);
+   if (!IS_ERR(s))
+   pinctrl_select_state(priv-pinctrl, s);
+   }
+}
+
 static int c_can_start(struct net_device *dev)
 {
struct c_can_priv *priv = netdev_priv(dev);
@@ -603,6 +619,8 @@ static int c_can_start(struct net_device *dev)
 
priv-can.state = CAN_STATE_ERROR_ACTIVE;
 
+   /* activate pins */
+   c_can_pinctrl_select_state(priv, PINCTRL_STATE_DEFAULT);
return 0;
 }
 
@@ -611,6 +629,9 @@ static void c_can_stop(struct net_device *dev)
struct c_can_priv *priv = netdev_priv(dev);
 
c_can_irq_control(priv, false);
+
+   /* deactivate pins */
+   c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP);
priv-can.state = CAN_STATE_STOPPED;
 }
 
@@ -1244,6 +1265,22 @@ int register_c_can_dev(struct net_device *dev)
struct c_can_priv *priv = netdev_priv(dev);
int err;
 
+   priv-pinctrl = devm_pinctrl_get(dev-dev.parent);
+   if (!IS_ERR(priv-pinctrl)) {
+   struct pinctrl_state *s;
+
+   /* Deactivate pins to prevent DRA7 DCAN IP from being
+* stuck in transition when module is disabled.
+* Pins are activated in c_can_start() and deactivated
+* in c_can_stop()
+*/
+   s = pinctrl_lookup_state(priv-pinctrl, PINCTRL_STATE_SLEEP);
+   if (!IS_ERR(s))
+   pinctrl_select_state(priv-pinctrl, s);
+   } else {
+   netdev_dbg(dev, failed to get pinctrl\n);
+   }
+
c_can_pm_runtime_enable(priv);
 
dev-flags |= IFF_ECHO; /* we support local echo */
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index c6715ca..3cedf48 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -210,6 +210,7 @@ struct c_can_priv {
u32 comm_rcv_high;
u32 rxmasked;
u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
+   struct pinctrl *pinctrl;
 };
 
 struct net_device *alloc_c_can_dev(void);
-- 
1.8.3.2


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 6/8] net: can: c_can: Disable pins when CAN interface is down

2014-11-13 Thread Marc Kleine-Budde
On 11/13/2014 04:23 PM, Roger Quadros wrote:
 DRA7 CAN IP suffers from a problem which causes it to be prevented
 from fully turning OFF (i.e. stuck in transition) if the module was
 disabled while there was traffic on the CAN_RX line.
 
 To work around this issue we select the SLEEP pin state by default
 on probe and use the DEFAULT pin state on CAN up and back to the
 SLEEP pin state on CAN down.
 
 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  drivers/net/can/c_can/c_can.c | 37 +
  drivers/net/can/c_can/c_can.h |  1 +
  2 files changed, 38 insertions(+)
 
 diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
 index 8e78bb4..c80cb3d 100644
 --- a/drivers/net/can/c_can/c_can.c
 +++ b/drivers/net/can/c_can/c_can.c
 @@ -35,6 +35,7 @@
  #include linux/list.h
  #include linux/io.h
  #include linux/pm_runtime.h
 +#include linux/pinctrl/consumer.h
  
  #include linux/can.h
  #include linux/can/dev.h
 @@ -587,6 +588,21 @@ static int c_can_chip_config(struct net_device *dev)
   return c_can_set_bittiming(dev);
  }
  
 +/*
 + * Selects the pinctrl state specified in the name.
 + */
 +static void c_can_pinctrl_select_state(struct c_can_priv *priv,
 +   const char *name)
 +{
 + if (!IS_ERR(priv-pinctrl)) {
 + struct pinctrl_state *s;
 +
 + s = pinctrl_lookup_state(priv-pinctrl, name);
 + if (!IS_ERR(s))
 + pinctrl_select_state(priv-pinctrl, s);
 + }
 +}
 +
  static int c_can_start(struct net_device *dev)
  {
   struct c_can_priv *priv = netdev_priv(dev);
 @@ -603,6 +619,8 @@ static int c_can_start(struct net_device *dev)
  
   priv-can.state = CAN_STATE_ERROR_ACTIVE;
  
 + /* activate pins */
 + c_can_pinctrl_select_state(priv, PINCTRL_STATE_DEFAULT);
   return 0;
  }
  
 @@ -611,6 +629,9 @@ static void c_can_stop(struct net_device *dev)
   struct c_can_priv *priv = netdev_priv(dev);
  
   c_can_irq_control(priv, false);
 +
 + /* deactivate pins */
 + c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP);
   priv-can.state = CAN_STATE_STOPPED;
  }
  
 @@ -1244,6 +1265,22 @@ int register_c_can_dev(struct net_device *dev)
   struct c_can_priv *priv = netdev_priv(dev);
   int err;
  
 + priv-pinctrl = devm_pinctrl_get(dev-dev.parent);

What's dev-dev.parent?
Ah, this is set by SET_NETDEV_DEV(dev, pdev-dev); Good work!

I thought we had to set priv-pinctrl in the platform.c.

 + if (!IS_ERR(priv-pinctrl)) {
 + struct pinctrl_state *s;
 +
 + /* Deactivate pins to prevent DRA7 DCAN IP from being
 +  * stuck in transition when module is disabled.
 +  * Pins are activated in c_can_start() and deactivated
 +  * in c_can_stop()
 +  */
 + s = pinctrl_lookup_state(priv-pinctrl, PINCTRL_STATE_SLEEP);
 + if (!IS_ERR(s))
 + pinctrl_select_state(priv-pinctrl, s);
 + } else {
 + netdev_dbg(dev, failed to get pinctrl\n);
 + }
 +
The above can be replace by this?

c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP)

Then we don't have the worrying looking error message if there isn't any
pinctrl, e.g. for the PCI driver with pinctrl enabled in the Kernel.

I just stumbled over pinctrl_pm_select_sleep_state(), is it possible to
integrate this into runtime pm?

http://lxr.free-electrons.com/source/drivers/pinctrl/core.c#L1282

   c_can_pm_runtime_enable(priv);
  
   dev-flags |= IFF_ECHO; /* we support local echo */
 diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
 index c6715ca..3cedf48 100644
 --- a/drivers/net/can/c_can/c_can.h
 +++ b/drivers/net/can/c_can/c_can.h
 @@ -210,6 +210,7 @@ struct c_can_priv {
   u32 comm_rcv_high;
   u32 rxmasked;
   u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
 + struct pinctrl *pinctrl;
  };
  
  struct net_device *alloc_c_can_dev(void);
 

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature


[PATCH v5 6/8] net: can: c_can: Disable pins when CAN interface is down

2014-11-12 Thread Roger Quadros
DRA7 CAN IP suffers from a problem which causes it to be prevented
from fully turning OFF (i.e. stuck in transition) if the module was
disabled while there was traffic on the CAN_RX line.

To work around this issue we select the SLEEP pin state by default
on probe and use the DEFAULT pin state on CAN up and back to the
SLEEP pin state on CAN down.

Signed-off-by: Roger Quadros rog...@ti.com
---
 drivers/net/can/c_can/c_can.c  | 21 +
 drivers/net/can/c_can/c_can.h  |  1 +
 drivers/net/can/c_can/c_can_platform.c | 20 
 3 files changed, 42 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 8e78bb4..a950eea 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -35,6 +35,7 @@
 #include linux/list.h
 #include linux/io.h
 #include linux/pm_runtime.h
+#include linux/pinctrl/consumer.h
 
 #include linux/can.h
 #include linux/can/dev.h
@@ -587,6 +588,21 @@ static int c_can_chip_config(struct net_device *dev)
return c_can_set_bittiming(dev);
 }
 
+/*
+ * Selects the pinctrl state specified in the name.
+ */
+static void c_can_pinctrl_select_state(struct c_can_priv *priv,
+ const char *name)
+{
+   if (!IS_ERR(priv-pinctrl)) {
+   struct pinctrl_state *s;
+
+   s = pinctrl_lookup_state(priv-pinctrl, name);
+   if (!IS_ERR(s))
+   pinctrl_select_state(priv-pinctrl, s);
+   }
+}
+
 static int c_can_start(struct net_device *dev)
 {
struct c_can_priv *priv = netdev_priv(dev);
@@ -603,6 +619,8 @@ static int c_can_start(struct net_device *dev)
 
priv-can.state = CAN_STATE_ERROR_ACTIVE;
 
+   /* activate pins */
+   c_can_pinctrl_select_state(priv, PINCTRL_STATE_DEFAULT);
return 0;
 }
 
@@ -611,6 +629,9 @@ static void c_can_stop(struct net_device *dev)
struct c_can_priv *priv = netdev_priv(dev);
 
c_can_irq_control(priv, false);
+
+   /* deactivate pins */
+   c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP);
priv-can.state = CAN_STATE_STOPPED;
 }
 
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index c6715ca..3cedf48 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -210,6 +210,7 @@ struct c_can_priv {
u32 comm_rcv_high;
u32 rxmasked;
u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
+   struct pinctrl *pinctrl;
 };
 
 struct net_device *alloc_c_can_dev(void);
diff --git a/drivers/net/can/c_can/c_can_platform.c 
b/drivers/net/can/c_can/c_can_platform.c
index b838c6b..71b9063 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -34,6 +34,7 @@
 #include linux/of_device.h
 #include linux/mfd/syscon.h
 #include linux/regmap.h
+#include linux/pinctrl/consumer.h
 
 #include linux/can/dev.h
 
@@ -230,6 +231,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
struct clk *clk;
const struct c_can_driver_data *drvdata;
struct device_node *np = pdev-dev.of_node;
+   struct pinctrl *pinctrl;
 
match = of_match_device(c_can_of_table, pdev-dev);
if (match) {
@@ -241,6 +243,23 @@ static int c_can_plat_probe(struct platform_device *pdev)
return -ENODEV;
}
 
+   pinctrl = devm_pinctrl_get(pdev-dev);
+   if (!IS_ERR(pinctrl)) {
+   struct pinctrl_state *s;
+
+   /* Deactivate pins to prevent DRA7 DCAN IP from being
+* stuck in transition when module is disabled.
+* Pins are activated in c_can_start() and deactivated
+* in c_can_stop()
+*/
+   s = pinctrl_lookup_state(pinctrl, PINCTRL_STATE_SLEEP);
+   if (!IS_ERR(s))
+   pinctrl_select_state(pinctrl, s);
+   } else {
+   dev_warn(pdev-dev,
+failed to get pinctrl\n);
+   }
+
/* get the appropriate clk */
clk = devm_clk_get(pdev-dev, NULL);
if (IS_ERR(clk)) {
@@ -270,6 +289,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
}
 
priv = netdev_priv(dev);
+   priv-pinctrl = pinctrl;
 
switch (drvdata-id) {
case BOSCH_C_CAN:
-- 
1.8.3.2


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html