net: dsa: Fixes and enhancements
Subsystems (4)
v2 cover net: dsa: Fixes and enhancements
Patch 01/15 addresses 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 resides and was tested on top of v3.18-rc1 in a 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 applies cleanly to net-next as of today (10/26).
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)
----------------------------------------------------------------
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 | 6 +- 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, 1199 insertions(+), 19 deletions(-) create mode 100644 drivers/net/dsa/mv88e6352.c
PATCH v2 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. Drop it.
---
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
@@ -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
@@ -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
@@ -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
Re: [PATCH v2 01/15] net: dsa: Don't set skb->protocol on outgoing tagged packets
Quoted 8 lines · Guenter Roeck
Setting
skb->protocolto a private protocol type may result in warning messages such ase1000e 0000:00:19.0 em1: checksum_partial proto=dada!This happens if the L3 protocol is IP or IPv6 and
skb->ip_summedis set toCHECKSUM_PARTIAL. Looking through the code, it appears that changingskb->protocolfor transmitted packets is not necessary and may actually be harmful. Drop it.
You could emphasize that overriding skb->protocol prevents purposely unmodified (from a DSA perspective) network drivers from properly setting up their transmit checksum offload pointers since they inspect skb->protocol to setup the IPv4 header or IPv6 header pointers.
Quoted 51 lines
Signed-off-by Guenter Roeck <[email protected]>---
v2:
- No changenet/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@@ -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@@ -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@@ -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);
Re: [PATCH v2 01/15] net: dsa: Don't set skb->protocol on outgoing tagged packets
Quoted 10 lines · Florian Fainelli
Quoted 8 lines · Guenter Roeck
Setting
skb->protocolto a private protocol type may result in warning messages such ase1000e 0000:00:19.0 em1: checksum_partial proto=dada!This happens if the L3 protocol is IP or IPv6 and
skb->ip_summedis set toCHECKSUM_PARTIAL. Looking through the code, it appears that changingskb->protocolfor transmitted packets is not necessary and may actually be harmful. Drop it.
Quoted 5 lines
You could emphasize that overriding
skb->protocolprevents purposely unmodified (from a DSA perspective) network drivers from properly setting up their transmit checksum offload pointers since they inspectskb->protocolto setup the IPv4 header or IPv6 header pointers.
Ok, I'll do that.
Thanks,
Guenter
PATCH v2 2/15 net: dsa: Report known silicon revisions for Marvell 88E6060
In reply to Guenter RoeckReport known silicon revisions when probing Marvell 88E6060 switches.
---
v2:
- No change
drivers/net/dsa/mv88e6060.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/net/dsa/mv88e6060.c +4 -1
@@ -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
PATCH v2 3/15 net: dsa: Report known silicon revisions for Marvell 88E6131
In reply to Guenter RoeckReport known silicon revisions when probing Marvell 88E6131 switches.
---
v2:
- No change
drivers/net/dsa/mv88e6131.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/net/dsa/mv88e6131.c +8 -4
@@ -21,6 +21,7 @@#define ID_6085 0x04a0#define ID_6095 0x0950#define ID_6131 0x1060 +#define ID_6131_B2 0x1066static 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
PATCH v2 4/15 net: dsa: Add support for Marvell 88E6352
In reply to Guenter RoeckMarvell 88E6352 is mostly compatible to MV88E6123/61/65, but requires indirect phy access. Also, its configuration registers are a bit different.
---
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
@@ -5845,6 +5845,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
@@ -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
@@ -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
@@ -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
@@ -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
PATCH v2 5/15 net: dsa/mv88e6352: Add support for MV88E6176
In reply to Guenter RoeckMV88E6176 is mostly compatible to MV88E6352 and is documented in the same functional specification. Add support for it.
---
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
@@ -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
@@ -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
PATCH v2 6/15 net: dsa: Add support for reporting switch chip temperatures
In reply to Guenter RoeckSome switches provide chip temperature data.
Add support for reporting it through the hwmon subsystem.
---
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
@@ -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
@@ -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
PATCH v2 7/15 net: dsa/mv88e6352: Report chip temperature
In reply to Guenter RoeckMV88E6352 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.
---
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
@@ -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
PATCH v2 8/15 net: dsa/mv88e6123_61_65: Report chip temperature
In reply to Guenter RoeckMV88E6123 and compatible chips support reading the chip temperature from PHY register 6:26.
---
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
@@ -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
PATCH v2 9/15 net: dsa: Add support for switch EEPROM access
In reply to Guenter RoeckOn some chips it is possible to access the switch eeprom. Add infrastructure support for it.
---
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
@@ -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
@@ -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_length;int ret; mdio =of_parse_phandle (np,"dsa,mii-bus" ,0 );
@@ -603,6 +604,9 @@ static int dsa_of_probe(struct platform_device *pdev)if (pd->nr_chips >DSA_MAX_SWITCHES ) pd->nr_chips =DSA_MAX_SWITCHES ; +if (!of_property_read_u32 (np,"eeprom-length" , &eeprom_length)) + pd->eeprom_length = eeprom_length; + pd->chip =kzalloc (pd->nr_chips *sizeof (struct dsa_chip_data ),GFP_KERNEL );if (!pd->chip) {
net/dsa/slave.c +41 -0
53 lines not on this page · raw message
Signature · 1 line
1.9.1
Andrew Lunn
maintainerRe: [PATCH v2 09/15] net: dsa: Add support for switch EEPROM access
Quoted 57 lines · Guenter Roeck
On some chips it is possible to access the switch eeprom. Add infrastructure support for it.
Signed-off-by Guenter Roeck <[email protected]>---
v2:
- Add support for configuring switch EEPROM size through platform data or devicetree data
- Do not compare new pointers against NULL
- Check ifget_eeprompointer is set before calling itinclude/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@@ -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 +1
-0@@ -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_length;int ret; mdio =of_parse_phandle (np,"dsa,mii-bus" ,0 );
net/dsa/dsa.c static int dsa_of_probe(struct platform_device *pdev) · line 609
Quoted 6 lines
net/dsa/dsa.c +3
-0@@ -603,6 +604,9 @@ static int dsa_of_probe(struct platform_device *pdev)if (pd->nr_chips >DSA_MAX_SWITCHES ) pd->nr_chips =DSA_MAX_SWITCHES ; +if (!of_property_read_u32 (np,"eeprom-length" , &eeprom_length)) + pd->eeprom_length = eeprom_length; +
Hi Guenter
I would of expected a property to indicate the eeprom is present, not a length value. The switch determines the length, or at least the minimum length. So the switch driver can return the length, if the eeprom is indicated to be present.
Also, all device tree properties need to be documented. I've not looked through all the patches, so maybe it is in a separate patch?
net/dsa/dsa.c static int dsa_of_probe(struct platform_device *pdev)
Quoted 13 lines
net/dsa/dsa.c +13
-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; + +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 ; +}
This also does not look right. The default is that there is no property in DT, meaning the EEPROM is not present. So ds->pd->eeprom_len is zero. So the first if is not taken. It then calls into the driver which is return a length, independent of if it is populated or not.
If we are going with your suggested binding, no value in DT, or a value of 0 mean no EEPROM. If there is a value in DT dsa_slave_get_eeprom_len() should return that value. Asking the driver is redundant, you can remove the get_eeprom_len() call.
However, if DT just indicates the eeprom is populated or not, you should call the driver get_eeprom_len() is the eeprom property is present.
net/dsa/dsa.c
Quoted 8 lines
net/dsa/dsa.c +8
-0+static int dsa_slave_get_eeprom (struct net_device *dev, +struct ethtool_eeprom *eeprom,u8 *data) +{ +struct dsa_slave_priv *p =netdev_priv (dev); +struct dsa_switch *ds = p->parent; + +if (ds->drv->get_eeprom) +return ds->drv->get_eeprom (ds, eeprom, data);
This should be conditional on the length/present.
net/dsa/dsa.c
Quoted 12 lines
net/dsa/dsa.c +12
-0+ +return -EOPNOTSUPP ; +} + +static int dsa_slave_set_eeprom (struct net_device *dev, +struct ethtool_eeprom *eeprom,u8 *data) +{ +struct dsa_slave_priv *p =netdev_priv (dev); +struct dsa_switch *ds = p->parent; + +if (ds->drv->set_eeprom) +return ds->drv->set_eeprom (ds, eeprom, data);
Same again.
Andrew
Quoted 19 lines
net/dsa/dsa.c +6
-0+return -EOPNOTSUPP ; +} +static void dsa_slave_get_strings (struct net_device *dev,uint32_t stringset,uint8_t *data) {@@ -387,6 +425,9 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = { .get_drvinfo = dsa_slave_get_drvinfo, .nway_reset = dsa_slave_nway_reset, .get_link = dsa_slave_get_link, + .get_eeprom_len = dsa_slave_get_eeprom_len, + .get_eeprom = dsa_slave_get_eeprom, + .set_eeprom = dsa_slave_set_eeprom, .get_strings = dsa_slave_get_strings, .get_ethtool_stats = dsa_slave_get_ethtool_stats, .get_sset_count = dsa_slave_get_sset_count,Signature · 1 line
1.9.1
Re: [PATCH v2 09/15] net: dsa: Add support for switch EEPROM access
Quoted 67 lines · Andrew Lunn
Quoted 57 lines · Guenter Roeck
On some chips it is possible to access the switch eeprom. Add infrastructure support for it.
Signed-off-by Guenter Roeck <[email protected]>---
v2:
- Add support for configuring switch EEPROM size through platform data or devicetree data
- Do not compare new pointers against NULL
- Check ifget_eeprompointer is set before calling itinclude/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@@ -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 +1
-0@@ -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_length;int ret; mdio =of_parse_phandle (np,"dsa,mii-bus" ,0 );net/dsa/dsa.c static int dsa_of_probe(struct platform_device *pdev) · line 609
Quoted 6 lines
net/dsa/dsa.c +3
-0@@ -603,6 +604,9 @@ static int dsa_of_probe(struct platform_device *pdev)if (pd->nr_chips >DSA_MAX_SWITCHES ) pd->nr_chips =DSA_MAX_SWITCHES ; +if (!of_property_read_u32 (np,"eeprom-length" , &eeprom_length)) + pd->eeprom_length = eeprom_length; +Hi Guenter
Quoted 5 lines
I would of expected a property to indicate the eeprom is present, not a length value. The switch determines the length, or at least the minimum length. So the switch driver can return the length, if the eeprom is indicated to be present.
MV88E6352 and compatible chips can neither provide presence nor length information, so both have to come from platform data or devicetree.
Quoted 3 lines
Also, all device tree properties need to be documented. I've not looked through all the patches, so maybe it is in a separate patch?
The documentation is in the next patch.
I merged both presence and length into one optional property, following the logic that providing a length implies presence and that presence without length doesn't really provide the required information.
I also wanted to give drivers a chance to indicate presence and length without the need of devicetree properties or platform data, if a chip can provide that information.
Also, the only indication to the calling code (ethtool) that there is not eeprom is through the length field, so I figured that a separate "presence" field would, for all practical purpose, be just noise and not be of practical value.
Quoted 11 lines
Quoted 11 lines
net/dsa/dsa.c +11
-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; + +if (ds->pd->eeprom_len) +return ds->pd->eeprom_len; + +if (ds->drv->get_eeprom_len) +return ds->drv->get_eeprom_len (ds); +
Quoted 9 lines
net/dsa/dsa.c
Quoted 2 lines
net/dsa/dsa.c +2
-0+return 0 ; +}This also does not look right. The default is that there is no property in DT, meaning the EEPROM is not present. So
ds->pd->eeprom_lenis zero. So the first if is not taken. It then calls into the driver which is return a length, independent of if it is populated or not.
No property was supposed to mean that the information about presence and length is not provided through devicetree or platform data, but must be provided by the driver. Devicetree and platform data were meant to augment auto-detection of presence and length, not to replace it.
Quoted 5 lines
If we are going with your suggested binding, no value in DT, or a value of 0 mean no EEPROM. If there is a value in DT
dsa_slave_get_eeprom_len()should return that value. Asking the driver is redundant, you can remove theget_eeprom_len()call.
Again, the MV chips can not provide length or presence information. That was the whole point of having platform / devicetree data.
Quoted 4 lines
However, if DT just indicates the eeprom is populated or not, you should call the driver
get_eeprom_len()is the eeprom property is present.
Guess I was trying to be too intelligent.
So lets take a step back: For the Marvell chips, I have to provide both length and presence in devicetree or platform data. Presence seemed to be implied by length, so I used only a single property and variable to indicate both.
Are you asking me to make that mandatory, ie to have presence and length _always_ be provided by devicetree or platform data ? Sure, I can do that and simply drop the get_length dsa callback function.
Also, it seems that you request two separate properties, one for presence and another for length. Is that correct ? Again, I thought that would not provide any value since presence is indicated by length != 0 in the ethtool callback function. No problem, though, I'll be happy to create two separate properties and platform data variables if you think that would be better.
Quoted 26 lines
net/dsa/dsa.c
Quoted 8 lines
net/dsa/dsa.c +8
-0+static int dsa_slave_get_eeprom (struct net_device *dev, +struct ethtool_eeprom *eeprom,u8 *data) +{ +struct dsa_slave_priv *p =netdev_priv (dev); +struct dsa_switch *ds = p->parent; + +if (ds->drv->get_eeprom) +return ds->drv->get_eeprom (ds, eeprom, data);This should be conditional on the length/present.
net/dsa/dsa.c
Quoted 12 lines
net/dsa/dsa.c +12
-0+ +return -EOPNOTSUPP ; +} + +static int dsa_slave_set_eeprom (struct net_device *dev, +struct ethtool_eeprom *eeprom,u8 *data) +{ +struct dsa_slave_priv *p =netdev_priv (dev); +struct dsa_switch *ds = p->parent; + +if (ds->drv->set_eeprom) +return ds->drv->set_eeprom (ds, eeprom, data);Same again.
Quoted 2 lines
Andrew
Sure, no problem.
Thanks,
Guenter
Quoted 21 lines
Quoted 20 lines
net/dsa/dsa.c +6
-1+return -EOPNOTSUPP ; +} +static void dsa_slave_get_strings (struct net_device *dev,uint32_t stringset,uint8_t *data) {@@ -387,6 +425,9 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = { .get_drvinfo = dsa_slave_get_drvinfo, .nway_reset = dsa_slave_nway_reset, .get_link = dsa_slave_get_link, + .get_eeprom_len = dsa_slave_get_eeprom_len, + .get_eeprom = dsa_slave_get_eeprom, + .set_eeprom = dsa_slave_set_eeprom, .get_strings = dsa_slave_get_strings, .get_ethtool_stats = dsa_slave_get_ethtool_stats, .get_sset_count = dsa_slave_get_sset_count, --1.9.1
Re: [PATCH v2 09/15] net: dsa: Add support for switch EEPROM access
Quoted 6 lines · Guenter Roeck
Also, it seems that you request two separate properties, one for presence and another for length. Is that correct ? Again, I thought that would not provide any value since presence is indicated by length != 0 in the ethtool callback function. No problem, though, I'll be happy to create two separate properties and platform data variables if you think that would be better.
The fewer properties, the better.
Thanks,
Richard
Re: [PATCH v2 09/15] net: dsa: Add support for switch EEPROM access
Quoted 10 lines · Richard Cochran
Quoted 6 lines · Guenter Roeck
Also, it seems that you request two separate properties, one for presence and another for length. Is that correct ? Again, I thought that would not provide any value since presence is indicated by length != 0 in the ethtool callback function. No problem, though, I'll be happy to create two separate properties and platform data variables if you think that would be better.
The fewer properties, the better.
Right now I have:
Optional properties:
- 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.
and I think Andrew is asking for the following:
Optional properties:
- eeprom-present : Boolean property indicating that an EEPROM is present.
Must be set if an EEPROM is present.
- eeprom-length : Set to the length of an EEPROM connected to the
switch. Must be set if the switch can not detect the size of a connected EEPROM, otherwise optional.
Platform data semantics would be the same.
I can go either way, but I would like to get some kind of agreement before I jump into writing the code.
Thanks,
Guenter
Re: [PATCH v2 09/15] net: dsa: Add support for switch EEPROM access
In reply to Guenter RoeckQuoted 3 lines
So lets take a step back: For the Marvell chips, I have to provide both length and presence in devicetree or platform data. Presence seemed to be implied by length, so I used only a single property and variable to indicate both.
Hi Guenter
What i was thinking, is that you don't need length in device tree. The datasheet specifies how big the EEPROM needs to be.
However, i read the datasheet for the 6060, the only public datasheet from Marvell. It does not work as i expected. Rather than being a fixed list of register values, it is a variable length list of command/value pairs.
In this situation, yes, you do need the length in DT.
I had a quick look at some other switch chips. e.g. the RTL8100. It has a fixed layout of its EEPROM, consisting of 0x80 bytes. In this case, the switch driver could be hard coded with 0x80, and all DT needs to indicate is if the EEPROM is present or not.
So, what you have proposed will work. It is maybe not optimal in the case of a well defined in the datasheet fixed size EEPROM, but it still works.
Andrew
Re: [PATCH v2 09/15] net: dsa: Add support for switch EEPROM access
Quoted 9 lines · Andrew Lunn
Quoted 3 lines
So lets take a step back: For the Marvell chips, I have to provide both length and presence in devicetree or platform data. Presence seemed to be implied by length, so I used only a single property and variable to indicate both.
Hi Guenter
What i was thinking, is that you don't need length in device tree. The datasheet specifies how big the EEPROM needs to be.
Quoted 7 lines
However, i read the datasheet for the 6060, the only public datasheet from Marvell. It does not work as i expected. Rather than being a fixed list of register values, it is a variable length list of command/value pairs.
In this situation, yes, you do need the length in DT.
Correct. The 6352 supports "2K bit (93C56) or 4K bit (93C66) 4-wire EEPROM devices as well as 1K bit (24C01), 2K bit (24C02) or 4K bit (24C04) 2-wire EEPROM", so the length can be anything from 128 to 512 bytes.
Quoted 11 lines
I had a quick look at some other switch chips. e.g. the RTL8100. It has a fixed layout of its EEPROM, consisting of
0x80bytes. In this case, the switch driver could be hard coded with0x80, and all DT needs to indicate is if the EEPROM is present or not.So, what you have proposed will work. It is maybe not optimal in the case of a well defined in the datasheet fixed size EEPROM, but it still works.
Acked-by Andrew Lunn <[email protected]>
Thanks!
Guenter
Re: [PATCH v2 09/15] net: dsa: Add support for switch EEPROM access
In reply to Guenter RoeckQuoted 5 lines · Guenter Roeck
On some chips it is possible to access the switch eeprom. Add infrastructure support for it.
Signed-off-by Guenter Roeck <[email protected]>---
[ ... ]
net/dsa/dsa.c static int dsa_of_probe(struct platform_device *pdev) · line 608
Quoted 6 lines
net/dsa/dsa.c +2
-0@@ -603,6 +604,9 @@ static int dsa_of_probe(struct platform_device *pdev)if (pd->nr_chips >DSA_MAX_SWITCHES ) pd->nr_chips =DSA_MAX_SWITCHES ; +if (!of_property_read_u32 (np,"eeprom-length" , &eeprom_length)) + pd->eeprom_length = eeprom_length;
This is buggy; please don't apply. Variable is named eeprom_len, and it is per switch.
Lesson to self: When writing devicetree code, compile it with devicetree enabled.
Guenter
PATCH v3 9/15 net: dsa: Add support for switch EEPROM access
In reply to Guenter RoeckOn 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
@@ -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
@@ -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
David Miller
maintainerRe: [PATCH v3 09/15] net: dsa: Add support for switch EEPROM access
When you need to post new versions of patches in a series, you must repost the entire series anew, not just the patches which are changing.
Re: [PATCH v3 09/15] net: dsa: Add support for switch EEPROM access
Quoted 3 lines · David Miller
When you need to post new versions of patches in a series, you must repost the entire series anew, not just the patches which are changing.
Ok, thanks for letting me know.
Guenter
PATCH v2 10/15 dsa: Add new optional devicetree property to describe EEPROM size
In reply to Guenter RoeckThe 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.
---
v2:
- Added patch
Documentation/devicetree/bindings/net/dsa/dsa.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/net/dsa/dsa.txt +5 -1
@@ -10,7 +10,11 @@ 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: +- 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. - interrupts : property with a value describing the switch interrupt number (not supported by the driver)
Signature · 1 line
1.9.1
PATCH v3 10/15 dsa: Add new optional devicetree property to describe EEPROM size
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
@@ -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
PATCH v2 11/15 net: dsa/mv88e6352: Implement EEPROM access functions
In reply to Guenter RoeckMV88E6352 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.
---
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
@@ -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
@@ -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
PATCH v2 12/15 net: dsa: Add support for reading switch registers with ethtool
In reply to Guenter RoeckAdd support for reading switch registers with 'ethtool -d'.
---
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
@@ -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
@@ -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
PATCH v2 13/15 net: dsa/mv88e6123_61_65: Add support for reading switch registers
In reply to Guenter RoeckThe infrastructure can now report switch registers to ethtool. Add support for it to the mv88e6123_61_65 driver.
---
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
@@ -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
@@ -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
@@ -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
PATCH v2 14/15 net: dsa/mv88e6352: Add support for reading switch registers
In reply to Guenter RoeckReport switch register values to ethtool.
---
v2:
- No change
drivers/net/dsa/mv88e6352.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/net/dsa/mv88e6352.c +2 -0
@@ -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
PATCH v2 15/15 net: dsa: Provide additional RMON statistics
In reply to Guenter RoeckDisplay 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.
---
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
@@ -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
@@ -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
@@ -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