[U-Boot] [PATCH v1 1/4] drivers: Add a new framework for multiplexer devices
Jean-Jacques Hiblot
jjhiblot at ti.com
Mon Nov 4 17:53:15 UTC 2019
Hi Simon,
Thank for the review.
On 30/10/2019 02:48, Simon Glass wrote:
> Hi Jean-Jacques,
>
> On Wed, 2 Oct 2019 at 06:47, Jean-Jacques Hiblot <jjhiblot at 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 at ti.com>
>> ---
>>
>> drivers/Kconfig | 2 +
>> drivers/Makefile | 1 +
>> drivers/mux/Kconfig | 7 +
>> drivers/mux/Makefile | 6 +
>> drivers/mux/mux-uclass.c | 296 ++++++++++++++++++++++++++++++++++
>> include/dm/uclass-id.h | 1 +
>> include/dt-bindings/mux/mux.h | 17 ++
>> include/mux-internal.h | 80 +++++++++
>> include/mux.h | 114 +++++++++++++
>> 9 files changed, 524 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
>>
> I feel this needs more documentation. What is a multiplexer?
I'll add a description of what a mux is in include/mux.h
>
>> diff --git a/drivers/Kconfig b/drivers/Kconfig
>> index 350acf81f3..5334974ad4 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 a4bb5e4975..f4d71f3b3c 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/
> SPL?
good catch
>
>> 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 at 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..58998af29c
>> --- /dev/null
>> +++ b/drivers/mux/mux-uclass.c
>> @@ -0,0 +1,296 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Multiplexer subsystem
>> + *
>> + * Based on the linux multiplexer framework
>> + *
>> + * Copyright (C) 2017 Axentia Technologies AB
>> + * Author: Peter Rosin <peda at axentia.se>
>> + *
>> + * Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/
>> + * Jean-Jacques Hiblot <jjhiblot at ti.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <mux-internal.h>
> should go above the dm/device-internal
>
>> +#include <dm.h>
>> +#include <dt-structs.h>
> Shouldn't need this
>> +#include <errno.h>
> Shouldn't need this
>
>> +#include <dm/device-internal.h>
>> +#include <dm/lists.h>
>> +#include <dm/read.h>
> Shouldn't need this
I'll cleanup the headers in v2.
>
>> +#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(mux=%p)\n", __func__, mux);
>> +
>> + 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;
>> +}
>> +
>> +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",
>> + .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 d4d96106b3..869c0616bd 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -64,6 +64,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.
> <add more details...what is a mux?>
>
> How do I use this API?
The API is described by the comments in include/mux.h
>
>> + */
>> +
>> +#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..63444f4c3d
>> --- /dev/null
>> +++ b/include/mux-internal.h
>> @@ -0,0 +1,80 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Based on the linux multiplexer framework
>> + *
>> + * Copyright (C) 2017 Axentia Technologies AB
>> + * Author: Peter Rosin <peda at axentia.se>
>> + *
>> + * Copyright (C) 2017-2018 Texas Instruments Incorporated - http://www.ti.com/
>> + * Jean-Jacques Hiblot <jjhiblot at 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.
> Mention that this is 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 {
>> + int (*set)(struct mux_control *mux, int state);
>> + int (*of_xlate)(struct mux_chip *dev_mux,
>> + struct ofnode_phandle_args *args,
>> + struct mux_control **mux);
> Function comments
>
>> +};
>> +
>> +/**
>> + * 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.
> How do I get from a struct udevice * to this struct? Is it private data?
This is not private data per-se. It is intended to be used by a client
device.
You get a mux_control* with mux_control_get() and devm_mux_control_get().
JJ
>
>> + */
>> +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
> [..]
>
> Regards,
> Simon
>
More information about the U-Boot
mailing list