Skip to contentSkip to conversation

net: dsa: Fixes and enhancements

PATCH In review Guenter Roeck 4 participants 25 messages Oct 29, 2014 – Oct 31, 2014

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

v3 cover net: dsa: Fixes and enhancements

Patch 01/15 addresses a bug indicated by an an annoying and unhelpful log message.

Patches 02/15 and 03/15 are minor enhancements, adding support for known switch revisions.

Patches 04/15 and 05/15 add support for MV88E6352 and MV88E6176.

Patch 06/15 adds support for hardware monitoring, specifically for reporting the chip temperature, to the dsa subsystem.

Patches 07/15 and 08/15 implement hardware monitoring for MV88E6352, MV88E6176, MV88E6123, MV88E6161, and MV88E6165.

Patch 09/15 and 10/15 add support for EEPROM access to the DSA subsystem.

Patch 11/15 implements EEPROM access for MV88E6352 and MV88E6176.

Patch 12/15 adds support for reading switch registers to the DSA subsystem.

Patches 13/15 amd 14/15 implement support for reading switch registers to the drivers for MV88E6352, MV88E6176, MV88E6123, MV88E6161, and MV88E6165.

Patch 15/15 adds support for reading additional RMON registers to the drivers for MV88E6352, MV88E6176, MV88E6123, MV88E6161, and MV88E6165.

The series was tested on top of v3.18-rc2 in an x86 system with MV88E6352. Testing in systems with 88E6131, 88E6060 and MV88E6165 was done earlier (I don't have access to those systems right now). The series was also build tested using my build system at http://server.roeck-us.net:8010/builders.
Look into the 'dsa' column for build results.

The series merges cleanly into net-next as of today (10/29).

v3:
- Fix bug in eeprom patches seen if devicetree is enabled:
eeprom-length property is attached to switch devicetree node, not to dsa node, and there was a compile error.
v2:
- Made reporting chip temperatures through the hwmon subsystem optional with new Kconfig option
- Changed the hwmon chip name to <network device name>_dsa<index>
- Made EEPROM presence and size configurable through platform and devicetree data
- Various minor changes and fixes (see individual patches for details)

----------------------------------------------------------------
The following changes since commit cac7f2429872d3733dc3f9915857b1691da2eb2f:

Linux 3.18-rc2 (2014-10-26 16:48:41 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git dsa

for you to fetch changes up to 245472a4f3ca2a68c5632f2c37da41aa2d3fb2f0:

net: dsa: Provide additional RMON statistics (2014-10-29 08:55:37 -0700)

----------------------------------------------------------------
Guenter Roeck (15):
net: dsa: Don't set skb->protocol on outgoing tagged packets
net: dsa: Report known silicon revisions for Marvell 88E6060
net: dsa: Report known silicon revisions for Marvell 88E6131
net: dsa: Add support for Marvell 88E6352
net: dsa/mv88e6352: Add support for MV88E6176
net: dsa: Add support for reporting switch chip temperatures
net: dsa/mv88e6352: Report chip temperature
net: dsa/mv88e6123_61_65: Report chip temperature
net: dsa: Add support for switch EEPROM access
dsa: Add new optional devicetree property to describe EEPROM size
net: dsa/mv88e6352: Implement EEPROM access functions
net: dsa: Add support for reading switch registers with ethtool
net: dsa/mv88e6123_61_65: Add support for reading switch registers
net: dsa/mv88e6352: Add support for reading switch registers
net: dsa: Provide additional RMON statistics

 Documentation/devicetree/bindings/net/dsa/dsa.txt |   9 +-
 MAINTAINERS                                       |   5 +
 drivers/net/dsa/Kconfig                           |   9 +
 drivers/net/dsa/Makefile                          |   3 +
 drivers/net/dsa/mv88e6060.c                       |   5 +-
 drivers/net/dsa/mv88e6123_61_65.c                 |  73 +-
 drivers/net/dsa/mv88e6131.c                       |  12 +-
 drivers/net/dsa/mv88e6352.c                       | 788 ++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.c                       |  53 +-
 drivers/net/dsa/mv88e6xxx.h                       |  15 +
 include/net/dsa.h                                 |  33 +
 net/dsa/Kconfig                                   |  11 +
 net/dsa/dsa.c                                     | 135 ++++
 net/dsa/slave.c                                   |  64 ++
 net/dsa/tag_dsa.c                                 |   2 -
 net/dsa/tag_edsa.c                                |   2 -
 net/dsa/tag_trailer.c                             |   2 -
 17 files changed, 1202 insertions(+), 19 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6352.c
#1 lore raw

PATCH v3 1/15 net: dsa: Don't set skb->protocol on outgoing tagged packets

Setting skb->protocol to a private protocol type may result in warning messages such as

	e1000e 0000:00:19.0 em1: checksum_partial proto=dada!

This happens if the L3 protocol is IP or IPv6 and skb->ip_summed is set to CHECKSUM_PARTIAL. Looking through the code, it appears that changing skb->protocol for transmitted packets is not necessary and may actually be harmful. For example, it prevents purposely unmodified (from a DSA perspective) network drivers from properly setting up their transmit checksum offload pointers since they inspect skb->protocol to set up the IPv4 header or IPv6 header pointers. So don't unnecessarily change the protocol field.

---
v3:
- Updated description
v2:
- No change

 net/dsa/tag_dsa.c     | 2 --
 net/dsa/tag_edsa.c    | 2 --
 net/dsa/tag_trailer.c | 2 --
 3 files changed, 6 deletions(-)
net/dsa/tag_dsa.c +0 -2
index ce90c8b..2dab270 100644
@@ -63,8 +63,6 @@ static netdev_tx_t dsa_xmit(struct sk_buff *skb, struct net_device *dev)
 		dsa_header[3] = 0x00;
 	}
 
-	skb->protocol = htons(ETH_P_DSA);
-
 	skb->dev = p->parent->dst->master_netdev;
 	dev_queue_xmit(skb);
 
net/dsa/tag_edsa.c +0 -2
index 94fcce7..9aeda59 100644
@@ -76,8 +76,6 @@ static netdev_tx_t edsa_xmit(struct sk_buff *skb, struct net_device *dev)
 		edsa_header[7] = 0x00;
 	}
 
-	skb->protocol = htons(ETH_P_EDSA);
-
 	skb->dev = p->parent->dst->master_netdev;
 	dev_queue_xmit(skb);
 
net/dsa/tag_trailer.c +0 -2
index 115fdca..e268f9d 100644
@@ -57,8 +57,6 @@ static netdev_tx_t trailer_xmit(struct sk_buff *skb, struct net_device *dev)
 	trailer[2] = 0x10;
 	trailer[3] = 0x00;
 
-	nskb->protocol = htons(ETH_P_TRAILER);
-
 	nskb->dev = p->parent->dst->master_netdev;
 	dev_queue_xmit(nskb);
 
Signature · 1 line
1.9.1
#2 lore raw

PATCH v3 2/15 net: dsa: Report known silicon revisions for Marvell 88E6060

In reply to Guenter Roeck

Report known silicon revisions when probing Marvell 88E6060 switches.

---
v3:
- No change
v2:
- No change

 drivers/net/dsa/mv88e6060.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/net/dsa/mv88e6060.c +4 -1
index 05b0ca3..c29aebe 100644
@@ -69,8 +69,11 @@ static char *mv88e6060_probe(struct device *host_dev, int sw_addr)
 
 	ret = mdiobus_read(bus, sw_addr + REG_PORT(0), 0x03);
 	if (ret >= 0) {
-		ret &= 0xfff0;
 		if (ret == 0x0600)
+			return "Marvell 88E6060 (A0)";
+		if (ret == 0x0601 || ret == 0x0602)
+			return "Marvell 88E6060 (B0)";
+		if ((ret & 0xfff0) == 0x0600)
 			return "Marvell 88E6060";
 	}
 
Signature · 1 line
1.9.1
#3 lore raw

PATCH v3 3/15 net: dsa: Report known silicon revisions for Marvell 88E6131

In reply to Guenter Roeck

