Re: [PATCH] can: rcar_canfd: Add Renesas R-Car CAN FD driver

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1993 bytes --]

On 03/02/2016 11:08 AM, Ramesh Shanmugasundaram wrote:
>>>> 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.

Ok. Which instruction starts the transmit? Please add a comment to the
code. You stop the tx_queue after this, so your code is racy, as the
interrupt might happen in between.

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

Ok - I haven't looked at the code in depth yet. Now it's clear, that
just the tx interrupt is shared.

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

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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]