[v2] stmmac: troubleshoot unexpected bits in des0 & des1

Current implementation of descriptor init procedure only takes
care about setting/clearing ownership flag in "des0"/"des1"
fields while it is perfectly possible to get unexpected bits
set because of the following factors:

 [1] On driver probe underlying memory allocated with
     dma_alloc_coherent() might not be zeroed and so
     it will be filled with garbage.

 [2] During driver operation some bits could be set by SD/MMC
     controller (for example error flags etc).

And unexpected and/or randomly set flags in "des0"/"des1"
fields may lead to unpredictable behavior of GMAC DMA block.

This change addresses both items above with:

 [1] Use of dma_zalloc_coherent() instead of simple
     dma_alloc_coherent() to make sure allocated memory is
     zeroed. That shouldn't affect performance because
     this allocation only happens once on driver probe.

 [2] Do explicit zeroing of both "des0" and "des1" fields
     of all buffer descriptors during initialization of
     DMA transfer.

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: arc-linux-dev@synopsys.com
Cc: linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org
Cc: David Miller <davem@davemloft.net>
---
 drivers/net/ethernet/stmicro/stmmac/descs.h       | 2 ++
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c    | 3 ++-
 drivers/net/ethernet/stmicro/stmmac/norm_desc.c   | 3 ++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++----
 4 files changed, 10 insertions(+), 6 deletions(-)

Hi David,

On Wed, 2015-06-24 at 01:44 -0700, David Miller wrote:
> From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
> Date: Wed, 24 Jun 2015 11:07:26 +0300
> 
> >  
> > -           priv->dma_tx = dma_alloc_coherent(priv->device, 
> > txsize *
> > +           priv->dma_tx = dma_zalloc_coherent(priv->device, 
> > txsize *
> >                                               sizeof(struct 
> > dma_desc),
> >                                               &priv
> > ->dma_tx_phy,
> >                                               GFP_KERNEL);
> 
> When you change the column at which the openning parenthesis of the
> function call occurs, you must indent the subsequent lines of the
> function call such that the all are adjusted to properly start
> at the very next column after that openning parenthesis.'
> 
> Please fix this up for all of the dma_zalloc_coherent()
> transformations.

I thought about that and actually made this change initially but later
reverted it because I noticed dma_free_coherent() had parameters not
-aligned/idented properly.

So do you think if I may fix identation of dma_free_coherent() while at
it as well or I need to send a separate patch that takes care about
dma_free_coherent() solely?

-Alexey--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
Date: Wed, 24 Jun 2015 11:07:26 +0300

>  
> -		priv->dma_tx = dma_alloc_coherent(priv->device, txsize *
> +		priv->dma_tx = dma_zalloc_coherent(priv->device, txsize *
>  						  sizeof(struct dma_desc),
>  						  &priv->dma_tx_phy,
>  						  GFP_KERNEL);

When you change the column at which the openning parenthesis of the
function call occurs, you must indent the subsequent lines of the
function call such that the all are adjusted to properly start
at the very next column after that openning parenthesis.'

Please fix this up for all of the dma_zalloc_coherent()
transformations.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html