Report known silicon revisions when probing Marvell 88E6131 switches.

---
v3:
- No change
v2:
- No change

 drivers/net/dsa/mv88e6131.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/net/dsa/mv88e6131.c +8 -4
index 244c735..1230f52 100644
@@ -21,6 +21,7 @@
 #define ID_6085		0x04a0
 #define ID_6095		0x0950
 #define ID_6131		0x1060
+#define ID_6131_B2	0x1066
 
 static char *mv88e6131_probe(struct device *host_dev, int sw_addr)
 {
@@ -32,12 +33,15 @@ static char *mv88e6131_probe(struct device *host_dev, int sw_addr)
 
 	ret = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), 0x03);
 	if (ret >= 0) {
-		ret &= 0xfff0;
-		if (ret == ID_6085)
+		int ret_masked = ret & 0xfff0;
+
+		if (ret_masked == ID_6085)
 			return "Marvell 88E6085";
-		if (ret == ID_6095)
+		if (ret_masked == ID_6095)
 			return "Marvell 88E6095/88E6095F";
-		if (ret == ID_6131)
+		if (ret == ID_6131_B2)
+			return "Marvell 88E6131 (B2)";
+		if (ret_masked == ID_6131)
 			return "Marvell 88E6131";
 	}
 
Signature · 1 line
1.9.1
#4 lore raw

PATCH v3 4/15 net: dsa: Add support for Marvell 88E6352

In reply to Guenter Roeck

Marvell 88E6352 is mostly compatible to MV88E6123/61/65, but requires indirect phy access. Also, its configuration registers are a bit different.

---
v3:
- No change
v2:
- No change

 MAINTAINERS                 |   5 +
 drivers/net/dsa/Kconfig     |   8 +
 drivers/net/dsa/Makefile    |   3 +
 drivers/net/dsa/mv88e6352.c | 464 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.c |   3 +
 drivers/net/dsa/mv88e6xxx.h |   7 +
 6 files changed, 490 insertions(+)
 create mode 100644 drivers/net/dsa/mv88e6352.c
MAINTAINERS +5 -0
index dab92a7..91db18e 100644
@@ -5860,6 +5860,11 @@ M:	Russell King <[email protected]>
 S:	Maintained
 F:	drivers/gpu/drm/armada/
 
+MARVELL 88E6352 DSA support
+M:	Guenter Roeck <[email protected]>
+S:	Maintained
+F:	drivers/net/dsa/mv88e6352.c
+
 MARVELL GIGABIT ETHERNET DRIVERS (skge/sky2)
 M:	Mirko Lindner <[email protected]>
 M:	Stephen Hemminger <[email protected]>
drivers/net/dsa/Kconfig +8 -0
index 9234d80..0987c33 100644
@@ -45,6 +45,14 @@ config NET_DSA_MV88E6171
 	  This enables support for the Marvell 88E6171 ethernet switch
 	  chip.
 
+config NET_DSA_MV88E6352
+	tristate "Marvell 88E6352 ethernet switch chip support"
+	select NET_DSA
+	select NET_DSA_MV88E6XXX
+	select NET_DSA_TAG_EDSA
+	---help---
+	  This enables support for the Marvell 88E6352 ethernet switch chip.
+
 config NET_DSA_BCM_SF2
 	tristate "Broadcom Starfighter 2 Ethernet switch support"
 	depends on HAS_IOMEM
drivers/net/dsa/Makefile +3 -0
index 23a90de..e2d51c4 100644
@@ -7,6 +7,9 @@ endif
 ifdef CONFIG_NET_DSA_MV88E6131
 mv88e6xxx_drv-y += mv88e6131.o
 endif
+ifdef CONFIG_NET_DSA_MV88E6352
+mv88e6xxx_drv-y += mv88e6352.o
+endif
 ifdef CONFIG_NET_DSA_MV88E6171
 mv88e6xxx_drv-y += mv88e6171.o
 endif
drivers/net/dsa/mv88e6352.c new file +464 -0

464 lines not on this page · raw message

drivers/net/dsa/mv88e6xxx.c +3 -0
index a6c90cf..8e1090b 100644
@@ -507,6 +507,9 @@ static int __init mv88e6xxx_init(void)
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6123_61_65)
 	register_switch_driver(&mv88e6123_61_65_switch_driver);
 #endif
+#if IS_ENABLED(CONFIG_NET_DSA_MV88E6352)
+	register_switch_driver(&mv88e6352_switch_driver);
+#endif
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6171)
 	register_switch_driver(&mv88e6171_switch_driver);
 #endif
drivers/net/dsa/mv88e6xxx.h +7 -0
index 5e5145a..c0ce133 100644
@@ -37,6 +37,12 @@ struct mv88e6xxx_priv_state {
 	 */
 	struct mutex	stats_mutex;
 
+	/* This mutex serializes phy access for chips with
+	 * indirect phy addressing. It is unused for chips
+	 * with direct phy access.
+	 */
+	struct mutex	phy_mutex;
+
 	int		id; /* switch product id */
 };
 
@@ -70,6 +76,7 @@ void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds,
 
 extern struct dsa_switch_driver mv88e6131_switch_driver;
 extern struct dsa_switch_driver mv88e6123_61_65_switch_driver;
+extern struct dsa_switch_driver mv88e6352_switch_driver;
 extern struct dsa_switch_driver mv88e6171_switch_driver;
 
 #define REG_READ(addr, reg)						\
Signature · 1 line
1.9.1
#5 lore raw

PATCH v3 5/15 net: dsa/mv88e6352: Add support for MV88E6176

In reply to Guenter Roeck

MV88E6176 is mostly compatible to MV88E6352 and is documented in the same functional specification. Add support for it.

---
v3:
- No change
v2:
- No change

 drivers/net/dsa/Kconfig     | 5 +++--
 drivers/net/dsa/mv88e6352.c | 2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)
drivers/net/dsa/Kconfig +3 -2
index 0987c33..2d1a55e 100644
@@ -46,12 +46,13 @@ config NET_DSA_MV88E6171
 	  chip.
 
 config NET_DSA_MV88E6352
-	tristate "Marvell 88E6352 ethernet switch chip support"
+	tristate "Marvell 88E6176/88E6352 ethernet switch chip support"
 	select NET_DSA
 	select NET_DSA_MV88E6XXX
 	select NET_DSA_TAG_EDSA
 	---help---
-	  This enables support for the Marvell 88E6352 ethernet switch chip.
+	  This enables support for the Marvell 88E6176 and 88E6352 ethernet
+	  switch chips.
 
 config NET_DSA_BCM_SF2
 	tristate "Broadcom Starfighter 2 Ethernet switch support"
drivers/net/dsa/mv88e6352.c +2 -0
index 43a5826..f17364f 100644
@@ -73,6 +73,8 @@ static char *mv88e6352_probe(struct device *host_dev, int sw_addr)
 
 	ret = __mv88e6xxx_reg_read(bus, sw_addr, REG_PORT(0), 0x03);
 	if (ret >= 0) {
+		if ((ret & 0xfff0) == 0x1760)
+			return "Marvell 88E6176";
 		if (ret == 0x3521)
 			return "Marvell 88E6352 (A0)";
 		if (ret == 0x3522)
Signature · 1 line
1.9.1
#6 lore raw

PATCH v3 6/15 net: dsa: Add support for reporting switch chip temperatures

In reply to Guenter Roeck

Some switches provide chip temperature data.
Add support for reporting it through the hwmon subsystem.

---
v3:
- No change
v2:
- Updated headline to reflect what is reported, not how.
- Make added functionality optional with new Kconfig flag
- Register with hwmon subsystem as virtual device (no parent)
- Generate hwmon 'name' attribute from sanitized master network device name plus _dsaX, where X is the DSA switch index and 'sanitized' means to pick all characters and digits from the network device name
- Do not use devm_ function to register hwmon device, since the hwmon device is not associated with a device and thus can not be auto-removed.

 include/net/dsa.h |  16 +++++++
 net/dsa/Kconfig   |  11 +++++
 net/dsa/dsa.c     | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+)
