Skip to contentSkip to conversation

bindings: net: stmmac: correct note about TSO

PATCH RESEND Applied Niklas Cassel 2 participants 6 messages Dec 7, 2016 – Dec 8, 2016

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH RESEND 1/3 bindings: net: stmmac: correct note about TSO

From: Niklas Cassel <[email protected]>

snps,tso was previously placed under AXI BUS Mode parameters, suggesting that the property should be in the stmmac-axi-config node.

TSO (TCP Segmentation Offloading) has nothing to do with AXI BUS Mode parameters, and the parser actually expects it to be in the root node, not in the stmmac-axi-config.

Also added a note about snps,tso only being available on GMAC4 and newer.

---
 Documentation/devicetree/bindings/net/stmmac.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/net/stmmac.txt +3 -3
index 41b49e6075f5..b95ff998ba73 100644
@@ -1,7 +1,7 @@
 * STMicroelectronics 10/100/1000 Ethernet driver (GMAC)
 
 Required properties:
-- compatible: Should be "snps,dwmac-<ip_version>" "snps,dwmac"
+- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac"
 	For backwards compatibility: "st,spear600-gmac" is also supported.
 - reg: Address and length of the register set for the device
 - interrupt-parent: Should be the phandle for the interrupt controller
@@ -50,6 +50,8 @@ Optional properties:
 - snps,ps-speed: port selection speed that can be passed to the core when
 		 PCS is supported. For example, this is used in case of SGMII
 		 and MAC2MAC connection.
+- snps,tso: this enables the TSO feature otherwise it will be managed by
+		 MAC HW capability register. Only for GMAC4 and newer.
 - AXI BUS Mode parameters: below the list of all the parameters to program the
 			   AXI register inside the DMA module:
 	- snps,lpi_en: enable Low Power Interface
@@ -62,8 +64,6 @@ Optional properties:
 	- snps,fb: fixed-burst
 	- snps,mb: mixed-burst
 	- snps,rb: rebuild INCRx Burst
-	- snps,tso: this enables the TSO feature otherwise it will be managed by
-	    MAC HW capability register.
 - mdio: with compatible = "snps,dwmac-mdio", create and register mdio bus.
 
 Examples:
Signature · 1 line
2.1.4
#1 lore raw

PATCH RESEND 2/3 net: stmmac: dwmac-generic: add missing compatible strings

From: Niklas Cassel <[email protected]>

devicetree binding for stmmac states:
- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac" For backwards compatibility: "st,spear600-gmac" is also supported.

Since dwmac-generic.c calls stmmac_probe_config_dt explicitly, another alternative would have been to remove all compatible strings other than "snps,dwmac" and "st,spear600-gmac" from dwmac-generic.c.

However, that would probably do more good than harm, since when trying to figure out what hardware a certain driver supports, you usually look at the compatible strings in the struct of_device_id, and not in some function defined in a completely different file.

No functional change intended.

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c +3 -0
index e6e6c2fcc4b7..3304095c934c 100644
@@ -71,9 +71,12 @@ static int dwmac_generic_probe(struct platform_device *pdev)
 
 static const struct of_device_id dwmac_generic_match[] = {
 	{ .compatible = "st,spear600-gmac"},
+	{ .compatible = "snps,dwmac-3.50a"},
 	{ .compatible = "snps,dwmac-3.610"},
 	{ .compatible = "snps,dwmac-3.70a"},
 	{ .compatible = "snps,dwmac-3.710"},
+	{ .compatible = "snps,dwmac-4.00"},
+	{ .compatible = "snps,dwmac-4.10a"},
 	{ .compatible = "snps,dwmac"},
 	{ }
 };
Signature · 1 line
2.1.4
maintainer Applied
#2 lore raw

Re: [RESEND PATCH 2/3] net: stmmac: dwmac-generic: add missing compatible strings

From: Niklas Cassel <[email protected]>
Date: Wed, 7 Dec 2016 13:41:07 +0100

Quoted 15 lines

From: Niklas Cassel <[email protected]>

devicetree binding for stmmac states:
- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac" For backwards compatibility: "st,spear600-gmac" is also supported.

Since dwmac-generic.c calls stmmac_probe_config_dt explicitly, another alternative would have been to remove all compatible strings other than "snps,dwmac" and "st,spear600-gmac" from dwmac-generic.c.

