Hi Andre,
On 29 November 2017 at 18:25, Andre Przywara <andre.przyw...@arm.com> wrote:
> From: Amit Singh Tomar <amittome...@gmail.com>
>
> The simplest and most generic form of a reset controller just exposes
> multiple MMIO registers, where each bit toggles a separate reset line.
> Add a generic driver to describe this kind of reset controller.
>
> This is used on the Nexell S5P6818, for instance, but also by other
> SoCs.
>
> Signed-off-by: Amit Singh Tomar <amittome...@gmail.com>
> [Andre: make more generic, let it cover multiple registers, slight rework]
> Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
> ---
> drivers/reset/Kconfig | 6 +++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-generic.c | 111
> ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 118 insertions(+)
> create mode 100644 drivers/reset/reset-generic.c
Reviewed-by: Simon Glass <s...@chromium.org>
Is there a DT binding for this?
Also please see below.
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index ce46e2752c..3032b0064c 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,6 +12,12 @@ config DM_RESET
> although driving such reset isgnals using GPIOs may be more
> appropriate in this case.
>
> +config GENERIC_RESET
> + bool "Generic Reset controller driver"
> + depends on DM_RESET
> + help
> + Support Generic reset controller.
What is this? This help needs to be expanded with details.
> +
> config SANDBOX_RESET
> bool "Enable the sandbox reset test driver"
> depends on DM_MAILBOX && SANDBOX
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 252cefeed5..81680837ef 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_TEGRA186_RESET) += tegra186-reset.o
> obj-$(CONFIG_RESET_BCM6345) += reset-bcm6345.o
> obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> obj-$(CONFIG_AST2500_RESET) += ast2500-reset.o
> +obj-$(CONFIG_GENERIC_RESET) += reset-generic.o
> diff --git a/drivers/reset/reset-generic.c b/drivers/reset/reset-generic.c
> new file mode 100644
> index 0000000000..0a7740202d
> --- /dev/null
> +++ b/drivers/reset/reset-generic.c
> @@ -0,0 +1,111 @@
> +/*
> + * Copyright (C) 2017 Amit Singh Tomar <amittome...@gmail.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <reset-uclass.h>
> +#include <linux/bitops.h>
> +#include <linux/io.h>
> +#include <linux/sizes.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct generic_reset_priv {
> + void __iomem *membase;
> + int max_reset;
comment
> +};
> +
> +#define BITS_PER_BYTE 8
blank line here
> +static int generic_reset_toggle(struct reset_ctl *rst, bool assert)
> +{
> + struct generic_reset_priv *priv = dev_get_priv(rst->dev);
> + int reg_width = sizeof(u32);
> + int bank, offset;
> + u32 reg;
> +
> + if (rst->id >= priv->max_reset)
> + return -EINVAL;
> +
> + bank = rst->id / (reg_width * BITS_PER_BYTE);
> + offset = rst->id % (reg_width * BITS_PER_BYTE);
> +
> + reg = readl(priv->membase + (bank * reg_width));
Can you put that expression in a local var?
> + if (assert)
> + writel(reg & ~BIT(offset), priv->membase + (bank *
> reg_width));
> + else
> + writel(reg | BIT(offset), priv->membase + (bank * reg_width));
> +
> + return 0;
> +}
> +
> +static int generic_reset_assert(struct reset_ctl *rst)
> +{
> + return generic_reset_toggle(rst, true);
> +}
> +
> +static int generic_reset_deassert(struct reset_ctl *rst)
> +{
> + return generic_reset_toggle(rst, false);
> +}
> +
> +static int generic_reset_free(struct reset_ctl *rst)
> +{
> + return 0;
> +}
> +
> +static int generic_reset_request(struct reset_ctl *rst)
> +{
> + struct generic_reset_priv *priv = dev_get_priv(rst->dev);
> +
> + if (rst->id >= priv->max_reset)
> + return -EINVAL;
OK I suppose. How about -ENOENT?
> +
> + return generic_reset_assert(rst);
> +}
> +
> +struct reset_ops generic_reset_reset_ops = {
> + .free = generic_reset_free,
> + .request = generic_reset_request,
> + .rst_assert = generic_reset_assert,
> + .rst_deassert = generic_reset_deassert,
> +};
> +
> +static const struct udevice_id generic_reset_ids[] = {
> + { .compatible = "generic-reset" },
> + { .compatible = "nexell,s5p6818-reset" },
> + { }
> +};
> +
> +static int generic_reset_probe(struct udevice *dev)
> +{
> + struct generic_reset_priv *priv = dev_get_priv(dev);
> + fdt_addr_t addr;
> + fdt_size_t size;
> +
> + addr = devfdt_get_addr_size_index(dev, 0, &size);
Can we use dev_read_... here?
> + if (addr == FDT_ADDR_T_NONE)
> + return -EINVAL;
> +
> + priv->max_reset = dev_read_u32_default(dev, "num-resets", -1);
> + if (priv->max_reset == -1)
> + priv->max_reset = size * BITS_PER_BYTE;
> +
> + priv->membase = devm_ioremap(dev, addr, size);
> + if (!priv->membase)
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +U_BOOT_DRIVER(generic_reset) = {
> + .name = "generic_reset",
> + .id = UCLASS_RESET,
> + .of_match = generic_reset_ids,
> + .ops = &generic_reset_reset_ops,
> + .probe = generic_reset_probe,
> + .priv_auto_alloc_size = sizeof(struct generic_reset_priv),
> +};
> --
> 2.14.1
>
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
