Às 7:41 PM de 3/9/2017, Thierry Reding escreveu:

On Thu, Mar 02, 2017 at 03:09:11PM +0000, Joao Pinto wrote:

quoted

Hi Thierry,

Às 5:24 PM de 2/23/2017, Thierry Reding escreveu:

quoted

From: Thierry Reding <redacted>

New version of this core encode the FIFO sizes in one of the feature
registers. Use these sizes as default, but still allow device tree to
override them for backwards compatibility.

Signed-off-by: Thierry Reding <redacted>
---
 drivers/net/ethernet/stmicro/stmmac/common.h      | 3 +++
 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c  | 2 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 144fe84e8a53..6ac653845d82 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h

@@ -324,6 +324,9 @@ struct dma_features {
 	unsigned int number_tx_queues;
 	/* Alternate (enhanced) DESC mode */
 	unsigned int enh_desc;
+	/* TX and RX FIFO sizes */
+	unsigned int tx_fifo_size;
+	unsigned int rx_fifo_size;
 };
 
 /* GMAC TX FIFO is 8K, Rx FIFO is 16K */

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 377d1b44d4f2..8d249f3b34c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c

@@ -296,6 +296,8 @@ static void dwmac4_get_hw_feature(void __iomem *ioaddr,
 	hw_cap = readl(ioaddr + GMAC_HW_FEATURE1);
 	dma_cap->av = (hw_cap & GMAC_HW_FEAT_AVSEL) >> 20;
 	dma_cap->tsoen = (hw_cap & GMAC_HW_TSOEN) >> 18;
+	dma_cap->tx_fifo_size = 128 << ((hw_cap >> 6) & 0x1f);
+	dma_cap->rx_fifo_size = 128 << ((hw_cap >> 0) & 0x1f);

I suggest you follow the way that has been done for other parameters:

dma_cap->rx_fifo_size = (hw_cap & GMAC_HW_RXFIFOSIZE) >> 0; where
GMAC_HW_RXFIFOSIZE is GENMASK(4, 0)
dma_cap->tx_fifo_size = (hw_cap & GMAC_HW_TXFIFOSIZE) >> 6; where
GMAC_HW_TXFIFOSIZE is GENMASK(10, 6)

Okay, can do.

quoted

quoted

 	/* MAC HW feature2 */
 	hw_cap = readl(ioaddr + GMAC_HW_FEATURE2);
 	/* TX and RX number of channels */

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d7387919bdb6..291e34f0ca94 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

@@ -1281,6 +1281,9 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
 {
 	int rxfifosz = priv->plat->rx_fifo_size;
 
+	if (rxfifosz == 0)
+		rxfifosz = priv->dma_cap.rx_fifo_size;
+
 	if (priv->plat->force_thresh_dma_mode)
 		priv->hw->dma->dma_mode(priv->ioaddr, tc, tc, rxfifosz);
 	else if (priv->plat->force_sf_dma_mode || priv->plat->tx_coe) {

In my current patch for adding support for MTL in stmmac, I also had this
approach (sizes comming from feature register and platform data) so nice to see
it here, because I will be able to reutilize it in my future versions.

There is only a slight twist that we have to pay attention to it. The RX and TX
queue size configuration needs to be one the following values:

FIFO_256 = 0x0,
FIFO_512 = 0x1,
FIFO_1k = 0x3,
FIFO_2k = 0x7,
FIFO_4k = 0xf,
FIFO_8k = 0x1f,
FIFO_16k = 0x3f,
FIFO_32k = 0x7f

These are the values you get from the features register, but are these the
values that users will configure in "plat->rx_fifo_size"? From a quick grep you
can see that users use real units:

arch/arm/boot/dts/socfpga.dtsi:			rx-fifo-depth = <4096>;
arch/arm/boot/dts/socfpga.dtsi:			rx-fifo-depth = <4096>;
arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
arch/arm/boot/dts/socfpga_arria10.dtsi:			rx-fifo-depth = <16384>;
arch/arm/boot/dts/lpc18xx.dtsi:			rx-fifo-depth = <256>;
arch/nios2/boot/dts/3c120_devboard.dts:				rx-fifo-depth = <8192>;
arch/nios2/boot/dts/10m50_devboard.dts:			rx-fifo-depth = <8192>;

So in order to have it configurable from platform and features register you need
to convert the features values to "real" units and then in the end when you
write to the DMA Op Mode you need to convert it back to the required values.

That's what the "128 << " part in dwmac4_get_hw_feature() does. As far
as I can tell, that's equivalent to dwmac4_get_real_fifo_sz() function
from your patch, but it's more compact.

Ok, great, but I suggest you put the operation clearer then (a convert macro
maybe), because this way will be more obvious.

quoted

You can check my patch here that already has this done:
https://urldefense.proofpoint.com/v2/url?u=http-3A__patchwork.ozlabs.org_patch_728566_&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=vfSNvldRkxJ8_e-lvP03KElsSdtNkACrt5RZt47TVZE&s=KByW4fH0QMLR16vmW6ekKqBC-cvqe0nBeOWpXB6uG_4&e= 

I see that there's a bit of overlap between that patch and this series.
How do you want to handle this? Do you want me to rebase on top of your
patch, or would you prefer this series to get merged first and rework
your patch on top of it?

I suggest you go first, and then I will adapt my development since I still
submiting the multiple queues MAC related operations.

Thanks.

Thanks,
Thierry