diff mbox

netdev/phy: add MDIO bus multiplexer driven by a memory-mapped FPGA

Message ID 1345671954-6398-1-git-send-email-timur@freescale.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Timur Tabi Aug. 22, 2012, 9:45 p.m. UTC
An FPGA controls which sub-bus is connected to the master MDIO bus.  The
FPGA must be memory-mapped and contain only 8-bit registers (which keeps
things simple).

Tested on a Freescale P5020DS board which uses the "PIXIS" FPGA attached
to the localbus.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 .../devicetree/bindings/net/mdio-mux-fpga.txt      |   74 ++++++++
 drivers/net/phy/Kconfig                            |   13 ++
 drivers/net/phy/Makefile                           |    1 +
 drivers/net/phy/mdio-mux-fpga.c                    |  186 ++++++++++++++++++++
 4 files changed, 274 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-fpga.txt
 create mode 100644 drivers/net/phy/mdio-mux-fpga.c

Comments

David Daney Aug. 22, 2012, 10:24 p.m. UTC | #1
On 08/22/2012 02:45 PM, Timur Tabi wrote:
> An FPGA controls which sub-bus is connected to the master MDIO bus.  The
> FPGA must be memory-mapped and contain only 8-bit registers (which keeps
> things simple).
>
> Tested on a Freescale P5020DS board which uses the "PIXIS" FPGA attached
> to the localbus.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>   .../devicetree/bindings/net/mdio-mux-fpga.txt      |   74 ++++++++
>   drivers/net/phy/Kconfig                            |   13 ++
>   drivers/net/phy/Makefile                           |    1 +
>   drivers/net/phy/mdio-mux-fpga.c                    |  186 ++++++++++++++++++++

I am fine with the general concept of the patch, so I am going to start 
a Bike Shedding session with it over the names of some of the things here.

I wonder if *fpga is really a good name for this.  It is a general 
purpose multiplexer with a memory mapped control register.  I would call 
it something like mdio-mux-mmioreg.


>   4 files changed, 274 insertions(+), 0 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-fpga.txt
>   create mode 100644 drivers/net/phy/mdio-mux-fpga.c
>
> diff --git a/Documentation/devicetree/bindings/net/mdio-mux-fpga.txt b/Documentation/devicetree/bindings/net/mdio-mux-fpga.txt
> new file mode 100644
> index 0000000..ef567c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mdio-mux-fpga.txt
> @@ -0,0 +1,74 @@
> +Properties for an MDIO bus multiplexer/switch controlled by an FPGA register.
> +
> +This is a special case of a MDIO bus multiplexer.  An FPGA register is used
> +to control which child bus is connected.
> +
> +Required properties in addition to the generic multiplexer properties:
> +
> +- compatible : string, must contain "mdio-mux-fpga"
> +
> +- mdio-mux-device : phandle, points to the FPGA (or similar) node.  This
> +	must be a memory-mapped device with 8-bit registers.

You shouldn't need this.  Just make the multiplexer a child of FPGA node 
to indicate where it lives.


> +
> +- mdio-mux-register : integer, contains the offset of the register that
> +	controls the bus multiplexer.

This should just be the normal "reg" properly


> +
> +- mdio-mux-mask : integer, contains an 8-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.
> +

"reg-mask" ??

Do you need a shift too?


David Daney
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Timur Tabi Aug. 22, 2012, 10:38 p.m. UTC | #2
David Daney wrote:

> I wonder if *fpga is really a good name for this.  It is a general 
> purpose multiplexer with a memory mapped control register.  I would call 
> it something like mdio-mux-mmioreg.

At one point, I thought of using mdio-mux-bitbang, but -mmioreg is better.
 Thanks.

>> +- mdio-mux-device : phandle, points to the FPGA (or similar) node.  This
>> +	must be a memory-mapped device with 8-bit registers.
> 
> You shouldn't need this.  Just make the multiplexer a child of FPGA node 
> to indicate where it lives.

The problem is that we don't normally consider the FPGA node to be a bus,
so its child nodes won't get probed.  That's why I have this:

