2016-12-07 14:20:48

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v3 0/6] net: stmmac: make DMA programmable burst length more configurable

Make DMA programmable burst length more configurable in the stmmac driver.

This is done by adding support for independent pbl for tx/rx through DT.
More fine grained tuning of pbl is possible thanks to a DT property saying
that we should NOT multiply pbl values by x8/x4 in hardware.

All new DT properties are optional, and created in a way that it will not
affect any existing DT configurations.

Changes since V1:
Created cover-letter.
Rebased patch set against next-20161205, since conflicting patches to
stmmac_platform.c has been merged since V1.

Changes since V2:
Moved default value initialization of pbl to stmmac_platform.c
and added a check for pbl != 0 in stmmac_main.c,
to catch a possble pbl == 0 from pci glue.


Niklas Cassel (6):
net: stmmac: return error if no DMA configuration is found
net: stmmac: simplify the common DMA init API
net: stmmac: stmmac_platform: fix parsing of DT binding
net: stmmac: dwmac1000: fix define DMA_BUS_MODE_RPBL_MASK
net: stmmac: add support for independent DMA pbl for tx/rx
net: smmac: allow configuring lower pbl values

Documentation/devicetree/bindings/net/stmmac.txt | 8 +++++-
Documentation/networking/stmmac.txt | 24 +++++++++++-----
drivers/net/ethernet/stmicro/stmmac/common.h | 4 +--
drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 26 ++++++++++--------
drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c | 7 +++--
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 25 ++++++++++-------
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++------
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 ++
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 32 ++++++++++++----------
include/linux/stmmac.h | 3 ++
11 files changed, 88 insertions(+), 59 deletions(-)

--
2.1.4


2016-12-07 14:21:28

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found

From: Niklas Cassel <[email protected]>

All drivers except pci glue layer calls stmmac_probe_config_dt.
stmmac_probe_config_dt does a kzalloc dma_cfg.

pci glue layer does kzalloc dma_cfg explicitly, so all current
drivers does a kzalloc dma_cfg.

Return an error if no DMA configuration is found, that way
we can assume that the DMA configuration always exists.

Signed-off-by: Niklas Cassel <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 982c95213da4..14366800e5e6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1578,16 +1578,12 @@ static void stmmac_check_ether_addr(struct stmmac_priv *priv)
*/
static int stmmac_init_dma_engine(struct stmmac_priv *priv)
{
- int pbl = DEFAULT_DMA_PBL, fixed_burst = 0, aal = 0;
- int mixed_burst = 0;
int atds = 0;
int ret = 0;

- if (priv->plat->dma_cfg) {
- pbl = priv->plat->dma_cfg->pbl;
- fixed_burst = priv->plat->dma_cfg->fixed_burst;
- mixed_burst = priv->plat->dma_cfg->mixed_burst;
- aal = priv->plat->dma_cfg->aal;
+ if (!priv->plat->dma_cfg) {
+ dev_err(priv->device, "DMA configuration not found\n");
+ return -EINVAL;
}

if (priv->extend_desc && (priv->mode == STMMAC_RING_MODE))
@@ -1599,8 +1595,12 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
return ret;
}

- priv->hw->dma->init(priv->ioaddr, pbl, fixed_burst, mixed_burst,
- aal, priv->dma_tx_phy, priv->dma_rx_phy, atds);
+ priv->hw->dma->init(priv->ioaddr,
+ priv->plat->dma_cfg->pbl,
+ priv->plat->dma_cfg->fixed_burst,
+ priv->plat->dma_cfg->mixed_burst,
+ priv->plat->dma_cfg->aal,
+ priv->dma_tx_phy, priv->dma_rx_phy, atds);

if (priv->synopsys_id >= DWMAC_CORE_4_00) {
priv->rx_tail_addr = priv->dma_rx_phy +
--
2.1.4

2016-12-07 14:22:14

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v3 2/6] net: stmmac: simplify the common DMA init API

From: Niklas Cassel <[email protected]>

Use struct stmmac_dma_cfg *dma_cfg as an argument rather
than using all the struct members as individual arguments.

Signed-off-by: Niklas Cassel <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 4 ++--
drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 13 +++++++------
drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c | 7 ++++---
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 14 ++++++++------
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +-----
5 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 3ced2e1703c1..b13a144f72ad 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -412,8 +412,8 @@ extern const struct stmmac_desc_ops ndesc_ops;
struct stmmac_dma_ops {
/* DMA core initialization */
int (*reset)(void __iomem *ioaddr);
- void (*init)(void __iomem *ioaddr, int pbl, int fb, int mb,
- int aal, u32 dma_tx, u32 dma_rx, int atds);
+ void (*init)(void __iomem *ioaddr, struct stmmac_dma_cfg *dma_cfg,
+ u32 dma_tx, u32 dma_rx, int atds);
/* Configure the AXI Bus Mode Register */
void (*axi)(void __iomem *ioaddr, struct stmmac_axi *axi);
/* Dump DMA registers */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
index f35385266fbf..318ae9f10104 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
@@ -84,8 +84,9 @@ static void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
writel(value, ioaddr + DMA_AXI_BUS_MODE);
}

-static void dwmac1000_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
- int aal, u32 dma_tx, u32 dma_rx, int atds)
+static void dwmac1000_dma_init(void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg,
+ u32 dma_tx, u32 dma_rx, int atds)
{
u32 value = readl(ioaddr + DMA_BUS_MODE);

@@ -101,20 +102,20 @@ static void dwmac1000_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
*/
value |= DMA_BUS_MODE_MAXPBL;
value &= ~DMA_BUS_MODE_PBL_MASK;
- value |= (pbl << DMA_BUS_MODE_PBL_SHIFT);
+ value |= (dma_cfg->pbl << DMA_BUS_MODE_PBL_SHIFT);

/* Set the Fixed burst mode */
- if (fb)
+ if (dma_cfg->fixed_burst)
value |= DMA_BUS_MODE_FB;

/* Mixed Burst has no effect when fb is set */
- if (mb)
+ if (dma_cfg->mixed_burst)
value |= DMA_BUS_MODE_MB;

if (atds)
value |= DMA_BUS_MODE_ATDS;

- if (aal)
+ if (dma_cfg->aal)
value |= DMA_BUS_MODE_AAL;

writel(value, ioaddr + DMA_BUS_MODE);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c
index 61f54c99a7de..e5664da382f3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c
@@ -32,11 +32,12 @@
#include "dwmac100.h"
#include "dwmac_dma.h"

-static void dwmac100_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
- int aal, u32 dma_tx, u32 dma_rx, int atds)
+static void dwmac100_dma_init(void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg,
+ u32 dma_tx, u32 dma_rx, int atds)
{
/* Enable Application Access by writing to DMA CSR0 */
- writel(DMA_BUS_MODE_DEFAULT | (pbl << DMA_BUS_MODE_PBL_SHIFT),
+ writel(DMA_BUS_MODE_DEFAULT | (dma_cfg->pbl << DMA_BUS_MODE_PBL_SHIFT),
ioaddr + DMA_BUS_MODE);

/* Mask interrupts by writing to CSR7 */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index e81b6e565c29..7d82a3464097 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -99,27 +99,29 @@ static void dwmac4_dma_init_channel(void __iomem *ioaddr, int pbl,
writel(dma_rx_phy, ioaddr + DMA_CHAN_RX_BASE_ADDR(channel));
}

-static void dwmac4_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
- int aal, u32 dma_tx, u32 dma_rx, int atds)
+static void dwmac4_dma_init(void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg,
+ u32 dma_tx, u32 dma_rx, int atds)
{
u32 value = readl(ioaddr + DMA_SYS_BUS_MODE);
int i;

/* Set the Fixed burst mode */
- if (fb)
+ if (dma_cfg->fixed_burst)
value |= DMA_SYS_BUS_FB;

/* Mixed Burst has no effect when fb is set */
- if (mb)
+ if (dma_cfg->mixed_burst)
value |= DMA_SYS_BUS_MB;

- if (aal)
+ if (dma_cfg->aal)
value |= DMA_SYS_BUS_AAL;

writel(value, ioaddr + DMA_SYS_BUS_MODE);

for (i = 0; i < DMA_CHANNEL_NB_MAX; i++)
- dwmac4_dma_init_channel(ioaddr, pbl, dma_tx, dma_rx, i);
+ dwmac4_dma_init_channel(ioaddr, dma_cfg->pbl,
+ dma_tx, dma_rx, i);
}

static void _dwmac4_dump_dma_regs(void __iomem *ioaddr, u32 channel)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 14366800e5e6..b1e42ddf0370 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1595,11 +1595,7 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
return ret;
}

