[U-Boot] [PATCH 1/2] drivers: net: driver for MDIO muxes controlled over I2C
This driver is used for MDIO muxes driven over I2C. This is currently used on Freescale LS1028A QDS board, on which the physical MDIO MUX is controlled by an on-board FPGA which in turn is configured through I2C.
Signed-off-by: Alex Marginean alexm.osslist@gmail.com ---
Depends on https://patchwork.ozlabs.org/project/uboot/list/?series=119124
drivers/net/Kconfig | 8 +++ drivers/net/Makefile | 1 + drivers/net/mdio_mux_i2c_reg.c | 108 +++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+) create mode 100644 drivers/net/mdio_mux_i2c_reg.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 4d85fb1716..93535f7acb 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -595,4 +595,12 @@ config FSL_ENETC This driver supports the NXP ENETC Ethernet controller found on some of the NXP SoCs.
+config MDIO_MUX_I2C_REG + bool "MDIO MUX accessed as a register over I2C" + depends on DM_MDIO_MUX && DM_I2C + help + This driver is used for MDIO muxes driven by writing to a register of + an I2C chip. The board it was developed for uses a mux controlled by + on-board FPGA which in turn is accessed as a chip over I2C. + endif # NETDEVICES diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 97119cec7c..9470f02dc6 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -80,3 +80,4 @@ obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o obj-$(CONFIG_FSL_ENETC) += fsl_enetc.o fsl_enetc_mdio.o obj-$(CONFIG_MDIO_MUX_SANDBOX) += mdio_mux_sandbox.o +obj-$(CONFIG_MDIO_MUX_I2C_REG) += mdio_mux_i2c_reg.o diff --git a/drivers/net/mdio_mux_i2c_reg.c b/drivers/net/mdio_mux_i2c_reg.c new file mode 100644 index 0000000000..eb75c5e032 --- /dev/null +++ b/drivers/net/mdio_mux_i2c_reg.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * (C) Copyright 2019 + * Alex Marginean, NXP + */ + +#include <dm.h> +#include <errno.h> +#include <miiphy.h> +#include <i2c.h> + +/* + * This driver is used for MDIO muxes driven by writing to a register of an I2C + * chip. The board it was developed for uses a mux controlled by on-board FPGA + * which in turn is accessed as a chip over I2C. + */ + +struct mmux_i2c_reg_priv { + struct udevice *chip; + int reg; + int mask; +}; + +static int mmux_i2c_reg_select(struct udevice *mux, int cur, int sel) +{ + struct mmux_i2c_reg_priv *priv = dev_get_priv(mux); + u8 val, val_old; + + /* if last selection didn't change we're good to go */ + if (cur == sel) + return 0; + + val_old = dm_i2c_reg_read(priv->chip, priv->reg); + val = (val_old & ~priv->mask) | (sel & priv->mask); + debug("%s: chip %s, reg %x, val %x => %x\n", __func__, priv->chip->name, + priv->reg, val_old, val); + dm_i2c_reg_write(priv->chip, priv->reg, val); + + return 0; +} + +static const struct mdio_mux_ops mmux_i2c_reg_ops = { + .select = mmux_i2c_reg_select, +}; + +static int mmux_i2c_reg_probe(struct udevice *dev) +{ + struct mmux_i2c_reg_priv *priv = dev_get_priv(dev); + ofnode chip_node, bus_node; + struct udevice *i2c_bus; + u32 reg_mask[2]; + u32 chip_addr; + int err; + + /* read the register addr/mask pair */ + err = dev_read_u32_array(dev, "mux-reg-masks", reg_mask, 2); + if (err) { + debug("%s: error reading mux-reg-masks property\n", __func__); + return err; + } + + /* parent should be an I2C chip, grandparent should be an I2C bus */ + chip_node = ofnode_get_parent(dev->node); + bus_node = ofnode_get_parent(chip_node); + + err = uclass_get_device_by_ofnode(UCLASS_I2C, bus_node, &i2c_bus); + if (err) { + debug("%s: can't find I2C bus for node %s\n", __func__, + ofnode_get_name(bus_node)); + return err; + } + + err = ofnode_read_u32(chip_node, "reg", &chip_addr); + if (err) { + debug("%s: can't read chip address in %s\n", __func__, + ofnode_get_name(chip_node)); + return err; + } + + err = i2c_get_chip(i2c_bus, (uint)chip_addr, 1, &priv->chip); + if (err) { + debug("%s: can't find i2c chip device for addr %x\n", __func__, + chip_addr); + return err; + } + + priv->reg = (int)reg_mask[0]; + priv->mask = (int)reg_mask[1]; + + debug("%s: chip %s, reg %x, mask %x\n", __func__, priv->chip->name, + priv->reg, priv->mask); + + return 0; +} + +static const struct udevice_id mmux_i2c_reg_ids[] = { + { .compatible = "mdio-mux-i2c-reg" }, + { } +}; + +U_BOOT_DRIVER(mmux_i2c_reg) = { + .name = "mdio_mux_i2c_reg", + .id = UCLASS_MDIO_MUX, + .of_match = mmux_i2c_reg_ids, + .probe = mmux_i2c_reg_probe, + .ops = &mmux_i2c_reg_ops, + .priv_auto_alloc_size = sizeof(struct mmux_i2c_reg_priv), +};
This binding documents two properties that describe the registers used to perform MUX selection.
Signed-off-by: Alex Marginean alexm.osslist@gmail.com --- doc/device-tree-bindings/net/mdio-mux-reg.txt | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 doc/device-tree-bindings/net/mdio-mux-reg.txt
diff --git a/doc/device-tree-bindings/net/mdio-mux-reg.txt b/doc/device-tree-bindings/net/mdio-mux-reg.txt new file mode 100644 index 0000000000..13cfe9117b --- /dev/null +++ b/doc/device-tree-bindings/net/mdio-mux-reg.txt @@ -0,0 +1,64 @@ +Device tree structures used by register based MDIO muxes is described here. +This binding is based on reg-mux.txt binding in Linux and is currently used by +mdio-mux-i2c-reg driver in U-Boot. + +Required properties: +#mux-control-cells = <1> indicates how many registers are used for mux + selection. mux-reg-mask property described below must + include this number of pairs. +mux-reg-masks = <reg mask> describes pairs of register offset and register mask. + Register bits enabled in mask are set to the selection + value defined in reg property of child MDIOs to control + selection. +Properties described in mdio-mux.txt also apply. + +Example structure, used on Freescale LS1028A QDS board: + +&i2c0 { + status = "okay"; + u-boot,dm-pre-reloc; + + fpga@66 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "simple-mfd"; + reg = <0x66>; + + mux-mdio@54 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "mdio-mux-i2c-reg"; + reg = <0x54>; + #mux-control-cells = <1>; + mux-reg-masks = <0x54 0xf0>; + mdio-parent-bus = <&mdio0>; + + /* on-board MDIO with a single RGMII PHY */ + mdio@00 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x00>; + + /* on-board 1G RGMII PHY */ + qds_phy0: phy@5 { + reg = <5>; + }; + }; + /* slot 1 */ + mdio@40 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x40>; + /* VSC8234 1G SGMII card */ + sgmii_port0: phy@1c { + reg = <0x1c>; + }; + }; + }; + }; +}; + +/* Parent MDIO, defined in SoC .dtsi file, just enabled here */ +&mdio0 { + status = "okay"; +};
Hi Alex,
On Fri, Jul 12, 2019 at 10:21 PM Alex Marginean alexandru.marginean@nxp.com wrote:
This driver is used for MDIO muxes driven over I2C. This is currently used on Freescale LS1028A QDS board, on which the physical MDIO MUX is controlled by an on-board FPGA which in turn is configured through I2C.
Signed-off-by: Alex Marginean alexm.osslist@gmail.com
Depends on https://patchwork.ozlabs.org/project/uboot/list/?series=119124
drivers/net/Kconfig | 8 +++ drivers/net/Makefile | 1 + drivers/net/mdio_mux_i2c_reg.c | 108 +++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+) create mode 100644 drivers/net/mdio_mux_i2c_reg.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 4d85fb1716..93535f7acb 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -595,4 +595,12 @@ config FSL_ENETC This driver supports the NXP ENETC Ethernet controller found on some of the NXP SoCs.
+config MDIO_MUX_I2C_REG
I don't see current Linux upstream has any I2CREG support. I believe you will upstream the Linux support too?
So based on existing Linux kernel MMIOREG support, (see mdio-mux-mmioreg.txt in the Linux devicetree bindings direoctory), looks we need name this as MDIO_MUX_I2CREG for consistency.
bool "MDIO MUX accessed as a register over I2C"depends on DM_MDIO_MUX && DM_I2ChelpThis driver is used for MDIO muxes driven by writing to a register ofan I2C chip. The board it was developed for uses a mux controlled byon-board FPGA which in turn is accessed as a chip over I2C.endif # NETDEVICES diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 97119cec7c..9470f02dc6 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -80,3 +80,4 @@ obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o obj-$(CONFIG_FSL_ENETC) += fsl_enetc.o fsl_enetc_mdio.o obj-$(CONFIG_MDIO_MUX_SANDBOX) += mdio_mux_sandbox.o +obj-$(CONFIG_MDIO_MUX_I2C_REG) += mdio_mux_i2c_reg.o diff --git a/drivers/net/mdio_mux_i2c_reg.c b/drivers/net/mdio_mux_i2c_reg.c new file mode 100644 index 0000000000..eb75c5e032 --- /dev/null +++ b/drivers/net/mdio_mux_i2c_reg.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- (C) Copyright 2019
- Alex Marginean, NXP
- */
+#include <dm.h> +#include <errno.h> +#include <miiphy.h> +#include <i2c.h>
+/*
- This driver is used for MDIO muxes driven by writing to a register of an I2C
- chip. The board it was developed for uses a mux controlled by on-board FPGA
- which in turn is accessed as a chip over I2C.
- */
+struct mmux_i2c_reg_priv {
struct udevice *chip;int reg;int mask;+};
+static int mmux_i2c_reg_select(struct udevice *mux, int cur, int sel) +{
struct mmux_i2c_reg_priv *priv = dev_get_priv(mux);u8 val, val_old;/* if last selection didn't change we're good to go */if (cur == sel)return 0;val_old = dm_i2c_reg_read(priv->chip, priv->reg);val = (val_old & ~priv->mask) | (sel & priv->mask);debug("%s: chip %s, reg %x, val %x => %x\n", __func__, priv->chip->name,priv->reg, val_old, val);dm_i2c_reg_write(priv->chip, priv->reg, val);return 0;+}
+static const struct mdio_mux_ops mmux_i2c_reg_ops = {
.select = mmux_i2c_reg_select,+};
+static int mmux_i2c_reg_probe(struct udevice *dev) +{
struct mmux_i2c_reg_priv *priv = dev_get_priv(dev);ofnode chip_node, bus_node;struct udevice *i2c_bus;u32 reg_mask[2];u32 chip_addr;int err;/* read the register addr/mask pair */err = dev_read_u32_array(dev, "mux-reg-masks", reg_mask, 2);if (err) {debug("%s: error reading mux-reg-masks property\n", __func__);return err;}/* parent should be an I2C chip, grandparent should be an I2C bus */chip_node = ofnode_get_parent(dev->node);bus_node = ofnode_get_parent(chip_node);err = uclass_get_device_by_ofnode(UCLASS_I2C, bus_node, &i2c_bus);if (err) {debug("%s: can't find I2C bus for node %s\n", __func__,ofnode_get_name(bus_node));return err;}err = ofnode_read_u32(chip_node, "reg", &chip_addr);if (err) {debug("%s: can't read chip address in %s\n", __func__,ofnode_get_name(chip_node));return err;}err = i2c_get_chip(i2c_bus, (uint)chip_addr, 1, &priv->chip);if (err) {debug("%s: can't find i2c chip device for addr %x\n", __func__,chip_addr);return err;}priv->reg = (int)reg_mask[0];priv->mask = (int)reg_mask[1];debug("%s: chip %s, reg %x, mask %x\n", __func__, priv->chip->name,priv->reg, priv->mask);return 0;+}
+static const struct udevice_id mmux_i2c_reg_ids[] = {
{ .compatible = "mdio-mux-i2c-reg" },
We should use the exact same compatible string as Linux, although Linux upstream does not have such support for now, but I believe it should be "mdio-mux-i2creg" for consistency with "mdio-mux-mmioreg".
{ }+};
+U_BOOT_DRIVER(mmux_i2c_reg) = {
.name = "mdio_mux_i2c_reg",.id = UCLASS_MDIO_MUX,.of_match = mmux_i2c_reg_ids,.probe = mmux_i2c_reg_probe,.ops = &mmux_i2c_reg_ops,.priv_auto_alloc_size = sizeof(struct mmux_i2c_reg_priv),+};
nits: please replace all mdio_mux_i2c_reg_xxx to mdio_mux_i2creg_xxx
Regards, Bin
On 7/13/2019 7:02 AM, Bin Meng wrote:
Hi Alex,
On Fri, Jul 12, 2019 at 10:21 PM Alex Marginean alexandru.marginean@nxp.com wrote:
This driver is used for MDIO muxes driven over I2C. This is currently used on Freescale LS1028A QDS board, on which the physical MDIO MUX is controlled by an on-board FPGA which in turn is configured through I2C.
Signed-off-by: Alex Marginean alexm.osslist@gmail.com
Depends on https://patchwork.ozlabs.org/project/uboot/list/?series=119124
drivers/net/Kconfig | 8 +++ drivers/net/Makefile | 1 + drivers/net/mdio_mux_i2c_reg.c | 108 +++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+) create mode 100644 drivers/net/mdio_mux_i2c_reg.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 4d85fb1716..93535f7acb 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -595,4 +595,12 @@ config FSL_ENETC This driver supports the NXP ENETC Ethernet controller found on some of the NXP SoCs.
+config MDIO_MUX_I2C_REG
I don't see current Linux upstream has any I2CREG support. I believe you will upstream the Linux support too?
Linux support is somewhat different so we won't have the same driver and binding there. The main difference is that Linux has the concept of a mux controller, independent of the type of bus it muxes. I didn't add this to U-Boot, not yet at least. This specific MDIO mux would be driven in Linux by a pair of devices: - a mux controller with bindings defined by reg-mux.txt [1], this one implements the select function, - MDIO mux as a client of the controller, defined by mdio-mux-multiplexer.txt [2], which deals with the MDIO specific side of things.
In U-Boot I picked up the relevant properties from the two bindings, the MDIO mux device in U-Boot is supposed to implement select like a generic Linux MUX, while the uclass code deals with MDIO. I noticed U-Boot already has an I2C MUX class, along the lines of the MDIO one, I'm not sure if it's useful enough to introduce a new class of MUX controllers and then make all other kind of muxes depend on it. For now at least the U-Boot mux drivers are simple enough and the extra class wouldn't help much.
The other difference is that in Linux the mux controller is driven by the 'MMIO register bitfield-controlled multiplexer driver' [3], which is not I2C specific. It is built on top of regmap which I also think is a bit too much for U-Boot. For the U-Boot driver I just called I2C API directly and in that context it made sense to add I2C to the driver name, but there won't be an equivalent I2C based driver for Linux.
I'd really appreciate some feedback on this. I think the fairly simple approach in U-Boot is good enough, but I'm open to suggestions.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu... [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu... [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
So based on existing Linux kernel MMIOREG support, (see mdio-mux-mmioreg.txt in the Linux devicetree bindings direoctory), looks we need name this as MDIO_MUX_I2CREG for consistency.
bool "MDIO MUX accessed as a register over I2C"depends on DM_MDIO_MUX && DM_I2ChelpThis driver is used for MDIO muxes driven by writing to a register ofan I2C chip. The board it was developed for uses a mux controlled byon-board FPGA which in turn is accessed as a chip over I2C.- endif # NETDEVICES
diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 97119cec7c..9470f02dc6 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -80,3 +80,4 @@ obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o obj-$(CONFIG_MDIO_SANDBOX) += mdio_sandbox.o obj-$(CONFIG_FSL_ENETC) += fsl_enetc.o fsl_enetc_mdio.o obj-$(CONFIG_MDIO_MUX_SANDBOX) += mdio_mux_sandbox.o +obj-$(CONFIG_MDIO_MUX_I2C_REG) += mdio_mux_i2c_reg.o diff --git a/drivers/net/mdio_mux_i2c_reg.c b/drivers/net/mdio_mux_i2c_reg.c new file mode 100644 index 0000000000..eb75c5e032 --- /dev/null +++ b/drivers/net/mdio_mux_i2c_reg.c @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- (C) Copyright 2019
- Alex Marginean, NXP
- */
+#include <dm.h> +#include <errno.h> +#include <miiphy.h> +#include <i2c.h>
+/*
- This driver is used for MDIO muxes driven by writing to a register of an I2C
- chip. The board it was developed for uses a mux controlled by on-board FPGA
- which in turn is accessed as a chip over I2C.
- */
+struct mmux_i2c_reg_priv {
struct udevice *chip;int reg;int mask;+};
+static int mmux_i2c_reg_select(struct udevice *mux, int cur, int sel) +{
struct mmux_i2c_reg_priv *priv = dev_get_priv(mux);u8 val, val_old;/* if last selection didn't change we're good to go */if (cur == sel)return 0;val_old = dm_i2c_reg_read(priv->chip, priv->reg);val = (val_old & ~priv->mask) | (sel & priv->mask);debug("%s: chip %s, reg %x, val %x => %x\n", __func__, priv->chip->name,priv->reg, val_old, val);dm_i2c_reg_write(priv->chip, priv->reg, val);return 0;+}
+static const struct mdio_mux_ops mmux_i2c_reg_ops = {
.select = mmux_i2c_reg_select,+};
+static int mmux_i2c_reg_probe(struct udevice *dev) +{
struct mmux_i2c_reg_priv *priv = dev_get_priv(dev);ofnode chip_node, bus_node;struct udevice *i2c_bus;u32 reg_mask[2];u32 chip_addr;int err;/* read the register addr/mask pair */err = dev_read_u32_array(dev, "mux-reg-masks", reg_mask, 2);if (err) {debug("%s: error reading mux-reg-masks property\n", __func__);return err;}/* parent should be an I2C chip, grandparent should be an I2C bus */chip_node = ofnode_get_parent(dev->node);bus_node = ofnode_get_parent(chip_node);err = uclass_get_device_by_ofnode(UCLASS_I2C, bus_node, &i2c_bus);if (err) {debug("%s: can't find I2C bus for node %s\n", __func__,ofnode_get_name(bus_node));return err;}err = ofnode_read_u32(chip_node, "reg", &chip_addr);if (err) {debug("%s: can't read chip address in %s\n", __func__,ofnode_get_name(chip_node));return err;}err = i2c_get_chip(i2c_bus, (uint)chip_addr, 1, &priv->chip);if (err) {debug("%s: can't find i2c chip device for addr %x\n", __func__,chip_addr);return err;}priv->reg = (int)reg_mask[0];priv->mask = (int)reg_mask[1];debug("%s: chip %s, reg %x, mask %x\n", __func__, priv->chip->name,priv->reg, priv->mask);return 0;+}
+static const struct udevice_id mmux_i2c_reg_ids[] = {
{ .compatible = "mdio-mux-i2c-reg" },We should use the exact same compatible string as Linux, although Linux upstream does not have such support for now, but I believe it should be "mdio-mux-i2creg" for consistency with "mdio-mux-mmioreg".
{ }+};
+U_BOOT_DRIVER(mmux_i2c_reg) = {
.name = "mdio_mux_i2c_reg",.id = UCLASS_MDIO_MUX,.of_match = mmux_i2c_reg_ids,.probe = mmux_i2c_reg_probe,.ops = &mmux_i2c_reg_ops,.priv_auto_alloc_size = sizeof(struct mmux_i2c_reg_priv),+};
nits: please replace all mdio_mux_i2c_reg_xxx to mdio_mux_i2creg_xxx
OK, I can do that.
Thank you! Alex
Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
+Simon,
Hi Alex,
On Mon, Jul 15, 2019 at 7:45 PM Alexandru Marginean alexandru.marginean@nxp.com wrote:
On 7/13/2019 7:02 AM, Bin Meng wrote:
Hi Alex,
On Fri, Jul 12, 2019 at 10:21 PM Alex Marginean alexandru.marginean@nxp.com wrote:
This driver is used for MDIO muxes driven over I2C. This is currently used on Freescale LS1028A QDS board, on which the physical MDIO MUX is controlled by an on-board FPGA which in turn is configured through I2C.
Signed-off-by: Alex Marginean alexm.osslist@gmail.com
Depends on https://patchwork.ozlabs.org/project/uboot/list/?series=119124
drivers/net/Kconfig | 8 +++ drivers/net/Makefile | 1 + drivers/net/mdio_mux_i2c_reg.c | 108 +++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+) create mode 100644 drivers/net/mdio_mux_i2c_reg.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 4d85fb1716..93535f7acb 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -595,4 +595,12 @@ config FSL_ENETC This driver supports the NXP ENETC Ethernet controller found on some of the NXP SoCs.
+config MDIO_MUX_I2C_REG
I don't see current Linux upstream has any I2CREG support. I believe you will upstream the Linux support too?
Linux support is somewhat different so we won't have the same driver and binding there. The main difference is that Linux has the concept of a mux controller, independent of the type of bus it muxes. I didn't add this to U-Boot, not yet at least. This specific MDIO mux would be driven in Linux by a pair of devices:
- a mux controller with bindings defined by reg-mux.txt [1], this one
implements the select function,
- MDIO mux as a client of the controller, defined by
mdio-mux-multiplexer.txt [2], which deals with the MDIO specific side of things.
Thanks for the detailed explanations! I thought people wanted to have exact the same DT bindings as Linux so that we can sync-up upstream kernel DT changes to U-Boot. That's why at first I checked whether kernel has such bindings in place.
In U-Boot I picked up the relevant properties from the two bindings, the MDIO mux device in U-Boot is supposed to implement select like a generic Linux MUX, while the uclass code deals with MDIO. I noticed U-Boot already has an I2C MUX class, along the lines of the MDIO one, I'm not sure if it's useful enough to introduce a new class of MUX controllers and then make all other kind of muxes depend on it. For now at least the U-Boot mux drivers are simple enough and the extra class wouldn't help much.
The other difference is that in Linux the mux controller is driven by the 'MMIO register bitfield-controlled multiplexer driver' [3], which is not I2C specific. It is built on top of regmap which I also think is a bit too much for U-Boot. For the U-Boot driver I just called I2C API directly and in that context it made sense to add I2C to the driver name, but there won't be an equivalent I2C based driver for Linux.
For your current implementation I believe that's OK, at least it is verified on LS1028. But there must be a reason that Linux kernel chose a DT that is different. I suspect the usage of kernel DT makes us stay future-proof.
I'd really appreciate some feedback on this. I think the fairly simple approach in U-Boot is good enough, but I'm open to suggestions.
I am open on this topic too. Agreed that for U-Boot we always want a simple implementation for device drivers. I am adding Simon for his opinion on this topic as well.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu... [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu... [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
Regards, Bin
Hi Bin, Simon,
On 7/16/2019 11:21 AM, Bin Meng wrote:
+Simon,
Hi Alex,
On Mon, Jul 15, 2019 at 7:45 PM Alexandru Marginean alexandru.marginean@nxp.com wrote:
On 7/13/2019 7:02 AM, Bin Meng wrote:
Hi Alex,
On Fri, Jul 12, 2019 at 10:21 PM Alex Marginean alexandru.marginean@nxp.com wrote:
This driver is used for MDIO muxes driven over I2C. This is currently used on Freescale LS1028A QDS board, on which the physical MDIO MUX is controlled by an on-board FPGA which in turn is configured through I2C.
Signed-off-by: Alex Marginean alexm.osslist@gmail.com
Depends on https://patchwork.ozlabs.org/project/uboot/list/?series=119124
drivers/net/Kconfig | 8 +++ drivers/net/Makefile | 1 + drivers/net/mdio_mux_i2c_reg.c | 108 +++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+) create mode 100644 drivers/net/mdio_mux_i2c_reg.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 4d85fb1716..93535f7acb 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -595,4 +595,12 @@ config FSL_ENETC This driver supports the NXP ENETC Ethernet controller found on some of the NXP SoCs.
+config MDIO_MUX_I2C_REG
I don't see current Linux upstream has any I2CREG support. I believe you will upstream the Linux support too?
Linux support is somewhat different so we won't have the same driver and binding there. The main difference is that Linux has the concept of a mux controller, independent of the type of bus it muxes. I didn't add this to U-Boot, not yet at least. This specific MDIO mux would be driven in Linux by a pair of devices:
- a mux controller with bindings defined by reg-mux.txt [1], this one
implements the select function,
- MDIO mux as a client of the controller, defined by
mdio-mux-multiplexer.txt [2], which deals with the MDIO specific side of things.
Thanks for the detailed explanations! I thought people wanted to have exact the same DT bindings as Linux so that we can sync-up upstream kernel DT changes to U-Boot. That's why at first I checked whether kernel has such bindings in place.
In U-Boot I picked up the relevant properties from the two bindings, the MDIO mux device in U-Boot is supposed to implement select like a generic Linux MUX, while the uclass code deals with MDIO. I noticed U-Boot already has an I2C MUX class, along the lines of the MDIO one, I'm not sure if it's useful enough to introduce a new class of MUX controllers and then make all other kind of muxes depend on it. For now at least the U-Boot mux drivers are simple enough and the extra class wouldn't help much.
The other difference is that in Linux the mux controller is driven by the 'MMIO register bitfield-controlled multiplexer driver' [3], which is not I2C specific. It is built on top of regmap which I also think is a bit too much for U-Boot. For the U-Boot driver I just called I2C API directly and in that context it made sense to add I2C to the driver name, but there won't be an equivalent I2C based driver for Linux.
For your current implementation I believe that's OK, at least it is verified on LS1028. But there must be a reason that Linux kernel chose a DT that is different. I suspect the usage of kernel DT makes us stay future-proof.
Having a generic mux controller that's independent of the muxed bus has its merit, of course. Right now the API that a MDIO MUX must implement in U-Boot has just a _select function, which, by itself, has not direct relation to MDIO and could be used for other buses. So mux controllers could work in U-Boot, at least to keep U-Boot in sync with Linux.
I'm not sure about reg/mmio-mux. Those work in Linux because of regmap and layers that abstract the underlying bus which I think are a bit too much for U-Boot. Of course without that abstraction we can't use "reg-mux" or "mdio-mux-multiplexer" compatibility strings for the more specific devices and drivers in U-Boot.
I'd really appreciate some feedback on this. I think the fairly simple approach in U-Boot is good enough, but I'm open to suggestions.
I am open on this topic too. Agreed that for U-Boot we always want a simple implementation for device drivers. I am adding Simon for his opinion on this topic as well.
Thank you, let's see what Simon thinks about this.
Alex
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu... [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu... [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On Fri, Jul 12, 2019 at 9:21 AM Alex Marginean alexandru.marginean@nxp.com wrote:
This driver is used for MDIO muxes driven over I2C. This is currently used on Freescale LS1028A QDS board, on which the physical MDIO MUX is controlled by an on-board FPGA which in turn is configured through I2C.
Signed-off-by: Alex Marginean alexm.osslist@gmail.com
Depends on https://patchwork.ozlabs.org/project/uboot/list/?series=119124
Acked-by: Joe Hershberger joe.hershberger@ni.com
Hi Joe,
On 7/15/2019 11:06 PM, Joe Hershberger wrote:
On Fri, Jul 12, 2019 at 9:21 AM Alex Marginean alexandru.marginean@nxp.com wrote:
This driver is used for MDIO muxes driven over I2C. This is currently used on Freescale LS1028A QDS board, on which the physical MDIO MUX is controlled by an on-board FPGA which in turn is configured through I2C.
Signed-off-by: Alex Marginean alexm.osslist@gmail.com
Depends on https://patchwork.ozlabs.org/project/uboot/list/?series=119124
Acked-by: Joe Hershberger joe.hershberger@ni.com
Bin suggested I should rename the APIs and config option for consistency. I'll send a v2 with that and I'll keep the ACK, I assume that's OK.
Thanks! Alex
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
participants (5)
-
Alex Marginean -
Alex Marginean -
Alexandru Marginean
-
Bin Meng -
Joe Hershberger