Hello!
This patch series is focused on simplifying the code, and improving the
logic of the support for MT7530, MT7531, and MT7988 SoC switches.
There're two fixes. One for fixing the corrupt frames using trgmii on MCM
MT7530 with 40 MHz oscillator on certain MT7621 SoCs. The other for fixing
the port capabilities of the switch on the MT7988 SoC.
Currently, using multiple ports as CPU ports won't work properly. I am
working on fixing it.
I am leaving this information for the record, as I have not received any
comments on my RFC series regarding this. The register may need to be
protected from being accessed by processes while the operation that
modifies the MT7530_MHWTRAP register on mt7530_setup_port6() is being done.
I don't see this being done on mt7530_setup() but it's there on
mt7530_setup_port5(). My tests haven't shown any apparent issues but if you
experience any issues with port 6, this is where to look first.
I have done iperf3 and ping tests on all ports of the MT7530 switch with
this patch series applied. I tested every possible configuration on the MCM
and standalone MT7530 switch. I'll let the name of the dtb files speak for
themselves.
MT7621 Unielec:
only-gmac0-mt7621-unielec-u7621-06-16m.dtb
rgmii-only-gmac0-mt7621-unielec-u7621-06-16m.dtb
only-gmac1-mt7621-unielec-u7621-06-16m.dtb
gmac0-and-gmac1-mt7621-unielec-u7621-06-16m.dtb
phy0-muxing-mt7621-unielec-u7621-06-16m.dtb
phy4-muxing-mt7621-unielec-u7621-06-16m.dtb
port5-as-user-mt7621-unielec-u7621-06-16m.dtb
tftpboot 0x80008000 mips-uzImage.bin; tftpboot 0x83000000 mips-rootfs.cpio.uboot; tftpboot 0x83f00000 $dtb; bootm 0x80008000 0x83000000 0x83f00000
MT7623 Bananapi:
only-gmac0-mt7623n-bananapi-bpi-r2.dtb
rgmii-only-gmac0-mt7623n-bananapi-bpi-r2.dtb
only-gmac1-mt7623n-bananapi-bpi-r2.dtb
gmac0-and-gmac1-mt7623n-bananapi-bpi-r2.dtb
phy0-muxing-mt7623n-bananapi-bpi-r2.dtb
phy4-muxing-mt7623n-bananapi-bpi-r2.dtb
port5-as-user-mt7623n-bananapi-bpi-r2.dtb
tftpboot 0x80008000 arm-uImage; tftpboot 0x83000000 arm-rootfs.cpio.uboot; tftpboot 0x83f00000 $dtb; bootm 0x80008000 0x83000000 0x83f00000
Arınç
Arınç ÜNAL (24):
net: dsa: mt7530: fix corrupt frames using trgmii on 40 MHz XTAL MT7621
net: dsa: mt7530: add missing @p5_interface to mt7530_priv description
net: dsa: mt7530: use p5_interface_select as data type for p5_intf_sel
net: dsa: mt7530: properly support MT7531AE and MT7531BE
net: dsa: mt7530: improve comments regarding port 5 and 6
net: dsa: mt7530: read XTAL value from correct register
net: dsa: mt7530: improve code path for setting up port 5
net: dsa: mt7530: do not run mt7530_setup_port5() if port 5 is disabled
net: dsa: mt7530: change p{5,6}_interface to p{5,6}_configured
net: dsa: mt7530: empty default case on mt7530_setup_port5()
net: dsa: mt7530: call port 6 setup from mt7530_mac_config()
net: dsa: mt7530: remove pad_setup function pointer
net: dsa: mt7530: move XTAL check to mt7530_setup()
net: dsa: mt7530: move enabling port 6 to mt7530_setup_port6()
net: dsa: mt7530: switch to if/else statements on mt7530_setup_port6()
net: dsa: mt7530: set TRGMII RD TAP if trgmii is being used
net: dsa: mt7530: move lowering port 5 RGMII driving to mt7530_setup()
net: dsa: mt7530: fix port capabilities for MT7988
net: dsa: mt7530: remove .mac_port_config for MT7988 and make it optional
net: dsa: mt7530: set interrupt register only for MT7530
net: dsa: mt7530: force link-down on MACs before reset on MT7530
net: dsa: mt7530: get rid of useless error returns on phylink code path
net: dsa: mt7530: rename p5_intf_sel and use only for MT7530 switch
net: dsa: mt7530: run mt7530_pll_setup() only with 40 MHz XTAL
drivers/net/dsa/mt7530-mdio.c | 7 +-
drivers/net/dsa/mt7530.c | 372 +++++++++++++------------------------
drivers/net/dsa/mt7530.h | 35 ++--
3 files changed, 145 insertions(+), 269 deletions(-)
From: Arınç ÜNAL <[email protected]>
The multi-chip module MT7530 switch with a 40 MHz oscillator on the
MT7621AT, MT7621DAT, and MT7621ST SoCs forwards corrupt frames using
trgmii.
This is caused by the assumption that MT7621 SoCs have got 150 MHz PLL,
hence using the ncpo1 value, 0x0780.
My testing shows this value works on Unielec U7621-06, Bartel's testing
shows it won't work on Hi-Link HLK-MT7621A and Netgear WAC104. All devices
tested have got 40 MHz oscillators.
Using the value for 125 MHz PLL, 0x0640, works on all boards at hand. The
definitions for 125 MHz PLL exist on the Banana Pi BPI-R2 BSP source code
whilst 150 MHz PLL don't.
Forwarding frames using trgmii on the MCM MT7530 switch with a 25 MHz
oscillator on the said MT7621 SoCs works fine because the ncpo1 value
defined for it is for 125 MHz PLL.
Change the 150 MHz PLL comment to 125 MHz PLL, and use the 125 MHz PLL
ncpo1 values for both oscillator frequencies.
Link: https://github.com/BPI-SINOVOIP/BPI-R2-bsp/blob/81d24bbce7d99524d0771a8bdb2d6663e4eb4faa/u-boot-mt/drivers/net/rt2880_eth.c#L2195
Fixes: 7ef6f6f8d237 ("net: dsa: mt7530: Add MT7621 TRGMII mode support")
Cc: [email protected]
Tested-by: Bartel Eerdekens <[email protected]>
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index c680873819b0..7d9f9563dbda 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -426,9 +426,9 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
else
ssc_delta = 0x87;
if (priv->id == ID_MT7621) {
- /* PLL frequency: 150MHz: 1.2GBit */
+ /* PLL frequency: 125MHz: 1.0GBit */
if (xtal == HWTRAP_XTAL_40MHZ)
- ncpo1 = 0x0780;
+ ncpo1 = 0x0640;
if (xtal == HWTRAP_XTAL_25MHZ)
ncpo1 = 0x0a00;
} else { /* PLL frequency: 250MHz: 2.0Gbit */
--
2.37.2
From: Arınç ÜNAL <[email protected]>
Add the missing p5_interface field to the mt7530_priv description. Sort out
the description in the process.
Signed-off-by: Arınç ÜNAL <[email protected]>
Acked-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/mt7530.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 5084f48a8869..845f5dd16d83 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -746,7 +746,8 @@ struct mt753x_info {
* @ports: Holding the state among ports
* @reg_mutex: The lock for protecting among process accessing
* registers
- * @p6_interface Holding the current port 6 interface
+ * @p6_interface: Holding the current port 6 interface
+ * @p5_interface: Holding the current port 5 interface
* @p5_intf_sel: Holding the current port 5 interface select
* @irq: IRQ number of the switch
* @irq_domain: IRQ domain of the switch irq_chip
--
2.37.2
From: Arınç ÜNAL <[email protected]>
Use the p5_interface_select enumeration as the data type for the
p5_intf_sel field. This ensures p5_intf_sel can only take the values
defined in the p5_interface_select enumeration.
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 845f5dd16d83..415d8ea07472 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -675,7 +675,7 @@ struct mt7530_port {
/* Port 5 interface select definitions */
enum p5_interface_select {
- P5_DISABLED = 0,
+ P5_DISABLED,
P5_INTF_SEL_PHY_P0,
P5_INTF_SEL_PHY_P4,
P5_INTF_SEL_GMAC5,
@@ -768,7 +768,7 @@ struct mt7530_priv {
bool mcm;
phy_interface_t p6_interface;
phy_interface_t p5_interface;
- unsigned int p5_intf_sel;
+ enum p5_interface_select p5_intf_sel;
u8 mirror_rx;
u8 mirror_tx;
struct mt7530_port ports[MT7530_NUM_PORTS];
--
2.37.2
From: Arınç ÜNAL <[email protected]>
Introduce the p5_sgmii pointer to store the information for whether port 5
has got SGMII or not.
Move the comment about MT7531AE and MT7531BE to mt7531_setup(), where the
switch is identified.
Get rid of mt7531_dual_sgmii_supported() now that priv->p5_sgmii stores the
information. Address the code where mt7531_dual_sgmii_supported() is used.
Get rid of mt7531_is_rgmii_port() which just prints the opposite of
priv->p5_sgmii.
Remove P5_INTF_SEL_GMAC5_SGMII. The p5_interface_select enum is supposed to
represent the mode that port 5 is being used in, not the hardware
information of port 5. Set p5_intf_sel to P5_INTF_SEL_GMAC5 instead, if
port 5 is not dsa_is_unused_port().
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530-mdio.c | 7 ++----
drivers/net/dsa/mt7530.c | 43 ++++++++++++-----------------------
drivers/net/dsa/mt7530.h | 6 +++--
3 files changed, 21 insertions(+), 35 deletions(-)
diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
index 088533663b83..fa3ee85a99c1 100644
--- a/drivers/net/dsa/mt7530-mdio.c
+++ b/drivers/net/dsa/mt7530-mdio.c
@@ -81,17 +81,14 @@ static const struct regmap_bus mt7530_regmap_bus = {
};
static int
-mt7531_create_sgmii(struct mt7530_priv *priv, bool dual_sgmii)
+mt7531_create_sgmii(struct mt7530_priv *priv)
{
struct regmap_config *mt7531_pcs_config[2] = {};
struct phylink_pcs *pcs;
struct regmap *regmap;
int i, ret = 0;
- /* MT7531AE has two SGMII units for port 5 and port 6
- * MT7531BE has only one SGMII unit for port 6
- */
- for (i = dual_sgmii ? 0 : 1; i < 2; i++) {
+ for (i = priv->p5_sgmii ? 0 : 1; i < 2; i++) {
mt7531_pcs_config[i] = devm_kzalloc(priv->dev,
sizeof(struct regmap_config),
GFP_KERNEL);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 7d9f9563dbda..29abf2745294 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -473,15 +473,6 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
return 0;
}
-static bool mt7531_dual_sgmii_supported(struct mt7530_priv *priv)
-{
- u32 val;
-
- val = mt7530_read(priv, MT7531_TOP_SIG_SR);
-
- return (val & PAD_DUAL_SGMII_EN) != 0;
-}
-
static int
mt7531_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
{
@@ -496,7 +487,7 @@ mt7531_pll_setup(struct mt7530_priv *priv)
u32 xtal;
u32 val;
- if (mt7531_dual_sgmii_supported(priv))
+ if (priv->p5_sgmii)
return;
val = mt7530_read(priv, MT7531_CREV);
@@ -907,8 +898,6 @@ static const char *p5_intf_modes(unsigned int p5_interface)
return "PHY P4";
case P5_INTF_SEL_GMAC5:
return "GMAC5";
- case P5_INTF_SEL_GMAC5_SGMII:
- return "GMAC5_SGMII";
default:
return "unknown";
}
@@ -2440,6 +2429,12 @@ mt7531_setup(struct dsa_switch *ds)
return -ENODEV;
}
+ /* MT7531AE has got two SGMII units. One for port 5, one for port 6.
+ * MT7531BE has got only one SGMII unit which is for port 6.
+ */
+ val = mt7530_read(priv, MT7531_TOP_SIG_SR);
+ priv->p5_sgmii = !!(val & PAD_DUAL_SGMII_EN);
+
/* all MACs must be forced link-down before sw reset */
for (i = 0; i < MT7530_NUM_PORTS; i++)
mt7530_write(priv, MT7530_PMCR_P(i), MT7531_FORCE_LNK);
@@ -2451,19 +2446,16 @@ mt7531_setup(struct dsa_switch *ds)
mt7531_pll_setup(priv);
- if (mt7531_dual_sgmii_supported(priv)) {
- priv->p5_intf_sel = P5_INTF_SEL_GMAC5_SGMII;
-
+ if (priv->p5_sgmii) {
/* Let ds->slave_mii_bus be able to access external phy. */
mt7530_rmw(priv, MT7531_GPIO_MODE1, MT7531_GPIO11_RG_RXD2_MASK,
MT7531_EXT_P_MDC_11);
mt7530_rmw(priv, MT7531_GPIO_MODE1, MT7531_GPIO12_RG_RXD3_MASK,
MT7531_EXT_P_MDIO_12);
- } else {
- priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
}
- dev_dbg(ds->dev, "P5 support %s interface\n",
- p5_intf_modes(priv->p5_intf_sel));
+
+ if (!dsa_is_unused_port(ds, 5))
+ priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
mt7530_rmw(priv, MT7531_GPIO_MODE0, MT7531_GPIO0_MASK,
MT7531_GPIO0_INTERRUPT);
@@ -2523,11 +2515,6 @@ static void mt7530_mac_port_get_caps(struct dsa_switch *ds, int port,
}
}
-static bool mt7531_is_rgmii_port(struct mt7530_priv *priv, u32 port)
-{
- return (port == 5) && (priv->p5_intf_sel != P5_INTF_SEL_GMAC5_SGMII);
-}
-
static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
struct phylink_config *config)
{
@@ -2540,7 +2527,7 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
break;
case 5: /* 2nd cpu port supports either rgmii or sgmii/8023z */
- if (mt7531_is_rgmii_port(priv, port)) {
+ if (!priv->p5_sgmii) {
phy_interface_set_rgmii(config->supported_interfaces);
break;
}
@@ -2607,7 +2594,7 @@ static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
{
u32 val;
- if (!mt7531_is_rgmii_port(priv, port)) {
+ if (priv->p5_sgmii) {
dev_err(priv->dev, "RGMII mode is not available for port %d\n",
port);
return -EINVAL;
@@ -2860,7 +2847,7 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
switch (port) {
case 5:
- if (mt7531_is_rgmii_port(priv, port))
+ if (!priv->p5_sgmii)
interface = PHY_INTERFACE_MODE_RGMII;
else
interface = PHY_INTERFACE_MODE_2500BASEX;
@@ -3019,7 +3006,7 @@ mt753x_setup(struct dsa_switch *ds)
mt7530_free_irq_common(priv);
if (priv->create_sgmii) {
- ret = priv->create_sgmii(priv, mt7531_dual_sgmii_supported(priv));
+ ret = priv->create_sgmii(priv);
if (ret && priv->irq)
mt7530_free_irq(priv);
}
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 415d8ea07472..2602c95fd3a5 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -679,7 +679,6 @@ enum p5_interface_select {
P5_INTF_SEL_PHY_P0,
P5_INTF_SEL_PHY_P4,
P5_INTF_SEL_GMAC5,
- P5_INTF_SEL_GMAC5_SGMII,
};
struct mt7530_priv;
@@ -749,6 +748,8 @@ struct mt753x_info {
* @p6_interface: Holding the current port 6 interface
* @p5_interface: Holding the current port 5 interface
* @p5_intf_sel: Holding the current port 5 interface select
+ * @p5_sgmii: Flag for distinguishing if port 5 of the MT7531 switch
+ * has got SGMII
* @irq: IRQ number of the switch
* @irq_domain: IRQ domain of the switch irq_chip
* @irq_enable: IRQ enable bits, synced to SYS_INT_EN
@@ -769,6 +770,7 @@ struct mt7530_priv {
phy_interface_t p6_interface;
phy_interface_t p5_interface;
enum p5_interface_select p5_intf_sel;
+ bool p5_sgmii;
u8 mirror_rx;
u8 mirror_tx;
struct mt7530_port ports[MT7530_NUM_PORTS];
@@ -778,7 +780,7 @@ struct mt7530_priv {
int irq;
struct irq_domain *irq_domain;
u32 irq_enable;
- int (*create_sgmii)(struct mt7530_priv *priv, bool dual_sgmii);
+ int (*create_sgmii)(struct mt7530_priv *priv);
};
struct mt7530_hw_vlan_entry {
--
2.37.2
From: Arınç ÜNAL <[email protected]>
There's no logic to numerically order the CPU ports. State the port number
and its capability of being used as a CPU port instead.
Remove the irrelevant PHY muxing information from
mt7530_mac_port_get_caps(). Explain the supported MII modes instead.
Remove the out of place PHY muxing information from
mt753x_phylink_mac_config(). The function is for both the MT7530 and MT7531
switches but there's no PHY muxing on MT7531.
These comments were gradually introduced with the commits below.
ca366d6c889b ("net: dsa: mt7530: Convert to PHYLINK API")
38f790a80560 ("net: dsa: mt7530: Add support for port 5")
88bdef8be9f6 ("net: dsa: mt7530: Extend device data ready for adding a new
hardware")
c288575f7810 ("net: dsa: mt7530: Add the support of MT7531 switch")
Signed-off-by: Arınç ÜNAL <[email protected]>
Acked-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/mt7530.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 29abf2745294..d0eae8e8c41d 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2498,7 +2498,9 @@ static void mt7530_mac_port_get_caps(struct dsa_switch *ds, int port,
config->supported_interfaces);
break;
- case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
+ case 5: /* Port 5 which can be used as a CPU port supports rgmii with
+ * delays, mii, and gmii.
+ */
phy_interface_set_rgmii(config->supported_interfaces);
__set_bit(PHY_INTERFACE_MODE_MII,
config->supported_interfaces);
@@ -2506,7 +2508,9 @@ static void mt7530_mac_port_get_caps(struct dsa_switch *ds, int port,
config->supported_interfaces);
break;
- case 6: /* 1st cpu port */
+ case 6: /* Port 6 which can be used as a CPU port supports rgmii and
+ * trgmii.
+ */
__set_bit(PHY_INTERFACE_MODE_RGMII,
config->supported_interfaces);
__set_bit(PHY_INTERFACE_MODE_TRGMII,
@@ -2526,14 +2530,17 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
config->supported_interfaces);
break;
- case 5: /* 2nd cpu port supports either rgmii or sgmii/8023z */
+ case 5: /* Port 5 which can be used as a CPU port supports rgmii with
+ * delays on MT7531BE, sgmii/802.3z on MT7531AE.
+ */
if (!priv->p5_sgmii) {
phy_interface_set_rgmii(config->supported_interfaces);
break;
}
fallthrough;
- case 6: /* 1st cpu port supports sgmii/8023z only */
+ case 6: /* Port 6 which can be used as a CPU port supports sgmii/802.3z.
+ */
__set_bit(PHY_INTERFACE_MODE_SGMII,
config->supported_interfaces);
__set_bit(PHY_INTERFACE_MODE_1000BASEX,
@@ -2725,7 +2732,7 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
state->interface != PHY_INTERFACE_MODE_INTERNAL)
goto unsupported;
break;
- case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
+ case 5: /* Port 5, can be used as a CPU port. */
if (priv->p5_interface == state->interface)
break;
@@ -2735,7 +2742,7 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
if (priv->p5_intf_sel != P5_DISABLED)
priv->p5_interface = state->interface;
break;
- case 6: /* 1st cpu port */
+ case 6: /* Port 6, can be used as a CPU port. */
if (priv->p6_interface == state->interface)
break;
--
2.37.2
From: Arınç ÜNAL <[email protected]>
On commit 7ef6f6f8d237 ("net: dsa: mt7530: Add MT7621 TRGMII mode support")
bit mask macros were added under the MT7530_HWTRAP register to read the
crystal frequency. However, the value given to the xtal variable on
mt7530_pad_clk_setup() is read from the MT7530_MHWTRAP register instead.
It doesn't seem to matter as my testing on MCM and standalone MT7530 shows
the value is correctly read from both registers but change it anyway.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index d0eae8e8c41d..98ef5ba0b19b 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -406,7 +406,7 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
struct mt7530_priv *priv = ds->priv;
u32 ncpo1, ssc_delta, trgint, xtal;
- xtal = mt7530_read(priv, MT7530_MHWTRAP) & HWTRAP_XTAL_MASK;
+ xtal = mt7530_read(priv, MT7530_HWTRAP) & HWTRAP_XTAL_MASK;
if (xtal == HWTRAP_XTAL_20MHZ) {
dev_err(priv->dev,
--
2.37.2
From: Arınç ÜNAL <[email protected]>
There're two code paths for setting up port 5:
mt7530_setup()
-> mt7530_setup_port5()
mt753x_phylink_mac_config()
-> mt753x_mac_config()
-> mt7530_mac_config()
-> mt7530_setup_port5()
Currently mt7530_setup_port5() from mt7530_setup() always runs. If port 5
is used as a CPU, DSA, or user port, mt7530_setup_port5() from
mt753x_phylink_mac_config() won't run. That is because priv->p5_interface
set on mt7530_setup_port5() will match state->interface on
mt753x_phylink_mac_config() which will stop running mt7530_setup_port5()
again.
mt7530_setup_port5() from mt753x_phylink_mac_config() won't run when port 5
is disabled or used for PHY muxing as port 5 won't be defined on the
devicetree.
Therefore, mt7530_setup_port5() will never run from
mt753x_phylink_mac_config().
Address this by not running mt7530_setup_port5() from mt7530_setup() if
port 5 is used as a CPU, DSA, or user port. For the cases of PHY muxing or
the port being disabled, call mt7530_setup_port5() from mt7530_setup().
Do not set priv->p5_interface on mt7530_setup_port5(). There won't be a
case where mt753x_phylink_mac_config() runs after mt7530_setup_port5()
anymore.
Do not set priv->p5_intf_sel to P5_DISABLED. It is already set to that when
"priv" is allocated.
Move setting the interface to a more specific location. It's supposed to be
overwritten if PHY muxing is detected.
Improve the comment which explain the process.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 98ef5ba0b19b..f3d238a46543 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -968,8 +968,6 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, intf_sel=%s, phy-mode=%s\n",
val, p5_intf_modes(priv->p5_intf_sel), phy_modes(interface));
- priv->p5_interface = interface;
-
unlock_exit:
mutex_unlock(&priv->reg_mutex);
}
@@ -2277,16 +2275,15 @@ mt7530_setup(struct dsa_switch *ds)
return ret;
/* Setup port 5 */
- priv->p5_intf_sel = P5_DISABLED;
- interface = PHY_INTERFACE_MODE_NA;
-
if (!dsa_is_unused_port(ds, 5)) {
priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
- ret = of_get_phy_mode(dsa_to_port(ds, 5)->dn, &interface);
- if (ret && ret != -ENODEV)
- return ret;
} else {
- /* Scan the ethernet nodes. look for GMAC1, lookup used phy */
+ /* Scan the ethernet nodes. Look for GMAC1, lookup the used PHY.
+ * Set priv->p5_intf_sel to the appropriate value if PHY muxing
+ * is detected.
+ */
+ interface = PHY_INTERFACE_MODE_NA;
+
for_each_child_of_node(dn, mac_np) {
if (!of_device_is_compatible(mac_np,
"mediatek,eth-mac"))
@@ -2317,6 +2314,8 @@ mt7530_setup(struct dsa_switch *ds)
of_node_put(phy_node);
break;
}
+
+ mt7530_setup_port5(ds, interface);
}
#ifdef CONFIG_GPIOLIB
@@ -2327,8 +2326,6 @@ mt7530_setup(struct dsa_switch *ds)
}
#endif /* CONFIG_GPIOLIB */
- mt7530_setup_port5(ds, interface);
-
/* Flush the FDB table */
ret = mt7530_fdb_cmd(priv, MT7530_FDB_FLUSH, NULL);
if (ret < 0)
--
2.37.2
From: Arınç ÜNAL <[email protected]>
There's no need to run all the code on mt7530_setup_port5() if port 5 is
disabled. The only case for calling mt7530_setup_port5() from
mt7530_setup() is when PHY muxing is enabled. That is because port 5 is not
defined as a port on the devicetree, therefore, it cannot be controlled by
phylink.
Because of this, run mt7530_setup_port5() if priv->p5_intf_sel is
P5_INTF_SEL_PHY_P0 or P5_INTF_SEL_PHY_P4. Remove the P5_DISABLED case from
mt7530_setup_port5().
Stop initialising the interface variable as the remaining cases will always
call mt7530_setup_port5() with it initialised.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index f3d238a46543..5ef348b6a4b2 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -932,9 +932,6 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
/* MT7530_P5_MODE_GMAC: P5 -> External phy or 2nd GMAC */
val &= ~MHWTRAP_P5_DIS;
break;
- case P5_DISABLED:
- interface = PHY_INTERFACE_MODE_NA;
- break;
default:
dev_err(ds->dev, "Unsupported p5_intf_sel %d\n",
priv->p5_intf_sel);
@@ -2282,8 +2279,6 @@ mt7530_setup(struct dsa_switch *ds)
* Set priv->p5_intf_sel to the appropriate value if PHY muxing
* is detected.
*/
- interface = PHY_INTERFACE_MODE_NA;
-
for_each_child_of_node(dn, mac_np) {
if (!of_device_is_compatible(mac_np,
"mediatek,eth-mac"))
@@ -2315,7 +2310,9 @@ mt7530_setup(struct dsa_switch *ds)
break;
}
- mt7530_setup_port5(ds, interface);
+ if (priv->p5_intf_sel == P5_INTF_SEL_PHY_P0 ||
+ priv->p5_intf_sel == P5_INTF_SEL_PHY_P4)
+ mt7530_setup_port5(ds, interface);
}
#ifdef CONFIG_GPIOLIB
--
2.37.2
From: Arınç ÜNAL <[email protected]>
The idea of p5_interface and p6_interface pointers is to prevent
mt753x_mac_config() from running twice for MT7531, as it's already run with
mt753x_cpu_port_enable() from mt7531_setup_common(), if the port is used as
a CPU port.
Change p5_interface and p6_interface to p5_configured and p6_configured.
Make them boolean.
Do not set them for any other reason.
The priv->p5_intf_sel check is useless as in this code path, it will always
be P5_INTF_SEL_GMAC5.
There was also no need to set priv->p5_interface and priv->p6_interface to
PHY_INTERFACE_MODE_NA on mt7530_setup() and mt7531_setup() as they would
already be set to that when "priv" is allocated. The pointers were of the
phy_interface_t enumeration type, and the first element of the enum is
PHY_INTERFACE_MODE_NA. There was nothing in between that would change this
beforehand.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
Acked-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/mt7530.c | 19 ++++---------------
drivers/net/dsa/mt7530.h | 10 ++++++----
2 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 5ef348b6a4b2..aab9ebb54d7d 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2237,8 +2237,6 @@ mt7530_setup(struct dsa_switch *ds)
val |= MHWTRAP_MANUAL;
mt7530_write(priv, MT7530_MHWTRAP, val);
- priv->p6_interface = PHY_INTERFACE_MODE_NA;
-
/* Enable and reset MIB counters */
mt7530_mib_reset(ds);
@@ -2454,10 +2452,6 @@ mt7531_setup(struct dsa_switch *ds)
mt7530_rmw(priv, MT7531_GPIO_MODE0, MT7531_GPIO0_MASK,
MT7531_GPIO0_INTERRUPT);
- /* Let phylink decide the interface later. */
- priv->p5_interface = PHY_INTERFACE_MODE_NA;
- priv->p6_interface = PHY_INTERFACE_MODE_NA;
-
/* Enable PHY core PLL, since phy_device has not yet been created
* provided for phy_[read,write]_mmd_indirect is called, we provide
* our own mt7531_ind_mmd_phy_[read,write] to complete this
@@ -2727,25 +2721,20 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
goto unsupported;
break;
case 5: /* Port 5, can be used as a CPU port. */
- if (priv->p5_interface == state->interface)
+ if (priv->p5_configured)
break;
if (mt753x_mac_config(ds, port, mode, state) < 0)
goto unsupported;
-
- if (priv->p5_intf_sel != P5_DISABLED)
- priv->p5_interface = state->interface;
break;
case 6: /* Port 6, can be used as a CPU port. */
- if (priv->p6_interface == state->interface)
+ if (priv->p6_configured)
break;
mt753x_pad_setup(ds, state);
if (mt753x_mac_config(ds, port, mode, state) < 0)
goto unsupported;
-
- priv->p6_interface = state->interface;
break;
default:
unsupported:
@@ -2853,12 +2842,12 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
else
interface = PHY_INTERFACE_MODE_2500BASEX;
- priv->p5_interface = interface;
+ priv->p5_configured = true;
break;
case 6:
interface = PHY_INTERFACE_MODE_2500BASEX;
- priv->p6_interface = interface;
+ priv->p6_configured = true;
break;
default:
return -EINVAL;
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 2602c95fd3a5..06037be5882c 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -745,8 +745,10 @@ struct mt753x_info {
* @ports: Holding the state among ports
* @reg_mutex: The lock for protecting among process accessing
* registers
- * @p6_interface: Holding the current port 6 interface
- * @p5_interface: Holding the current port 5 interface
+ * @p6_configured: Flag for distinguishing if port 6 of the MT7531 switch
+ * is already configured
+ * @p5_configured: Flag for distinguishing if port 5 of the MT7531 switch
+ * is already configured
* @p5_intf_sel: Holding the current port 5 interface select
* @p5_sgmii: Flag for distinguishing if port 5 of the MT7531 switch
* has got SGMII
@@ -767,8 +769,8 @@ struct mt7530_priv {
const struct mt753x_info *info;
unsigned int id;
bool mcm;
- phy_interface_t p6_interface;
- phy_interface_t p5_interface;
+ bool p6_configured;
+ bool p5_configured;
enum p5_interface_select p5_intf_sel;
bool p5_sgmii;
u8 mirror_rx;
--
2.37.2
From: Arınç ÜNAL <[email protected]>
mt7530_pad_clk_setup() is called if port 6 is enabled. It used to do more
things than setting up port 6. That part was moved to more appropriate
locations, mt7530_setup() and mt7530_pll_setup().
Now that all it does is set up port 6, rename it to mt7530_setup_port6(),
and move it to a more appropriate location, under mt7530_mac_config().
Leave an empty mt7530_pad_clk_setup() to satisfy the pad_setup function
pointer.
This is the call path for setting up the ports before:
mt753x_phylink_mac_config()
-> mt753x_mac_config()
-> mt7530_mac_config()
-> mt7530_setup_port5()
-> mt753x_pad_setup()
-> mt7530_pad_clk_setup()
This is after:
mt753x_phylink_mac_config()
-> mt753x_mac_config()
-> mt7530_mac_config()
-> mt7530_setup_port5()
-> mt7530_setup_port6()
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index b3db68d6939a..6815d2579f37 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -401,7 +401,7 @@ static void mt7530_pll_setup(struct mt7530_priv *priv)
/* Setup port 6 interface mode and TRGMII TX circuit */
static int
-mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
+mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
{
struct mt7530_priv *priv = ds->priv;
u32 ncpo1, ssc_delta, trgint, xtal;
@@ -473,6 +473,12 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
return 0;
}
+static int
+mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
+{
+ return 0;
+}
+
static int
mt7531_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
{
@@ -2570,12 +2576,15 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
{
struct mt7530_priv *priv = ds->priv;
+ int ret;
- /* Only need to setup port5. */
- if (port != 5)
- return 0;
-
- mt7530_setup_port5(priv->ds, interface);
+ if (port == 5) {
+ mt7530_setup_port5(priv->ds, interface);
+ } else if (port == 6) {
+ ret = mt7530_setup_port6(priv->ds, interface);
+ if (ret)
+ return ret;
+ }
return 0;
}
--
2.37.2
From: Arınç ÜNAL <[email protected]>
There're two code paths for setting up port 5:
mt7530_setup()
-> mt7530_setup_port5()
mt753x_phylink_mac_config()
-> mt753x_mac_config()
-> mt7530_mac_config()
-> mt7530_setup_port5()
On the first code path, priv->p5_intf_sel is either set to
P5_INTF_SEL_PHY_P0 or P5_INTF_SEL_PHY_P4 when mt7530_setup_port5() is run.
On the second code path, priv->p5_intf_sel is set to P5_INTF_SEL_GMAC5 when
mt7530_setup_port5() is run.
Empty the default case which will never run but is needed nonetheless to
handle all the remaining enumeration values.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index aab9ebb54d7d..b3db68d6939a 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -933,9 +933,7 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
val &= ~MHWTRAP_P5_DIS;
break;
default:
- dev_err(ds->dev, "Unsupported p5_intf_sel %d\n",
- priv->p5_intf_sel);
- goto unlock_exit;
+ break;
}
/* Setup RGMII settings */
@@ -965,7 +963,6 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, intf_sel=%s, phy-mode=%s\n",
val, p5_intf_modes(priv->p5_intf_sel), phy_modes(interface));
-unlock_exit:
mutex_unlock(&priv->reg_mutex);
}
--
2.37.2
From: Arınç ÜNAL <[email protected]>
The pad_setup function pointer was introduced with 88bdef8be9f6 ("net: dsa:
mt7530: Extend device data ready for adding a new hardware"). It was being
used to set up the core clock and port 6 of the MT7530 switch, and pll of
the MT7531 switch.
All of these were moved to more appropriate locations, and it was never
used for the switch on the MT7988 SoC. Therefore, this function pointer
hasn't got a use anymore. Remove it.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
Acked-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/mt7530.c | 36 ++----------------------------------
drivers/net/dsa/mt7530.h | 3 ---
2 files changed, 2 insertions(+), 37 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 6815d2579f37..2addd5e7fbe6 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -473,18 +473,6 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
return 0;
}
-static int
-mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
-{
- return 0;
-}
-
-static int
-mt7531_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
-{
- return 0;
-}
-
static void
mt7531_pll_setup(struct mt7530_priv *priv)
{
@@ -2563,14 +2551,6 @@ static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
}
}
-static int
-mt753x_pad_setup(struct dsa_switch *ds, const struct phylink_link_state *state)
-{
- struct mt7530_priv *priv = ds->priv;
-
- return priv->info->pad_setup(ds, state->interface);
-}
-
static int
mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
@@ -2737,8 +2717,6 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
if (priv->p6_configured)
break;
- mt753x_pad_setup(ds, state);
-
if (mt753x_mac_config(ds, port, mode, state) < 0)
goto unsupported;
break;
@@ -3040,11 +3018,6 @@ static int mt753x_set_mac_eee(struct dsa_switch *ds, int port,
return 0;
}
-static int mt7988_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
-{
- return 0;
-}
-
static int mt7988_setup(struct dsa_switch *ds)
{
struct mt7530_priv *priv = ds->priv;
@@ -3106,7 +3079,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c22 = mt7530_phy_write_c22,
.phy_read_c45 = mt7530_phy_read_c45,
.phy_write_c45 = mt7530_phy_write_c45,
- .pad_setup = mt7530_pad_clk_setup,
.mac_port_get_caps = mt7530_mac_port_get_caps,
.mac_port_config = mt7530_mac_config,
},
@@ -3118,7 +3090,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c22 = mt7530_phy_write_c22,
.phy_read_c45 = mt7530_phy_read_c45,
.phy_write_c45 = mt7530_phy_write_c45,
- .pad_setup = mt7530_pad_clk_setup,
.mac_port_get_caps = mt7530_mac_port_get_caps,
.mac_port_config = mt7530_mac_config,
},
@@ -3130,7 +3101,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c22 = mt7531_ind_c22_phy_write,
.phy_read_c45 = mt7531_ind_c45_phy_read,
.phy_write_c45 = mt7531_ind_c45_phy_write,
- .pad_setup = mt7531_pad_setup,
.cpu_port_config = mt7531_cpu_port_config,
.mac_port_get_caps = mt7531_mac_port_get_caps,
.mac_port_config = mt7531_mac_config,
@@ -3143,7 +3113,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c22 = mt7531_ind_c22_phy_write,
.phy_read_c45 = mt7531_ind_c45_phy_read,
.phy_write_c45 = mt7531_ind_c45_phy_write,
- .pad_setup = mt7988_pad_setup,
.cpu_port_config = mt7988_cpu_port_config,
.mac_port_get_caps = mt7988_mac_port_get_caps,
.mac_port_config = mt7988_mac_config,
@@ -3173,9 +3142,8 @@ mt7530_probe_common(struct mt7530_priv *priv)
/* Sanity check if these required device operations are filled
* properly.
*/
- if (!priv->info->sw_setup || !priv->info->pad_setup ||
- !priv->info->phy_read_c22 || !priv->info->phy_write_c22 ||
- !priv->info->mac_port_get_caps ||
+ if (!priv->info->sw_setup || !priv->info->phy_read_c22 ||
+ !priv->info->phy_write_c22 || !priv->info->mac_port_get_caps ||
!priv->info->mac_port_config)
return -EINVAL;
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 06037be5882c..f7a504e4c17b 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -696,8 +696,6 @@ struct mt753x_pcs {
* @phy_write_c22: Holding the way writing PHY port using C22
* @phy_read_c45: Holding the way reading PHY port using C45
* @phy_write_c45: Holding the way writing PHY port using C45
- * @pad_setup: Holding the way setting up the bus pad for a certain
- * MAC port
* @phy_mode_supported: Check if the PHY type is being supported on a certain
* port
* @mac_port_validate: Holding the way to set addition validate type for a
@@ -718,7 +716,6 @@ struct mt753x_info {
int regnum);
int (*phy_write_c45)(struct mt7530_priv *priv, int port, int devad,
int regnum, u16 val);
- int (*pad_setup)(struct dsa_switch *ds, phy_interface_t interface);
int (*cpu_port_config)(struct dsa_switch *ds, int port);
void (*mac_port_get_caps)(struct dsa_switch *ds, int port,
struct phylink_config *config);
--
2.37.2
From: Arınç ÜNAL <[email protected]>
The crystal frequency concerns the switch core. The frequency should be
checked when the switch is being set up so the driver can reject the
unsupported hardware earlier and without requiring port 6 to be used.
Move it to mt7530_setup().
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 2addd5e7fbe6..04a48829465c 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -408,13 +408,6 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
xtal = mt7530_read(priv, MT7530_HWTRAP) & HWTRAP_XTAL_MASK;
- if (xtal == HWTRAP_XTAL_20MHZ) {
- dev_err(priv->dev,
- "%s: MT7530 with a 20MHz XTAL is not supported!\n",
- __func__);
- return -EINVAL;
- }
-
switch (interface) {
case PHY_INTERFACE_MODE_RGMII:
trgint = 0;
@@ -2136,7 +2129,7 @@ mt7530_setup(struct dsa_switch *ds)
struct mt7530_dummy_poll p;
phy_interface_t interface;
struct dsa_port *cpu_dp;
- u32 id, val;
+ u32 id, val, xtal;
int ret, i;
/* The parent node of master netdev which holds the common system
@@ -2206,6 +2199,15 @@ mt7530_setup(struct dsa_switch *ds)
return -ENODEV;
}
+ xtal = mt7530_read(priv, MT7530_HWTRAP) & HWTRAP_XTAL_MASK;
+
+ if (xtal == HWTRAP_XTAL_20MHZ) {
+ dev_err(priv->dev,
+ "%s: MT7530 with a 20MHz XTAL is not supported!\n",
+ __func__);
+ return -EINVAL;
+ }
+
/* Reset the switch through internal reset */
mt7530_write(priv, MT7530_SYS_CTRL,
SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
--
2.37.2
From: Arınç ÜNAL <[email protected]>
Enable port 6 only when port 6 is being used. Update the comment on
mt7530_setup() with a better explanation.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 04a48829465c..980d59170ba2 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -404,7 +404,11 @@ static int
mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
{
struct mt7530_priv *priv = ds->priv;
- u32 ncpo1, ssc_delta, trgint, xtal;
+ u32 ncpo1, ssc_delta, trgint, xtal, val;
+
+ val = mt7530_read(priv, MT7530_MHWTRAP);
+ val &= ~MHWTRAP_P6_DIS;
+ mt7530_write(priv, MT7530_MHWTRAP, val);
xtal = mt7530_read(priv, MT7530_HWTRAP) & HWTRAP_XTAL_MASK;
@@ -2224,9 +2228,9 @@ mt7530_setup(struct dsa_switch *ds)
mt7530_rmw(priv, MT7530_TRGMII_RD(i),
RD_TAP_MASK, RD_TAP(16));
- /* Enable port 6 */
+ /* Enable PHY access and operate in manual mode */
val = mt7530_read(priv, MT7530_MHWTRAP);
- val &= ~MHWTRAP_P6_DIS & ~MHWTRAP_PHY_ACCESS;
+ val &= ~MHWTRAP_PHY_ACCESS;
val |= MHWTRAP_MANUAL;
mt7530_write(priv, MT7530_MHWTRAP, val);
--
2.37.2
From: Arınç ÜNAL <[email protected]>
This code is from before this driver was converted to phylink API. Phylink
deals with the unsupported interface cases before mt7530_setup_port6() is
run. Therefore, the default case would never run. However, it must be
defined nonetheless to handle all the remaining enumeration values, the
phy-modes.
Switch to if/else statements which simplifies the code.
Change mt7530_setup_port6() to void now that there're no error cases left.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 35 +++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 980d59170ba2..ea023e32313c 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -400,11 +400,11 @@ static void mt7530_pll_setup(struct mt7530_priv *priv)
}
/* Setup port 6 interface mode and TRGMII TX circuit */
-static int
+static void
mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
{
struct mt7530_priv *priv = ds->priv;
- u32 ncpo1, ssc_delta, trgint, xtal, val;
+ u32 ncpo1, ssc_delta, xtal, val;
val = mt7530_read(priv, MT7530_MHWTRAP);
val &= ~MHWTRAP_P6_DIS;
@@ -412,16 +412,18 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
xtal = mt7530_read(priv, MT7530_HWTRAP) & HWTRAP_XTAL_MASK;
- switch (interface) {
- case PHY_INTERFACE_MODE_RGMII:
- trgint = 0;
- break;
- case PHY_INTERFACE_MODE_TRGMII:
- trgint = 1;
+ if (interface == PHY_INTERFACE_MODE_RGMII) {
+ mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK,
+ P6_INTF_MODE(0));
+ } else {
+ mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK,
+ P6_INTF_MODE(1));
+
if (xtal == HWTRAP_XTAL_25MHZ)
ssc_delta = 0x57;
else
ssc_delta = 0x87;
+
if (priv->id == ID_MT7621) {
/* PLL frequency: 125MHz: 1.0GBit */
if (xtal == HWTRAP_XTAL_40MHZ)
@@ -434,17 +436,7 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
if (xtal == HWTRAP_XTAL_25MHZ)
ncpo1 = 0x1400;
}
- break;
- default:
- dev_err(priv->dev, "xMII interface %d not supported\n",
- interface);
- return -EINVAL;
- }
- mt7530_rmw(priv, MT7530_P6ECR, P6_INTF_MODE_MASK,
- P6_INTF_MODE(trgint));
-
- if (trgint) {
/* Disable the MT7530 TRGMII clocks */
core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN);
@@ -466,8 +458,6 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
/* Enable the MT7530 TRGMII clocks */
core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN);
}
-
- return 0;
}
static void
@@ -2562,14 +2552,11 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
{
struct mt7530_priv *priv = ds->priv;
- int ret;
if (port == 5) {
mt7530_setup_port5(priv->ds, interface);
} else if (port == 6) {
- ret = mt7530_setup_port6(priv->ds, interface);
- if (ret)
- return ret;
+ mt7530_setup_port6(priv->ds, interface);
}
return 0;
--
2.37.2
From: Arınç ÜNAL <[email protected]>
This code sets the Read Data (RD) TAP value to 16 for all TRGMII control
registers.
The for loop iterates over all the TRGMII control registers, and
mt7530_rmw() function is used to perform a read-modify-write operation on
each register's RD_TAP field to set its value to 16.
This operation is used to tune the timing of the read data signal in
TRGMII to match the TX signal of the link partner.
Run this if trgmii is being used. Since this code doesn't lower the
driving, there's no apparent benefit to run this if trgmii is not being
used.
Add a comment to explain the code.
Thanks to 趙皎宏 (Landen Chao) for pointing out what the code does.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index ea023e32313c..0108af681d50 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -404,7 +404,7 @@ static void
mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
{
struct mt7530_priv *priv = ds->priv;
- u32 ncpo1, ssc_delta, xtal, val;
+ u32 ncpo1, ssc_delta, i, xtal, val;
val = mt7530_read(priv, MT7530_MHWTRAP);
val &= ~MHWTRAP_P6_DIS;
@@ -457,6 +457,11 @@ mt7530_setup_port6(struct dsa_switch *ds, phy_interface_t interface)
/* Enable the MT7530 TRGMII clocks */
core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_TRGMIICK_EN);
+
+ /* Set the Read Data TAP value of the MT7530 TRGMII */
+ for (i = 0; i < NUM_TRGMII_CTRL; i++)
+ mt7530_rmw(priv, MT7530_TRGMII_RD(i),
+ RD_TAP_MASK, RD_TAP(16));
}
}
@@ -2214,10 +2219,6 @@ mt7530_setup(struct dsa_switch *ds)
mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
TD_DM_DRVP(8) | TD_DM_DRVN(8));
- for (i = 0; i < NUM_TRGMII_CTRL; i++)
- mt7530_rmw(priv, MT7530_TRGMII_RD(i),
- RD_TAP_MASK, RD_TAP(16));
-
/* Enable PHY access and operate in manual mode */
val = mt7530_read(priv, MT7530_MHWTRAP);
val &= ~MHWTRAP_PHY_ACCESS;
--
2.37.2
From: Arınç ÜNAL <[email protected]>
Move lowering Tx driving of rgmii on port 5 to right before lowering of Tx
driving of trgmii on port 6 on mt7530_setup().
This way, the switch should consume less power regardless of port 5 being
used.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 0108af681d50..468c50b3f43b 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -938,10 +938,6 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
/* P5 RGMII TX Clock Control: delay x */
mt7530_write(priv, MT7530_P5RGMIITXCR,
CSR_RGMII_TXC_CFG(0x10 + tx_delay));
-
- /* reduce P5 RGMII Tx driving, 8mA */
- mt7530_write(priv, MT7530_IO_DRV_CR,
- P5_IO_CLK_DRV(1) | P5_IO_DATA_DRV(1));
}
mt7530_write(priv, MT7530_MHWTRAP, val);
@@ -2214,6 +2210,10 @@ mt7530_setup(struct dsa_switch *ds)
mt7530_pll_setup(priv);
+ /* Lower P5 RGMII Tx driving, 8mA */
+ mt7530_write(priv, MT7530_IO_DRV_CR,
+ P5_IO_CLK_DRV(1) | P5_IO_DATA_DRV(1));
+
/* Lower Tx driving for TRGMII path */
for (i = 0; i < NUM_TRGMII_CTRL; i++)
mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
--
2.37.2
From: Arınç ÜNAL <[email protected]>
On the switch on the MT7988 SoC, there are only 4 PHYs. That's port 0 to 3.
Set the internal phy cases to '0 ... 3'.
There's no need to clear the config->supported_interfaces bitmap before
reporting the supported interfaces as all bits in the bitmap will already
be initialized to zero when the phylink_config structure is allocated.
There's no code that would change the bitmap beforehand. Remove it.
Fixes: 110c18bfed41 ("net: dsa: mt7530: introduce driver for MT7988 built-in switch")
Signed-off-by: Arınç ÜNAL <[email protected]>
Acked-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/mt7530.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 468c50b3f43b..aee1e4d71547 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2532,10 +2532,8 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
struct phylink_config *config)
{
- phy_interface_zero(config->supported_interfaces);
-
switch (port) {
- case 0 ... 4: /* Internal phy */
+ case 0 ... 3: /* Internal phy */
__set_bit(PHY_INTERFACE_MODE_INTERNAL,
config->supported_interfaces);
break;
--
2.37.2
From: Arınç ÜNAL <[email protected]>
For the switch on the MT7988 SoC, the code in mac_port_config for MT7988 is
not needed as the interface of the CPU port is already handled on
mt7988_mac_port_get_caps().
Make .mac_port_config optional. Before calling
priv->info->mac_port_config(), if there's no mac_port_config member in the
priv->info table, exit mt753x_mac_config() successfully.
Remove mac_port_config from the sanity check as the sanity check requires a
pointer to a mac_port_config function to be non-NULL. This will fail for
MT7988 as mac_port_config won't be a member of its info table.
Signed-off-by: Arınç ÜNAL <[email protected]>
Co-authored-by: Daniel Golle <[email protected]>
Signed-off-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/mt7530.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index aee1e4d71547..bdd3f63fe1ef 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2613,17 +2613,6 @@ static bool mt753x_is_mac_port(u32 port)
return (port == 5 || port == 6);
}
-static int
-mt7988_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
- phy_interface_t interface)
-{
- if (dsa_is_cpu_port(ds, port) &&
- interface == PHY_INTERFACE_MODE_INTERNAL)
- return 0;
-
- return -EINVAL;
-}
-
static int
mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
@@ -2664,6 +2653,9 @@ mt753x_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
{
struct mt7530_priv *priv = ds->priv;
+ if (!priv->info->mac_port_config)
+ return 0;
+
return priv->info->mac_port_config(ds, port, mode, state->interface);
}
@@ -3107,7 +3099,6 @@ const struct mt753x_info mt753x_table[] = {
.phy_write_c45 = mt7531_ind_c45_phy_write,
.cpu_port_config = mt7988_cpu_port_config,
.mac_port_get_caps = mt7988_mac_port_get_caps,
- .mac_port_config = mt7988_mac_config,
},
};
EXPORT_SYMBOL_GPL(mt753x_table);
@@ -3135,8 +3126,7 @@ mt7530_probe_common(struct mt7530_priv *priv)
* properly.
*/
if (!priv->info->sw_setup || !priv->info->phy_read_c22 ||
- !priv->info->phy_write_c22 || !priv->info->mac_port_get_caps ||
- !priv->info->mac_port_config)
+ !priv->info->phy_write_c22 || !priv->info->mac_port_get_caps)
return -EINVAL;
priv->id = priv->info->id;
--
2.37.2
From: Arınç ÜNAL <[email protected]>
Setting this register related to interrupts is only needed for the MT7530
switch. Make an exclusive check to ensure this.
Signed-off-by: Arınç ÜNAL <[email protected]>
Acked-by: Daniel Golle <[email protected]>
Tested-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/mt7530.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index bdd3f63fe1ef..651c5803706b 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2034,7 +2034,7 @@ mt7530_setup_irq(struct mt7530_priv *priv)
}
/* This register must be set for MT7530 to properly fire interrupts */
- if (priv->id != ID_MT7531)
+ if (priv->id == ID_MT7530 || priv->id == ID_MT7621)
mt7530_set(priv, MT7530_TOP_SIG_CTRL, TOP_SIG_CTRL_NORMAL);
ret = request_threaded_irq(priv->irq, NULL, mt7530_irq_thread_fn,
--
2.37.2
From: Arınç ÜNAL <[email protected]>
The p5_intf_sel pointer is used to store the information of whether PHY
muxing is used or not. PHY muxing is a feature specific to port 5 of the
MT7530 switch. Do not use it for other switch models.
Rename the pointer to p5_mode to store the mode the port is being used in.
Rename the p5_interface_select enum to mt7530_p5_mode, the string
representation to mt7530_p5_mode_str, and the enum elements.
If PHY muxing is not detected, the default mode, GMAC5, will be used.
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 61 ++++++++++++++++------------------------
drivers/net/dsa/mt7530.h | 15 +++++-----
2 files changed, 32 insertions(+), 44 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 02e6ba5a4403..62e55df273cc 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -873,19 +873,15 @@ mt7530_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
return 0;
}
-static const char *p5_intf_modes(unsigned int p5_interface)
-{
- switch (p5_interface) {
- case P5_DISABLED:
- return "DISABLED";
- case P5_INTF_SEL_PHY_P0:
- return "PHY P0";
- case P5_INTF_SEL_PHY_P4:
- return "PHY P4";
- case P5_INTF_SEL_GMAC5:
- return "GMAC5";
+static const char *mt7530_p5_mode_str(unsigned int mode)
+{
+ switch (mode) {
+ case MUX_PHY_P0:
+ return "MUX PHY P0";
+ case MUX_PHY_P4:
+ return "MUX PHY P4";
default:
- return "unknown";
+ return "GMAC5";
}
}
@@ -902,23 +898,21 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
val |= MHWTRAP_MANUAL | MHWTRAP_P5_MAC_SEL | MHWTRAP_P5_DIS;
val &= ~MHWTRAP_P5_RGMII_MODE & ~MHWTRAP_PHY0_SEL;
- switch (priv->p5_intf_sel) {
- case P5_INTF_SEL_PHY_P0:
- /* MT7530_P5_MODE_GPHY_P0: 2nd GMAC -> P5 -> P0 */
+ switch (priv->p5_mode) {
+ case MUX_PHY_P0:
+ /* MUX_PHY_P0: P0 -> P5 -> SoC MAC */
val |= MHWTRAP_PHY0_SEL;
fallthrough;
- case P5_INTF_SEL_PHY_P4:
- /* MT7530_P5_MODE_GPHY_P4: 2nd GMAC -> P5 -> P4 */
+ case MUX_PHY_P4:
+ /* MUX_PHY_P4: P4 -> P5 -> SoC MAC */
val &= ~MHWTRAP_P5_MAC_SEL & ~MHWTRAP_P5_DIS;
/* Setup the MAC by default for the cpu port */
mt7530_write(priv, MT7530_PMCR_P(5), 0x56300);
break;
- case P5_INTF_SEL_GMAC5:
- /* MT7530_P5_MODE_GMAC: P5 -> External phy or 2nd GMAC */
- val &= ~MHWTRAP_P5_DIS;
- break;
default:
+ /* GMAC5: P5 -> SoC MAC or external PHY */
+ val &= ~MHWTRAP_P5_DIS;
break;
}
@@ -942,8 +936,8 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
mt7530_write(priv, MT7530_MHWTRAP, val);
- dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, intf_sel=%s, phy-mode=%s\n",
- val, p5_intf_modes(priv->p5_intf_sel), phy_modes(interface));
+ dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, mode=%s, phy-mode=%s\n", val,
+ mt7530_p5_mode_str(priv->p5_mode), phy_modes(interface));
mutex_unlock(&priv->reg_mutex);
}
@@ -2261,13 +2255,11 @@ mt7530_setup(struct dsa_switch *ds)
if (ret)
return ret;
- /* Setup port 5 */
- if (!dsa_is_unused_port(ds, 5)) {
- priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
- } else {
+ /* Check for PHY muxing on port 5 */
+ if (dsa_is_unused_port(ds, 5)) {
/* Scan the ethernet nodes. Look for GMAC1, lookup the used PHY.
- * Set priv->p5_intf_sel to the appropriate value if PHY muxing
- * is detected.
+ * Set priv->p5_mode to the appropriate value if PHY muxing is
+ * detected.
*/
for_each_child_of_node(dn, mac_np) {
if (!of_device_is_compatible(mac_np,
@@ -2291,17 +2283,17 @@ mt7530_setup(struct dsa_switch *ds)
}
id = of_mdio_parse_addr(ds->dev, phy_node);
if (id == 0)
- priv->p5_intf_sel = P5_INTF_SEL_PHY_P0;
+ priv->p5_mode = MUX_PHY_P0;
if (id == 4)
- priv->p5_intf_sel = P5_INTF_SEL_PHY_P4;
+ priv->p5_mode = MUX_PHY_P4;
}
of_node_put(mac_np);
of_node_put(phy_node);
break;
}
- if (priv->p5_intf_sel == P5_INTF_SEL_PHY_P0 ||
- priv->p5_intf_sel == P5_INTF_SEL_PHY_P4)
+ if (priv->p5_mode == MUX_PHY_P0 ||
+ priv->p5_mode == MUX_PHY_P4)
mt7530_setup_port5(ds, interface);
}
@@ -2438,9 +2430,6 @@ mt7531_setup(struct dsa_switch *ds)
MT7531_EXT_P_MDIO_12);
}
- if (!dsa_is_unused_port(ds, 5))
- priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
-
mt7530_rmw(priv, MT7531_GPIO_MODE0, MT7531_GPIO0_MASK,
MT7531_GPIO0_INTERRUPT);
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index b7f80a487073..216081fb1c12 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -673,12 +673,11 @@ struct mt7530_port {
struct phylink_pcs *sgmii_pcs;
};
-/* Port 5 interface select definitions */
-enum p5_interface_select {
- P5_DISABLED,
- P5_INTF_SEL_PHY_P0,
- P5_INTF_SEL_PHY_P4,
- P5_INTF_SEL_GMAC5,
+/* Port 5 mode definitions of the MT7530 switch */
+enum mt7530_p5_mode {
+ GMAC5,
+ MUX_PHY_P0,
+ MUX_PHY_P4,
};
struct mt7530_priv;
@@ -746,7 +745,7 @@ struct mt753x_info {
* is already configured
* @p5_configured: Flag for distinguishing if port 5 of the MT7531 switch
* is already configured
- * @p5_intf_sel: Holding the current port 5 interface select
+ * @p5_mode: Holding the current mode of port 5 of the MT7530 switch
* @p5_sgmii: Flag for distinguishing if port 5 of the MT7531 switch
* has got SGMII
* @irq: IRQ number of the switch
@@ -768,7 +767,7 @@ struct mt7530_priv {
bool mcm;
bool p6_configured;
bool p5_configured;
- enum p5_interface_select p5_intf_sel;
+ enum mt7530_p5_mode p5_mode;
bool p5_sgmii;
u8 mirror_rx;
u8 mirror_tx;
--
2.37.2
From: Arınç ÜNAL <[email protected]>
Remove error returns on the cases where they are already handled with the
function the mac_port_get_caps member points to.
mt7531_mac_config() is also called from mt7531_cpu_port_config() outside of
phylink but the port and interface modes are already handled there.
Change the functions and the mac_port_config function pointer to void now
that there're no error returns anymore.
Remove mt753x_is_mac_port() that used to help the said error returns.
On mt7531_mac_config(), switch to if statements to simplify the code.
Remove internal phy cases from mt753x_phylink_mac_config() as there is no
configuration to be done for them. There's also no need to check the
interface mode as that's already handled with the function the
mac_port_get_caps member points to.
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
Acked-by: Daniel Golle <[email protected]>
Tested-by: Daniel Golle <[email protected]>
---
drivers/net/dsa/mt7530.c | 81 ++++++++--------------------------------
drivers/net/dsa/mt7530.h | 2 +-
2 files changed, 17 insertions(+), 66 deletions(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 0bd38323e2b6..02e6ba5a4403 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2550,7 +2550,7 @@ static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
}
}
-static int
+static void
mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
{
@@ -2561,22 +2561,14 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
} else if (port == 6) {
mt7530_setup_port6(priv->ds, interface);
}
-
- return 0;
}
-static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
- phy_interface_t interface,
- struct phy_device *phydev)
+static void mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
+ phy_interface_t interface,
+ struct phy_device *phydev)
{
u32 val;
- if (priv->p5_sgmii) {
- dev_err(priv->dev, "RGMII mode is not available for port %d\n",
- port);
- return -EINVAL;
- }
-
val = mt7530_read(priv, MT7531_CLKGEN_CTRL);
val |= GP_CLK_EN;
val &= ~GP_MODE_MASK;
@@ -2604,20 +2596,14 @@ static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
case PHY_INTERFACE_MODE_RGMII_ID:
break;
default:
- return -EINVAL;
+ break;
}
}
- mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
- return 0;
-}
-
-static bool mt753x_is_mac_port(u32 port)
-{
- return (port == 5 || port == 6);
+ mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
}
-static int
+static void
mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
phy_interface_t interface)
{
@@ -2625,42 +2611,21 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
struct phy_device *phydev;
struct dsa_port *dp;
- if (!mt753x_is_mac_port(port)) {
- dev_err(priv->dev, "port %d is not a MAC port\n", port);
- return -EINVAL;
- }
-
- switch (interface) {
- case PHY_INTERFACE_MODE_RGMII:
- case PHY_INTERFACE_MODE_RGMII_ID:
- case PHY_INTERFACE_MODE_RGMII_RXID:
- case PHY_INTERFACE_MODE_RGMII_TXID:
+ if (phy_interface_mode_is_rgmii(interface)) {
dp = dsa_to_port(ds, port);
phydev = dp->slave->phydev;
- return mt7531_rgmii_setup(priv, port, interface, phydev);
- case PHY_INTERFACE_MODE_SGMII:
- case PHY_INTERFACE_MODE_NA:
- case PHY_INTERFACE_MODE_1000BASEX:
- case PHY_INTERFACE_MODE_2500BASEX:
- /* handled in SGMII PCS driver */
- return 0;
- default:
- return -EINVAL;
+ mt7531_rgmii_setup(priv, port, interface, phydev);
}
-
- return -EINVAL;
}
-static int
+static void
mt753x_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
const struct phylink_link_state *state)
{
struct mt7530_priv *priv = ds->priv;
- if (!priv->info->mac_port_config)
- return 0;
-
- return priv->info->mac_port_config(ds, port, mode, state->interface);
+ if (priv->info->mac_port_config)
+ priv->info->mac_port_config(ds, port, mode, state->interface);
}
static struct phylink_pcs *
@@ -2689,30 +2654,18 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
u32 mcr_cur, mcr_new;
switch (port) {
- case 0 ... 4: /* Internal phy */
- if (state->interface != PHY_INTERFACE_MODE_GMII &&
- state->interface != PHY_INTERFACE_MODE_INTERNAL)
- goto unsupported;
- break;
case 5: /* Port 5, can be used as a CPU port. */
if (priv->p5_configured)
break;
- if (mt753x_mac_config(ds, port, mode, state) < 0)
- goto unsupported;
+ mt753x_mac_config(ds, port, mode, state);
break;
case 6: /* Port 6, can be used as a CPU port. */
if (priv->p6_configured)
break;
- if (mt753x_mac_config(ds, port, mode, state) < 0)
- goto unsupported;
+ mt753x_mac_config(ds, port, mode, state);
break;
- default:
-unsupported:
- dev_err(ds->dev, "%s: unsupported %s port: %i\n",
- __func__, phy_modes(state->interface), port);
- return;
}
mcr_cur = mt7530_read(priv, MT7530_PMCR_P(port));
@@ -2805,7 +2758,6 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
struct mt7530_priv *priv = ds->priv;
phy_interface_t interface;
int speed;
- int ret;
switch (port) {
case 5:
@@ -2830,9 +2782,8 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
else
speed = SPEED_1000;
- ret = mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
- if (ret)
- return ret;
+ mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
+
mt7530_write(priv, MT7530_PMCR_P(port),
PMCR_CPU_PORT_SETTING(priv->id));
mt753x_phylink_pcs_link_up(&priv->pcs[port].pcs, MLO_AN_FIXED,
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index f7a504e4c17b..b7f80a487073 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -722,7 +722,7 @@ struct mt753x_info {
void (*mac_port_validate)(struct dsa_switch *ds, int port,
phy_interface_t interface,
unsigned long *supported);
- int (*mac_port_config)(struct dsa_switch *ds, int port,
+ void (*mac_port_config)(struct dsa_switch *ds, int port,
unsigned int mode,
phy_interface_t interface);
};
--
2.37.2
From: Arınç ÜNAL <[email protected]>
The code on mt7530_pll_setup() needs to be run only on the MT7530 switch
with a 40 MHz oscillator. Introduce a check to do this.
Link: https://github.com/BPI-SINOVOIP/BPI-R2-bsp/blob/4a5dd143f2172ec97a2872fa29c7c4cd520f45b5/linux-mt/drivers/net/ethernet/mediatek/gsw_mt7623.c#L1039
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 62e55df273cc..e079b45fad07 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2206,7 +2206,8 @@ mt7530_setup(struct dsa_switch *ds)
SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
SYS_CTRL_REG_RST);
- mt7530_pll_setup(priv);
+ if (xtal == HWTRAP_XTAL_40MHZ)
+ mt7530_pll_setup(priv);
/* Lower P5 RGMII Tx driving, 8mA */
mt7530_write(priv, MT7530_IO_DRV_CR,
--
2.37.2
From: Arınç ÜNAL <[email protected]>
Force link-down on all MACs before internal reset. Let's follow suit commit
728c2af6ad8c ("net: mt7531: ensure all MACs are powered down before
reset").
Tested-by: Arınç ÜNAL <[email protected]>
Signed-off-by: Arınç ÜNAL <[email protected]>
---
drivers/net/dsa/mt7530.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 651c5803706b..0bd38323e2b6 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2203,6 +2203,10 @@ mt7530_setup(struct dsa_switch *ds)
return -EINVAL;
}
+ /* Force link-down on all MACs before internal reset */
+ for (i = 0; i < MT7530_NUM_PORTS; i++)
+ mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
+
/* Reset the switch through internal reset */
mt7530_write(priv, MT7530_SYS_CTRL,
SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
--
2.37.2
On Tue, Apr 25, 2023 at 11:29:13AM +0300, [email protected] wrote:
> From: Arınç ÜNAL <[email protected]>
>
> Introduce the p5_sgmii pointer to store the information for whether port 5
> has got SGMII or not.
The p5_sgmii your are introducing to struct mt7530_priv is a boolean
variable, and not a pointer.
>
> Move the comment about MT7531AE and MT7531BE to mt7531_setup(), where the
> switch is identified.
>
> Get rid of mt7531_dual_sgmii_supported() now that priv->p5_sgmii stores the
> information. Address the code where mt7531_dual_sgmii_supported() is used.
>
> Get rid of mt7531_is_rgmii_port() which just prints the opposite of
> priv->p5_sgmii.
>
> Remove P5_INTF_SEL_GMAC5_SGMII. The p5_interface_select enum is supposed to
> represent the mode that port 5 is being used in, not the hardware
> information of port 5. Set p5_intf_sel to P5_INTF_SEL_GMAC5 instead, if
> port 5 is not dsa_is_unused_port().
>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---
Other than the comment above this change makes sense and looks good to
me, so once you correct the commit message, you may add my Acked-by.
> drivers/net/dsa/mt7530-mdio.c | 7 ++----
> drivers/net/dsa/mt7530.c | 43 ++++++++++++-----------------------
> drivers/net/dsa/mt7530.h | 6 +++--
> 3 files changed, 21 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
> index 088533663b83..fa3ee85a99c1 100644
> --- a/drivers/net/dsa/mt7530-mdio.c
> +++ b/drivers/net/dsa/mt7530-mdio.c
> @@ -81,17 +81,14 @@ static const struct regmap_bus mt7530_regmap_bus = {
> };
>
> static int
> -mt7531_create_sgmii(struct mt7530_priv *priv, bool dual_sgmii)
> +mt7531_create_sgmii(struct mt7530_priv *priv)
> {
> struct regmap_config *mt7531_pcs_config[2] = {};
> struct phylink_pcs *pcs;
> struct regmap *regmap;
> int i, ret = 0;
>
> - /* MT7531AE has two SGMII units for port 5 and port 6
> - * MT7531BE has only one SGMII unit for port 6
> - */
> - for (i = dual_sgmii ? 0 : 1; i < 2; i++) {
> + for (i = priv->p5_sgmii ? 0 : 1; i < 2; i++) {
> mt7531_pcs_config[i] = devm_kzalloc(priv->dev,
> sizeof(struct regmap_config),
> GFP_KERNEL);
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 7d9f9563dbda..29abf2745294 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -473,15 +473,6 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
> return 0;
> }
>
> -static bool mt7531_dual_sgmii_supported(struct mt7530_priv *priv)
> -{
> - u32 val;
> -
> - val = mt7530_read(priv, MT7531_TOP_SIG_SR);
> -
> - return (val & PAD_DUAL_SGMII_EN) != 0;
> -}
> -
> static int
> mt7531_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
> {
> @@ -496,7 +487,7 @@ mt7531_pll_setup(struct mt7530_priv *priv)
> u32 xtal;
> u32 val;
>
> - if (mt7531_dual_sgmii_supported(priv))
> + if (priv->p5_sgmii)
> return;
>
> val = mt7530_read(priv, MT7531_CREV);
> @@ -907,8 +898,6 @@ static const char *p5_intf_modes(unsigned int p5_interface)
> return "PHY P4";
> case P5_INTF_SEL_GMAC5:
> return "GMAC5";
> - case P5_INTF_SEL_GMAC5_SGMII:
> - return "GMAC5_SGMII";
> default:
> return "unknown";
> }
> @@ -2440,6 +2429,12 @@ mt7531_setup(struct dsa_switch *ds)
> return -ENODEV;
> }
>
> + /* MT7531AE has got two SGMII units. One for port 5, one for port 6.
> + * MT7531BE has got only one SGMII unit which is for port 6.
> + */
> + val = mt7530_read(priv, MT7531_TOP_SIG_SR);
> + priv->p5_sgmii = !!(val & PAD_DUAL_SGMII_EN);
> +
> /* all MACs must be forced link-down before sw reset */
> for (i = 0; i < MT7530_NUM_PORTS; i++)
> mt7530_write(priv, MT7530_PMCR_P(i), MT7531_FORCE_LNK);
> @@ -2451,19 +2446,16 @@ mt7531_setup(struct dsa_switch *ds)
>
> mt7531_pll_setup(priv);
>
> - if (mt7531_dual_sgmii_supported(priv)) {
> - priv->p5_intf_sel = P5_INTF_SEL_GMAC5_SGMII;
> -
> + if (priv->p5_sgmii) {
> /* Let ds->slave_mii_bus be able to access external phy. */
> mt7530_rmw(priv, MT7531_GPIO_MODE1, MT7531_GPIO11_RG_RXD2_MASK,
> MT7531_EXT_P_MDC_11);
> mt7530_rmw(priv, MT7531_GPIO_MODE1, MT7531_GPIO12_RG_RXD3_MASK,
> MT7531_EXT_P_MDIO_12);
> - } else {
> - priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
> }
> - dev_dbg(ds->dev, "P5 support %s interface\n",
> - p5_intf_modes(priv->p5_intf_sel));
> +
> + if (!dsa_is_unused_port(ds, 5))
> + priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
>
> mt7530_rmw(priv, MT7531_GPIO_MODE0, MT7531_GPIO0_MASK,
> MT7531_GPIO0_INTERRUPT);
> @@ -2523,11 +2515,6 @@ static void mt7530_mac_port_get_caps(struct dsa_switch *ds, int port,
> }
> }
>
> -static bool mt7531_is_rgmii_port(struct mt7530_priv *priv, u32 port)
> -{
> - return (port == 5) && (priv->p5_intf_sel != P5_INTF_SEL_GMAC5_SGMII);
> -}
> -
> static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
> struct phylink_config *config)
> {
> @@ -2540,7 +2527,7 @@ static void mt7531_mac_port_get_caps(struct dsa_switch *ds, int port,
> break;
>
> case 5: /* 2nd cpu port supports either rgmii or sgmii/8023z */
> - if (mt7531_is_rgmii_port(priv, port)) {
> + if (!priv->p5_sgmii) {
> phy_interface_set_rgmii(config->supported_interfaces);
> break;
> }
> @@ -2607,7 +2594,7 @@ static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
> {
> u32 val;
>
> - if (!mt7531_is_rgmii_port(priv, port)) {
> + if (priv->p5_sgmii) {
> dev_err(priv->dev, "RGMII mode is not available for port %d\n",
> port);
> return -EINVAL;
> @@ -2860,7 +2847,7 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
>
> switch (port) {
> case 5:
> - if (mt7531_is_rgmii_port(priv, port))
> + if (!priv->p5_sgmii)
> interface = PHY_INTERFACE_MODE_RGMII;
> else
> interface = PHY_INTERFACE_MODE_2500BASEX;
> @@ -3019,7 +3006,7 @@ mt753x_setup(struct dsa_switch *ds)
> mt7530_free_irq_common(priv);
>
> if (priv->create_sgmii) {
> - ret = priv->create_sgmii(priv, mt7531_dual_sgmii_supported(priv));
> + ret = priv->create_sgmii(priv);
> if (ret && priv->irq)
> mt7530_free_irq(priv);
> }
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 415d8ea07472..2602c95fd3a5 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -679,7 +679,6 @@ enum p5_interface_select {
> P5_INTF_SEL_PHY_P0,
> P5_INTF_SEL_PHY_P4,
> P5_INTF_SEL_GMAC5,
> - P5_INTF_SEL_GMAC5_SGMII,
> };
>
> struct mt7530_priv;
> @@ -749,6 +748,8 @@ struct mt753x_info {
> * @p6_interface: Holding the current port 6 interface
> * @p5_interface: Holding the current port 5 interface
> * @p5_intf_sel: Holding the current port 5 interface select
> + * @p5_sgmii: Flag for distinguishing if port 5 of the MT7531 switch
> + * has got SGMII
> * @irq: IRQ number of the switch
> * @irq_domain: IRQ domain of the switch irq_chip
> * @irq_enable: IRQ enable bits, synced to SYS_INT_EN
> @@ -769,6 +770,7 @@ struct mt7530_priv {
> phy_interface_t p6_interface;
> phy_interface_t p5_interface;
> enum p5_interface_select p5_intf_sel;
> + bool p5_sgmii;
> u8 mirror_rx;
> u8 mirror_tx;
> struct mt7530_port ports[MT7530_NUM_PORTS];
> @@ -778,7 +780,7 @@ struct mt7530_priv {
> int irq;
> struct irq_domain *irq_domain;
> u32 irq_enable;
> - int (*create_sgmii)(struct mt7530_priv *priv, bool dual_sgmii);
> + int (*create_sgmii)(struct mt7530_priv *priv);
> };
>
> struct mt7530_hw_vlan_entry {
> --
> 2.37.2
>
On Tue, Apr 25, 2023 at 11:29:12AM +0300, [email protected] wrote:
> From: Arınç ÜNAL <[email protected]>
>
> Use the p5_interface_select enumeration as the data type for the
> p5_intf_sel field. This ensures p5_intf_sel can only take the values
> defined in the p5_interface_select enumeration.
>
> Signed-off-by: Arınç ÜNAL <[email protected]>
Acked-by: Daniel Golle <[email protected]>
> ---
> drivers/net/dsa/mt7530.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 845f5dd16d83..415d8ea07472 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -675,7 +675,7 @@ struct mt7530_port {
>
> /* Port 5 interface select definitions */
> enum p5_interface_select {
> - P5_DISABLED = 0,
> + P5_DISABLED,
> P5_INTF_SEL_PHY_P0,
> P5_INTF_SEL_PHY_P4,
> P5_INTF_SEL_GMAC5,
> @@ -768,7 +768,7 @@ struct mt7530_priv {
> bool mcm;
> phy_interface_t p6_interface;
> phy_interface_t p5_interface;
> - unsigned int p5_intf_sel;
> + enum p5_interface_select p5_intf_sel;
> u8 mirror_rx;
> u8 mirror_tx;
> struct mt7530_port ports[MT7530_NUM_PORTS];
> --
> 2.37.2
>
On Tue, Apr 25, 2023 at 11:29:19AM +0300, [email protected] wrote:
> From: Arınç ÜNAL <[email protected]>
>
> There're two code paths for setting up port 5:
>
> mt7530_setup()
> -> mt7530_setup_port5()
>
> mt753x_phylink_mac_config()
> -> mt753x_mac_config()
> -> mt7530_mac_config()
> -> mt7530_setup_port5()
>
> On the first code path, priv->p5_intf_sel is either set to
> P5_INTF_SEL_PHY_P0 or P5_INTF_SEL_PHY_P4 when mt7530_setup_port5() is run.
>
> On the second code path, priv->p5_intf_sel is set to P5_INTF_SEL_GMAC5 when
> mt7530_setup_port5() is run.
>
> Empty the default case which will never run but is needed nonetheless to
> handle all the remaining enumeration values.
If the default: case is really just unreachable code because of the
sound reasoning you presented above, then you should just remove it.
>
> Tested-by: Arınç ÜNAL <[email protected]>
> Signed-off-by: Arınç ÜNAL <[email protected]>
> ---
> drivers/net/dsa/mt7530.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index aab9ebb54d7d..b3db68d6939a 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -933,9 +933,7 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
> val &= ~MHWTRAP_P5_DIS;
> break;
> default:
> - dev_err(ds->dev, "Unsupported p5_intf_sel %d\n",
> - priv->p5_intf_sel);
> - goto unlock_exit;
> + break;
I suppose you can also rather just remove the default: case alltogether
instead of keeping it and making it a no-op.
> }
>
> /* Setup RGMII settings */
> @@ -965,7 +963,6 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
> dev_dbg(ds->dev, "Setup P5, HWTRAP=0x%x, intf_sel=%s, phy-mode=%s\n",
> val, p5_intf_modes(priv->p5_intf_sel), phy_modes(interface));
>
> -unlock_exit:
> mutex_unlock(&priv->reg_mutex);
> }
>
> --
> 2.37.2
>
On Tue, 25 Apr 2023 11:29:09 +0300 [email protected] wrote:
> This patch series is focused on simplifying the code, and improving the
> logic of the support for MT7530, MT7531, and MT7988 SoC switches.
>
> There're two fixes. One for fixing the corrupt frames using trgmii on MCM
> MT7530 with 40 MHz oscillator on certain MT7621 SoCs. The other for fixing
> the port capabilities of the switch on the MT7988 SoC.
## Form letter - net-next-closed
The merge window for v6.3 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.
Please repost when net-next reopens after May 8th.
RFC patches sent for review only are obviously welcome at any time.
--
pw-bot: defer
On 25.04.2023 18:04, Daniel Golle wrote:
> On Tue, Apr 25, 2023 at 11:29:13AM +0300, [email protected] wrote:
>> From: Arınç ÜNAL <[email protected]>
>>
>> Introduce the p5_sgmii pointer to store the information for whether port 5
>> has got SGMII or not.
>
> The p5_sgmii your are introducing to struct mt7530_priv is a boolean
> variable, and not a pointer.
I must've meant to say field.
>
>>
>> Move the comment about MT7531AE and MT7531BE to mt7531_setup(), where the
>> switch is identified.
>>
>> Get rid of mt7531_dual_sgmii_supported() now that priv->p5_sgmii stores the
>> information. Address the code where mt7531_dual_sgmii_supported() is used.
>>
>> Get rid of mt7531_is_rgmii_port() which just prints the opposite of
>> priv->p5_sgmii.
>>
>> Remove P5_INTF_SEL_GMAC5_SGMII. The p5_interface_select enum is supposed to
>> represent the mode that port 5 is being used in, not the hardware
>> information of port 5. Set p5_intf_sel to P5_INTF_SEL_GMAC5 instead, if
>> port 5 is not dsa_is_unused_port().
>>
>> Signed-off-by: Arınç ÜNAL <[email protected]>
>> ---
>
> Other than the comment above this change makes sense and looks good to
> me, so once you correct the commit message, you may add my Acked-by.
Will do, thanks.
Arınç
On 25.04.2023 18:13, Daniel Golle wrote:
> On Tue, Apr 25, 2023 at 11:29:19AM +0300, [email protected] wrote:
>> From: Arınç ÜNAL <[email protected]>
>>
>> There're two code paths for setting up port 5:
>>
>> mt7530_setup()
>> -> mt7530_setup_port5()
>>
>> mt753x_phylink_mac_config()
>> -> mt753x_mac_config()
>> -> mt7530_mac_config()
>> -> mt7530_setup_port5()
>>
>> On the first code path, priv->p5_intf_sel is either set to
>> P5_INTF_SEL_PHY_P0 or P5_INTF_SEL_PHY_P4 when mt7530_setup_port5() is run.
>>
>> On the second code path, priv->p5_intf_sel is set to P5_INTF_SEL_GMAC5 when
>> mt7530_setup_port5() is run.
>>
>> Empty the default case which will never run but is needed nonetheless to
>> handle all the remaining enumeration values.
>
> If the default: case is really just unreachable code because of the
> sound reasoning you presented above, then you should just remove it.
The compiler prints warnings if all enumeration values are not handled.
CC drivers/net/dsa/mt7530.o
CC drivers/net/dsa/mt7530-mdio.o
CC drivers/net/dsa/mt7530-mmio.o
drivers/net/dsa/mt7530.c: In function ‘mt7530_setup_port5’:
drivers/net/dsa/mt7530.c:919:9: warning: enumeration value ‘P5_DISABLED’
not handled in switch [-Wswitch]
919 | switch (priv->p5_intf_sel) {
| ^~~~~~
>
>>
>> Tested-by: Arınç ÜNAL <[email protected]>
>> Signed-off-by: Arınç ÜNAL <[email protected]>
>> ---
>> drivers/net/dsa/mt7530.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
>> index aab9ebb54d7d..b3db68d6939a 100644
>> --- a/drivers/net/dsa/mt7530.c
>> +++ b/drivers/net/dsa/mt7530.c
>> @@ -933,9 +933,7 @@ static void mt7530_setup_port5(struct dsa_switch *ds, phy_interface_t interface)
>> val &= ~MHWTRAP_P5_DIS;
>> break;
>> default:
>> - dev_err(ds->dev, "Unsupported p5_intf_sel %d\n",
>> - priv->p5_intf_sel);
>> - goto unlock_exit;
>> + break;
>
> I suppose you can also rather just remove the default: case alltogether
> instead of keeping it and making it a no-op.
I make use of the default case with ("net: dsa: mt7530: rename
p5_intf_sel and use only for MT7530 switch") so I'd rather keep this.
Arınç
On Wed, Apr 26, 2023 at 11:12:09AM +0300, Arınç ÜNAL wrote:
> On 25.04.2023 18:04, Daniel Golle wrote:
> > On Tue, Apr 25, 2023 at 11:29:13AM +0300, [email protected] wrote:
> > > From: Arınç ÜNAL <[email protected]>
> > >
> > > Introduce the p5_sgmii pointer to store the information for whether port 5
> > > has got SGMII or not.
> >
> > The p5_sgmii your are introducing to struct mt7530_priv is a boolean
> > variable, and not a pointer.
>
> I must've meant to say field.
Being just a single boolean variable also 'field' would not be the right
word here. We use 'field' as in 'bitfield', ie. usually disguised integer
types in which each bit has an assigned meaning.
>
> >
> > >
> > > Move the comment about MT7531AE and MT7531BE to mt7531_setup(), where the
> > > switch is identified.
> > >
> > > Get rid of mt7531_dual_sgmii_supported() now that priv->p5_sgmii stores the
> > > information. Address the code where mt7531_dual_sgmii_supported() is used.
> > >
> > > Get rid of mt7531_is_rgmii_port() which just prints the opposite of
> > > priv->p5_sgmii.
> > >
> > > Remove P5_INTF_SEL_GMAC5_SGMII. The p5_interface_select enum is supposed to
> > > represent the mode that port 5 is being used in, not the hardware
> > > information of port 5. Set p5_intf_sel to P5_INTF_SEL_GMAC5 instead, if
> > > port 5 is not dsa_is_unused_port().
> > >
> > > Signed-off-by: Arınç ÜNAL <[email protected]>
> > > ---
> >
> > Other than the comment above this change makes sense and looks good to
> > me, so once you correct the commit message, you may add my Acked-by.
>
> Will do, thanks.
>
> Arınç
On Wed, Apr 26, 2023 at 02:07:45PM +0100, Daniel Golle wrote:
> On Wed, Apr 26, 2023 at 11:12:09AM +0300, Arınç ÜNAL wrote:
> > On 25.04.2023 18:04, Daniel Golle wrote:
> > > On Tue, Apr 25, 2023 at 11:29:13AM +0300, [email protected] wrote:
> > > > From: Arınç ÜNAL <[email protected]>
> > > >
> > > > Introduce the p5_sgmii pointer to store the information for whether port 5
> > > > has got SGMII or not.
> > >
> > > The p5_sgmii your are introducing to struct mt7530_priv is a boolean
> > > variable, and not a pointer.
> >
> > I must've meant to say field.
>
> Being just a single boolean variable also 'field' would not be the right
> word here. We use 'field' as in 'bitfield', ie. usually disguised integer
> types in which each bit has an assigned meaning.
"field" is a perfectly legal name for a member of a C structure.
https://en.wikipedia.org/wiki/Struct_(C_programming_language)
Not to be confused with bitfield.
On Wed, Apr 26, 2023 at 05:39:44PM +0300, Vladimir Oltean wrote:
> On Wed, Apr 26, 2023 at 02:07:45PM +0100, Daniel Golle wrote:
> > On Wed, Apr 26, 2023 at 11:12:09AM +0300, Arınç ÜNAL wrote:
> > > On 25.04.2023 18:04, Daniel Golle wrote:
> > > > On Tue, Apr 25, 2023 at 11:29:13AM +0300, [email protected] wrote:
> > > > > From: Arınç ÜNAL <[email protected]>
> > > > >
> > > > > Introduce the p5_sgmii pointer to store the information for whether port 5
> > > > > has got SGMII or not.
> > > >
> > > > The p5_sgmii your are introducing to struct mt7530_priv is a boolean
> > > > variable, and not a pointer.
> > >
> > > I must've meant to say field.
> >
> > Being just a single boolean variable also 'field' would not be the right
> > word here. We use 'field' as in 'bitfield', ie. usually disguised integer
> > types in which each bit has an assigned meaning.
>
> "field" is a perfectly legal name for a member of a C structure.
> https://en.wikipedia.org/wiki/Struct_(C_programming_language)
> Not to be confused with bitfield.
Right, thank you for pointing that out.
Must have slipped off my mind that all this is inside a struct, of
course...