Marc, On Wed, Sep 12, 2012 at 18:32:53, Marc Kleine-Budde wrote: > 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()? > Either would be fine printing a warning message or WARN_ON() Thanks AnilKumar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
