[PATCH] net: dwc_eth_qos: Pad descriptors to cacheline size

[Patrick DELAUNAY]

Hi Stephen and Marek

> From: Stephen Warren <swarren at wwwdotorg.org>
> Sent: mercredi 29 avril 2020 23:51
> To: Marek Vasut <marex at denx.de>
> Cc: u-boot at lists.denx.de; Joe Hershberger <joe.hershberger at ni.com>; Patrice
> CHOTARD <patrice.chotard at st.com>; Patrick DELAUNAY
> <patrick.delaunay at st.com>; Ramon Fried <rfried.dev at gmail.com>; Stephen
> Warren <swarren at nvidia.com>; Tom Warren <twarren at nvidia.com>
> Subject: Re: [PATCH] net: dwc_eth_qos: Pad descriptors to cacheline size
> Importance: High
> 
> On 4/29/20 3:41 PM, Marek Vasut wrote:
> > On 4/29/20 11:25 PM, Stephen Warren wrote:
> >> On 4/29/20 1:56 PM, Marek Vasut wrote:
> >>> On 4/29/20 9:51 PM, Stephen Warren wrote:
> >>>> On 4/29/20 1:14 PM, Marek Vasut wrote:


[...]

> >>>
> >>> Could you at least test it on the tegra ?
> >>
> >> This patch (applied on top of current u-boot/master) does cause
> >> network failures on Jetson TX2 (Tegra186), which obviously uses this driver.
> >
> > That's what I was afraid of.
> >
> >> The docs for our version of the IP block do indicate that the DSL
> >> shift is supported, so I think the patch should work. I wonder if
> >> it's because of the bus width of our EQoS IP block?
> >
> > The STM32MP1 docs say that the shift is in 64bit words, maybe that
> > actually translate into bus words rather than 8 bytes indeed ?
> 
> I assume the STM32 doc writer only wrote the specific case that applies to their
> HW, which presumably has a 64-bit bus. Our docs are IIRC unmodified from the
> generic IP docs that Synopsis supplied, so mention all the cases and leave the
> reader to figure out which option applies!
> 

[...]

> 
> >> Our docs indicate that DSL is a
> >> multiple of the bus width, so describes a skip of word/dword/lword
> >> count based on 32/64/128 bus width. Perhaps the bus width needs to be
> >> parametrized when calculating the value? Our docs don't seem to
> >> indicate which bus width our HW uses:-( Aha, but changing the "/2" to
> >> "/4" in the code in this patch which calculates DSL value does yield
> >> a working system, so I guess we have a 128-bit bus width. And indeed
> >> this matches a comment I wrote in the driver:
> >>
> >>         /*
> >>          * Burst length must be < 1/2 FIFO size.
> >>          * FIFO size in tqs is encoded as (n / 256) - 1.
> >>          * Each burst is n * 8 (PBLX8) * 16 (AXI width) == 128 bytes.
> >>
> >> 16 byte AXI width == 128 bit width.
> >
> > So how do we parametrize that, based on compatible string I guess ?
> > We already have params for the tegra186 and stm32mp1 variants of the
> > IP, so adding one more should be OK.
> 
> Yes, adding another field to struct eqos_config would make sense.
> 
> > Also, thanks for looking into it.

I confirm that ETH is configurated with AXI 64bit on STM32MP15x.

But we have the same description of DSL in ETH documentation before final Datasheet generation
(DSL in Word, Dword, or Lword, depending on the 32-bit, 64-bit, or 128-bit bus).

NB: 32/64/128-bit also imply alignment of address used for DMA
       (ch0_txdesc_tail_pointer/ch0_rxdesc_tail_pointer for example).

The width of this field depends on the configuration:
■ 31:2 for 32-bit configuration
■ 31:3 for 64-bit configuration
■ 31:4 for 128-bit configuration

Regards 

Patrick