From: Alexis Lothoré <[email protected]>
This series brings initial support for Marvell 88E6361 switch.
MV88E6361 is a 8 ports switch with 5 integrated Gigabit PHYs and 3
2.5Gigabit SerDes interfaces. It is in fact a new variant in the
88E639X/88E6193X/88E6191X family with a subset of existing features:
- port 0: MII, RMII, RGMII, 1000BaseX, 2500BaseX
- port 3 to 7: triple speed internal phys
- port 9 and 10: 1000BaseX, 25000BaseX
Since said family is already well supported in mv88e6xxx driver, adding
initial support for this new switch mostly consists in finding the ID
exposed in its identification register, adding a proper description
in switch description tables in mv88e6xxx driver, and enforcing 88E6361
specificities in mv88e6393x_XXX methods.
- first 4 commits introduce an internal phy offset field for switches which
have internal phys but not starting from port 0
- 5th commit is a fix on existing switches based on first commits
- 6th commit is a slight modification to prepare 886361 support
- last commit introduces 88E6361 support in 88E6393X family
This initial support has been tested with two samples of a custom board
with the following hardware configuration:
- a main CPU connected to MV88E6361 using port 0 as CPU port
- port 9 wired to a SFP cage
- port 10 wired to a G.Hn transceiver
The following setup was used:
PC <-ethernet-> (copper SFP) - Board 1 - (G.hn) <-phone line(RJ11)-> (G.hn) Board 2
The unit 1 has been configured to bridge SFP port and G.hn port together,
which allowed to successfully ping Board 2 from PC.
Now that this series brings fixes for existing switches, I am not sure if
a split into two series is desirable. If so, please let me know. Also, my
current testing hardware does not use ports with internal PHYs, so further
feedback/testing on 6393X family would be highly appreciated
Changes since v1:
- rework mv88e6xxx_port_ppu_updates to use internal helper
- add internal phys offset field to manage switches which do not have
internal PHYs right on first ports
- fix 88E639X/88E6193X/88E6191X internal phy layout
- enforce 88E6361 features in mv88e6393x_port_set_speed_duplex
- enforce 88E6361 features in mv88e6393x_port_max_speed_mode
- enforce 88E6361 features in mv88e6393x_phylink_get_caps
- add Reviewed-By and Acked-By on untouched patch
Alexis Lothoré (7):
dt-bindings: net: dsa: marvell: add MV88E6361 switch to compatibility
list
net: dsa: mv88e6xxx: pass directly chip structure to
mv88e6xxx_phy_is_internal
net: dsa: mv88e6xxx: use mv88e6xxx_phy_is_internal in
mv88e6xxx_port_ppu_updates
net: dsa: mv88e6xxx: add field to specify internal phys layout
net: dsa: mv88e6xxx: fix 88E6393X family internal phys layout
net: dsa: mv88e6xxx: pass mv88e6xxx_chip structure to
port_max_speed_mode
net: dsa: mv88e6xxx: enable support for 88E6361 switch
.../devicetree/bindings/net/dsa/marvell.txt | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 69 ++++++++++++++-----
drivers/net/dsa/mv88e6xxx/chip.h | 11 ++-
drivers/net/dsa/mv88e6xxx/global2.c | 6 +-
drivers/net/dsa/mv88e6xxx/port.c | 23 +++++--
drivers/net/dsa/mv88e6xxx/port.h | 13 ++--
6 files changed, 94 insertions(+), 30 deletions(-)
--
2.40.1
From: Alexis Lothoré <[email protected]>
Marvell 88E6361 is an 8-port switch derived from the
88E6393X/88E9193X/88E6191X switches family. It can benefit from the
existing mv88e6xxx driver by simply adding the proper switch description in
the driver. Main differences with other switches from this
family are:
- 8 ports exposed (instead of 11): ports 1, 2 and 8 not available
- No 5GBase-x nor SFI/USXGMII support
---
Changes since v1:
- define internal phys offset
- enforce 88e6361 features in mv88e6393x_phylink_get_caps
- enforce 88e6361 features in mv88e6393x_port_set_speed_duplex
- enforce 88e6361 features in mv88e6393x_port_max_speed_mode
Signed-off-by: Alexis Lothoré <[email protected]>
---
drivers/net/dsa/mv88e6xxx/chip.c | 42 ++++++++++++++++++++++++++++----
drivers/net/dsa/mv88e6xxx/chip.h | 3 ++-
drivers/net/dsa/mv88e6xxx/port.c | 11 ++++++++-
drivers/net/dsa/mv88e6xxx/port.h | 1 +
4 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 0e6267193ac1..7c77b4b634f9 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -790,6 +790,8 @@ static void mv88e6393x_phylink_get_caps(struct mv88e6xxx_chip *chip, int port,
unsigned long *supported = config->supported_interfaces;
bool is_6191x =
chip->info->prod_num == MV88E6XXX_PORT_SWITCH_ID_PROD_6191X;
+ bool is_6361 =
+ chip->info->prod_num == MV88E6XXX_PORT_SWITCH_ID_PROD_6361;
mv88e6xxx_translate_cmode(chip->ports[port].cmode, supported);
@@ -804,13 +806,17 @@ static void mv88e6393x_phylink_get_caps(struct mv88e6xxx_chip *chip, int port,
/* 6191X supports >1G modes only on port 10 */
if (!is_6191x || port == 10) {
__set_bit(PHY_INTERFACE_MODE_2500BASEX, supported);
- __set_bit(PHY_INTERFACE_MODE_5GBASER, supported);
- __set_bit(PHY_INTERFACE_MODE_10GBASER, supported);
+ config->mac_capabilities |= MAC_2500FD;
+
+ /* 6361 only supports up to 2500BaseX */
+ if (!is_6361) {
+ __set_bit(PHY_INTERFACE_MODE_5GBASER, supported);
+ __set_bit(PHY_INTERFACE_MODE_10GBASER, supported);
+ config->mac_capabilities |= MAC_5000FD |
+ MAC_10000FD;
+ }
/* FIXME: USXGMII is not supported yet */
/* __set_bit(PHY_INTERFACE_MODE_USXGMII, supported); */
-
- config->mac_capabilities |= MAC_2500FD | MAC_5000FD |
- MAC_10000FD;
}
}
@@ -6311,6 +6317,32 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.ptp_support = true,
.ops = &mv88e6352_ops,
},
+ [MV88E6361] = {
+ .prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6361,
+ .family = MV88E6XXX_FAMILY_6393,
+ .name = "Marvell 88E6361",
+ .num_databases = 4096,
+ .num_macs = 16384,
+ .num_ports = 11,
+ /* Ports 1, 2 and 8 are not routed */
+ .invalid_port_mask = BIT(1) | BIT(2) | BIT(8),
+ .num_internal_phys = 5,
+ .internal_phys_offset = 3,
+ .max_vid = 4095,
+ .max_sid = 63,
+ .port_base_addr = 0x0,
+ .phy_base_addr = 0x0,
+ .global1_addr = 0x1b,
+ .global2_addr = 0x1c,
+ .age_time_coeff = 3750,
+ .g1_irqs = 10,
+ .g2_irqs = 14,
+ .atu_move_port_mask = 0x1f,
+ .pvt = true,
+ .multi_chip = true,
+ .ptp_support = true,
+ .ops = &mv88e6393x_ops,
+ },
[MV88E6390] = {
.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6390,
.family = MV88E6XXX_FAMILY_6390,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index dd7c8880e987..79c06ba42c54 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -82,6 +82,7 @@ enum mv88e6xxx_model {
MV88E6350,
MV88E6351,
MV88E6352,
+ MV88E6361,
MV88E6390,
MV88E6390X,
MV88E6393X,
@@ -100,7 +101,7 @@ enum mv88e6xxx_family {
MV88E6XXX_FAMILY_6351, /* 6171 6175 6350 6351 */
MV88E6XXX_FAMILY_6352, /* 6172 6176 6240 6352 */
MV88E6XXX_FAMILY_6390, /* 6190 6190X 6191 6290 6390 6390X */
- MV88E6XXX_FAMILY_6393, /* 6191X 6193X 6393X */
+ MV88E6XXX_FAMILY_6393, /* 6191X 6193X 6361 6393X */
};
/**
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index 66f1b40b4e96..e72ea3c8092f 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -421,9 +421,14 @@ phy_interface_t mv88e6390x_port_max_speed_mode(struct mv88e6xxx_chip *chip,
int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
int speed, int duplex)
{
+ bool is_6361 =
+ chip->info->prod_num == MV88E6XXX_PORT_SWITCH_ID_PROD_6361;
u16 reg, ctrl;
int err;
+ if (is_6361 && speed > 2500)
+ return -EOPNOTSUPP;
+
if (speed == 200 && port != 0)
return -EOPNOTSUPP;
@@ -506,8 +511,12 @@ int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
phy_interface_t mv88e6393x_port_max_speed_mode(struct mv88e6xxx_chip *chip,
int port)
{
+ bool is_6361 =
+ chip->info->prod_num == MV88E6XXX_PORT_SWITCH_ID_PROD_6361;
+
if (port == 0 || port == 9 || port == 10)
- return PHY_INTERFACE_MODE_10GBASER;
+ return is_6361 ? PHY_INTERFACE_MODE_2500BASEX :
+ PHY_INTERFACE_MODE_10GBASER;
return PHY_INTERFACE_MODE_NA;
}
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 3c9fc17abdd2..56dfa9d3d4e0 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -138,6 +138,7 @@
#define MV88E6XXX_PORT_SWITCH_ID_PROD_6141 0x3400
#define MV88E6XXX_PORT_SWITCH_ID_PROD_6341 0x3410
#define MV88E6XXX_PORT_SWITCH_ID_PROD_6352 0x3520
+#define MV88E6XXX_PORT_SWITCH_ID_PROD_6361 0x2610
#define MV88E6XXX_PORT_SWITCH_ID_PROD_6350 0x3710
#define MV88E6XXX_PORT_SWITCH_ID_PROD_6351 0x3750
#define MV88E6XXX_PORT_SWITCH_ID_PROD_6390 0x3900
--
2.40.1
From: Alexis Lothoré <[email protected]>
Since this function is a simple helper, we do not need to pass a full
dsa_switch structure, we can directly pass the mv88e6xxx_chip structure.
Doing so will allow to share this function with any other function
not manipulating dsa_switch structure but needing info about number of
internal phys
Signed-off-by: Alexis Lothoré <[email protected]>
---
drivers/net/dsa/mv88e6xxx/chip.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 64a2f2f83735..93bcfa5c80e1 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -463,10 +463,8 @@ static int mv88e6xxx_port_setup_mac(struct mv88e6xxx_chip *chip, int port,
return err;
}
-static int mv88e6xxx_phy_is_internal(struct dsa_switch *ds, int port)
+static int mv88e6xxx_phy_is_internal(struct mv88e6xxx_chip *chip, int port)
{
- struct mv88e6xxx_chip *chip = ds->priv;
-
return port < chip->info->num_internal_phys;
}
@@ -584,7 +582,7 @@ static void mv88e6095_phylink_get_caps(struct mv88e6xxx_chip *chip, int port,
config->mac_capabilities = MAC_SYM_PAUSE | MAC_10 | MAC_100;
- if (mv88e6xxx_phy_is_internal(chip->ds, port)) {
+ if (mv88e6xxx_phy_is_internal(chip, port)) {
__set_bit(PHY_INTERFACE_MODE_MII, config->supported_interfaces);
} else {
if (cmode < ARRAY_SIZE(mv88e6185_phy_interface_modes) &&
@@ -832,7 +830,7 @@ static void mv88e6xxx_get_caps(struct dsa_switch *ds, int port,
chip->info->ops->phylink_get_caps(chip, port, config);
mv88e6xxx_reg_unlock(chip);
- if (mv88e6xxx_phy_is_internal(ds, port)) {
+ if (mv88e6xxx_phy_is_internal(chip, port)) {
__set_bit(PHY_INTERFACE_MODE_INTERNAL,
config->supported_interfaces);
/* Internal ports with no phy-mode need GMII for PHYLIB */
@@ -853,7 +851,7 @@ static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port,
mv88e6xxx_reg_lock(chip);
- if (mode != MLO_AN_PHY || !mv88e6xxx_phy_is_internal(ds, port)) {
+ if (mode != MLO_AN_PHY || !mv88e6xxx_phy_is_internal(chip, port)) {
/* In inband mode, the link may come up at any time while the
* link is not forced down. Force the link down while we
* reconfigure the interface mode.
--
2.40.1
From: Alexis Lothoré <[email protected]>
Make sure to use existing helper to get internal PHYs count instead of
redoing it manually
Signed-off-by: Alexis Lothoré <[email protected]>
---
drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 93bcfa5c80e1..c812e52bb5b7 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -477,7 +477,7 @@ static int mv88e6xxx_port_ppu_updates(struct mv88e6xxx_chip *chip, int port)
* report whether the port is internal.
*/
if (chip->info->family == MV88E6XXX_FAMILY_6250)
- return port < chip->info->num_internal_phys;
+ return mv88e6xxx_phy_is_internal(chip, port);
err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_STS, ®);
if (err) {
--
2.40.1
From: Alexis Lothoré <[email protected]>
Some switches families have minor differences on supported link speed for
ports. Instead of redefining a new port_max_speed_mode for each different
configuration, allow to pass mv88e6xxx_chip structure to allow
differentiating those chips by known chip id
Signed-off-by: Alexis Lothoré <[email protected]>
---
drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.h | 3 ++-
drivers/net/dsa/mv88e6xxx/port.c | 12 ++++++++----
drivers/net/dsa/mv88e6xxx/port.h | 12 ++++++++----
4 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index f15ca17bf9e2..0e6267193ac1 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3311,7 +3311,7 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
caps = pl_config.mac_capabilities;
if (chip->info->ops->port_max_speed_mode)
- mode = chip->info->ops->port_max_speed_mode(port);
+ mode = chip->info->ops->port_max_speed_mode(chip, port);
else
mode = PHY_INTERFACE_MODE_NA;
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index eca51946c100..dd7c8880e987 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -518,7 +518,8 @@ struct mv88e6xxx_ops {
int speed, int duplex);
/* What interface mode should be used for maximum speed? */
- phy_interface_t (*port_max_speed_mode)(int port);
+ phy_interface_t (*port_max_speed_mode)(struct mv88e6xxx_chip *chip,
+ int port);
int (*port_tag_remap)(struct mv88e6xxx_chip *chip, int port);
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index f79cf716c541..66f1b40b4e96 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -342,7 +342,8 @@ int mv88e6341_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
duplex);
}
-phy_interface_t mv88e6341_port_max_speed_mode(int port)
+phy_interface_t mv88e6341_port_max_speed_mode(struct mv88e6xxx_chip *chip,
+ int port)
{
if (port == 5)
return PHY_INTERFACE_MODE_2500BASEX;
@@ -381,7 +382,8 @@ int mv88e6390_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
duplex);
}
-phy_interface_t mv88e6390_port_max_speed_mode(int port)
+phy_interface_t mv88e6390_port_max_speed_mode(struct mv88e6xxx_chip *chip,
+ int port)
{
if (port == 9 || port == 10)
return PHY_INTERFACE_MODE_2500BASEX;
@@ -403,7 +405,8 @@ int mv88e6390x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
duplex);
}
-phy_interface_t mv88e6390x_port_max_speed_mode(int port)
+phy_interface_t mv88e6390x_port_max_speed_mode(struct mv88e6xxx_chip *chip,
+ int port)
{
if (port == 9 || port == 10)
return PHY_INTERFACE_MODE_XAUI;
@@ -500,7 +503,8 @@ int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
return 0;
}
-phy_interface_t mv88e6393x_port_max_speed_mode(int port)
+phy_interface_t mv88e6393x_port_max_speed_mode(struct mv88e6xxx_chip *chip,
+ int port)
{
if (port == 0 || port == 9 || port == 10)
return PHY_INTERFACE_MODE_10GBASER;
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index aec9d4fd20e3..3c9fc17abdd2 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -359,10 +359,14 @@ int mv88e6390x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
int speed, int duplex);
-phy_interface_t mv88e6341_port_max_speed_mode(int port);
-phy_interface_t mv88e6390_port_max_speed_mode(int port);
-phy_interface_t mv88e6390x_port_max_speed_mode(int port);
-phy_interface_t mv88e6393x_port_max_speed_mode(int port);
+phy_interface_t mv88e6341_port_max_speed_mode(struct mv88e6xxx_chip *chip,
+ int port);
+phy_interface_t mv88e6390_port_max_speed_mode(struct mv88e6xxx_chip *chip,
+ int port);
+phy_interface_t mv88e6390x_port_max_speed_mode(struct mv88e6xxx_chip *chip,
+ int port);
+phy_interface_t mv88e6393x_port_max_speed_mode(struct mv88e6xxx_chip *chip,
+ int port);
int mv88e6xxx_port_set_state(struct mv88e6xxx_chip *chip, int port, u8 state);
--
2.40.1
From: Alexis Lothoré <[email protected]>
88E6393X/88E6193X/88E6191X swicthes have in fact 8 internal PHYs, but those
are not present starting at port 0: supported ports go from 1 to 8
Signed-off-by: Alexis Lothoré <[email protected]>
---
drivers/net/dsa/mv88e6xxx/chip.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 2716d17c5c49..f15ca17bf9e2 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -6024,7 +6024,8 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.name = "Marvell 88E6191X",
.num_databases = 4096,
.num_ports = 11, /* 10 + Z80 */
- .num_internal_phys = 9,
+ .num_internal_phys = 8,
+ .internal_phys_offset = 1,
.max_vid = 8191,
.max_sid = 63,
.port_base_addr = 0x0,
@@ -6047,7 +6048,8 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.name = "Marvell 88E6193X",
.num_databases = 4096,
.num_ports = 11, /* 10 + Z80 */
- .num_internal_phys = 9,
+ .num_internal_phys = 8,
+ .internal_phys_offset = 1,
.max_vid = 8191,
.max_sid = 63,
.port_base_addr = 0x0,
@@ -6366,7 +6368,8 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
.name = "Marvell 88E6393X",
.num_databases = 4096,
.num_ports = 11, /* 10 + Z80 */
- .num_internal_phys = 9,
+ .num_internal_phys = 8,
+ .internal_phys_offset = 1,
.max_vid = 8191,
.max_sid = 63,
.port_base_addr = 0x0,
--
2.40.1
From: Alexis Lothoré <[email protected]>
Marvell MV88E6361 is an 8-port switch derived from the
88E6393X/88E9193X/88E6191X switches family. Since its functional behavior
is very close to switches from this family, it can benefit from existing
drivers for this family, so add it to the list of compatible switches
Reviewed-by: Andrew Lunn <[email protected]>
Acked-by: Conor Dooley <[email protected]>
Signed-off-by: Alexis Lothoré <[email protected]>
---
Documentation/devicetree/bindings/net/dsa/marvell.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
index 2363b412410c..33726134f5c9 100644
--- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
+++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
@@ -20,7 +20,7 @@ which is at a different MDIO base address in different switch families.
6171, 6172, 6175, 6176, 6185, 6240, 6320, 6321,
6341, 6350, 6351, 6352
- "marvell,mv88e6190" : Switch has base address 0x00. Use with models:
- 6190, 6190X, 6191, 6290, 6390, 6390X
+ 6163, 6190, 6190X, 6191, 6290, 6390, 6390X
- "marvell,mv88e6250" : Switch has base address 0x08 or 0x18. Use with model:
6220, 6250
--
2.40.1
From: Alexis Lothoré <[email protected]>
mv88e6xxx currently assumes that switch equipped with internal phys have
those phys mapped contiguously starting from port 0 (see
mv88e6xxx_phy_is_internal). However, some switches have internal PHYs but
NOT starting from port 0. For example 88e6393X, 88E6193X and 88E6191X have
integrated PHYs available on ports 1 to 8
To properly support this offset, add a new field to allow specifying an
internal PHYs layout. If field is not set, default layout is assumed (start
at port 0)
Signed-off-by: Alexis Lothoré <[email protected]>
---
drivers/net/dsa/mv88e6xxx/chip.c | 4 +++-
drivers/net/dsa/mv88e6xxx/chip.h | 5 +++++
drivers/net/dsa/mv88e6xxx/global2.c | 6 +++++-
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c812e52bb5b7..2716d17c5c49 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -465,7 +465,9 @@ static int mv88e6xxx_port_setup_mac(struct mv88e6xxx_chip *chip, int port,
static int mv88e6xxx_phy_is_internal(struct mv88e6xxx_chip *chip, int port)
{
- return port < chip->info->num_internal_phys;
+ return port >= chip->info->internal_phys_offset &&
+ port < chip->info->num_internal_phys +
+ chip->info->internal_phys_offset;
}
static int mv88e6xxx_port_ppu_updates(struct mv88e6xxx_chip *chip, int port)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index da6e1339f809..eca51946c100 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -167,6 +167,11 @@ struct mv88e6xxx_info {
/* Supports PTP */
bool ptp_support;
+
+ /* Internal PHY start index. 0 means that internal PHYs range starts at
+ * port 0, 1 means internal PHYs range starts at port 1, etc
+ */
+ unsigned int internal_phys_offset;
};
struct mv88e6xxx_atu_entry {
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index 615896893076..d460f7290012 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -1198,13 +1198,17 @@ int mv88e6xxx_g2_irq_mdio_setup(struct mv88e6xxx_chip *chip,
{
int phy, irq;
- for (phy = 0; phy < chip->info->num_internal_phys; phy++) {
+ for (phy = chip->info->internal_phys_offset;
+ phy <
+ chip->info->num_internal_phys + chip->info->internal_phys_offset;
+ phy++) {
irq = irq_find_mapping(chip->g2_irq.domain, phy);
if (irq < 0)
return irq;
bus->irq[chip->info->phy_base_addr + phy] = irq;
}
+
return 0;
}
--
2.40.1
On Fri, May 19, 2023 at 04:12:59PM +0200, [email protected] wrote:
> From: Alexis Lothor? <[email protected]>
>
> Make sure to use existing helper to get internal PHYs count instead of
> redoing it manually
>
> Signed-off-by: Alexis Lothor? <[email protected]>
Reviewed-by: Russell King (Oracle) <[email protected]>
Thanks!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Fri, May 19, 2023 at 04:12:58PM +0200, [email protected] wrote:
> From: Alexis Lothor? <[email protected]>
>
> Since this function is a simple helper, we do not need to pass a full
> dsa_switch structure, we can directly pass the mv88e6xxx_chip structure.
> Doing so will allow to share this function with any other function
> not manipulating dsa_switch structure but needing info about number of
> internal phys
>
> Signed-off-by: Alexis Lothor? <[email protected]>
Reviewed-by: Russell King (Oracle) <[email protected]>
Thanks!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Fri, May 19, 2023 at 04:13:03PM +0200, [email protected] wrote:
> From: Alexis Lothor? <[email protected]>
>
> Marvell 88E6361 is an 8-port switch derived from the
> 88E6393X/88E9193X/88E6191X switches family. It can benefit from the
> existing mv88e6xxx driver by simply adding the proper switch description in
> the driver. Main differences with other switches from this
> family are:
> - 8 ports exposed (instead of 11): ports 1, 2 and 8 not available
> - No 5GBase-x nor SFI/USXGMII support
>
> ---
> Changes since v1:
> - define internal phys offset
> - enforce 88e6361 features in mv88e6393x_phylink_get_caps
> - enforce 88e6361 features in mv88e6393x_port_set_speed_duplex
> - enforce 88e6361 features in mv88e6393x_port_max_speed_mode
Not exactly related to this patch, but please do not rely on this "max
speed mode" - please always ensure that you specify the phy-mode and
fixed-link settings for CPU and DSA ports in firmware. Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> @@ -1198,13 +1198,17 @@ int mv88e6xxx_g2_irq_mdio_setup(struct mv88e6xxx_chip *chip,
> {
> int phy, irq;
>
> - for (phy = 0; phy < chip->info->num_internal_phys; phy++) {
> + for (phy = chip->info->internal_phys_offset;
> + phy <
> + chip->info->num_internal_phys + chip->info->internal_phys_offset;
> + phy++) {
The code style is not so nice. How about moving this addition out of
the for loop, it is static anyway. And then you can avoid splitting
the expression over multiple lines.
> irq = irq_find_mapping(chip->g2_irq.domain, phy);
> if (irq < 0)
> return irq;
>
> bus->irq[chip->info->phy_base_addr + phy] = irq;
> }
> +
No whitespace changed please.
Andrew
---
pw-bot: cr
On Fri, May 19, 2023 at 04:13:01PM +0200, [email protected] wrote:
> From: Alexis Lothor? <[email protected]>
>
> 88E6393X/88E6193X/88E6191X swicthes have in fact 8 internal PHYs, but those
> are not present starting at port 0: supported ports go from 1 to 8
>
> Signed-off-by: Alexis Lothor? <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
> @@ -421,9 +421,14 @@ phy_interface_t mv88e6390x_port_max_speed_mode(struct mv88e6xxx_chip *chip,
> int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
> int speed, int duplex)
> {
> + bool is_6361 =
> + chip->info->prod_num == MV88E6XXX_PORT_SWITCH_ID_PROD_6361;
> u16 reg, ctrl;
> int err;
>
> + if (is_6361 && speed > 2500)
> + return -EOPNOTSUPP;
I would move the comparison inside the if, so removing the ugly looking split is_6361 line.
> +
> if (speed == 200 && port != 0)
> return -EOPNOTSUPP;
>
> @@ -506,8 +511,12 @@ int mv88e6393x_port_set_speed_duplex(struct mv88e6xxx_chip *chip, int port,
> phy_interface_t mv88e6393x_port_max_speed_mode(struct mv88e6xxx_chip *chip,
> int port)
> {
> + bool is_6361 =
> + chip->info->prod_num == MV88E6XXX_PORT_SWITCH_ID_PROD_6361;
> +
> if (port == 0 || port == 9 || port == 10)
> - return PHY_INTERFACE_MODE_10GBASER;
> + return is_6361 ? PHY_INTERFACE_MODE_2500BASEX :
> + PHY_INTERFACE_MODE_10GBASER;
Please see if you can rearrange this code as well.
Thanks
Andrew
On 5/19/2023 7:12 AM, [email protected] wrote:
> From: Alexis Lothoré <[email protected]>
>
> Marvell MV88E6361 is an 8-port switch derived from the
> 88E6393X/88E9193X/88E6191X switches family. Since its functional behavior
> is very close to switches from this family, it can benefit from existing
> drivers for this family, so add it to the list of compatible switches
>
> Reviewed-by: Andrew Lunn <[email protected]>
> Acked-by: Conor Dooley <[email protected]>
> Signed-off-by: Alexis Lothoré <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
On 5/19/2023 7:12 AM, [email protected] wrote:
> From: Alexis Lothoré <[email protected]>
>
> Make sure to use existing helper to get internal PHYs count instead of
> redoing it manually
>
> Signed-off-by: Alexis Lothoré <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
On 5/19/2023 7:13 AM, [email protected] wrote:
> From: Alexis Lothoré <[email protected]>
>
> 88E6393X/88E6193X/88E6191X swicthes have in fact 8 internal PHYs, but those
> are not present starting at port 0: supported ports go from 1 to 8
>
> Signed-off-by: Alexis Lothoré <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
On 5/19/2023 7:12 AM, [email protected] wrote:
> From: Alexis Lothoré <[email protected]>
>
> Since this function is a simple helper, we do not need to pass a full
> dsa_switch structure, we can directly pass the mv88e6xxx_chip structure.
> Doing so will allow to share this function with any other function
> not manipulating dsa_switch structure but needing info about number of
> internal phys
>
> Signed-off-by: Alexis Lothoré <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
On 5/19/2023 7:13 AM, [email protected] wrote:
> From: Alexis Lothoré <[email protected]>
>
> Some switches families have minor differences on supported link speed for
> ports. Instead of redefining a new port_max_speed_mode for each different
> configuration, allow to pass mv88e6xxx_chip structure to allow
> differentiating those chips by known chip id
>
> Signed-off-by: Alexis Lothoré <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
Hi Russell, thanks for review
On 5/19/23 16:43, Russell King (Oracle) wrote:
> On Fri, May 19, 2023 at 04:13:03PM +0200, [email protected] wrote:
>> From: Alexis Lothoré <[email protected]>
>>
>> Marvell 88E6361 is an 8-port switch derived from the
>> 88E6393X/88E9193X/88E6191X switches family. It can benefit from the
>> existing mv88e6xxx driver by simply adding the proper switch description in
>> the driver. Main differences with other switches from this
>> family are:
>> - 8 ports exposed (instead of 11): ports 1, 2 and 8 not available
>> - No 5GBase-x nor SFI/USXGMII support
>>
>> ---
>> Changes since v1:
>> - define internal phys offset
>> - enforce 88e6361 features in mv88e6393x_phylink_get_caps
>> - enforce 88e6361 features in mv88e6393x_port_set_speed_duplex
>> - enforce 88e6361 features in mv88e6393x_port_max_speed_mode
>
> Not exactly related to this patch, but please do not rely on this "max
> speed mode" - please always ensure that you specify the phy-mode and
> fixed-link settings for CPU and DSA ports in firmware. Thanks.
I would like to make sure to fully understand your point:
- when telling so specify phy-mode and fixed-link in firmware, you mean
device-tree, right ?
- when checking for code and execution flow, I observe that port_max_speed is
always called and its output is always used to configure shared ports mode in
mv88e6xxx driver. Are you telling that eventually, the whole mv88e6xxx driver
should stop relying on port_max_speed_mode for shared ports ?
Kind regards,
--
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Hello Andrew,
On 5/22/23 14:19, Andrew Lunn wrote:
>>> Not exactly related to this patch, but please do not rely on this "max
>>> speed mode" - please always ensure that you specify the phy-mode and
>>> fixed-link settings for CPU and DSA ports in firmware. Thanks.
>>
>> I would like to make sure to fully understand your point:
>> - when telling so specify phy-mode and fixed-link in firmware, you mean
>> device-tree, right ?
>> - when checking for code and execution flow, I observe that port_max_speed is
>> always called and its output is always used to configure shared ports mode in
>> mv88e6xxx driver. Are you telling that eventually, the whole mv88e6xxx driver
>> should stop relying on port_max_speed_mode for shared ports ?
>
> Yes, the concept of port_max_speed_mode causes problems for PHYLINK,
> and we want to remove it. Russell and i have been updating DT
> descriptions adding fixed-link and phy-mode properties to all
> mv88e6xxx systems so that it is not needed. Either at the end of this
> cycle, or the beginning of the next we will change the code to
> actually enforce this.
Understood, thanks for clarification
>
> Andrew
--
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
> > Not exactly related to this patch, but please do not rely on this "max
> > speed mode" - please always ensure that you specify the phy-mode and
> > fixed-link settings for CPU and DSA ports in firmware. Thanks.
>
> I would like to make sure to fully understand your point:
> - when telling so specify phy-mode and fixed-link in firmware, you mean
> device-tree, right ?
> - when checking for code and execution flow, I observe that port_max_speed is
> always called and its output is always used to configure shared ports mode in
> mv88e6xxx driver. Are you telling that eventually, the whole mv88e6xxx driver
> should stop relying on port_max_speed_mode for shared ports ?
Yes, the concept of port_max_speed_mode causes problems for PHYLINK,
and we want to remove it. Russell and i have been updating DT
descriptions adding fixed-link and phy-mode properties to all
mv88e6xxx systems so that it is not needed. Either at the end of this
cycle, or the beginning of the next we will change the code to
actually enforce this.
Andrew