All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] add MDIO bus multiplexer driven by a regmap device
@ 2019-02-04  8:59 Pankaj Bansal
  2019-02-04  8:59 ` [PATCH v2 1/2] dt-bindings: net: " Pankaj Bansal
  2019-02-04  8:59 ` [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap Pankaj Bansal
  0 siblings, 2 replies; 9+ messages in thread
From: Pankaj Bansal @ 2019-02-04  8:59 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev, Pankaj Bansal, Varun Sethi

Add support for an MDIO bus multiplexer controlled by a regmap device,
like an FPGA.

These apis is an extension of the existing driver
drivers/net/phy/mdio-mux-mmioreg.c.

The problem with mmioreg driver is that it can operate only on memory
mapped devices.
but if we have a device that controls mdio muxing and that device is
controlled using i2c or spi, then it will not work.

Therefore, added apis that can be used by regmap device to control mdio mux.

Tested on a NXP LX2160AQDS board which uses the "QIXIS" FPGA attached to the
i2c bus.

This is my second attempt at this.
In my previous approach i wrote a separate driver for regmap apis.
But then i realized that it is not meant to control a specific device.
It is meant to control some registers of parent device.
Therefore, IMO this should not be a Platform driver and there should not
be any "compatible" property to which this driver is associated.

The previous approach patches and discussion can be accessed here:
https://www.mail-archive.com/netdev@vger.kernel.org/msg252744.html

Cc: Varun Sethi <V.Sethi@nxp.com>
---

Notes:
	V1:
	- https://www.spinics.net/lists/netdev/msg548027.html

Pankaj Bansal (2):
  dt-bindings: net: add MDIO bus multiplexer driven by a regmap device
  netdev/phy: add MDIO bus multiplexer driven by a regmap

 .../bindings/net/mdio-mux-regmap.txt          | 167 +++++++++++++++++
 drivers/net/phy/Kconfig                       |  14 ++
 drivers/net/phy/Makefile                      |   1 +
 drivers/net/phy/mdio-mux-regmap.c             | 171 ++++++++++++++++++
 include/linux/mdio-mux.h                      |  34 ++++
 5 files changed, 387 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-regmap.txt
 create mode 100644 drivers/net/phy/mdio-mux-regmap.c

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/2] dt-bindings: net: add MDIO bus multiplexer driven by a regmap device
  2019-02-04  8:59 [PATCH v2 0/2] add MDIO bus multiplexer driven by a regmap device Pankaj Bansal