include/net/dsa.h +16 -0
index b765592..55e75e7 100644
@@ -139,6 +139,14 @@ struct dsa_switch {
 	 */
 	struct device		*master_dev;
 
+#ifdef CONFIG_NET_DSA_HWMON
+	/*
+	 * Hardware monitoring information
+	 */
+	char			hwmon_name[IFNAMSIZ + 8];
+	struct device		*hwmon_dev;
+#endif
+
 	/*
 	 * Slave mii_bus and devices for the individual ports.
 	 */
@@ -242,6 +250,14 @@ struct dsa_switch_driver {
 			   struct ethtool_eee *e);
 	int	(*get_eee)(struct dsa_switch *ds, int port,
 			   struct ethtool_eee *e);
+
+#ifdef CONFIG_NET_DSA_HWMON
+	/* Hardware monitoring */
+	int	(*get_temp)(struct dsa_switch *ds, int *temp);
+	int	(*get_temp_limit)(struct dsa_switch *ds, int *temp);
+	int	(*set_temp_limit)(struct dsa_switch *ds, int temp);
+	int	(*get_temp_alarm)(struct dsa_switch *ds, bool *alarm);
+#endif
 };
 
 void register_switch_driver(struct dsa_switch_driver *type);
net/dsa/Kconfig +11 -0
index a585fd6..5f8ac40 100644
@@ -11,6 +11,17 @@ config NET_DSA
 
 if NET_DSA
 
+config NET_DSA_HWMON
+	bool "Distributed Switch Architecture HWMON support"
+	default y
+	depends on HWMON && !(NET_DSA=y && HWMON=m)
+	---help---
+	  Say Y if you want to expose thermal sensor data on switches supported
+	  by the Distributed Switch Architecture.
+
+	  Some of those switches contain thermal sensors. This data is available
+	  via the hwmon sysfs interface and exposes the onboard sensors.
+
 # tagging formats
 config NET_DSA_TAG_BRCM
 	bool
net/dsa/dsa.c +131 -0

161 lines not on this page · raw message

Signature · 1 line
1.9.1
#7 lore raw

PATCH v3 7/15 net: dsa/mv88e6352: Report chip temperature

In reply to Guenter Roeck

MV88E6352 supports reading the chip temperature from two PHY registers, 6:26 and 6:27. Report it using the more accurate register 6:27. Also report temperature limit and alarm.

---
v3:
- No change
v2:
- Make new functionality optional, depending on CONFIG_NET_DSA_HWMON

 drivers/net/dsa/mv88e6352.c | 101 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)
drivers/net/dsa/mv88e6352.c +101 -0
index f17364f..744e6fa 100644
@@ -325,6 +325,101 @@ static int mv88e6352_setup_port(struct dsa_switch *ds, int p)
 	return 0;
 }
 
+#ifdef CONFIG_NET_DSA_HWMON
+
+static int mv88e6352_phy_page_read(struct dsa_switch *ds,
+				   int port, int page, int reg)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int ret;
+
+	mutex_lock(&ps->phy_mutex);
+	ret = __mv88e6352_phy_write(ds, port, 0x16, page);
+	if (ret < 0)
+		goto error;
+	ret = __mv88e6352_phy_read(ds, port, reg);
+error:
+	__mv88e6352_phy_write(ds, port, 0x16, 0x0);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
+}
+
+static int mv88e6352_phy_page_write(struct dsa_switch *ds,
+				    int port, int page, int reg, int val)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int ret;
+
+	mutex_lock(&ps->phy_mutex);
+	ret = __mv88e6352_phy_write(ds, port, 0x16, page);
+	if (ret < 0)
+		goto error;
+
+	ret = __mv88e6352_phy_write(ds, port, reg, val);
+error:
+	__mv88e6352_phy_write(ds, port, 0x16, 0x0);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
+}
+
+static int mv88e6352_get_temp(struct dsa_switch *ds, int *temp)
+{
+	int ret;
+
+	*temp = 0;
+
+	ret = mv88e6352_phy_page_read(ds, 0, 6, 27);
+	if (ret < 0)
+		return ret;
+
+	*temp = (ret & 0xff) - 25;
+
+	return 0;
+}
+
+static int mv88e6352_get_temp_limit(struct dsa_switch *ds, int *temp)
+{
+	int ret;
+
+	*temp = 0;
+
+	ret = mv88e6352_phy_page_read(ds, 0, 6, 26);
+	if (ret < 0)
+		return ret;
+
+	*temp = (((ret >> 8) & 0x1f) * 5) - 25;
+
+	return 0;
+}
+
+static int mv88e6352_set_temp_limit(struct dsa_switch *ds, int temp)
+{
+	int ret;
+
+	ret = mv88e6352_phy_page_read(ds, 0, 6, 26);
+	if (ret < 0)
+		return ret;
+	temp = clamp_val(DIV_ROUND_CLOSEST(temp, 5) + 5, 0, 0x1f);
+	return mv88e6352_phy_page_write(ds, 0, 6, 26,
+					(ret & 0xe0ff) | (temp << 8));
+}
+
+static int mv88e6352_get_temp_alarm(struct dsa_switch *ds, bool *alarm)
+{
+	int ret;
+
+	*alarm = false;
+
+	ret = mv88e6352_phy_page_read(ds, 0, 6, 26);
+	if (ret < 0)
+		return ret;
+
+	*alarm = !!(ret & 0x40);
+
+	return 0;
+}
+#endif /* CONFIG_NET_DSA_HWMON */
+
 static int mv88e6352_setup(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
@@ -461,6 +556,12 @@ struct dsa_switch_driver mv88e6352_switch_driver = {
 	.get_strings		= mv88e6352_get_strings,
 	.get_ethtool_stats	= mv88e6352_get_ethtool_stats,
 	.get_sset_count		= mv88e6352_get_sset_count,
+#ifdef CONFIG_NET_DSA_HWMON
+	.get_temp		= mv88e6352_get_temp,
+	.get_temp_limit		= mv88e6352_get_temp_limit,
+	.set_temp_limit		= mv88e6352_set_temp_limit,
+	.get_temp_alarm		= mv88e6352_get_temp_alarm,
+#endif
 };
 
 MODULE_ALIAS("platform:mv88e6352");
Signature · 1 line
1.9.1
#8 lore raw

PATCH v3 8/15 net: dsa/mv88e6123_61_65: Report chip temperature

In reply to Guenter Roeck

MV88E6123 and compatible chips support reading the chip temperature from PHY register 6:26.

---
v3:
- No change
v2:
- Make new functionality optional, depending on CONFIG_NET_DSA_HWMON

 drivers/net/dsa/mv88e6123_61_65.c | 68 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 66 insertions(+), 2 deletions(-)
drivers/net/dsa/mv88e6123_61_65.c +66 -2
index a332c53..9f43c9b 100644
@@ -291,6 +291,54 @@ static int mv88e6123_61_65_setup_port(struct dsa_switch *ds, int p)
 	return 0;
 }
 
