[PATCH v2 1/2] dt-bindings: net: Add bindings for mdio mux consumers

[PATCH v2 1/2] dt-bindings: net: Add bindings for mdio mux consumers

[Pankaj Bansal]

When we use the bindings defined in Documentation/devicetree/bindings/mux
to define mdio mux in producer and consumer terms, it results in two
devices. one is mux producer and other is mux consumer.

Add the bindings needed for Mdio mux consumer devices.

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    V2:
    - moved the bindings from mdio-mux.txt to new file mdio-mux-multiplexer.txt
    - changed compatible to mdio-mux-multiplexer

diff --git a/Documentation/devicetree/bindings/net/mdio-mux-multiplexer.txt b/Documentation/devicetree/bindings/net/mdio-mux-multiplexer.txt
new file mode 100644
index 000000000000..9d5384849929
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mdio-mux-multiplexer.txt
@@ -0,0 +1,83 @@
+Properties for an MDIO bus multiplexer consumer device
+
+This is a special case of MDIO mux  when MDIO mux is defined as a consumer
+of a mux producer device. The mux producer can be of any type like mmio mux
+producer, gpio mux producer or generic register based mux producer.
+
+Required properties in addition to the MDIO Bus multiplexer properties:
+
+- compatible : should be "mmio-mux-multiplexer"
+- mux-controls : mux controller node to use for operating the mux
+- mdio-parent-bus : phandle to the parent MDIO bus.
+
+each child node of mdio bus multiplexer consumer device represent a mdio
+bus.
+
+for more information please refer
+Documentation/devicetree/bindings/mux/mux-controller.txt
+and Documentation/devicetree/bindings/net/mdio-mux.txt
+
+Example:
+In below example the Mux producer and consumer are separate nodes.
+
+&i2c0 {
+	fpga@66 { // fpga connected to i2c
+		compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c",
+			     "simple-mfd";
+		reg = <0x66>;
+
+		mux: mux-controller { // Mux Producer
+			compatible = "reg-mux";
+			#mux-control-cells = <1>;
+			mux-reg-masks = <0x54 0xf8>, /* 0: reg 0x54, bits 7:3 */
+					<0x54 0x07>; /* 1: reg 0x54, bits 2:0 */
+		};
+	};
+};
+
+mdio-mux-1 { // Mux consumer
+	compatible = "mdio-mux-multiplexer";
+	mux-controls = <&mux 0>;
+	mdio-parent-bus = <&emdio1>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	mdio@0 {
+		reg = <0x0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+	mdio@8 {
+		reg = <0x8>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+	..
+	..
+};
+
+mdio-mux-2 { // Mux consumer
+	compatible = "mdio-mux-multiplexer";
+	mux-controls = <&mux 1>;
+	mdio-parent-bus = <&emdio2>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	mdio@0 {
+		reg = <0x0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+	mdio@1 {
+		reg = <0x1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+	..
+	..
+};
+
-- 
2.17.1

[PATCH v2 2/2] drivers: net: phy: mdio-mux: Add support for Generic Mux controls

[Pankaj Bansal]

Add support for Generic Mux controls, when Mdio mux node is a consumer
of mux produced by some other device.

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    V2:
    - Moved the changes from mdio-mux module to new module mdio-mux-multiplexer
    - defined new CONFIG
    - using new compatible "mdio-mux-multiplexer"

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3d187cd50eb0..7f66af446ec7 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -87,6 +87,18 @@ config MDIO_BUS_MUX_MMIOREG
 
 	  Currently, only 8/16/32 bits registers are supported.
 
+config MDIO_BUS_MUX_MULTIPLEXER
+	tristate "MDIO bus multiplexer using kernel multiplexer subsystem"
+	depends on OF
+	select MULTIPLEXER
+	select MDIO_BUS_MUX
+	help
+	  This module provides a driver for MDIO bus multiplexer
+	  that is controlled via the kernel multiplexer subsystem. The
+	  bus multiplexer connects one of several child MDIO busses to
+	  a parent bus.  Child bus selection is under the control of
+	  the kernel multiplexer subsystem.
+
 config MDIO_CAVIUM
 	tristate
 
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 5805c0b7d60e..766106237a90 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_MULTIPLEXER) += mdio-mux-multiplexer.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-multiplexer.c b/drivers/net/phy/mdio-mux-multiplexer.c
new file mode 100644
index 000000000000..22adbbb93aa6
--- /dev/null
+++ b/drivers/net/phy/mdio-mux-multiplexer.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* MDIO bus multiplexer using kernel multiplexer subsystem
+ *
+ * Copyright 2019 NXP
+ */
+
+#include <linux/platform_device.h>
+#include <linux/mdio-mux.h>
+#include <linux/module.h>
+#include <linux/mux/consumer.h>
+
+struct mdio_mux_multiplexer_state {
+	struct mux_control *muxc;
+	void *mux_handle;
+};
+
+/**
+ * mdio_mux_multiplexer_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.
+ * @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_multiplexer_switch_fn(int current_child, int desired_child,
+					  void *data)
+{
+	struct platform_device *pdev;
+	struct mdio_mux_multiplexer_state *s;
+	int ret = 0;
+
+	pdev = (struct platform_device *)data;
+	s = (struct mdio_mux_multiplexer_state *)platform_get_drvdata(pdev);
+	if (current_child ^ desired_child) {
+		if (current_child != -1)
+			ret = mux_control_deselect(s->muxc);
+		if (ret)
+			return ret;
+
+		ret =  mux_control_select(s->muxc, desired_child);
+		if (!ret)
+			dev_dbg(&pdev->dev, "%s %d -> %d\n", __func__,
+				current_child, desired_child);
+	}
+
+	return ret;
+}
+
+static int mdio_mux_multiplexer_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mdio_mux_multiplexer_state *s;
+	int ret = 0;
+
+	s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	s->muxc = devm_mux_control_get(dev, NULL);
+	if (IS_ERR(s->muxc)) {
+		ret = PTR_ERR(s->muxc);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Failed to get mux: %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, s);
+
+	ret = mdio_mux_init(&pdev->dev, pdev->dev.of_node,
+			    mdio_mux_multiplexer_switch_fn, &s->mux_handle,
+			    pdev, NULL);
+
+	return ret;
+}
+
+static int mdio_mux_multiplexer_remove(struct platform_device *pdev)
+{
+	struct mdio_mux_multiplexer_state *s = platform_get_drvdata(pdev);
+
+	mdio_mux_uninit(s->mux_handle);
+
+	return 0;
+}
+
+static const struct of_device_id mdio_mux_multiplexer_match[] = {
+	{ .compatible = "mdio-mux-multiplexer", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mdio_mux_multiplexer_match);
+
+static struct platform_driver mdio_mux_multiplexer_driver = {
+	.driver = {
+		.name		= "mdio-mux-multiplexer",
+		.of_match_table	= mdio_mux_multiplexer_match,
+	},
+	.probe		= mdio_mux_multiplexer_probe,
+	.remove		= mdio_mux_multiplexer_remove,
+};
+
+module_platform_driver(mdio_mux_multiplexer_driver);
+
+MODULE_DESCRIPTION("MDIO bus multiplexer using kernel multiplexer subsystem");
+MODULE_AUTHOR("Pankaj Bansal");
+MODULE_LICENSE("GPL");
-- 
2.17.1

Re: [PATCH v2 2/2] drivers: net: phy: mdio-mux: Add support for Generic Mux controls

[Andrew Lunn]

> +static int mdio_mux_multiplexer_switch_fn(int current_child, int desired_child,
> +					  void *data)
> +{
> +	struct platform_device *pdev;
> +	struct mdio_mux_multiplexer_state *s;
> +	int ret = 0;
> +
> +	pdev = (struct platform_device *)data;
> +	s = (struct mdio_mux_multiplexer_state *)platform_get_drvdata(pdev);

platform_get_drvdata() returns a void *. So you don't need the cast.

> +	if (current_child ^ desired_child) {
> +		if (current_child != -1)
> +			ret = mux_control_deselect(s->muxc);
> +		if (ret)
> +			return ret;

Please use {} here, so you check the error when current_child != -1.

> +
> +		ret =  mux_control_select(s->muxc, desired_child);
> +		if (!ret)
> +			dev_dbg(&pdev->dev, "%s %d -> %d\n", __func__,
> +				current_child, desired_child);
> +	}
> +
> +	return ret;
> +}
> +
> +static int mdio_mux_multiplexer_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mdio_mux_multiplexer_state *s;
> +	int ret = 0;
> +
> +	s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
> +	if (!s)
> +		return -ENOMEM;
> +
> +	s->muxc = devm_mux_control_get(dev, NULL);
> +	if (IS_ERR(s->muxc)) {
> +		ret = PTR_ERR(s->muxc);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Failed to get mux: %d\n", ret);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, s);
> +
> +	ret = mdio_mux_init(&pdev->dev, pdev->dev.of_node,
> +			    mdio_mux_multiplexer_switch_fn, &s->mux_handle,
> +			    pdev, NULL);
> +
> +	return ret;
> +}
> +
> +static int mdio_mux_multiplexer_remove(struct platform_device *pdev)
> +{
> +	struct mdio_mux_multiplexer_state *s = platform_get_drvdata(pdev);
> +
> +	mdio_mux_uninit(s->mux_handle);

I've never used the multiplexer framework before. But i think you need
a call to mux_control_deselect() here in order to unlock the
multiplexer. Without that, it will deadlock when you reload the
module.

	Andrew

Re: [PATCH v2 1/2] dt-bindings: net: Add bindings for mdio mux consumers

[Andrew Lunn]

On Sun, Feb 24, 2019 at 08:49:21AM +0000, Pankaj Bansal wrote:
> When we use the bindings defined in Documentation/devicetree/bindings/mux
> to define mdio mux in producer and consumer terms, it results in two
> devices. one is mux producer and other is mux consumer.
> 
> Add the bindings needed for Mdio mux consumer devices.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v2 2/2] drivers: net: phy: mdio-mux: Add support for Generic Mux controls

[Peter Rosin]

On 2019-02-24 15:57, Andrew Lunn wrote:
>> +static int mdio_mux_multiplexer_remove(struct platform_device *pdev)
>> +{
>> +	struct mdio_mux_multiplexer_state *s = platform_get_drvdata(pdev);
>> +
>> +	mdio_mux_uninit(s->mux_handle);
> 
> I've never used the multiplexer framework before. But i think you need
> a call to mux_control_deselect() here in order to unlock the
> multiplexer. Without that, it will deadlock when you reload the
> module.

That is correct.

Some background; The mux framework was designed for cases where

a) two (or more) mux consumers "compete" for the same mux controller, or
b) where a single mux chip shares several mux controllers where each
   controller might be used for some independent purpose.

The design is also built around short accesses, such that no single
mux consumer starves other consumers of a shared mux controller. I.e.

- select the mux and implicitly set it in the desired position
- do something short-ish which requires the mux
- deselect the mux

But it does work for exclusive consumers of a mux controller as in this
series. However, if the consumer

1) requires the implementation of a new mux controller,
2) does not need any of the existing mux drivers and
3) the situation is not in one of the a) and b) categories

then the mux framework is not really providing any killer features. I.e.
it seems like it is certainly an option to just open code the needed
regmap accesses and ignore the multiplexer subsystem in this case.
However, the changes needed in the mux framework are quite minimal in
this case [1], so 1) is arguably not applicable...

More details on the design can be found in the original mux series [2].

Cheers,
Peter

[1] https://lkml.org/lkml/2019/2/24/23
[2] https://lkml.org/lkml/2017/5/14/160