Re: [PATCH v5 04/10] reset: Add Sunplus SP7021 reset driver
From: Philipp Zabel <p.zabel@pengutronix.de>
Date: 2021-12-07 08:35:59
Also in:
linux-arm-kernel, linux-clk, lkml
On Fri, 2021-12-03 at 15:34 +0800, Qin Jian wrote: [...]
quoted hunk ↗ jump to hunk
diff --git a/drivers/reset/reset-sunplus.c b/drivers/reset/reset-sunplus.c new file mode 100644 index 000000000..a1d88dbaf --- /dev/null +++ b/drivers/reset/reset-sunplus.c@@ -0,0 +1,132 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +/* + * SP7021 reset driver + * + * Copyright (C) Sunplus Technology Co., Ltd. + * All rights reserved. + */ +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/reset-controller.h> +#include <linux/reboot.h> + +/* HIWORD_MASK_REG BITS */ +#define BITS_PER_HWM_REG 16 + +struct sp_reset_data { + struct reset_controller_dev rcdev; + void __iomem *membase; +} *sp_reset;
^^^^^^^^^^ I'd prefer if you removed the global sp_reset pointer. [...]
+static int sp_restart(struct notifier_block *this, unsigned long mode,
+ void *cmd)
+{You could embed the sp_restart_nb notifier block in struct sp_reset_data and use container_of(this, struct sp_reset_data, notifier) to get to the rcdev here.
+ sp_reset_assert(&sp_reset->rcdev, 0);
+ sp_reset_deassert(&sp_reset->rcdev, 0);
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block sp_restart_nb = {
+ .notifier_call = sp_restart,
+ .priority = 192,
+};
+
+static const struct reset_control_ops sp_reset_ops = {
+ .assert = sp_reset_assert,
+ .deassert = sp_reset_deassert,
+ .status = sp_reset_status,
+};
+
+static const struct of_device_id sp_reset_dt_ids[] = {
+ {.compatible = "sunplus,sp7021-reset",},
+ { /* sentinel */ },
+};
+
+static int sp_reset_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ void __iomem *membase;
+ struct resource *res;
+
+ sp_reset = devm_kzalloc(&pdev->dev, sizeof(*sp_reset), GFP_KERNEL);
+ if (!sp_reset)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ membase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(membase))
+ return PTR_ERR(membase);
+
+ sp_reset->membase = membase;
+ sp_reset->rcdev.owner = THIS_MODULE;
+ sp_reset->rcdev.nr_resets = resource_size(res) / 4 * 16; /* HIWORD_MASK */
+ sp_reset->rcdev.ops = &sp_reset_ops;
+ sp_reset->rcdev.of_node = dev->of_node;
+ register_restart_handler(&sp_restart_nb);Either do this after devm_reset_controller_register(), which could theoretically fail with -ENOMEM, or call unregister_restart_handler() in the error case below.
+
+ return devm_reset_controller_register(dev, &sp_reset->rcdev);
+}
+
+static struct platform_driver sp_reset_driver = {
+ .probe = sp_reset_probe,
+ .driver = {
+ .name = "sunplus-reset",
+ .of_match_table = sp_reset_dt_ids,^ Please fix the indentation, two tabs here. .suppress_bind_attrs = true, to stop unbinding the driver. Alternatively, add a driver remove function that unregisters the restart handler.
+ },
^ One tab here.
+};
+
+module_platform_driver(sp_reset_driver);
+
+MODULE_AUTHOR("Edwin Chiu [off-list ref]");
+MODULE_DESCRIPTION("Sunplus Reset Driver");
+MODULE_LICENSE("GPL v2");regards Philipp