[PATCH 4/4] can: c_can: Add d_can suspend resume support

[mkl@pengutronix.de (Marc Kleine-Budde)]

On 09/12/2012 02:48 PM, AnilKumar, Chimata wrote:
> 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:
>>> Marc,
>>>
>>> Thanks for the comments,
>>>
>>> On Tue, Sep 04, 2012 at 01:31:35, Marc Kleine-Budde wrote:
>>>> On 09/03/2012 01:52 PM, AnilKumar Ch wrote:
>>>>> 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.
>>>
>>>> [1] http://lxr.free-electrons.com/source/drivers/net/can/at91_can.c#L1198
>>>>
>>>>> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
>>>>> ---
>>>>>  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]
> 
>>>>> +#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.

Good point, better safe then sorry.
But this should not happen. What about WARN_ON()?

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120912/684e24fd/attachment.sig>