- priv->hw->dma->init(priv->ioaddr,
- priv->plat->dma_cfg->pbl,
- priv->plat->dma_cfg->fixed_burst,
- priv->plat->dma_cfg->mixed_burst,
- priv->plat->dma_cfg->aal,
+ priv->hw->dma->init(priv->ioaddr, priv->plat->dma_cfg,
priv->dma_tx_phy, priv->dma_rx_phy, atds);

if (priv->synopsys_id >= DWMAC_CORE_4_00) {
--
2.1.4

2016-12-07 14:23:01

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v3 3/6] net: stmmac: stmmac_platform: fix parsing of DT binding

From: Niklas Cassel <[email protected]>

commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with DT")
changed the parsing of the DT binding.

Before 64c3b252e9fc, snps,fixed-burst and snps,mixed-burst were parsed
regardless if the property snps,pbl existed or not.
After the commit, fixed burst and mixed burst are only parsed if
snps,pbl exists. Now when snps,aal has been added, it too is only
parsed if snps,pbl exists.

Since the DT binding does not specify that fixed burst, mixed burst
or aal depend on snps,pbl being specified, undo changes introduced
by 64c3b252e9fc.

The issue commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with
DT") tries to address is solved in another way:
The databook specifies that all values other than
1, 2, 4, 8, 16, or 32 results in undefined behavior,
so snps,pbl = <0> is invalid.

If pbl is 0 after parsing, set pbl to DEFAULT_DMA_PBL.
This handles the case where the property is omitted, and also handles
the case where the property is specified without any data.

Signed-off-by: Niklas Cassel <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +--
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 29 +++++++++++-----------
2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b1e42ddf0370..b5188122bc15 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1581,8 +1581,8 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
int atds = 0;
int ret = 0;

- if (!priv->plat->dma_cfg) {
- dev_err(priv->device, "DMA configuration not found\n");
+ if (!priv->plat->dma_cfg || !priv->plat->dma_cfg->pbl) {
+ dev_err(priv->device, "Invalid DMA configuration\n");
return -EINVAL;
}

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index d3b6f92f350a..81800f23a9c4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -304,21 +304,22 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
plat->force_sf_dma_mode = 1;
}

- if (of_find_property(np, "snps,pbl", NULL)) {
- dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
- GFP_KERNEL);
- if (!dma_cfg) {
- stmmac_remove_config_dt(pdev, plat);
- return ERR_PTR(-ENOMEM);
- }
- plat->dma_cfg = dma_cfg;
- of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
- dma_cfg->aal = of_property_read_bool(np, "snps,aal");
- dma_cfg->fixed_burst =
- of_property_read_bool(np, "snps,fixed-burst");
- dma_cfg->mixed_burst =
- of_property_read_bool(np, "snps,mixed-burst");
+ dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
+ GFP_KERNEL);
+ if (!dma_cfg) {
+ stmmac_remove_config_dt(pdev, plat);
+ return ERR_PTR(-ENOMEM);
}
+ plat->dma_cfg = dma_cfg;
+
+ of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
+ if (!dma_cfg->pbl)
+ dma_cfg->pbl = DEFAULT_DMA_PBL;
+
+ dma_cfg->aal = of_property_read_bool(np, "snps,aal");
+ dma_cfg->fixed_burst = of_property_read_bool(np, "snps,fixed-burst");
+ dma_cfg->mixed_burst = of_property_read_bool(np, "snps,mixed-burst");
+
plat->force_thresh_dma_mode = of_property_read_bool(np, "snps,force_thresh_dma_mode");
if (plat->force_thresh_dma_mode) {
plat->force_sf_dma_mode = 0;
--
2.1.4

2016-12-07 14:23:46

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v3 4/6] net: stmmac: dwmac1000: fix define DMA_BUS_MODE_RPBL_MASK

From: Niklas Cassel <[email protected]>

DMA_BUS_MODE_RPBL_MASK is really 6 bits,
just like DMA_BUS_MODE_PBL_MASK.

Signed-off-by: Niklas Cassel <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
index ff3e5ab39bd0..52b9407a8a39 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
@@ -225,7 +225,7 @@ enum rx_tx_priority_ratio {

#define DMA_BUS_MODE_FB 0x00010000 /* Fixed burst */
#define DMA_BUS_MODE_MB 0x04000000 /* Mixed burst */
-#define DMA_BUS_MODE_RPBL_MASK 0x003e0000 /* Rx-Programmable Burst Len */
+#define DMA_BUS_MODE_RPBL_MASK 0x007e0000 /* Rx-Programmable Burst Len */
#define DMA_BUS_MODE_RPBL_SHIFT 17
#define DMA_BUS_MODE_USP 0x00800000
#define DMA_BUS_MODE_MAXPBL 0x01000000
--
2.1.4

2016-12-07 14:24:39

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v3 5/6] net: stmmac: add support for independent DMA pbl for tx/rx

From: Niklas Cassel <[email protected]>

GMAC and newer supports independent programmable burst lengths for
DMA tx/rx. Add new optional devicetree properties representing this.

To be backwards compatible, snps,pbl will still be valid, but
snps,txpbl/snps,rxpbl will override the value in snps,pbl if set.

If the IP is synthesized to use the AXI interface, there is a register
and a matching DT property inside the optional stmmac-axi-config DT node
for controlling burst lengths, named snps,blen.
However, using this register, it is not possible to control tx and rx
independently. Also, this register is not available if the IP was
synthesized with, e.g., the AHB interface.

Signed-off-by: Niklas Cassel <[email protected]>
---
Documentation/devicetree/bindings/net/stmmac.txt | 6 +++++-
Documentation/networking/stmmac.txt | 19 +++++++++++++------
drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 12 ++++++------
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++-----
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 ++
include/linux/stmmac.h | 2 ++
6 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index b95ff998ba73..8080038ff1b2 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -34,7 +34,11 @@ Optional properties:
platforms.
- tx-fifo-depth: See ethernet.txt file in the same directory
- rx-fifo-depth: See ethernet.txt file in the same directory
-- snps,pbl Programmable Burst Length
+- snps,pbl Programmable Burst Length (tx and rx)
+- snps,txpbl Tx Programmable Burst Length. Only for GMAC and newer.
+ If set, DMA tx will use this value rather than snps,pbl.
+- snps,rxpbl Rx Programmable Burst Length. Only for GMAC and newer.
+ If set, DMA rx will use this value rather than snps,pbl.
- snps,aal Address-Aligned Beats
- snps,fixed-burst Program the DMA to use the fixed burst mode
- snps,mixed-burst Program the DMA to use the mixed burst mode
diff --git a/Documentation/networking/stmmac.txt b/Documentation/networking/stmmac.txt
index 014f4f756cb7..6add57374f70 100644
--- a/Documentation/networking/stmmac.txt
+++ b/Documentation/networking/stmmac.txt
@@ -153,7 +153,8 @@ Where:
o pbl: the Programmable Burst Length is maximum number of beats to
be transferred in one DMA transaction.
GMAC also enables the 4xPBL by default.
- o fixed_burst/mixed_burst/burst_len
+ o txpbl/rxpbl: GMAC and newer supports independent DMA pbl for tx/rx.
+ o fixed_burst/mixed_burst/aal
o clk_csr: fixed CSR Clock range selection.
o has_gmac: uses the GMAC core.
o enh_desc: if sets the MAC will use the enhanced descriptor structure.
@@ -205,16 +206,22 @@ tuned according to the HW capabilities.

struct stmmac_dma_cfg {
int pbl;
+ int txpbl;
+ int rxpbl;
int fixed_burst;
- int burst_len_supported;
+ int mixed_burst;
+ bool aal;
};

Where:
- o pbl: Programmable Burst Length
+ o pbl: Programmable Burst Length (tx and rx)
+ o txpbl: Transmit Programmable Burst Length. Only for GMAC and newer.
+ If set, DMA tx will use this value rather than pbl.
+ o rxpbl: Receive Programmable Burst Length. Only for GMAC and newer.
+ If set, DMA rx will use this value rather than pbl.
o fixed_burst: program the DMA to use the fixed burst mode
- o burst_len: this is the value we put in the register
- supported values are provided as macros in
- linux/stmmac.h header file.
+ o mixed_burst: program the DMA to use the mixed burst mode
+ o aal: Address-Aligned Beats

