[PATCH v4 6/7] cmd: Add a mux command
Simon Glass
sjg at chromium.org
Tue Oct 27 05:52:07 CET 2020
Hi Pratyush,
On Fri, 16 Oct 2020 at 04:46, Pratyush Yadav <p.yadav at ti.com> wrote:
>
> This command lets the user list, select, and deselect mux controllers
> introduced with the mux framework on the fly. It has 3 subcommands:
> list, select, and deselect.
>
> List: Lists all the mux present on the system. The muxes are listed for
> each chip. The chip is identified by its device name. Each chip can have
> a number of mux controllers. Each is listed in sequence and is assigned
> a sequential ID based on its position in the mux chip. It lists details
> like ID, whether the mux is currently selected or not, the current
> state, the idle state, and the number of states.
>
> A sample output would look something like:
>
> => mux list
> a-mux-controller:
> ID Selected Current State Idle State Num States
> 0 no unknown as-is 0x4
> 1 no 0x2 0x2 0x10
> 2 no 0x73 0x73 0x100
>
> another-mux-controller:
> ID Selected Current State Idle State Num States
> 0 no 0x1 0x1 0x4
> 1 no 0x2 0x2 0x4
>
> Select: Selects a given mux and puts it in the specified state. This
> subcommand takes 3 arguments: mux chip, mux ID, state to set
> the mux in. The arguments mux chip and mux ID are used to identify which
> mux needs to be selected, and then it is selected to the given state.
> The mux needs to be deselected before it can be selected again in
> another state. The state should be a hexadecimal number.
>
> For example:
> => mux list
> a-mux-controller:
> ID Selected Current State Idle State Num States
> 0 no 0x1 0x1 0x4
> 1 no 0x1 0x1 0x4
> => mux select a-mux-controller 0 0x3
> => mux list
> a-mux-controller:
> ID Selected Current State Idle State Num States
> 0 yes 0x3 0x1 0x4
> 1 no 0x1 0x1 0x4
>
> Deselect: Deselects a given mux and puts it in its idle state. This
> subcommand takes 2 arguments: the mux chip and mux ID to identify which
> mux needs to be deselected. So in the above example, we can deselect mux
> 0 using:
>
> => mux deselect a-mux-controller 0
> => mux list
> a-mux-controller:
> ID Selected Current State Idle State Num States
> 0 no 0x1 0x1 0x4
> 1 no 0x1 0x1 0x4
>
> Signed-off-by: Pratyush Yadav <p.yadav at ti.com>
> ---
>
> Notes:
> Changes in v4:
>
> - Use spaces to print columns in 'mux list' instead of tabs. Tabs don't
> work well with ut_assert_nextline() because it calls
> membuff_readline() down the call chain and tells it to consider
> characters below the space character as EOL. This means
> ut_assert_nextline() can only assert for the next column not the next
> line if a command is using tabs.
>
> cmd/Kconfig | 6 ++
> cmd/Makefile | 1 +
> cmd/mux.c | 183 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 190 insertions(+)
> create mode 100644 cmd/mux.c
Reviewed-by: Simon Glass <sjg at chromium.org>
A few comments below.
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index a3166e4f31..a25d6d62d0 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1073,6 +1073,12 @@ config CMD_MTD
> help
> MTD commands support.
>
> +config CMD_MUX
> + bool "mux"
> + depends on MULTIPLEXER
> + help
> + List, select, and deselect mux controllers on the fly.
> +
> config CMD_NAND
> bool "nand"
> default y if NAND_SUNXI
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 19a891633f..79a57b8482 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -104,6 +104,7 @@ obj-$(CONFIG_CMD_CLONE) += clone.o
> ifneq ($(CONFIG_CMD_NAND)$(CONFIG_CMD_SF),)
> obj-y += legacy-mtd-utils.o
> endif
> +obj-$(CONFIG_CMD_MUX) += mux.o
> obj-$(CONFIG_CMD_NAND) += nand.o
> obj-$(CONFIG_CMD_NET) += net.o
> obj-$(CONFIG_CMD_NVEDIT_EFI) += nvedit_efi.o
> diff --git a/cmd/mux.c b/cmd/mux.c
> new file mode 100644
> index 0000000000..3a0839ec1f
> --- /dev/null
> +++ b/cmd/mux.c
> @@ -0,0 +1,183 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * List, select, and deselect mux controllers on the fly.
> + *
> + * Copyright (c) 2020 Texas Instruments Inc.
> + * Author: Pratyush Yadav <p.yadav at ti.com>
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <mux.h>
> +#include <mux-internal.h>
> +#include <linux/err.h>
> +#include <dt-bindings/mux/mux.h>
> +
> +#define COLUMN_SIZE 16
> +
> +/*
> + * Print a member of a column. The total size of the text printed, including
> + * trailing whitespace, will always be COLUMN_SIZE.
> + */
> +#define PRINT_COLUMN(fmt, args...) do { \
> + char buf[COLUMN_SIZE + 1]; \
> + snprintf(buf, COLUMN_SIZE + 1, fmt, ##args); \
> + printf("%-*s", COLUMN_SIZE, buf); \
> +} while (0)
This blows out the code size. How about a static function instead?
> +
> +/*
> + * Find a mux based on its device name in argv[1] and index in the chip in
> + * argv[2].
> + */
> +static struct mux_control *cmd_mux_find(char *const argv[])
Is it worth having a pointer here? You could return the argument as an
int and have a struct mux_control** param here. Up to you.
> +{
> + struct udevice *dev;
> + struct mux_chip *chip;
> + int ret;
> + unsigned long id;
> +
> + ret = strict_strtoul(argv[2], 10, &id);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + ret = uclass_get_device_by_name(UCLASS_MUX, argv[1], &dev);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + chip = dev_get_uclass_priv(dev);
> + if (!chip)
> + return ERR_PTR(ret);
> +
> + if (id >= chip->controllers)
> + return ERR_PTR(-EINVAL);
> +
> + return &chip->mux[id];
> +}
> +
> +/*
> + * Print the details of a mux. The columns printed correspond to: "Selected",
> + * "Current State", "Idle State", and "Num States".
> + */
> +static void print_mux(struct mux_control *mux)
> +{
> + PRINT_COLUMN("%s", mux->in_use ? "yes" : "no");
> +
> + if (mux->cached_state == MUX_IDLE_AS_IS)
> + PRINT_COLUMN("%s", "unknown");
> + else
> + PRINT_COLUMN("0x%x", mux->cached_state);
> +
> + if (mux->idle_state == MUX_IDLE_AS_IS)
> + PRINT_COLUMN("%s", "as-is");
> + else if (mux->idle_state == MUX_IDLE_DISCONNECT)
> + PRINT_COLUMN("%s", "disconnect");
> + else
> + PRINT_COLUMN("0x%x", mux->idle_state);
> +
> + PRINT_COLUMN("0x%x", mux->states);
> +
> + printf("\n");
> +}
> +
> +static int do_mux_list(struct cmd_tbl *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + struct udevice *dev;
> + struct mux_chip *chip;
> + int j;
> +
> + for (uclass_first_device(UCLASS_MUX, &dev);
> + dev;
> + uclass_next_device(&dev)) {
uclass_for_each_device() or similar
> + chip = dev_get_uclass_priv(dev);
> + if (!chip) {
> + dev_err(dev, "can't find mux chip\n");
> + continue;
> + }
> +
Regards,
Simon
More information about the U-Boot
mailing list