[U-Boot] [PATCH 1/2] drivers: net: driver for MDIO muxes controlled over I2C
Alexandru Marginean
alexandru.marginean at nxp.com
Mon Jul 15 11:45:39 UTC 2019
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 at 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 at 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/Documentation/devicetree/bindings/mux/reg-mux.txt
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/mdio-mux-multiplexer.txt
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mux/mmio.c
>
> 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_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" },
>
> 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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>
More information about the U-Boot
mailing list