On 10/07/18 11:24, Pankaj Bansal wrote:
> Add support for an MDIO bus multiplexer controlled by a regmap
> device, like an FPGA.
>
> Tested on a NXP LX2160AQDS board which uses the "QIXIS" FPGA
> attached to the i2c bus.
>
> Signed-off-by: Pankaj Bansal <pankaj.ban...@nxp.com>
> ---
>
> Notes:
> V2:
> - replaced be32_to_cpup with of_property_read_u32
> - incorporated Andrew's comments
>
> drivers/net/phy/Kconfig | 13 +++
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/mdio-mux-regmap.c | 171 ++++++++++++++++++++++++++++
> 3 files changed, 185 insertions(+)
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 82070792edbb..d1ac9e70cbb2 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -87,6 +87,19 @@ config MDIO_BUS_MUX_MMIOREG
>
> Currently, only 8/16/32 bits registers are supported.
>
> +config MDIO_BUS_MUX_REGMAP
> + tristate "REGMAP controlled MDIO bus multiplexers"
> + depends on OF_MDIO && REGMAP
> + select MDIO_BUS_MUX
> + help
> + This module provides a driver for MDIO bus multiplexers that
> + are controlled via a regmap device, like an FPGA connected to i2c.
> + The multiplexer connects one of several child MDIO busses to a
> + parent bus.Child bus selection is under the control of one of
> + the FPGA's registers.
> +
> + Currently, only upto 32 bits registers are supported.
> +
> config MDIO_CAVIUM
> tristate
>
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 5805c0b7d60e..33053f9f320d 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_MDIO_BUS_MUX) += mdio-mux.o
> obj-$(CONFIG_MDIO_BUS_MUX_BCM_IPROC) += mdio-mux-bcm-iproc.o
> obj-$(CONFIG_MDIO_BUS_MUX_GPIO) += mdio-mux-gpio.o
> obj-$(CONFIG_MDIO_BUS_MUX_MMIOREG) += mdio-mux-mmioreg.o
> +obj-$(CONFIG_MDIO_BUS_MUX_REGMAP) += mdio-mux-regmap.o
> obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o
> obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o
> obj-$(CONFIG_MDIO_HISI_FEMAC) += mdio-hisi-femac.o
> diff --git a/drivers/net/phy/mdio-mux-regmap.c
> b/drivers/net/phy/mdio-mux-regmap.c
> new file mode 100644
> index 000000000000..6068d05a728a
> --- /dev/null
> +++ b/drivers/net/phy/mdio-mux-regmap.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/* Simple regmap based MDIO MUX driver
> + *
> + * Copyright 2018 NXP
> + *
> + * Based on mdio-mux-mmioreg.c by Timur Tabi
> + *
> + * Author:
> + * Pankaj Bansal <pankaj.ban...@nxp.com>
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/device.h>
> +#include <linux/of_mdio.h>
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +#include <linux/mdio-mux.h>
> +#include <linux/regmap.h>
> +
> +struct mdio_mux_regmap_state {
> + void *mux_handle;
> + struct regmap *regmap;
> + u32 mux_reg;
> + u32 mask;
> +};
> +
> +/* MDIO multiplexing switch function
> + *
> + * This function is called by the mdio-mux layer when it thinks the mdio bus
> + * multiplexer needs to switch.
> + *
> + * 'current_child' is the current value of the mux register (masked via
> + * s->mask).
> + *
> + * 'desired_child' is the value of the 'reg' property of the target child
> MDIO
> + * node.
> + *
> + * The first time this function is called, current_child == -1.
> + *
> + * If current_child == desired_child, then the mux is already set to the
> + * correct bus.
> + */
> +static int mdio_mux_regmap_switch_fn(int current_child, int desired_child,
> + void *data)
> +{
> + struct mdio_mux_regmap_state *s = data;
> + bool change;
> + int ret;
> +
> + ret = regmap_update_bits_check(s->regmap,
> + s->mux_reg,
> + s->mask,
> + desired_child,
> + &change);
> +
> + if (ret)
> + return ret;
> + if (change)
> + pr_debug("%s %d -> %d\n", __func__, current_child,
> + desired_child);
If you add a struct platform_device or struct device reference to struct
mdio_mux_regmap_state, the you can use dev_dbg() here with the correct
device, which would be helpful if you are debugging problems, and there
are more than once instance of them in the system.
> + return ret;
> +}
> +
> +static int mdio_mux_regmap_probe(struct platform_device *pdev)
> +{
> + struct device_node *np2, *np = pdev->dev.of_node;
How about naming "np2", "child" instead?
Everything else looks fine to me, thanks!
--
Florian
