Re: [PATCH 2/3] soc: amazon: al-pos: Introduce Amazon's Annapurna Labs POS driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 9/9/2019 6:16 PM, Arnd Bergmann wrote:
On Mon, Sep 9, 2019 at 4:11 PM Shenhar, Talel <talel@xxxxxxxxxx> wrote:
On 9/9/2019 4:41 PM, Arnd Bergmann wrote:

In current implementation of v1, I am not doing any read barrier, Hence,
using the non-relaxed will add unneeded memory barrier.

I have no strong objection moving to the non-relaxed version and have an
unneeded memory barrier, as this path is not "hot" one.
Ok, then please add it.
ok, shall be part of v2

Beside of avoiding the unneeded memory barrier, I would be happy to keep
common behavior for our drivers:

e.g.

https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-al-fic.c#L49


So what do you think we should go with? relaxed or non-relaxed?
The al_fic_set_trigger() function is clearly a slow-path and should use the
non-relaxed functions. In case of al_fic_irq_handler(), the extra barrier
might introduce a measurable overhead, but at the same time I'm
not sure if that one is correct without the barrier:

If you have an MSI-type interrupt for notifying a device driver of
a DMA completion, there might not be any other barrier between
the arrival of the MSI message and the CPU accessing the data.
Depending on how strict the hardware implements MSI and how
the IRQ is chained, this could lead to data corruption.

If the interrupt is only used for level or edge triggered interrupts,
this is ok since you already need another register read in
the driver before it can safely access a DMA buffer.

In either case, if you can prove that it's safe to use the relaxed
version here and you think that it may help, it would be good to
add a comment explaining the reasoning.
Decided to go with the non-relaxed version as this is not hot path and likely be more clear to the common reader to have non relaxed version.

        Arnd



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux