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

AnilKumar, Chimata anilkumar at ti.com
Wed Sep 12 08:48:39 EDT 2012


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 at 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.

Thanks
AnilKumar



More information about the linux-arm-kernel mailing list