> -----Original Message----- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Sunday, October 7, 2018 11:39 PM > To: Pankaj Bansal <pankaj.ban...@nxp.com>; Andrew Lunn <and...@lunn.ch> > Cc: netdev@vger.kernel.org > Subject: Re: [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a > regmap > > > > 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.
Ok, I will add platform_device reference to struct. > > > + 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? Ok. I will rename this variable > > Everything else looks fine to me, thanks! > -- > Florian