+#ifdef CONFIG_NET_DSA_HWMON
+
+static int  mv88e6123_61_65_get_temp(struct dsa_switch *ds, int *temp)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int ret;
+	int val;
+
+	*temp = 0;
+
+	mutex_lock(&ps->phy_mutex);
+
+	ret = mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x6);
+	if (ret < 0)
+		goto error;
+
+	/* Enable temperature sensor */
+	ret = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
+	if (ret < 0)
+		goto error;
+
+	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret | (1 << 5));
+	if (ret < 0)
+		goto error;
+
+	/* Wait for temperature to stabilize */
+	usleep_range(10000, 12000);
+
+	val = mv88e6xxx_phy_read(ds, 0x0, 0x1a);
+	if (val < 0) {
+		ret = val;
+		goto error;
+	}
+
+	/* Disable temperature sensor */
+	ret = mv88e6xxx_phy_write(ds, 0x0, 0x1a, ret & ~(1 << 5));
+	if (ret < 0)
+		goto error;
+
+	*temp = ((val & 0x1f) - 5) * 5;
+
+error:
+	mv88e6xxx_phy_write(ds, 0x0, 0x16, 0x0);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
+}
+#endif /* CONFIG_NET_DSA_HWMON */
+
 static int mv88e6123_61_65_setup(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
@@ -299,6 +347,7 @@ static int mv88e6123_61_65_setup(struct dsa_switch *ds)
 
 	mutex_init(&ps->smi_mutex);
 	mutex_init(&ps->stats_mutex);
+	mutex_init(&ps->phy_mutex);
 
 	ret = mv88e6123_61_65_switch_reset(ds);
 	if (ret < 0)
@@ -329,16 +378,28 @@ static int mv88e6123_61_65_port_to_phy_addr(int port)
 static int
 mv88e6123_61_65_phy_read(struct dsa_switch *ds, int port, int regnum)
 {
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int addr = mv88e6123_61_65_port_to_phy_addr(port);
-	return mv88e6xxx_phy_read(ds, addr, regnum);
+	int ret;
+
+	mutex_lock(&ps->phy_mutex);
+	ret = mv88e6xxx_phy_read(ds, addr, regnum);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
 }
 
 static int
 mv88e6123_61_65_phy_write(struct dsa_switch *ds,
 			      int port, int regnum, u16 val)
 {
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int addr = mv88e6123_61_65_port_to_phy_addr(port);
-	return mv88e6xxx_phy_write(ds, addr, regnum, val);
+	int ret;
+
+	mutex_lock(&ps->phy_mutex);
+	ret = mv88e6xxx_phy_write(ds, addr, regnum, val);
+	mutex_unlock(&ps->phy_mutex);
+	return ret;
 }
 
 static struct mv88e6xxx_hw_stat mv88e6123_61_65_hw_stats[] = {
@@ -406,6 +467,9 @@ struct dsa_switch_driver mv88e6123_61_65_switch_driver = {
 	.get_strings		= mv88e6123_61_65_get_strings,
 	.get_ethtool_stats	= mv88e6123_61_65_get_ethtool_stats,
 	.get_sset_count		= mv88e6123_61_65_get_sset_count,
+#ifdef CONFIG_NET_DSA_HWMON
+	.get_temp		= mv88e6123_61_65_get_temp,
+#endif
 };
 
 MODULE_ALIAS("platform:mv88e6123");
Signature · 1 line
1.9.1
#9 lore raw

PATCH v3 9/15 net: dsa: Add support for switch EEPROM access

In reply to Guenter Roeck

On some chips it is possible to access the switch eeprom. Add infrastructure support for it.

---
v3:
- Fix bug seen if devicetree is enabled; eeprom-length property is attached to switch devicetree node, not to dsa node.
v2:
- Add support for configuring switch EEPROM size through platform data or devicetree data
- Do not compare new pointers against NULL
- Check if get_eeprom pointer is set before calling it

 include/net/dsa.h | 10 ++++++++++
 net/dsa/dsa.c     |  4 ++++
 net/dsa/slave.c   | 41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+)
include/net/dsa.h +10 -0
index 55e75e7..37856a2 100644
@@ -38,6 +38,9 @@ struct dsa_chip_data {
 	struct device	*host_dev;
 	int		sw_addr;
 
+	/* set to size of eeprom if supported by the switch */
+	int		eeprom_len;
+
 	/* Device tree node pointer for this specific switch chip
 	 * used during switch setup in case additional properties
 	 * and resources needs to be used
@@ -258,6 +261,13 @@ struct dsa_switch_driver {
 	int	(*set_temp_limit)(struct dsa_switch *ds, int temp);
 	int	(*get_temp_alarm)(struct dsa_switch *ds, bool *alarm);
 #endif
+
+	/* EEPROM access */
+	int	(*get_eeprom_len)(struct dsa_switch *ds);
+	int	(*get_eeprom)(struct dsa_switch *ds,
+			      struct ethtool_eeprom *eeprom, u8 *data);
+	int	(*set_eeprom)(struct dsa_switch *ds,
+			      struct ethtool_eeprom *eeprom, u8 *data);
 };
 
 void register_switch_driver(struct dsa_switch_driver *type);
net/dsa/dsa.c +4 -0
index 5edbbca..b51ef592 100644
@@ -575,6 +575,7 @@ static int dsa_of_probe(struct platform_device *pdev)
 	const char *port_name;
 	int chip_index, port_index;
 	const unsigned int *sw_addr, *port_reg;
+	u32 eeprom_len;
 	int ret;
 
 	mdio = of_parse_phandle(np, "dsa,mii-bus", 0);
@@ -626,6 +627,9 @@ static int dsa_of_probe(struct platform_device *pdev)
 		if (cd->sw_addr > PHY_MAX_ADDR)
 			continue;
 
+		if (!of_property_read_u32(np, "eeprom-length", &eeprom_len))
+			cd->eeprom_len = eeprom_len;
+
 		for_each_available_child_of_node(child, port) {
 			port_reg = of_get_property(port, "reg", NULL);
 			if (!port_reg)
net/dsa/slave.c +41 -0

53 lines not on this page · raw message

Signature · 1 line
1.9.1
maintainer
#10 lore raw

Re: [PATCH v3 09/15] net: dsa: Add support for switch EEPROM access

Quoted 4 lines
net/dsa/slave.c +4 -0
+static int dsa_slave_get_eeprom_len(struct net_device *dev)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;

net/dsa/slave.c static u32 dsa_slave_get_link(struct net_device *dev) · line 287

Quoted 10 lines
net/dsa/slave.c +10 -0
+
+	if (ds->pd->eeprom_len)
+		return ds->pd->eeprom_len;
+
+	if (ds->drv->get_eeprom_len)
+		return ds->drv->get_eeprom_len(ds);
+
+	return 0;
+}
+

Hi Guenter

I just started doing some testing with this patchset. A bit late since David just accepted it, but...

root@dir665:~# ethtool -e lan4
Cannot get EEPROM data: Invalid argument
root@dir665:~# ethtool -e eth0
Cannot get EEPROM data: Operation not supported

There is no eeprom for the hardware i'm testing. Operation not supported seems like a better error code and Invalid argument, and is what other network drivers i tried returned.

Andrew

#11 lore raw

Re: [PATCH v3 09/15] net: dsa: Add support for switch EEPROM access

Quoted 20 lines · Andrew Lunn
Quoted 4 lines
net/dsa/slave.c +4 -0
+static int dsa_slave_get_eeprom_len(struct net_device *dev)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;

net/dsa/slave.c static u32 dsa_slave_get_link(struct net_device *dev) · line 287

Quoted 10 lines
net/dsa/slave.c +10 -0
+
+	if (ds->pd->eeprom_len)
+		return ds->pd->eeprom_len;
+
+	if (ds->drv->get_eeprom_len)
+		return ds->drv->get_eeprom_len(ds);
+
+	return 0;
+}
+

Hi Guenter

I just started doing some testing with this patchset. A bit late since David just accepted it, but...

Quoted 9 lines
root@dir665:~# ethtool -e lan4
Cannot get EEPROM data: Invalid argument
root@dir665:~# ethtool -e eth0
Cannot get EEPROM data: Operation not supported

There is no eeprom for the hardware i'm testing. Operation not supported seems like a better error code and Invalid argument, and is what other network drivers i tried returned.

Hi Andrew,

I think the problem is that the infrastructure code (net/core/ethtool.c) does not accept an error from the get_eeprom_len function, but instead assumes that reporting eeprom data is supported if a driver provides the access functions. The get_eeprom_len function returns 0 in your case, which in ethtool_get_any_eeprom() translates to -EINVAL because user space either requests no data or more data than available. I wonder why user space requests anything in the first place; I would have assumed that it reads the driver information first and is told that the eeprom length is 0, but I guess that is a different question.

I quickly browsed through a couple of other drivers supporting get_eprom_len, and they all return 0 if there is no eeprom. Doesn't that mean that they all end up reporting -EINVAL if an attempt is made to read the eeprom ?

The only solution that comes to my mind would be to have the infrastructure code check the return value from get_eeprom_len and return -EOPNOTSUPP if the reported eeprom length is 0. That would be an infrastructure change, though. Does that sound reasonable, or do you have a better idea ?