compatible = "mdio-mux-fpga", "mdio-mux";
                               ^^^^^^^^

This allows me to have multiple mdio-mux parent nodes (which I do, since I
have multiple mdio bus muxes), and they all get registered and probed
properly because I also do this:

static const struct of_device_id of_device_ids[] __devinitconst = {
	{
		.compatible	= "simple-bus"
	},
	{
		.compatible	= "fsl,srio",
	},
...
	{
		.compatible	= "mdio-mux",
	},
	{}
};

The .compatible = "mdio-mux" is what causes all of the mdio-mux nodes to
be registered.  Therefore, it's simpler if all the mdio-mux nodes are root
nodes.

>> +
>> +- mdio-mux-register : integer, contains the offset of the register that
>> +	controls the bus multiplexer.
> 
> This should just be the normal "reg" properly

Ok.

>> +- mdio-mux-mask : integer, contains an 8-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.
>> +
> 
> "reg-mask" ??

Ok.

> 
> Do you need a shift too?

The 'reg' property of the mdio bus child nodes should take the shift into
account.  That's why, in the example, I have mask=0x6 and reg=0 or reg=2.
 There's even code in the driver to make sure that the 'reg' values are
constrained to the mask.
David Daney Aug. 22, 2012, 10:52 p.m. UTC | #3
On 08/22/2012 03:38 PM, Timur Tabi wrote:
> David Daney wrote:
>
>> I wonder if *fpga is really a good name for this.  It is a general
>> purpose multiplexer with a memory mapped control register.  I would call
>> it something like mdio-mux-mmioreg.
>
> At one point, I thought of using mdio-mux-bitbang, but -mmioreg is better.
>   Thanks.
>
>>> +- mdio-mux-device : phandle, points to the FPGA (or similar) node.  This
>>> +	must be a memory-mapped device with 8-bit registers.
>>
>> You shouldn't need this.  Just make the multiplexer a child of FPGA node
>> to indicate where it lives.
>
> The problem is that we don't normally consider the FPGA node to be a bus,
> so its child nodes won't get probed.  That's why I have this:
>

That would seem to be a mistake/error.

You should be able to arrive at any directly addressable register by 
walking down the tree to the children and applying any "ranges" 
properties at each node.  The OF infrastructure will take care of 
resolving all the addresses and you get rid of much of the code you 
added to duplicate its function.




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tabi Timur-B04825 Aug. 23, 2012, 12:06 a.m. UTC | #4
David Daney wrote:
>>
>> The problem is that we don't normally consider the FPGA node to be a bus,
>> so its child nodes won't get probed.  That's why I have this:
>>
>
> That would seem to be a mistake/error.

Maybe I'm not explaining myself well.  A node that has children will not 
automatically probe the children unless something in arch code registers 
the parent as a bus.  If you look in corenet_ds.c, you'll see what we do 
with array of_device_ids[].  We list all of the nodes that are buses.  The 
mdio-mux root-level nodes that I have today won't work unless I add 
.compatible = "mdio-mux" to of_device_ids[].

>
> You should be able to arrive at any directly addressable register by
> walking down the tree to the children and applying any "ranges" properties
> at each node.  The OF infrastructure will take care of resolving all the
> addresses and you get rid of much of the code you added to duplicate its
> function.

