Re: [PATCH] can: flexcan: add PM support

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 09.05.2012 10:16, Kurt Van Dijck wrote:

> On Tue, May 08, 2012 at 08:34:56PM +0200, Marc Kleine-Budde wrote:
>> On 05/08/2012 07:38 PM, Oliver Hartkopp wrote:
>>> On 08.05.2012 19:14, Eric Bénard wrote:
>>>
>>>> Le Tue, 08 May 2012 17:46:09 +0200,
>>>> Marc Kleine-Budde <mkl@pengutronix.de> a écrit :
>>>>
>>>>> On 05/08/2012 05:30 PM, Eric Bénard wrote:
>>>>>> Hi Marc,
>>>>>>
>>>>>> Le Tue, 08 May 2012 17:18:34 +0200,
>>>>>> Marc Kleine-Budde <mkl@pengutronix.de> a écrit :
>>>>>>
>>>>>>> On 05/08/2012 05:12 PM, Eric Bénard wrote:
>>>>>>>> tested on an i.MX257
>>>>>>>
>>>>>>> What about the transceiver? Does is make sense to switch it off, too?
>>>>>>>
>>>>>> this could make sense on platform which have a transceiver with an
>>>>>> enable input. I can add flexcan_transceiver_switch after/before
>>>>>> flexcan_chip_disable/enable but won't be able to test that feature.
>>>>>
>>>>> I looked at two ethernet drivers: ethernet/intel/ixgbe/ixgbe_main.c and
>>>>> ethernet/intel/ixgbe/ixgbe_main.c and AFAICS they basically do the same
>>>>> as in open/close. (btw: the flexcan driver switches the transceiver
>>>>> on/off on open/close.) I haven't implemented any network driver pm yet,
>>>>> but what is a driver supposed to do in suspend/resume?
>>>>>
>>>> concerning the transceiver : that depends if you need to resume when
>>>> receiving a message on the CAN bus in which case you can't disable the
>>>> transceiver (btw, I have an other patch which enable wakeup when
>>>> receiving a message on the CAN bus, but I need to clean and test it
>>>> again).
>>>
>>>
>>> I wonder if you mix up the enabling/disabling of the CAN transceiver (trx)
>>> with the entire system power management.
>>>
>>> E.g. if you take the TJA1054A as an example, you can switch the trx into a
>>> mode that switches off the power supply (INH-line) in order to wake up with a
>>> received CAN frame, which switches on the power supply and boots the system.
>>>
>>> Even if you don't switch the power supply with the INH pin the trx wakes up
>>> with CAN traffic in this mode.
>>>
>>> To me switching the trx modes only makes sense to implement system power
>>> management states. IMO there's no need to switch them when setting the
>>> interface to up/down state. Or did i miss anything?
> * the transceiver consumes power when on, while the netif if off.
>   That just feels a bit stupid.
> * From outside, the wires looks 'active', while there's just noone listening.
> 
> For these 2 arguments, I would think a regular system switches them off too.looks
> 
>>
>> In the current implementation you can register a callback via
>> platform_data to enable/disable the transceiver. It's up to the board to
>> do something useful with it.
> Like in: The driver must call the callback, and the board can act upon this.
>>
>> I think for now I'll take Eric's patch as it is, until we've implemented
>> something more complicated to support automotive style transceivers :)
> 
> To implement something more complicated:
> http://www.mail-archive.com/socketcan-core@lists.berlios.de/msg00269.html
> 


Wow - indeed i was not remembering your effort for these CAN transceiver class
support.

It looks interesting. Btw. i would name all the transceiver stuff with 'trx'
instead of 'tr' (see http://www.acronymfinder.com/TRX.html) and i think there
are some more modes than

+
+/*
+ * CAN transceiver modes
+ */
+#define CANTR_MODE_OFF 0
+#define CANTR_MODE_ON  1
+

especially for automotive power switching.

Additionally a netlink config interface is preferred these days too.

Btw. do you plan to pick the trx support up sometime?

Regards,
Oliver