Hi Marc,

On Tue, Sep 04, 2012 at 12:57:18, Marc Kleine-Budde wrote:

On 09/04/2012 08:14 AM, AnilKumar, Chimata wrote:

quoted

Marc,

Thanks for the comments,

On Tue, Sep 04, 2012 at 01:31:35, Marc Kleine-Budde wrote:

quoted

On 09/03/2012 01:52 PM, AnilKumar Ch wrote:

quoted

Adds suspend resume support to DCAN driver which enables
DCAN power down mode bit (PDR). Then DCAN will ack the local
power-down mode by setting PDA bit in STATUS register.

Also adds a status flag to know the status of DCAN module,
whether it is opened or not.

Use "ndev->flags & IFF_UP" for that. Have a look at the at91_can driver
[1]. I'm not sure if you need locking here.

Then I can use this to check the status, lock is not
required.

quoted

[1] http://lxr.free-electrons.com/source/drivers/net/can/at91_can.c#L1198

quoted

Signed-off-by: AnilKumar Ch <redacted>
---
 drivers/net/can/c_can/c_can.c          |   78 ++++++++++++++++++++++++++++++++
 drivers/net/can/c_can/c_can.h          |    5 ++
 drivers/net/can/c_can/c_can_platform.c |   70 ++++++++++++++++++++++++++--
 3 files changed, 150 insertions(+), 3 deletions(-)

[snip]

quoted

quoted

quoted

+#ifdef CONFIG_PM
+int c_can_power_down(struct net_device *dev)
+{
+	unsigned long time_out;
+	struct c_can_priv *priv = netdev_priv(dev);
+
+	if (!priv->is_opened)
+		return 0;

Should we add a BUG_ON(id->driver_data != BOSCH_D_CAN)?

These APIs are called from platform driver where device type
device type is verified. I think we have to add BUG_ON() in
platform driver.

The platform driver returns if not on D_CAN and will not call this
function. But this functions are exported, so can be called by someone
else. So if the caller is not D_CAN, it's a bug.

I agree with you, but I have some concern here.
I think we should do "return 0;" instead of BUG_ON(). With
BUG_ON() system will hang and ultimately user lost his/her
contents.

Thanks
AnilKumar