In parallel, I'll have a look into the ethtool command to see why it requests eeprom data even though the reported eeprom length is 0.

Thanks,
Guenter

#12 lore raw

Re: [PATCH v3 09/15] net: dsa: Add support for switch EEPROM access

Quoted 46 lines · Guenter Roeck
Quoted 20 lines · Andrew Lunn
Quoted 4 lines
(quoted diff) +4 -0
+static int dsa_slave_get_eeprom_len(struct net_device *dev)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;

Review

Quoted 10 lines
(quoted diff) +10 -0
+
+	if (ds->pd->eeprom_len)
+		return ds->pd->eeprom_len;
+
+	if (ds->drv->get_eeprom_len)
+		return ds->drv->get_eeprom_len(ds);
+
+	return 0;
+}
+

Hi Guenter

I just started doing some testing with this patchset. A bit late since David just accepted it, but...

Quoted 9 lines
root@dir665:~# ethtool -e lan4
Cannot get EEPROM data: Invalid argument
root@dir665:~# ethtool -e eth0
Cannot get EEPROM data: Operation not supported

There is no eeprom for the hardware i'm testing. Operation not supported seems like a better error code and Invalid argument, and is what other network drivers i tried returned.

Hi Andrew,

I think the problem is that the infrastructure code (net/core/ethtool.c) does not accept an error from the get_eeprom_len function, but instead assumes that reporting eeprom data is supported if a driver provides the access functions. The get_eeprom_len function returns 0 in your case, which in ethtool_get_any_eeprom() translates to -EINVAL because user space either requests no data or more data than available. I wonder why user space requests anything in the first place; I would have assumed that it reads the driver information first and is told that the eeprom length is 0, but I guess that is a different question.

I quickly browsed through a couple of other drivers supporting get_eprom_len, and they all return 0 if there is no eeprom. Doesn't that mean that they all end up reporting -EINVAL if an attempt is made to read the eeprom ?

Quoted 8 lines

The only solution that comes to my mind would be to have the infrastructure code check the return value from get_eeprom_len and return -EOPNOTSUPP if the reported eeprom length is 0. That would be an infrastructure change, though. Does that sound reasonable, or do you have a better idea ?

In parallel, I'll have a look into the ethtool command to see why it requests eeprom data even though the reported eeprom length is 0.

As suspected, ethtool will attempt to read a zero-length eeprom.

The following patch should solve the problem. Not sure if it is worth it, though, since this will change behavior for existing drivers.

Thanks,
Guenter

---
From: Guenter Roeck <[email protected]>
Date: Thu, 30 Oct 2014 17:51:34 -0700
Subject: [RFC PATCH] net: ethtool: Return -EOPNOTSUPP if user space tries to read
EEPROM with lengh 0

If a driver supports reading EEPROM but no EEPROM is installed in the system, the driver's get_eeprom_len function will return 0. ethtool will subsequently try to read that zero-length EEPROM anyway. If the driver does not support EEPROM access at all, this operation will return -EOPNOTSUPP. If the driver does support EEPROM access but no EEPROM is installed, the operation will return -EINVAL. Return -EOPNOTSUPP in both cases for consistency.

---
 net/core/ethtool.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
