[U-Boot] [PATCH v2 0/4] drivers: Add a framework for MUX drivers
Add a new minimalistic subsystem that handles multiplexer controllers. It provides the same API as Linux and mux drivers should be portable with a minimum effort. This series also includes a port of the Linux's mmio-mux driver.
This series relies on a series that extend the regmap [1].
[1] : https://patchwork.ozlabs.org/project/uboot/list/?series=140752
Changes in v2: - Fixed warning in mux_of_xlate_default() - Improved documentation - Fixed SPL build - insert the mux initialization in init_sequence_r[], just before the console is initialized as its serial port may be muxed - moved the definition of dm_mux_init() in this commit - Call sandbox_set_enable_memio(true) before running the test
Jean-Jacques Hiblot (4): drivers: Add a new framework for multiplexer devices dm: board: complete the initialization of the muxes in initr_dm() drivers: mux: mmio-based syscon mux controller test: Add tests for the multiplexer framework
arch/sandbox/dts/test.dts | 26 +++ common/board_r.c | 16 ++ configs/sandbox_defconfig | 2 + drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/mux/Kconfig | 22 +++ drivers/mux/Makefile | 7 + drivers/mux/mmio.c | 155 ++++++++++++++++++ drivers/mux/mux-uclass.c | 292 ++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/dt-bindings/mux/mux.h | 17 ++ include/mux-internal.h | 103 ++++++++++++ include/mux.h | 115 +++++++++++++ test/dm/Makefile | 1 + test/dm/mux-mmio.c | 147 +++++++++++++++++ 15 files changed, 907 insertions(+) create mode 100644 drivers/mux/Kconfig create mode 100644 drivers/mux/Makefile create mode 100644 drivers/mux/mmio.c create mode 100644 drivers/mux/mux-uclass.c create mode 100644 include/dt-bindings/mux/mux.h create mode 100644 include/mux-internal.h create mode 100644 include/mux.h create mode 100644 test/dm/mux-mmio.c
Add a new subsystem that handles multiplexer controllers. The API is the same as in Linux.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
Changes in v2: - Fixed warning in mux_of_xlate_default() - Improved documentation - Fixed SPL build
drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/mux/Kconfig | 7 + drivers/mux/Makefile | 6 + drivers/mux/mux-uclass.c | 270 ++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/dt-bindings/mux/mux.h | 17 +++ include/mux-internal.h | 103 +++++++++++++ include/mux.h | 113 ++++++++++++++ 9 files changed, 520 insertions(+) create mode 100644 drivers/mux/Kconfig create mode 100644 drivers/mux/Makefile create mode 100644 drivers/mux/mux-uclass.c create mode 100644 include/dt-bindings/mux/mux.h create mode 100644 include/mux-internal.h create mode 100644 include/mux.h
diff --git a/drivers/Kconfig b/drivers/Kconfig index 9d99ce0226..450aa76e82 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -60,6 +60,8 @@ source "drivers/mmc/Kconfig"
source "drivers/mtd/Kconfig"
+source "drivers/mux/Kconfig" + source "drivers/net/Kconfig"
source "drivers/nvme/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 0befeddfcb..9d64742580 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_$(SPL_TPL_)INPUT) += input/ obj-$(CONFIG_$(SPL_TPL_)LED) += led/ obj-$(CONFIG_$(SPL_TPL_)MMC_SUPPORT) += mmc/ obj-$(CONFIG_$(SPL_TPL_)NAND_SUPPORT) += mtd/nand/raw/ +obj-$(CONFIG_$(SPL_)MULTIPLEXER) += mux/ obj-$(CONFIG_$(SPL_TPL_)PCH_SUPPORT) += pch/ obj-$(CONFIG_$(SPL_TPL_)PCI) += pci/ obj-$(CONFIG_$(SPL_TPL_)PHY) += phy/ diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig new file mode 100644 index 0000000000..ad0199c058 --- /dev/null +++ b/drivers/mux/Kconfig @@ -0,0 +1,7 @@ +menu "Multiplexer drivers" + +config MULTIPLEXER + bool "Multiplexer Support" + depends on DM + +endmenu diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile new file mode 100644 index 0000000000..351e4363d3 --- /dev/null +++ b/drivers/mux/Makefile @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# (C) Copyright 2019 +# Jean-Jacques Hiblot jjhiblot@ti.com + +obj-$(CONFIG_$(SPL_)MULTIPLEXER) += mux-uclass.o diff --git a/drivers/mux/mux-uclass.c b/drivers/mux/mux-uclass.c new file mode 100644 index 0000000000..6aaf4dc964 --- /dev/null +++ b/drivers/mux/mux-uclass.c @@ -0,0 +1,270 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Multiplexer subsystem + * + * Based on the linux multiplexer framework + * + * Copyright (C) 2017 Axentia Technologies AB + * Author: Peter Rosin peda@axentia.se + * + * Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/ + * Jean-Jacques Hiblot jjhiblot@ti.com + */ + +#include <common.h> +#include <dm.h> +#include <mux-internal.h> +#include <dm/device-internal.h> +#include <dt-bindings/mux/mux.h> + +/* + * The idle-as-is "state" is not an actual state that may be selected, it + * only implies that the state should not be changed. So, use that state + * as indication that the cached state of the multiplexer is unknown. + */ +#define MUX_CACHE_UNKNOWN MUX_IDLE_AS_IS + +static inline const struct mux_control_ops *mux_dev_ops(struct udevice *dev) +{ + return (const struct mux_control_ops *)dev->driver->ops; +} + +static int mux_control_set(struct mux_control *mux, int state) +{ + int ret = mux_dev_ops(mux->dev)->set(mux, state); + + mux->cached_state = ret < 0 ? MUX_CACHE_UNKNOWN : state; + + return ret; +} + +unsigned int mux_control_states(struct mux_control *mux) +{ + return mux->states; +} + +static int __mux_control_select(struct mux_control *mux, int state) +{ + int ret; + + if (WARN_ON(state < 0 || state >= mux->states)) + return -EINVAL; + + if (mux->cached_state == state) + return 0; + + ret = mux_control_set(mux, state); + if (ret >= 0) + return 0; + + /* The mux update failed, try to revert if appropriate... */ + if (mux->idle_state != MUX_IDLE_AS_IS) + mux_control_set(mux, mux->idle_state); + + return ret; +} + +int mux_control_select(struct mux_control *mux, unsigned int state) +{ + int ret; + + if (mux->in_use) + return -EBUSY; + + ret = __mux_control_select(mux, state); + + if (ret < 0) + return ret; + + mux->in_use = true; + + return 0; +} + +int mux_control_deselect(struct mux_control *mux) +{ + int ret = 0; + + if (mux->idle_state != MUX_IDLE_AS_IS && + mux->idle_state != mux->cached_state) + ret = mux_control_set(mux, mux->idle_state); + + mux->in_use = false; + + return ret; +} + +static int mux_of_xlate_default(struct mux_chip *mux_chip, + struct ofnode_phandle_args *args, + struct mux_control **muxp) +{ + struct mux_control *mux; + int id; + + debug("%s(muxp=%p)\n", __func__, muxp); + + if (args->args_count > 1) { + debug("Invaild args_count: %d\n", args->args_count); + return -EINVAL; + } + + if (args->args_count) + id = args->args[0]; + else + id = 0; + + if (id >= mux_chip->controllers) { + dev_err(dev, "bad mux controller %u specified in %s\n", + id, ofnode_get_name(args->node)); + return -ERANGE; + } + + mux = &mux_chip->mux[id]; + mux->id = id; + *muxp = mux; + return 0; +} + +static int mux_get_by_indexed_prop(struct udevice *dev, const char *prop_name, + int index, struct mux_control **mux) +{ + int ret; + struct ofnode_phandle_args args; + struct udevice *dev_mux; + const struct mux_control_ops *ops; + struct mux_chip *mux_chip; + + debug("%s(dev=%p, index=%d, mux=%p)\n", __func__, dev, index, mux); + + ret = dev_read_phandle_with_args(dev, prop_name, "#mux-control-cells", + 0, index, &args); + if (ret) { + debug("%s: fdtdec_parse_phandle_with_args failed: err=%d\n", + __func__, ret); + return ret; + } + + ret = uclass_get_device_by_ofnode(UCLASS_MUX, args.node, &dev_mux); + if (ret) { + debug("%s: uclass_get_device_by_ofnode failed: err=%d\n", + __func__, ret); + return ret; + } + + mux_chip = dev_get_uclass_priv(dev_mux); + + ops = mux_dev_ops(dev_mux); + if (ops->of_xlate) + ret = ops->of_xlate(mux_chip, &args, mux); + else + ret = mux_of_xlate_default(mux_chip, &args, mux); + if (ret) { + debug("of_xlate() failed: %d\n", ret); + return ret; + } + (*mux)->dev = dev_mux; + + return 0; +} + +int mux_get_by_index(struct udevice *dev, int index, struct mux_control **mux) +{ + return mux_get_by_indexed_prop(dev, "mux-controls", index, mux); +} + +int mux_control_get(struct udevice *dev, const char *name, + struct mux_control **mux) +{ + int index; + + debug("%s(dev=%p, name=%s, mux=%p)\n", __func__, dev, name, mux); + + index = dev_read_stringlist_search(dev, "mux-control-names", name); + if (index < 0) { + debug("fdt_stringlist_search() failed: %d\n", index); + return index; + } + + return mux_get_by_index(dev, index, mux); +} + +void mux_control_put(struct mux_control *mux) +{ + mux_control_deselect(mux); +} + +static void devm_mux_control_release(struct udevice *dev, void *res) +{ + mux_control_put(*(struct mux_control **)res); +} + +struct mux_control *devm_mux_control_get(struct udevice *dev, const char *id) +{ + int rc; + struct mux_control **mux; + + mux = devres_alloc(devm_mux_control_release, + sizeof(struct mux_control *), __GFP_ZERO); + if (unlikely(!mux)) + return ERR_PTR(-ENOMEM); + + rc = mux_control_get(dev, id, mux); + if (rc) + return ERR_PTR(rc); + + devres_add(dev, mux); + return *mux; +} + +int mux_alloc_controllers(struct udevice *dev, unsigned int controllers) +{ + int i; + struct mux_chip *mux_chip = dev_get_uclass_priv(dev); + + mux_chip->mux = devm_kmalloc(dev, + sizeof(struct mux_control) * controllers, + __GFP_ZERO); + if (!mux_chip->mux) + return -ENOMEM; + + mux_chip->controllers = controllers; + + for (i = 0; i < mux_chip->controllers; ++i) { + struct mux_control *mux = &mux_chip->mux[i]; + + mux->dev = dev; + mux->cached_state = MUX_CACHE_UNKNOWN; + mux->idle_state = MUX_IDLE_AS_IS; + mux->in_use = false; + mux->id = i; + } + + return 0; +} + +int mux_uclass_post_probe(struct udevice *dev) +{ + int i, ret; + struct mux_chip *mux_chip = dev_get_uclass_priv(dev); + + for (i = 0; i < mux_chip->controllers; ++i) { + struct mux_control *mux = &mux_chip->mux[i]; + + if (mux->idle_state == mux->cached_state) + continue; + + ret = mux_control_set(mux, mux->idle_state); + if (ret < 0) { + dev_err(&mux_chip->dev, "unable to set idle state\n"); + return ret; + } + } + return 0; +} + +UCLASS_DRIVER(mux) = { + .id = UCLASS_MUX, + .name = "mux", + .post_probe = mux_uclass_post_probe, + .per_device_auto_alloc_size = sizeof(struct mux_chip), +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 0c563d898b..28822689a9 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -66,6 +66,7 @@ enum uclass_id { UCLASS_MMC, /* SD / MMC card or chip */ UCLASS_MOD_EXP, /* RSA Mod Exp device */ UCLASS_MTD, /* Memory Technology Device (MTD) device */ + UCLASS_MUX, /* Multiplexer device */ UCLASS_NOP, /* No-op devices */ UCLASS_NORTHBRIDGE, /* Intel Northbridge / SDRAM controller */ UCLASS_NVME, /* NVM Express device */ diff --git a/include/dt-bindings/mux/mux.h b/include/dt-bindings/mux/mux.h new file mode 100644 index 0000000000..042719218d --- /dev/null +++ b/include/dt-bindings/mux/mux.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * This header provides constants for most Multiplexer bindings. + * + * Most Multiplexer bindings specify an idle state. In most cases, the + * the multiplexer can be left as is when idle, and in some cases it can + * disconnect the input/output and leave the multiplexer in a high + * impedance state. + */ + +#ifndef _DT_BINDINGS_MUX_MUX_H +#define _DT_BINDINGS_MUX_MUX_H + +#define MUX_IDLE_AS_IS (-1) +#define MUX_IDLE_DISCONNECT (-2) + +#endif diff --git a/include/mux-internal.h b/include/mux-internal.h new file mode 100644 index 0000000000..c590bd0c74 --- /dev/null +++ b/include/mux-internal.h @@ -0,0 +1,103 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Based on the linux multiplexer framework + * + * Copyright (C) 2017 Axentia Technologies AB + * Author: Peter Rosin peda@axentia.se + * + * Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/ + * Jean-Jacques Hiblot jjhiblot@ti.com + */ + +#ifndef _MUX_UCLASS_H +#define _MUX_UCLASS_H + +/* See mux.h for background documentation. */ + +#include <mux.h> + +struct ofnode_phandle_args; + +/** + * struct mux_chip - Represents a chip holding mux controllers. + * @controllers: Number of mux controllers handled by the chip. + * @mux: Array of mux controllers that are handled. + * @dev: Device structure. + * @ops: Mux controller operations. + * + * This a per-device uclass-private data. + */ +struct mux_chip { + unsigned int controllers; + struct mux_control *mux; +}; + +/** + * struct mux_control_ops - Mux controller operations for a mux chip. + * @set: Set the state of the given mux controller. + */ +struct mux_control_ops { + /** + * set - Apply a state to a multiplexer control + * + * @mux: A multiplexer control + * @return 0 if OK, or a negative error code. + */ + int (*set)(struct mux_control *mux, int state); + + /** + * of_xlate - Translate a client's device-tree (OF) multiplexer + * specifier. + * + * If this function pointer is set to NULL, the multiplexer core will + * use a default implementation, which assumes #mux-control-cells = <1> + * and that the DT cell contains a simple integer channel ID. + * + * @dev_mux: The multiplexer device. A single device may handle + * several multiplexer controls. + * @args: The multiplexer specifier values from device tree. + * @mux: (out) A multiplexer control + * @return 0 if OK, or a negative error code. + */ + int (*of_xlate)(struct mux_chip *dev_mux, + struct ofnode_phandle_args *args, + struct mux_control **mux); +}; + +/** + * struct mux_control - Represents a mux controller. + * @chip: The mux chip that is handling this mux controller. + * @cached_state: The current mux controller state, or -1 if none. + * @states: The number of mux controller states. + * @idle_state: The mux controller state to use when inactive, or one + * of MUX_IDLE_AS_IS and MUX_IDLE_DISCONNECT. + * + * Mux drivers may only change @states and @idle_state, and may only do so + * between allocation and registration of the mux controller. Specifically, + * @cached_state is internal to the mux core and should never be written by + * mux drivers. + */ +struct mux_control { + bool in_use; + struct udevice *dev; + int cached_state; + unsigned int states; + int idle_state; + int id; +}; + +/** + * mux_control_get_index() - Get the index of the given mux controller + * @mux: The mux-control to get the index for. + * + * Return: The index of the mux controller within the mux chip the mux + * controller is a part of. + */ +static inline unsigned int mux_control_get_index(struct mux_control *mux) +{ + return mux->id; +} + +int mux_alloc_controllers(struct udevice *dev, unsigned int controllers); + +#endif diff --git a/include/mux.h b/include/mux.h new file mode 100644 index 0000000000..060f71a47c --- /dev/null +++ b/include/mux.h @@ -0,0 +1,113 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Based on the linux multiplexer framework + * + * At its core, a multiplexer (or mux), also known as a data selector, is a + * device that selects between several analog or digital input signals and + * forwards it to a single output line. This notion can be extended to work + * with buses, like a I2C bus multiplexer for example. + * + * Copyright (C) 2017 Axentia Technologies AB + * Author: Peter Rosin peda@axentia.se + * + * Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/ + * Jean-Jacques Hiblot jjhiblot@ti.com + */ + +#ifndef _MUX_H_ +#define _MUX_H_ + +#include <linux/errno.h> +#include <linux/types.h> + +struct udevice; +struct mux_control; + +#if CONFIG_IS_ENABLED(MULTIPLEXER) +/** + * mux_control_states() - Query the number of multiplexer states. + * @mux: The mux-control to query. + * + * Return: The number of multiplexer states. + */ +unsigned int mux_control_states(struct mux_control *mux); + +/** + * mux_control_select() - Select the given multiplexer state. + * @mux: The mux-control to request a change of state from. + * @state: The new requested state. + * + * On successfully selecting the mux-control state, it will be locked until + * there is a call to mux_control_deselect(). If the mux-control is already + * selected when mux_control_select() is called, the function will indicate + * -EBUSY + * + * Therefore, make sure to call mux_control_deselect() when the operation is + * complete and the mux-control is free for others to use, but do not call + * mux_control_deselect() if mux_control_select() fails. + * + * Return: 0 when the mux-control state has the requested state or a negative + * errno on error. + */ +int __must_check mux_control_select(struct mux_control *mux, + unsigned int state); +#define mux_control_try_select(mux) mux_control_select(mux) + +/** + * mux_control_deselect() - Deselect the previously selected multiplexer state. + * @mux: The mux-control to deselect. + * + * It is required that a single call is made to mux_control_deselect() for + * each and every successful call made to either of mux_control_select() or + * mux_control_try_select(). + * + * Return: 0 on success and a negative errno on error. An error can only + * occur if the mux has an idle state. Note that even if an error occurs, the + * mux-control is unlocked and is thus free for the next access. + */ +int mux_control_deselect(struct mux_control *mux); + +int mux_get_by_index(struct udevice *dev, int index, struct mux_control **mux); +int mux_control_get(struct udevice *dev, const char *name, + struct mux_control **mux); + +void mux_control_put(struct mux_control *mux); + +struct mux_control *devm_mux_control_get(struct udevice *dev, + const char *mux_name); +#else +unsigned int mux_control_states(struct mux_control *mux) +{ + return -ENOSYS; +} + +int __must_check mux_control_select(struct mux_control *mux, + unsigned int state) +{ + return -ENOSYS; +} + +#define mux_control_try_select(mux) mux_control_select(mux) + +int mux_control_deselect(struct mux_control *mux) +{ + return -ENOSYS; +} + +struct mux_control *mux_control_get(struct udevice *dev, const char *mux_name) +{ + return NULL; +} + +void mux_control_put(struct mux_control *mux) +{ +} + +struct mux_control *devm_mux_control_get(struct udevice *dev, + const char *mux_name) +{ + return NULL; +} +#endif + +#endif
Hi Jean-Jacques,
On Tue, 5 Nov 2019 at 04:50, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Add a new subsystem that handles multiplexer controllers. The API is the same as in Linux.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2:
- Fixed warning in mux_of_xlate_default()
- Improved documentation
- Fixed SPL build
drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/mux/Kconfig | 7 + drivers/mux/Makefile | 6 + drivers/mux/mux-uclass.c | 270 ++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/dt-bindings/mux/mux.h | 17 +++ include/mux-internal.h | 103 +++++++++++++ include/mux.h | 113 ++++++++++++++ 9 files changed, 520 insertions(+) create mode 100644 drivers/mux/Kconfig create mode 100644 drivers/mux/Makefile create mode 100644 drivers/mux/mux-uclass.c create mode 100644 include/dt-bindings/mux/mux.h create mode 100644 include/mux-internal.h create mode 100644 include/mux.h
diff --git a/drivers/Kconfig b/drivers/Kconfig index 9d99ce0226..450aa76e82 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -60,6 +60,8 @@ source "drivers/mmc/Kconfig"
source "drivers/mtd/Kconfig"
+source "drivers/mux/Kconfig"
source "drivers/net/Kconfig"
source "drivers/nvme/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 0befeddfcb..9d64742580 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_$(SPL_TPL_)INPUT) += input/ obj-$(CONFIG_$(SPL_TPL_)LED) += led/ obj-$(CONFIG_$(SPL_TPL_)MMC_SUPPORT) += mmc/ obj-$(CONFIG_$(SPL_TPL_)NAND_SUPPORT) += mtd/nand/raw/ +obj-$(CONFIG_$(SPL_)MULTIPLEXER) += mux/ obj-$(CONFIG_$(SPL_TPL_)PCH_SUPPORT) += pch/ obj-$(CONFIG_$(SPL_TPL_)PCI) += pci/ obj-$(CONFIG_$(SPL_TPL_)PHY) += phy/ diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig new file mode 100644 index 0000000000..ad0199c058 --- /dev/null +++ b/drivers/mux/Kconfig @@ -0,0 +1,7 @@ +menu "Multiplexer drivers"
+config MULTIPLEXER
bool "Multiplexer Support"depends on DM
Please add help here.
+endmenu diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile new file mode 100644 index 0000000000..351e4363d3 --- /dev/null +++ b/drivers/mux/Makefile @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# (C) Copyright 2019 +# Jean-Jacques Hiblot jjhiblot@ti.com
+obj-$(CONFIG_$(SPL_)MULTIPLEXER) += mux-uclass.o diff --git a/drivers/mux/mux-uclass.c b/drivers/mux/mux-uclass.c new file mode 100644 index 0000000000..6aaf4dc964 --- /dev/null +++ b/drivers/mux/mux-uclass.c @@ -0,0 +1,270 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Multiplexer subsystem
- Based on the linux multiplexer framework
- Copyright (C) 2017 Axentia Technologies AB
- Author: Peter Rosin peda@axentia.se
- Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/
- Jean-Jacques Hiblot jjhiblot@ti.com
- */
+#include <common.h> +#include <dm.h> +#include <mux-internal.h> +#include <dm/device-internal.h> +#include <dt-bindings/mux/mux.h>
+/*
- The idle-as-is "state" is not an actual state that may be selected, it
- only implies that the state should not be changed. So, use that state
- as indication that the cached state of the multiplexer is unknown.
- */
+#define MUX_CACHE_UNKNOWN MUX_IDLE_AS_IS
+static inline const struct mux_control_ops *mux_dev_ops(struct udevice *dev) +{
return (const struct mux_control_ops *)dev->driver->ops;+}
+static int mux_control_set(struct mux_control *mux, int state)
Please add comments to these static functions.
+{
int ret = mux_dev_ops(mux->dev)->set(mux, state);mux->cached_state = ret < 0 ? MUX_CACHE_UNKNOWN : state;return ret;+}
+unsigned int mux_control_states(struct mux_control *mux) +{
return mux->states;+}
+static int __mux_control_select(struct mux_control *mux, int state) +{
int ret;if (WARN_ON(state < 0 || state >= mux->states))return -EINVAL;if (mux->cached_state == state)return 0;ret = mux_control_set(mux, state);if (ret >= 0)return 0;/* The mux update failed, try to revert if appropriate... */if (mux->idle_state != MUX_IDLE_AS_IS)mux_control_set(mux, mux->idle_state);return ret;+}
+int mux_control_select(struct mux_control *mux, unsigned int state) +{
int ret;if (mux->in_use)return -EBUSY;ret = __mux_control_select(mux, state);if (ret < 0)return ret;mux->in_use = true;return 0;+}
+int mux_control_deselect(struct mux_control *mux) +{
int ret = 0;if (mux->idle_state != MUX_IDLE_AS_IS &&mux->idle_state != mux->cached_state)ret = mux_control_set(mux, mux->idle_state);mux->in_use = false;return ret;+}
+static int mux_of_xlate_default(struct mux_chip *mux_chip,
struct ofnode_phandle_args *args,struct mux_control **muxp)+{
struct mux_control *mux;int id;debug("%s(muxp=%p)\n", __func__, muxp);
Could use log_debug() perhaps?
if (args->args_count > 1) {debug("Invaild args_count: %d\n", args->args_count);return -EINVAL;}if (args->args_count)id = args->args[0];elseid = 0;if (id >= mux_chip->controllers) {dev_err(dev, "bad mux controller %u specified in %s\n",id, ofnode_get_name(args->node));return -ERANGE;}mux = &mux_chip->mux[id];mux->id = id;*muxp = mux;return 0;+}
+static int mux_get_by_indexed_prop(struct udevice *dev, const char *prop_name,
int index, struct mux_control **mux)
Again please comment these static functions.
+{
int ret;struct ofnode_phandle_args args;struct udevice *dev_mux;const struct mux_control_ops *ops;struct mux_chip *mux_chip;debug("%s(dev=%p, index=%d, mux=%p)\n", __func__, dev, index, mux);ret = dev_read_phandle_with_args(dev, prop_name, "#mux-control-cells",0, index, &args);if (ret) {debug("%s: fdtdec_parse_phandle_with_args failed: err=%d\n",__func__, ret);return ret;}ret = uclass_get_device_by_ofnode(UCLASS_MUX, args.node, &dev_mux);if (ret) {debug("%s: uclass_get_device_by_ofnode failed: err=%d\n",__func__, ret);return ret;}mux_chip = dev_get_uclass_priv(dev_mux);ops = mux_dev_ops(dev_mux);if (ops->of_xlate)ret = ops->of_xlate(mux_chip, &args, mux);elseret = mux_of_xlate_default(mux_chip, &args, mux);if (ret) {debug("of_xlate() failed: %d\n", ret);return ret;}(*mux)->dev = dev_mux;return 0;+}
+int mux_get_by_index(struct udevice *dev, int index, struct mux_control **mux) +{
return mux_get_by_indexed_prop(dev, "mux-controls", index, mux);+}
+int mux_control_get(struct udevice *dev, const char *name,
struct mux_control **mux)+{
int index;debug("%s(dev=%p, name=%s, mux=%p)\n", __func__, dev, name, mux);index = dev_read_stringlist_search(dev, "mux-control-names", name);if (index < 0) {debug("fdt_stringlist_search() failed: %d\n", index);return index;}return mux_get_by_index(dev, index, mux);+}
+void mux_control_put(struct mux_control *mux) +{
mux_control_deselect(mux);+}
+static void devm_mux_control_release(struct udevice *dev, void *res) +{
mux_control_put(*(struct mux_control **)res);+}
+struct mux_control *devm_mux_control_get(struct udevice *dev, const char *id)
This should return an integer error with mux_control as a parameter passed in by the caller, not allocated here. The madness below and in the caller to convert everything to pointers makes no sense to me.
+{
int rc;struct mux_control **mux;mux = devres_alloc(devm_mux_control_release,sizeof(struct mux_control *), __GFP_ZERO);if (unlikely(!mux))return ERR_PTR(-ENOMEM);
Then you don't need to alloc this since it is passed in.
rc = mux_control_get(dev, id, mux);if (rc)return ERR_PTR(rc);devres_add(dev, mux);return *mux;+}
+int mux_alloc_controllers(struct udevice *dev, unsigned int controllers)
This function needs comments in its header file.
+{
int i;struct mux_chip *mux_chip = dev_get_uclass_priv(dev);mux_chip->mux = devm_kmalloc(dev,sizeof(struct mux_control) * controllers,__GFP_ZERO);if (!mux_chip->mux)return -ENOMEM;mux_chip->controllers = controllers;for (i = 0; i < mux_chip->controllers; ++i) {struct mux_control *mux = &mux_chip->mux[i];mux->dev = dev;mux->cached_state = MUX_CACHE_UNKNOWN;mux->idle_state = MUX_IDLE_AS_IS;mux->in_use = false;mux->id = i;}return 0;+}
+int mux_uclass_post_probe(struct udevice *dev)
static
+{
int i, ret;struct mux_chip *mux_chip = dev_get_uclass_priv(dev);
Can you add a comment here as to what this is doing?
for (i = 0; i < mux_chip->controllers; ++i) {struct mux_control *mux = &mux_chip->mux[i];if (mux->idle_state == mux->cached_state)continue;ret = mux_control_set(mux, mux->idle_state);if (ret < 0) {dev_err(&mux_chip->dev, "unable to set idle state\n");return ret;}}return 0;+}
+UCLASS_DRIVER(mux) = {
.id = UCLASS_MUX,.name = "mux",.post_probe = mux_uclass_post_probe,.per_device_auto_alloc_size = sizeof(struct mux_chip),+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 0c563d898b..28822689a9 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -66,6 +66,7 @@ enum uclass_id { UCLASS_MMC, /* SD / MMC card or chip */ UCLASS_MOD_EXP, /* RSA Mod Exp device */ UCLASS_MTD, /* Memory Technology Device (MTD) device */
UCLASS_MUX, /* Multiplexer device */ UCLASS_NOP, /* No-op devices */ UCLASS_NORTHBRIDGE, /* Intel Northbridge / SDRAM controller */ UCLASS_NVME, /* NVM Express device */diff --git a/include/dt-bindings/mux/mux.h b/include/dt-bindings/mux/mux.h new file mode 100644 index 0000000000..042719218d --- /dev/null +++ b/include/dt-bindings/mux/mux.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*
- This header provides constants for most Multiplexer bindings.
- Most Multiplexer bindings specify an idle state. In most cases, the
- the multiplexer can be left as is when idle, and in some cases it can
- disconnect the input/output and leave the multiplexer in a high
- impedance state.
- */
+#ifndef _DT_BINDINGS_MUX_MUX_H +#define _DT_BINDINGS_MUX_MUX_H
+#define MUX_IDLE_AS_IS (-1) +#define MUX_IDLE_DISCONNECT (-2)
+#endif diff --git a/include/mux-internal.h b/include/mux-internal.h new file mode 100644 index 0000000000..c590bd0c74 --- /dev/null +++ b/include/mux-internal.h @@ -0,0 +1,103 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Based on the linux multiplexer framework
- Copyright (C) 2017 Axentia Technologies AB
- Author: Peter Rosin peda@axentia.se
- Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/
- Jean-Jacques Hiblot jjhiblot@ti.com
- */
+#ifndef _MUX_UCLASS_H +#define _MUX_UCLASS_H
MUX_INTERNAL
+/* See mux.h for background documentation. */
+#include <mux.h>
Can you drop this? I don't think it is needed.
+struct ofnode_phandle_args;
+/**
- struct mux_chip - Represents a chip holding mux controllers.
- @controllers: Number of mux controllers handled by the chip.
- @mux: Array of mux controllers that are handled.
- @dev: Device structure.
- @ops: Mux controller operations.
Doesn't seem to match the struct.
- This a per-device uclass-private data.
- */
+struct mux_chip {
unsigned int controllers;struct mux_control *mux;+};
+/**
- struct mux_control_ops - Mux controller operations for a mux chip.
- @set: Set the state of the given mux controller.
- */
+struct mux_control_ops {
/*** set - Apply a state to a multiplexer control** @mux: A multiplexer control* @return 0 if OK, or a negative error code.*/int (*set)(struct mux_control *mux, int state);/*** of_xlate - Translate a client's device-tree (OF) multiplexer* specifier.** If this function pointer is set to NULL, the multiplexer core will* use a default implementation, which assumes #mux-control-cells = <1>* and that the DT cell contains a simple integer channel ID.** @dev_mux: The multiplexer device. A single device may handle* several multiplexer controls.* @args: The multiplexer specifier values from device tree.* @mux: (out) A multiplexer control
Can you rename @muxp with the 'p' indicating it is a pointer to the value? This is the convention in driver model.
* @return 0 if OK, or a negative error code.*/int (*of_xlate)(struct mux_chip *dev_mux,struct ofnode_phandle_args *args,struct mux_control **mux);+};
+/**
- struct mux_control - Represents a mux controller.
- @chip: The mux chip that is handling this mux controller.
- @cached_state: The current mux controller state, or -1 if none.
- @states: The number of mux controller states.
- @idle_state: The mux controller state to use when inactive, or one
of MUX_IDLE_AS_IS and MUX_IDLE_DISCONNECT.
Missing comments on a few fields
- Mux drivers may only change @states and @idle_state, and may only do so
- between allocation and registration of the mux controller. Specifically,
- @cached_state is internal to the mux core and should never be written by
- mux drivers.
- */
+struct mux_control {
bool in_use;struct udevice *dev;int cached_state;unsigned int states;int idle_state;int id;+};
+/**
- mux_control_get_index() - Get the index of the given mux controller
- @mux: The mux-control to get the index for.
- Return: The index of the mux controller within the mux chip the mux
- controller is a part of.
- */
+static inline unsigned int mux_control_get_index(struct mux_control *mux) +{
return mux->id;+}
+int mux_alloc_controllers(struct udevice *dev, unsigned int controllers);
+#endif diff --git a/include/mux.h b/include/mux.h new file mode 100644 index 0000000000..060f71a47c --- /dev/null +++ b/include/mux.h @@ -0,0 +1,113 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Based on the linux multiplexer framework
That explains the lack of comments I suppose! But we need to add them tor U-Boot.
- At its core, a multiplexer (or mux), also known as a data selector, is a
- device that selects between several analog or digital input signals and
- forwards it to a single output line. This notion can be extended to work
- with buses, like a I2C bus multiplexer for example.
- Copyright (C) 2017 Axentia Technologies AB
- Author: Peter Rosin peda@axentia.se
- Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/
- Jean-Jacques Hiblot jjhiblot@ti.com
- */
+#ifndef _MUX_H_ +#define _MUX_H_
+#include <linux/errno.h> +#include <linux/types.h>
+struct udevice; +struct mux_control;
+#if CONFIG_IS_ENABLED(MULTIPLEXER) +/**
- mux_control_states() - Query the number of multiplexer states.
- @mux: The mux-control to query.
- Return: The number of multiplexer states.
- */
+unsigned int mux_control_states(struct mux_control *mux);
+/**
- mux_control_select() - Select the given multiplexer state.
- @mux: The mux-control to request a change of state from.
- @state: The new requested state.
- On successfully selecting the mux-control state, it will be locked until
- there is a call to mux_control_deselect(). If the mux-control is already
- selected when mux_control_select() is called, the function will indicate
- -EBUSY
- Therefore, make sure to call mux_control_deselect() when the operation is
- complete and the mux-control is free for others to use, but do not call
- mux_control_deselect() if mux_control_select() fails.
- Return: 0 when the mux-control state has the requested state or a negative
- errno on error.
- */
+int __must_check mux_control_select(struct mux_control *mux,
unsigned int state);+#define mux_control_try_select(mux) mux_control_select(mux)
+/**
- mux_control_deselect() - Deselect the previously selected multiplexer state.
- @mux: The mux-control to deselect.
- It is required that a single call is made to mux_control_deselect() for
- each and every successful call made to either of mux_control_select() or
- mux_control_try_select().
- Return: 0 on success and a negative errno on error. An error can only
- occur if the mux has an idle state. Note that even if an error occurs, the
- mux-control is unlocked and is thus free for the next access.
- */
+int mux_control_deselect(struct mux_control *mux);
+int mux_get_by_index(struct udevice *dev, int index, struct mux_control **mux); +int mux_control_get(struct udevice *dev, const char *name,
struct mux_control **mux);+void mux_control_put(struct mux_control *mux);
+struct mux_control *devm_mux_control_get(struct udevice *dev,
const char *mux_name);
Many functions missing comments here.
+#else +unsigned int mux_control_states(struct mux_control *mux) +{
return -ENOSYS;+}
+int __must_check mux_control_select(struct mux_control *mux,
unsigned int state)+{
return -ENOSYS;+}
+#define mux_control_try_select(mux) mux_control_select(mux)
+int mux_control_deselect(struct mux_control *mux) +{
return -ENOSYS;+}
+struct mux_control *mux_control_get(struct udevice *dev, const char *mux_name) +{
return NULL;+}
+void mux_control_put(struct mux_control *mux) +{ +}
+struct mux_control *devm_mux_control_get(struct udevice *dev,
const char *mux_name)+{
return NULL;+} +#endif
+#endif
2.17.1
Regards, Simon
This will probe the multiplexer devices that have a "u-boot,mux-autoprobe" property. As a consequence they will be put in their idle state.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
Changes in v2: - insert the mux initialization in init_sequence_r[], just before the console is initialized as its serial port may be muxed - moved the definition of dm_mux_init() in this commit
common/board_r.c | 16 ++++++++++++++++ drivers/mux/mux-uclass.c | 22 ++++++++++++++++++++++ include/mux.h | 2 ++ 3 files changed, 40 insertions(+)
diff --git a/common/board_r.c b/common/board_r.c index c1ecb06b74..3d410f3504 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -32,6 +32,7 @@ #include <miiphy.h> #endif #include <mmc.h> +#include <mux.h> #include <nand.h> #include <of_live.h> #include <onenand_uboot.h> @@ -178,6 +179,18 @@ static int initr_serial(void) return 0; }
+#if CONFIG_MULTIPLEXER +static int initr_mux(void) +{ + /* + * Initialize the multiplexer controls to their default state. + * This must be done early as other drivers may unknowingly rely on it. + */ + dm_mux_init(); + return 0; +} +#endif + #if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_MIPS) static int initr_trap(void) { @@ -691,6 +704,9 @@ static init_fnc_t init_sequence_r[] = { #endif #ifdef CONFIG_EFI_LOADER efi_memory_init, +#endif +#if CONFIG_MULTIPLEXER + initr_mux, #endif stdio_init_tables, initr_serial, diff --git a/drivers/mux/mux-uclass.c b/drivers/mux/mux-uclass.c index 6aaf4dc964..71392e9e50 100644 --- a/drivers/mux/mux-uclass.c +++ b/drivers/mux/mux-uclass.c @@ -262,6 +262,28 @@ int mux_uclass_post_probe(struct udevice *dev) return 0; }
+void dm_mux_init(void) +{ + struct uclass *uc; + struct udevice *dev; + int ret; + + ret = uclass_get(UCLASS_MUX, &uc); + if (ret < 0) { + debug("unable to get MUX uclass\n"); + return; + } + uclass_foreach_dev(dev, uc) { + if (dev_read_bool(dev, "u-boot,mux-autoprobe")) { + ret = device_probe(dev); + if (ret) + debug("unable to probe device %s\n", dev->name); + } else { + printf("not found for dev %s\n", dev->name); + } + } +} + UCLASS_DRIVER(mux) = { .id = UCLASS_MUX, .name = "mux", diff --git a/include/mux.h b/include/mux.h index 060f71a47c..2467723951 100644 --- a/include/mux.h +++ b/include/mux.h @@ -75,6 +75,8 @@ void mux_control_put(struct mux_control *mux);
struct mux_control *devm_mux_control_get(struct udevice *dev, const char *mux_name); +void dm_mux_init(void); + #else unsigned int mux_control_states(struct mux_control *mux) {
Hi JJ,
On 05/11/19 5:20 PM, Jean-Jacques Hiblot wrote:
This will probe the multiplexer devices that have a "u-boot,mux-autoprobe" property. As a consequence they will be put in their idle state.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
[...]
diff --git a/drivers/mux/mux-uclass.c b/drivers/mux/mux-uclass.c index 6aaf4dc964..71392e9e50 100644 --- a/drivers/mux/mux-uclass.c +++ b/drivers/mux/mux-uclass.c @@ -262,6 +262,28 @@ int mux_uclass_post_probe(struct udevice *dev) return 0; }
+void dm_mux_init(void) +{
- struct uclass *uc;
- struct udevice *dev;
- int ret;
- ret = uclass_get(UCLASS_MUX, &uc);
- if (ret < 0) {
debug("unable to get MUX uclass\n");return;- }
- uclass_foreach_dev(dev, uc) {
if (dev_read_bool(dev, "u-boot,mux-autoprobe")) {ret = device_probe(dev);if (ret)debug("unable to probe device %s\n", dev->name);} else {printf("not found for dev %s\n", dev->name);}
Is "u-boot,mux-autoprobe" a required property? The fact that its in DT makes me think its optional. If that's the case, above printf() should be reduced to debug() to avoid confusion
- }
+}
UCLASS_DRIVER(mux) = { .id = UCLASS_MUX, .name = "mux", diff --git a/include/mux.h b/include/mux.h index 060f71a47c..2467723951 100644 --- a/include/mux.h +++ b/include/mux.h @@ -75,6 +75,8 @@ void mux_control_put(struct mux_control *mux);
struct mux_control *devm_mux_control_get(struct udevice *dev, const char *mux_name); +void dm_mux_init(void);
#else unsigned int mux_control_states(struct mux_control *mux) {
On 05/11/2019 14:05, Vignesh Raghavendra wrote:
Hi JJ,
On 05/11/19 5:20 PM, Jean-Jacques Hiblot wrote:
This will probe the multiplexer devices that have a "u-boot,mux-autoprobe" property. As a consequence they will be put in their idle state.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
[...]
diff --git a/drivers/mux/mux-uclass.c b/drivers/mux/mux-uclass.c index 6aaf4dc964..71392e9e50 100644 --- a/drivers/mux/mux-uclass.c +++ b/drivers/mux/mux-uclass.c @@ -262,6 +262,28 @@ int mux_uclass_post_probe(struct udevice *dev) return 0; }
+void dm_mux_init(void) +{
- struct uclass *uc;
- struct udevice *dev;
- int ret;
- ret = uclass_get(UCLASS_MUX, &uc);
- if (ret < 0) {
debug("unable to get MUX uclass\n");return;- }
- uclass_foreach_dev(dev, uc) {
if (dev_read_bool(dev, "u-boot,mux-autoprobe")) {ret = device_probe(dev);if (ret)debug("unable to probe device %s\n", dev->name);} else {printf("not found for dev %s\n", dev->name);}Is "u-boot,mux-autoprobe" a required property? The fact that its in DT makes me think its optional. If that's the case, above printf() should be reduced to debug() to avoid confusion
Thanks Vignesh. It was for debug and forgot to remove it.
- }
+}
- UCLASS_DRIVER(mux) = { .id = UCLASS_MUX, .name = "mux",
diff --git a/include/mux.h b/include/mux.h index 060f71a47c..2467723951 100644 --- a/include/mux.h +++ b/include/mux.h @@ -75,6 +75,8 @@ void mux_control_put(struct mux_control *mux);
struct mux_control *devm_mux_control_get(struct udevice *dev, const char *mux_name); +void dm_mux_init(void);
- #else unsigned int mux_control_states(struct mux_control *mux) {
Hi Jean-Jacques,
On Tue, 5 Nov 2019 at 04:50, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
This will probe the multiplexer devices that have a "u-boot,mux-autoprobe" property. As a consequence they will be put in their idle state.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2:
- insert the mux initialization in init_sequence_r[], just before the
console is initialized as its serial port may be muxed
- moved the definition of dm_mux_init() in this commit
common/board_r.c | 16 ++++++++++++++++ drivers/mux/mux-uclass.c | 22 ++++++++++++++++++++++ include/mux.h | 2 ++ 3 files changed, 40 insertions(+)
diff --git a/common/board_r.c b/common/board_r.c index c1ecb06b74..3d410f3504 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -32,6 +32,7 @@ #include <miiphy.h> #endif #include <mmc.h> +#include <mux.h> #include <nand.h> #include <of_live.h> #include <onenand_uboot.h> @@ -178,6 +179,18 @@ static int initr_serial(void) return 0; }
+#if CONFIG_MULTIPLEXER +static int initr_mux(void) +{
/** Initialize the multiplexer controls to their default state.* This must be done early as other drivers may unknowingly rely on it.*/dm_mux_init();
Needs to get an error code and at least log_debug() it here, even if it continues.
return 0;+}
Can you rebase on x86/next? It has a initr_dm_devices() function which you can add this to.
+#endif
#if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_MIPS) static int initr_trap(void) { @@ -691,6 +704,9 @@ static init_fnc_t init_sequence_r[] = { #endif #ifdef CONFIG_EFI_LOADER efi_memory_init, +#endif +#if CONFIG_MULTIPLEXER
initr_mux,#endif stdio_init_tables, initr_serial, diff --git a/drivers/mux/mux-uclass.c b/drivers/mux/mux-uclass.c index 6aaf4dc964..71392e9e50 100644 --- a/drivers/mux/mux-uclass.c +++ b/drivers/mux/mux-uclass.c @@ -262,6 +262,28 @@ int mux_uclass_post_probe(struct udevice *dev) return 0; }
+void dm_mux_init(void) +{
struct uclass *uc;struct udevice *dev;int ret;ret = uclass_get(UCLASS_MUX, &uc);if (ret < 0) {debug("unable to get MUX uclass\n");return;
return ret
This should be a fatal error.
}uclass_foreach_dev(dev, uc) {if (dev_read_bool(dev, "u-boot,mux-autoprobe")) {ret = device_probe(dev);if (ret)debug("unable to probe device %s\n", dev->name);
Doesn't this need to be reported to the caller?
} else {printf("not found for dev %s\n", dev->name);
What does this mean? If autoprobe is off we can't find the device? I suggest changing the message, and debug() as Vignesh suggests.
}}+}
UCLASS_DRIVER(mux) = { .id = UCLASS_MUX, .name = "mux", diff --git a/include/mux.h b/include/mux.h index 060f71a47c..2467723951 100644 --- a/include/mux.h +++ b/include/mux.h @@ -75,6 +75,8 @@ void mux_control_put(struct mux_control *mux);
struct mux_control *devm_mux_control_get(struct udevice *dev, const char *mux_name); +void dm_mux_init(void);
Function comments again.
#else unsigned int mux_control_states(struct mux_control *mux) { -- 2.17.1
Regards, Simon
This adds a driver for mmio-based syscon multiplexers controlled by bitfields in a syscon register range. This is heavily based on the linux mmio-mux driver.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com ---
Changes in v2: None
drivers/mux/Kconfig | 15 +++++ drivers/mux/Makefile | 1 + drivers/mux/mmio.c | 155 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 drivers/mux/mmio.c
diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig index ad0199c058..bda6a2d9f5 100644 --- a/drivers/mux/Kconfig +++ b/drivers/mux/Kconfig @@ -4,4 +4,19 @@ config MULTIPLEXER bool "Multiplexer Support" depends on DM
+ +if MULTIPLEXER + +config MUX_MMIO + bool "MMIO register bitfield-controlled Multiplexer" + depends on MULTIPLEXER && SYSCON + help + MMIO register bitfield-controlled Multiplexer controller. + + The driver builds multiplexer controllers for bitfields in a syscon + register. For N bit wide bitfields, there will be 2^N possible + multiplexer states. + +endif + endmenu diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile index 351e4363d3..78ebf04c7a 100644 --- a/drivers/mux/Makefile +++ b/drivers/mux/Makefile @@ -4,3 +4,4 @@ # Jean-Jacques Hiblot jjhiblot@ti.com
obj-$(CONFIG_$(SPL_)MULTIPLEXER) += mux-uclass.o +obj-$(CONFIG_$(SPL_)MUX_MMIO) += mmio.o diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c new file mode 100644 index 0000000000..a9faaeb9fd --- /dev/null +++ b/drivers/mux/mmio.c @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * MMIO register bitfield-controlled multiplexer driver + * Based on the linux mmio multiplexer driver + * + * Copyright (C) 2017 Pengutronix, Philipp Zabel kernel@pengutronix.de + * Copyright (C) 2019 Texas Instrument, Jean-jacques Hiblot jjhiblot@ti.com + */ +#include <common.h> +#include <dm.h> +#include <mux-internal.h> +#include <regmap.h> +#include <syscon.h> +#include <dm/device.h> +#include <dm/read.h> +#include <dt-bindings/mux/mux.h> + +static int mux_mmio_set(struct mux_control *mux, int state) +{ + struct regmap_field **fields = dev_get_priv(mux->dev); + + return regmap_field_write(fields[mux_control_get_index(mux)], state); +} + +static const struct mux_control_ops mux_mmio_ops = { + .set = mux_mmio_set, +}; + +static const struct udevice_id mmio_mux_of_match[] = { + { .compatible = "mmio-mux" }, + { /* sentinel */ }, +}; + +static int mmio_mux_probe(struct udevice *dev) +{ + struct regmap_field **fields; + struct mux_chip *mux_chip = dev_get_uclass_priv(dev); + struct regmap *regmap; + u32 *mux_reg_masks; + u32 *idle_states; + int num_fields; + int ret; + int i; + + regmap = syscon_node_to_regmap(dev_ofnode(dev->parent)); + if (IS_ERR(regmap)) { + ret = PTR_ERR(regmap); + dev_err(dev, "failed to get regmap: %d\n", ret); + return ret; + } + + num_fields = dev_read_size(dev, "mux-reg-masks"); + if (num_fields < 0) { + dev_err(dev, "mux-reg-masks property missing or invalid: %d\n", + num_fields); + return num_fields; + } + num_fields /= sizeof(u32); + if (num_fields == 0 || num_fields % 2) + ret = -EINVAL; + num_fields = num_fields / 2; + + ret = mux_alloc_controllers(dev, num_fields); + if (ret < 0) { + dev_err(dev, "failed to allocate mux controllers: %d\n", + ret); + return ret; + } + + fields = devm_kmalloc(dev, num_fields * sizeof(*fields), __GFP_ZERO); + if (!fields) + return -ENOMEM; + dev->priv = fields; + + mux_reg_masks = devm_kmalloc(dev, num_fields * 2 * sizeof(u32), + __GFP_ZERO); + if (!mux_reg_masks) + return -ENOMEM; + + ret = dev_read_u32_array(dev, "mux-reg-masks", mux_reg_masks, + num_fields * 2); + if (ret < 0) { + dev_err(dev, "failed to read mux-reg-masks property: %d\n", + ret); + return ret; + } + + idle_states = devm_kmalloc(dev, num_fields * sizeof(u32), __GFP_ZERO); + if (!idle_states) + return -ENOMEM; + + ret = dev_read_u32_array(dev, "idle-states", idle_states, num_fields); + if (ret < 0) { + dev_err(dev, "failed to read idle-states property: %d\n", + ret); + devm_kfree(dev, idle_states); + idle_states = NULL; + } + + for (i = 0; i < num_fields; i++) { + struct mux_control *mux = &mux_chip->mux[i]; + struct reg_field field; + u32 reg, mask; + int bits; + + reg = mux_reg_masks[2 * i]; + mask = mux_reg_masks[2 * i + 1]; + + field.reg = reg; + field.msb = fls(mask) - 1; + field.lsb = ffs(mask) - 1; + + if (mask != GENMASK(field.msb, field.lsb)) { + dev_err(dev, "bitfield %d: invalid mask 0x%x\n", + i, mask); + return -EINVAL; + } + + fields[i] = devm_regmap_field_alloc(dev, regmap, field); + if (IS_ERR(fields[i])) { + ret = PTR_ERR(fields[i]); + dev_err(dev, "bitfield %d: failed allocate: %d\n", + i, ret); + return ret; + } + + bits = 1 + field.msb - field.lsb; + mux->states = 1 << bits; + + if (!idle_states) + continue; + + if (idle_states[i] != MUX_IDLE_AS_IS && + idle_states[i] >= mux->states) { + dev_err(dev, "bitfield: %d: out of range idle state %d\n", + i, idle_states[i]); + return -EINVAL; + } + mux->idle_state = idle_states[i]; + } + + devm_kfree(dev, mux_reg_masks); + if (idle_states) + devm_kfree(dev, idle_states); + + return 0; +} + +U_BOOT_DRIVER(mmio_mux) = { + .name = "mmio-mux", + .id = UCLASS_MUX, + .of_match = mmio_mux_of_match, + .probe = mmio_mux_probe, + .ops = &mux_mmio_ops, +};
Hi JJ,
On 11/5/2019 12:50 PM, Jean-Jacques Hiblot wrote:
This adds a driver for mmio-based syscon multiplexers controlled by bitfields in a syscon register range. This is heavily based on the linux mmio-mux driver.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2: None
drivers/mux/Kconfig | 15 +++++ drivers/mux/Makefile | 1 + drivers/mux/mmio.c | 155 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 drivers/mux/mmio.c
do you plan to add support for reg-mux too, for parent devices which are not syscon?
Thanks! Alex
On 08/11/2019 13:16, Alexandru Marginean wrote:
Hi JJ,
On 11/5/2019 12:50 PM, Jean-Jacques Hiblot wrote:
This adds a driver for mmio-based syscon multiplexers controlled by bitfields in a syscon register range. This is heavily based on the linux mmio-mux driver.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2: None
drivers/mux/Kconfig | 15 +++++ drivers/mux/Makefile | 1 + drivers/mux/mmio.c | 155 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 drivers/mux/mmio.c
do you plan to add support for reg-mux too, for parent devices which are not syscon?
I haven't planned to work on it. I kept the framework close to the linux version, so porting mux drivers from linux shouldn't be hard.
JJ
Thanks! Alex
Hi Jean-Jacques,
On Tue, 5 Nov 2019 at 04:50, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
This adds a driver for mmio-based syscon multiplexers controlled by bitfields in a syscon register range. This is heavily based on the linux mmio-mux driver.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2: None
drivers/mux/Kconfig | 15 +++++ drivers/mux/Makefile | 1 + drivers/mux/mmio.c | 155 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 drivers/mux/mmio.c
Reviewed-by: Simon Glass sjg@chromium.org
Nits below.
diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig index ad0199c058..bda6a2d9f5 100644 --- a/drivers/mux/Kconfig +++ b/drivers/mux/Kconfig @@ -4,4 +4,19 @@ config MULTIPLEXER bool "Multiplexer Support" depends on DM
+if MULTIPLEXER
+config MUX_MMIO
bool "MMIO register bitfield-controlled Multiplexer"depends on MULTIPLEXER && SYSCONhelpMMIO register bitfield-controlled Multiplexer controller.The driver builds multiplexer controllers for bitfields in a sysconregister. For N bit wide bitfields, there will be 2^N possiblemultiplexer states.+endif
endmenu diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile index 351e4363d3..78ebf04c7a 100644 --- a/drivers/mux/Makefile +++ b/drivers/mux/Makefile @@ -4,3 +4,4 @@ # Jean-Jacques Hiblot jjhiblot@ti.com
obj-$(CONFIG_$(SPL_)MULTIPLEXER) += mux-uclass.o +obj-$(CONFIG_$(SPL_)MUX_MMIO) += mmio.o diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c new file mode 100644 index 0000000000..a9faaeb9fd --- /dev/null +++ b/drivers/mux/mmio.c @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- MMIO register bitfield-controlled multiplexer driver
- Based on the linux mmio multiplexer driver
- Copyright (C) 2017 Pengutronix, Philipp Zabel kernel@pengutronix.de
- Copyright (C) 2019 Texas Instrument, Jean-jacques Hiblot jjhiblot@ti.com
- */
+#include <common.h> +#include <dm.h> +#include <mux-internal.h> +#include <regmap.h> +#include <syscon.h> +#include <dm/device.h> +#include <dm/read.h> +#include <dt-bindings/mux/mux.h>
+static int mux_mmio_set(struct mux_control *mux, int state) +{
struct regmap_field **fields = dev_get_priv(mux->dev);return regmap_field_write(fields[mux_control_get_index(mux)], state);+}
+static const struct mux_control_ops mux_mmio_ops = {
.set = mux_mmio_set,+};
+static const struct udevice_id mmio_mux_of_match[] = {
{ .compatible = "mmio-mux" },{ /* sentinel */ },+};
+static int mmio_mux_probe(struct udevice *dev) +{
struct regmap_field **fields;struct mux_chip *mux_chip = dev_get_uclass_priv(dev);struct regmap *regmap;u32 *mux_reg_masks;u32 *idle_states;int num_fields;int ret;int i;regmap = syscon_node_to_regmap(dev_ofnode(dev->parent));if (IS_ERR(regmap)) {ret = PTR_ERR(regmap);dev_err(dev, "failed to get regmap: %d\n", ret);return ret;}num_fields = dev_read_size(dev, "mux-reg-masks");if (num_fields < 0) {dev_err(dev, "mux-reg-masks property missing or invalid: %d\n",num_fields);
These seem like errors that should not happen in a real system.
You could use
return log_msg_ret("mux-reg-masks" ,-EINVAL)
which reduces to nothing unless CONFIG_LOG_ERROR_RETURN is set.
That gives a nice backtrace of all errors that happened with a little string tag to explain each one.
return num_fields;}num_fields /= sizeof(u32);if (num_fields == 0 || num_fields % 2)ret = -EINVAL;num_fields = num_fields / 2;ret = mux_alloc_controllers(dev, num_fields);if (ret < 0) {dev_err(dev, "failed to allocate mux controllers: %d\n",ret);
Same here and below. Seems like a waste to include this string when we are just out of memory.
return ret;}fields = devm_kmalloc(dev, num_fields * sizeof(*fields), __GFP_ZERO);if (!fields)return -ENOMEM;dev->priv = fields;mux_reg_masks = devm_kmalloc(dev, num_fields * 2 * sizeof(u32),__GFP_ZERO);if (!mux_reg_masks)return -ENOMEM;ret = dev_read_u32_array(dev, "mux-reg-masks", mux_reg_masks,num_fields * 2);if (ret < 0) {dev_err(dev, "failed to read mux-reg-masks property: %d\n",ret);return ret;}idle_states = devm_kmalloc(dev, num_fields * sizeof(u32), __GFP_ZERO);if (!idle_states)return -ENOMEM;ret = dev_read_u32_array(dev, "idle-states", idle_states, num_fields);if (ret < 0) {dev_err(dev, "failed to read idle-states property: %d\n",ret);devm_kfree(dev, idle_states);idle_states = NULL;}for (i = 0; i < num_fields; i++) {struct mux_control *mux = &mux_chip->mux[i];struct reg_field field;u32 reg, mask;int bits;reg = mux_reg_masks[2 * i];mask = mux_reg_masks[2 * i + 1];field.reg = reg;field.msb = fls(mask) - 1;field.lsb = ffs(mask) - 1;if (mask != GENMASK(field.msb, field.lsb)) {dev_err(dev, "bitfield %d: invalid mask 0x%x\n",i, mask);return -EINVAL;}fields[i] = devm_regmap_field_alloc(dev, regmap, field);if (IS_ERR(fields[i])) {ret = PTR_ERR(fields[i]);dev_err(dev, "bitfield %d: failed allocate: %d\n",i, ret);return ret;}bits = 1 + field.msb - field.lsb;mux->states = 1 << bits;if (!idle_states)continue;if (idle_states[i] != MUX_IDLE_AS_IS &&idle_states[i] >= mux->states) {dev_err(dev, "bitfield: %d: out of range idle state %d\n",i, idle_states[i]);return -EINVAL;}mux->idle_state = idle_states[i];}devm_kfree(dev, mux_reg_masks);if (idle_states)devm_kfree(dev, idle_states);return 0;+}
+U_BOOT_DRIVER(mmio_mux) = {
.name = "mmio-mux",.id = UCLASS_MUX,.of_match = mmio_mux_of_match,.probe = mmio_mux_probe,.ops = &mux_mmio_ops,+};
2.17.1
Regards, Simon
Provide tests to check the behavior of the multiplexer framework. The test uses a mmio-based multiplexer.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
---
Changes in v2: - Call sandbox_set_enable_memio(true) before running the test
arch/sandbox/dts/test.dts | 26 +++++++ configs/sandbox_defconfig | 2 + test/dm/Makefile | 1 + test/dm/mux-mmio.c | 147 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+) create mode 100644 test/dm/mux-mmio.c
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index aa9eaec338..3224a8389c 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -1,3 +1,5 @@ +#include <dt-bindings/mux/mux.h> + /dts-v1/;
/ { @@ -93,6 +95,11 @@ <&gpio_b 9 0xc 3 2 1>; int-value = <1234>; uint-value = <(-1234)>; + + mux-controls = <&muxcontroller0 0>, <&muxcontroller0 1>, + <&muxcontroller0 2>, <&muxcontroller0 3>; + mux-control-names = "mux0", "mux1", "mux2", "mux3"; + mux-syscon = <&syscon3>; };
junk { @@ -129,6 +136,9 @@ compatible = "denx,u-boot-fdt-test"; ping-expect = <3>; ping-add = <3>; + + mux-controls = <&muxcontroller0 0>; + mux-control-names = "mux0"; };
phy_provider0: gen_phy@0 { @@ -665,6 +675,22 @@ 0x58 8>; };
+ syscon3: syscon@3 { + compatible = "simple-mfd", "syscon"; + reg = <0x000100 0x10>; + + muxcontroller0: a-mux-controller { + compatible = "mmio-mux"; + #mux-control-cells = <1>; + + mux-reg-masks = <0x0 0x30>, /* 0: reg 0x0, bits 5:4 */ + <0x3 0x1E>, /* 1: reg 0x3, bits 4:1 */ + <0x1 0xFF>; /* 2: reg 0x1, bits 7:0 */ + idle-states = <MUX_IDLE_AS_IS>, <0x02>, <0x73>; + u-boot,mux-autoprobe; + }; + }; + timer { compatible = "sandbox,timer"; clock-frequency = <1000000>; diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 20ebc68997..2822dd9c74 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -144,6 +144,8 @@ CONFIG_SPI_FLASH_SPANSION=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_SST=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_MULTIPLEXER=y +CONFIG_MUX_MMIO=y CONFIG_DM_ETH=y CONFIG_NVME=y CONFIG_PCI=y diff --git a/test/dm/Makefile b/test/dm/Makefile index 0c2fd5cb5e..a3fc23e527 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_DM_SPI_FLASH) += sf.o obj-$(CONFIG_SMEM) += smem.o obj-$(CONFIG_DM_SPI) += spi.o obj-y += syscon.o +obj-$(CONFIG_MUX_MMIO) += mux-mmio.o obj-$(CONFIG_DM_USB) += usb.o obj-$(CONFIG_DM_PMIC) += pmic.o obj-$(CONFIG_DM_REGULATOR) += regulator.o diff --git a/test/dm/mux-mmio.c b/test/dm/mux-mmio.c new file mode 100644 index 0000000000..a3dfd34120 --- /dev/null +++ b/test/dm/mux-mmio.c @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/ + * Jean-Jacques Hiblot jjhiblot@ti.com + */ + +#include <common.h> +#include <fdtdec.h> +#include <dm.h> +#include <mux.h> +#include <regmap.h> +#include <syscon.h> +#include <asm/test.h> +#include <dm/root.h> +#include <dm/test.h> +#include <dm/util.h> +#include <test/ut.h> + +/* Test that mmio mux work correctly */ +static int dm_test_mux_mmio(struct unit_test_state *uts) +{ + struct udevice *dev, *dev_b; + struct regmap *map; + struct mux_control *ctl0_a, *ctl0_b; + struct mux_control *ctl1; + struct mux_control *ctl_err; + u32 val; + int i; + + sandbox_set_enable_memio(true); + + ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev)); + ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 1, &dev_b)); + ut_asserteq_str("a-test", dev->name); + ut_asserteq_str("b-test", dev_b->name); + map = syscon_regmap_lookup_by_phandle(dev, "mux-syscon"); + ut_assert(!IS_ERR(map)); + ut_assert(map); + + /* check default states */ + ut_assertok(regmap_read(map, 3, &val)); + ut_asserteq(0x02, (val & 0x1E) >> 1); + ut_assertok(regmap_read(map, 1, &val)); + ut_asserteq(0x73, (val & 0xFF) >> 0); + + ut_assertok(mux_control_get(dev, "mux0", &ctl0_a)); + ut_assertok(mux_control_get(dev, "mux1", &ctl1)); + ut_asserteq(-ERANGE, mux_control_get(dev, "mux3", &ctl_err)); + ut_asserteq(-ENODATA, mux_control_get(dev, "dummy", &ctl_err)); + ut_assertok(mux_control_get(dev_b, "mux0", &ctl0_b)); + + for (i = 0; i < mux_control_states(ctl0_a); i++) { + /* select a new state and verify the value in the regmap */ + ut_assertok(mux_control_select(ctl0_a, i)); + ut_assertok(regmap_read(map, 0, &val)); + ut_asserteq(i, (val & 0x30) >> 4); + /* + * deselect the mux and verify that the value in the regmap + * reflects the idle state (fixed to MUX_IDLE_AS_IS) + */ + ut_assertok(mux_control_deselect(ctl0_a)); + ut_assertok(regmap_read(map, 0, &val)); + ut_asserteq(i, (val & 0x30) >> 4); + } + + for (i = 0; i < mux_control_states(ctl1); i++) { + /* select a new state and verify the value in the regmap */ + ut_assertok(mux_control_select(ctl1, i)); + ut_assertok(regmap_read(map, 3, &val)); + ut_asserteq(i, (val & 0x1E) >> 1); + /* + * deselect the mux and verify that the value in the regmap + * reflects the idle state (fixed to 2) + */ + ut_assertok(mux_control_deselect(ctl1)); + ut_assertok(regmap_read(map, 3, &val)); + ut_asserteq(2, (val & 0x1E) >> 1); + } + + // try unbalanced selection/deselection + ut_assertok(mux_control_select(ctl0_a, 0)); + ut_asserteq(-EBUSY, mux_control_select(ctl0_a, 1)); + ut_asserteq(-EBUSY, mux_control_select(ctl0_a, 0)); + ut_assertok(mux_control_deselect(ctl0_a)); + + // try concurent selection + ut_assertok(mux_control_select(ctl0_a, 0)); + ut_assert(mux_control_select(ctl0_b, 0)); + ut_assertok(mux_control_deselect(ctl0_a)); + ut_assertok(mux_control_select(ctl0_b, 0)); + ut_assert(mux_control_select(ctl0_a, 0)); + ut_assertok(mux_control_deselect(ctl0_b)); + ut_assertok(mux_control_select(ctl0_a, 0)); + ut_assertok(mux_control_deselect(ctl0_a)); + + return 0; +} +DM_TEST(dm_test_mux_mmio, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +/* Test that managed API for mux work correctly */ +static int dm_test_devm_mux_mmio(struct unit_test_state *uts) +{ + struct udevice *dev, *dev_b; + struct mux_control *ctl0_a, *ctl0_b; + struct mux_control *ctl1; + struct mux_control *ctl_err; + + sandbox_set_enable_memio(true); + + ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev)); + ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 1, &dev_b)); + ut_asserteq_str("a-test", dev->name); + ut_asserteq_str("b-test", dev_b->name); + + ctl0_a = devm_mux_control_get(dev, "mux0"); + ut_assertok(IS_ERR(ctl0_a)); + ut_assert(ctl0_a); + ctl1 = devm_mux_control_get(dev, "mux1"); + ut_assertok(IS_ERR(ctl1)); + ut_assert(ctl1); + ctl_err = devm_mux_control_get(dev, "mux3"); + ut_asserteq(-ERANGE, PTR_ERR(ctl_err)); + ctl_err = devm_mux_control_get(dev, "dummy"); + ut_asserteq(-ENODATA, PTR_ERR(ctl_err)); + + ctl0_b = devm_mux_control_get(dev_b, "mux0"); + ut_assertok(IS_ERR(ctl0_b)); + ut_assert(ctl0_b); + + /* try concurent selection */ + ut_assertok(mux_control_select(ctl0_a, 0)); + ut_assert(mux_control_select(ctl0_b, 0)); + ut_assertok(mux_control_deselect(ctl0_a)); + ut_assertok(mux_control_select(ctl0_b, 0)); + ut_assert(mux_control_select(ctl0_a, 0)); + ut_assertok(mux_control_deselect(ctl0_b)); + + /* removed one device and check that the mux is released */ + ut_assertok(mux_control_select(ctl0_a, 0)); + ut_assert(mux_control_select(ctl0_b, 0)); + device_remove(dev, DM_REMOVE_NORMAL); + ut_assertok(mux_control_select(ctl0_b, 0)); + + device_remove(dev_b, DM_REMOVE_NORMAL); + return 0; +} +DM_TEST(dm_test_devm_mux_mmio, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
Hi Jean-Jacques,
On Tue, 5 Nov 2019 at 04:50, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Provide tests to check the behavior of the multiplexer framework. The test uses a mmio-based multiplexer.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2:
- Call sandbox_set_enable_memio(true) before running the test
arch/sandbox/dts/test.dts | 26 +++++++ configs/sandbox_defconfig | 2 + test/dm/Makefile | 1 + test/dm/mux-mmio.c | 147 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 176 insertions(+) create mode 100644 test/dm/mux-mmio.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index aa9eaec338..3224a8389c 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -1,3 +1,5 @@ +#include <dt-bindings/mux/mux.h>
/dts-v1/;
/ { @@ -93,6 +95,11 @@ <&gpio_b 9 0xc 3 2 1>; int-value = <1234>; uint-value = <(-1234)>;
mux-controls = <&muxcontroller0 0>, <&muxcontroller0 1>,<&muxcontroller0 2>, <&muxcontroller0 3>;mux-control-names = "mux0", "mux1", "mux2", "mux3";mux-syscon = <&syscon3>; }; junk {@@ -129,6 +136,9 @@ compatible = "denx,u-boot-fdt-test"; ping-expect = <3>; ping-add = <3>;
mux-controls = <&muxcontroller0 0>;mux-control-names = "mux0"; }; phy_provider0: gen_phy@0 {@@ -665,6 +675,22 @@ 0x58 8>; };
syscon3: syscon@3 {compatible = "simple-mfd", "syscon";reg = <0x000100 0x10>;muxcontroller0: a-mux-controller {compatible = "mmio-mux";#mux-control-cells = <1>;mux-reg-masks = <0x0 0x30>, /* 0: reg 0x0, bits 5:4 */<0x3 0x1E>, /* 1: reg 0x3, bits 4:1 */<0x1 0xFF>; /* 2: reg 0x1, bits 7:0 */idle-states = <MUX_IDLE_AS_IS>, <0x02>, <0x73>;u-boot,mux-autoprobe;};};timer { compatible = "sandbox,timer"; clock-frequency = <1000000>;diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 20ebc68997..2822dd9c74 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -144,6 +144,8 @@ CONFIG_SPI_FLASH_SPANSION=y CONFIG_SPI_FLASH_STMICRO=y CONFIG_SPI_FLASH_SST=y CONFIG_SPI_FLASH_WINBOND=y +CONFIG_MULTIPLEXER=y +CONFIG_MUX_MMIO=y CONFIG_DM_ETH=y CONFIG_NVME=y CONFIG_PCI=y diff --git a/test/dm/Makefile b/test/dm/Makefile index 0c2fd5cb5e..a3fc23e527 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_DM_SPI_FLASH) += sf.o obj-$(CONFIG_SMEM) += smem.o obj-$(CONFIG_DM_SPI) += spi.o obj-y += syscon.o +obj-$(CONFIG_MUX_MMIO) += mux-mmio.o obj-$(CONFIG_DM_USB) += usb.o obj-$(CONFIG_DM_PMIC) += pmic.o obj-$(CONFIG_DM_REGULATOR) += regulator.o diff --git a/test/dm/mux-mmio.c b/test/dm/mux-mmio.c new file mode 100644 index 0000000000..a3dfd34120 --- /dev/null +++ b/test/dm/mux-mmio.c @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/
- Jean-Jacques Hiblot jjhiblot@ti.com
- */
+#include <common.h> +#include <fdtdec.h> +#include <dm.h> +#include <mux.h> +#include <regmap.h> +#include <syscon.h> +#include <asm/test.h> +#include <dm/root.h> +#include <dm/test.h> +#include <dm/util.h> +#include <test/ut.h>
Missing header file for device_remove() here.
+/* Test that mmio mux work correctly */ +static int dm_test_mux_mmio(struct unit_test_state *uts) +{
struct udevice *dev, *dev_b;struct regmap *map;struct mux_control *ctl0_a, *ctl0_b;struct mux_control *ctl1;struct mux_control *ctl_err;u32 val;int i;sandbox_set_enable_memio(true);ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 1, &dev_b));ut_asserteq_str("a-test", dev->name);ut_asserteq_str("b-test", dev_b->name);map = syscon_regmap_lookup_by_phandle(dev, "mux-syscon");ut_assert(!IS_ERR(map));ut_assert(map);/* check default states */ut_assertok(regmap_read(map, 3, &val));ut_asserteq(0x02, (val & 0x1E) >> 1);ut_assertok(regmap_read(map, 1, &val));ut_asserteq(0x73, (val & 0xFF) >> 0);ut_assertok(mux_control_get(dev, "mux0", &ctl0_a));ut_assertok(mux_control_get(dev, "mux1", &ctl1));ut_asserteq(-ERANGE, mux_control_get(dev, "mux3", &ctl_err));ut_asserteq(-ENODATA, mux_control_get(dev, "dummy", &ctl_err));ut_assertok(mux_control_get(dev_b, "mux0", &ctl0_b));for (i = 0; i < mux_control_states(ctl0_a); i++) {/* select a new state and verify the value in the regmap */ut_assertok(mux_control_select(ctl0_a, i));ut_assertok(regmap_read(map, 0, &val));ut_asserteq(i, (val & 0x30) >> 4);/** deselect the mux and verify that the value in the regmap* reflects the idle state (fixed to MUX_IDLE_AS_IS)*/ut_assertok(mux_control_deselect(ctl0_a));ut_assertok(regmap_read(map, 0, &val));ut_asserteq(i, (val & 0x30) >> 4);}
Tests should be short and targeted. Could this test be split up a bit>
for (i = 0; i < mux_control_states(ctl1); i++) {/* select a new state and verify the value in the regmap */ut_assertok(mux_control_select(ctl1, i));ut_assertok(regmap_read(map, 3, &val));ut_asserteq(i, (val & 0x1E) >> 1);/** deselect the mux and verify that the value in the regmap* reflects the idle state (fixed to 2)*/ut_assertok(mux_control_deselect(ctl1));ut_assertok(regmap_read(map, 3, &val));ut_asserteq(2, (val & 0x1E) >> 1);}// try unbalanced selection/deselectionut_assertok(mux_control_select(ctl0_a, 0));ut_asserteq(-EBUSY, mux_control_select(ctl0_a, 1));ut_asserteq(-EBUSY, mux_control_select(ctl0_a, 0));ut_assertok(mux_control_deselect(ctl0_a));// try concurent selection
concurrent
Please use C comment style.
ut_assertok(mux_control_select(ctl0_a, 0));ut_assert(mux_control_select(ctl0_b, 0));ut_assertok(mux_control_deselect(ctl0_a));ut_assertok(mux_control_select(ctl0_b, 0));ut_assert(mux_control_select(ctl0_a, 0));ut_assertok(mux_control_deselect(ctl0_b));ut_assertok(mux_control_select(ctl0_a, 0));ut_assertok(mux_control_deselect(ctl0_a));return 0;+} +DM_TEST(dm_test_mux_mmio, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+/* Test that managed API for mux work correctly */ +static int dm_test_devm_mux_mmio(struct unit_test_state *uts) +{
struct udevice *dev, *dev_b;struct mux_control *ctl0_a, *ctl0_b;struct mux_control *ctl1;struct mux_control *ctl_err;sandbox_set_enable_memio(true);ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 1, &dev_b));ut_asserteq_str("a-test", dev->name);ut_asserteq_str("b-test", dev_b->name);ctl0_a = devm_mux_control_get(dev, "mux0");ut_assertok(IS_ERR(ctl0_a));
Probably need to define ut_assert_not_err() or similar.
ut_assert(ctl0_a);ctl1 = devm_mux_control_get(dev, "mux1");ut_assertok(IS_ERR(ctl1));ut_assert(ctl1);ctl_err = devm_mux_control_get(dev, "mux3");ut_asserteq(-ERANGE, PTR_ERR(ctl_err));ctl_err = devm_mux_control_get(dev, "dummy");ut_asserteq(-ENODATA, PTR_ERR(ctl_err));ctl0_b = devm_mux_control_get(dev_b, "mux0");ut_assertok(IS_ERR(ctl0_b));ut_assert(ctl0_b);/* try concurent selection */
spelling again
ut_assertok(mux_control_select(ctl0_a, 0));ut_assert(mux_control_select(ctl0_b, 0));ut_assertok(mux_control_deselect(ctl0_a));ut_assertok(mux_control_select(ctl0_b, 0));ut_assert(mux_control_select(ctl0_a, 0));ut_assertok(mux_control_deselect(ctl0_b));/* removed one device and check that the mux is released */ut_assertok(mux_control_select(ctl0_a, 0));ut_assert(mux_control_select(ctl0_b, 0));device_remove(dev, DM_REMOVE_NORMAL);ut_assertok(mux_control_select(ctl0_b, 0));device_remove(dev_b, DM_REMOVE_NORMAL);return 0;+}
+DM_TEST(dm_test_devm_mux_mmio, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
2.17.1
Regards, Simon
Hi Jean-Jacques,
On Tue, 24 Dec 2019 at 08:58, Simon Glass sjg@chromium.org wrote:
Hi Jean-Jacques,
On Tue, 5 Nov 2019 at 04:50, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Provide tests to check the behavior of the multiplexer framework. The test uses a mmio-based multiplexer.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2:
- Call sandbox_set_enable_memio(true) before running the test
One more thing...I think this multiplexer thing need some documentation and an example (e.g. using sandbox).
Also we should have a mux command to list and control muxes.
Regards, Simon
participants (4)
-
Alexandru Marginean -
Jean-Jacques Hiblot -
Simon Glass -
Vignesh Raghavendra