* [U-Boot] [PATCH v2 0/4] drivers: Add a framework for MUX drivers
@ 2019-11-05 11:50 Jean-Jacques Hiblot
2019-11-05 11:50 ` [U-Boot] [PATCH v2 1/4] drivers: Add a new framework for multiplexer devices Jean-Jacques Hiblot
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Jean-Jacques Hiblot @ 2019-11-05 11:50 UTC (permalink / raw)
To: u-boot
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
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v2 1/4] drivers: Add a new framework for multiplexer devices
2019-11-05 11:50 [U-Boot] [PATCH v2 0/4] drivers: Add a framework for MUX drivers Jean-Jacques Hiblot
@ 2019-11-05 11:50 ` Jean-Jacques Hiblot
2019-12-24 15:58 ` Simon Glass
2019-11-05 11:50 ` [U-Boot] [PATCH v2 2/4] dm: board: complete the initialization of the muxes in initr_dm() Jean-Jacques Hiblot
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Jean-Jacques Hiblot @ 2019-11-05 11:50 UTC (permalink / raw)
To: u-boot
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
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v2 2/4] dm: board: complete the initialization of the muxes in initr_dm()
2019-11-05 11:50 [U-Boot] [PATCH v2 0/4] drivers: Add a framework for MUX drivers Jean-Jacques Hiblot
2019-11-05 11:50 ` [U-Boot] [PATCH v2 1/4] drivers: Add a new framework for multiplexer devices Jean-Jacques Hiblot
@ 2019-11-05 11:50 ` Jean-Jacques Hiblot
2019-11-05 13:05 ` Vignesh Raghavendra
2019-12-24 15:58 ` Simon Glass
2019-11-05 11:50 ` [U-Boot] [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller Jean-Jacques Hiblot
2019-11-05 11:50 ` [U-Boot] [PATCH v2 4/4] test: Add tests for the multiplexer framework Jean-Jacques Hiblot
3 siblings, 2 replies; 14+ messages in thread
From: Jean-Jacques Hiblot @ 2019-11-05 11:50 UTC (permalink / raw)
To: u-boot
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)
{
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller
2019-11-05 11:50 [U-Boot] [PATCH v2 0/4] drivers: Add a framework for MUX drivers Jean-Jacques Hiblot
2019-11-05 11:50 ` [U-Boot] [PATCH v2 1/4] drivers: Add a new framework for multiplexer devices Jean-Jacques Hiblot
2019-11-05 11:50 ` [U-Boot] [PATCH v2 2/4] dm: board: complete the initialization of the muxes in initr_dm() Jean-Jacques Hiblot
@ 2019-11-05 11:50 ` Jean-Jacques Hiblot
2019-11-08 12:16 ` Alexandru Marginean
2019-12-24 15:58 ` Simon Glass
2019-11-05 11:50 ` [U-Boot] [PATCH v2 4/4] test: Add tests for the multiplexer framework Jean-Jacques Hiblot
3 siblings, 2 replies; 14+ messages in thread
From: Jean-Jacques Hiblot @ 2019-11-05 11:50 UTC (permalink / raw)
To: u-boot
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,
+};
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v2 4/4] test: Add tests for the multiplexer framework
2019-11-05 11:50 [U-Boot] [PATCH v2 0/4] drivers: Add a framework for MUX drivers Jean-Jacques Hiblot
` (2 preceding siblings ...)
2019-11-05 11:50 ` [U-Boot] [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller Jean-Jacques Hiblot
@ 2019-11-05 11:50 ` Jean-Jacques Hiblot
2019-12-24 15:58 ` Simon Glass
3 siblings, 1 reply; 14+ messages in thread
From: Jean-Jacques Hiblot @ 2019-11-05 11:50 UTC (permalink / raw)
To: u-boot
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 at 0 {
@@ -665,6 +675,22 @@
0x58 8>;
};
+ syscon3: syscon at 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);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v2 2/4] dm: board: complete the initialization of the muxes in initr_dm()
2019-11-05 11:50 ` [U-Boot] [PATCH v2 2/4] dm: board: complete the initialization of the muxes in initr_dm() Jean-Jacques Hiblot
@ 2019-11-05 13:05 ` Vignesh Raghavendra
2019-11-05 15:13 ` Jean-Jacques Hiblot
2019-12-24 15:58 ` Simon Glass
1 sibling, 1 reply; 14+ messages in thread
From: Vignesh Raghavendra @ 2019-11-05 13:05 UTC (permalink / raw)
To: u-boot
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)
> {
>
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v2 2/4] dm: board: complete the initialization of the muxes in initr_dm()
2019-11-05 13:05 ` Vignesh Raghavendra
@ 2019-11-05 15:13 ` Jean-Jacques Hiblot
0 siblings, 0 replies; 14+ messages in thread
From: Jean-Jacques Hiblot @ 2019-11-05 15:13 UTC (permalink / raw)
To: u-boot
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)
>> {
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller
2019-11-05 11:50 ` [U-Boot] [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller Jean-Jacques Hiblot
@ 2019-11-08 12:16 ` Alexandru Marginean
2019-11-08 16:25 ` Jean-Jacques Hiblot
2019-12-24 15:58 ` Simon Glass
1 sibling, 1 reply; 14+ messages in thread
From: Alexandru Marginean @ 2019-11-08 12:16 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller
2019-11-08 12:16 ` Alexandru Marginean
@ 2019-11-08 16:25 ` Jean-Jacques Hiblot
0 siblings, 0 replies; 14+ messages in thread
From: Jean-Jacques Hiblot @ 2019-11-08 16:25 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] dm: board: complete the initialization of the muxes in initr_dm()
2019-11-05 11:50 ` [U-Boot] [PATCH v2 2/4] dm: board: complete the initialization of the muxes in initr_dm() Jean-Jacques Hiblot
2019-11-05 13:05 ` Vignesh Raghavendra
@ 2019-12-24 15:58 ` Simon Glass
1 sibling, 0 replies; 14+ messages in thread
From: Simon Glass @ 2019-12-24 15:58 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller
2019-11-05 11:50 ` [U-Boot] [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller Jean-Jacques Hiblot
2019-11-08 12:16 ` Alexandru Marginean
@ 2019-12-24 15:58 ` Simon Glass
1 sibling, 0 replies; 14+ messages in thread
From: Simon Glass @ 2019-12-24 15:58 UTC (permalink / raw)
To: u-boot
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 && 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);
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] test: Add tests for the multiplexer framework
2019-11-05 11:50 ` [U-Boot] [PATCH v2 4/4] test: Add tests for the multiplexer framework Jean-Jacques Hiblot
@ 2019-12-24 15:58 ` Simon Glass
2019-12-24 16:02 ` Simon Glass
0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2019-12-24 15:58 UTC (permalink / raw)
To: u-boot
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 at 0 {
> @@ -665,6 +675,22 @@
> 0x58 8>;
> };
>
> + syscon3: syscon at 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/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
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/4] drivers: Add a new framework for multiplexer devices
2019-11-05 11:50 ` [U-Boot] [PATCH v2 1/4] drivers: Add a new framework for multiplexer devices Jean-Jacques Hiblot
@ 2019-12-24 15:58 ` Simon Glass
0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2019-12-24 15:58 UTC (permalink / raw)
To: u-boot
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];
> + 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)
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);
> + 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)
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] test: Add tests for the multiplexer framework
2019-12-24 15:58 ` Simon Glass
@ 2019-12-24 16:02 ` Simon Glass
0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2019-12-24 16:02 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-12-24 16:02 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05 11:50 [U-Boot] [PATCH v2 0/4] drivers: Add a framework for MUX drivers Jean-Jacques Hiblot
2019-11-05 11:50 ` [U-Boot] [PATCH v2 1/4] drivers: Add a new framework for multiplexer devices Jean-Jacques Hiblot
2019-12-24 15:58 ` Simon Glass
2019-11-05 11:50 ` [U-Boot] [PATCH v2 2/4] dm: board: complete the initialization of the muxes in initr_dm() Jean-Jacques Hiblot
2019-11-05 13:05 ` Vignesh Raghavendra
2019-11-05 15:13 ` Jean-Jacques Hiblot
2019-12-24 15:58 ` Simon Glass
2019-11-05 11:50 ` [U-Boot] [PATCH v2 3/4] drivers: mux: mmio-based syscon mux controller Jean-Jacques Hiblot
2019-11-08 12:16 ` Alexandru Marginean
2019-11-08 16:25 ` Jean-Jacques Hiblot
2019-12-24 15:58 ` Simon Glass
2019-11-05 11:50 ` [U-Boot] [PATCH v2 4/4] test: Add tests for the multiplexer framework Jean-Jacques Hiblot
2019-12-24 15:58 ` Simon Glass
2019-12-24 16:02 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.