On 11/30/2020 1:18 AM, Loic Poulain wrote:
On Sat, 28 Nov 2020 at 07:03, Manivannan Sadhasivam
<manivannan.sadhasivam@xxxxxxxxxx> wrote:
On Thu, Nov 26, 2020 at 04:06:41PM +0100, Loic Poulain wrote:
The ring element data, though being part of coherent memory, still need
to be performed before updating the ring context to point to this new
element. That can be guaranteed with a memory barrier (dma_wmb).
Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx>
---
v2: fix comment style
drivers/bus/mhi/core/main.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 67188ea..ea39df0 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -111,7 +111,14 @@ void mhi_ring_chan_db(struct mhi_controller *mhi_cntrl,
dma_addr_t db;
db = ring->iommu_base + (ring->wp - ring->base);
+
+ /*
+ * Writes to the new ring element must be visible to the hardware
+ * before letting h/w know there is new element to fetch.
+ */
+ dma_wmb();
*ring->ctxt_wp = db;
As Jeff pointed out, the barrier should come after updating ctxt_wp.
Actually, device can poll for the write pointer (e.g. in burst mode),
so we need to be sure the element data are written before writing this
write pointer (since it can be accessed at any time on device side,
not only after doorbell).
I think that what jeff pointed is that we also need to ensure that
write pointer is also updated before we ring the doorbell (doorbell
mode), but this is implicitly done by the MMIO writing (using
writeX()) of the doorbell register.(cf
https://www.kernel.org/doc/Documentation/memory-barriers.txt).
If we are using the version of the writeX API that includes a barrier,
then that would be sufficient.
--
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