However, that would probably do more good than harm, since when trying to figure out what hardware a certain driver supports, you usually look at the compatible strings in the struct of_device_id, and not in some function defined in a completely different file.

Quoted 4 lines

No functional change intended.

Applied.

#3 lore raw

PATCH RESEND 3/3 net: stmmac: stmmac_platform: use correct setup function for gmac4

In reply to Niklas Cassel

From: Niklas Cassel <[email protected]>

devicetree binding for stmmac states:
- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac" For backwards compatibility: "st,spear600-gmac" is also supported.

Previously, when specifying "snps,dwmac-4.10a", "snps,dwmac" as your compatible string, plat_stmmacenet_data would have both has_gmac and has_gmac4 set.

This would lead to stmmac_hw_init calling dwmac1000_setup rather than dwmac4_setup, resulting in a non-functional driver.
This happened since the check for has_gmac is done before the check for has_gmac4. However, the order should not matter, so it does not make sense to have both set.

If something is valid for both, you should do as the stmmac_interrupt does:
if (priv->plat->has_gmac || priv->plat->has_gmac4) ...

The places where it was obvious that the author actually meant if (has_gmac || has_gmac4) rather than if (has_gmac) has been updated.

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c  | 4 ++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +2 -2
index d5a8122b6033..dd5b38e4cd1f 100644
@@ -263,7 +263,7 @@ static void stmmac_ethtool_getdrvinfo(struct net_device *dev,
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
 
-	if (priv->plat->has_gmac)
+	if (priv->plat->has_gmac || priv->plat->has_gmac4)
 		strlcpy(info->driver, GMAC_ETHTOOL_NAME, sizeof(info->driver));
 	else
 		strlcpy(info->driver, MAC100_ETHTOOL_NAME,
@@ -448,7 +448,7 @@ static void stmmac_ethtool_gregs(struct net_device *dev,
 
 	memset(reg_space, 0x0, REG_SPACE_SIZE);
 
-	if (!priv->plat->has_gmac) {
+	if (!(priv->plat->has_gmac || priv->plat->has_gmac4)) {
 		/* MAC registers */
 		for (i = 0; i < 12; i++)
 			reg_space[i] = readl(priv->ioaddr + (i * 4));
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +1 -0
index e528e7126b65..d3b6f92f350a 100644
@@ -292,6 +292,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 	if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
 	    of_device_is_compatible(np, "snps,dwmac-4.10a")) {
 		plat->has_gmac4 = 1;
+		plat->has_gmac = 0;
 		plat->pmt = 1;
 		plat->tso_en = of_property_read_bool(np, "snps,tso");
 	}
Signature · 1 line
2.1.4
maintainer Applied
#4 lore raw

Re: [RESEND PATCH 3/3] net: stmmac: stmmac_platform: use correct setup function for gmac4

From: Niklas Cassel <[email protected]>
Date: Wed, 7 Dec 2016 13:41:08 +0100

Quoted 16 lines

From: Niklas Cassel <[email protected]>

devicetree binding for stmmac states:
- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac" For backwards compatibility: "st,spear600-gmac" is also supported.

Previously, when specifying "snps,dwmac-4.10a", "snps,dwmac" as your compatible string, plat_stmmacenet_data would have both has_gmac and has_gmac4 set.

This would lead to stmmac_hw_init calling dwmac1000_setup rather than dwmac4_setup, resulting in a non-functional driver.
This happened since the check for has_gmac is done before the check for has_gmac4. However, the order should not matter, so it does not make sense to have both set.

Quoted 8 lines

If something is valid for both, you should do as the stmmac_interrupt does:
if (priv->plat->has_gmac || priv->plat->has_gmac4) ...

The places where it was obvious that the author actually meant if (has_gmac || has_gmac4) rather than if (has_gmac) has been updated.

Applied.

maintainer
#5 lore raw
In reply to Niklas Cassel

From: Niklas Cassel <[email protected]>
Date: Wed, 7 Dec 2016 13:41:06 +0100

Quoted 9 lines

From: Niklas Cassel <[email protected]>

snps,tso was previously placed under AXI BUS Mode parameters, suggesting that the property should be in the stmmac-axi-config node.

TSO (TCP Segmentation Offloading) has nothing to do with AXI BUS Mode parameters, and the parser actually expects it to be in the root node, not in the stmmac-axi-config.

Quoted 6 lines

Also added a note about snps,tso only being available on GMAC4 and newer.

Applied.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html