net/core/ethtool.c +4 -2
index 1600aa2..06dfb29 100644
@@ -1036,7 +1036,8 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)
 {
 	const struct ethtool_ops *ops = dev->ethtool_ops;
 
-	if (!ops->get_eeprom || !ops->get_eeprom_len)
+	if (!ops->get_eeprom || !ops->get_eeprom_len ||
+	    !ops->get_eeprom_len(dev))
 		return -EOPNOTSUPP;
 
 	return ethtool_get_any_eeprom(dev, useraddr, ops->get_eeprom,
@@ -1052,7 +1053,8 @@ static int ethtool_set_eeprom(struct net_device *dev, void __user *useraddr)
 	u8 *data;
 	int ret = 0;
 
-	if (!ops->set_eeprom || !ops->get_eeprom_len)
+	if (!ops->set_eeprom || !ops->get_eeprom_len ||
+	    !ops->get_eeprom_len(dev))
 		return -EOPNOTSUPP;
 
 	if (copy_from_user(&eeprom, useraddr, sizeof(eeprom)))
Signature · 1 line
1.9.1
maintainer
#13 lore raw

Re: [PATCH v3 09/15] net: dsa: Add support for switch EEPROM access

Quoted 4 lines

As suspected, ethtool will attempt to read a zero-length eeprom.

The following patch should solve the problem. Not sure if it is worth it, though, since this will change behavior for existing drivers.

Yes, it changes behaviour, but it does make it more consistent.

Probably it should be up to core network people to decide if this is the write fix or leave it as is.

Andrew

#14 lore raw

Re: [PATCH v3 09/15] net: dsa: Add support for switch EEPROM access

Quoted 10 lines · Andrew Lunn
Quoted 4 lines

As suspected, ethtool will attempt to read a zero-length eeprom.

The following patch should solve the problem. Not sure if it is worth it, though, since this will change behavior for existing drivers.

Yes, it changes behaviour, but it does make it more consistent.

Probably it should be up to core network people to decide if this is the write fix or leave it as is.

s/write/right/.

The patch shows up in the netdev patchwork. David marked it as RFC, so we'll see where it goes.

Thanks,
Guenter

maintainer
#15 lore raw

Re: [PATCH v3 09/15] net: dsa: Add support for switch EEPROM access

From: Guenter Roeck <[email protected]>
Date: Thu, 30 Oct 2014 19:53:17 -0700

Quoted 9 lines · Andrew Lunn
Quoted 8 lines
Quoted 5 lines

As suspected, ethtool will attempt to read a zero-length eeprom.

The following patch should solve the problem. Not sure if it is worth it,
though, since this will change behavior for existing drivers.

Yes, it changes behaviour, but it does make it more consistent.

Quoted 8 lines
Quoted 3 lines

Probably it should be up to core network people to decide if this is the write fix or leave it as is.

s/write/right/.

The patch shows up in the netdev patchwork. David marked it as RFC, so we'll see where it goes.

As per your patch, I think we should fix this to not return a zero length eeprom when the driver doesn't support it.

If dsa behaves outside of the defined behavior of what ethtool implementations in ethernet drivers do, then it's ok to change it because it's a bonafide bug fix in my eyes.

So if you submit this formally, I would apply it.

Thanks.

#16 lore raw

PATCH v3 10/15 dsa: Add new optional devicetree property to describe EEPROM size

In reply to Guenter Roeck

The dsa core now supports reading from and writing to a switch EEPROM if connected. Describe optional devicetree property indicating that an EEPROM is present and its size.

---
v3:
- eeprom-length property is attached to switch devicetree node, not to dsa node.
v2:
- Added patch

 Documentation/devicetree/bindings/net/dsa/dsa.txt | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/net/dsa/dsa.txt +8 -1
index a62c889..e124847 100644
@@ -10,7 +10,7 @@ Required properties:
 - dsa,ethernet		: Should be a phandle to a valid Ethernet device node
 - dsa,mii-bus		: Should be a phandle to a valid MDIO bus device node
 
-Optionnal properties:
+Optional properties:
 - interrupts		: property with a value describing the switch
 			  interrupt number (not supported by the driver)
 
@@ -23,6 +23,13 @@ Each of these switch child nodes should have the following required properties:
 - #address-cells	: Must be 1
 - #size-cells		: Must be 0
 
+A switch child node has the following optional property:
+
+- eeprom-length		: Set to the length of an EEPROM connected to the
+			  switch. Must be set if the switch can not detect
+			  the presence and/or size of a connected EEPROM,
+			  otherwise optional.
+
 A switch may have multiple "port" children nodes
 
 Each port children node must have the following mandatory properties:
Signature · 1 line
1.9.1
#17 lore raw

PATCH v3 11/15 net: dsa/mv88e6352: Implement EEPROM access functions

In reply to Guenter Roeck

MV88E6352 supports read and write access to its configuration eeprom.

There is no means to detect if an EEPROM is connected to the switch. Also, the switch supports EEPROMs with different sizes, but can not detect or report the type or size of connected EEPROMs. Therefore, do not implement the get_eeprom_len callback but depend on platform or devicetree data to provide information about EEPROM presence and size.

---
v3:
- No change
v2:
- EEPROM length must now be provided through platform or devicetree data

 drivers/net/dsa/mv88e6352.c | 222 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/net/dsa/mv88e6xxx.h |   5 +
 2 files changed, 224 insertions(+), 3 deletions(-)
drivers/net/dsa/mv88e6352.c +219 -3
index 744e6fa..8a956f9 100644
@@ -22,18 +22,18 @@
 #include <net/dsa.h>
 #include "mv88e6xxx.h"
 
-static int mv88e6352_phy_wait(struct dsa_switch *ds)
+static int mv88e6352_wait(struct dsa_switch *ds, int reg, u16 mask)
 {
 	unsigned long timeout = jiffies + HZ / 10;
 
 	while (time_before(jiffies, timeout)) {
 		int ret;
 
-		ret = REG_READ(REG_GLOBAL2, 0x18);
+		ret = REG_READ(REG_GLOBAL2, reg);
 		if (ret < 0)
 			return ret;
 
-		if (!(ret & 0x8000))
+		if (!(ret & mask))
 			return 0;
 
 		usleep_range(1000, 2000);
@@ -41,6 +41,21 @@ static int mv88e6352_phy_wait(struct dsa_switch *ds)
 	return -ETIMEDOUT;
 }
 
+static inline int mv88e6352_phy_wait(struct dsa_switch *ds)
+{
+	return mv88e6352_wait(ds, 0x18, 0x8000);
+}
+
+static inline int mv88e6352_eeprom_load_wait(struct dsa_switch *ds)
+{
+	return mv88e6352_wait(ds, 0x14, 0x0800);
+}
+
+static inline int mv88e6352_eeprom_busy_wait(struct dsa_switch *ds)
+{
+	return mv88e6352_wait(ds, 0x14, 0x8000);
+}
+
 static int __mv88e6352_phy_read(struct dsa_switch *ds, int addr, int regnum)
 {
 	int ret;
@@ -429,6 +444,7 @@ static int mv88e6352_setup(struct dsa_switch *ds)
 	mutex_init(&ps->smi_mutex);
 	mutex_init(&ps->stats_mutex);
 	mutex_init(&ps->phy_mutex);
+	mutex_init(&ps->eeprom_mutex);
 
 	ps->id = REG_READ(REG_PORT(0), 0x03) & 0xfff0;
 
@@ -525,6 +541,204 @@ static struct mv88e6xxx_hw_stat mv88e6352_hw_stats[] = {
 	{ "hist_1024_max_bytes", 4, 0x0d, },
 };
 
+static int mv88e6352_read_eeprom_word(struct dsa_switch *ds, int addr)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int ret;
+
+	mutex_lock(&ps->eeprom_mutex);
+
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, 0x14,
+				  0xc000 | (addr & 0xff));
+	if (ret < 0)
+		goto error;
+
+	ret = mv88e6352_eeprom_busy_wait(ds);
+	if (ret < 0)
+		goto error;
+
+	ret = mv88e6xxx_reg_read(ds, REG_GLOBAL2, 0x15);
+error:
+	mutex_unlock(&ps->eeprom_mutex);
+	return ret;
+}
+
+static int mv88e6352_get_eeprom(struct dsa_switch *ds,
+				struct ethtool_eeprom *eeprom, u8 *data)
+{
+	int offset;
+	int len;
+	int ret;
+
+	offset = eeprom->offset;
+	len = eeprom->len;
+	eeprom->len = 0;
+
+	eeprom->magic = 0xc3ec4951;
+
+	ret = mv88e6352_eeprom_load_wait(ds);
+	if (ret < 0)
+		return ret;
+
+	if (offset & 1) {
+		int word;
+
+		word = mv88e6352_read_eeprom_word(ds, offset >> 1);
+		if (word < 0)
+			return word;
+
+		*data++ = (word >> 8) & 0xff;
+
+		offset++;
+		len--;
+		eeprom->len++;
+	}
+
+	while (len >= 2) {
+		int word;
+
+		word = mv88e6352_read_eeprom_word(ds, offset >> 1);
+		if (word < 0)
+			return word;
+
+		*data++ = word & 0xff;
+		*data++ = (word >> 8) & 0xff;
+
+		offset += 2;
+		len -= 2;
+		eeprom->len += 2;
+	}
+
+	if (len) {
+		int word;
+
+		word = mv88e6352_read_eeprom_word(ds, offset >> 1);
+		if (word < 0)
+			return word;
+
+		*data++ = word & 0xff;
+
+		offset++;
+		len--;
+		eeprom->len++;
+	}
+
+	return 0;
+}
+
+static int mv88e6352_eeprom_is_readonly(struct dsa_switch *ds)
+{
+	int ret;
+
+	ret = mv88e6xxx_reg_read(ds, REG_GLOBAL2, 0x14);
+	if (ret < 0)
+		return ret;
+
+	if (!(ret & 0x0400))
+		return -EROFS;
+
+	return 0;
+}
+
+static int mv88e6352_write_eeprom_word(struct dsa_switch *ds, int addr,
+				       u16 data)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	int ret;
+
+	mutex_lock(&ps->eeprom_mutex);
+
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, 0x15, data);
+	if (ret < 0)
+		goto error;
+
+	ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, 0x14,
+				  0xb000 | (addr & 0xff));
+	if (ret < 0)
+		goto error;
+
+	ret = mv88e6352_eeprom_busy_wait(ds);
+error:
+	mutex_unlock(&ps->eeprom_mutex);
+	return ret;
+}
+
+static int mv88e6352_set_eeprom(struct dsa_switch *ds,
+				struct ethtool_eeprom *eeprom, u8 *data)
+{
+	int offset;
+	int ret;
+	int len;
+
+	if (eeprom->magic != 0xc3ec4951)
+		return -EINVAL;
+
+	ret = mv88e6352_eeprom_is_readonly(ds);
+	if (ret)
+		return ret;
+
+	offset = eeprom->offset;
+	len = eeprom->len;
+	eeprom->len = 0;
+
+	ret = mv88e6352_eeprom_load_wait(ds);
+	if (ret < 0)
+		return ret;
+
+	if (offset & 1) {
+		int word;
+
+		word = mv88e6352_read_eeprom_word(ds, offset >> 1);
+		if (word < 0)
+			return word;
+
+		word = (*data++ << 8) | (word & 0xff);
+
+		ret = mv88e6352_write_eeprom_word(ds, offset >> 1, word);
+		if (ret < 0)
+			return ret;
+
+		offset++;
+		len--;
+		eeprom->len++;
+	}
+
+	while (len >= 2) {
+		int word;
+
+		word = *data++;
+		word |= *data++ << 8;
+
+		ret = mv88e6352_write_eeprom_word(ds, offset >> 1, word);
+		if (ret < 0)
+			return ret;
+
+		offset += 2;
+		len -= 2;
+		eeprom->len += 2;
+	}
+
+	if (len) {
+		int word;
+
+		word = mv88e6352_read_eeprom_word(ds, offset >> 1);
+		if (word < 0)
+			return word;
+
+		word = (word & 0xff00) | *data++;
+
+		ret = mv88e6352_write_eeprom_word(ds, offset >> 1, word);
+		if (ret < 0)
+			return ret;
+
+		offset++;
+		len--;
+		eeprom->len++;
+	}
+
+	return 0;
+}
+
 static void
 mv88e6352_get_strings(struct dsa_switch *ds, int port, uint8_t *data)
 {
@@ -562,6 +776,8 @@ struct dsa_switch_driver mv88e6352_switch_driver = {
 	.set_temp_limit		= mv88e6352_set_temp_limit,
 	.get_temp_alarm		= mv88e6352_get_temp_alarm,
 #endif
+	.get_eeprom		= mv88e6352_get_eeprom,
+	.set_eeprom		= mv88e6352_set_eeprom,
 };
 
 MODULE_ALIAS("platform:mv88e6352");
drivers/net/dsa/mv88e6xxx.h +5 -0
index c0ce133..29feed0 100644
@@ -43,6 +43,11 @@ struct mv88e6xxx_priv_state {
 	 */
 	struct mutex	phy_mutex;
 
+	/* This mutex serializes eeprom access for chips with
+	 * eeprom support.
+	 */
+	struct mutex eeprom_mutex;
+
 	int		id; /* switch product id */
 };
 
Signature · 1 line
1.9.1
#18 lore raw

PATCH v3 12/15 net: dsa: Add support for reading switch registers with ethtool

In reply to Guenter Roeck

Add support for reading switch registers with 'ethtool -d'.

---
v3:
- No change
v2:
- Do not compare new function pointers against NULL
- Check if get_regs is set before calling it

 include/net/dsa.h |  7 +++++++
 net/dsa/slave.c   | 23 +++++++++++++++++++++++
 2 files changed, 30 insertions(+)
include/net/dsa.h +7 -0
index 37856a2..ed3c34b 100644
@@ -268,6 +268,13 @@ struct dsa_switch_driver {
 			      struct ethtool_eeprom *eeprom, u8 *data);
 	int	(*set_eeprom)(struct dsa_switch *ds,
 			      struct ethtool_eeprom *eeprom, u8 *data);
+
+	/*
+	 * Register access.
+	 */
+	int	(*get_regs_len)(struct dsa_switch *ds, int port);
+	void	(*get_regs)(struct dsa_switch *ds, int port,
+			    struct ethtool_regs *regs, void *p);
 };
 
 void register_switch_driver(struct dsa_switch_driver *type);
