| Message ID | 20220105031515.29276-7-luizluca@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: realtek: MDIO interface and RTL8367S | expand |
Luiz Angelo Daros de Luca <luizluca@gmail.com> writes: > This driver is a mdio_driver instead of a platform driver (like > realtek-smi). > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com> This looks good but I still wonder about the START_OPs, see below. > --- > drivers/net/dsa/realtek/Kconfig | 11 +- > drivers/net/dsa/realtek/Makefile | 1 + > drivers/net/dsa/realtek/realtek-mdio.c | 221 +++++++++++++++++++++++++ > drivers/net/dsa/realtek/realtek.h | 2 + > 4 files changed, 233 insertions(+), 2 deletions(-) > create mode 100644 drivers/net/dsa/realtek/realtek-mdio.c > > diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig > index cd1aa95b7bf0..73b26171fade 100644 > --- a/drivers/net/dsa/realtek/Kconfig > +++ b/drivers/net/dsa/realtek/Kconfig > @@ -9,6 +9,13 @@ menuconfig NET_DSA_REALTEK > help > Select to enable support for Realtek Ethernet switch chips. > > +config NET_DSA_REALTEK_MDIO > + tristate "Realtek MDIO connected switch driver" > + depends on NET_DSA_REALTEK > + default y > + help > + Select to enable support for registering switches configured > + through MDIO. Missing newline here > config NET_DSA_REALTEK_SMI > tristate "Realtek SMI connected switch driver" > depends on NET_DSA_REALTEK > @@ -21,7 +28,7 @@ config NET_DSA_REALTEK_RTL8365MB > tristate "Realtek RTL8365MB switch subdriver" > default y > depends on NET_DSA_REALTEK > - depends on NET_DSA_REALTEK_SMI > + depends on NET_DSA_REALTEK_SMI || NET_DSA_REALTEK_MDIO > select NET_DSA_TAG_RTL8_4 > help > Select to enable support for Realtek RTL8365MB > @@ -30,7 +37,7 @@ config NET_DSA_REALTEK_RTL8366RB > tristate "Realtek RTL8366RB switch subdriver" > default y > depends on NET_DSA_REALTEK > - depends on NET_DSA_REALTEK_SMI > + 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/Makefile b/drivers/net/dsa/realtek/Makefile > index 8b5a4abcedd3..0aab57252a7c 100644 > --- a/drivers/net/dsa/realtek/Makefile > +++ b/drivers/net/dsa/realtek/Makefile > @@ -1,4 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > +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 > rtl8366-objs := rtl8366-core.o rtl8366rb.o > diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c > new file mode 100644 > index 000000000000..b505f4d3c5f0 > --- /dev/null > +++ b/drivers/net/dsa/realtek/realtek-mdio.c > @@ -0,0 +1,221 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* Realtek MDIO interface driver > + * > + * ASICs we intend to support with this driver: > + * > + * RTL8366 - The original version, apparently > + * RTL8369 - Similar enough to have the same datsheet as RTL8366 > + * RTL8366RB - Probably reads out "RTL8366 revision B", has a quite > + * different register layout from the other two > + * RTL8366S - Is this "RTL8366 super"? > + * RTL8367 - Has an OpenWRT driver as well > + * RTL8368S - Seems to be an alternative name for RTL8366RB > + * RTL8370 - Also uses SMI > + * > + * Copyright (C) 2017 Linus Walleij <linus.walleij@linaro.org> > + * Copyright (C) 2010 Antti Seppälä <a.seppala@gmail.com> > + * Copyright (C) 2010 Roman Yeryomin <roman@advem.lv> > + * Copyright (C) 2011 Colin Leitner <colin.leitner@googlemail.com> > + * Copyright (C) 2009-2010 Gabor Juhos <juhosg@openwrt.org> > + */ > + > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> > + > +#include "realtek.h" > + > +/* Read/write via mdiobus */ > +#define REALTEK_MDIO_CTRL0_REG 31 > +#define REALTEK_MDIO_START_REG 29 > +#define REALTEK_MDIO_CTRL1_REG 21 > +#define REALTEK_MDIO_ADDRESS_REG 23 > +#define REALTEK_MDIO_DATA_WRITE_REG 24 > +#define REALTEK_MDIO_DATA_READ_REG 25 > + > +#define REALTEK_MDIO_START_OP 0xFFFF > +#define REALTEK_MDIO_ADDR_OP 0x000E > +#define REALTEK_MDIO_READ_OP 0x0001 > +#define REALTEK_MDIO_WRITE_OP 0x0003 > + > +static int realtek_mdio_read_reg(struct realtek_priv *priv, u32 addr, u32 *data) > +{ > + u32 phy_id = priv->phy_id; > + struct mii_bus *bus = priv->bus; > + > + mutex_lock(&bus->mdio_lock); > + > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL0_REG, REALTEK_MDIO_ADDR_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); Hm, I thought you said it works without these START_OPs? Or just without the first one? I don't know if it matters but I would suggest removing all START_OPs and seeing if things still work. If so then it's probably best to omit this because it is not present in the vendor driver, and nobody seems to know what it's for. I agree it could be something for older models but if it works on your model then let's leave it out :-) > + bus->write(bus, phy_id, REALTEK_MDIO_ADDRESS_REG, addr); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL1_REG, REALTEK_MDIO_READ_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + *data = bus->read(bus, phy_id, REALTEK_MDIO_DATA_READ_REG); > + > + mutex_unlock(&bus->mdio_lock); > + > + return 0; > +} > + > +static int realtek_mdio_write_reg(struct realtek_priv *priv, u32 addr, u32 data) > +{ > + u32 phy_id = priv->phy_id; > + struct mii_bus *bus = priv->bus; > + > + mutex_lock(&bus->mdio_lock); > + > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL0_REG, REALTEK_MDIO_ADDR_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_ADDRESS_REG, addr); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_DATA_WRITE_REG, data); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL1_REG, REALTEK_MDIO_WRITE_OP); > + > + mutex_unlock(&bus->mdio_lock); > + > + return 0; > +} > + > +/* Regmap accessors */ > + > +static int realtek_mdio_write(void *ctx, u32 reg, u32 val) > +{ > + struct realtek_priv *priv = ctx; > + > + return realtek_mdio_write_reg(priv, reg, val); > +} > + > +static int realtek_mdio_read(void *ctx, u32 reg, u32 *val) > +{ > + struct realtek_priv *priv = ctx; > + > + return realtek_mdio_read_reg(priv, reg, val); > +} > + > +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, > +}; > + > +static int realtek_mdio_probe(struct mdio_device *mdiodev) > +{ > + struct realtek_priv *priv; > + struct device *dev = &mdiodev->dev; > + const struct realtek_variant *var; > + int ret; > + struct device_node *np; > + > + var = of_device_get_match_data(dev); > + priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->map = devm_regmap_init(dev, NULL, priv, &realtek_mdio_regmap_config); > + if (IS_ERR(priv->map)) { > + ret = PTR_ERR(priv->map); > + dev_err(dev, "regmap init failed: %d\n", ret); > + return ret; > + } > + > + priv->phy_id = mdiodev->addr; > + priv->bus = mdiodev->bus; > + 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->ops = var->ops; > + > + priv->write_reg_noack = realtek_mdio_write_reg; > + > + 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"); > + > + 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; > + > + ret = dsa_register_switch(priv->ds); > + if (ret) { > + dev_err(priv->dev, "unable to register switch ret = %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static void realtek_mdio_remove(struct mdio_device *mdiodev) > +{ > + struct realtek_priv *priv = dev_get_drvdata(&mdiodev->dev); > + > + if (!priv) > + return; > + > + dsa_unregister_switch(priv->ds); > + > + dev_set_drvdata(&mdiodev->dev, NULL); > +} > + > +static void realtek_mdio_shutdown(struct mdio_device *mdiodev) > +{ > + struct realtek_priv *priv = dev_get_drvdata(&mdiodev->dev); > + > + if (!priv) > + return; > + > + dsa_switch_shutdown(priv->ds); > + > + 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 = of_match_ptr(realtek_mdio_of_match), > + }, > + .probe = realtek_mdio_probe, > + .remove = realtek_mdio_remove, > + .shutdown = realtek_mdio_shutdown, > +}; > + > +mdio_module_driver(realtek_mdio_driver); > + > +MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); > +MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h > index a03de15c4a94..97274273cb3b 100644 > --- a/drivers/net/dsa/realtek/realtek.h > +++ b/drivers/net/dsa/realtek/realtek.h > @@ -50,6 +50,8 @@ struct realtek_priv { > struct gpio_desc *mdio; > struct regmap *map; > struct mii_bus *slave_mii_bus; > + struct mii_bus *bus; > + int phy_id; > > unsigned int clk_delay; > u8 cmd_read;
On 1/4/2022 7:15 PM, Luiz Angelo Daros de Luca wrote: > This driver is a mdio_driver instead of a platform driver (like > realtek-smi). > > Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> > Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com> > --- > drivers/net/dsa/realtek/Kconfig | 11 +- > drivers/net/dsa/realtek/Makefile | 1 + > drivers/net/dsa/realtek/realtek-mdio.c | 221 +++++++++++++++++++++++++ > drivers/net/dsa/realtek/realtek.h | 2 + > 4 files changed, 233 insertions(+), 2 deletions(-) > create mode 100644 drivers/net/dsa/realtek/realtek-mdio.c > > diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig > index cd1aa95b7bf0..73b26171fade 100644 > --- a/drivers/net/dsa/realtek/Kconfig > +++ b/drivers/net/dsa/realtek/Kconfig > @@ -9,6 +9,13 @@ menuconfig NET_DSA_REALTEK > help > Select to enable support for Realtek Ethernet switch chips. > > +config NET_DSA_REALTEK_MDIO > + tristate "Realtek MDIO connected switch driver" > + depends on NET_DSA_REALTEK > + default y I suppose this is fine since we depend on NET_DSA_REALTEK. [snip] > +static int realtek_mdio_read_reg(struct realtek_priv *priv, u32 addr, u32 *data) > +{ > + u32 phy_id = priv->phy_id; > + struct mii_bus *bus = priv->bus; > + > + mutex_lock(&bus->mdio_lock); > + > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL0_REG, REALTEK_MDIO_ADDR_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_ADDRESS_REG, addr); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL1_REG, REALTEK_MDIO_READ_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + *data = bus->read(bus, phy_id, REALTEK_MDIO_DATA_READ_REG); Do you have no way to return an error for instance, if you read from a non-existent PHY device on the MDIO bus, -EIO would be expected for instance. If the data returned is 0xffff that ought to be enough. > + > + mutex_unlock(&bus->mdio_lock); > + > + return 0; > +} > + > +static int realtek_mdio_write_reg(struct realtek_priv *priv, u32 addr, u32 data) > +{ > + u32 phy_id = priv->phy_id; > + struct mii_bus *bus = priv->bus; > + > + mutex_lock(&bus->mdio_lock); > + > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL0_REG, REALTEK_MDIO_ADDR_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_ADDRESS_REG, addr); This repeats between read and writes, might be worth a helper function. > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_DATA_WRITE_REG, data); > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL1_REG, REALTEK_MDIO_WRITE_OP); > + > + mutex_unlock(&bus->mdio_lock); > + > + return 0; > +} > + > +/* Regmap accessors */ > + > +static int realtek_mdio_write(void *ctx, u32 reg, u32 val) > +{ > + struct realtek_priv *priv = ctx; > + > + return realtek_mdio_write_reg(priv, reg, val); > +} > + > +static int realtek_mdio_read(void *ctx, u32 reg, u32 *val) > +{ > + struct realtek_priv *priv = ctx; > + > + return realtek_mdio_read_reg(priv, reg, val); > +} Do you see a value for this function as oppposed to inlining the bodies of realtek_mdio_read_reg and realtek_mdio_write_reg directly into these two functions? > + > +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, > +}; > + > +static int realtek_mdio_probe(struct mdio_device *mdiodev) > +{ > + struct realtek_priv *priv; > + struct device *dev = &mdiodev->dev; > + const struct realtek_variant *var; > + int ret; > + struct device_node *np; > + > + var = of_device_get_match_data(dev); Don't you have to check that var is non-NULL just in case? > + priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->map = devm_regmap_init(dev, NULL, priv, &realtek_mdio_regmap_config); > + if (IS_ERR(priv->map)) { > + ret = PTR_ERR(priv->map); > + dev_err(dev, "regmap init failed: %d\n", ret); > + return ret; > + } > + > + priv->phy_id = mdiodev->addr; Please use a more descriptive variable name such as mdio_addr or something like that. I know that phy_id is typically used but it could also mean a 32-bit PHY unique identifier, which a MDIO device does not have typically. Looks fine otherwise.
> > +static int realtek_mdio_read_reg(struct realtek_priv *priv, u32 addr, u32 *data) > > +{ > > + u32 phy_id = priv->phy_id; > > + struct mii_bus *bus = priv->bus; > > + > > + mutex_lock(&bus->mdio_lock); > > + > > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL0_REG, REALTEK_MDIO_ADDR_OP); > > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > > + bus->write(bus, phy_id, REALTEK_MDIO_ADDRESS_REG, addr); > > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL1_REG, REALTEK_MDIO_READ_OP); > > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > > + *data = bus->read(bus, phy_id, REALTEK_MDIO_DATA_READ_REG); > > Do you have no way to return an error for instance, if you read from a > non-existent PHY device on the MDIO bus, -EIO would be expected for > instance. If the data returned is 0xffff that ought to be enough. I'll check for error (non zero for write, negative for read) and return that value > > + > > + mutex_unlock(&bus->mdio_lock); > > + > > + return 0; > > +} > > + > > +static int realtek_mdio_write_reg(struct realtek_priv *priv, u32 addr, u32 data) > > +{ > > + u32 phy_id = priv->phy_id; > > + struct mii_bus *bus = priv->bus; > > + > > + mutex_lock(&bus->mdio_lock); > > + > > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL0_REG, REALTEK_MDIO_ADDR_OP); > > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > > + bus->write(bus, phy_id, REALTEK_MDIO_ADDRESS_REG, addr); > > This repeats between read and writes, might be worth a helper function. Without the REALTEK_MDIO_START_OP Alvin asked, it is not worth it anymore. > > > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > > + bus->write(bus, phy_id, REALTEK_MDIO_DATA_WRITE_REG, data); > > + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); > > + bus->write(bus, phy_id, REALTEK_MDIO_CTRL1_REG, REALTEK_MDIO_WRITE_OP); > > + > > + mutex_unlock(&bus->mdio_lock); > > + > > + return 0; > > +} > > + > > +/* Regmap accessors */ > > + > > +static int realtek_mdio_write(void *ctx, u32 reg, u32 val) > > +{ > > + struct realtek_priv *priv = ctx; > > + > > + return realtek_mdio_write_reg(priv, reg, val); > > +} > > + > > +static int realtek_mdio_read(void *ctx, u32 reg, u32 *val) > > +{ > > + struct realtek_priv *priv = ctx; > > + > > + return realtek_mdio_read_reg(priv, reg, val); > > +} > > Do you see a value for this function as oppposed to inlining the bodies > of realtek_mdio_read_reg and realtek_mdio_write_reg directly into these > two functions? > I merged them. I also changed the write_reg_noack signature to match regmap->write_reg, so I can use it without a wrapper. > > + > > +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, > > +}; > > + > > +static int realtek_mdio_probe(struct mdio_device *mdiodev) > > +{ > > + struct realtek_priv *priv; > > + struct device *dev = &mdiodev->dev; > > + const struct realtek_variant *var; > > + int ret; > > + struct device_node *np; > > + > > + var = of_device_get_match_data(dev); > > Don't you have to check that var is non-NULL just in case? I'll add that check but it is not likely to happen. > > > + priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->map = devm_regmap_init(dev, NULL, priv, &realtek_mdio_regmap_config); > > + if (IS_ERR(priv->map)) { > > + ret = PTR_ERR(priv->map); > > + dev_err(dev, "regmap init failed: %d\n", ret); > > + return ret; > > + } > > + > > + priv->phy_id = mdiodev->addr; > > Please use a more descriptive variable name such as mdio_addr or > something like that. I know that phy_id is typically used but it could > also mean a 32-bit PHY unique identifier, which a MDIO device does not > have typically. Renamed to mdio_addr. As you said, I just used what is typically used but you know best. > > Looks fine otherwise. Thanks, Florian. Luiz
diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig index cd1aa95b7bf0..73b26171fade 100644 --- a/drivers/net/dsa/realtek/Kconfig +++ b/drivers/net/dsa/realtek/Kconfig @@ -9,6 +9,13 @@ menuconfig NET_DSA_REALTEK help Select to enable support for Realtek Ethernet switch chips. +config NET_DSA_REALTEK_MDIO + tristate "Realtek MDIO connected switch driver" + depends on NET_DSA_REALTEK + default y + help + Select to enable support for registering switches configured + through MDIO. config NET_DSA_REALTEK_SMI tristate "Realtek SMI connected switch driver" depends on NET_DSA_REALTEK @@ -21,7 +28,7 @@ config NET_DSA_REALTEK_RTL8365MB tristate "Realtek RTL8365MB switch subdriver" default y depends on NET_DSA_REALTEK - depends on NET_DSA_REALTEK_SMI + depends on NET_DSA_REALTEK_SMI || NET_DSA_REALTEK_MDIO select NET_DSA_TAG_RTL8_4 help Select to enable support for Realtek RTL8365MB @@ -30,7 +37,7 @@ config NET_DSA_REALTEK_RTL8366RB tristate "Realtek RTL8366RB switch subdriver" default y depends on NET_DSA_REALTEK - depends on NET_DSA_REALTEK_SMI + 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/Makefile b/drivers/net/dsa/realtek/Makefile index 8b5a4abcedd3..0aab57252a7c 100644 --- a/drivers/net/dsa/realtek/Makefile +++ b/drivers/net/dsa/realtek/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +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 rtl8366-objs := rtl8366-core.o rtl8366rb.o diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c new file mode 100644 index 000000000000..b505f4d3c5f0 --- /dev/null +++ b/drivers/net/dsa/realtek/realtek-mdio.c @@ -0,0 +1,221 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Realtek MDIO interface driver + * + * ASICs we intend to support with this driver: + * + * RTL8366 - The original version, apparently + * RTL8369 - Similar enough to have the same datsheet as RTL8366 + * RTL8366RB - Probably reads out "RTL8366 revision B", has a quite + * different register layout from the other two + * RTL8366S - Is this "RTL8366 super"? + * RTL8367 - Has an OpenWRT driver as well + * RTL8368S - Seems to be an alternative name for RTL8366RB + * RTL8370 - Also uses SMI + * + * Copyright (C) 2017 Linus Walleij <linus.walleij@linaro.org> + * Copyright (C) 2010 Antti Seppälä <a.seppala@gmail.com> + * Copyright (C) 2010 Roman Yeryomin <roman@advem.lv> + * Copyright (C) 2011 Colin Leitner <colin.leitner@googlemail.com> + * Copyright (C) 2009-2010 Gabor Juhos <juhosg@openwrt.org> + */ + +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/regmap.h> + +#include "realtek.h" + +/* Read/write via mdiobus */ +#define REALTEK_MDIO_CTRL0_REG 31 +#define REALTEK_MDIO_START_REG 29 +#define REALTEK_MDIO_CTRL1_REG 21 +#define REALTEK_MDIO_ADDRESS_REG 23 +#define REALTEK_MDIO_DATA_WRITE_REG 24 +#define REALTEK_MDIO_DATA_READ_REG 25 + +#define REALTEK_MDIO_START_OP 0xFFFF +#define REALTEK_MDIO_ADDR_OP 0x000E +#define REALTEK_MDIO_READ_OP 0x0001 +#define REALTEK_MDIO_WRITE_OP 0x0003 + +static int realtek_mdio_read_reg(struct realtek_priv *priv, u32 addr, u32 *data) +{ + u32 phy_id = priv->phy_id; + struct mii_bus *bus = priv->bus; + + mutex_lock(&bus->mdio_lock); + + bus->write(bus, phy_id, REALTEK_MDIO_CTRL0_REG, REALTEK_MDIO_ADDR_OP); + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); + bus->write(bus, phy_id, REALTEK_MDIO_ADDRESS_REG, addr); + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); + bus->write(bus, phy_id, REALTEK_MDIO_CTRL1_REG, REALTEK_MDIO_READ_OP); + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); + *data = bus->read(bus, phy_id, REALTEK_MDIO_DATA_READ_REG); + + mutex_unlock(&bus->mdio_lock); + + return 0; +} + +static int realtek_mdio_write_reg(struct realtek_priv *priv, u32 addr, u32 data) +{ + u32 phy_id = priv->phy_id; + struct mii_bus *bus = priv->bus; + + mutex_lock(&bus->mdio_lock); + + bus->write(bus, phy_id, REALTEK_MDIO_CTRL0_REG, REALTEK_MDIO_ADDR_OP); + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); + bus->write(bus, phy_id, REALTEK_MDIO_ADDRESS_REG, addr); + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); + bus->write(bus, phy_id, REALTEK_MDIO_DATA_WRITE_REG, data); + bus->write(bus, phy_id, REALTEK_MDIO_START_REG, REALTEK_MDIO_START_OP); + bus->write(bus, phy_id, REALTEK_MDIO_CTRL1_REG, REALTEK_MDIO_WRITE_OP); + + mutex_unlock(&bus->mdio_lock); + + return 0; +} + +/* Regmap accessors */ + +static int realtek_mdio_write(void *ctx, u32 reg, u32 val) +{ + struct realtek_priv *priv = ctx; + + return realtek_mdio_write_reg(priv, reg, val); +} + +static int realtek_mdio_read(void *ctx, u32 reg, u32 *val) +{ + struct realtek_priv *priv = ctx; + + return realtek_mdio_read_reg(priv, reg, val); +} + +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, +}; + +static int realtek_mdio_probe(struct mdio_device *mdiodev) +{ + struct realtek_priv *priv; + struct device *dev = &mdiodev->dev; + const struct realtek_variant *var; + int ret; + struct device_node *np; + + var = of_device_get_match_data(dev); + priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->map = devm_regmap_init(dev, NULL, priv, &realtek_mdio_regmap_config); + if (IS_ERR(priv->map)) { + ret = PTR_ERR(priv->map); + dev_err(dev, "regmap init failed: %d\n", ret); + return ret; + } + + priv->phy_id = mdiodev->addr; + priv->bus = mdiodev->bus; + 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->ops = var->ops; + + priv->write_reg_noack = realtek_mdio_write_reg; + + 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"); + + 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; + + ret = dsa_register_switch(priv->ds); + if (ret) { + dev_err(priv->dev, "unable to register switch ret = %d\n", ret); + return ret; + } + + return 0; +} + +static void realtek_mdio_remove(struct mdio_device *mdiodev) +{ + struct realtek_priv *priv = dev_get_drvdata(&mdiodev->dev); + + if (!priv) + return; + + dsa_unregister_switch(priv->ds); + + dev_set_drvdata(&mdiodev->dev, NULL); +} + +static void realtek_mdio_shutdown(struct mdio_device *mdiodev) +{ + struct realtek_priv *priv = dev_get_drvdata(&mdiodev->dev); + + if (!priv) + return; + + dsa_switch_shutdown(priv->ds); + + 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 = of_match_ptr(realtek_mdio_of_match), + }, + .probe = realtek_mdio_probe, + .remove = realtek_mdio_remove, + .shutdown = realtek_mdio_shutdown, +}; + +mdio_module_driver(realtek_mdio_driver); + +MODULE_AUTHOR("Luiz Angelo Daros de Luca <luizluca@gmail.com>"); +MODULE_DESCRIPTION("Driver for Realtek ethernet switch connected via MDIO interface"); +MODULE_LICENSE("GPL"); diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h index a03de15c4a94..97274273cb3b 100644 --- a/drivers/net/dsa/realtek/realtek.h +++ b/drivers/net/dsa/realtek/realtek.h @@ -50,6 +50,8 @@ struct realtek_priv { struct gpio_desc *mdio; struct regmap *map; struct mii_bus *slave_mii_bus; + struct mii_bus *bus; + int phy_id; unsigned int clk_delay; u8 cmd_read;