Hello Marc, replying to v2, there is a v3 already.. On 10/11/19 12:54 PM, Marc Kleine-Budde wrote: > On 10/10/19 8:31 PM, Jeroen Hofstee wrote: >>> drivers/net/can/ti_hecc.c | 34 +++++++++++++++++++++++++++++----- >>> 1 file changed, 29 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c >>> index 6ea29126c60b..87815a5b9170 100644 >>> --- a/drivers/net/can/ti_hecc.c >>> +++ b/drivers/net/can/ti_hecc.c [...] >> I really doubt if there is something to justify to treat the >> HECC_RX_LAST_MBOX specially. It is a comparison versus a bit >> check of which the mask is already available..... > You miss the point that we will read the HECC_CANRML register for > _every_ mailbox and reading a register from a peripheral is much more > expensive then first checking if we're working on the last RX mailbox at > all. Well the last part, I am aware it always reads the register, but not that it can be significantly expensive compared to the whole stack in between. I can't easily figure out how expensive it can be, since e.g. it depends on the actual L3 state on the am3517, latencies of ti_hecc. Anyway, for sure you are right it is not going to beat the mbxno check, since it is likely in a register. >> I like v1 more actually, just because it is simpler / less code... > v3 looks quite nice now. Indeed.. >> I have my doubts about the error counter, see the other mail. >> I don't know which source has the ultimate definition, I just >> googled for them. > I've moved the error counter handling to rx-offload now. > > The kernel doc always says: > > "See the network driver for the exact meaning of this value." > Good, my intend was to get it reported at all, I don't really care how, I just picked the one which seems most logical to me. Regards, Jeroen