net/dsa/slave.c +23 -0
index ff2fbe7..474f296 100644
@@ -249,6 +249,27 @@ static void dsa_slave_get_drvinfo(struct net_device *dev,
 	strlcpy(drvinfo->bus_info, "platform", sizeof(drvinfo->bus_info));
 }
 
+static int dsa_slave_get_regs_len(struct net_device *dev)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;
+
+	if (ds->drv->get_regs_len)
+		return ds->drv->get_regs_len(ds, p->port);
+
+	return -EOPNOTSUPP;
+}
+
+static void
+dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	struct dsa_switch *ds = p->parent;
+
+	if (ds->drv->get_regs)
+		ds->drv->get_regs(ds, p->port, regs, _p);
+}
+
 static int dsa_slave_nway_reset(struct net_device *dev)
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
@@ -423,6 +444,8 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
 	.get_settings		= dsa_slave_get_settings,
 	.set_settings		= dsa_slave_set_settings,
 	.get_drvinfo		= dsa_slave_get_drvinfo,
+	.get_regs_len		= dsa_slave_get_regs_len,
+	.get_regs		= dsa_slave_get_regs,
 	.nway_reset		= dsa_slave_nway_reset,
 	.get_link		= dsa_slave_get_link,
 	.get_eeprom_len		= dsa_slave_get_eeprom_len,
Signature · 1 line
1.9.1
#19 lore raw

PATCH v3 13/15 net: dsa/mv88e6123_61_65: Add support for reading switch registers

In reply to Guenter Roeck

The infrastructure can now report switch registers to ethtool. Add support for it to the mv88e6123_61_65 driver.

---
v3:
- No change
v2:
- No change

 drivers/net/dsa/mv88e6123_61_65.c |  2 ++
 drivers/net/dsa/mv88e6xxx.c       | 24 ++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.h       |  3 +++
 3 files changed, 29 insertions(+)
drivers/net/dsa/mv88e6123_61_65.c +2 -0
index 9f43c9b..e2d2e37 100644
@@ -470,6 +470,8 @@ struct dsa_switch_driver mv88e6123_61_65_switch_driver = {
 #ifdef CONFIG_NET_DSA_HWMON
 	.get_temp		= mv88e6123_61_65_get_temp,
 #endif
+	.get_regs_len		= mv88e6xxx_get_regs_len,
+	.get_regs		= mv88e6xxx_get_regs,
 };
 
 MODULE_ALIAS("platform:mv88e6123");
drivers/net/dsa/mv88e6xxx.c +24 -0
index 8e1090b..c071fde 100644
@@ -499,6 +499,30 @@ void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds,
 	mutex_unlock(&ps->stats_mutex);
 }
 
+int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port)
+{
+	return 32 * sizeof(u16);
+}
+
+void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
+			struct ethtool_regs *regs, void *_p)
+{
+	u16 *p = _p;
+	int i;
+
+	regs->version = 0;
+
+	memset(p, 0xff, 32 * sizeof(u16));
+
+	for (i = 0; i < 32; i++) {
+		int ret;
+
+		ret = mv88e6xxx_reg_read(ds, REG_PORT(port), i);
+		if (ret >= 0)
+			p[i] = ret;
+	}
+}
+
 static int __init mv88e6xxx_init(void)
 {
 #if IS_ENABLED(CONFIG_NET_DSA_MV88E6131)
drivers/net/dsa/mv88e6xxx.h +3 -0
index 29feed0..a0780b0 100644
@@ -78,6 +78,9 @@ void mv88e6xxx_get_strings(struct dsa_switch *ds,
 void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds,
 				 int nr_stats, struct mv88e6xxx_hw_stat *stats,
 				 int port, uint64_t *data);
+int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port);
+void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
+			struct ethtool_regs *regs, void *_p);
 
 extern struct dsa_switch_driver mv88e6131_switch_driver;
 extern struct dsa_switch_driver mv88e6123_61_65_switch_driver;
Signature · 1 line
1.9.1
#20 lore raw

PATCH v3 14/15 net: dsa/mv88e6352: Add support for reading switch registers

In reply to Guenter Roeck

Report switch register values to ethtool.

---
v3:
- No change
v2:
- No change

 drivers/net/dsa/mv88e6352.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/dsa/mv88e6352.c +2 -0
index 8a956f9..d5bbe49 100644
@@ -778,6 +778,8 @@ struct dsa_switch_driver mv88e6352_switch_driver = {
 #endif
 	.get_eeprom		= mv88e6352_get_eeprom,
 	.set_eeprom		= mv88e6352_set_eeprom,
+	.get_regs_len		= mv88e6xxx_get_regs_len,
+	.get_regs		= mv88e6xxx_get_regs,
 };
 
 MODULE_ALIAS("platform:mv88e6352");
Signature · 1 line
1.9.1
#21 lore raw

PATCH v3 15/15 net: dsa: Provide additional RMON statistics

In reply to Guenter Roeck

Display sw_in_discards, sw_in_filtered, and sw_out_filtered for chips supported by mv88e6123_61_65 and mv88e6352 drivers.

The variables are provided in port registers, not the normal status registers. Mark by adding 0x100 to the register offset and add special handling code to mv88e6xxx_get_ethtool_stats.

---
v3:
- No change
v2:
- No change

 drivers/net/dsa/mv88e6123_61_65.c |  3 +++
 drivers/net/dsa/mv88e6352.c       |  3 +++
 drivers/net/dsa/mv88e6xxx.c       | 26 +++++++++++++++++++++-----
 3 files changed, 27 insertions(+), 5 deletions(-)
drivers/net/dsa/mv88e6123_61_65.c +3 -0
index e2d2e37..9a3f9e0 100644
@@ -433,6 +433,9 @@ static struct mv88e6xxx_hw_stat mv88e6123_61_65_hw_stats[] = {
 	{ "hist_256_511bytes", 4, 0x0b, },
 	{ "hist_512_1023bytes", 4, 0x0c, },
 	{ "hist_1024_max_bytes", 4, 0x0d, },
+	{ "sw_in_discards", 4, 0x110, },
+	{ "sw_in_filtered", 2, 0x112, },
+	{ "sw_out_filtered", 2, 0x113, },
 };
 
 static void
