Re: [BULK]Re: peak_pci: TX Frame Loss

[Stephane Grosjean <s.grosjean@peak-system.com>]

Le 08/12/2015 13:24, Andri Yngvason a écrit :
> Hi Stephane,
>
> On Tue, Dec 08, 2015 at 12:42:58PM +0100, Stephane Grosjean wrote:
>> Hi Andri,
>>
>> So, you mean that, for example, the below sequence (extract from _xmit()):
>>
>>                  priv->write_reg(priv, SJA1000_FI, fi);
>>                  priv->write_reg(priv, SJA1000_ID1, (id & 0x000007f8) >> 3);
>>                  priv->write_reg(priv, SJA1000_ID2, (id & 0x00000007) << 5);
>>                  ...
>>                  (1)
>>                  ...
>>                  sja1000_write_cmdreg(priv, CMD_TR);
>>
>> can't be interrupted at all (especially at (1)), by any IRQ which ISR could
>> write (again) some new ID?
>> I mean, shouldn't the SJA1000 and the sequence order of the cmds be
>> considered as shared resource?
>>
> AFAIK only _xmit() writes to these registers and I think the _xmit() function is
> only ever executed on one core at a time. Executing two of them at the same time
> wouldn't make much sense. It would just cause out-of-order transmission.
>
> Also, the interrupt routine does not call _xmit() directly. It just calls
> netif_wake_queue() which AFAIK schedules the _xmit() routine to be called later.
>
> I'm just saying that we shouldn't place locks where they are not needed because
> it really just obfuscates the real issue. It's the equivalent of leaving the
> mess on the ground and just walking around it instead of cleaning it up. Someone
> else will step into it.
Ok. If you're right with what you have written above, I do also agree 
with you: there is no need to lock the _xmit() function nor the ISR.
> Besides, ftrace does not show _xmit() being called concurrently for the frames
> that are lost.
>
> By the way, I'm not sure that I understood you correctly. Did placing the spin
> locks prevent the issue? I'm not sure this "guess what" rhetoric works well on a
> mailing list. ;)

Yes, it did.

Stéphane
> Thanks,
> Andri
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt
Handelsregister Darmstadt HRB 9183 
Geschaeftsfuehrung: Alexander Gach, Uwe Wilhelm
--