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@xxxxxx> >>>>> --- >>>>> 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 |
Attachment:
signature.asc
Description: OpenPGP digital signature