That only works for buses that are already registered.  I think if I add 
"simple-bus" to the compatible of the FPGA node, it will work.  I'll try 
that tomorrow.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/net/mdio-mux-fpga.txt b/Documentation/devicetree/bindings/net/mdio-mux-fpga.txt
new file mode 100644
index 0000000..ef567c6
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mdio-mux-fpga.txt
@@ -0,0 +1,74 @@ 
+Properties for an MDIO bus multiplexer/switch controlled by an FPGA register.
+
+This is a special case of a MDIO bus multiplexer.  An FPGA register is used
+to control which child bus is connected.
+
+Required properties in addition to the generic multiplexer properties:
+
+- compatible : string, must contain "mdio-mux-fpga"
+
+- mdio-mux-device : phandle, points to the FPGA (or similar) node.  This
+	must be a memory-mapped device with 8-bit registers.
+
+- mdio-mux-register : integer, contains the offset of the register that
+	controls the bus multiplexer.
+
+- mdio-mux-mask : integer, contains an 8-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:
+
+The FPGA node defines a memory-mapped FPGA with a register space of 0x30 bytes.
+For the "EMI2" MDIO bus, register 9 (BRDCFG1) controls the mux on that bus.
+A bitmask of 0x6 means that bits 1 and 2 (bit 0 is lsb) are the bits on
+BRDCFG1 that control the actual mux.
+
+	/* The FPGA node */
+	fpga: board-control@3,0 {
+		compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
+		reg = <3 0 0x30>;
+	};
+
+	/* The parent MDIO bus. */
+	xmdio0: mdio@f1000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "fsl,fman-xmdio";
+		reg = <0xf1000 0x1000>;
+		interrupts = <100 1 0 0>;
+	};
+
+	mdio-mux-emi2 {
+		compatible = "mdio-mux-fpga", "mdio-mux";
+		mdio-parent-bus = <&xmdio0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		mdio-mux-device = <&fpga>;
+		mdio-mux-register = <9>; // BRDCFG1
+		mdio-mux-mask = <0x6>; // EMI2
+
+		emi2_slot1: mdio@0 {	// Slot 1 XAUI (FM2)
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			phy_xgmii_slot1: ethernet-phy@0 {
+				compatible = "ethernet-phy-ieee802.3-c45";
+				reg = <4>;
+			};
+		};
+
+		emi2_slot2: mdio@2 {	// Slot 2 XAUI (FM1)
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			phy_xgmii_slot2: ethernet-phy@4 {
+				compatible = "ethernet-phy-ieee802.3-c45";
+				reg = <0>;
+			};
+		};
+	};
+
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3090dc6..c3fc957 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -159,6 +159,19 @@  config MDIO_BUS_MUX_GPIO
 	  several child MDIO busses to a parent bus.  Child bus
 	  selection is under the control of GPIO lines.
 
+config MDIO_BUS_MUX_FPGA
+	tristate "Support for FPGA-controlled MDIO bus multiplexers"
+	depends on OF_MDIO
+	select MDIO_BUS_MUX
+	help
+	  This module provides a driver for MDIO bus multiplexers that
+	  are controlled via a simple memory-mapped FPGA device.  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 8-bit registers are supported.
+
 endif # PHYLIB
 
 config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 6d2dc6c..3bf4d7a 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -28,3 +28,4 @@  obj-$(CONFIG_MICREL_KS8995MA)	+= spi_ks8995.o
 obj-$(CONFIG_AMD_PHY)		+= amd.o
 obj-$(CONFIG_MDIO_BUS_MUX)	+= mdio-mux.o
 obj-$(CONFIG_MDIO_BUS_MUX_GPIO)	+= mdio-mux-gpio.o
