Thread (6 messages) 6 messages, 3 authors, 2016-03-03

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help