---

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
index 318ae9f10104..99b8040af592 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
@@ -89,20 +89,20 @@ static void dwmac1000_dma_init(void __iomem *ioaddr,
u32 dma_tx, u32 dma_rx, int atds)
{
u32 value = readl(ioaddr + DMA_BUS_MODE);
+ int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl;
+ int rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl;

/*
* Set the DMA PBL (Programmable Burst Length) mode.
*
* Note: before stmmac core 3.50 this mode bit was 4xPBL, and
* post 3.5 mode bit acts as 8*PBL.
- *
- * This configuration doesn't take care about the Separate PBL
- * so only the bits: 13-8 are programmed with the PBL passed from the
- * platform.
*/
value |= DMA_BUS_MODE_MAXPBL;
- value &= ~DMA_BUS_MODE_PBL_MASK;
- value |= (dma_cfg->pbl << DMA_BUS_MODE_PBL_SHIFT);
+ value |= DMA_BUS_MODE_USP;
+ value &= ~(DMA_BUS_MODE_PBL_MASK | DMA_BUS_MODE_RPBL_MASK);
+ value |= (txpbl << DMA_BUS_MODE_PBL_SHIFT);
+ value |= (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);

/* Set the Fixed burst mode */
if (dma_cfg->fixed_burst)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 7d82a3464097..2c3b2098f350 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -71,11 +71,14 @@ static void dwmac4_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
writel(value, ioaddr + DMA_SYS_BUS_MODE);
}

