Re: [PATCH 3.2 047/110] stmmac: troubleshoot unexpected bits in des0 & des1

[Alexey Brodkin]

Hi Ben,

On Mon, 2015-08-10 at 12:12 +-0200, Ben Hutchings wrote:
+AD4- 3.2.71-rc1 review patch.  If anyone has any objections, please let me know.
+AD4- 
+AD4- ------------------
+AD4- 
+AD4- From: Alexey Brodkin +ADw-Alexey.Brodkin+AEA-synopsys.com+AD4-
+AD4- 
+AD4- commit f1590670ce069eefeb93916391a67643e6ad1630 upstream.
+AD4- 
+AD4- Current implementation of descriptor init procedure only takes
+AD4- care about setting/clearing ownership flag in +ACI-des0+ACI-/+ACI-des1+ACI-
+AD4- fields while it is perfectly possible to get unexpected bits
+AD4- set because of the following factors:
+AD4- 
+AD4-  +AFs-1+AF0- On driver probe underlying memory allocated with
+AD4-      dma+AF8-alloc+AF8-coherent() might not be zeroed and so
+AD4-      it will be filled with garbage.
+AD4- 
+AD4-  +AFs-2+AF0- During driver operation some bits could be set by SD/MMC
+AD4-      controller (for example error flags etc).
+AD4- 
+AD4- And unexpected and/or randomly set flags in +ACI-des0+ACI-/+ACI-des1+ACI-
+AD4- fields may lead to unpredictable behavior of GMAC DMA block.
+AD4- 
+AD4- This change addresses both items above with:
+AD4- 
+AD4-  +AFs-1+AF0- Use of dma+AF8-zalloc+AF8-coherent() instead of simple
+AD4-      dma+AF8-alloc+AF8-coherent() to make sure allocated memory is
+AD4-      zeroed. That shouldn't affect performance because
+AD4-      this allocation only happens once on driver probe.
+AD4- 
+AD4-  +AFs-2+AF0- Do explicit zeroing of both +ACI-des0+ACI- and +ACI-des1+ACI- fields
+AD4-      of all buffer descriptors during initialization of
+AD4-      DMA transfer.
+AD4- 
+AD4- And while at it fixed identation of dma+AF8-free+AF8-coherent()
+AD4- counterpart as well.
+AD4- 
+AD4- Signed-off-by: Alexey Brodkin +ADw-abrodkin+AEA-synopsys.com+AD4-
+AD4- Cc: Giuseppe Cavallaro +ADw-peppe.cavallaro+AEA-st.com+AD4-
+AD4- Cc: arc-linux-dev+AEA-synopsys.com
+AD4- Cc: linux-kernel+AEA-vger.kernel.org
+AD4- Cc: David Miller +ADw-davem+AEA-davemloft.net+AD4-
+AD4- Signed-off-by: David S. Miller +ADw-davem+AEA-davemloft.net+AD4-
+AD4- +AFs-bwh: Backported to 3.2:
+AD4-  - Adjust context, indentation
+AD4-  - Normal and extended descriptors are allocated in the same place here+AF0-
+AD4- Signed-off-by: Ben Hutchings +ADw-ben+AEA-decadent.org.uk+AD4-

This patch looks good to me.

Moreover that was exactly what I initially done on top of 3.18, see
https://github.com/foss-for-synopsys-dwc-arc-processors/linux/commit/f2105b2ba9b3444568b32caca1ab253b88058fc2

So feel free to add Acked-by and/or Tested-by: Alexey Brodkin +ADw-abrodkin+AEA-synopsys.com+AD4-

-Alexey--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/