diff mbox series

[net-next,08/13] net: dsa: realtek: add new mdio interface for drivers

Message ID 20211216201342.25587-9-luizluca@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: realtek: MDIO interface and RTL8367S | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 4 this patch: 6
netdev/cc_maintainers warning 2 maintainers not CCed: kuba@kernel.org davem@davemloft.net
netdev/build_clang fail Errors and warnings before: 3 this patch: 5
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 4 this patch: 6
netdev/checkpatch fail CHECK: Please don't use multiple blank lines CHECK: Please use a blank line after function/struct/union/enum declarations ERROR: code indent should use tabs where possible ERROR: spaces required around that '=' (ctx:VxV) WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON() WARNING: Block comments use * on subsequent lines WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 82 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: please write a paragraph that describes the config symbol fully WARNING: please, no space before tabs WARNING: please, no spaces at the start of a line
netdev/kdoc success Errors and warnings before: 19 this patch: 19
netdev/source_inline success Was 0 now: 0

Commit Message

Luiz Angelo Daros de Luca Dec. 16, 2021, 8:13 p.m. UTC
From: Luiz Angelo Daros de Luca <luizluca@gmail.com>

This driver is a mdio_driver instead of a platform driver (like
realtek-smi).

Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 drivers/net/dsa/realtek/Kconfig        |  11 +-
 drivers/net/dsa/realtek/Makefile       |   1 +
 drivers/net/dsa/realtek/realtek-mdio.c | 284 +++++++++++++++++++++++++
 drivers/net/dsa/realtek/realtek.h      |   3 +
 4 files changed, 297 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/dsa/realtek/realtek-mdio.c

Comments

Alvin Šipraga Dec. 17, 2021, 12:11 a.m. UTC | #1
Hi Luiz,

On 12/16/21 21:13, luizluca@gmail.com wrote:
> From: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> 
> This driver is a mdio_driver instead of a platform driver (like
> realtek-smi).
> 
> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> ---

Great work giving the switches MDIO support. Left comments down below.

I'm not so pleased with how this driver is split up - nor was I before - 
but something feels a bit off with all the code duplication. I'll think 
about it but maybe somebody else can share some experience they have 
from other drivers.

I didn't check the register access details just yet.


>   drivers/net/dsa/realtek/Kconfig        |  11 +-
>   drivers/net/dsa/realtek/Makefile       |   1 +
>   drivers/net/dsa/realtek/realtek-mdio.c | 284 +++++++++++++++++++++++++
>   drivers/net/dsa/realtek/realtek.h      |   3 +
>   4 files changed, 297 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 874574db9177..48194d0dd51f 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
> @@ -20,7 +27,7 @@ config NET_DSA_REALTEK_RTL8367C
>   	tristate "Realtek RTL8367C 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-VC. This subdriver
> @@ -32,7 +39,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 84d5ab062c89..01df2ccbb77f 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..b7febd63e04f
> --- /dev/null
> +++ b/drivers/net/dsa/realtek/realtek-mdio.c
> @@ -0,0 +1,284 @@
> +// 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/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/spinlock.h>
> +#include <linux/skbuff.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_mdio.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/bitops.h>
> +#include <linux/if_bridge.h>
> +
> +#include "realtek.h"
> +
> +/* Read/write via mdiobus */
> +#define MDC_MDIO_CTRL0_REG              31
> +#define MDC_MDIO_START_REG              29
> +#define MDC_MDIO_CTRL1_REG              21
> +#define MDC_MDIO_ADDRESS_REG            23
> +#define MDC_MDIO_DATA_WRITE_REG         24
> +#define MDC_MDIO_DATA_READ_REG          25
> +
> +#define MDC_MDIO_START_OP               0xFFFF
> +#define MDC_MDIO_ADDR_OP                0x000E
> +#define MDC_MDIO_READ_OP                0x0001
> +#define MDC_MDIO_WRITE_OP               0x0003
> +#define MDC_REALTEK_DEFAULT_PHY_ADDR    0x0
> +
> +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;
> +
> +        BUG_ON(in_interrupt());

No BUG_ON please, just make sure that this never happens. This will 
crash the system.

> +
> +        mutex_lock(&bus->mdio_lock);
> +        /* Write Start command to register 29 */
> +        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);

