Re: [PATCH v3 0/8] arm64: rockchip: Initial GeekBox enablement

[Giuseppe CAVALLARO]

Hello Tomeu, Andreas,

On 3/9/2016 10:52 AM, Giuseppe CAVALLARO wrote:
> > * today's linux-next: probe failed
> >
> > * today's linux-next + revert of 88f8b1bb41c6 stmmac: Fix 'eth0: No
> > PHY found' regression: probe succeeded but no network at all
> >
> > * today's linux-next + revert of 88f8b1bb41c6 (stmmac: Fix 'eth0: No
> > PHY found' regression) + revert of 0e80bdc9a72d (stmmac: first frame
> > prep at the end of xmit routine): probe succeeded, dhcp succeeds and
> > nfsroot works for a few seconds before timing out
>
> ok, I was looking at this problem now that seems to related
> the "stmmac: first frame prep at the end of xmit routine"
> that, at first glance, is breaking the gmac 3.50 with normal descriptor.

I have no Hw where to test this use case. So, I wonder if may I ask you 
to test some patch.

This first patch adds a missing barrier to the normal routine that inits 
the descriptor. Barrier was needed to well manage the OWN
descriptor and it was not added in case of normal desc case after
the xmit rework.

Then I will check the algo behind the new xmit and in case of problems,
if you agree, we will decide to revert it because it aimed to add an
optimization.

Let me know if you agree.

Regards
Peppe
diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
index e13228f..4392610 100644
--- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
@@ -210,7 +210,7 @@ static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 		tdes1 &= ~TDES1_FIRST_SEGMENT;
 
 	if (likely(csum_flag))
-		tdes1 |= (TX_CIC_FULL) << TDES1_CHECKSUM_INSERTION_SHIFT;
+		tdes1 |= (TX_CIC_FULL << TDES1_CHECKSUM_INSERTION_SHIFT);
 	else
 		tdes1 &= ~(TX_CIC_FULL << TDES1_CHECKSUM_INSERTION_SHIFT);
 
@@ -220,6 +220,13 @@ static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 	if (tx_own)
 		tdes1 |= TDES0_OWN;
 
+	if (is_fs & tx_own)
+		/* When the own bit, for the first frame, has to be set, all
+		 * descriptors for the same frame has to be set before, to
+		 * avoid race condition.
+		 */
+		wmb();
+
 	p->des1 = tdes1;
 }