+obj-$(CONFIG_MDIO_BUS_MUX_FPGA) += mdio-mux-fpga.o
diff --git a/drivers/net/phy/mdio-mux-fpga.c b/drivers/net/phy/mdio-mux-fpga.c
new file mode 100644
index 0000000..7b4e69c
--- /dev/null
+++ b/drivers/net/phy/mdio-mux-fpga.c
@@ -0,0 +1,186 @@ 
+/*
+ * FPGA MDIO MUX driver
+ *
+ * This driver supports
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/of_mdio.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/phy.h>
+#include <linux/mdio-mux.h>
+
+struct mdio_mux_fpga_state {
+	void *mux_handle;
+	phys_addr_t phys;
+	unsigned int offset;
+	uint8_t 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_fpga_switch_fn(int current_child, int desired_child,
+				      void *data)
+{
+	struct mdio_mux_fpga_state *s = data;
+
+	if (current_child ^ desired_child) {
+		void *p = ioremap(s->phys + s->offset, 1);
+		uint8_t x;
+
+		if (!p)
+			return -ENOMEM;
+
+		x = ioread8(p);
+		iowrite8((x & ~s->mask) | desired_child, p);
+
+		iounmap(p);
+	}
+
+	return 0;
+}
+
+static int __devinit mdio_mux_fpga_probe(struct platform_device *pdev)
+{
+	struct device_node *np2, *np = pdev->dev.of_node;
+	struct mdio_mux_fpga_state *s;
+	struct resource res;
+	const __be32 *iprop;
+	int len, ret;
+
+	dev_dbg(&pdev->dev, "probing node %s\n", np->full_name);
+
+	s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
+	if (!s)
+		return -ENOMEM;
+
+	iprop = of_get_property(np, "mdio-mux-device", &len);
+	if (!iprop || len != sizeof(phandle)) {
+		dev_err(&pdev->dev, "missing mdio-mux-device property\n");
+		return -ENODEV;
+	}
+	np2 = of_find_node_by_phandle(be32_to_cpup(iprop));
+	if (!np2) {
+		dev_err(&pdev->dev, "mdio-mux-device points to invalid node\n");
+		return -ENODEV;
+	}
+
+	ret = of_address_to_resource(np2, 0, &res);
+	if (ret) {
+		dev_err(&pdev->dev, "cannot obtain memory map for node %s\n",
+			np2->full_name);
+		return ret;
+	}
+	s->phys = res.start;
+
+	iprop = of_get_property(np, "mdio-mux-register", &len);
+	if (!iprop || len != sizeof(uint32_t)) {
+		dev_err(&pdev->dev, "missing mdio-mux-register property\n");
+		return -EINVAL;
+	}
+	s->offset = be32_to_cpup(iprop);
+	if (s->offset >= resource_size(&res)) {
+		dev_err(&pdev->dev, "mdio-mux-register value %u is too large\n",
+			s->offset);
+		return -EINVAL;
+	}
+
+	iprop = of_get_property(np, "mdio-mux-mask", &len);
+	if (!iprop || len != sizeof(uint32_t)) {
+		dev_err(&pdev->dev, "missing mdio-mux-mask property\n");
+		return -ENODEV;
+	}
+	if (be32_to_cpup(iprop) > 255) {
+		dev_err(&pdev->dev, "only 8-bit registers are supported\n");
+		return -EINVAL;
+	}
+	s->mask = be32_to_cpup(iprop);
+
+	/*
+	 * 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) {
+		iprop = of_get_property(np2, "reg", &len);
+		if (!iprop || len != sizeof(uint32_t)) {
+			dev_err(&pdev->dev, "mdio-mux child node %s is "
+				"missing a 'reg' property\n", np2->full_name);
+			return -ENODEV;
+		}
+		if (be32_to_cpup(iprop) & ~s->mask) {
+			dev_err(&pdev->dev, "mdio-mux child node %s has "
+				"a 'reg' value with unmasked bits\n",
+				np2->full_name);
+			return -ENODEV;
+		}
+	}
+
+	ret = mdio_mux_init(&pdev->dev, mdio_mux_fpga_switch_fn,
+			    &s->mux_handle, s);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register mdio-mux bus %s\n",
+			np->full_name);
+		return ret;
+	}
+
+	pdev->dev.platform_data = s;
+
+	return 0;
+}
+
+static int __devexit mdio_mux_fpga_remove(struct platform_device *pdev)
+{
+	struct mdio_mux_fpga_state *s = dev_get_platdata(&pdev->dev);
+
+	mdio_mux_uninit(s->mux_handle);
+
+	return 0;
+}
+
+static struct of_device_id mdio_mux_fpga_match[] = {
+	{
+		.compatible = "mdio-mux-fpga",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mdio_mux_fpga_match);
+
+static struct platform_driver mdio_mux_fpga_driver = {
+	.driver = {
+		.name		= "mdio-mux-fpga",
+		.owner		= THIS_MODULE,
+		.of_match_table = mdio_mux_fpga_match,
+	},
+	.probe		= mdio_mux_fpga_probe,
+	.remove		= __devexit_p(mdio_mux_fpga_remove),
+};
+
+module_platform_driver(mdio_mux_fpga_driver);
+
+MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
+MODULE_DESCRIPTION("FPGA MDIO MUX driver");
+MODULE_LICENSE("GPL v2");