@ 2019-02-04  8:59 ` Pankaj Bansal
  2019-02-04  8:59 ` [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap Pankaj Bansal
  1 sibling, 0 replies; 9+ messages in thread
From: Pankaj Bansal @ 2019-02-04  8:59 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev, Pankaj Bansal

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.bansal@nxp.com>
---

Notes:
    V2:
    - New file describing the device tree bindings for regmap controlled devices'
      mdio mux

 .../bindings/net/mdio-mux-regmap.txt         | 167 +++++++++++++++++
 1 file changed, 167 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/mdio-mux-regmap.txt b/Documentation/devicetree/bindings/net/mdio-mux-regmap.txt
new file mode 100644
index 000000000000..8968f317965f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mdio-mux-regmap.txt
@@ -0,0 +1,167 @@
+Properties for an MDIO bus multiplexer controlled by a regmap
+
+This is a special case of a MDIO bus multiplexer.  A regmap device,
+like an FPGA, is used to control which child bus is connected.  The mdio-mux
+node must be a child of the device that is controlled by a regmap.
+The driver currently only supports devices with upto 32-bit registers.
+
+Required properties in addition to the generic multiplexer properties:
+
+- reg : integer, contains the offset of the register that controls the bus
+	multiplexer. it can be 32 bit number.
+
+- mux-mask : integer, contains an 32 bit mask that specifies which
+	bits in the register control the actual bus multiplexer.  The
+	'reg' property of each child mdio-mux node must be constrained by
+	this mask.
+
+Example 1:
+
+The FPGA node defines a i2c connected FPGA with a register space of 0x30 bytes.
+For the "EMI2" MDIO bus, register 0x54 (BRDCFG4) controls the mux on that bus.
+A bitmask of 0x07 means that bits 0, 1 and 2 (bit 0 is lsb) are the bits on
+BRDCFG4 that control the actual mux.
+
+i2c@2000000 {
+	compatible = "fsl,vf610-i2c";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x0 0x2000000 0x0 0x10000>;
+	interrupts = <0 34 0x4>; // Level high type
+	clock-names = "i2c";
+	clocks = <&clockgen 4 7>;
+	fsl-scl-gpio = <&gpio2 15 0>;
+	status = "okay";
+
+	/* The FPGA node */
+	fpga@66 {
+		compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
+		reg = <0x66>; // fpga device address on i2c bus
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mdio-mux-2@54 {
+			mdio-parent-bus = <&emdio2>; /* MDIO bus */
+			reg = <0x54>;	    /* BRDCFG4 */
+			mux-mask = <0x07>;      /* EMI2_MDIO */
+			#address-cells=<1>;
+			#size-cells = <0>;
+
+			mdio@0 {   // Slot 1
+				reg = <0x00>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				ethernet-phy@@1 {
+					reg = <1>;
+					compatible = "ethernet-phy-id0210.7441";
+				};
+
+				ethernet-phy@@0 {
+					reg = <0>;
+					compatible = "ethernet-phy-id0210.7441";
+				};
+			};
+
+			mdio@1 {   // Slot 2
+				reg = <0x01>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+			};
+
+			mdio@2 {   // Slot 3
+				reg = <0x02>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+			};
+		};
+	};
+};
+
+/* The parent MDIO bus. */
+emdio2: mdio@0x8B97000 {
+	compatible = "fsl,fman-memac-mdio";
+	reg = <0x0 0x8B97000 0x0 0x1000>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+	little-endian;
+};
+
+Example 2:
+
+The FPGA node defines a memory mapped FPGA with a register space of 0x100 bytes.
+For the "EMI1" MDIO bus, register 0x54 (BRDCFG4) controls the mux on that bus.
+A bitmask of 0xe0 means that bits 5, 6 and 7 (bit 0 is lsb) are the bits on
+BRDCFG4 that control the actual mux.
+
+ifc: ifc@1530000 {
+	compatible = "fsl,ifc", "simple-bus";
+	reg = <0x0 0x1530000 0x0 0x10000>;
+	interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
+	#address-cells = <2>;
+	#size-cells = <1>;
+	/* NOR, NAND Flashes and FPGA on board */
+	ranges = <0x0 0x0 0x0 0x60000000 0x08000000
+		  0x2 0x0 0x0 0x7e800000 0x00010000
+		  0x3 0x0 0x0 0x7fb00000 0x00000100>;
+	status = "okay";
+
+	/* The FPGA node */
+	fpga: board-control@3,0 {
+		compatible = "fsl,ls1021aqds-fpga", "fsl,fpga-qixis";
+		reg = <0x3 0x0 0x0000100>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mdio-mux-1@54 {
+			mdio-parent-bus = <&emdio1>; /* MDIO bus */
+			reg = <0x54>;	    /* BRDCFG4 */
+			mux-mask = <0xe0>;      /* EMI1_MDIO */
+			#address-cells=<1>;
+			#size-cells = <0>;
+
+			mdio@0 {   // Onboard PHYs rgmii_phy1
+				reg = <0x00>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				ethernet-phy@@1 {
+					reg = <1>;
+					compatible = "ethernet-phy-ieee802.3-c22";
+				};
+			};
+
+			mdio@20 {   // Onboard PHYs rgmii_phy2
+				reg = <0x20>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				ethernet-phy@2 {
+					reg = <0x2>;
+					compatible = "ethernet-phy-ieee802.3-c22";
+				};
+			};
+
+			mdio@40 {   // Onboard PHYs rgmii_phy3
+				reg = <0x40>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+				ethernet-phy@3 {
+					reg = <0x3>;
+					compatible = "ethernet-phy-ieee802.3-c22";
+				};
+			};
+		};
+	};
+};
+
+/* The parent MDIO bus. */
+emdio1: mdio@2d24000 {
+	compatible = "gianfar";
+	device_type = "mdio";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x0 0x2d24000 0x0 0x4000>,
+	      <0x0 0x2d10030 0x0 0x4>;
+};
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap
  2019-02-04  8:59 [PATCH v2 0/2] add MDIO bus multiplexer driven by a regmap device Pankaj Bansal
  2019-02-04  8:59 ` [PATCH v2 1/2] dt-bindings: net: " Pankaj Bansal
@ 2019-02-04  8:59 ` Pankaj Bansal
  2019-02-04 13:46   ` Andrew Lunn
  2019-02-04 14:51   ` kbuild test robot
  1 sibling, 2 replies; 9+ messages in thread
From: Pankaj Bansal @ 2019-02-04  8:59 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev, Pankaj Bansal

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.bansal@nxp.com>
---

Notes:
    V2:
    - Added Kconfig entry for regmap based mdio mux
    - restrict the comment lines to 80 chars
    - use kerneldoc formatting for this function documentation.

 drivers/net/phy/Kconfig           |  14 +++
 drivers/net/phy/Makefile          |   1 +
 drivers/net/phy/mdio-mux-regmap.c | 171 ++++++++++++++++++++++++++++
 include/linux/mdio-mux.h          |  34 ++++++
 4 files changed, 220 insertions(+)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3d187cd50eb0..93ef2505caba 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -87,6 +87,20 @@ config MDIO_BUS_MUX_MMIOREG
 
 	  Currently, only 8/16/32 bits registers are supported.
 
+config MDIO_BUS_MUX_REGMAP
+	tristate "regmap device 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 bus
+	  or spi bus or memory mapped FPGA.
+	  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 32 bits registers are supported.
+
 config MDIO_CAVIUM
 	tristate
 
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 5805c0b7d60e..0827a700eb31 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_MDIO_BCM_IPROC)	+= mdio-bcm-iproc.o
 obj-$(CONFIG_MDIO_BCM_UNIMAC)	+= mdio-bcm-unimac.o
 obj-$(CONFIG_MDIO_BITBANG)	+= mdio-bitbang.o
 obj-$(CONFIG_MDIO_BUS_MUX)	+= mdio-mux.o
