Re: [PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

Philipp Zabel
2016-05-20T09:28:04Z

Hi Neil,

Am Freitag, den 20.05.2016, 10:27 +0200 schrieb Neil Armstrong:
> This patch adds the platform driver for the Amlogic Meson GXBB Reset
> Controller.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/reset/Kconfig | 6 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-meson-gxbb.c | 129 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 136 insertions(+)
> create mode 100644 drivers/reset/reset-meson-gxbb.c
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index df37212..4ac5c4d 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,5 +12,11 @@ menuconfig RESET_CONTROLLER
>
> If unsure, say no.
>
> +config MESON_GXBB_RESET
> + tristate "Amlogic Meson GXBB Reset Driver"
> + depends on (ARCH_MESON && RESET_CONTROLLER)

With the "reset: fix Kconfig menu to include reset drivers in sub-menu"
patch [1] applied this is wrapped in "if RESET_CONTROLLER ... endif", so
no need to depend on RESET_CONTROLLER.
Is there a reason to have this configurable at all, though?

[1] https://patchwork.kernel.org/patch/9000591/

> + help
> + Build the Amlogic Meson GxBB reset driver.
> +
> source "drivers/reset/sti/Kconfig"
> source "drivers/reset/hisilicon/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index a1fc8ed..5ff83a1 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_ARCH_LPC18XX) += reset-lpc18xx.o
> obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
> obj-$(CONFIG_ARCH_BERLIN) += reset-berlin.o
> obj-$(CONFIG_MACH_PISTACHIO) += reset-pistachio.o
> +obj-$(CONFIG_MESON_GXBB_RESET) += reset-meson-gxbb.o

Or drop CONFIG_MESON_GXBB_RESET and use CONFIG_ARCH_MESON directly.

[...]
> +static int meson_gxbb_reset_probe(struct platform_device *pdev)
> +{
> + struct meson_gxbb_reset *data;
> + struct resource *res;
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + data->reg_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(data->reg_base))
> + return PTR_ERR(data->reg_base);
> +
> + platform_set_drvdata(pdev, data);
> +
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.nr_resets = REG_COUNT * BITS_PER_REG;
> + data->rcdev.ops = &meson_gxbb_reset_ops;
> + data->rcdev.of_node = pdev->dev.of_node;
> +
> + return reset_controller_register(&data->rcdev);

With the "reset: add devm_reset_controller_register API" patch [2]
applied you can use devm_reset_controller_register() here and remove the
meson_gxbb_reset_remove() function below.

[2] https://patchwork.kernel.org/patch/8988471/

> +}
> +
> +static int meson_gxbb_reset_remove(struct platform_device *pdev)
> +{
> + struct meson_gxbb_reset *data = platform_get_drvdata(pdev);
> +
> + reset_controller_unregister(&data->rcdev);
> +
> + return 0;
> +}

Could be removed, then.

regards
Philipp