Just noticed you are mixing tabs and spaces too. You can try running 
clang-format or similar on your code before sending v2. Be aware it will 
mess up the #defines.

> +
> +        /* Write address control code to register 31 */
> +        bus->write(bus, phy_id, MDC_MDIO_CTRL0_REG, MDC_MDIO_ADDR_OP);
> +
> +        /* Write Start command to register 29 */
> +        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
> +
> +        /* Write address to register 23 */
> +        bus->write(bus, phy_id, MDC_MDIO_ADDRESS_REG, addr);
> +
> +        /* Write Start command to register 29 */
> +        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
> +
> +        /* Write read control code to register 21 */
> +        bus->write(bus, phy_id, MDC_MDIO_CTRL1_REG, MDC_MDIO_READ_OP);
> +
> +        /* Write Start command to register 29 */
> +        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
> +
> +        /* Read data from register 25 */
> +        *data = bus->read(bus, phy_id, MDC_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;
> +
> +        BUG_ON(in_interrupt());
> +
> +        mutex_lock(&bus->mdio_lock);
> +
> +        /* Write Start command to register 29 */
> +        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
> +
> +        /* Write address control code to register 31 */
> +        bus->write(bus, phy_id, MDC_MDIO_CTRL0_REG, MDC_MDIO_ADDR_OP);
> +
> +        /* Write Start command to register 29 */
> +        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
> +
> +        /* Write address to register 23 */
> +        bus->write(bus, phy_id, MDC_MDIO_ADDRESS_REG, addr);
> +
> +        /* Write Start command to register 29 */
> +        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
> +
> +        /* Write data to register 24 */
> +        bus->write(bus, phy_id, MDC_MDIO_DATA_WRITE_REG, data);
> +
> +        /* Write Start command to register 29 */
> +        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
> +
> +        /* Write data control code to register 21 */
> +        bus->write(bus, phy_id, MDC_MDIO_CTRL1_REG, MDC_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->phy_id = mdiodev->addr;
> +
> +	// Start by setting up the register mapping
> +	priv->map = devm_regmap_init(dev, NULL, priv, &realtek_mdio_regmap_config);
> +
> +	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;
> +
> +	if (IS_ERR(priv->map))
> +		dev_warn(dev, "regmap initialization failed");
> +
> +	priv->write_reg_noack=realtek_mdio_write_reg;
> +
> +	np = dev->of_node;
> +
> +	dev_set_drvdata(dev, priv);
> +	spin_lock_init(&priv->lock);

It's not even used here.

> +
> +	/* TODO: if power is software controlled, set up any regulators here */
> +
> +	/* FIXME: maybe skip if no gpio but reset after the switch was detected */
> +	/* Assert then deassert RESET */
> +	/*
> +	priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(priv->reset)) {
> +		dev_err(dev, "failed to get RESET GPIO\n");
> +		return PTR_ERR(priv->reset);
> +	}
> +	msleep(REALTEK_SMI_HW_STOP_DELAY);

After all the renaming I'm surprised to see _SMI in the MDIO code now...

> +	gpiod_set_value(priv->reset, 0);
> +	msleep(REALTEK_SMI_HW_START_DELAY);
> +	dev_info(dev, "deasserted RESET\n");
> +	*/

Leftover debug? Nobody wants to see block commented code :-)

> +
> +	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);
> +	//gpiod_set_value(smi->reset, 1);

More here.

> +	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
> +	/* FIXME: add support for RTL8366S and more */
> +	{ .compatible = "realtek,rtl8366s", .data = NULL, },
> +#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8367C)
> +	{ .compatible = "realtek,rtl8365mb", .data = &rtl8367c_variant, },

Did you test on an RTL8365MB-VC with MDIO? I know the 66RB is not tested.

