[U-Boot] [PATCH 1/2] Add a reset driver framework/uclass

Simon Glass sjg at chromium.org
Tue May 17 23:56:36 CEST 2016


Hi Stephen,

On 17 May 2016 at 10:46, Stephen Warren <swarren at wwwdotorg.org> wrote:
> From: Stephen Warren <swarren at nvidia.com>
>
> A reset controller is a hardware module that controls reset signals that
> affect other hardware modules or chips.
>
> This patch defines a standard API that connects reset clients (i.e. the
> drivers for devices affected by reset signals) to drivers for reset
> controllers/providers. Initially, DT is the only supported method for
> connecting the two.
>
> The DT binding specification (reset.txt) was taken from Linux kernel
> v4.5's Documentation/devicetree/bindings/reset/reset.txt.
>
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> ---
> This series depends on my recent mailbox series, solely for a trivial
> amount of diff context.
>
> This series will eventually be a dependency for the Tegra186 clock/reset
> driver implementation, but that isn't ready yet.
>
>  doc/device-tree-bindings/reset/reset.txt |  75 +++++++++++++++++
>  drivers/Kconfig                          |   2 +
>  drivers/Makefile                         |   1 +
>  drivers/reset/Kconfig                    |  15 ++++
>  drivers/reset/Makefile                   |   5 ++
>  drivers/reset/reset-uclass.c             | 131 ++++++++++++++++++++++++++++++
>  include/dm/uclass-id.h                   |   1 +
>  include/reset_client.h                   | 135 +++++++++++++++++++++++++++++++
>  include/reset_uclass.h                   |  81 +++++++++++++++++++
>  9 files changed, 446 insertions(+)
>  create mode 100644 doc/device-tree-bindings/reset/reset.txt
>  create mode 100644 drivers/reset/Kconfig
>  create mode 100644 drivers/reset/Makefile
>  create mode 100644 drivers/reset/reset-uclass.c
>  create mode 100644 include/reset_client.h
>  create mode 100644 include/reset_uclass.h
>

This loks fine but for one thing, below.

