On 12.02.2016 20:10, bryan.whiteh...@microchip.com wrote:
> Lino,
>
> Regarding "a matching smp_rmb() in the irq handler"
> There is a smp_wmb() in the irq handler, since in both cases we are forcing a
> write operation on software_irq_signal.
>
> I suppose using atomic operations on software_irq_signal would also work, but
> this driver was based on
> drivers/net/ethernet/smsc/smsc911x.c
> And if possible I'd prefer to keep logical changes to a minimum.
> Plus this is not a "read modify write" scenario so I think the memory barrier
> is sufficient.
> Do you agree?
>
Hi Bryan,
youre right, smsc911x.c does the same thing and probably its ok. As far
as I have understood smp memory barriers (mainly from reading
memory-barriers.txt), they normally should be paired to ensure that a
"reader" thread actually sees what an "updater" thread writes - paired
in a sense that there is a corresponding smp_rmb() for a smp_wmb().
So in this case I expected the need for a smp_rmb() at least in that
loop in open() which waits for the software_irq_signal flag to toggle.
Something like
while (timeout--) {
smp_rmb();
if (pdata->software_irq_signal)
break;
usleep_range(1000, 10000);
}
But AFAICS calling usleep_range() already implies memory barriers, so I
agree that there is probably no need for an explicit smp_rmb.
Regards,
Lino
