The current driver consists of two interface modules (SMI and MDIO) and
two family/variant modules (RTL8365MB and RTL8366RB). The SMI and MDIO
modules serve as the platform and MDIO drivers, respectively, calling
functions from the variant modules. In this setup, one interface module
can be loaded independently of the other, but both variants must be
loaded (if not disabled at build time) for any type of interface. This
approach doesn't scale well, especially with the addition of more switch
variants (e.g., RTL8366B), leading to loaded but unused modules.
Additionally, this also seems upside down, as the specific driver code
normally depends on the more generic functions and not the other way
around.
Each variant module was converted into real drivers, serving as both a
platform driver (for switches connected using the SMI interface) and an
MDIO driver (for MDIO-connected switches). The relationship between the
variant and interface modules is reversed, with the variant module now
calling both interface functions (if not disabled at build time). While
in most devices only one interface is likely used, the interface code is
significantly smaller than a variant module, consuming fewer resources
than the previous code. With variant modules now functioning as real
drivers, compatible strings are published only in a single variant
module, preventing conflicts.
The patch series introduces a new common module for functions shared by
both variants. This module also absorbs the two previous interface
modules, as they would always be loaded anyway.
The series relocates the user MII driver from realtek-smi to rtl83xx. It
is now used by MDIO-connected switches instead of the generic DSA
driver. There's a change in how this driver locates the MDIO node. It
now only searches for a child node named "mdio".
The dsa_switch in realtek_priv->ds is now embedded in the struct. It is
always in use and avoids dynamic memory allocation.
Testing has been performed with an RTL8367S (rtl8365mb) using MDIO
interface and an RTL8366RB (rtl8366) with SMI interface.
Luiz
---
Changes:
v4-v5:
1) Removed empty line at the end of files
2) Multiple kdoc fixes
3) added rtl83xx_unregister_switch and rtl83xx_shutdown
4) moved include <linux/regmap.h> from rtl83xx.h to rtl83xx.c
5) moved dev_set_drvdata to the end of rtl83xx_probe()
6) changed mii_user bus is name to devicename(switch):user_mii
v3-v4:
1) Changed Makefile to use ifdef instead of dynamic variable names.
2) Added comments for all exported symbols.
3) Migrated exported symbols to REALTEK_DSA namespace.
4) renamed realtek_common to rtl83xx.
5) put the mdio node just after registration and not in driver remove.
6) rtl83xx_probe now receives a struct with regmap read/write functions
and build regmap_config dynamically.
7) pulled into a new patch the realtek_priv change from "common
realtek-dsa module".
8) pulled into a new patch the user_mii_bus setup changes from "migrate
user_mii_bus setup to realtek-dsa".
9) removed the revert "net: dsa: OF-ware slave_mii_bus" patch from the
series.
v2-v3:
1) Look for the MDIO bus searching for a child node named "mdio" instead
of the compatible string.
2) Removed the check for a phy-handle in ports. ds->user_mii_bus will
not be used anymore.
3) Dropped comments for realtek_common_{probe,register_switch}.
4) Fixed a compile error in "net: dsa: OF-ware slave_mii_bus".
5) Used the wrapper realtek_smi_driver_register instead of
platform_driver_register.
v1-v2:
1) Renamed realtek_common module to realtek-dsa.
2) Removed the warning when the MDIO node is not named "mdio."
3) ds->user_mii_bus is only assigned if all user ports do not have a
phy-handle.
4) of_node_put is now back to the driver remove method.
5) Renamed realtek_common_probe_{pre,post} to
realtek_common_{probe,register_switch}.
6) Added some comments for realtek_common_{probe,register_switch}.
7) Using dev_err_probe whenever possible.
8) Embedded priv->ds into realtek_priv, removing its dynamic
allocation.
9) Fixed realtek-common.h macros.
10) Save and check the return value in functions, even when it is the
last one.
11) Added the #if expression as a comment to #else and #endif in header
files.
12) Unregister the platform and the MDIO driver in the reverse order
they are registered.
13) Unregister the first driver if the second one failed to register.
14) Added the revert patch for "net: dsa: OF-ware slave_mii_bus."
---
Luiz Angelo Daros de Luca (11):
net: dsa: realtek: drop cleanup from realtek_ops
net: dsa: realtek: introduce REALTEK_DSA namespace
net: dsa: realtek: convert variants into real drivers
net: dsa: realtek: keep variant reference in realtek_priv
net: dsa: realtek: common rtl83xx module
net: dsa: realtek: merge rtl83xx and interface modules into realtek-dsa
net: dsa: realtek: get internal MDIO node by name
net: dsa: realtek: clean user_mii_bus setup
net: dsa: realtek: migrate user_mii_bus setup to realtek-dsa
net: dsa: realtek: use the same mii bus driver for both interfaces
net: dsa: realtek: embed dsa_switch into realtek_priv
drivers/net/dsa/realtek/Kconfig | 20 +--
drivers/net/dsa/realtek/Makefile | 13 +-
drivers/net/dsa/realtek/realtek-mdio.c | 207 ++++++-----------------
drivers/net/dsa/realtek/realtek-mdio.h | 48 ++++++
drivers/net/dsa/realtek/realtek-smi.c | 273 ++++++------------------------
drivers/net/dsa/realtek/realtek-smi.h | 48 ++++++
drivers/net/dsa/realtek/realtek.h | 12 +-
drivers/net/dsa/realtek/rtl8365mb.c | 125 ++++++++------
drivers/net/dsa/realtek/rtl8366-core.c | 22 +--
drivers/net/dsa/realtek/rtl8366rb.c | 118 +++++++------
drivers/net/dsa/realtek/rtl83xx.c | 299 +++++++++++++++++++++++++++++++++
drivers/net/dsa/realtek/rtl83xx.h | 22 +++
12 files changed, 691 insertions(+), 516 deletions(-)
---
base-commit: 2121c43f88f593eea51d483bedd638cb0623c7e2
change-id: 20240127-realtek_reverse-1b6021490c85
Best regards,
--
Luiz Angelo Daros de Luca <[email protected]>
It was never used and never referenced.
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
Reviewed-by: Alvin Šipraga <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
---
drivers/net/dsa/realtek/realtek.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
index 790488e9c667..e9ee778665b2 100644
--- a/drivers/net/dsa/realtek/realtek.h
+++ b/drivers/net/dsa/realtek/realtek.h
@@ -91,7 +91,6 @@ struct realtek_ops {
int (*detect)(struct realtek_priv *priv);
int (*reset_chip)(struct realtek_priv *priv);
int (*setup)(struct realtek_priv *priv);
- void (*cleanup)(struct realtek_priv *priv);
int (*get_mib_counter)(struct realtek_priv *priv,
int port,
struct rtl8366_mib_counter *mib,
--
2.43.0
Create a namespace to group the exported symbols.
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/realtek/realtek-mdio.c | 1 +
drivers/net/dsa/realtek/realtek-smi.c | 1 +
drivers/net/dsa/realtek/rtl8365mb.c | 1 +
drivers/net/dsa/realtek/rtl8366-core.c | 22 +++++++++++-----------
drivers/net/dsa/realtek/rtl8366rb.c | 1 +
5 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
index 292e6d087e8b..c2572463679f 100644
--- a/drivers/net/dsa/realtek/realtek-mdio.c
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -288,3 +288,4 @@ mdio_module_driver(realtek_mdio_driver);
MODULE_AUTHOR("Luiz Angelo Daros de Luca <[email protected]>");
MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(REALTEK_DSA);
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index 755546ed8db6..668336515119 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -565,3 +565,4 @@ module_platform_driver(realtek_smi_driver);
MODULE_AUTHOR("Linus Walleij <[email protected]>");
MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via SMI interface");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(REALTEK_DSA);
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
index b072045eb154..c42ee8241ca2 100644
--- a/drivers/net/dsa/realtek/rtl8365mb.c
+++ b/drivers/net/dsa/realtek/rtl8365mb.c
@@ -2177,3 +2177,4 @@ EXPORT_SYMBOL_GPL(rtl8365mb_variant);
MODULE_AUTHOR("Alvin Šipraga <[email protected]>");
MODULE_DESCRIPTION("Driver for RTL8365MB-VC ethernet switch");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(REALTEK_DSA);
diff --git a/drivers/net/dsa/realtek/rtl8366-core.c b/drivers/net/dsa/realtek/rtl8366-core.c
index 59f98d2c8769..7c6520ba3a26 100644
--- a/drivers/net/dsa/realtek/rtl8366-core.c
+++ b/drivers/net/dsa/realtek/rtl8366-core.c
@@ -34,7 +34,7 @@ int rtl8366_mc_is_used(struct realtek_priv *priv, int mc_index, int *used)
return 0;
}
-EXPORT_SYMBOL_GPL(rtl8366_mc_is_used);
+EXPORT_SYMBOL_NS_GPL(rtl8366_mc_is_used, REALTEK_DSA);
/**
* rtl8366_obtain_mc() - retrieve or allocate a VLAN member configuration
@@ -187,7 +187,7 @@ int rtl8366_set_vlan(struct realtek_priv *priv, int vid, u32 member,
return ret;
}
-EXPORT_SYMBOL_GPL(rtl8366_set_vlan);
+EXPORT_SYMBOL_NS_GPL(rtl8366_set_vlan, REALTEK_DSA);
int rtl8366_set_pvid(struct realtek_priv *priv, unsigned int port,
unsigned int vid)
@@ -217,7 +217,7 @@ int rtl8366_set_pvid(struct realtek_priv *priv, unsigned int port,
return 0;
}
-EXPORT_SYMBOL_GPL(rtl8366_set_pvid);
+EXPORT_SYMBOL_NS_GPL(rtl8366_set_pvid, REALTEK_DSA);
int rtl8366_enable_vlan4k(struct realtek_priv *priv, bool enable)
{
@@ -243,7 +243,7 @@ int rtl8366_enable_vlan4k(struct realtek_priv *priv, bool enable)
priv->vlan4k_enabled = enable;
return 0;
}
-EXPORT_SYMBOL_GPL(rtl8366_enable_vlan4k);
+EXPORT_SYMBOL_NS_GPL(rtl8366_enable_vlan4k, REALTEK_DSA);
int rtl8366_enable_vlan(struct realtek_priv *priv, bool enable)
{
@@ -265,7 +265,7 @@ int rtl8366_enable_vlan(struct realtek_priv *priv, bool enable)
return ret;
}
-EXPORT_SYMBOL_GPL(rtl8366_enable_vlan);
+EXPORT_SYMBOL_NS_GPL(rtl8366_enable_vlan, REALTEK_DSA);
int rtl8366_reset_vlan(struct realtek_priv *priv)
{
@@ -290,7 +290,7 @@ int rtl8366_reset_vlan(struct realtek_priv *priv)
return 0;
}
-EXPORT_SYMBOL_GPL(rtl8366_reset_vlan);
+EXPORT_SYMBOL_NS_GPL(rtl8366_reset_vlan, REALTEK_DSA);
int rtl8366_vlan_add(struct dsa_switch *ds, int port,
const struct switchdev_obj_port_vlan *vlan,
@@ -345,7 +345,7 @@ int rtl8366_vlan_add(struct dsa_switch *ds, int port,
return 0;
}
-EXPORT_SYMBOL_GPL(rtl8366_vlan_add);
+EXPORT_SYMBOL_NS_GPL(rtl8366_vlan_add, REALTEK_DSA);
int rtl8366_vlan_del(struct dsa_switch *ds, int port,
const struct switchdev_obj_port_vlan *vlan)
@@ -389,7 +389,7 @@ int rtl8366_vlan_del(struct dsa_switch *ds, int port,
return 0;
}
-EXPORT_SYMBOL_GPL(rtl8366_vlan_del);
+EXPORT_SYMBOL_NS_GPL(rtl8366_vlan_del, REALTEK_DSA);
void rtl8366_get_strings(struct dsa_switch *ds, int port, u32 stringset,
uint8_t *data)
@@ -403,7 +403,7 @@ void rtl8366_get_strings(struct dsa_switch *ds, int port, u32 stringset,
for (i = 0; i < priv->num_mib_counters; i++)
ethtool_puts(&data, priv->mib_counters[i].name);
}
-EXPORT_SYMBOL_GPL(rtl8366_get_strings);
+EXPORT_SYMBOL_NS_GPL(rtl8366_get_strings, REALTEK_DSA);
int rtl8366_get_sset_count(struct dsa_switch *ds, int port, int sset)
{
@@ -417,7 +417,7 @@ int rtl8366_get_sset_count(struct dsa_switch *ds, int port, int sset)
return priv->num_mib_counters;
}
-EXPORT_SYMBOL_GPL(rtl8366_get_sset_count);
+EXPORT_SYMBOL_NS_GPL(rtl8366_get_sset_count, REALTEK_DSA);
void rtl8366_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data)
{
@@ -441,4 +441,4 @@ void rtl8366_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data)
data[i] = mibvalue;
}
}
-EXPORT_SYMBOL_GPL(rtl8366_get_ethtool_stats);
+EXPORT_SYMBOL_NS_GPL(rtl8366_get_ethtool_stats, REALTEK_DSA);
diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c
index e3b6a470ca67..6661d4ba6882 100644
--- a/drivers/net/dsa/realtek/rtl8366rb.c
+++ b/drivers/net/dsa/realtek/rtl8366rb.c
@@ -1938,3 +1938,4 @@ EXPORT_SYMBOL_GPL(rtl8366rb_variant);
MODULE_AUTHOR("Linus Walleij <[email protected]>");
MODULE_DESCRIPTION("Driver for RTL8366RB ethernet switch");
MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(REALTEK_DSA);
--
2.43.0
Instead of copying values from the variant, we can keep a reference in
realtek_priv.
This is a preliminary change for sharing code betwen interfaces. It will
allow to move most of the probe into a common module while still allow
code specific to each interface to read variant fields.
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/realtek/realtek-mdio.c | 4 +---
drivers/net/dsa/realtek/realtek-smi.c | 10 ++++------
drivers/net/dsa/realtek/realtek.h | 5 ++---
3 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
index 3433f64fb0d7..5b78402b1683 100644
--- a/drivers/net/dsa/realtek/realtek-mdio.c
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -196,9 +196,7 @@ int realtek_mdio_probe(struct mdio_device *mdiodev)
priv->dev = &mdiodev->dev;
priv->chip_data = (void *)priv + sizeof(*priv);
- priv->clk_delay = var->clk_delay;
- priv->cmd_read = var->cmd_read;
- priv->cmd_write = var->cmd_write;
+ priv->variant = var;
priv->ops = var->ops;
priv->write_reg_noack = realtek_mdio_write;
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index d8a9a6a6b5bc..147260f77d2f 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -46,7 +46,7 @@
static inline void realtek_smi_clk_delay(struct realtek_priv *priv)
{
- ndelay(priv->clk_delay);
+ ndelay(priv->variant->clk_delay);
}
static void realtek_smi_start(struct realtek_priv *priv)
@@ -209,7 +209,7 @@ static int realtek_smi_read_reg(struct realtek_priv *priv, u32 addr, u32 *data)
realtek_smi_start(priv);
/* Send READ command */
- ret = realtek_smi_write_byte(priv, priv->cmd_read);
+ ret = realtek_smi_write_byte(priv, priv->variant->cmd_read);
if (ret)
goto out;
@@ -250,7 +250,7 @@ static int realtek_smi_write_reg(struct realtek_priv *priv,
realtek_smi_start(priv);
/* Send WRITE command */
- ret = realtek_smi_write_byte(priv, priv->cmd_write);
+ ret = realtek_smi_write_byte(priv, priv->variant->cmd_write);
if (ret)
goto out;
@@ -459,9 +459,7 @@ int realtek_smi_probe(struct platform_device *pdev)
/* Link forward and backward */
priv->dev = dev;
- priv->clk_delay = var->clk_delay;
- priv->cmd_read = var->cmd_read;
- priv->cmd_write = var->cmd_write;
+ priv->variant = var;
priv->ops = var->ops;
priv->setup_interface = realtek_smi_setup_mdio;
diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
index e9ee778665b2..c7d5ef99e9db 100644
--- a/drivers/net/dsa/realtek/realtek.h
+++ b/drivers/net/dsa/realtek/realtek.h
@@ -58,9 +58,8 @@ struct realtek_priv {
struct mii_bus *bus;
int mdio_addr;
- unsigned int clk_delay;
- u8 cmd_read;
- u8 cmd_write;
+ const struct realtek_variant *variant;
+
spinlock_t lock; /* Locks around command writes */
struct dsa_switch *ds;
struct irq_domain *irqdomain;
--
2.43.0
Some code can be shared between both interface modules (MDIO and SMI)
and among variants. These interface functions migrated to a common
module:
- rtl83xx_lock
- rtl83xx_unlock
- rtl83xx_probe
- rtl83xx_register_switch
- rtl83xx_unregister_switch
- rtl83xx_shutdown
- rtl83xx_remove
The reset during probe was moved to the end of the common probe. This way,
we avoid a reset if anything else fails.
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
---
drivers/net/dsa/realtek/Makefile | 2 +
drivers/net/dsa/realtek/realtek-mdio.c | 154 +++-------------------
drivers/net/dsa/realtek/realtek-smi.c | 160 ++++------------------
drivers/net/dsa/realtek/realtek.h | 1 +
drivers/net/dsa/realtek/rtl8365mb.c | 9 +-
drivers/net/dsa/realtek/rtl8366rb.c | 9 +-
drivers/net/dsa/realtek/rtl83xx.c | 233 +++++++++++++++++++++++++++++++++
drivers/net/dsa/realtek/rtl83xx.h | 21 +++
8 files changed, 314 insertions(+), 275 deletions(-)
diff --git a/drivers/net/dsa/realtek/Makefile b/drivers/net/dsa/realtek/Makefile
index 0aab57252a7c..67b5ee1c43a9 100644
--- a/drivers/net/dsa/realtek/Makefile
+++ b/drivers/net/dsa/realtek/Makefile
@@ -1,4 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_NET_DSA_REALTEK) += realtek-dsa.o
+realtek-dsa-objs := rtl83xx.o
obj-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o
obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o
obj-$(CONFIG_NET_DSA_REALTEK_RTL8366RB) += rtl8366.o
diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
index 5b78402b1683..7cbce3e893e1 100644
--- a/drivers/net/dsa/realtek/realtek-mdio.c
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -26,6 +26,7 @@
#include "realtek.h"
#include "realtek-mdio.h"
+#include "rtl83xx.h"
/* Read/write via mdiobus */
#define REALTEK_MDIO_CTRL0_REG 31
@@ -100,149 +101,41 @@ static int realtek_mdio_read(void *ctx, u32 reg, u32 *val)
return ret;
}
-static void realtek_mdio_lock(void *ctx)
-{
- struct realtek_priv *priv = ctx;
-
- mutex_lock(&priv->map_lock);
-}
-
-static void realtek_mdio_unlock(void *ctx)
-{
- struct realtek_priv *priv = ctx;
-
- mutex_unlock(&priv->map_lock);
-}
-
-static const struct regmap_config realtek_mdio_regmap_config = {
- .reg_bits = 10, /* A4..A0 R4..R0 */
- .val_bits = 16,
- .reg_stride = 1,
- /* PHY regs are at 0x8000 */
- .max_register = 0xffff,
- .reg_format_endian = REGMAP_ENDIAN_BIG,
- .reg_read = realtek_mdio_read,
- .reg_write = realtek_mdio_write,
- .cache_type = REGCACHE_NONE,
- .lock = realtek_mdio_lock,
- .unlock = realtek_mdio_unlock,
-};
-
-static const struct regmap_config realtek_mdio_nolock_regmap_config = {
- .reg_bits = 10, /* A4..A0 R4..R0 */
- .val_bits = 16,
- .reg_stride = 1,
- /* PHY regs are at 0x8000 */
- .max_register = 0xffff,
- .reg_format_endian = REGMAP_ENDIAN_BIG,
+static const struct realtek_interface_info realtek_mdio_info = {
.reg_read = realtek_mdio_read,
.reg_write = realtek_mdio_write,
- .cache_type = REGCACHE_NONE,
- .disable_locking = true,
};
/**
* realtek_mdio_probe() - Probe a platform device for an MDIO-connected switch
* @mdiodev: mdio_device to probe on.
*
- * This function should be used as the .probe in an mdio_driver. It
- * initializes realtek_priv and read data from the device-tree node. The switch
- * is hard resetted if a method is provided. It checks the switch chip ID and,
- * finally, a DSA switch is registered.
+ * This function should be used as the .probe in an mdio_driver. After
+ * calling the common probe function for both interfaces, it initializes the
+ * values specific for MDIO-connected devices. Finally, it calls a common
+ * function to register the DSA switch.
*
* Context: Can sleep. Takes and releases priv->map_lock.
* Return: Returns 0 on success, a negative error on failure.
*/
int realtek_mdio_probe(struct mdio_device *mdiodev)
{
- struct realtek_priv *priv;
struct device *dev = &mdiodev->dev;
- const struct realtek_variant *var;
- struct regmap_config rc;
- struct device_node *np;
+ struct realtek_priv *priv;
int ret;
- var = of_device_get_match_data(dev);
- if (!var)
- return -EINVAL;
-
- priv = devm_kzalloc(&mdiodev->dev,
- size_add(sizeof(*priv), var->chip_data_sz),
- GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
- mutex_init(&priv->map_lock);
-
- rc = realtek_mdio_regmap_config;
- rc.lock_arg = priv;
- priv->map = devm_regmap_init(dev, NULL, priv, &rc);
- if (IS_ERR(priv->map)) {
- ret = PTR_ERR(priv->map);
- dev_err(dev, "regmap init failed: %d\n", ret);
- return ret;
- }
-
- rc = realtek_mdio_nolock_regmap_config;
- priv->map_nolock = devm_regmap_init(dev, NULL, priv, &rc);
- if (IS_ERR(priv->map_nolock)) {
- ret = PTR_ERR(priv->map_nolock);
- dev_err(dev, "regmap init failed: %d\n", ret);
- return ret;
- }
+ priv = rtl83xx_probe(dev, &realtek_mdio_info);
+ if (IS_ERR(priv))
+ return PTR_ERR(priv);
- priv->mdio_addr = mdiodev->addr;
priv->bus = mdiodev->bus;
- priv->dev = &mdiodev->dev;
- priv->chip_data = (void *)priv + sizeof(*priv);
-
- priv->variant = var;
- priv->ops = var->ops;
-
+ priv->mdio_addr = mdiodev->addr;
priv->write_reg_noack = realtek_mdio_write;
+ priv->ds_ops = priv->variant->ds_ops_mdio;
- np = dev->of_node;
-
- dev_set_drvdata(dev, priv);
-
- /* TODO: if power is software controlled, set up any regulators here */
- priv->leds_disabled = of_property_read_bool(np, "realtek,disable-leds");
-
- priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(priv->reset)) {
- dev_err(dev, "failed to get RESET GPIO\n");
- return PTR_ERR(priv->reset);
- }
-
- if (priv->reset) {
- gpiod_set_value(priv->reset, 1);
- dev_dbg(dev, "asserted RESET\n");
- msleep(REALTEK_HW_STOP_DELAY);
- gpiod_set_value(priv->reset, 0);
- msleep(REALTEK_HW_START_DELAY);
- dev_dbg(dev, "deasserted RESET\n");
- }
-
- ret = priv->ops->detect(priv);
- if (ret) {
- dev_err(dev, "unable to detect switch\n");
- return ret;
- }
-
- priv->ds = devm_kzalloc(dev, sizeof(*priv->ds), GFP_KERNEL);
- if (!priv->ds)
- return -ENOMEM;
-
- priv->ds->dev = dev;
- priv->ds->num_ports = priv->num_ports;
- priv->ds->priv = priv;
- priv->ds->ops = var->ds_ops_mdio;
-
- ret = dsa_register_switch(priv->ds);
- if (ret) {
- dev_err(priv->dev, "unable to register switch ret = %d\n", ret);
+ ret = rtl83xx_register_switch(priv);
+ if (ret)
return ret;
- }
return 0;
}
@@ -253,8 +146,7 @@ EXPORT_SYMBOL_NS_GPL(realtek_mdio_probe, REALTEK_DSA);
* @mdiodev: mdio_device to be removed.
*
* This function should be used as the .remove_new in an mdio_driver. First
- * it unregisters the DSA switch and cleans internal data. If a method is
- * provided, the hard reset is asserted to avoid traffic leakage.
+ * it unregisters the DSA switch and then it calls the common remove function.
*
* Context: Can sleep.
* Return: Nothing.
@@ -266,11 +158,9 @@ void realtek_mdio_remove(struct mdio_device *mdiodev)
if (!priv)
return;
- dsa_unregister_switch(priv->ds);
+ rtl83xx_unregister_switch(priv);
- /* leave the device reset asserted */
- if (priv->reset)
- gpiod_set_value(priv->reset, 1);
+ rtl83xx_remove(priv);
}
EXPORT_SYMBOL_NS_GPL(realtek_mdio_remove, REALTEK_DSA);
@@ -278,10 +168,8 @@ EXPORT_SYMBOL_NS_GPL(realtek_mdio_remove, REALTEK_DSA);
* realtek_mdio_shutdown() - Shutdown the driver of a MDIO-connected switch
* @mdiodev: mdio_device shutting down.
*
- * This function should be used as the .shutdown in an mdio_driver. It shuts
- * down the DSA switch and cleans the platform driver data, to prevent
- * realtek_mdio_remove() from running afterwards, which is possible if the
- * parent bus implements its own .shutdown() as .remove().
+ * This function should be used as the .shutdown in a platform_driver. It calls
+ * the common shutdown function.
*
* Context: Can sleep.
* Return: Nothing.
@@ -293,9 +181,7 @@ void realtek_mdio_shutdown(struct mdio_device *mdiodev)
if (!priv)
return;
- dsa_switch_shutdown(priv->ds);
-
- dev_set_drvdata(&mdiodev->dev, NULL);
+ rtl83xx_shutdown(priv);
}
EXPORT_SYMBOL_NS_GPL(realtek_mdio_shutdown, REALTEK_DSA);
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index 147260f77d2f..ffa68272d0d4 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -41,6 +41,7 @@
#include "realtek.h"
#include "realtek-smi.h"
+#include "rtl83xx.h"
#define REALTEK_SMI_ACK_RETRY_COUNT 5
@@ -311,47 +312,6 @@ static int realtek_smi_read(void *ctx, u32 reg, u32 *val)
return realtek_smi_read_reg(priv, reg, val);
}
-static void realtek_smi_lock(void *ctx)
-{
- struct realtek_priv *priv = ctx;
-
- mutex_lock(&priv->map_lock);
-}
-
-static void realtek_smi_unlock(void *ctx)
-{
- struct realtek_priv *priv = ctx;
-
- mutex_unlock(&priv->map_lock);
-}
-
-static const struct regmap_config realtek_smi_regmap_config = {
- .reg_bits = 10, /* A4..A0 R4..R0 */
- .val_bits = 16,
- .reg_stride = 1,
- /* PHY regs are at 0x8000 */
- .max_register = 0xffff,
- .reg_format_endian = REGMAP_ENDIAN_BIG,
- .reg_read = realtek_smi_read,
- .reg_write = realtek_smi_write,
- .cache_type = REGCACHE_NONE,
- .lock = realtek_smi_lock,
- .unlock = realtek_smi_unlock,
-};
-
-static const struct regmap_config realtek_smi_nolock_regmap_config = {
- .reg_bits = 10, /* A4..A0 R4..R0 */
- .val_bits = 16,
- .reg_stride = 1,
- /* PHY regs are at 0x8000 */
- .max_register = 0xffff,
- .reg_format_endian = REGMAP_ENDIAN_BIG,
- .reg_read = realtek_smi_read,
- .reg_write = realtek_smi_write,
- .cache_type = REGCACHE_NONE,
- .disable_locking = true,
-};
-
static int realtek_smi_mdio_read(struct mii_bus *bus, int addr, int regnum)
{
struct realtek_priv *priv = bus->priv;
@@ -409,111 +369,50 @@ static int realtek_smi_setup_mdio(struct dsa_switch *ds)
return ret;
}
+static const struct realtek_interface_info realtek_smi_info = {
+ .reg_read = realtek_smi_read,
+ .reg_write = realtek_smi_write,
+};
+
/**
* realtek_smi_probe() - Probe a platform device for an SMI-connected switch
* @pdev: platform_device to probe on.
*
- * This function should be used as the .probe in a platform_driver. It
- * initializes realtek_priv and read data from the device-tree node. The switch
- * is hard resetted if a method is provided. It checks the switch chip ID and,
- * finally, a DSA switch is registered.
+ * This function should be used as the .probe in a platform_driver. After
+ * calling the common probe function for both interfaces, it initializes the
+ * values specific for SMI-connected devices. Finally, it calls a common
+ * function to register the DSA switch.
*
* Context: Can sleep. Takes and releases priv->map_lock.
* Return: Returns 0 on success, a negative error on failure.
*/
int realtek_smi_probe(struct platform_device *pdev)
{
- const struct realtek_variant *var;
struct device *dev = &pdev->dev;
struct realtek_priv *priv;
- struct regmap_config rc;
- struct device_node *np;
int ret;
- var = of_device_get_match_data(dev);
- np = dev->of_node;
-
- priv = devm_kzalloc(dev, sizeof(*priv) + var->chip_data_sz, GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
- priv->chip_data = (void *)priv + sizeof(*priv);
-
- mutex_init(&priv->map_lock);
-
- rc = realtek_smi_regmap_config;
- rc.lock_arg = priv;
- priv->map = devm_regmap_init(dev, NULL, priv, &rc);
- if (IS_ERR(priv->map)) {
- ret = PTR_ERR(priv->map);
- dev_err(dev, "regmap init failed: %d\n", ret);
- return ret;
- }
-
- rc = realtek_smi_nolock_regmap_config;
- priv->map_nolock = devm_regmap_init(dev, NULL, priv, &rc);
- if (IS_ERR(priv->map_nolock)) {
- ret = PTR_ERR(priv->map_nolock);
- dev_err(dev, "regmap init failed: %d\n", ret);
- return ret;
- }
-
- /* Link forward and backward */
- priv->dev = dev;
- priv->variant = var;
- priv->ops = var->ops;
-
- priv->setup_interface = realtek_smi_setup_mdio;
- priv->write_reg_noack = realtek_smi_write_reg_noack;
-
- dev_set_drvdata(dev, priv);
- spin_lock_init(&priv->lock);
-
- /* TODO: if power is software controlled, set up any regulators here */
-
- priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
- if (IS_ERR(priv->reset)) {
- dev_err(dev, "failed to get RESET GPIO\n");
- return PTR_ERR(priv->reset);
- }
- if (priv->reset) {
- gpiod_set_value(priv->reset, 1);
- dev_dbg(dev, "asserted RESET\n");
- msleep(REALTEK_HW_STOP_DELAY);
- gpiod_set_value(priv->reset, 0);
- msleep(REALTEK_HW_START_DELAY);
- dev_dbg(dev, "deasserted RESET\n");
- }
+ priv = rtl83xx_probe(dev, &realtek_smi_info);
+ if (IS_ERR(priv))
+ return PTR_ERR(priv);
/* Fetch MDIO pins */
priv->mdc = devm_gpiod_get_optional(dev, "mdc", GPIOD_OUT_LOW);
if (IS_ERR(priv->mdc))
return PTR_ERR(priv->mdc);
+
priv->mdio = devm_gpiod_get_optional(dev, "mdio", GPIOD_OUT_LOW);
if (IS_ERR(priv->mdio))
return PTR_ERR(priv->mdio);
- priv->leds_disabled = of_property_read_bool(np, "realtek,disable-leds");
+ priv->write_reg_noack = realtek_smi_write_reg_noack;
+ priv->setup_interface = realtek_smi_setup_mdio;
+ priv->ds_ops = priv->variant->ds_ops_smi;
- ret = priv->ops->detect(priv);
- if (ret) {
- dev_err(dev, "unable to detect switch\n");
+ ret = rtl83xx_register_switch(priv);
+ if (ret)
return ret;
- }
-
- priv->ds = devm_kzalloc(dev, sizeof(*priv->ds), GFP_KERNEL);
- if (!priv->ds)
- return -ENOMEM;
- priv->ds->dev = dev;
- priv->ds->num_ports = priv->num_ports;
- priv->ds->priv = priv;
-
- priv->ds->ops = var->ds_ops_smi;
- ret = dsa_register_switch(priv->ds);
- if (ret) {
- dev_err_probe(dev, ret, "unable to register switch\n");
- return ret;
- }
return 0;
}
EXPORT_SYMBOL_NS_GPL(realtek_smi_probe, REALTEK_DSA);
@@ -523,8 +422,8 @@ EXPORT_SYMBOL_NS_GPL(realtek_smi_probe, REALTEK_DSA);
* @pdev: platform_device to be removed.
*
* This function should be used as the .remove_new in a platform_driver. First
- * it unregisters the DSA switch and cleans internal data. If a method is
- * provided, the hard reset is asserted to avoid traffic leakage.
+ * it unregisters the DSA switch and cleans internal data. Finally, it calls
+ * the common remove function.
*
* Context: Can sleep.
* Return: Nothing.
@@ -536,13 +435,12 @@ void realtek_smi_remove(struct platform_device *pdev)
if (!priv)
return;
- dsa_unregister_switch(priv->ds);
+ rtl83xx_unregister_switch(priv);
+
if (priv->user_mii_bus)
of_node_put(priv->user_mii_bus->dev.of_node);
- /* leave the device reset asserted */
- if (priv->reset)
- gpiod_set_value(priv->reset, 1);
+ rtl83xx_remove(priv);
}
EXPORT_SYMBOL_NS_GPL(realtek_smi_remove, REALTEK_DSA);
@@ -550,10 +448,8 @@ EXPORT_SYMBOL_NS_GPL(realtek_smi_remove, REALTEK_DSA);
* realtek_smi_shutdown() - Shutdown the driver of a SMI-connected switch
* @pdev: platform_device shutting down.
*
- * This function should be used as the .shutdown in a platform_driver. It shuts
- * down the DSA switch and cleans the platform driver data, to prevent
- * realtek_smi_remove() from running afterwards, which is possible if the
- * parent bus implements its own .shutdown() as .remove().
+ * This function should be used as the .shutdown in a platform_driver. It calls
+ * the common shutdown function.
*
* Context: Can sleep.
* Return: Nothing.
@@ -565,9 +461,7 @@ void realtek_smi_shutdown(struct platform_device *pdev)
if (!priv)
return;
- dsa_switch_shutdown(priv->ds);
-
- platform_set_drvdata(pdev, NULL);
+ rtl83xx_shutdown(priv);
}
EXPORT_SYMBOL_NS_GPL(realtek_smi_shutdown, REALTEK_DSA);
diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
index c7d5ef99e9db..e9e28b189509 100644
--- a/drivers/net/dsa/realtek/realtek.h
+++ b/drivers/net/dsa/realtek/realtek.h
@@ -62,6 +62,7 @@ struct realtek_priv {
spinlock_t lock; /* Locks around command writes */
struct dsa_switch *ds;
+ const struct dsa_switch_ops *ds_ops;
struct irq_domain *irqdomain;
bool leds_disabled;
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
index e67c4562f5db..60f826a5a00a 100644
--- a/drivers/net/dsa/realtek/rtl8365mb.c
+++ b/drivers/net/dsa/realtek/rtl8365mb.c
@@ -103,6 +103,7 @@
#include "realtek.h"
#include "realtek-smi.h"
#include "realtek-mdio.h"
+#include "rtl83xx.h"
/* Family-specific data and limits */
#define RTL8365MB_PHYADDRMAX 7
@@ -691,7 +692,7 @@ static int rtl8365mb_phy_ocp_read(struct realtek_priv *priv, int phy,
u32 val;
int ret;
- mutex_lock(&priv->map_lock);
+ rtl83xx_lock(priv);
ret = rtl8365mb_phy_poll_busy(priv);
if (ret)
@@ -724,7 +725,7 @@ static int rtl8365mb_phy_ocp_read(struct realtek_priv *priv, int phy,
*data = val & 0xFFFF;
out:
- mutex_unlock(&priv->map_lock);
+ rtl83xx_unlock(priv);
return ret;
}
@@ -735,7 +736,7 @@ static int rtl8365mb_phy_ocp_write(struct realtek_priv *priv, int phy,
u32 val;
int ret;
- mutex_lock(&priv->map_lock);
+ rtl83xx_lock(priv);
ret = rtl8365mb_phy_poll_busy(priv);
if (ret)
@@ -766,7 +767,7 @@ static int rtl8365mb_phy_ocp_write(struct realtek_priv *priv, int phy,
goto out;
out:
- mutex_unlock(&priv->map_lock);
+ rtl83xx_unlock(priv);
return 0;
}
diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c
index 747407ae8225..a0c365325b4a 100644
--- a/drivers/net/dsa/realtek/rtl8366rb.c
+++ b/drivers/net/dsa/realtek/rtl8366rb.c
@@ -25,6 +25,7 @@
#include "realtek.h"
#include "realtek-smi.h"
#include "realtek-mdio.h"
+#include "rtl83xx.h"
#define RTL8366RB_PORT_NUM_CPU 5
#define RTL8366RB_NUM_PORTS 6
@@ -1720,7 +1721,7 @@ static int rtl8366rb_phy_read(struct realtek_priv *priv, int phy, int regnum)
if (phy > RTL8366RB_PHY_NO_MAX)
return -EINVAL;
- mutex_lock(&priv->map_lock);
+ rtl83xx_lock(priv);
ret = regmap_write(priv->map_nolock, RTL8366RB_PHY_ACCESS_CTRL_REG,
RTL8366RB_PHY_CTRL_READ);
@@ -1748,7 +1749,7 @@ static int rtl8366rb_phy_read(struct realtek_priv *priv, int phy, int regnum)
phy, regnum, reg, val);
out:
- mutex_unlock(&priv->map_lock);
+ rtl83xx_unlock(priv);
return ret;
}
@@ -1762,7 +1763,7 @@ static int rtl8366rb_phy_write(struct realtek_priv *priv, int phy, int regnum,
if (phy > RTL8366RB_PHY_NO_MAX)
return -EINVAL;
- mutex_lock(&priv->map_lock);
+ rtl83xx_lock(priv);
ret = regmap_write(priv->map_nolock, RTL8366RB_PHY_ACCESS_CTRL_REG,
RTL8366RB_PHY_CTRL_WRITE);
@@ -1779,7 +1780,7 @@ static int rtl8366rb_phy_write(struct realtek_priv *priv, int phy, int regnum,
goto out;
out:
- mutex_unlock(&priv->map_lock);
+ rtl83xx_unlock(priv);
return ret;
}
diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c
new file mode 100644
index 000000000000..702ee0d40b0b
--- /dev/null
+++ b/drivers/net/dsa/realtek/rtl83xx.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "realtek.h"
+#include "rtl83xx.h"
+
+/**
+ * rtl83xx_lock() - Locks the mutex used by regmaps
+ * @ctx: realtek_priv pointer
+ *
+ * This function is passed to regmap to be used as the lock function.
+ * It is also used externally to block regmap before executing multiple
+ * operations that must happen in sequence (which will use
+ * realtek_priv.map_nolock instead).
+ *
+ * Context: Can sleep. Holds priv->map_lock lock.
+ * Return: nothing
+ */
+void rtl83xx_lock(void *ctx)
+{
+ struct realtek_priv *priv = ctx;
+
+ mutex_lock(&priv->map_lock);
+}
+EXPORT_SYMBOL_NS_GPL(rtl83xx_lock, REALTEK_DSA);
+
+/**
+ * rtl83xx_unlock() - Unlocks the mutex used by regmaps
+ * @ctx: realtek_priv pointer
+ *
+ * This function unlocks the lock acquired by rtl83xx_lock.
+ *
+ * Context: Releases priv->map_lock lock.
+ * Return: nothing
+ */
+void rtl83xx_unlock(void *ctx)
+{
+ struct realtek_priv *priv = ctx;
+
+ mutex_unlock(&priv->map_lock);
+}
+EXPORT_SYMBOL_NS_GPL(rtl83xx_unlock, REALTEK_DSA);
+
+/**
+ * rtl83xx_probe() - probe a Realtek switch
+ * @dev: the device being probed
+ * @interface_info: specific management interface info.
+ *
+ * This function initializes realtek_priv and reads data from the device tree
+ * node. The switch is hard resetted if a method is provided.
+ *
+ * Context: Can sleep.
+ * Return: Pointer to the realtek_priv or ERR_PTR() in case of failure.
+ *
+ * The realtek_priv pointer does not need to be freed as it is controlled by
+ * devres.
+ */
+struct realtek_priv *
+rtl83xx_probe(struct device *dev,
+ const struct realtek_interface_info *interface_info)
+{
+ const struct realtek_variant *var;
+ struct realtek_priv *priv;
+ struct regmap_config rc = {
+ .reg_bits = 10, /* A4..A0 R4..R0 */
+ .val_bits = 16,
+ .reg_stride = 1,
+ .max_register = 0xffff,
+ .reg_format_endian = REGMAP_ENDIAN_BIG,
+ .reg_read = interface_info->reg_read,
+ .reg_write = interface_info->reg_write,
+ .cache_type = REGCACHE_NONE,
+ .lock = rtl83xx_lock,
+ .unlock = rtl83xx_unlock,
+ };
+ int ret;
+
+ var = of_device_get_match_data(dev);
+ if (!var)
+ return ERR_PTR(-EINVAL);
+
+ priv = devm_kzalloc(dev, size_add(sizeof(*priv), var->chip_data_sz),
+ GFP_KERNEL);
+ if (!priv)
+ return ERR_PTR(-ENOMEM);
+
+ mutex_init(&priv->map_lock);
+
+ rc.lock_arg = priv;
+ priv->map = devm_regmap_init(dev, NULL, priv, &rc);
+ if (IS_ERR(priv->map)) {
+ ret = PTR_ERR(priv->map);
+ dev_err(dev, "regmap init failed: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ rc.disable_locking = true;
+ priv->map_nolock = devm_regmap_init(dev, NULL, priv, &rc);
+ if (IS_ERR(priv->map_nolock)) {
+ ret = PTR_ERR(priv->map_nolock);
+ dev_err(dev, "regmap init failed: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ /* Link forward and backward */
+ priv->dev = dev;
+ priv->variant = var;
+ priv->ops = var->ops;
+ priv->chip_data = (void *)priv + sizeof(*priv);
+
+ spin_lock_init(&priv->lock);
+
+ priv->leds_disabled = of_property_read_bool(dev->of_node,
+ "realtek,disable-leds");
+
+ /* TODO: if power is software controlled, set up any regulators here */
+ priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(priv->reset)) {
+ dev_err(dev, "failed to get RESET GPIO\n");
+ return ERR_CAST(priv->reset);
+ }
+
+ dev_set_drvdata(dev, priv);
+
+ if (priv->reset) {
+ gpiod_set_value(priv->reset, 1);
+ dev_dbg(dev, "asserted RESET\n");
+ msleep(REALTEK_HW_STOP_DELAY);
+ gpiod_set_value(priv->reset, 0);
+ msleep(REALTEK_HW_START_DELAY);
+ dev_dbg(dev, "deasserted RESET\n");
+ }
+
+ return priv;
+}
+EXPORT_SYMBOL_NS_GPL(rtl83xx_probe, REALTEK_DSA);
+
+/**
+ * rtl83xx_register_switch() - detects and register a switch
+ * @priv: realtek_priv pointer
+ *
+ * This function first checks the switch chip ID and register a DSA
+ * switch.
+ *
+ * Context: Can sleep. Takes and releases priv->map_lock.
+ * Return: 0 on success, negative value for failure.
+ */
+int rtl83xx_register_switch(struct realtek_priv *priv)
+{
+ int ret;
+
+ ret = priv->ops->detect(priv);
+ if (ret) {
+ dev_err_probe(priv->dev, ret, "unable to detect switch\n");
+ return ret;
+ }
+
+ priv->ds = devm_kzalloc(priv->dev, sizeof(*priv->ds), GFP_KERNEL);
+ if (!priv->ds)
+ return -ENOMEM;
+
+ priv->ds->priv = priv;
+ priv->ds->dev = priv->dev;
+ priv->ds->ops = priv->ds_ops;
+ priv->ds->num_ports = priv->num_ports;
+
+ ret = dsa_register_switch(priv->ds);
+ if (ret) {
+ dev_err_probe(priv->dev, ret, "unable to register switch\n");
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(rtl83xx_register_switch, REALTEK_DSA);
+
+/**
+ * rtl83xx_unregister_switch() - unregister a switch
+ * @priv: realtek_priv pointer
+ *
+ * This function unregister a DSA switch.
+ *
+ * Context: Can sleep.
+ * Return: Nothing.
+ */
+void rtl83xx_unregister_switch(struct realtek_priv *priv)
+{
+ dsa_unregister_switch(priv->ds);
+}
+EXPORT_SYMBOL_NS_GPL(rtl83xx_unregister_switch, REALTEK_DSA);
+
+/**
+ * rtl83xx_shutdown() - shutdown a switch
+ * @priv: realtek_priv pointer
+ *
+ * This function shuts down the DSA switch and cleans the platform driver data,
+ * to prevent realtek_{smi,mdio}_remove() from running afterwards, which is
+ * possible if the parent bus implements its own .shutdown() as .remove().
+ *
+ * Context: Can sleep.
+ * Return: Nothing.
+ */
+void rtl83xx_shutdown(struct realtek_priv *priv)
+{
+ dsa_switch_shutdown(priv->ds);
+
+ dev_set_drvdata(priv->dev, NULL);
+}
+EXPORT_SYMBOL_NS_GPL(rtl83xx_shutdown, REALTEK_DSA);
+
+/**
+ * rtl83xx_remove() - Cleanup a realtek switch driver
+ * @priv: realtek_priv pointer
+ *
+ * If a method is provided, this function asserts the hard reset of the switch
+ * in order to avoid leaking traffic when the driver is gone.
+ *
+ * Context: Might sleep if priv->gdev->chip->can_sleep.
+ * Return: nothing
+ */
+void rtl83xx_remove(struct realtek_priv *priv)
+{
+ /* leave the device reset asserted */
+ if (priv->reset)
+ gpiod_set_value(priv->reset, 1);
+}
+EXPORT_SYMBOL_NS_GPL(rtl83xx_remove, REALTEK_DSA);
+
+MODULE_AUTHOR("Luiz Angelo Daros de Luca <[email protected]>");
+MODULE_DESCRIPTION("Realtek DSA switches common module");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/realtek/rtl83xx.h b/drivers/net/dsa/realtek/rtl83xx.h
new file mode 100644
index 000000000000..a8eed92bce1a
--- /dev/null
+++ b/drivers/net/dsa/realtek/rtl83xx.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _RTL83XX_H
+#define _RTL83XX_H
+
+struct realtek_interface_info {
+ int (*reg_read)(void *ctx, u32 reg, u32 *val);
+ int (*reg_write)(void *ctx, u32 reg, u32 val);
+};
+
+void rtl83xx_lock(void *ctx);
+void rtl83xx_unlock(void *ctx);
+struct realtek_priv *
+rtl83xx_probe(struct device *dev,
+ const struct realtek_interface_info *interface_info);
+int rtl83xx_register_switch(struct realtek_priv *priv);
+void rtl83xx_unregister_switch(struct realtek_priv *priv);
+void rtl83xx_shutdown(struct realtek_priv *priv);
+void rtl83xx_remove(struct realtek_priv *priv);
+
+#endif /* _RTL83XX_H */
--
2.43.0
Since rtl83xx and realtek-{smi,mdio} are always loaded together,
we can optimize resource usage by consolidating them into a single
module.
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/realtek/Kconfig | 4 ++--
drivers/net/dsa/realtek/Makefile | 11 +++++++++--
drivers/net/dsa/realtek/realtek-mdio.c | 5 -----
drivers/net/dsa/realtek/realtek-smi.c | 5 -----
drivers/net/dsa/realtek/rtl83xx.c | 1 +
5 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig
index 9d182fde11b4..6989972eebc3 100644
--- a/drivers/net/dsa/realtek/Kconfig
+++ b/drivers/net/dsa/realtek/Kconfig
@@ -16,14 +16,14 @@ menuconfig NET_DSA_REALTEK
if NET_DSA_REALTEK
config NET_DSA_REALTEK_MDIO
- tristate "Realtek MDIO interface support"
+ bool "Realtek MDIO interface support"
depends on OF
help
Select to enable support for registering switches configured
through MDIO.
config NET_DSA_REALTEK_SMI
- tristate "Realtek SMI interface support"
+ bool "Realtek SMI interface support"
depends on OF
help
Select to enable support for registering switches connected
diff --git a/drivers/net/dsa/realtek/Makefile b/drivers/net/dsa/realtek/Makefile
index 67b5ee1c43a9..6ed6b4598d2e 100644
--- a/drivers/net/dsa/realtek/Makefile
+++ b/drivers/net/dsa/realtek/Makefile
@@ -1,8 +1,15 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_NET_DSA_REALTEK) += realtek-dsa.o
realtek-dsa-objs := rtl83xx.o
-obj-$(CONFIG_NET_DSA_REALTEK_MDIO) += realtek-mdio.o
-obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o
+
+ifdef CONFIG_NET_DSA_REALTEK_MDIO
+realtek-dsa-objs += realtek-mdio.o
+endif
+
+ifdef CONFIG_NET_DSA_REALTEK_SMI
+realtek-dsa-objs += realtek-smi.o
+endif
+
obj-$(CONFIG_NET_DSA_REALTEK_RTL8366RB) += rtl8366.o
rtl8366-objs := rtl8366-core.o rtl8366rb.o
obj-$(CONFIG_NET_DSA_REALTEK_RTL8365MB) += rtl8365mb.o
diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
index 7cbce3e893e1..45d9ef43fe3d 100644
--- a/drivers/net/dsa/realtek/realtek-mdio.c
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -184,8 +184,3 @@ void realtek_mdio_shutdown(struct mdio_device *mdiodev)
rtl83xx_shutdown(priv);
}
EXPORT_SYMBOL_NS_GPL(realtek_mdio_shutdown, REALTEK_DSA);
-
-MODULE_AUTHOR("Luiz Angelo Daros de Luca <[email protected]>");
-MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface");
-MODULE_LICENSE("GPL");
-MODULE_IMPORT_NS(REALTEK_DSA);
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index ffa68272d0d4..67bdad7ac910 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -464,8 +464,3 @@ void realtek_smi_shutdown(struct platform_device *pdev)
rtl83xx_shutdown(priv);
}
EXPORT_SYMBOL_NS_GPL(realtek_smi_shutdown, REALTEK_DSA);
-
-MODULE_AUTHOR("Linus Walleij <[email protected]>");
-MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via SMI interface");
-MODULE_LICENSE("GPL");
-MODULE_IMPORT_NS(REALTEK_DSA);
diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c
index 702ee0d40b0b..fb3b3b2305d1 100644
--- a/drivers/net/dsa/realtek/rtl83xx.c
+++ b/drivers/net/dsa/realtek/rtl83xx.c
@@ -229,5 +229,6 @@ void rtl83xx_remove(struct realtek_priv *priv)
EXPORT_SYMBOL_NS_GPL(rtl83xx_remove, REALTEK_DSA);
MODULE_AUTHOR("Luiz Angelo Daros de Luca <[email protected]>");
+MODULE_AUTHOR("Linus Walleij <[email protected]>");
MODULE_DESCRIPTION("Realtek DSA switches common module");
MODULE_LICENSE("GPL");
--
2.43.0
Previously, the interface modules realtek-smi and realtek-mdio served as
a platform and an MDIO driver, respectively. Each interface module
redundantly specified the same compatible strings for both variants and
referenced symbols from the variants.
Now, each variant module has been transformed into a unified driver
serving both as a platform and an MDIO driver. This modification
reverses the relationship between the interface and variant modules,
with the variant module now utilizing symbols from the interface
modules.
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
---
drivers/net/dsa/realtek/Kconfig | 20 +++-------
drivers/net/dsa/realtek/realtek-mdio.c | 68 +++++++++++++++++++------------
drivers/net/dsa/realtek/realtek-mdio.h | 48 ++++++++++++++++++++++
drivers/net/dsa/realtek/realtek-smi.c | 73 +++++++++++++++++++---------------
drivers/net/dsa/realtek/realtek-smi.h | 48 ++++++++++++++++++++++
drivers/net/dsa/realtek/rtl8365mb.c | 54 ++++++++++++++++++++++++-
drivers/net/dsa/realtek/rtl8366rb.c | 54 ++++++++++++++++++++++++-
7 files changed, 292 insertions(+), 73 deletions(-)
diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig
index 060165a85fb7..9d182fde11b4 100644
--- a/drivers/net/dsa/realtek/Kconfig
+++ b/drivers/net/dsa/realtek/Kconfig
@@ -16,37 +16,29 @@ menuconfig NET_DSA_REALTEK
if NET_DSA_REALTEK
config NET_DSA_REALTEK_MDIO
- tristate "Realtek MDIO interface driver"
+ tristate "Realtek MDIO interface support"
depends on OF
- depends on NET_DSA_REALTEK_RTL8365MB || NET_DSA_REALTEK_RTL8366RB
- depends on NET_DSA_REALTEK_RTL8365MB || !NET_DSA_REALTEK_RTL8365MB
- depends on NET_DSA_REALTEK_RTL8366RB || !NET_DSA_REALTEK_RTL8366RB
help
Select to enable support for registering switches configured
through MDIO.
config NET_DSA_REALTEK_SMI
- tristate "Realtek SMI interface driver"
+ tristate "Realtek SMI interface support"
depends on OF
- depends on NET_DSA_REALTEK_RTL8365MB || NET_DSA_REALTEK_RTL8366RB
- depends on NET_DSA_REALTEK_RTL8365MB || !NET_DSA_REALTEK_RTL8365MB
- depends on NET_DSA_REALTEK_RTL8366RB || !NET_DSA_REALTEK_RTL8366RB
help
Select to enable support for registering switches connected
through SMI.
config NET_DSA_REALTEK_RTL8365MB
- tristate "Realtek RTL8365MB switch subdriver"
- imply NET_DSA_REALTEK_SMI
- imply NET_DSA_REALTEK_MDIO
+ tristate "Realtek RTL8365MB switch driver"
+ depends on NET_DSA_REALTEK_SMI || NET_DSA_REALTEK_MDIO
select NET_DSA_TAG_RTL8_4
help
Select to enable support for Realtek RTL8365MB-VC and RTL8367S.
config NET_DSA_REALTEK_RTL8366RB
- tristate "Realtek RTL8366RB switch subdriver"
- imply NET_DSA_REALTEK_SMI
- imply NET_DSA_REALTEK_MDIO
+ tristate "Realtek RTL8366RB switch driver"
+ depends on NET_DSA_REALTEK_SMI || NET_DSA_REALTEK_MDIO
select NET_DSA_TAG_RTL4_A
help
Select to enable support for Realtek RTL8366RB.
diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
index c2572463679f..3433f64fb0d7 100644
--- a/drivers/net/dsa/realtek/realtek-mdio.c
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -25,6 +25,7 @@
#include <linux/regmap.h>
#include "realtek.h"
+#include "realtek-mdio.h"
/* Read/write via mdiobus */
#define REALTEK_MDIO_CTRL0_REG 31
@@ -140,7 +141,19 @@ static const struct regmap_config realtek_mdio_nolock_regmap_config = {
.disable_locking = true,
};
-static int realtek_mdio_probe(struct mdio_device *mdiodev)
+/**
+ * realtek_mdio_probe() - Probe a platform device for an MDIO-connected switch
+ * @mdiodev: mdio_device to probe on.
+ *
+ * This function should be used as the .probe in an mdio_driver. It
+ * initializes realtek_priv and read data from the device-tree node. The switch
+ * is hard resetted if a method is provided. It checks the switch chip ID and,
+ * finally, a DSA switch is registered.
+ *
+ * Context: Can sleep. Takes and releases priv->map_lock.
+ * Return: Returns 0 on success, a negative error on failure.
+ */
+int realtek_mdio_probe(struct mdio_device *mdiodev)
{
struct realtek_priv *priv;
struct device *dev = &mdiodev->dev;
@@ -235,8 +248,20 @@ static int realtek_mdio_probe(struct mdio_device *mdiodev)
return 0;
}
+EXPORT_SYMBOL_NS_GPL(realtek_mdio_probe, REALTEK_DSA);
-static void realtek_mdio_remove(struct mdio_device *mdiodev)
+/**
+ * realtek_mdio_remove() - Remove the driver of an MDIO-connected switch
+ * @mdiodev: mdio_device to be removed.
+ *
+ * This function should be used as the .remove_new in an mdio_driver. First
+ * it unregisters the DSA switch and cleans internal data. If a method is
+ * provided, the hard reset is asserted to avoid traffic leakage.
+ *
+ * Context: Can sleep.
+ * Return: Nothing.
+ */
+void realtek_mdio_remove(struct mdio_device *mdiodev)
{
struct realtek_priv *priv = dev_get_drvdata(&mdiodev->dev);
@@ -249,8 +274,21 @@ static void realtek_mdio_remove(struct mdio_device *mdiodev)
if (priv->reset)
gpiod_set_value(priv->reset, 1);
}
+EXPORT_SYMBOL_NS_GPL(realtek_mdio_remove, REALTEK_DSA);
-static void realtek_mdio_shutdown(struct mdio_device *mdiodev)
+/**
+ * realtek_mdio_shutdown() - Shutdown the driver of a MDIO-connected switch
+ * @mdiodev: mdio_device shutting down.
+ *
+ * This function should be used as the .shutdown in an mdio_driver. It shuts
+ * down the DSA switch and cleans the platform driver data, to prevent
+ * realtek_mdio_remove() from running afterwards, which is possible if the
+ * parent bus implements its own .shutdown() as .remove().
+ *
+ * Context: Can sleep.
+ * Return: Nothing.
+ */
+void realtek_mdio_shutdown(struct mdio_device *mdiodev)
{
struct realtek_priv *priv = dev_get_drvdata(&mdiodev->dev);
@@ -261,29 +299,7 @@ static void realtek_mdio_shutdown(struct mdio_device *mdiodev)
dev_set_drvdata(&mdiodev->dev, NULL);
}
-
-static const struct of_device_id realtek_mdio_of_match[] = {
-#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8366RB)
- { .compatible = "realtek,rtl8366rb", .data = &rtl8366rb_variant, },
-#endif
-#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8365MB)
- { .compatible = "realtek,rtl8365mb", .data = &rtl8365mb_variant, },
-#endif
- { /* sentinel */ },
-};
-MODULE_DEVICE_TABLE(of, realtek_mdio_of_match);
-
-static struct mdio_driver realtek_mdio_driver = {
- .mdiodrv.driver = {
- .name = "realtek-mdio",
- .of_match_table = realtek_mdio_of_match,
- },
- .probe = realtek_mdio_probe,
- .remove = realtek_mdio_remove,
- .shutdown = realtek_mdio_shutdown,
-};
-
-mdio_module_driver(realtek_mdio_driver);
+EXPORT_SYMBOL_NS_GPL(realtek_mdio_shutdown, REALTEK_DSA);
MODULE_AUTHOR("Luiz Angelo Daros de Luca <[email protected]>");
MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface");
diff --git a/drivers/net/dsa/realtek/realtek-mdio.h b/drivers/net/dsa/realtek/realtek-mdio.h
new file mode 100644
index 000000000000..ee70f6a5b8ff
--- /dev/null
+++ b/drivers/net/dsa/realtek/realtek-mdio.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _REALTEK_MDIO_H
+#define _REALTEK_MDIO_H
+
+#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_MDIO)
+
+static inline int realtek_mdio_driver_register(struct mdio_driver *drv)
+{
+ return mdio_driver_register(drv);
+}
+
+static inline void realtek_mdio_driver_unregister(struct mdio_driver *drv)
+{
+ mdio_driver_unregister(drv);
+}
+
+int realtek_mdio_probe(struct mdio_device *mdiodev);
+void realtek_mdio_remove(struct mdio_device *mdiodev);
+void realtek_mdio_shutdown(struct mdio_device *mdiodev);
+
+#else /* IS_ENABLED(CONFIG_NET_DSA_REALTEK_MDIO) */
+
+static inline int realtek_mdio_driver_register(struct mdio_driver *drv)
+{
+ return 0;
+}
+
+static inline void realtek_mdio_driver_unregister(struct mdio_driver *drv)
+{
+}
+
+static inline int realtek_mdio_probe(struct mdio_device *mdiodev)
+{
+ return -ENOENT;
+}
+
+static inline void realtek_mdio_remove(struct mdio_device *mdiodev)
+{
+}
+
+static inline void realtek_mdio_shutdown(struct mdio_device *mdiodev)
+{
+}
+
+#endif /* IS_ENABLED(CONFIG_NET_DSA_REALTEK_MDIO) */
+
+#endif /* _REALTEK_MDIO_H */
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index 668336515119..d8a9a6a6b5bc 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -40,6 +40,7 @@
#include <linux/if_bridge.h>
#include "realtek.h"
+#include "realtek-smi.h"
#define REALTEK_SMI_ACK_RETRY_COUNT 5
@@ -408,7 +409,19 @@ static int realtek_smi_setup_mdio(struct dsa_switch *ds)
return ret;
}
-static int realtek_smi_probe(struct platform_device *pdev)
+/**
+ * realtek_smi_probe() - Probe a platform device for an SMI-connected switch
+ * @pdev: platform_device to probe on.
+ *
+ * This function should be used as the .probe in a platform_driver. It
+ * initializes realtek_priv and read data from the device-tree node. The switch
+ * is hard resetted if a method is provided. It checks the switch chip ID and,
+ * finally, a DSA switch is registered.
+ *
+ * Context: Can sleep. Takes and releases priv->map_lock.
+ * Return: Returns 0 on success, a negative error on failure.
+ */
+int realtek_smi_probe(struct platform_device *pdev)
{
const struct realtek_variant *var;
struct device *dev = &pdev->dev;
@@ -505,8 +518,20 @@ static int realtek_smi_probe(struct platform_device *pdev)
}
return 0;
}
+EXPORT_SYMBOL_NS_GPL(realtek_smi_probe, REALTEK_DSA);
-static void realtek_smi_remove(struct platform_device *pdev)
+/**
+ * realtek_smi_remove() - Remove the driver of a SMI-connected switch
+ * @pdev: platform_device to be removed.
+ *
+ * This function should be used as the .remove_new in a platform_driver. First
+ * it unregisters the DSA switch and cleans internal data. If a method is
+ * provided, the hard reset is asserted to avoid traffic leakage.
+ *
+ * Context: Can sleep.
+ * Return: Nothing.
+ */
+void realtek_smi_remove(struct platform_device *pdev)
{
struct realtek_priv *priv = platform_get_drvdata(pdev);
@@ -521,8 +546,21 @@ static void realtek_smi_remove(struct platform_device *pdev)
if (priv->reset)
gpiod_set_value(priv->reset, 1);
}
+EXPORT_SYMBOL_NS_GPL(realtek_smi_remove, REALTEK_DSA);
-static void realtek_smi_shutdown(struct platform_device *pdev)
+/**
+ * realtek_smi_shutdown() - Shutdown the driver of a SMI-connected switch
+ * @pdev: platform_device shutting down.
+ *
+ * This function should be used as the .shutdown in a platform_driver. It shuts
+ * down the DSA switch and cleans the platform driver data, to prevent
+ * realtek_smi_remove() from running afterwards, which is possible if the
+ * parent bus implements its own .shutdown() as .remove().
+ *
+ * Context: Can sleep.
+ * Return: Nothing.
+ */
+void realtek_smi_shutdown(struct platform_device *pdev)
{
struct realtek_priv *priv = platform_get_drvdata(pdev);
@@ -533,34 +571,7 @@ static void realtek_smi_shutdown(struct platform_device *pdev)
platform_set_drvdata(pdev, NULL);
}
-
-static const struct of_device_id realtek_smi_of_match[] = {
-#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8366RB)
- {
- .compatible = "realtek,rtl8366rb",
- .data = &rtl8366rb_variant,
- },
-#endif
-#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8365MB)
- {
- .compatible = "realtek,rtl8365mb",
- .data = &rtl8365mb_variant,
- },
-#endif
- { /* sentinel */ },
-};
-MODULE_DEVICE_TABLE(of, realtek_smi_of_match);
-
-static struct platform_driver realtek_smi_driver = {
- .driver = {
- .name = "realtek-smi",
- .of_match_table = realtek_smi_of_match,
- },
- .probe = realtek_smi_probe,
- .remove_new = realtek_smi_remove,
- .shutdown = realtek_smi_shutdown,
-};
-module_platform_driver(realtek_smi_driver);
+EXPORT_SYMBOL_NS_GPL(realtek_smi_shutdown, REALTEK_DSA);
MODULE_AUTHOR("Linus Walleij <[email protected]>");
MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via SMI interface");
diff --git a/drivers/net/dsa/realtek/realtek-smi.h b/drivers/net/dsa/realtek/realtek-smi.h
new file mode 100644
index 000000000000..ea49a2edd3c8
--- /dev/null
+++ b/drivers/net/dsa/realtek/realtek-smi.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _REALTEK_SMI_H
+#define _REALTEK_SMI_H
+
+#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_SMI)
+
+static inline int realtek_smi_driver_register(struct platform_driver *drv)
+{
+ return platform_driver_register(drv);
+}
+
+static inline void realtek_smi_driver_unregister(struct platform_driver *drv)
+{
+ platform_driver_unregister(drv);
+}
+
+int realtek_smi_probe(struct platform_device *pdev);
+void realtek_smi_remove(struct platform_device *pdev);
+void realtek_smi_shutdown(struct platform_device *pdev);
+
+#else /* IS_ENABLED(CONFIG_NET_DSA_REALTEK_SMI) */
+
+static inline int realtek_smi_driver_register(struct platform_driver *drv)
+{
+ return 0;
+}
+
+static inline void realtek_smi_driver_unregister(struct platform_driver *drv)
+{
+}
+
+static inline int realtek_smi_probe(struct platform_device *pdev)
+{
+ return -ENOENT;
+}
+
+static inline void realtek_smi_remove(struct platform_device *pdev)
+{
+}
+
+static inline void realtek_smi_shutdown(struct platform_device *pdev)
+{
+}
+
+#endif /* IS_ENABLED(CONFIG_NET_DSA_REALTEK_SMI) */
+
+#endif /* _REALTEK_SMI_H */
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
index c42ee8241ca2..e67c4562f5db 100644
--- a/drivers/net/dsa/realtek/rtl8365mb.c
+++ b/drivers/net/dsa/realtek/rtl8365mb.c
@@ -101,6 +101,8 @@
#include <linux/if_vlan.h>
#include "realtek.h"
+#include "realtek-smi.h"
+#include "realtek-mdio.h"
/* Family-specific data and limits */
#define RTL8365MB_PHYADDRMAX 7
@@ -2172,7 +2174,57 @@ const struct realtek_variant rtl8365mb_variant = {
.cmd_write = 0xb8,
.chip_data_sz = sizeof(struct rtl8365mb),
};
-EXPORT_SYMBOL_GPL(rtl8365mb_variant);
+
+static const struct of_device_id rtl8365mb_of_match[] = {
+ { .compatible = "realtek,rtl8365mb", .data = &rtl8365mb_variant, },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, rtl8365mb_of_match);
+
+static struct platform_driver rtl8365mb_smi_driver = {
+ .driver = {
+ .name = "rtl8365mb-smi",
+ .of_match_table = rtl8365mb_of_match,
+ },
+ .probe = realtek_smi_probe,
+ .remove_new = realtek_smi_remove,
+ .shutdown = realtek_smi_shutdown,
+};
+
+static struct mdio_driver rtl8365mb_mdio_driver = {
+ .mdiodrv.driver = {
+ .name = "rtl8365mb-mdio",
+ .of_match_table = rtl8365mb_of_match,
+ },
+ .probe = realtek_mdio_probe,
+ .remove = realtek_mdio_remove,
+ .shutdown = realtek_mdio_shutdown,
+};
+
+static int rtl8365mb_init(void)
+{
+ int ret;
+
+ ret = realtek_mdio_driver_register(&rtl8365mb_mdio_driver);
+ if (ret)
+ return ret;
+
+ ret = realtek_smi_driver_register(&rtl8365mb_smi_driver);
+ if (ret) {
+ realtek_mdio_driver_unregister(&rtl8365mb_mdio_driver);
+ return ret;
+ }
+
+ return 0;
+}
+module_init(rtl8365mb_init);
+
+static void __exit rtl8365mb_exit(void)
+{
+ realtek_smi_driver_unregister(&rtl8365mb_smi_driver);
+ realtek_mdio_driver_unregister(&rtl8365mb_mdio_driver);
+}
+module_exit(rtl8365mb_exit);
MODULE_AUTHOR("Alvin Šipraga <[email protected]>");
MODULE_DESCRIPTION("Driver for RTL8365MB-VC ethernet switch");
diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c
index 6661d4ba6882..747407ae8225 100644
--- a/drivers/net/dsa/realtek/rtl8366rb.c
+++ b/drivers/net/dsa/realtek/rtl8366rb.c
@@ -23,6 +23,8 @@
#include <linux/regmap.h>
#include "realtek.h"
+#include "realtek-smi.h"
+#include "realtek-mdio.h"
#define RTL8366RB_PORT_NUM_CPU 5
#define RTL8366RB_NUM_PORTS 6
@@ -1933,7 +1935,57 @@ const struct realtek_variant rtl8366rb_variant = {
.cmd_write = 0xa8,
.chip_data_sz = sizeof(struct rtl8366rb),
};
-EXPORT_SYMBOL_GPL(rtl8366rb_variant);
+
+static const struct of_device_id rtl8366rb_of_match[] = {
+ { .compatible = "realtek,rtl8366rb", .data = &rtl8366rb_variant, },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, rtl8366rb_of_match);
+
+static struct platform_driver rtl8366rb_smi_driver = {
+ .driver = {
+ .name = "rtl8366rb-smi",
+ .of_match_table = rtl8366rb_of_match,
+ },
+ .probe = realtek_smi_probe,
+ .remove_new = realtek_smi_remove,
+ .shutdown = realtek_smi_shutdown,
+};
+
+static struct mdio_driver rtl8366rb_mdio_driver = {
+ .mdiodrv.driver = {
+ .name = "rtl8366rb-mdio",
+ .of_match_table = rtl8366rb_of_match,
+ },
+ .probe = realtek_mdio_probe,
+ .remove = realtek_mdio_remove,
+ .shutdown = realtek_mdio_shutdown,
+};
+
+static int rtl8366rb_init(void)
+{
+ int ret;
+
+ ret = realtek_mdio_driver_register(&rtl8366rb_mdio_driver);
+ if (ret)
+ return ret;
+
+ ret = realtek_smi_driver_register(&rtl8366rb_smi_driver);
+ if (ret) {
+ realtek_mdio_driver_unregister(&rtl8366rb_mdio_driver);
+ return ret;
+ }
+
+ return 0;
+}
+module_init(rtl8366rb_init);
+
+static void __exit rtl8366rb_exit(void)
+{
+ realtek_smi_driver_unregister(&rtl8366rb_smi_driver);
+ realtek_mdio_driver_unregister(&rtl8366rb_mdio_driver);
+}
+module_exit(rtl8366rb_exit);
MODULE_AUTHOR("Linus Walleij <[email protected]>");
MODULE_DESCRIPTION("Driver for RTL8366RB ethernet switch");
--
2.43.0
The binding docs requires for SMI-connected devices that the switch
must have a child node named "mdio" and with a compatible string of
"realtek,smi-mdio". Meanwile, for MDIO-connected switches, the binding
docs only requires a child node named "mdio".
This patch changes the driver to use the common denominator for both
interfaces, looking for the MDIO node by name, ignoring the compatible
string.
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
Reviewed-by: Vladimir Oltean <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/realtek/realtek-smi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index 67bdad7ac910..ad47dcbbd2b2 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -333,7 +333,7 @@ static int realtek_smi_setup_mdio(struct dsa_switch *ds)
struct device_node *mdio_np;
int ret;
- mdio_np = of_get_compatible_child(priv->dev->of_node, "realtek,smi-mdio");
+ mdio_np = of_get_child_by_name(priv->dev->of_node, "mdio");
if (!mdio_np) {
dev_err(priv->dev, "no MDIO bus node\n");
return -ENODEV;
--
2.43.0
Remove the line assigning dev.of_node in mdio_bus as subsequent
of_mdiobus_register will always overwrite it.
As discussed in [1], allow the DSA core to be simplified, by not
assigning ds->user_mii_bus when the MDIO bus is described in OF, as it
is unnecessary.
Since commit 3b73a7b8ec38 ("net: mdio_bus: add refcounting for fwnodes
to mdiobus"), we can put the "mdio" node just after the MDIO bus
registration.
[1] https://lkml.kernel.org/netdev/20231213120656.x46fyad6ls7sqyzv@skbuf/T/#u
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
---
drivers/net/dsa/realtek/realtek-smi.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index ad47dcbbd2b2..b84df4564c15 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -331,7 +331,7 @@ static int realtek_smi_setup_mdio(struct dsa_switch *ds)
{
struct realtek_priv *priv = ds->priv;
struct device_node *mdio_np;
- int ret;
+ int ret = 0;
mdio_np = of_get_child_by_name(priv->dev->of_node, "mdio");
if (!mdio_np) {
@@ -344,15 +344,14 @@ static int realtek_smi_setup_mdio(struct dsa_switch *ds)
ret = -ENOMEM;
goto err_put_node;
}
+
priv->user_mii_bus->priv = priv;
priv->user_mii_bus->name = "SMI user MII";
priv->user_mii_bus->read = realtek_smi_mdio_read;
priv->user_mii_bus->write = realtek_smi_mdio_write;
snprintf(priv->user_mii_bus->id, MII_BUS_ID_SIZE, "SMI-%d",
ds->index);
- priv->user_mii_bus->dev.of_node = mdio_np;
priv->user_mii_bus->parent = priv->dev;
- ds->user_mii_bus = priv->user_mii_bus;
ret = devm_of_mdiobus_register(priv->dev, priv->user_mii_bus, mdio_np);
if (ret) {
@@ -361,8 +360,6 @@ static int realtek_smi_setup_mdio(struct dsa_switch *ds)
goto err_put_node;
}
- return 0;
-
err_put_node:
of_node_put(mdio_np);
@@ -422,8 +419,7 @@ EXPORT_SYMBOL_NS_GPL(realtek_smi_probe, REALTEK_DSA);
* @pdev: platform_device to be removed.
*
* This function should be used as the .remove_new in a platform_driver. First
- * it unregisters the DSA switch and cleans internal data. Finally, it calls
- * the common remove function.
+ * it unregisters the DSA switch and then it calls the common remove function.
*
* Context: Can sleep.
* Return: Nothing.
@@ -437,9 +433,6 @@ void realtek_smi_remove(struct platform_device *pdev)
rtl83xx_unregister_switch(priv);
- if (priv->user_mii_bus)
- of_node_put(priv->user_mii_bus->dev.of_node);
-
rtl83xx_remove(priv);
}
EXPORT_SYMBOL_NS_GPL(realtek_smi_remove, REALTEK_DSA);
--
2.43.0
In the user MDIO driver, despite numerous references to SMI, including
its compatible string, there's nothing inherently specific about the SMI
interface in the user MDIO bus. Consequently, the code has been migrated
to the rtl83xx module. All references to SMI have been eliminated.
The MDIO bus id was changed from Realtek-<switch id> to the switch
devname suffixed with :user_mii, giving more information about the bus
it is referencing.
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
---
drivers/net/dsa/realtek/realtek-smi.c | 57 +----------------------------
drivers/net/dsa/realtek/rtl83xx.c | 69 +++++++++++++++++++++++++++++++++++
drivers/net/dsa/realtek/rtl83xx.h | 1 +
3 files changed, 71 insertions(+), 56 deletions(-)
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index b84df4564c15..4d35621b5583 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -31,7 +31,6 @@
#include <linux/spinlock.h>
#include <linux/skbuff.h>
#include <linux/of.h>
-#include <linux/of_mdio.h>
#include <linux/delay.h>
#include <linux/gpio/consumer.h>
#include <linux/platform_device.h>
@@ -312,60 +311,6 @@ static int realtek_smi_read(void *ctx, u32 reg, u32 *val)
return realtek_smi_read_reg(priv, reg, val);
}
-static int realtek_smi_mdio_read(struct mii_bus *bus, int addr, int regnum)
-{
- struct realtek_priv *priv = bus->priv;
-
- return priv->ops->phy_read(priv, addr, regnum);
-}
-
-static int realtek_smi_mdio_write(struct mii_bus *bus, int addr, int regnum,
- u16 val)
-{
- struct realtek_priv *priv = bus->priv;
-
- return priv->ops->phy_write(priv, addr, regnum, val);
-}
-
-static int realtek_smi_setup_mdio(struct dsa_switch *ds)
-{
- struct realtek_priv *priv = ds->priv;
- struct device_node *mdio_np;
- int ret = 0;
-
- mdio_np = of_get_child_by_name(priv->dev->of_node, "mdio");
- if (!mdio_np) {
- dev_err(priv->dev, "no MDIO bus node\n");
- return -ENODEV;
- }
-
- priv->user_mii_bus = devm_mdiobus_alloc(priv->dev);
- if (!priv->user_mii_bus) {
- ret = -ENOMEM;
- goto err_put_node;
- }
-
- priv->user_mii_bus->priv = priv;
- priv->user_mii_bus->name = "SMI user MII";
- priv->user_mii_bus->read = realtek_smi_mdio_read;
- priv->user_mii_bus->write = realtek_smi_mdio_write;
- snprintf(priv->user_mii_bus->id, MII_BUS_ID_SIZE, "SMI-%d",
- ds->index);
- priv->user_mii_bus->parent = priv->dev;
-
- ret = devm_of_mdiobus_register(priv->dev, priv->user_mii_bus, mdio_np);
- if (ret) {
- dev_err(priv->dev, "unable to register MDIO bus %s\n",
- priv->user_mii_bus->id);
- goto err_put_node;
- }
-
-err_put_node:
- of_node_put(mdio_np);
-
- return ret;
-}
-
static const struct realtek_interface_info realtek_smi_info = {
.reg_read = realtek_smi_read,
.reg_write = realtek_smi_write,
@@ -403,7 +348,7 @@ int realtek_smi_probe(struct platform_device *pdev)
return PTR_ERR(priv->mdio);
priv->write_reg_noack = realtek_smi_write_reg_noack;
- priv->setup_interface = realtek_smi_setup_mdio;
+ priv->setup_interface = rtl83xx_setup_user_mdio;
priv->ds_ops = priv->variant->ds_ops_smi;
ret = rtl83xx_register_switch(priv);
diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c
index fb3b3b2305d1..f05fe6efe5a4 100644
--- a/drivers/net/dsa/realtek/rtl83xx.c
+++ b/drivers/net/dsa/realtek/rtl83xx.c
@@ -2,6 +2,7 @@
#include <linux/module.h>
#include <linux/regmap.h>
+#include <linux/of_mdio.h>
#include "realtek.h"
#include "rtl83xx.h"
@@ -43,6 +44,74 @@ void rtl83xx_unlock(void *ctx)
}
EXPORT_SYMBOL_NS_GPL(rtl83xx_unlock, REALTEK_DSA);
+static int rtl83xx_user_mdio_read(struct mii_bus *bus, int addr, int regnum)
+{
+ struct realtek_priv *priv = bus->priv;
+
+ return priv->ops->phy_read(priv, addr, regnum);
+}
+
+static int rtl83xx_user_mdio_write(struct mii_bus *bus, int addr, int regnum,
+ u16 val)
+{
+ struct realtek_priv *priv = bus->priv;
+
+ return priv->ops->phy_write(priv, addr, regnum, val);
+}
+
+/**
+ * rtl83xx_setup_user_mdio() - register the user mii bus driver
+ * @ds: DSA switch associated with this user_mii_bus
+ *
+ * This function first gets and mdio node under the dev OF node, aborting
+ * if missing. That mdio node describing an mdio bus is used to register a
+ * new mdio bus.
+ *
+ * Context: Can sleep.
+ * Return: 0 on success, negative value for failure.
+ */
+int rtl83xx_setup_user_mdio(struct dsa_switch *ds)
+{
+ struct realtek_priv *priv = ds->priv;
+ struct device_node *mdio_np;
+ struct mii_bus *bus;
+ int ret = 0;
+
+ mdio_np = of_get_child_by_name(priv->dev->of_node, "mdio");
+ if (!mdio_np) {
+ dev_err(priv->dev, "no MDIO bus node\n");
+ return -ENODEV;
+ }
+
+ bus = devm_mdiobus_alloc(priv->dev);
+ if (!bus) {
+ ret = -ENOMEM;
+ goto err_put_node;
+ }
+
+ bus->priv = priv;
+ bus->name = "Realtek user MII";
+ bus->read = rtl83xx_user_mdio_read;
+ bus->write = rtl83xx_user_mdio_write;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s:user_mii", dev_name(priv->dev));
+ bus->parent = priv->dev;
+
+ ret = devm_of_mdiobus_register(priv->dev, bus, mdio_np);
+ if (ret) {
+ dev_err(priv->dev, "unable to register MDIO bus %s\n",
+ bus->id);
+ goto err_put_node;
+ }
+
+ priv->user_mii_bus = bus;
+
+err_put_node:
+ of_node_put(mdio_np);
+
+ return ret;
+}
+EXPORT_SYMBOL_NS_GPL(rtl83xx_setup_user_mdio, REALTEK_DSA);
+
/**
* rtl83xx_probe() - probe a Realtek switch
* @dev: the device being probed
diff --git a/drivers/net/dsa/realtek/rtl83xx.h b/drivers/net/dsa/realtek/rtl83xx.h
index a8eed92bce1a..0ddff33df6b0 100644
--- a/drivers/net/dsa/realtek/rtl83xx.h
+++ b/drivers/net/dsa/realtek/rtl83xx.h
@@ -10,6 +10,7 @@ struct realtek_interface_info {
void rtl83xx_lock(void *ctx);
void rtl83xx_unlock(void *ctx);
+int rtl83xx_setup_user_mdio(struct dsa_switch *ds);
struct realtek_priv *
rtl83xx_probe(struct device *dev,
const struct realtek_interface_info *interface_info);
--
2.43.0
The realtek-mdio will now use this driver instead of the generic DSA
driver ("dsa user smi"), which should not be used with OF[1].
With a single ds_ops for both interfaces, the ds_ops in realtek_priv is
no longer necessary. Now, the realtek_variant.ds_ops can be used
directly.
The realtek_priv.setup_interface() has been removed as we can directly
call the new common function.
[1] https://lkml.kernel.org/netdev/[email protected]/T/
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
---
drivers/net/dsa/realtek/realtek-mdio.c | 1 -
drivers/net/dsa/realtek/realtek-smi.c | 2 --
drivers/net/dsa/realtek/realtek.h | 5 +---
drivers/net/dsa/realtek/rtl8365mb.c | 49 ++++----------------------------
drivers/net/dsa/realtek/rtl8366rb.c | 52 ++++------------------------------
drivers/net/dsa/realtek/rtl83xx.c | 2 +-
6 files changed, 14 insertions(+), 97 deletions(-)
diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
index 45d9ef43fe3d..1ae31575d096 100644
--- a/drivers/net/dsa/realtek/realtek-mdio.c
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -131,7 +131,6 @@ int realtek_mdio_probe(struct mdio_device *mdiodev)
priv->bus = mdiodev->bus;
priv->mdio_addr = mdiodev->addr;
priv->write_reg_noack = realtek_mdio_write;
- priv->ds_ops = priv->variant->ds_ops_mdio;
ret = rtl83xx_register_switch(priv);
if (ret)
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index 4d35621b5583..c075770c6986 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -348,8 +348,6 @@ int realtek_smi_probe(struct platform_device *pdev)
return PTR_ERR(priv->mdio);
priv->write_reg_noack = realtek_smi_write_reg_noack;
- priv->setup_interface = rtl83xx_setup_user_mdio;
- priv->ds_ops = priv->variant->ds_ops_smi;
ret = rtl83xx_register_switch(priv);
if (ret)
diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
index e9e28b189509..864bb9a88f14 100644
--- a/drivers/net/dsa/realtek/realtek.h
+++ b/drivers/net/dsa/realtek/realtek.h
@@ -62,7 +62,6 @@ struct realtek_priv {
spinlock_t lock; /* Locks around command writes */
struct dsa_switch *ds;
- const struct dsa_switch_ops *ds_ops;
struct irq_domain *irqdomain;
bool leds_disabled;
@@ -73,7 +72,6 @@ struct realtek_priv {
struct rtl8366_mib_counter *mib_counters;
const struct realtek_ops *ops;
- int (*setup_interface)(struct dsa_switch *ds);
int (*write_reg_noack)(void *ctx, u32 addr, u32 data);
int vlan_enabled;
@@ -115,8 +113,7 @@ struct realtek_ops {
};
struct realtek_variant {
- const struct dsa_switch_ops *ds_ops_smi;
- const struct dsa_switch_ops *ds_ops_mdio;
+ const struct dsa_switch_ops *ds_ops;
const struct realtek_ops *ops;
unsigned int clk_delay;
u8 cmd_read;
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
index 60f826a5a00a..778a962727ab 100644
--- a/drivers/net/dsa/realtek/rtl8365mb.c
+++ b/drivers/net/dsa/realtek/rtl8365mb.c
@@ -828,17 +828,6 @@ static int rtl8365mb_phy_write(struct realtek_priv *priv, int phy, int regnum,
return 0;
}
-static int rtl8365mb_dsa_phy_read(struct dsa_switch *ds, int phy, int regnum)
-{
- return rtl8365mb_phy_read(ds->priv, phy, regnum);
-}
-
-static int rtl8365mb_dsa_phy_write(struct dsa_switch *ds, int phy, int regnum,
- u16 val)
-{
- return rtl8365mb_phy_write(ds->priv, phy, regnum, val);
-}
-
static const struct rtl8365mb_extint *
rtl8365mb_get_port_extint(struct realtek_priv *priv, int port)
{
@@ -2017,12 +2006,10 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
if (ret)
goto out_teardown_irq;
- if (priv->setup_interface) {
- ret = priv->setup_interface(ds);
- if (ret) {
- dev_err(priv->dev, "could not set up MDIO bus\n");
- goto out_teardown_irq;
- }
+ ret = rtl83xx_setup_user_mdio(ds);
+ if (ret) {
+ dev_err(priv->dev, "could not set up MDIO bus\n");
+ goto out_teardown_irq;
}
/* Start statistics counter polling */
@@ -2116,28 +2103,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv)
return 0;
}
-static const struct dsa_switch_ops rtl8365mb_switch_ops_smi = {
- .get_tag_protocol = rtl8365mb_get_tag_protocol,
- .change_tag_protocol = rtl8365mb_change_tag_protocol,
- .setup = rtl8365mb_setup,
- .teardown = rtl8365mb_teardown,
- .phylink_get_caps = rtl8365mb_phylink_get_caps,
- .phylink_mac_config = rtl8365mb_phylink_mac_config,
- .phylink_mac_link_down = rtl8365mb_phylink_mac_link_down,
- .phylink_mac_link_up = rtl8365mb_phylink_mac_link_up,
- .port_stp_state_set = rtl8365mb_port_stp_state_set,
- .get_strings = rtl8365mb_get_strings,
- .get_ethtool_stats = rtl8365mb_get_ethtool_stats,
- .get_sset_count = rtl8365mb_get_sset_count,
- .get_eth_phy_stats = rtl8365mb_get_phy_stats,
- .get_eth_mac_stats = rtl8365mb_get_mac_stats,
- .get_eth_ctrl_stats = rtl8365mb_get_ctrl_stats,
- .get_stats64 = rtl8365mb_get_stats64,
- .port_change_mtu = rtl8365mb_port_change_mtu,
- .port_max_mtu = rtl8365mb_port_max_mtu,
-};
-
-static const struct dsa_switch_ops rtl8365mb_switch_ops_mdio = {
+static const struct dsa_switch_ops rtl8365mb_switch_ops = {
.get_tag_protocol = rtl8365mb_get_tag_protocol,
.change_tag_protocol = rtl8365mb_change_tag_protocol,
.setup = rtl8365mb_setup,
@@ -2146,8 +2112,6 @@ static const struct dsa_switch_ops rtl8365mb_switch_ops_mdio = {
.phylink_mac_config = rtl8365mb_phylink_mac_config,
.phylink_mac_link_down = rtl8365mb_phylink_mac_link_down,
.phylink_mac_link_up = rtl8365mb_phylink_mac_link_up,
- .phy_read = rtl8365mb_dsa_phy_read,
- .phy_write = rtl8365mb_dsa_phy_write,
.port_stp_state_set = rtl8365mb_port_stp_state_set,
.get_strings = rtl8365mb_get_strings,
.get_ethtool_stats = rtl8365mb_get_ethtool_stats,
@@ -2167,8 +2131,7 @@ static const struct realtek_ops rtl8365mb_ops = {
};
const struct realtek_variant rtl8365mb_variant = {
- .ds_ops_smi = &rtl8365mb_switch_ops_smi,
- .ds_ops_mdio = &rtl8365mb_switch_ops_mdio,
+ .ds_ops = &rtl8365mb_switch_ops,
.ops = &rtl8365mb_ops,
.clk_delay = 10,
.cmd_read = 0xb9,
diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c
index a0c365325b4a..54eff9cd0c03 100644
--- a/drivers/net/dsa/realtek/rtl8366rb.c
+++ b/drivers/net/dsa/realtek/rtl8366rb.c
@@ -1033,12 +1033,10 @@ static int rtl8366rb_setup(struct dsa_switch *ds)
if (ret)
dev_info(priv->dev, "no interrupt support\n");
- if (priv->setup_interface) {
- ret = priv->setup_interface(ds);
- if (ret) {
- dev_err(priv->dev, "could not set up MDIO bus\n");
- return -ENODEV;
- }
+ ret = rtl83xx_setup_user_mdio(ds);
+ if (ret) {
+ dev_err(priv->dev, "could not set up MDIO bus\n");
+ return -ENODEV;
}
return 0;
@@ -1785,17 +1783,6 @@ static int rtl8366rb_phy_write(struct realtek_priv *priv, int phy, int regnum,
return ret;
}
-static int rtl8366rb_dsa_phy_read(struct dsa_switch *ds, int phy, int regnum)
-{
- return rtl8366rb_phy_read(ds->priv, phy, regnum);
-}
-
-static int rtl8366rb_dsa_phy_write(struct dsa_switch *ds, int phy, int regnum,
- u16 val)
-{
- return rtl8366rb_phy_write(ds->priv, phy, regnum, val);
-}
-
static int rtl8366rb_reset_chip(struct realtek_priv *priv)
{
int timeout = 10;
@@ -1861,35 +1848,9 @@ static int rtl8366rb_detect(struct realtek_priv *priv)
return 0;
}
-static const struct dsa_switch_ops rtl8366rb_switch_ops_smi = {
- .get_tag_protocol = rtl8366_get_tag_protocol,
- .setup = rtl8366rb_setup,
- .phylink_get_caps = rtl8366rb_phylink_get_caps,
- .phylink_mac_link_up = rtl8366rb_mac_link_up,
- .phylink_mac_link_down = rtl8366rb_mac_link_down,
- .get_strings = rtl8366_get_strings,
- .get_ethtool_stats = rtl8366_get_ethtool_stats,
- .get_sset_count = rtl8366_get_sset_count,
- .port_bridge_join = rtl8366rb_port_bridge_join,
- .port_bridge_leave = rtl8366rb_port_bridge_leave,
- .port_vlan_filtering = rtl8366rb_vlan_filtering,
- .port_vlan_add = rtl8366_vlan_add,
- .port_vlan_del = rtl8366_vlan_del,
- .port_enable = rtl8366rb_port_enable,
- .port_disable = rtl8366rb_port_disable,
- .port_pre_bridge_flags = rtl8366rb_port_pre_bridge_flags,
- .port_bridge_flags = rtl8366rb_port_bridge_flags,
- .port_stp_state_set = rtl8366rb_port_stp_state_set,
- .port_fast_age = rtl8366rb_port_fast_age,
- .port_change_mtu = rtl8366rb_change_mtu,
- .port_max_mtu = rtl8366rb_max_mtu,
-};
-
-static const struct dsa_switch_ops rtl8366rb_switch_ops_mdio = {
+static const struct dsa_switch_ops rtl8366rb_switch_ops = {
.get_tag_protocol = rtl8366_get_tag_protocol,
.setup = rtl8366rb_setup,
- .phy_read = rtl8366rb_dsa_phy_read,
- .phy_write = rtl8366rb_dsa_phy_write,
.phylink_get_caps = rtl8366rb_phylink_get_caps,
.phylink_mac_link_up = rtl8366rb_mac_link_up,
.phylink_mac_link_down = rtl8366rb_mac_link_down,
@@ -1928,8 +1889,7 @@ static const struct realtek_ops rtl8366rb_ops = {
};
const struct realtek_variant rtl8366rb_variant = {
- .ds_ops_smi = &rtl8366rb_switch_ops_smi,
- .ds_ops_mdio = &rtl8366rb_switch_ops_mdio,
+ .ds_ops = &rtl8366rb_switch_ops,
.ops = &rtl8366rb_ops,
.clk_delay = 10,
.cmd_read = 0xa9,
diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c
index f05fe6efe5a4..aa998e15c42b 100644
--- a/drivers/net/dsa/realtek/rtl83xx.c
+++ b/drivers/net/dsa/realtek/rtl83xx.c
@@ -232,7 +232,7 @@ int rtl83xx_register_switch(struct realtek_priv *priv)
priv->ds->priv = priv;
priv->ds->dev = priv->dev;
- priv->ds->ops = priv->ds_ops;
+ priv->ds->ops = priv->variant->ds_ops;
priv->ds->num_ports = priv->num_ports;
ret = dsa_register_switch(priv->ds);
--
2.43.0
To eliminate the need for a second memory allocation for dsa_switch, it
has been embedded within realtek_priv.
Suggested-by: Alvin Šipraga <[email protected]>
Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
---
drivers/net/dsa/realtek/realtek.h | 2 +-
drivers/net/dsa/realtek/rtl8365mb.c | 12 ++++++------
drivers/net/dsa/realtek/rtl8366rb.c | 2 +-
drivers/net/dsa/realtek/rtl83xx.c | 18 +++++++-----------
4 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
index 864bb9a88f14..b80bfde1ad04 100644
--- a/drivers/net/dsa/realtek/realtek.h
+++ b/drivers/net/dsa/realtek/realtek.h
@@ -61,7 +61,7 @@ struct realtek_priv {
const struct realtek_variant *variant;
spinlock_t lock; /* Locks around command writes */
- struct dsa_switch *ds;
+ struct dsa_switch ds;
struct irq_domain *irqdomain;
bool leds_disabled;
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
index 778a962727ab..9066e34e9ace 100644
--- a/drivers/net/dsa/realtek/rtl8365mb.c
+++ b/drivers/net/dsa/realtek/rtl8365mb.c
@@ -880,7 +880,7 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port,
if (!extint)
return -ENODEV;
- dp = dsa_to_port(priv->ds, port);
+ dp = dsa_to_port(&priv->ds, port);
dn = dp->dn;
/* Set the RGMII TX/RX delay
@@ -1543,7 +1543,7 @@ static void rtl8365mb_stats_setup(struct realtek_priv *priv)
for (i = 0; i < priv->num_ports; i++) {
struct rtl8365mb_port *p = &mb->ports[i];
- if (dsa_is_unused_port(priv->ds, i))
+ if (dsa_is_unused_port(&priv->ds, i))
continue;
/* Per-port spinlock to protect the stats64 data */
@@ -1564,7 +1564,7 @@ static void rtl8365mb_stats_teardown(struct realtek_priv *priv)
for (i = 0; i < priv->num_ports; i++) {
struct rtl8365mb_port *p = &mb->ports[i];
- if (dsa_is_unused_port(priv->ds, i))
+ if (dsa_is_unused_port(&priv->ds, i))
continue;
cancel_delayed_work_sync(&p->mib_work);
@@ -1963,7 +1963,7 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
dev_info(priv->dev, "no interrupt support\n");
/* Configure CPU tagging */
- dsa_switch_for_each_cpu_port(cpu_dp, priv->ds) {
+ dsa_switch_for_each_cpu_port(cpu_dp, &priv->ds) {
cpu->mask |= BIT(cpu_dp->index);
if (cpu->trap_port == RTL8365MB_MAX_NUM_PORTS)
@@ -1978,7 +1978,7 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
for (i = 0; i < priv->num_ports; i++) {
struct rtl8365mb_port *p = &mb->ports[i];
- if (dsa_is_unused_port(priv->ds, i))
+ if (dsa_is_unused_port(&priv->ds, i))
continue;
/* Forward only to the CPU */
@@ -1995,7 +1995,7 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
* ports will still forward frames to the CPU despite being
* administratively down by default.
*/
- rtl8365mb_port_stp_state_set(priv->ds, i, BR_STATE_DISABLED);
+ rtl8365mb_port_stp_state_set(&priv->ds, i, BR_STATE_DISABLED);
/* Set up per-port private data */
p->priv = priv;
diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c
index 54eff9cd0c03..cdc37be1ed2c 100644
--- a/drivers/net/dsa/realtek/rtl8366rb.c
+++ b/drivers/net/dsa/realtek/rtl8366rb.c
@@ -1675,7 +1675,7 @@ static int rtl8366rb_set_mc_index(struct realtek_priv *priv, int port, int index
* not drop any untagged or C-tagged frames. Make sure to update the
* filtering setting.
*/
- if (dsa_port_is_vlan_filtering(dsa_to_port(priv->ds, port)))
+ if (dsa_port_is_vlan_filtering(dsa_to_port(&priv->ds, port)))
ret = rtl8366rb_drop_untagged(priv, port, !pvid_enabled);
return ret;
diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c
index aa998e15c42b..f65e47339d5b 100644
--- a/drivers/net/dsa/realtek/rtl83xx.c
+++ b/drivers/net/dsa/realtek/rtl83xx.c
@@ -226,16 +226,12 @@ int rtl83xx_register_switch(struct realtek_priv *priv)
return ret;
}
- priv->ds = devm_kzalloc(priv->dev, sizeof(*priv->ds), GFP_KERNEL);
- if (!priv->ds)
- return -ENOMEM;
+ priv->ds.priv = priv;
+ priv->ds.dev = priv->dev;
+ priv->ds.ops = priv->variant->ds_ops;
+ priv->ds.num_ports = priv->num_ports;
- priv->ds->priv = priv;
- priv->ds->dev = priv->dev;
- priv->ds->ops = priv->variant->ds_ops;
- priv->ds->num_ports = priv->num_ports;
-
- ret = dsa_register_switch(priv->ds);
+ ret = dsa_register_switch(&priv->ds);
if (ret) {
dev_err_probe(priv->dev, ret, "unable to register switch\n");
return ret;
@@ -256,7 +252,7 @@ EXPORT_SYMBOL_NS_GPL(rtl83xx_register_switch, REALTEK_DSA);
*/
void rtl83xx_unregister_switch(struct realtek_priv *priv)
{
- dsa_unregister_switch(priv->ds);
+ dsa_unregister_switch(&priv->ds);
}
EXPORT_SYMBOL_NS_GPL(rtl83xx_unregister_switch, REALTEK_DSA);
@@ -273,7 +269,7 @@ EXPORT_SYMBOL_NS_GPL(rtl83xx_unregister_switch, REALTEK_DSA);
*/
void rtl83xx_shutdown(struct realtek_priv *priv)
{
- dsa_switch_shutdown(priv->ds);
+ dsa_switch_shutdown(&priv->ds);
dev_set_drvdata(priv->dev, NULL);
}
--
2.43.0
On Tue, Jan 30, 2024 at 08:13:20PM -0300, Luiz Angelo Daros de Luca wrote:
> It was never used and never referenced.
>
> Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
> Reviewed-by: Alvin Šipraga <[email protected]>
> Reviewed-by: Florian Fainelli <[email protected]>
> Reviewed-by: Linus Walleij <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
On Tue, Jan 30, 2024 at 08:13:22PM -0300, Luiz Angelo Daros de Luca wrote:
> Previously, the interface modules realtek-smi and realtek-mdio served as
> a platform and an MDIO driver, respectively. Each interface module
> redundantly specified the same compatible strings for both variants and
> referenced symbols from the variants.
>
> Now, each variant module has been transformed into a unified driver
> serving both as a platform and an MDIO driver. This modification
> reverses the relationship between the interface and variant modules,
> with the variant module now utilizing symbols from the interface
> modules.
>
> Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
Some minor non-functional comments below which you might decide not to
address now, depending on what else is pointed out during review.
> diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
> index c2572463679f..3433f64fb0d7 100644
> --- a/drivers/net/dsa/realtek/realtek-mdio.c
> +++ b/drivers/net/dsa/realtek/realtek-mdio.c
> @@ -140,7 +141,19 @@ static const struct regmap_config realtek_mdio_nolock_regmap_config = {
> .disable_locking = true,
> };
>
> -static int realtek_mdio_probe(struct mdio_device *mdiodev)
> +/**
> + * realtek_mdio_probe() - Probe a platform device for an MDIO-connected switch
> + * @mdiodev: mdio_device to probe on.
> + *
> + * This function should be used as the .probe in an mdio_driver. It
> + * initializes realtek_priv and read data from the device-tree node. The switch
> + * is hard resetted if a method is provided. It checks the switch chip ID and,
nitpick: participle of 'reset' is 'reset'. Same comment for realtek_smi_probe().
> + * finally, a DSA switch is registered.
> + *
> + * Context: Can sleep. Takes and releases priv->map_lock.
> + * Return: Returns 0 on success, a negative error on failure.
> + */
> +int realtek_mdio_probe(struct mdio_device *mdiodev)
> {
> struct realtek_priv *priv;
> struct device *dev = &mdiodev->dev;
> diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
> index 668336515119..d8a9a6a6b5bc 100644
> --- a/drivers/net/dsa/realtek/realtek-smi.c
> +++ b/drivers/net/dsa/realtek/realtek-smi.c
> @@ -505,8 +518,20 @@ static int realtek_smi_probe(struct platform_device *pdev)
> }
> return 0;
> }
> +EXPORT_SYMBOL_NS_GPL(realtek_smi_probe, REALTEK_DSA);
>
> -static void realtek_smi_remove(struct platform_device *pdev)
> +/**
> + * realtek_smi_remove() - Remove the driver of a SMI-connected switch
> + * @pdev: platform_device to be removed.
> + *
> + * This function should be used as the .remove_new in a platform_driver. First
> + * it unregisters the DSA switch and cleans internal data. If a method is
> + * provided, the hard reset is asserted to avoid traffic leakage.
FWIW, removing the driver unregisters the net devices, which disables
the ports and performs an orderly shutdown of any upper virtual net
devices as well, like bridges. So ports automatically roll back to
standalone operation before this callback is even issued.
Traffic leakage is prevented, and the switch is hard reset, but I don't
think there is any causal relationship between the two.
> + *
> + * Context: Can sleep.
> + * Return: Nothing.
> + */
> +void realtek_smi_remove(struct platform_device *pdev)
> {
> struct realtek_priv *priv = platform_get_drvdata(pdev);
>
On Tue, Jan 30, 2024 at 08:13:24PM -0300, Luiz Angelo Daros de Luca wrote:
> /**
> * realtek_smi_probe() - Probe a platform device for an SMI-connected switch
> * @pdev: platform_device to probe on.
> *
> - * This function should be used as the .probe in a platform_driver. It
> - * initializes realtek_priv and read data from the device-tree node. The switch
> - * is hard resetted if a method is provided. It checks the switch chip ID and,
> - * finally, a DSA switch is registered.
> + * This function should be used as the .probe in a platform_driver. After
> + * calling the common probe function for both interfaces, it initializes the
> + * values specific for SMI-connected devices. Finally, it calls a common
> + * function to register the DSA switch.
> *
> * Context: Can sleep. Takes and releases priv->map_lock.
> * Return: Returns 0 on success, a negative error on failure.
> */
> int realtek_smi_probe(struct platform_device *pdev)
> {
> - const struct realtek_variant *var;
> struct device *dev = &pdev->dev;
> struct realtek_priv *priv;
> - struct regmap_config rc;
> - struct device_node *np;
> int ret;
>
> - var = of_device_get_match_data(dev);
> - np = dev->of_node;
> -
> - priv = devm_kzalloc(dev, sizeof(*priv) + var->chip_data_sz, GFP_KERNEL);
> - if (!priv)
> - return -ENOMEM;
> - priv->chip_data = (void *)priv + sizeof(*priv);
> -
> - mutex_init(&priv->map_lock);
> -
> - rc = realtek_smi_regmap_config;
> - rc.lock_arg = priv;
> - priv->map = devm_regmap_init(dev, NULL, priv, &rc);
> - if (IS_ERR(priv->map)) {
> - ret = PTR_ERR(priv->map);
> - dev_err(dev, "regmap init failed: %d\n", ret);
> - return ret;
> - }
> -
> - rc = realtek_smi_nolock_regmap_config;
> - priv->map_nolock = devm_regmap_init(dev, NULL, priv, &rc);
> - if (IS_ERR(priv->map_nolock)) {
> - ret = PTR_ERR(priv->map_nolock);
> - dev_err(dev, "regmap init failed: %d\n", ret);
> - return ret;
> - }
> -
> - /* Link forward and backward */
> - priv->dev = dev;
> - priv->variant = var;
> - priv->ops = var->ops;
> -
> - priv->setup_interface = realtek_smi_setup_mdio;
> - priv->write_reg_noack = realtek_smi_write_reg_noack;
> -
> - dev_set_drvdata(dev, priv);
> - spin_lock_init(&priv->lock);
> -
> - /* TODO: if power is software controlled, set up any regulators here */
> -
> - priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> - if (IS_ERR(priv->reset)) {
> - dev_err(dev, "failed to get RESET GPIO\n");
> - return PTR_ERR(priv->reset);
> - }
> - if (priv->reset) {
> - gpiod_set_value(priv->reset, 1);
> - dev_dbg(dev, "asserted RESET\n");
> - msleep(REALTEK_HW_STOP_DELAY);
> - gpiod_set_value(priv->reset, 0);
> - msleep(REALTEK_HW_START_DELAY);
> - dev_dbg(dev, "deasserted RESET\n");
> - }
> + priv = rtl83xx_probe(dev, &realtek_smi_info);
> + if (IS_ERR(priv))
> + return PTR_ERR(priv);
>
> /* Fetch MDIO pins */
> priv->mdc = devm_gpiod_get_optional(dev, "mdc", GPIOD_OUT_LOW);
> if (IS_ERR(priv->mdc))
> return PTR_ERR(priv->mdc);
Be consistent in the usage of the API you create. Every time rtl83xx_probe()
succeeds and something else down the line fails, you must call
rtl83xx_remove(). Otherwise, it is a shaky base to build upon.
The rtl83xx_remove() function can even be left empty if you feel that
the existing hard reset is of no value (which I would agree with) - see
mv88e6xxx_hwtstamp_free() as an example.
> +
> priv->mdio = devm_gpiod_get_optional(dev, "mdio", GPIOD_OUT_LOW);
> if (IS_ERR(priv->mdio))
> return PTR_ERR(priv->mdio);
>
> - priv->leds_disabled = of_property_read_bool(np, "realtek,disable-leds");
> + priv->write_reg_noack = realtek_smi_write_reg_noack;
> + priv->setup_interface = realtek_smi_setup_mdio;
> + priv->ds_ops = priv->variant->ds_ops_smi;
>
> - ret = priv->ops->detect(priv);
> - if (ret) {
> - dev_err(dev, "unable to detect switch\n");
> + ret = rtl83xx_register_switch(priv);
> + if (ret)
> return ret;
> - }
> -
> - priv->ds = devm_kzalloc(dev, sizeof(*priv->ds), GFP_KERNEL);
> - if (!priv->ds)
> - return -ENOMEM;
>
> - priv->ds->dev = dev;
> - priv->ds->num_ports = priv->num_ports;
> - priv->ds->priv = priv;
> -
> - priv->ds->ops = var->ds_ops_smi;
> - ret = dsa_register_switch(priv->ds);
> - if (ret) {
> - dev_err_probe(dev, ret, "unable to register switch\n");
> - return ret;
> - }
> return 0;
> }
> EXPORT_SYMBOL_NS_GPL(realtek_smi_probe, REALTEK_DSA);
On Tue, Jan 30, 2024 at 08:13:27PM -0300, Luiz Angelo Daros de Luca wrote:
> Remove the line assigning dev.of_node in mdio_bus as subsequent
> of_mdiobus_register will always overwrite it.
>
> As discussed in [1], allow the DSA core to be simplified, by not
> assigning ds->user_mii_bus when the MDIO bus is described in OF, as it
> is unnecessary.
>
> Since commit 3b73a7b8ec38 ("net: mdio_bus: add refcounting for fwnodes
> to mdiobus"), we can put the "mdio" node just after the MDIO bus
> registration.
>
> [1] https://lkml.kernel.org/netdev/20231213120656.x46fyad6ls7sqyzv@skbuf/T/#u
>
> Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
On Tue, Jan 30, 2024 at 08:13:28PM -0300, Luiz Angelo Daros de Luca wrote:
> +/**
> + * rtl83xx_setup_user_mdio() - register the user mii bus driver
> + * @ds: DSA switch associated with this user_mii_bus
> + *
> + * This function first gets and mdio node under the dev OF node, aborting
> + * if missing. That mdio node describing an mdio bus is used to register a
> + * new mdio bus.
The description has the overall feel of "Family Guy - Peter narrates his life"
(https://www.youtube.com/watch?v=zw8zUMjEW0I).
You could be a bit more succinct and say something like "Registers the
MDIO bus for built-in Ethernet PHYs, and associates it with the
mandatory 'mdio' child OF node of the switch".
> + *
> + * Context: Can sleep.
> + * Return: 0 on success, negative value for failure.
> + */
> +int rtl83xx_setup_user_mdio(struct dsa_switch *ds)
> +{
> + struct realtek_priv *priv = ds->priv;
> + struct device_node *mdio_np;
> + struct mii_bus *bus;
> + int ret = 0;
> +
> + mdio_np = of_get_child_by_name(priv->dev->of_node, "mdio");
> + if (!mdio_np) {
> + dev_err(priv->dev, "no MDIO bus node\n");
> + return -ENODEV;
> + }
> +
> + bus = devm_mdiobus_alloc(priv->dev);
> + if (!bus) {
> + ret = -ENOMEM;
> + goto err_put_node;
> + }
> +
> + bus->priv = priv;
> + bus->name = "Realtek user MII";
> + bus->read = rtl83xx_user_mdio_read;
> + bus->write = rtl83xx_user_mdio_write;
> + snprintf(bus->id, MII_BUS_ID_SIZE, "%s:user_mii", dev_name(priv->dev));
> + bus->parent = priv->dev;
> +
> + ret = devm_of_mdiobus_register(priv->dev, bus, mdio_np);
> + if (ret) {
> + dev_err(priv->dev, "unable to register MDIO bus %s\n",
> + bus->id);
> + goto err_put_node;
> + }
> +
> + priv->user_mii_bus = bus;
> +
> +err_put_node:
> + of_node_put(mdio_np);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(rtl83xx_setup_user_mdio, REALTEK_DSA);
On Tue, Jan 30, 2024 at 08:13:29PM -0300, Luiz Angelo Daros de Luca wrote:
> The realtek-mdio will now use this driver instead of the generic DSA
> driver ("dsa user smi"), which should not be used with OF[1].
>
> With a single ds_ops for both interfaces, the ds_ops in realtek_priv is
> no longer necessary. Now, the realtek_variant.ds_ops can be used
> directly.
>
> The realtek_priv.setup_interface() has been removed as we can directly
> call the new common function.
>
> [1] https://lkml.kernel.org/netdev/[email protected]/T/
>
> Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
On Tue, Jan 30, 2024 at 08:13:30PM -0300, Luiz Angelo Daros de Luca wrote:
> To eliminate the need for a second memory allocation for dsa_switch, it
> has been embedded within realtek_priv.
>
> Suggested-by: Alvin Šipraga <[email protected]>
> Signed-off-by: Luiz Angelo Daros de Luca <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
I don't think it would be bad if you could just define a local variable
struct dsa_switch *ds = &priv->ds;
in all functions, including those which reference &priv->ds just once.
> diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
> index 864bb9a88f14..b80bfde1ad04 100644
> --- a/drivers/net/dsa/realtek/realtek.h
> +++ b/drivers/net/dsa/realtek/realtek.h
> @@ -61,7 +61,7 @@ struct realtek_priv {
> const struct realtek_variant *variant;
>
> spinlock_t lock; /* Locks around command writes */
> - struct dsa_switch *ds;
> + struct dsa_switch ds;
> struct irq_domain *irqdomain;
> bool leds_disabled;
>
> diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
> index 778a962727ab..9066e34e9ace 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb.c
> @@ -880,7 +880,7 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port,
> if (!extint)
> return -ENODEV;
>
> - dp = dsa_to_port(priv->ds, port);
> + dp = dsa_to_port(&priv->ds, port);
Nitpick: I don't think it would be bad if you could just define a local variable
struct dsa_switch *ds = &priv->ds;
in all functions, including those which reference &priv->ds just once,
like this one.
> dn = dp->dn;
>
> /* Set the RGMII TX/RX delay
> diff --git a/drivers/net/dsa/realtek/rtl83xx.c b/drivers/net/dsa/realtek/rtl83xx.c
> index aa998e15c42b..f65e47339d5b 100644
> --- a/drivers/net/dsa/realtek/rtl83xx.c
> +++ b/drivers/net/dsa/realtek/rtl83xx.c
> @@ -226,16 +226,12 @@ int rtl83xx_register_switch(struct realtek_priv *priv)
> return ret;
> }
>
> - priv->ds = devm_kzalloc(priv->dev, sizeof(*priv->ds), GFP_KERNEL);
> - if (!priv->ds)
> - return -ENOMEM;
> + priv->ds.priv = priv;
> + priv->ds.dev = priv->dev;
> + priv->ds.ops = priv->variant->ds_ops;
> + priv->ds.num_ports = priv->num_ports;
And here, I think it would actually look better to dereference just
through "ds".
Also, priv->dev is dereferenced 4 times in rtl83xx_register_switch(),
maybe you could add a local variable for it in the patch that introduces
rtl83xx_register_switch().
>
> - priv->ds->priv = priv;
> - priv->ds->dev = priv->dev;
> - priv->ds->ops = priv->variant->ds_ops;
> - priv->ds->num_ports = priv->num_ports;
> -
> - ret = dsa_register_switch(priv->ds);
> + ret = dsa_register_switch(&priv->ds);
> if (ret) {
> dev_err_probe(priv->dev, ret, "unable to register switch\n");
> return ret;
>
>
> I don't think it would be bad if you could just define a local variable
>
> struct dsa_switch *ds = &priv->ds;
Yes, I'll use it. It does look better.
Regards,
Luiz
> > +/**
> > + * rtl83xx_setup_user_mdio() - register the user mii bus driver
> > + * @ds: DSA switch associated with this user_mii_bus
> > + *
> > + * This function first gets and mdio node under the dev OF node, aborting
> > + * if missing. That mdio node describing an mdio bus is used to register a
> > + * new mdio bus.
>
> The description has the overall feel of "Family Guy - Peter narrates his life"
> (https://www.youtube.com/watch?v=zw8zUMjEW0I).
Hahaha. Yes it does.
> You could be a bit more succinct and say something like "Registers the
> MDIO bus for built-in Ethernet PHYs, and associates it with the
> mandatory 'mdio' child OF node of the switch".
Done. Thanks.
Regards,
Luiz