+obj-$(CONFIG_MDIO_BUS_MUX_REGMAP)	+= mdio-mux-regmap.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
diff --git a/drivers/net/phy/mdio-mux-regmap.c b/drivers/net/phy/mdio-mux-regmap.c
new file mode 100644
index 000000000000..f2cb3cad1d9b
--- /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-2019 NXP
+ *
+ * Based on mdio-mux-mmioreg.c by Timur Tabi
+ *
+ * Author:
+ *     Pankaj Bansal <pankaj.bansal@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 device	*dev;
+	struct regmap	*regmap;
+	u32		mux_reg;
+	u32		mask;
+};
+
+/**
+ * mdio_mux_regmap_switch_fn - This function is called by the mdio-mux layer
+ *			       when it thinks the mdio bus multiplexer needs
+ *			       to switch.
+ * @current_child:  current value of the mux register (masked via s->mask).
+ * @desired_child: value of the 'reg' property of the target child MDIO node.
+ * @data: Private data used by this switch_fn passed to mdio_mux_init function
+ *	  via mdio_mux_init(.., .., .., .., data, ..).
+ *
+ * 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)
+		dev_dbg(s->dev, "%s %d -> %d\n", __func__, current_child,
+			desired_child);
+	return ret;
+}
+
+/**
+ * mdio_mux_regmap_init - control MDIO bus muxing using regmap constructs.
+ * @dev: device with which regmap construct is associated.
+ * @mux_node: mdio bus mux node that contains parent mdio bus phandle.
+ *	      This node also contains sub nodes, where each subnode denotes
+ *	      a child mdio bus. All the child mdio buses are muxed, i.e. at a
+ *	      time only one of the child mdio buses can be used.
+ * @data: to store the address of data allocated by this function
+ */
+int mdio_mux_regmap_init(struct device *dev,
+			 struct device_node *mux_node,
+			 void **data)
+{
+	struct device_node *child;
+	struct mdio_mux_regmap_state *s;
+	int ret;
+	u32 val;
+
+	dev_dbg(dev, "probing node %pOF\n", mux_node);
+
+	s = devm_kzalloc(dev, sizeof(*s), GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	s->regmap = dev_get_regmap(dev, NULL);
+	if (IS_ERR(s->regmap)) {
+		dev_err(dev, "Failed to get parent regmap\n");
+		return PTR_ERR(s->regmap);
+	}
+
+	ret = of_property_read_u32(mux_node, "reg", &s->mux_reg);
+	if (ret) {
+		dev_err(dev, "missing or invalid reg property\n");
+		return -ENODEV;
+	}
+
+	/* Test Register read write */
+	ret = regmap_read(s->regmap, s->mux_reg, &val);
+	if (ret) {
+		dev_err(dev, "error while reading reg\n");
+		return ret;
+	}
+
+	ret = regmap_write(s->regmap, s->mux_reg, val);
+	if (ret) {
+		dev_err(dev, "error while writing reg\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(mux_node, "mux-mask", &s->mask);
+	if (ret) {
+		dev_err(dev, "missing or invalid mux-mask property\n");
+		return -ENODEV;
+	}
+
+	/* Verify that the 'reg' property of each child MDIO bus does not
+	 * set any bits outside of the 'mask'.
+	 */
+	for_each_available_child_of_node(mux_node, child) {
+		ret = of_property_read_u32(child, "reg", &val);
+		if (ret) {
+			dev_err(dev, "%pOF is missing a 'reg' property\n",
+				child);
+			of_node_put(child);
+			return -ENODEV;
+		}
+		if (val & ~s->mask) {
+			dev_err(dev,
+				"%pOF has a 'reg' value with unmasked bits\n",
+				child);
+			of_node_put(child);
+			return -ENODEV;
+		}
+	}
+
+	ret = mdio_mux_init(dev, mux_node, mdio_mux_regmap_switch_fn,
+			    &s->mux_handle, s, NULL);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to register mdio-mux bus %pOF\n",
+				mux_node);
+		return ret;
+	}
+
+	*data = s;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mdio_mux_regmap_init);
+
+/**
+ * mdio_mux_regmap_uninit - relinquish the control of MDIO bus muxing using
+ *			    regmap constructs.
+ * @data: address of data allocated by mdio_mux_regmap_init
+ */
+int mdio_mux_regmap_uninit(void *data)
+{
+	struct mdio_mux_regmap_state *s = data;
+
+	mdio_mux_uninit(s->mux_handle);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mdio_mux_regmap_uninit);
+
+MODULE_AUTHOR("Pankaj Bansal <pankaj.bansal@nxp.com>");
+MODULE_DESCRIPTION("regmap based MDIO MUX driver");
+MODULE_LICENSE("GPL");
+
diff --git a/include/linux/mdio-mux.h b/include/linux/mdio-mux.h
index a5d58f221939..fb715d56999b 100644
--- a/include/linux/mdio-mux.h
+++ b/include/linux/mdio-mux.h
@@ -29,4 +29,38 @@ int mdio_mux_init(struct device *dev,
 
 void mdio_mux_uninit(void *mux_handle);
 
+#ifdef CONFIG_MDIO_BUS_MUX_REGMAP
+/**
+ * mdio_mux_regmap_init - control MDIO bus muxing using regmap constructs.
+ * @dev: device with which regmap construct is associated.
+ * @mux_node: mdio bus mux node that contains parent mdio bus phandle.
+ *	      This node also contains sub nodes, where each subnode denotes
+ *	      a child mdio bus. All the child mdio buses are muxed, i.e. at a
+ *	      time only one of the child mdio buses can be used.
+ * @data: to store the address of data allocated by this function
+ */
+int mdio_mux_regmap_init(struct device *dev,
+			 struct device_node *mux_node,
+			 void **data);
+
+/**
+ * mdio_mux_regmap_uninit - relinquish the control of MDIO bus muxing using
+ *			    regmap constructs.
+ * @data: address of data allocated by mdio_mux_regmap_init
+ */
+int mdio_mux_regmap_uninit(void *data);
+#else /* CONFIG_MDIO_BUS_MUX_REGMAP */
+static inline int mdio_mux_regmap_init(struct device *dev,
+				       struct device_node *mux_node,
+				       void **data)
+{
+	return -ENODEV;
+}
+
+static inline int mdio_mux_regmap_uninit(void *data)
+{
+	return 0;
+}
+#endif /* CONFIG_MDIO_BUS_MUX_REGMAP */
+
 #endif /* __LINUX_MDIO_MUX_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap
  2019-02-04  8:59 ` [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap Pankaj Bansal
@ 2019-02-04 13:46   ` Andrew Lunn
  2019-02-04 14:51   ` kbuild test robot
  1 sibling, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2019-02-04 13:46 UTC (permalink / raw)
  To: Pankaj Bansal; +Cc: Florian Fainelli, netdev

On Mon, Feb 04, 2019 at 08:59:50AM +0000, Pankaj Bansal wrote:
> Add support for an MDIO bus multiplexer controlled by a regmap
> device, like an FPGA.

Hi Pankaj

Thanks for adding the binding documentation.

> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 5805c0b7d60e..0827a700eb31 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_MDIO_BCM_IPROC)	+= mdio-bcm-iproc.o
>  obj-$(CONFIG_MDIO_BCM_UNIMAC)	+= mdio-bcm-unimac.o
>  obj-$(CONFIG_MDIO_BITBANG)	+= mdio-bitbang.o
>  obj-$(CONFIG_MDIO_BUS_MUX)	+= mdio-mux.o
> +obj-$(CONFIG_MDIO_BUS_MUX_REGMAP)	+= mdio-mux-regmap.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

We try to keep the Makefile sorting in alphabetical order.


> +/**
> + * mdio_mux_regmap_uninit - relinquish the control of MDIO bus muxing using
> + *			    regmap constructs.
> + * @data: address of data allocated by mdio_mux_regmap_init
> + */
> +int mdio_mux_regmap_uninit(void *data)
> +{
> +	struct mdio_mux_regmap_state *s = data;
> +
> +	mdio_mux_uninit(s->mux_handle);
> +
> +	return 0;
> +}

Please make this a void function, since there is nothing to return.

       Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap
  2019-02-04  8:59 ` [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap Pankaj Bansal
  2019-02-04 13:46   ` Andrew Lunn
@ 2019-02-04 14:51   ` kbuild test robot
  2019-02-05  3:52     ` Pankaj Bansal
  1 sibling, 1 reply; 9+ messages in thread
From: kbuild test robot @ 2019-02-04 14:51 UTC (permalink / raw)
  To: Pankaj Bansal
  Cc: kbuild-all, Andrew Lunn, Florian Fainelli, netdev, Pankaj Bansal

[-- Attachment #1: Type: text/plain, Size: 5241 bytes --]

Hi Pankaj,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on v5.0-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Pankaj-Bansal/add-MDIO-bus-multiplexer-driven-by-a-regmap-device/20190204-213429
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=sh 

All errors (new ones prefixed by >>):

>> drivers/net/phy/mdio-mux-regmap.c:72:5: error: redefinition of 'mdio_mux_regmap_init'
    int mdio_mux_regmap_init(struct device *dev,
        ^~~~~~~~~~~~~~~~~~~~
   In file included from drivers/net/phy/mdio-mux-regmap.c:18:
   include/linux/mdio-mux.h:53:19: note: previous definition of 'mdio_mux_regmap_init' was here
    static inline int mdio_mux_regmap_init(struct device *dev,
                      ^~~~~~~~~~~~~~~~~~~~
>> drivers/net/phy/mdio-mux-regmap.c:158:5: error: redefinition of 'mdio_mux_regmap_uninit'
    int mdio_mux_regmap_uninit(void *data)
        ^~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/net/phy/mdio-mux-regmap.c:18:
   include/linux/mdio-mux.h:60:19: note: previous definition of 'mdio_mux_regmap_uninit' was here
    static inline int mdio_mux_regmap_uninit(void *data)
                      ^~~~~~~~~~~~~~~~~~~~~~

vim +/mdio_mux_regmap_init +72 drivers/net/phy/mdio-mux-regmap.c

    62	
    63	/**
    64	 * mdio_mux_regmap_init - control MDIO bus muxing using regmap constructs.
    65	 * @dev: device with which regmap construct is associated.
    66	 * @mux_node: mdio bus mux node that contains parent mdio bus phandle.
    67	 *	      This node also contains sub nodes, where each subnode denotes
    68	 *	      a child mdio bus. All the child mdio buses are muxed, i.e. at a
    69	 *	      time only one of the child mdio buses can be used.
    70	 * @data: to store the address of data allocated by this function
    71	 */
  > 72	int mdio_mux_regmap_init(struct device *dev,
    73				 struct device_node *mux_node,
    74				 void **data)
    75	{
    76		struct device_node *child;
    77		struct mdio_mux_regmap_state *s;
    78		int ret;
    79		u32 val;
    80	
    81		dev_dbg(dev, "probing node %pOF\n", mux_node);
    82	
    83		s = devm_kzalloc(dev, sizeof(*s), GFP_KERNEL);
    84		if (!s)
    85			return -ENOMEM;
    86	
    87		s->regmap = dev_get_regmap(dev, NULL);
    88		if (IS_ERR(s->regmap)) {
    89			dev_err(dev, "Failed to get parent regmap\n");
    90			return PTR_ERR(s->regmap);
    91		}
    92	
    93		ret = of_property_read_u32(mux_node, "reg", &s->mux_reg);
    94		if (ret) {
    95			dev_err(dev, "missing or invalid reg property\n");
    96			return -ENODEV;
    97		}
    98	
    99		/* Test Register read write */
   100		ret = regmap_read(s->regmap, s->mux_reg, &val);
   101		if (ret) {
   102			dev_err(dev, "error while reading reg\n");
   103			return ret;
   104		}
   105	
   106		ret = regmap_write(s->regmap, s->mux_reg, val);
   107		if (ret) {
   108			dev_err(dev, "error while writing reg\n");
   109			return ret;
   110		}
   111	
   112		ret = of_property_read_u32(mux_node, "mux-mask", &s->mask);
   113		if (ret) {
   114			dev_err(dev, "missing or invalid mux-mask property\n");
   115			return -ENODEV;
   116		}
   117	
   118		/* Verify that the 'reg' property of each child MDIO bus does not
   119		 * set any bits outside of the 'mask'.
   120		 */
   121		for_each_available_child_of_node(mux_node, child) {
   122			ret = of_property_read_u32(child, "reg", &val);
   123			if (ret) {
   124				dev_err(dev, "%pOF is missing a 'reg' property\n",
   125					child);
   126				of_node_put(child);
   127				return -ENODEV;
   128			}
   129			if (val & ~s->mask) {
   130				dev_err(dev,
   131					"%pOF has a 'reg' value with unmasked bits\n",
   132					child);
   133				of_node_put(child);
   134				return -ENODEV;
   135			}
   136		}
   137	
   138		ret = mdio_mux_init(dev, mux_node, mdio_mux_regmap_switch_fn,
   139				    &s->mux_handle, s, NULL);
   140		if (ret) {
   141			if (ret != -EPROBE_DEFER)
   142				dev_err(dev, "failed to register mdio-mux bus %pOF\n",
   143					mux_node);
   144			return ret;
   145		}
   146	
   147		*data = s;
   148	
   149		return 0;
   150	}
   151	EXPORT_SYMBOL_GPL(mdio_mux_regmap_init);
   152	
   153	/**
   154	 * mdio_mux_regmap_uninit - relinquish the control of MDIO bus muxing using
   155	 *			    regmap constructs.
   156	 * @data: address of data allocated by mdio_mux_regmap_init
   157	 */
 > 158	int mdio_mux_regmap_uninit(void *data)
   159	{
   160		struct mdio_mux_regmap_state *s = data;
   161	
   162		mdio_mux_uninit(s->mux_handle);
   163	
   164		return 0;
   165	}
   166	EXPORT_SYMBOL_GPL(mdio_mux_regmap_uninit);
   167	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50803 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap
  2019-02-04 14:51   ` kbuild test robot
@ 2019-02-05  3:52     ` Pankaj Bansal
  0 siblings, 0 replies; 9+ messages in thread
From: Pankaj Bansal @ 2019-02-05  3:52 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev

Hi Andrew,

I am getting this compilation error when the mdio-mux-regmap.ko is built as standalone module.
This error doesn't come when the mdio-mux-regmap is built as part of linux kernel.

I don't understand the reason for it. Inline definitions of functions are only defined if CONFIG_MDIO_BUS_MUX_REGMAP
Is NOT defined. 

BUT it is defined as " CONFIG_MDIO_BUS_MUX_REGMAP = m"

Can you please help me to solve this?

Regards,
Pankaj Bansal

> -----Original Message-----
> From: kbuild test robot [mailto:lkp@intel.com]
> Sent: Monday, 4 February, 2019 08:22 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>
> Cc: kbuild-all@01.org; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
> <f.fainelli@gmail.com>; netdev@vger.kernel.org; Pankaj Bansal
> <pankaj.bansal@nxp.com>
> Subject: Re: [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a
> regmap
> 
> Hi Pankaj,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on net/master]
> [also build test ERROR on v5.0-rc4]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system]
> 
> url:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2F0day-ci%2Flinux%2Fcommits%2FPankaj-Bansal%2Fadd-MDIO-bus-
> multiplexer-driven-by-a-regmap-device%2F20190204-
> 213429&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7Caabb2bdecd8a4
> 2c19f8108d68ab053d6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C636848890628820621&amp;sdata=WLP4yuLXqfoNLfrPSIWhNkfg24YdR6Ny8jb
> cUgvcrDQ%3D&amp;reserved=0
> config: sh-allmodconfig (attached as .config)
> compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
> reproduce:
>         wget
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.git
> hubusercontent.com%2Fintel%2Flkp-
> tests%2Fmaster%2Fsbin%2Fmake.cross&amp;data=02%7C01%7Cpankaj.bansal
> %40nxp.com%7Caabb2bdecd8a42c19f8108d68ab053d6%7C686ea1d3bc2b4c6f
> a92cd99c5c301635%7C0%7C0%7C636848890628820621&amp;sdata=pGqb5xq
> RsWH6fXd8NWLUKX86qQ0ZO8OOy8nj9rckqV8%3D&amp;reserved=0 -O
> ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=8.2.0 make.cross ARCH=sh
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/net/phy/mdio-mux-regmap.c:72:5: error: redefinition of
> 'mdio_mux_regmap_init'
>     int mdio_mux_regmap_init(struct device *dev,
>         ^~~~~~~~~~~~~~~~~~~~
>    In file included from drivers/net/phy/mdio-mux-regmap.c:18:
>    include/linux/mdio-mux.h:53:19: note: previous definition of
> 'mdio_mux_regmap_init' was here
>     static inline int mdio_mux_regmap_init(struct device *dev,
>                       ^~~~~~~~~~~~~~~~~~~~
> >> drivers/net/phy/mdio-mux-regmap.c:158:5: error: redefinition of
> 'mdio_mux_regmap_uninit'
>     int mdio_mux_regmap_uninit(void *data)
>         ^~~~~~~~~~~~~~~~~~~~~~
>    In file included from drivers/net/phy/mdio-mux-regmap.c:18:
>    include/linux/mdio-mux.h:60:19: note: previous definition of
> 'mdio_mux_regmap_uninit' was here
>     static inline int mdio_mux_regmap_uninit(void *data)
>                       ^~~~~~~~~~~~~~~~~~~~~~
> 
> vim +/mdio_mux_regmap_init +72 drivers/net/phy/mdio-mux-regmap.c
> 
>     62
>     63	/**
>     64	 * mdio_mux_regmap_init - control MDIO bus muxing using regmap
> constructs.
>     65	 * @dev: device with which regmap construct is associated.
>     66	 * @mux_node: mdio bus mux node that contains parent mdio bus
> phandle.
>     67	 *	      This node also contains sub nodes, where each subnode
> denotes
>     68	 *	      a child mdio bus. All the child mdio buses are muxed, i.e. at a
>     69	 *	      time only one of the child mdio buses can be used.
>     70	 * @data: to store the address of data allocated by this function
>     71	 */
>   > 72	int mdio_mux_regmap_init(struct device *dev,
>     73				 struct device_node *mux_node,
>     74				 void **data)
>     75	{
>     76		struct device_node *child;
>     77		struct mdio_mux_regmap_state *s;
>     78		int ret;
>     79		u32 val;
>     80
>     81		dev_dbg(dev, "probing node %pOF\n", mux_node);
>     82
>     83		s = devm_kzalloc(dev, sizeof(*s), GFP_KERNEL);
>     84		if (!s)
>     85			return -ENOMEM;
>     86
>     87		s->regmap = dev_get_regmap(dev, NULL);
>     88		if (IS_ERR(s->regmap)) {
>     89			dev_err(dev, "Failed to get parent regmap\n");
>     90			return PTR_ERR(s->regmap);
>     91		}
>     92
>     93		ret = of_property_read_u32(mux_node, "reg", &s->mux_reg);
>     94		if (ret) {
>     95			dev_err(dev, "missing or invalid reg property\n");
>     96			return -ENODEV;
>     97		}
>     98
>     99		/* Test Register read write */
>    100		ret = regmap_read(s->regmap, s->mux_reg, &val);
>    101		if (ret) {
>    102			dev_err(dev, "error while reading reg\n");
>    103			return ret;
>    104		}
>    105
>    106		ret = regmap_write(s->regmap, s->mux_reg, val);
>    107		if (ret) {
>    108			dev_err(dev, "error while writing reg\n");
>    109			return ret;
>    110		}
>    111
>    112		ret = of_property_read_u32(mux_node, "mux-mask", &s-
> >mask);
>    113		if (ret) {
>    114			dev_err(dev, "missing or invalid mux-mask property\n");
>    115			return -ENODEV;
>    116		}
>    117
>    118		/* Verify that the 'reg' property of each child MDIO bus does
> not
>    119		 * set any bits outside of the 'mask'.
>    120		 */
>    121		for_each_available_child_of_node(mux_node, child) {
>    122			ret = of_property_read_u32(child, "reg", &val);
>    123			if (ret) {
>    124				dev_err(dev, "%pOF is missing a 'reg'
> property\n",
>    125					child);
>    126				of_node_put(child);
>    127				return -ENODEV;
>    128			}
>    129			if (val & ~s->mask) {
>    130				dev_err(dev,
>    131					"%pOF has a 'reg' value with unmasked
> bits\n",
>    132					child);
>    133				of_node_put(child);
>    134				return -ENODEV;
>    135			}
>    136		}
>    137
>    138		ret = mdio_mux_init(dev, mux_node,
> mdio_mux_regmap_switch_fn,
>    139				    &s->mux_handle, s, NULL);
>    140		if (ret) {
>    141			if (ret != -EPROBE_DEFER)
>    142				dev_err(dev, "failed to register mdio-mux
> bus %pOF\n",
>    143					mux_node);
>    144			return ret;
>    145		}
>    146
>    147		*data = s;
>    148
>    149		return 0;
>    150	}
>    151	EXPORT_SYMBOL_GPL(mdio_mux_regmap_init);
>    152
>    153	/**
>    154	 * mdio_mux_regmap_uninit - relinquish the control of MDIO bus
> muxing using
>    155	 *			    regmap constructs.
>    156	 * @data: address of data allocated by mdio_mux_regmap_init
>    157	 */
>  > 158	int mdio_mux_regmap_uninit(void *data)
>    159	{
>    160		struct mdio_mux_regmap_state *s = data;
>    161
>    162		mdio_mux_uninit(s->mux_handle);
>    163
>    164		return 0;
>    165	}
>    166	EXPORT_SYMBOL_GPL(mdio_mux_regmap_uninit);
>    167
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01
> .org%2Fpipermail%2Fkbuild-
> all&amp;data=02%7C01%7Cpankaj.bansal%40nxp.com%7Caabb2bdecd8a42c19
> f8108d68ab053d6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636
> 848890628820621&amp;sdata=P7RrlnHYStMpoDpwIDMmlRiDTCemyEE3bxuUrq
> d0uxY%3D&amp;reserved=0                   Intel Corporation

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap
  2018-10-07 18:08     ` Florian Fainelli
@ 2018-10-18  4:35       ` Pankaj Bansal
  0 siblings, 0 replies; 9+ messages in thread
From: Pankaj Bansal @ 2018-10-18  4:35 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn; +Cc: netdev



> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Sunday, October 7, 2018 11:39 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>; Andrew Lunn <andrew@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.bansal@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.bansal@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap
  2018-10-07 18:24 ` [PATCH v2 0/2] add MDIO bus multiplexer driven by a regmap device Pankaj Bansal
@ 2018-10-07 18:24   ` Pankaj Bansal
  2018-10-07 18:08     ` Florian Fainelli
  0 siblings, 1 reply; 9+ messages in thread
From: Pankaj Bansal @ 2018-10-07 18:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev, Pankaj Bansal

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.bansal@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.bansal@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);
+	return ret;
+}
+
+static int mdio_mux_regmap_probe(struct platform_device *pdev)
+{
+	struct device_node *np2, *np = pdev->dev.of_node;
+	struct mdio_mux_regmap_state *s;
+	int ret;
+	u32 val;
+
+	dev_dbg(&pdev->dev, "probing node %pOF\n", np);
+
+	s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	s->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	if (IS_ERR(s->regmap)) {
+		dev_err(&pdev->dev, "Failed to get parent regmap\n");
+		return PTR_ERR(s->regmap);
+	}
+
+	ret = of_property_read_u32(np, "reg", &s->mux_reg);
+	if (ret) {
+		dev_err(&pdev->dev, "missing or invalid reg property\n");
+		return -ENODEV;
+	}
+
+	/* Test Register read write */
+	ret = regmap_read(s->regmap, s->mux_reg, &val);
+	if (ret) {
+		dev_err(&pdev->dev, "error while reading reg\n");
+		return ret;
+	}
+
+	ret = regmap_write(s->regmap, s->mux_reg, val);
+	if (ret) {
+		dev_err(&pdev->dev, "error while writing reg\n");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "mux-mask", &s->mask);
+	if (ret) {
+		dev_err(&pdev->dev, "missing or invalid mux-mask property\n");
+		return -ENODEV;
+	}
+
+	/* Verify that the 'reg' property of each child MDIO bus does not
+	 * set any bits outside of the 'mask'.
+	 */
+	for_each_available_child_of_node(np, np2) {
+		ret = of_property_read_u32(np2, "reg", &val);
+		if (ret) {
+			dev_err(&pdev->dev, "mdio-mux child node %pOF is missing a 'reg' property\n", np2);
+			of_node_put(np2);
+			return -ENODEV;
+		}
+		if (val & ~s->mask) {
+			dev_err(&pdev->dev, "mdio-mux child node %pOF has a 'reg' value with unmasked bits\n", np2);
+			of_node_put(np2);
+			return -ENODEV;
+		}
+	}
+
+	ret = mdio_mux_init(&pdev->dev, pdev->dev.of_node,
+			    mdio_mux_regmap_switch_fn,
+			    &s->mux_handle, s, NULL);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"failed to register mdio-mux bus %pOF\n", np);
+		return ret;
+	}
+
+	pdev->dev.platform_data = s;
+
+	return 0;
+}
+
+static int mdio_mux_regmap_remove(struct platform_device *pdev)
+{
+	struct mdio_mux_regmap_state *s = dev_get_platdata(&pdev->dev);
+
+	mdio_mux_uninit(s->mux_handle);
+
+	return 0;
+}
+
+static const struct of_device_id mdio_mux_regmap_match[] = {
+	{
+		.compatible = "mdio-mux-regmap",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mdio_mux_regmap_match);
+
+static struct platform_driver mdio_mux_regmap_driver = {
+	.driver = {
+		.name		= "mdio-mux-regmap",
+		.of_match_table = mdio_mux_regmap_match,
+	},
+	.probe		= mdio_mux_regmap_probe,
+	.remove		= mdio_mux_regmap_remove,
+};
+
+module_platform_driver(mdio_mux_regmap_driver);
+
+MODULE_AUTHOR("Pankaj Bansal <pankaj.bansal@nxp.com>");
+MODULE_DESCRIPTION("regmap device MDIO MUX driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap
  2018-10-07 18:24   ` [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap Pankaj Bansal
@ 2018-10-07 18:08     ` Florian Fainelli
  2018-10-18  4:35       ` Pankaj Bansal
  0 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2018-10-07 18:08 UTC (permalink / raw)
  To: Pankaj Bansal, Andrew Lunn; +Cc: netdev



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.bansal@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.bansal@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-02-05  3:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-04  8:59 [PATCH v2 0/2] add MDIO bus multiplexer driven by a regmap device Pankaj Bansal
2019-02-04  8:59 ` [PATCH v2 1/2] dt-bindings: net: " Pankaj Bansal
2019-02-04  8:59 ` [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap Pankaj Bansal
2019-02-04 13:46   ` Andrew Lunn
2019-02-04 14:51   ` kbuild test robot
2019-02-05  3:52     ` Pankaj Bansal
     [not found] <20181005082926.27845-1-pankaj.bansal@nxp.com>
2018-10-07 18:24 ` [PATCH v2 0/2] add MDIO bus multiplexer driven by a regmap device Pankaj Bansal
2018-10-07 18:24   ` [PATCH v2 2/2] netdev/phy: add MDIO bus multiplexer driven by a regmap Pankaj Bansal
2018-10-07 18:08     ` Florian Fainelli
2018-10-18  4:35       ` Pankaj Bansal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.