-static void dwmac4_dma_init_channel(void __iomem *ioaddr, int pbl,
+static void dwmac4_dma_init_channel(void __iomem *ioaddr,
+ struct stmmac_dma_cfg *dma_cfg,
u32 dma_tx_phy, u32 dma_rx_phy,
u32 channel)
{
u32 value;
+ int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl;
+ int rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl;

/* set PBL for each channels. Currently we affect same configuration
* on each channel
@@ -85,11 +88,11 @@ static void dwmac4_dma_init_channel(void __iomem *ioaddr, int pbl,
writel(value, ioaddr + DMA_CHAN_CONTROL(channel));

value = readl(ioaddr + DMA_CHAN_TX_CONTROL(channel));
- value = value | (pbl << DMA_BUS_MODE_PBL_SHIFT);
+ value = value | (txpbl << DMA_BUS_MODE_PBL_SHIFT);
writel(value, ioaddr + DMA_CHAN_TX_CONTROL(channel));

value = readl(ioaddr + DMA_CHAN_RX_CONTROL(channel));
- value = value | (pbl << DMA_BUS_MODE_RPBL_SHIFT);
+ value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
writel(value, ioaddr + DMA_CHAN_RX_CONTROL(channel));

/* Mask interrupts by writing to CSR7 */
@@ -120,8 +123,7 @@ static void dwmac4_dma_init(void __iomem *ioaddr,
writel(value, ioaddr + DMA_SYS_BUS_MODE);

for (i = 0; i < DMA_CHANNEL_NB_MAX; i++)
- dwmac4_dma_init_channel(ioaddr, dma_cfg->pbl,
- dma_tx, dma_rx, i);
+ dwmac4_dma_init_channel(ioaddr, dma_cfg, dma_tx, dma_rx, i);
}

static void _dwmac4_dump_dma_regs(void __iomem *ioaddr, u32 channel)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 81800f23a9c4..96afe0561c99 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -315,6 +315,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
if (!dma_cfg->pbl)
dma_cfg->pbl = DEFAULT_DMA_PBL;
+ of_property_read_u32(np, "snps,txpbl", &dma_cfg->txpbl);
+ of_property_read_u32(np, "snps,rxpbl", &dma_cfg->rxpbl);

dma_cfg->aal = of_property_read_bool(np, "snps,aal");
dma_cfg->fixed_burst = of_property_read_bool(np, "snps,fixed-burst");
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 3537fb33cc90..e6d7a5940819 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -88,6 +88,8 @@ struct stmmac_mdio_bus_data {

struct stmmac_dma_cfg {
int pbl;
+ int txpbl;
+ int rxpbl;
int fixed_burst;
int mixed_burst;
bool aal;
--
2.1.4

2016-12-07 14:25:24

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH v3 6/6] net: smmac: allow configuring lower pbl values

From: Niklas Cassel <[email protected]>

The driver currently always sets the PBLx8/PBLx4 bit, which means that
the pbl values configured via the pbl/txpbl/rxpbl DT properties are
always multiplied by 8/4 in the hardware.

In order to allow the DT to configure lower pbl values, while at the
same time not changing behavior of any existing device trees using the
pbl/txpbl/rxpbl settings, add a property to disable the multiplication
of the pbl by 8/4 in the hardware.

Suggested-by: Rabin Vincent <[email protected]>
Signed-off-by: Niklas Cassel <[email protected]>
---
Documentation/devicetree/bindings/net/stmmac.txt | 2 ++
Documentation/networking/stmmac.txt | 5 ++++-
drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 3 ++-
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 3 ++-
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 ++
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 +
include/linux/stmmac.h | 1 +
7 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index 8080038ff1b2..128da752fec9 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -39,6 +39,8 @@ Optional properties:
If set, DMA tx will use this value rather than snps,pbl.
- snps,rxpbl Rx Programmable Burst Length. Only for GMAC and newer.
If set, DMA rx will use this value rather than snps,pbl.
+- snps,no-pbl-x8 Don't multiply the pbl/txpbl/rxpbl values by 8.
+ For core rev < 3.50, don't multiply the values by 4.
- snps,aal Address-Aligned Beats
- snps,fixed-burst Program the DMA to use the fixed burst mode
- snps,mixed-burst Program the DMA to use the mixed burst mode
diff --git a/Documentation/networking/stmmac.txt b/Documentation/networking/stmmac.txt
index 6add57374f70..2bb07078f535 100644
--- a/Documentation/networking/stmmac.txt
+++ b/Documentation/networking/stmmac.txt
@@ -152,8 +152,9 @@ Where:
o dma_cfg: internal DMA parameters
o pbl: the Programmable Burst Length is maximum number of beats to
be transferred in one DMA transaction.
- GMAC also enables the 4xPBL by default.
+ GMAC also enables the 4xPBL by default. (8xPBL for GMAC 3.50 and newer)
o txpbl/rxpbl: GMAC and newer supports independent DMA pbl for tx/rx.
+ o pblx8: Enable 8xPBL (4xPBL for core rev < 3.50). Enabled by default.
o fixed_burst/mixed_burst/aal
o clk_csr: fixed CSR Clock range selection.
o has_gmac: uses the GMAC core.
@@ -208,6 +209,7 @@ struct stmmac_dma_cfg {
int pbl;
int txpbl;
int rxpbl;
+ bool pblx8;
int fixed_burst;
int mixed_burst;
bool aal;
@@ -219,6 +221,7 @@ Where:
If set, DMA tx will use this value rather than pbl.
o rxpbl: Receive Programmable Burst Length. Only for GMAC and newer.
If set, DMA rx will use this value rather than pbl.
+ o pblx8: Enable 8xPBL (4xPBL for core rev < 3.50). Enabled by default.
o fixed_burst: program the DMA to use the fixed burst mode
o mixed_burst: program the DMA to use the mixed burst mode
o aal: Address-Aligned Beats
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
index 99b8040af592..612d3aaac9a4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
@@ -98,7 +98,8 @@ static void dwmac1000_dma_init(void __iomem *ioaddr,
* Note: before stmmac core 3.50 this mode bit was 4xPBL, and
* post 3.5 mode bit acts as 8*PBL.
*/
- value |= DMA_BUS_MODE_MAXPBL;
+ if (dma_cfg->pblx8)
+ value |= DMA_BUS_MODE_MAXPBL;
value |= DMA_BUS_MODE_USP;
value &= ~(DMA_BUS_MODE_PBL_MASK | DMA_BUS_MODE_RPBL_MASK);
value |= (txpbl << DMA_BUS_MODE_PBL_SHIFT);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 2c3b2098f350..8196ab5fc33c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -84,7 +84,8 @@ static void dwmac4_dma_init_channel(void __iomem *ioaddr,
* on each channel
*/
value = readl(ioaddr + DMA_CHAN_CONTROL(channel));
- value = value | DMA_BUS_MODE_PBL;
+ if (dma_cfg->pblx8)
+ value = value | DMA_BUS_MODE_PBL;
writel(value, ioaddr + DMA_CHAN_CONTROL(channel));

value = readl(ioaddr + DMA_CHAN_TX_CONTROL(channel));
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 56c8a2342c14..a2831773431a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -81,6 +81,7 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
plat->mdio_bus_data->phy_mask = 0;

plat->dma_cfg->pbl = 32;
+ plat->dma_cfg->pblx8 = true;
/* TODO: AXI */

/* Set default value for multicast hash bins */
@@ -115,6 +116,7 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
plat->mdio_bus_data->phy_mask = 0;

plat->dma_cfg->pbl = 16;
+ plat->dma_cfg->pblx8 = true;
plat->dma_cfg->fixed_burst = 1;
/* AXI (TODO) */

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 96afe0561c99..082cd48db6a7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -317,6 +317,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
dma_cfg->pbl = DEFAULT_DMA_PBL;
of_property_read_u32(np, "snps,txpbl", &dma_cfg->txpbl);
of_property_read_u32(np, "snps,rxpbl", &dma_cfg->rxpbl);
+ dma_cfg->pblx8 = !of_property_read_bool(np, "snps,no-pbl-x8");

dma_cfg->aal = of_property_read_bool(np, "snps,aal");
dma_cfg->fixed_burst = of_property_read_bool(np, "snps,fixed-burst");
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index e6d7a5940819..266dab9ad782 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -90,6 +90,7 @@ struct stmmac_dma_cfg {
int pbl;
int txpbl;
int rxpbl;
+ bool pblx8;
int fixed_burst;
int mixed_burst;
bool aal;
--
2.1.4

2016-12-08 08:50:57

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH v3 2/6] net: stmmac: simplify the common DMA init API

Hi Niklas,

On 12/07/2016 03:20 PM, Niklas Cassel wrote:
> From: Niklas Cassel <[email protected]>
>
> Use struct stmmac_dma_cfg *dma_cfg as an argument rather
> than using all the struct members as individual arguments.
>
> Signed-off-by: Niklas Cassel <[email protected]>

Thanks for this patch. You can add my Acked-by.

Regards
Alex


> ---
> drivers/net/ethernet/stmicro/stmmac/common.h | 4 ++--
> drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 13 +++++++------
> drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c | 7 ++++---
> drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 14 ++++++++------
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +-----
> 5 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 3ced2e1703c1..b13a144f72ad 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -412,8 +412,8 @@ extern const struct stmmac_desc_ops ndesc_ops;
> struct stmmac_dma_ops {
> /* DMA core initialization */
> int (*reset)(void __iomem *ioaddr);
> - void (*init)(void __iomem *ioaddr, int pbl, int fb, int mb,
> - int aal, u32 dma_tx, u32 dma_rx, int atds);
> + void (*init)(void __iomem *ioaddr, struct stmmac_dma_cfg *dma_cfg,
> + u32 dma_tx, u32 dma_rx, int atds);
> /* Configure the AXI Bus Mode Register */
> void (*axi)(void __iomem *ioaddr, struct stmmac_axi *axi);
> /* Dump DMA registers */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> index f35385266fbf..318ae9f10104 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> @@ -84,8 +84,9 @@ static void dwmac1000_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
> writel(value, ioaddr + DMA_AXI_BUS_MODE);
> }
>
> -static void dwmac1000_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
> - int aal, u32 dma_tx, u32 dma_rx, int atds)
> +static void dwmac1000_dma_init(void __iomem *ioaddr,
> + struct stmmac_dma_cfg *dma_cfg,
> + u32 dma_tx, u32 dma_rx, int atds)
> {
> u32 value = readl(ioaddr + DMA_BUS_MODE);
>
> @@ -101,20 +102,20 @@ static void dwmac1000_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
> */
> value |= DMA_BUS_MODE_MAXPBL;
> value &= ~DMA_BUS_MODE_PBL_MASK;
> - value |= (pbl << DMA_BUS_MODE_PBL_SHIFT);
> + value |= (dma_cfg->pbl << DMA_BUS_MODE_PBL_SHIFT);
>
> /* Set the Fixed burst mode */
> - if (fb)
> + if (dma_cfg->fixed_burst)
> value |= DMA_BUS_MODE_FB;
>
> /* Mixed Burst has no effect when fb is set */
> - if (mb)
> + if (dma_cfg->mixed_burst)
> value |= DMA_BUS_MODE_MB;
>
> if (atds)
> value |= DMA_BUS_MODE_ATDS;
>
> - if (aal)
> + if (dma_cfg->aal)
> value |= DMA_BUS_MODE_AAL;
>
> writel(value, ioaddr + DMA_BUS_MODE);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c
> index 61f54c99a7de..e5664da382f3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c
> @@ -32,11 +32,12 @@
> #include "dwmac100.h"
> #include "dwmac_dma.h"
>
> -static void dwmac100_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
> - int aal, u32 dma_tx, u32 dma_rx, int atds)
> +static void dwmac100_dma_init(void __iomem *ioaddr,
> + struct stmmac_dma_cfg *dma_cfg,
> + u32 dma_tx, u32 dma_rx, int atds)
> {
> /* Enable Application Access by writing to DMA CSR0 */
> - writel(DMA_BUS_MODE_DEFAULT | (pbl << DMA_BUS_MODE_PBL_SHIFT),
> + writel(DMA_BUS_MODE_DEFAULT | (dma_cfg->pbl << DMA_BUS_MODE_PBL_SHIFT),
> ioaddr + DMA_BUS_MODE);
>
> /* Mask interrupts by writing to CSR7 */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index e81b6e565c29..7d82a3464097 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -99,27 +99,29 @@ static void dwmac4_dma_init_channel(void __iomem *ioaddr, int pbl,
> writel(dma_rx_phy, ioaddr + DMA_CHAN_RX_BASE_ADDR(channel));
> }
>
> -static void dwmac4_dma_init(void __iomem *ioaddr, int pbl, int fb, int mb,
> - int aal, u32 dma_tx, u32 dma_rx, int atds)
> +static void dwmac4_dma_init(void __iomem *ioaddr,
> + struct stmmac_dma_cfg *dma_cfg,
> + u32 dma_tx, u32 dma_rx, int atds)
> {
> u32 value = readl(ioaddr + DMA_SYS_BUS_MODE);
> int i;
>
> /* Set the Fixed burst mode */
> - if (fb)
> + if (dma_cfg->fixed_burst)
> value |= DMA_SYS_BUS_FB;
>
> /* Mixed Burst has no effect when fb is set */
> - if (mb)
> + if (dma_cfg->mixed_burst)
> value |= DMA_SYS_BUS_MB;
>
> - if (aal)
> + if (dma_cfg->aal)
> value |= DMA_SYS_BUS_AAL;
>
> writel(value, ioaddr + DMA_SYS_BUS_MODE);
>
> for (i = 0; i < DMA_CHANNEL_NB_MAX; i++)
> - dwmac4_dma_init_channel(ioaddr, pbl, dma_tx, dma_rx, i);
> + dwmac4_dma_init_channel(ioaddr, dma_cfg->pbl,
> + dma_tx, dma_rx, i);
> }
>
> static void _dwmac4_dump_dma_regs(void __iomem *ioaddr, u32 channel)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 14366800e5e6..b1e42ddf0370 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1595,11 +1595,7 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
> return ret;
> }
>
> - priv->hw->dma->init(priv->ioaddr,
> - priv->plat->dma_cfg->pbl,
> - priv->plat->dma_cfg->fixed_burst,
> - priv->plat->dma_cfg->mixed_burst,
> - priv->plat->dma_cfg->aal,
> + priv->hw->dma->init(priv->ioaddr, priv->plat->dma_cfg,
> priv->dma_tx_phy, priv->dma_rx_phy, atds);
>
> if (priv->synopsys_id >= DWMAC_CORE_4_00) {
>

2016-12-08 09:03:13

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] net: stmmac: stmmac_platform: fix parsing of DT binding

Hi Niklas

On 12/07/2016 03:20 PM, Niklas Cassel wrote:
> From: Niklas Cassel <[email protected]>
>
> commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with DT")
> changed the parsing of the DT binding.
>
> Before 64c3b252e9fc, snps,fixed-burst and snps,mixed-burst were parsed
> regardless if the property snps,pbl existed or not.
> After the commit, fixed burst and mixed burst are only parsed if
> snps,pbl exists. Now when snps,aal has been added, it too is only
> parsed if snps,pbl exists.
>
> Since the DT binding does not specify that fixed burst, mixed burst
> or aal depend on snps,pbl being specified, undo changes introduced
> by 64c3b252e9fc.
>
> The issue commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with
> DT") tries to address is solved in another way:
> The databook specifies that all values other than
> 1, 2, 4, 8, 16, or 32 results in undefined behavior,
> so snps,pbl = <0> is invalid.
>
> If pbl is 0 after parsing, set pbl to DEFAULT_DMA_PBL.
> This handles the case where the property is omitted, and also handles
> the case where the property is specified without any data.
>
> Signed-off-by: Niklas Cassel <[email protected]>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +--
> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 29 +++++++++++-----------
> 2 files changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b1e42ddf0370..b5188122bc15 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1581,8 +1581,8 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
> int atds = 0;
> int ret = 0;
>
> - if (!priv->plat->dma_cfg) {
> - dev_err(priv->device, "DMA configuration not found\n");
> + if (!priv->plat->dma_cfg || !priv->plat->dma_cfg->pbl) {

How "priv->plat->dma_cfg->pbl" could be equal to 0 if you force it to
DEFAULT_DMA_PBL in "stmmac_probe_config_dt" in case of DT doesn't set
pbl value?


> + dev_err(priv->device, "Invalid DMA configuration\n");
> return -EINVAL;
> }
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index d3b6f92f350a..81800f23a9c4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -304,21 +304,22 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> plat->force_sf_dma_mode = 1;
> }
>
> - if (of_find_property(np, "snps,pbl", NULL)) {
> - dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
> - GFP_KERNEL);
> - if (!dma_cfg) {
> - stmmac_remove_config_dt(pdev, plat);
> - return ERR_PTR(-ENOMEM);
> - }
> - plat->dma_cfg = dma_cfg;
> - of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
> - dma_cfg->aal = of_property_read_bool(np, "snps,aal");
> - dma_cfg->fixed_burst =
> - of_property_read_bool(np, "snps,fixed-burst");
> - dma_cfg->mixed_burst =
> - of_property_read_bool(np, "snps,mixed-burst");
> + dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
> + GFP_KERNEL);
> + if (!dma_cfg) {
> + stmmac_remove_config_dt(pdev, plat);
> + return ERR_PTR(-ENOMEM);
> }
> + plat->dma_cfg = dma_cfg;
> +
> + of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
> + if (!dma_cfg->pbl)
> + dma_cfg->pbl = DEFAULT_DMA_PBL;
> +
> + dma_cfg->aal = of_property_read_bool(np, "snps,aal");
> + dma_cfg->fixed_burst = of_property_read_bool(np, "snps,fixed-burst");
> + dma_cfg->mixed_burst = of_property_read_bool(np, "snps,mixed-burst");
> +
> plat->force_thresh_dma_mode = of_property_read_bool(np, "snps,force_thresh_dma_mode");
> if (plat->force_thresh_dma_mode) {
> plat->force_sf_dma_mode = 0;
>

2016-12-08 09:12:33

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH v3 4/6] net: stmmac: dwmac1000: fix define DMA_BUS_MODE_RPBL_MASK

Hi Niklas

On 12/07/2016 03:20 PM, Niklas Cassel wrote:
> From: Niklas Cassel <[email protected]>
>
> DMA_BUS_MODE_RPBL_MASK is really 6 bits,
> just like DMA_BUS_MODE_PBL_MASK.
>
> Signed-off-by: Niklas Cassel <[email protected]>
> ---
> drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
> index ff3e5ab39bd0..52b9407a8a39 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
> @@ -225,7 +225,7 @@ enum rx_tx_priority_ratio {
>
> #define DMA_BUS_MODE_FB 0x00010000 /* Fixed burst */
> #define DMA_BUS_MODE_MB 0x04000000 /* Mixed burst */
> -#define DMA_BUS_MODE_RPBL_MASK 0x003e0000 /* Rx-Programmable Burst Len */
> +#define DMA_BUS_MODE_RPBL_MASK 0x007e0000 /* Rx-Programmable Burst Len */

Well spot. You can add my Acked-by.

Regards
Alex

> #define DMA_BUS_MODE_RPBL_SHIFT 17
> #define DMA_BUS_MODE_USP 0x00800000
> #define DMA_BUS_MODE_MAXPBL 0x01000000
>

2016-12-08 09:46:39

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] net: stmmac: stmmac_platform: fix parsing of DT binding

On 12/08/2016 10:02 AM, Alexandre Torgue wrote:
> Hi Niklas
>
> On 12/07/2016 03:20 PM, Niklas Cassel wrote:
>> From: Niklas Cassel <[email protected]>
>>
>> commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with DT")
>> changed the parsing of the DT binding.
>>
>> Before 64c3b252e9fc, snps,fixed-burst and snps,mixed-burst were parsed
>> regardless if the property snps,pbl existed or not.
>> After the commit, fixed burst and mixed burst are only parsed if
>> snps,pbl exists. Now when snps,aal has been added, it too is only
>> parsed if snps,pbl exists.
>>
>> Since the DT binding does not specify that fixed burst, mixed burst
>> or aal depend on snps,pbl being specified, undo changes introduced
>> by 64c3b252e9fc.
>>
>> The issue commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with
>> DT") tries to address is solved in another way:
>> The databook specifies that all values other than
>> 1, 2, 4, 8, 16, or 32 results in undefined behavior,
>> so snps,pbl = <0> is invalid.
>>
>> If pbl is 0 after parsing, set pbl to DEFAULT_DMA_PBL.
>> This handles the case where the property is omitted, and also handles
>> the case where the property is specified without any data.
>>
>> Signed-off-by: Niklas Cassel <[email protected]>
>> ---
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +--
>> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 29 +++++++++++-----------
>> 2 files changed, 17 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index b1e42ddf0370..b5188122bc15 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -1581,8 +1581,8 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>> int atds = 0;
>> int ret = 0;
>>
>> - if (!priv->plat->dma_cfg) {
>> - dev_err(priv->device, "DMA configuration not found\n");
>> + if (!priv->plat->dma_cfg || !priv->plat->dma_cfg->pbl) {
>
> How "priv->plat->dma_cfg->pbl" could be equal to 0 if you force it to DEFAULT_DMA_PBL in "stmmac_probe_config_dt" in case of DT doesn't set pbl value?

The PCI glue code does not call stmmac_probe_config_dt.

Also any glue driver could override the value set by stmmac_probe_config_dt
before calling stmmac_dvr_probe. So I guess if we want any trustworthy
sanity-checking, it actually has to be done in stmmac_main.c.


>
>
>> + dev_err(priv->device, "Invalid DMA configuration\n");
>> return -EINVAL;
>> }
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index d3b6f92f350a..81800f23a9c4 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -304,21 +304,22 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>> plat->force_sf_dma_mode = 1;
>> }
>>
>> - if (of_find_property(np, "snps,pbl", NULL)) {
>> - dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
>> - GFP_KERNEL);
>> - if (!dma_cfg) {
>> - stmmac_remove_config_dt(pdev, plat);
>> - return ERR_PTR(-ENOMEM);
>> - }
>> - plat->dma_cfg = dma_cfg;
>> - of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
>> - dma_cfg->aal = of_property_read_bool(np, "snps,aal");
>> - dma_cfg->fixed_burst =
>> - of_property_read_bool(np, "snps,fixed-burst");
>> - dma_cfg->mixed_burst =
>> - of_property_read_bool(np, "snps,mixed-burst");
>> + dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
>> + GFP_KERNEL);
>> + if (!dma_cfg) {
>> + stmmac_remove_config_dt(pdev, plat);
>> + return ERR_PTR(-ENOMEM);
>> }
>> + plat->dma_cfg = dma_cfg;
>> +
>> + of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
>> + if (!dma_cfg->pbl)
>> + dma_cfg->pbl = DEFAULT_DMA_PBL;
>> +
>> + dma_cfg->aal = of_property_read_bool(np, "snps,aal");
>> + dma_cfg->fixed_burst = of_property_read_bool(np, "snps,fixed-burst");
>> + dma_cfg->mixed_burst = of_property_read_bool(np, "snps,mixed-burst");
>> +
>> plat->force_thresh_dma_mode = of_property_read_bool(np, "snps,force_thresh_dma_mode");
>> if (plat->force_thresh_dma_mode) {
>> plat->force_sf_dma_mode = 0;
>>

2016-12-08 09:54:30

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] net: stmmac: stmmac_platform: fix parsing of DT binding

Hi

On 12/08/2016 10:46 AM, Niklas Cassel wrote:
> On 12/08/2016 10:02 AM, Alexandre Torgue wrote:
>> Hi Niklas
>>
>> On 12/07/2016 03:20 PM, Niklas Cassel wrote:
>>> From: Niklas Cassel <[email protected]>
>>>
>>> commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with DT")
>>> changed the parsing of the DT binding.
>>>
>>> Before 64c3b252e9fc, snps,fixed-burst and snps,mixed-burst were parsed
>>> regardless if the property snps,pbl existed or not.
>>> After the commit, fixed burst and mixed burst are only parsed if
>>> snps,pbl exists. Now when snps,aal has been added, it too is only
>>> parsed if snps,pbl exists.
>>>
>>> Since the DT binding does not specify that fixed burst, mixed burst
>>> or aal depend on snps,pbl being specified, undo changes introduced
>>> by 64c3b252e9fc.
>>>
>>> The issue commit 64c3b252e9fc ("net: stmmac: fixed the pbl setting with
>>> DT") tries to address is solved in another way:
>>> The databook specifies that all values other than
>>> 1, 2, 4, 8, 16, or 32 results in undefined behavior,
>>> so snps,pbl = <0> is invalid.
>>>
>>> If pbl is 0 after parsing, set pbl to DEFAULT_DMA_PBL.
>>> This handles the case where the property is omitted, and also handles
>>> the case where the property is specified without any data.
>>>
>>> Signed-off-by: Niklas Cassel <[email protected]>
>>> ---
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +--
>>> .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 29 +++++++++++-----------
>>> 2 files changed, 17 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> index b1e42ddf0370..b5188122bc15 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> @@ -1581,8 +1581,8 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>>> int atds = 0;
>>> int ret = 0;
>>>
>>> - if (!priv->plat->dma_cfg) {
>>> - dev_err(priv->device, "DMA configuration not found\n");
>>> + if (!priv->plat->dma_cfg || !priv->plat->dma_cfg->pbl) {
>>
>> How "priv->plat->dma_cfg->pbl" could be equal to 0 if you force it to DEFAULT_DMA_PBL in "stmmac_probe_config_dt" in case of DT doesn't set pbl value?
>
> The PCI glue code does not call stmmac_probe_config_dt.
>
> Also any glue driver could override the value set by stmmac_probe_config_dt
> before calling stmmac_dvr_probe. So I guess if we want any trustworthy
> sanity-checking, it actually has to be done in stmmac_main.c.

Ok I see, it is more safe. You can add my Acked-by.

Thanks
Alex

>
>
>>
>>
>>> + dev_err(priv->device, "Invalid DMA configuration\n");
>>> return -EINVAL;
>>> }
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> index d3b6f92f350a..81800f23a9c4 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>>> @@ -304,21 +304,22 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
>>> plat->force_sf_dma_mode = 1;
>>> }
>>>
>>> - if (of_find_property(np, "snps,pbl", NULL)) {
>>> - dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
>>> - GFP_KERNEL);
>>> - if (!dma_cfg) {
>>> - stmmac_remove_config_dt(pdev, plat);
>>> - return ERR_PTR(-ENOMEM);
>>> - }
>>> - plat->dma_cfg = dma_cfg;
>>> - of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
>>> - dma_cfg->aal = of_property_read_bool(np, "snps,aal");
>>> - dma_cfg->fixed_burst =
>>> - of_property_read_bool(np, "snps,fixed-burst");
>>> - dma_cfg->mixed_burst =
>>> - of_property_read_bool(np, "snps,mixed-burst");
>>> + dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*dma_cfg),
>>> + GFP_KERNEL);
>>> + if (!dma_cfg) {
>>> + stmmac_remove_config_dt(pdev, plat);
>>> + return ERR_PTR(-ENOMEM);
>>> }
>>> + plat->dma_cfg = dma_cfg;
>>> +
>>> + of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
>>> + if (!dma_cfg->pbl)
>>> + dma_cfg->pbl = DEFAULT_DMA_PBL;
>>> +
>>> + dma_cfg->aal = of_property_read_bool(np, "snps,aal");
>>> + dma_cfg->fixed_burst = of_property_read_bool(np, "snps,fixed-burst");
>>> + dma_cfg->mixed_burst = of_property_read_bool(np, "snps,mixed-burst");
>>> +
>>> plat->force_thresh_dma_mode = of_property_read_bool(np, "snps,force_thresh_dma_mode");
>>> if (plat->force_thresh_dma_mode) {
>>> plat->force_sf_dma_mode = 0;
>>>
>

2016-12-08 10:31:13

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH v3 5/6] net: stmmac: add support for independent DMA pbl for tx/rx

Hi Niklas

On 12/07/2016 03:20 PM, Niklas Cassel wrote:
> From: Niklas Cassel <[email protected]>
>
> GMAC and newer supports independent programmable burst lengths for
> DMA tx/rx. Add new optional devicetree properties representing this.
>
> To be backwards compatible, snps,pbl will still be valid, but
> snps,txpbl/snps,rxpbl will override the value in snps,pbl if set.
>
> If the IP is synthesized to use the AXI interface, there is a register
> and a matching DT property inside the optional stmmac-axi-config DT node
> for controlling burst lengths, named snps,blen.
> However, using this register, it is not possible to control tx and rx
> independently. Also, this register is not available if the IP was
> synthesized with, e.g., the AHB interface.
>
> Signed-off-by: Niklas Cassel <[email protected]>

Thanks, you can add my Acked-by.

Regards
Alex

> ---
> Documentation/devicetree/bindings/net/stmmac.txt | 6 +++++-
> Documentation/networking/stmmac.txt | 19 +++++++++++++------
> drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 12 ++++++------
> drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 12 +++++++-----
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 ++
> include/linux/stmmac.h | 2 ++
> 6 files changed, 35 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
> index b95ff998ba73..8080038ff1b2 100644
> --- a/Documentation/devicetree/bindings/net/stmmac.txt
> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
> @@ -34,7 +34,11 @@ Optional properties:
> platforms.
> - tx-fifo-depth: See ethernet.txt file in the same directory
> - rx-fifo-depth: See ethernet.txt file in the same directory
> -- snps,pbl Programmable Burst Length
> +- snps,pbl Programmable Burst Length (tx and rx)
> +- snps,txpbl Tx Programmable Burst Length. Only for GMAC and newer.
> + If set, DMA tx will use this value rather than snps,pbl.
> +- snps,rxpbl Rx Programmable Burst Length. Only for GMAC and newer.
> + If set, DMA rx will use this value rather than snps,pbl.
> - snps,aal Address-Aligned Beats
> - snps,fixed-burst Program the DMA to use the fixed burst mode
> - snps,mixed-burst Program the DMA to use the mixed burst mode
> diff --git a/Documentation/networking/stmmac.txt b/Documentation/networking/stmmac.txt
> index 014f4f756cb7..6add57374f70 100644
> --- a/Documentation/networking/stmmac.txt
> +++ b/Documentation/networking/stmmac.txt
> @@ -153,7 +153,8 @@ Where:
> o pbl: the Programmable Burst Length is maximum number of beats to
> be transferred in one DMA transaction.
> GMAC also enables the 4xPBL by default.
> - o fixed_burst/mixed_burst/burst_len
> + o txpbl/rxpbl: GMAC and newer supports independent DMA pbl for tx/rx.
> + o fixed_burst/mixed_burst/aal
> o clk_csr: fixed CSR Clock range selection.
> o has_gmac: uses the GMAC core.
> o enh_desc: if sets the MAC will use the enhanced descriptor structure.
> @@ -205,16 +206,22 @@ tuned according to the HW capabilities.
>
> struct stmmac_dma_cfg {
> int pbl;
> + int txpbl;
> + int rxpbl;
> int fixed_burst;
> - int burst_len_supported;
> + int mixed_burst;
> + bool aal;
> };
>
> Where:
> - o pbl: Programmable Burst Length
> + o pbl: Programmable Burst Length (tx and rx)
> + o txpbl: Transmit Programmable Burst Length. Only for GMAC and newer.
> + If set, DMA tx will use this value rather than pbl.
> + o rxpbl: Receive Programmable Burst Length. Only for GMAC and newer.
> + If set, DMA rx will use this value rather than pbl.
> o fixed_burst: program the DMA to use the fixed burst mode
> - o burst_len: this is the value we put in the register
> - supported values are provided as macros in
> - linux/stmmac.h header file.
> + o mixed_burst: program the DMA to use the mixed burst mode
> + o aal: Address-Aligned Beats
>
> ---
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> index 318ae9f10104..99b8040af592 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> @@ -89,20 +89,20 @@ static void dwmac1000_dma_init(void __iomem *ioaddr,
> u32 dma_tx, u32 dma_rx, int atds)
> {
> u32 value = readl(ioaddr + DMA_BUS_MODE);
> + int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl;
> + int rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl;
>
> /*
> * Set the DMA PBL (Programmable Burst Length) mode.
> *
> * Note: before stmmac core 3.50 this mode bit was 4xPBL, and
> * post 3.5 mode bit acts as 8*PBL.
> - *
> - * This configuration doesn't take care about the Separate PBL
> - * so only the bits: 13-8 are programmed with the PBL passed from the
> - * platform.
> */
> value |= DMA_BUS_MODE_MAXPBL;
> - value &= ~DMA_BUS_MODE_PBL_MASK;
> - value |= (dma_cfg->pbl << DMA_BUS_MODE_PBL_SHIFT);
> + value |= DMA_BUS_MODE_USP;
> + value &= ~(DMA_BUS_MODE_PBL_MASK | DMA_BUS_MODE_RPBL_MASK);
> + value |= (txpbl << DMA_BUS_MODE_PBL_SHIFT);
> + value |= (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
>
> /* Set the Fixed burst mode */
> if (dma_cfg->fixed_burst)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index 7d82a3464097..2c3b2098f350 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -71,11 +71,14 @@ static void dwmac4_dma_axi(void __iomem *ioaddr, struct stmmac_axi *axi)
> writel(value, ioaddr + DMA_SYS_BUS_MODE);
> }
>
> -static void dwmac4_dma_init_channel(void __iomem *ioaddr, int pbl,
> +static void dwmac4_dma_init_channel(void __iomem *ioaddr,
> + struct stmmac_dma_cfg *dma_cfg,
> u32 dma_tx_phy, u32 dma_rx_phy,
> u32 channel)
> {
> u32 value;
> + int txpbl = dma_cfg->txpbl ?: dma_cfg->pbl;
> + int rxpbl = dma_cfg->rxpbl ?: dma_cfg->pbl;
>
> /* set PBL for each channels. Currently we affect same configuration
> * on each channel
> @@ -85,11 +88,11 @@ static void dwmac4_dma_init_channel(void __iomem *ioaddr, int pbl,
> writel(value, ioaddr + DMA_CHAN_CONTROL(channel));
>
> value = readl(ioaddr + DMA_CHAN_TX_CONTROL(channel));
> - value = value | (pbl << DMA_BUS_MODE_PBL_SHIFT);
> + value = value | (txpbl << DMA_BUS_MODE_PBL_SHIFT);
> writel(value, ioaddr + DMA_CHAN_TX_CONTROL(channel));
>
> value = readl(ioaddr + DMA_CHAN_RX_CONTROL(channel));
> - value = value | (pbl << DMA_BUS_MODE_RPBL_SHIFT);
> + value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT);
> writel(value, ioaddr + DMA_CHAN_RX_CONTROL(channel));
>
> /* Mask interrupts by writing to CSR7 */
> @@ -120,8 +123,7 @@ static void dwmac4_dma_init(void __iomem *ioaddr,
> writel(value, ioaddr + DMA_SYS_BUS_MODE);
>
> for (i = 0; i < DMA_CHANNEL_NB_MAX; i++)
> - dwmac4_dma_init_channel(ioaddr, dma_cfg->pbl,
> - dma_tx, dma_rx, i);
> + dwmac4_dma_init_channel(ioaddr, dma_cfg, dma_tx, dma_rx, i);
> }
>
> static void _dwmac4_dump_dma_regs(void __iomem *ioaddr, u32 channel)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 81800f23a9c4..96afe0561c99 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -315,6 +315,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> of_property_read_u32(np, "snps,pbl", &dma_cfg->pbl);
> if (!dma_cfg->pbl)
> dma_cfg->pbl = DEFAULT_DMA_PBL;
> + of_property_read_u32(np, "snps,txpbl", &dma_cfg->txpbl);
> + of_property_read_u32(np, "snps,rxpbl", &dma_cfg->rxpbl);
>
> dma_cfg->aal = of_property_read_bool(np, "snps,aal");
> dma_cfg->fixed_burst = of_property_read_bool(np, "snps,fixed-burst");
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 3537fb33cc90..e6d7a5940819 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -88,6 +88,8 @@ struct stmmac_mdio_bus_data {
>
> struct stmmac_dma_cfg {
> int pbl;
> + int txpbl;
> + int rxpbl;
> int fixed_burst;
> int mixed_burst;
> bool aal;
>

2016-12-08 10:43:24

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH v3 6/6] net: smmac: allow configuring lower pbl values

Hi Niklas,

On 12/07/2016 03:20 PM, Niklas Cassel wrote:
> From: Niklas Cassel <[email protected]>
>
> The driver currently always sets the PBLx8/PBLx4 bit, which means that
> the pbl values configured via the pbl/txpbl/rxpbl DT properties are
> always multiplied by 8/4 in the hardware.
>
> In order to allow the DT to configure lower pbl values, while at the
> same time not changing behavior of any existing device trees using the
> pbl/txpbl/rxpbl settings, add a property to disable the multiplication
> of the pbl by 8/4 in the hardware.
>
> Suggested-by: Rabin Vincent <[email protected]>
> Signed-off-by: Niklas Cassel <[email protected]>

Thanks for this patch, you can add my Acked-by.

Thanks for the whole series.

Alex

> ---
> Documentation/devicetree/bindings/net/stmmac.txt | 2 ++
> Documentation/networking/stmmac.txt | 5 ++++-
> drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 3 ++-
> drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 3 ++-
> drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 ++
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 +
> include/linux/stmmac.h | 1 +
> 7 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
> index 8080038ff1b2..128da752fec9 100644
> --- a/Documentation/devicetree/bindings/net/stmmac.txt
> +++ b/Documentation/devicetree/bindings/net/stmmac.txt
> @@ -39,6 +39,8 @@ Optional properties:
> If set, DMA tx will use this value rather than snps,pbl.
> - snps,rxpbl Rx Programmable Burst Length. Only for GMAC and newer.
> If set, DMA rx will use this value rather than snps,pbl.
> +- snps,no-pbl-x8 Don't multiply the pbl/txpbl/rxpbl values by 8.
> + For core rev < 3.50, don't multiply the values by 4.
> - snps,aal Address-Aligned Beats
> - snps,fixed-burst Program the DMA to use the fixed burst mode
> - snps,mixed-burst Program the DMA to use the mixed burst mode
> diff --git a/Documentation/networking/stmmac.txt b/Documentation/networking/stmmac.txt
> index 6add57374f70..2bb07078f535 100644
> --- a/Documentation/networking/stmmac.txt
> +++ b/Documentation/networking/stmmac.txt
> @@ -152,8 +152,9 @@ Where:
> o dma_cfg: internal DMA parameters
> o pbl: the Programmable Burst Length is maximum number of beats to
> be transferred in one DMA transaction.
> - GMAC also enables the 4xPBL by default.
> + GMAC also enables the 4xPBL by default. (8xPBL for GMAC 3.50 and newer)
> o txpbl/rxpbl: GMAC and newer supports independent DMA pbl for tx/rx.
> + o pblx8: Enable 8xPBL (4xPBL for core rev < 3.50). Enabled by default.
> o fixed_burst/mixed_burst/aal
> o clk_csr: fixed CSR Clock range selection.
> o has_gmac: uses the GMAC core.
> @@ -208,6 +209,7 @@ struct stmmac_dma_cfg {
> int pbl;
> int txpbl;
> int rxpbl;
> + bool pblx8;
> int fixed_burst;
> int mixed_burst;
> bool aal;
> @@ -219,6 +221,7 @@ Where:
> If set, DMA tx will use this value rather than pbl.
> o rxpbl: Receive Programmable Burst Length. Only for GMAC and newer.
> If set, DMA rx will use this value rather than pbl.
> + o pblx8: Enable 8xPBL (4xPBL for core rev < 3.50). Enabled by default.
> o fixed_burst: program the DMA to use the fixed burst mode
> o mixed_burst: program the DMA to use the mixed burst mode
> o aal: Address-Aligned Beats
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> index 99b8040af592..612d3aaac9a4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
> @@ -98,7 +98,8 @@ static void dwmac1000_dma_init(void __iomem *ioaddr,
> * Note: before stmmac core 3.50 this mode bit was 4xPBL, and
> * post 3.5 mode bit acts as 8*PBL.
> */
> - value |= DMA_BUS_MODE_MAXPBL;
> + if (dma_cfg->pblx8)
> + value |= DMA_BUS_MODE_MAXPBL;
> value |= DMA_BUS_MODE_USP;
> value &= ~(DMA_BUS_MODE_PBL_MASK | DMA_BUS_MODE_RPBL_MASK);
> value |= (txpbl << DMA_BUS_MODE_PBL_SHIFT);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index 2c3b2098f350..8196ab5fc33c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -84,7 +84,8 @@ static void dwmac4_dma_init_channel(void __iomem *ioaddr,
> * on each channel
> */
> value = readl(ioaddr + DMA_CHAN_CONTROL(channel));
> - value = value | DMA_BUS_MODE_PBL;
> + if (dma_cfg->pblx8)
> + value = value | DMA_BUS_MODE_PBL;
> writel(value, ioaddr + DMA_CHAN_CONTROL(channel));
>
> value = readl(ioaddr + DMA_CHAN_TX_CONTROL(channel));
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 56c8a2342c14..a2831773431a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -81,6 +81,7 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
> plat->mdio_bus_data->phy_mask = 0;
>
> plat->dma_cfg->pbl = 32;
> + plat->dma_cfg->pblx8 = true;
> /* TODO: AXI */
>
> /* Set default value for multicast hash bins */
> @@ -115,6 +116,7 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
> plat->mdio_bus_data->phy_mask = 0;
>
> plat->dma_cfg->pbl = 16;
> + plat->dma_cfg->pblx8 = true;
> plat->dma_cfg->fixed_burst = 1;
> /* AXI (TODO) */
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 96afe0561c99..082cd48db6a7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -317,6 +317,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
> dma_cfg->pbl = DEFAULT_DMA_PBL;
> of_property_read_u32(np, "snps,txpbl", &dma_cfg->txpbl);
> of_property_read_u32(np, "snps,rxpbl", &dma_cfg->rxpbl);
> + dma_cfg->pblx8 = !of_property_read_bool(np, "snps,no-pbl-x8");
>
> dma_cfg->aal = of_property_read_bool(np, "snps,aal");
> dma_cfg->fixed_burst = of_property_read_bool(np, "snps,fixed-burst");
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index e6d7a5940819..266dab9ad782 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -90,6 +90,7 @@ struct stmmac_dma_cfg {
> int pbl;
> int txpbl;
> int rxpbl;
> + bool pblx8;
> int fixed_burst;
> int mixed_burst;
> bool aal;
>

2016-12-08 10:44:42

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found

Hi Niklas,

On 12/07/2016 03:20 PM, Niklas Cassel wrote:
> From: Niklas Cassel <[email protected]>
>
> All drivers except pci glue layer calls stmmac_probe_config_dt.
> stmmac_probe_config_dt does a kzalloc dma_cfg.
>
> pci glue layer does kzalloc dma_cfg explicitly, so all current
> drivers does a kzalloc dma_cfg.
>
> Return an error if no DMA configuration is found, that way
> we can assume that the DMA configuration always exists.
>
> Signed-off-by: Niklas Cassel <[email protected]>

Acked-by: Alexandre Torgue <alexandre.torgue@stcom>

> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 982c95213da4..14366800e5e6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1578,16 +1578,12 @@ static void stmmac_check_ether_addr(struct stmmac_priv *priv)
> */
> static int stmmac_init_dma_engine(struct stmmac_priv *priv)
> {
> - int pbl = DEFAULT_DMA_PBL, fixed_burst = 0, aal = 0;
> - int mixed_burst = 0;
> int atds = 0;
> int ret = 0;
>
> - if (priv->plat->dma_cfg) {
> - pbl = priv->plat->dma_cfg->pbl;
> - fixed_burst = priv->plat->dma_cfg->fixed_burst;
> - mixed_burst = priv->plat->dma_cfg->mixed_burst;
> - aal = priv->plat->dma_cfg->aal;
> + if (!priv->plat->dma_cfg) {
> + dev_err(priv->device, "DMA configuration not found\n");
> + return -EINVAL;
> }
>
> if (priv->extend_desc && (priv->mode == STMMAC_RING_MODE))
> @@ -1599,8 +1595,12 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
> return ret;
> }
>
> - priv->hw->dma->init(priv->ioaddr, pbl, fixed_burst, mixed_burst,
> - aal, priv->dma_tx_phy, priv->dma_rx_phy, atds);
> + priv->hw->dma->init(priv->ioaddr,
> + priv->plat->dma_cfg->pbl,
> + priv->plat->dma_cfg->fixed_burst,
> + priv->plat->dma_cfg->mixed_burst,
> + priv->plat->dma_cfg->aal,
> + priv->dma_tx_phy, priv->dma_rx_phy, atds);
>
> if (priv->synopsys_id >= DWMAC_CORE_4_00) {
> priv->rx_tail_addr = priv->dma_rx_phy +
>

2016-12-08 14:04:49

by Andreas Färber

[permalink] [raw]
Subject: Re: [PATCH v3 6/6] net: smmac: allow configuring lower pbl values

Hi,

In subject: s/smmac/stmmac/

Regards,
Andreas

--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)

2016-12-08 15:18:21

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v3 6/6] net: smmac: allow configuring lower pbl values

From: Alexandre Torgue <[email protected]>
Date: Thu, 8 Dec 2016 11:42:56 +0100

> Thanks for this patch, you can add my Acked-by.

Please simply state:

Acked-by: Alexandre Torgue <[email protected]>

in your reply and it will automatically appear when the patch is
applied. You don't have to ask the patch submitter or the person who
applies it to do it as you are doing here.

2016-12-08 15:19:18

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found

From: Alexandre Torgue <[email protected]>
Date: Thu, 8 Dec 2016 11:44:35 +0100

> Acked-by: Alexandre Torgue <alexandre.torgue@stcom>

Typo in your email.

I would suggest that you put this into an editor macro or
similar in order to avoid such typos in the future. That's
what people do who review a lot of patches.

2016-12-08 15:41:47

by Alexandre Torgue

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] net: stmmac: return error if no DMA configuration is found

Hi David,

On 12/08/2016 04:19 PM, David Miller wrote:
> From: Alexandre Torgue <[email protected]>
> Date: Thu, 8 Dec 2016 11:44:35 +0100
>
>> Acked-by: Alexandre Torgue <alexandre.torgue@stcom>
>
> Typo in your email.
>
> I would suggest that you put this into an editor macro or
> similar in order to avoid such typos in the future. That's
> what people do who review a lot of patches.

Sorry, I will pay attention next time.

>

2016-12-08 18:07:36

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] net: stmmac: make DMA programmable burst length more configurable

From: Niklas Cassel <[email protected]>
Date: Wed, 7 Dec 2016 15:20:02 +0100

> Make DMA programmable burst length more configurable in the stmmac driver.
>
> This is done by adding support for independent pbl for tx/rx through DT.
> More fine grained tuning of pbl is possible thanks to a DT property saying
> that we should NOT multiply pbl values by x8/x4 in hardware.
>
> All new DT properties are optional, and created in a way that it will not
> affect any existing DT configurations.

Series applied to net-next, thanks.