drivers/net/dsa/mv88e6352.c +3 -0
index d5bbe49..258d9ef 100644
@@ -539,6 +539,9 @@ static struct mv88e6xxx_hw_stat mv88e6352_hw_stats[] = {
 	{ "hist_256_511bytes", 4, 0x0b, },
 	{ "hist_512_1023bytes", 4, 0x0c, },
 	{ "hist_1024_max_bytes", 4, 0x0d, },
+	{ "sw_in_discards", 4, 0x110, },
+	{ "sw_in_filtered", 2, 0x112, },
+	{ "sw_out_filtered", 2, 0x113, },
 };
 
 static int mv88e6352_read_eeprom_word(struct dsa_switch *ds, int addr)
drivers/net/dsa/mv88e6xxx.c +21 -5
index c071fde..da558d8 100644
@@ -485,17 +485,33 @@ void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds,
 	for (i = 0; i < nr_stats; i++) {
 		struct mv88e6xxx_hw_stat *s = stats + i;
 		u32 low;
-		u32 high;
-
+		u32 high = 0;
+
+		if (s->reg >= 0x100) {
+			int ret;
+
+			ret = mv88e6xxx_reg_read(ds, REG_PORT(port),
+						 s->reg - 0x100);
+			if (ret < 0)
+				goto error;
+			low = ret;
+			if (s->sizeof_stat == 4) {
+				ret = mv88e6xxx_reg_read(ds, REG_PORT(port),
+							 s->reg - 0x100 + 1);
+				if (ret < 0)
+					goto error;
+				high = ret;
+			}
+			data[i] = (((u64)high) << 16) | low;
+			continue;
+		}
 		mv88e6xxx_stats_read(ds, s->reg, &low);
 		if (s->sizeof_stat == 8)
 			mv88e6xxx_stats_read(ds, s->reg + 1, &high);
-		else
-			high = 0;
 
 		data[i] = (((u64)high) << 32) | low;
 	}
-
+error:
 	mutex_unlock(&ps->stats_mutex);
 }
 
Signature · 1 line
1.9.1
Reviewed-by on the v3 series
#22 lore raw
In reply to Guenter Roeck
Quoted 27 lines · Guenter Roeck

Patch 01/15 addresses a bug indicated by an an annoying and unhelpful log message.

Patches 02/15 and 03/15 are minor enhancements, adding support for known switch revisions.

Patches 04/15 and 05/15 add support for MV88E6352 and MV88E6176.

Patch 06/15 adds support for hardware monitoring, specifically for reporting the chip temperature, to the dsa subsystem.

Patches 07/15 and 08/15 implement hardware monitoring for MV88E6352, MV88E6176, MV88E6123, MV88E6161, and MV88E6165.

Patch 09/15 and 10/15 add support for EEPROM access to the DSA subsystem.

Patch 11/15 implements EEPROM access for MV88E6352 and MV88E6176.

Patch 12/15 adds support for reading switch registers to the DSA subsystem.

Patches 13/15 amd 14/15 implement support for reading switch registers to the drivers for MV88E6352, MV88E6176, MV88E6123, MV88E6161, and MV88E6165.

Patch 15/15 adds support for reading additional RMON registers to the drivers for MV88E6352, MV88E6176, MV88E6123, MV88E6161, and MV88E6165.

Quoted 7 lines

The series was tested on top of v3.18-rc2 in an x86 system with MV88E6352. Testing in systems with 88E6131, 88E6060 and MV88E6165 was done earlier (I don't have access to those systems right now). The series was also build tested using my build system at http://server.roeck-us.net:8010/builders.
Look into the 'dsa' column for build results.

The series merges cleanly into net-next as of today (10/29).

Thanks Guenter!

Quoted 64 lines

v3:
- Fix bug in eeprom patches seen if devicetree is enabled:
eeprom-length property is attached to switch devicetree node, not to dsa node, and there was a compile error.
v2:
- Made reporting chip temperatures through the hwmon subsystem optional with new Kconfig option
- Changed the hwmon chip name to <network device name>_dsa<index>
- Made EEPROM presence and size configurable through platform and devicetree data
- Various minor changes and fixes (see individual patches for details)

----------------------------------------------------------------
The following changes since commit cac7f2429872d3733dc3f9915857b1691da2eb2f:

Linux 3.18-rc2 (2014-10-26 16:48:41 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git dsa

for you to fetch changes up to 245472a4f3ca2a68c5632f2c37da41aa2d3fb2f0:

net: dsa: Provide additional RMON statistics (2014-10-29 08:55:37 -0700)

----------------------------------------------------------------
Guenter Roeck (15):
net: dsa: Don't set skb->protocol on outgoing tagged packets
net: dsa: Report known silicon revisions for Marvell 88E6060
net: dsa: Report known silicon revisions for Marvell 88E6131
net: dsa: Add support for Marvell 88E6352
net: dsa/mv88e6352: Add support for MV88E6176
net: dsa: Add support for reporting switch chip temperatures
net: dsa/mv88e6352: Report chip temperature
net: dsa/mv88e6123_61_65: Report chip temperature
net: dsa: Add support for switch EEPROM access
dsa: Add new optional devicetree property to describe EEPROM size
net: dsa/mv88e6352: Implement EEPROM access functions
net: dsa: Add support for reading switch registers with ethtool
net: dsa/mv88e6123_61_65: Add support for reading switch registers
net: dsa/mv88e6352: Add support for reading switch registers
net: dsa: Provide additional RMON statistics

 Documentation/devicetree/bindings/net/dsa/dsa.txt |   9 +-
 MAINTAINERS                                       |   5 +
 drivers/net/dsa/Kconfig                           |   9 +
 drivers/net/dsa/Makefile                          |   3 +
 drivers/net/dsa/mv88e6060.c                       |   5 +-
 drivers/net/dsa/mv88e6123_61_65.c                 |  73 +-
 drivers/net/dsa/mv88e6131.c                       |  12 +-
 drivers/net/dsa/mv88e6352.c                       | 788 ++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx.c                       |  53 +-
 drivers/net/dsa/mv88e6xxx.h                       |  15 +
 include/net/dsa.h                                 |  33 +
 net/dsa/Kconfig                                   |  11 +
 net/dsa/dsa.c                                     | 135 ++++
 net/dsa/slave.c                                   |  64 ++
 net/dsa/tag_dsa.c                                 |   2 -
 net/dsa/tag_edsa.c                                |   2 -
 net/dsa/tag_trailer.c                             |   2 -
 17 files changed, 1202 insertions(+), 19 deletions(-)
 create mode 100644 drivers/net/dsa/mv88e6352.c
#23 lore raw
Quoted 34 lines · Florian Fainelli
Quoted 33 lines · Guenter Roeck

Patch 01/15 addresses a bug indicated by an an annoying and unhelpful log message.

Patches 02/15 and 03/15 are minor enhancements, adding support for known switch revisions.

Patches 04/15 and 05/15 add support for MV88E6352 and MV88E6176.

Patch 06/15 adds support for hardware monitoring, specifically for reporting the chip temperature, to the dsa subsystem.

Patches 07/15 and 08/15 implement hardware monitoring for MV88E6352, MV88E6176, MV88E6123, MV88E6161, and MV88E6165.

Patch 09/15 and 10/15 add support for EEPROM access to the DSA subsystem.

Patch 11/15 implements EEPROM access for MV88E6352 and MV88E6176.

Patch 12/15 adds support for reading switch registers to the DSA subsystem.

Patches 13/15 amd 14/15 implement support for reading switch registers to the drivers for MV88E6352, MV88E6176, MV88E6123, MV88E6161, and MV88E6165.

Patch 15/15 adds support for reading additional RMON registers to the drivers for MV88E6352, MV88E6176, MV88E6123, MV88E6161, and MV88E6165.

The series was tested on top of v3.18-rc2 in an x86 system with MV88E6352. Testing in systems with 88E6131, 88E6060 and MV88E6165 was done earlier (I don't have access to those systems right now). The series was also build tested using my build system at http://server.roeck-us.net:8010/builders.
Look into the 'dsa' column for build results.

Quoted 6 lines
Quoted 1 line

The series merges cleanly into net-next as of today (10/29).

Thanks Guenter!

Thanks a lot for the review!

Guenter