> +#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_LICENSE("GPL");
> diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
> index 976cb7823c92..c1f20a23ab9e 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;
> @@ -66,6 +68,7 @@ struct realtek_priv {
>   	struct rtl8366_mib_counter *mib_counters;
>   
>   	const struct realtek_ops *ops;
> +	int 		 	(*setup)(struct dsa_switch *ds);

What's this for?

>   	int 		 	(*setup_interface)(struct dsa_switch *ds);
>   	int 			(*write_reg_noack)(struct realtek_priv *priv, u32 addr, u32 data);
>
kernel test robot Dec. 17, 2021, 3:33 a.m. UTC | #2
Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/luizluca-gmail-com/net-dsa-realtek-MDIO-interface-and-RTL8367S/20211217-041735
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 0f473bb6ed2d0b8533a079ee133f625f83de5315
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20211217/202112171150.GzuAOv0N-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/15bfe75ad3669cdcef7bfab281d7744c226fc503
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review luizluca-gmail-com/net-dsa-realtek-MDIO-interface-and-RTL8367S/20211217-041735
        git checkout 15bfe75ad3669cdcef7bfab281d7744c226fc503
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash drivers/net/dsa/realtek/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/net/dsa/realtek/realtek-mdio.c:54:5: warning: no previous prototype for 'realtek_mdio_read_reg' [-Wmissing-prototypes]
      54 | int realtek_mdio_read_reg(struct realtek_priv *priv, u32 addr, u32 *data)
         |     ^~~~~~~~~~~~~~~~~~~~~


vim +/realtek_mdio_read_reg +54 drivers/net/dsa/realtek/realtek-mdio.c

    53	
  > 54	int realtek_mdio_read_reg(struct realtek_priv *priv, u32 addr, u32 *data)
    55	{
    56	        u32 phy_id = priv->phy_id;
    57		struct mii_bus *bus = priv->bus;
    58	
    59	        BUG_ON(in_interrupt());
    60	
    61	        mutex_lock(&bus->mdio_lock);
    62	        /* Write Start command to register 29 */
    63	        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
    64	
    65	        /* Write address control code to register 31 */
    66	        bus->write(bus, phy_id, MDC_MDIO_CTRL0_REG, MDC_MDIO_ADDR_OP);
    67	
    68	        /* Write Start command to register 29 */
    69	        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
    70	
    71	        /* Write address to register 23 */
    72	        bus->write(bus, phy_id, MDC_MDIO_ADDRESS_REG, addr);
    73	
    74	        /* Write Start command to register 29 */
    75	        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
    76	
    77	        /* Write read control code to register 21 */
    78	        bus->write(bus, phy_id, MDC_MDIO_CTRL1_REG, MDC_MDIO_READ_OP);
    79	
    80	        /* Write Start command to register 29 */
    81	        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
    82	
    83	        /* Read data from register 25 */
    84	        *data = bus->read(bus, phy_id, MDC_MDIO_DATA_READ_REG);
    85	
    86	        mutex_unlock(&bus->mdio_lock);
    87	
    88	        return 0;
    89	}
    90	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Linus Walleij Dec. 18, 2021, 2:54 a.m. UTC | #3
On Thu, Dec 16, 2021 at 9:14 PM <luizluca@gmail.com> wrote:
> From: Luiz Angelo Daros de Luca <luizluca@gmail.com>
>
> This driver is a mdio_driver instead of a platform driver (like
> realtek-smi).
>
> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>

Needless to say I'm a big fan of the patch, I'll wait until the next
version and then I'll test it on the actual hardware as well so we
make sure RTL8366RB is still working after all this!

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/net/dsa/realtek/Kconfig b/drivers/net/dsa/realtek/Kconfig
index 874574db9177..48194d0dd51f 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
@@ -20,7 +27,7 @@  config NET_DSA_REALTEK_RTL8367C
 	tristate "Realtek RTL8367C 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-VC. This subdriver
@@ -32,7 +39,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 84d5ab062c89..01df2ccbb77f 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..b7febd63e04f
--- /dev/null
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -0,0 +1,284 @@ 
+// 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/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/spinlock.h>
+#include <linux/skbuff.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_mdio.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/bitops.h>
+#include <linux/if_bridge.h>
+
+#include "realtek.h"
+
+/* Read/write via mdiobus */
+#define MDC_MDIO_CTRL0_REG              31
+#define MDC_MDIO_START_REG              29
+#define MDC_MDIO_CTRL1_REG              21
+#define MDC_MDIO_ADDRESS_REG            23
+#define MDC_MDIO_DATA_WRITE_REG         24
+#define MDC_MDIO_DATA_READ_REG          25
+
+#define MDC_MDIO_START_OP               0xFFFF
+#define MDC_MDIO_ADDR_OP                0x000E
+#define MDC_MDIO_READ_OP                0x0001
+#define MDC_MDIO_WRITE_OP               0x0003
+#define MDC_REALTEK_DEFAULT_PHY_ADDR    0x0
+
+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;
+
+        BUG_ON(in_interrupt());
+
+        mutex_lock(&bus->mdio_lock);
+        /* Write Start command to register 29 */
+        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
+
+        /* Write address control code to register 31 */
+        bus->write(bus, phy_id, MDC_MDIO_CTRL0_REG, MDC_MDIO_ADDR_OP);
+
+        /* Write Start command to register 29 */
+        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
+
+        /* Write address to register 23 */
+        bus->write(bus, phy_id, MDC_MDIO_ADDRESS_REG, addr);
+
+        /* Write Start command to register 29 */
+        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
+
+        /* Write read control code to register 21 */
+        bus->write(bus, phy_id, MDC_MDIO_CTRL1_REG, MDC_MDIO_READ_OP);
+
+        /* Write Start command to register 29 */
+        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
+
+        /* Read data from register 25 */
+        *data = bus->read(bus, phy_id, MDC_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;
+
+        BUG_ON(in_interrupt());
+
+        mutex_lock(&bus->mdio_lock);
+
+        /* Write Start command to register 29 */
+        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
+
+        /* Write address control code to register 31 */
+        bus->write(bus, phy_id, MDC_MDIO_CTRL0_REG, MDC_MDIO_ADDR_OP);
+
+        /* Write Start command to register 29 */
+        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
+
+        /* Write address to register 23 */
+        bus->write(bus, phy_id, MDC_MDIO_ADDRESS_REG, addr);
+
+        /* Write Start command to register 29 */
+        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
+
+        /* Write data to register 24 */
+        bus->write(bus, phy_id, MDC_MDIO_DATA_WRITE_REG, data);
+
+        /* Write Start command to register 29 */
+        bus->write(bus, phy_id, MDC_MDIO_START_REG, MDC_MDIO_START_OP);
+
+        /* Write data control code to register 21 */
+        bus->write(bus, phy_id, MDC_MDIO_CTRL1_REG, MDC_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->phy_id = mdiodev->addr;
+
+	// Start by setting up the register mapping
+	priv->map = devm_regmap_init(dev, NULL, priv, &realtek_mdio_regmap_config);
+
+	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;
+
+	if (IS_ERR(priv->map))
+		dev_warn(dev, "regmap initialization failed");
+
+	priv->write_reg_noack=realtek_mdio_write_reg;
+
+	np = dev->of_node;
+
+	dev_set_drvdata(dev, priv);
+	spin_lock_init(&priv->lock);
+
+	/* TODO: if power is software controlled, set up any regulators here */
+
+	/* FIXME: maybe skip if no gpio but reset after the switch was detected */
+	/* Assert then deassert RESET */
+	/*
+	priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->reset)) {
+		dev_err(dev, "failed to get RESET GPIO\n");
+		return PTR_ERR(priv->reset);
+	}
+	msleep(REALTEK_SMI_HW_STOP_DELAY);
+	gpiod_set_value(priv->reset, 0);
+	msleep(REALTEK_SMI_HW_START_DELAY);
+	dev_info(dev, "deasserted RESET\n");
+	*/
+
+	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);
+	//gpiod_set_value(smi->reset, 1);
+	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
+	/* FIXME: add support for RTL8366S and more */
+	{ .compatible = "realtek,rtl8366s", .data = NULL, },
+#if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8367C)
+	{ .compatible = "realtek,rtl8365mb", .data = &rtl8367c_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_LICENSE("GPL");
diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
index 976cb7823c92..c1f20a23ab9e 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;
@@ -66,6 +68,7 @@  struct realtek_priv {
 	struct rtl8366_mib_counter *mib_counters;
 
 	const struct realtek_ops *ops;
+	int 		 	(*setup)(struct dsa_switch *ds);
 	int 		 	(*setup_interface)(struct dsa_switch *ds);
 	int 			(*write_reg_noack)(struct realtek_priv *priv, u32 addr, u32 data);