RE: [PATCH] can: rcar_canfd: Add Renesas R-Car CAN FD driver
From: Ramesh Shanmugasundaram <hidden>
Date: 2016-03-02 10:08:16
Also in:
linux-can, linux-renesas-soc, netdev
Hi Marc,
On 03/02/2016 09:41 AM, Ramesh Shanmugasundaram wrote:quoted
quoted
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.quoted
quoted
quoted
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.
Yes. Tx completion interrupt is shared by both channels but the Tx FIFO, echo skbs, head, tail are maintained on a per channel basis. Yes, net subsystem can send one frame to each channel at the same time and the tx completion interrupt handler will traverse each channel & update per channel context accordingly.
quoted
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.
For a channel represented as netdev, there is still one producer (one SW instance) isn't it? net acquires lock before calling xmit. The consumer is tx completion interrupt handler which updates per channel attributes only. Sorry if I am annoying. I have tested this with two channels transmitting & receiving at the same time and it works fine. If I am missing lock, I would like to understand it thoroughly before making the change. Thanks for the help, Ramesh.