On 03/02/2016 09:41 AM, Ramesh Shanmugasundaram wrote:
>> Is the channel loopback mode configurable per channel? If so, please
>> remove the module parameter and make use of CAN_CTRLMODE_LOOPBACK to
>> configure it.
> 
> The loopback setting is not truly a per channel attribute. It
> requires touching Rx rules which can be done only when the
> controller's global state is reset (during probe).
> CAN_CTRLMODE_LOOPBACK config option is possible only through
> .ndo_open of that channel but the global controller state needs to be
> operational by this time. As it is a global attribute & for the above
> reason, I choose the module option.

I see, ok then.

>>> CAN FD mode supports both Classical CAN & CAN FD frame formats. The
>>> controller supports ISO 11898-1:2015 CAN FD format only.
>>>
>>> This controller supports two channels and the driver can enable either
>>> or both of the channels.
>>>
>>> Driver uses Rx FIFOs (one per channel) for reception & Common FIFOs
>>> (one per channel) for transmission. Rx filter rules are configured to
>>> the minimum (one per channel) and it accepts Standard, Extended, Data
>>> & Remote Frame combinations.
>>
>> I see no locking for the tx-path.
>  
> I am not sure why locking is needed in tx-path? 

If the tx-path is shared between both channels you need locking as the
networking subsystem may send one frame to each controller at the same time.

> However, looking at it again, I should move the incrementing of head
> after the "sts" handing to be apt. What do you think?

With one producer (one SW instance) and one consumer (the HW) you can
write lockless code (if the HW allows it), but with two producers it's
not possible.

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   |