> diff --git a/doc/device-tree-bindings/reset/reset.txt b/doc/device-tree-bindings/reset/reset.txt
> new file mode 100644
> index 000000000000..31db6ff84908
> --- /dev/null
> +++ b/doc/device-tree-bindings/reset/reset.txt
> @@ -0,0 +1,75 @@
> += Reset Signal Device Tree Bindings =
> +
> +This binding is intended to represent the hardware reset signals present
> +internally in most IC (SoC, FPGA, ...) designs. Reset signals for whole
> +standalone chips are most likely better represented as GPIOs, although there
> +are likely to be exceptions to this rule.
> +
> +Hardware blocks typically receive a reset signal. This signal is generated by
> +a reset provider (e.g. power management or clock module) and received by a
> +reset consumer (the module being reset, or a module managing when a sub-
> +ordinate module is reset). This binding exists to represent the provider and
> +consumer, and provide a way to couple the two together.
> +
> +A reset signal is represented by the phandle of the provider, plus a reset
> +specifier - a list of DT cells that represents the reset signal within the
> +provider. The length (number of cells) and semantics of the reset specifier
> +are dictated by the binding of the reset provider, although common schemes
> +are described below.
> +
> +A word on where to place reset signal consumers in device tree: It is possible
> +in hardware for a reset signal to affect multiple logically separate HW blocks
> +at once. In this case, it would be unwise to represent this reset signal in
> +the DT node of each affected HW block, since if activated, an unrelated block
> +may be reset. Instead, reset signals should be represented in the DT node
> +where it makes most sense to control it; this may be a bus node if all
> +children of the bus are affected by the reset signal, or an individual HW
> +block node for dedicated reset signals. The intent of this binding is to give
> +appropriate software access to the reset signals in order to manage the HW,
> +rather than to slavishly enumerate the reset signal that affects each HW
> +block.
> +
> += Reset providers =
> +
> +Required properties:
> +#reset-cells:  Number of cells in a reset specifier; Typically 0 for nodes
> +               with a single reset output and 1 for nodes with multiple
> +               reset outputs.
> +
> +For example:
> +
> +       rst: reset-controller {
> +               #reset-cells = <1>;
> +       };
> +
> += Reset consumers =
> +
> +Required properties:
> +resets:                List of phandle and reset specifier pairs, one pair
> +               for each reset signal that affects the device, or that the
> +               device manages. Note: if the reset provider specifies '0' for
> +               #reset-cells, then only the phandle portion of the pair will
> +               appear.
> +
> +Optional properties:
> +reset-names:   List of reset signal name strings sorted in the same order as
> +               the resets property. Consumers drivers will use reset-names to
> +               match reset signal names with reset specifiers.
> +
> +For example:
> +
> +       device {
> +               resets = <&rst 20>;
> +               reset-names = "reset";
> +       };
> +
> +This represents a device with a single reset signal named "reset".
> +
> +       bus {
> +               resets = <&rst 10> <&rst 11> <&rst 12> <&rst 11>;
> +               reset-names = "i2s1", "i2s2", "dma", "mixer";
> +       };
> +
> +This represents a bus that controls the reset signal of each of four sub-
> +ordinate devices. Consider for example a bus that fails to operate unless no
> +child device has reset asserted.
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index f2a137ad87fc..f6003a0a593a 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -56,6 +56,8 @@ source "drivers/ram/Kconfig"
>
>  source "drivers/remoteproc/Kconfig"
>
> +source "drivers/reset/Kconfig"
> +
>  source "drivers/rtc/Kconfig"
>
>  source "drivers/serial/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 47fddff084b5..1634efc5fc98 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_U_QE) += qe/
>  obj-y += mailbox/
>  obj-y += memory/
>  obj-y += pwm/
> +obj-y += reset/
>  obj-y += input/
>  # SOC specific infrastructure drivers.
>  obj-y += soc/
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> new file mode 100644
> index 000000000000..5c449a99d39d
> --- /dev/null
> +++ b/drivers/reset/Kconfig
> @@ -0,0 +1,15 @@
> +menu "Reset Controller Support"
> +
> +config DM_RESET
> +       bool "Enable reset controllers using Driver Model"
> +       depends on DM && OF_CONTROL
> +       help
> +         Enable support for the reset controller driver class. Many hardware
> +         modules are equipped with a reset signal, typically driven by some
> +         reset controller hardware module within the chip. In U-Boot, reset
> +         controller drivers allow control over these reset signals. In some
> +         cases this API is applicable to chips outside the CPU as well,
> +         although driving such reset isgnals using GPIOs may be more
> +         appropriate in this case.
> +
> +endmenu
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> new file mode 100644
> index 000000000000..508608e83f87
> --- /dev/null
> +++ b/drivers/reset/Makefile
> @@ -0,0 +1,5 @@
> +# Copyright (c) 2016, NVIDIA CORPORATION.
> +#
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_DM_RESET) += reset-uclass.o
> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c
> new file mode 100644
> index 000000000000..ac0614eb9ab9
> --- /dev/null
> +++ b/drivers/reset/reset-uclass.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright (c) 2016, NVIDIA CORPORATION.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <reset_client.h>
> +#include <reset_uclass.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static inline struct reset_ops *reset_dev_ops(struct udevice *dev)
> +{
> +       return (struct reset_ops *)dev->driver->ops;
> +}
> +
> +static int reset_of_xlate_default(struct reset_ctl *reset_ctl,
> +                                 struct fdtdec_phandle_args *args)
> +{
> +       debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
> +
> +       if (args->args_count != 1) {
> +               debug("Invaild args_count: %d\n", args->args_count);
> +               return -EINVAL;
> +       }
> +
> +       reset_ctl->id = args->args[0];
> +
> +       return 0;
> +}
> +
> +int reset_get_by_index(struct udevice *dev, int index,
> +                      struct reset_ctl *reset_ctl)
> +{
> +       struct fdtdec_phandle_args args;
> +       int ret;
> +       struct udevice *dev_reset;
> +       struct reset_ops *ops;
> +
> +       debug("%s(dev=%p, index=%d, reset_ctl=%p)\n", __func__, dev, index,
> +             reset_ctl);
> +
> +       ret = fdtdec_parse_phandle_with_args(gd->fdt_blob, dev->of_offset,
> +                                            "resets", "#reset-cells", 0,
> +                                            index, &args);
> +       if (ret) {
> +               debug("%s: fdtdec_parse_phandle_with_args failed: %d\n",
> +                     __func__, ret);
> +               return ret;
> +       }
> +
> +       ret = uclass_get_device_by_of_offset(UCLASS_RESET, args.node,
> +                                            &dev_reset);
> +       if (ret) {
> +               debug("%s: uclass_get_device_by_of_offset failed: %d\n",
> +                     __func__, ret);
> +               return ret;
> +       }
> +       ops = reset_dev_ops(dev_reset);
> +
> +       reset_ctl->dev = dev_reset;
> +       if (ops->of_xlate)
> +               ret = ops->of_xlate(reset_ctl, &args);
> +       else
> +               ret = reset_of_xlate_default(reset_ctl, &args);
> +       if (ret) {
> +               debug("of_xlate() failed: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = ops->request(reset_ctl);
> +       if (ret) {
> +               debug("ops->request() failed: %d\n", ret);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +int reset_get_by_name(struct udevice *dev, const char *name,
> +                    struct reset_ctl *reset_ctl)
> +{
> +       int index;
> +
> +       debug("%s(dev=%p, name=%s, reset_ctl=%p)\n", __func__, dev, name,
> +             reset_ctl);
> +
> +       index = fdt_find_string(gd->fdt_blob, dev->of_offset, "reset-names",
> +                               name);
> +       if (index < 0) {
> +               debug("fdt_find_string() failed: %d\n", index);
> +               return index;
> +       }
> +
> +       return reset_get_by_index(dev, index, reset_ctl);
> +}
> +
> +int reset_free(struct reset_ctl *reset_ctl)
> +{
> +       struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +
> +       debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
> +
> +       return ops->free(reset_ctl);
> +}
> +
> +int reset_assert(struct reset_ctl *reset_ctl)
> +{
> +       struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +
> +       debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
> +
> +       return ops->rst_assert(reset_ctl);
> +}
> +
> +int reset_deassert(struct reset_ctl *reset_ctl)
> +{
> +       struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +
> +       debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
> +
> +       return ops->rst_deassert(reset_ctl);
> +}
> +
> +UCLASS_DRIVER(reset) = {
> +       .id             = UCLASS_RESET,
> +       .name           = "reset",
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 4e252ae210f7..e31f699cdb65 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -63,6 +63,7 @@ enum uclass_id {
>         UCLASS_PWRSEQ,          /* Power sequence device */
>         UCLASS_REGULATOR,       /* Regulator device */
>         UCLASS_REMOTEPROC,      /* Remote Processor device */
> +       UCLASS_RESET,           /* Reset controller device */
>         UCLASS_RTC,             /* Real time clock device */
>         UCLASS_SERIAL,          /* Serial UART */
>         UCLASS_SPI,             /* SPI bus */
> diff --git a/include/reset_client.h b/include/reset_client.h
> new file mode 100644
> index 000000000000..3bdc8256853a
> --- /dev/null
> +++ b/include/reset_client.h
> @@ -0,0 +1,135 @@
> +/*
> + * Copyright (c) 2016, NVIDIA CORPORATION.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#ifndef _RESET_CLIENT_H
> +#define _RESET_CLIENT_H
> +
> +/**
> + * A reset is a hardware signal indicating that a HW module (or IP block, or
> + * sometimes an entire off-CPU chip) reset all of its internal state to some
> + * known-good initial state. Drivers will often reset HW modules when they
> + * begin execution to ensure that hardware correctly responds to all requests,
> + * or in response to some error condition. Reset signals are often controlled
> + * externally to the HW module being reset, by an entity this API calls a reset
> + * controller. This API provides a standard means for drivers to request that
> + * reset controllers set or clear reset signals.
> + *
> + * A driver that implements UCLASS_RESET is a reset controller or provider. A
> + * controller will often implement multiple separate reset signals, since the
> + * hardware it manages often has this capability. reset_uclass.h describes the
> + * interface which reset controllers must implement.
> + *
> + * Reset consumers/clients are the HW modules affected by reset signals. This
> + * header file describes the API used by drivers for those HW modules.
> + */
> +
> +struct udevice;
> +
> +/**
> + * struct reset_ctl - A handle to (allowing control of) a single reset signal.
> + *
> + * Clients provide storage for reset control handles. The content of the
> + * structure is managed solely by the reset API and reset drivers. A reset
> + * control struct is initialized by "get"ing the reset control struct. The
> + * reset control struct is passed to all other reset APIs to identify which
> + * reset signal to operate upon.
> + *
> + * @dev: The device which implements the reset signal.
> + * @id: The reset signal ID within the provider.
> + *
> + * Currently, the reset API assumes that a single integer ID is enough to
> + * identify and configure any reset signal for any reset provider. If this
> + * assumption becomes invalid in the future, the struct could be expanded to
> + * either (a) add more fields to allow reset providers to store additional
> + * information, or (b) replace the id field with an opaque pointer, which the
> + * provider would dynamically allocated during its .of_xlate op, and process
> + * during is .request op. This may require the addition of an extra op to clean
> + * up the allocation.
> + */
> +struct reset_ctl {
> +       struct udevice *dev;
> +       /*
> +        * Written by of_xlate. We assume a single id is enough for now. In the
> +        * future, we might add more fields here.
> +        */
> +       unsigned long id;

This feels like an obfuscation to me. Why not just use an int instead
of this struct? This is what clock does.

> +};
> +
> +/**
> + * reset_get_by_index - Get/request a reset signal by integer index.
> + *
> + * This looks up and requests a reset signal. The index is relative to the
> + * client device; each device is assumed to have n reset signals associated
> + * with it somehow, and this function finds and requests one of them. The
> + * mapping of client device reset signal indices to provider reset signals may
> + * be via device-tree properties, board-provided mapping tables, or some other
> + * mechanism.
> + *
> + * @dev:       The client device.
> + * @index:     The index of the reset signal to request, within the client's
> + *             list of reset signals.
> + * @reset_ctl  A pointer to a reset control struct to initialize.
> + * @return 0 if OK, or a negative error code.
> + */
> +int reset_get_by_index(struct udevice *dev, int index,
> +                      struct reset_ctl *reset_ctl);
> +
> +/**
> + * reset_get_by_name - Get/request a reset signal by name.
> + *
> + * This looks up and requests a reset signal. The name is relative to the
> + * client device; each device is assumed to have n reset signals associated
> + * with it somehow, and this function finds and requests one of them. The
> + * mapping of client device reset signal names to provider reset signal may be
> + * via device-tree properties, board-provided mapping tables, or some other
> + * mechanism.
> + *
> + * @dev:       The client device.
> + * @name:      The name of the reset signal to request, within the client's
> + *             list of reset signals.
> + * @reset_ctl: A pointer to a reset control struct to initialize.
> + * @return 0 if OK, or a negative error code.
> + */
> +int reset_get_by_name(struct udevice *dev, const char *name,
> +                     struct reset_ctl *reset_ctl);
> +
> +/**
> + * reset_free - Free a previously requested reset signal.
> + *
> + * @reset_ctl: A reset control struct that was previously successfully
> + *             requested by reset_get_by_*().
> + * @return 0 if OK, or a negative error code.
> + */
> +int reset_free(struct reset_ctl *reset_ctl);
> +
> +/**
> + * reset_assert - Assert a reset signal.
> + *
> + * This function will assert the specified reset signal, thus resetting the
> + * affected HW module(s). Depending on the reset controller hardware, the reset
> + * signal will either stay asserted until reset_deassert() is called, or the
> + * hardware may autonomously clear the reset signal itself.
> + *
> + * @reset_ctl: A reset control struct that was previously successfully
> + *             requested by reset_get_by_*().
> + * @return 0 if OK, or a negative error code.
> + */
> +int reset_assert(struct reset_ctl *reset_ctl);
> +
> +/**
> + * reset_deassert - Deassert a reset signal.
> + *
> + * This function will deassert the specified reset signal, thus releasing the
> + * affected HW modules() from reset, and allowing them to continue normal
> + * operation.
> + *
> + * @reset_ctl: A reset control struct that was previously successfully
> + *             requested by reset_get_by_*().
> + * @return 0 if OK, or a negative error code.
> + */
> +int reset_deassert(struct reset_ctl *reset_ctl);
> +
> +#endif
> diff --git a/include/reset_uclass.h b/include/reset_uclass.h
> new file mode 100644
> index 000000000000..6e27a7bbd1cf
> --- /dev/null
> +++ b/include/reset_uclass.h
> @@ -0,0 +1,81 @@
> +/*
> + * Copyright (c) 2016, NVIDIA CORPORATION.
> + *
> + * SPDX-License-Identifier: GPL-2.0
> + */
> +
> +#ifndef _RESET_UCLASS_H
> +#define _RESET_UCLASS_H
> +
> +/* See reset_client.h for background documentation. */
> +
> +#include <reset_client.h>
> +
> +struct udevice;
> +
> +/**
> + * struct reset_ops - The functions that a reset controller driver must
> + * implement.
> + */
> +struct reset_ops {
> +       /**
> +        * of_xlate - Translate a client's device-tree (OF) reset specifier.
> +        *
> +        * The reset core calls this function as the first step in implementing
> +        * a client's reset_get_by_*() call.
> +        *
> +        * If this function pointer is set to NULL, the reset core will use a
> +        * default implementation, which assumes #reset-cells = <1>, and that
> +        * the DT cell contains a simple integer reset signal ID.
> +        *
> +        * At present, the reset API solely supports device-tree. If this
> +        * changes, other xxx_xlate() functions may be added to support those
> +        * other mechanisms.
> +        *
> +        * @reset_ctl:  The reset control struct to hold the translation result.
> +        * @args:       The reset specifier values from device tree.
> +        * @return 0 if OK, or a negative error code.
> +        */
> +       int (*of_xlate)(struct reset_ctl *reset_ctl,
> +                       struct fdtdec_phandle_args *args);
> +       /**
> +        * request - Request a translated reset control.
> +        *
> +        * The reset core calls this function as the second step in
> +        * implementing a client's reset_get_by_*() call, following a
> +        * successful xxx_xlate() call.
> +        *
> +        * @reset_ctl:  The reset control struct to request; this has been
> +        *              filled in by a previoux xxx_xlate() function call.
> +        * @return 0 if OK, or a negative error code.
> +        */
> +       int (*request)(struct reset_ctl *reset_ctl);
> +       /**
> +        * free - Free a previously requested reset control.
> +        *
> +        * This is the implementation of the client reset_free() API.
> +        *
> +        * @reset_ctl:  The reset control to free.
> +        * @return 0 if OK, or a negative error code.
> +        */
> +       int (*free)(struct reset_ctl *reset_ctl);
> +       /**
> +        * rst_assert - Assert a reset signal.
> +        *
> +        * Note: This function is named rst_assert not assert to avoid
> +        * conflicting with global macro assert().
> +        *
> +        * @reset_ctl:  The reset signal to assert.
> +        * @return 0 if OK, or a negative error code.
> +        */
> +       int (*rst_assert)(struct reset_ctl *reset_ctl);
> +       /**
> +        * rst_deassert - Deassert a reset signal.
> +        *
> +        * @reset_ctl:  The reset signal to deassert.
> +        * @return 0 if OK, or a negative error code.
> +        */
> +       int (*rst_deassert)(struct reset_ctl *reset_ctl);
> +};
> +
> +#endif
> --
> 2.8.2
>

Regards,
Simon


More information about the U-Boot mailing list