[PATCH 7/9] clk: actions: Add Actions Semi Owl SoCs Reset Management Unit support
From: p.zabel@pengutronix.de (Philipp Zabel)
Date: 2018-07-30 10:22:20
Also in:
linux-clk, linux-devicetree, lkml
Hi Manivannan, Thank you for the patch, a few small comments below: On Sat, 2018-07-28 at 00:15 +0530, Manivannan Sadhasivam wrote:
quoted hunk ↗ jump to hunk
Add Reset Management Unit (RMU) support for Actions Semi Owl SoCs. Signed-off-by: Manivannan Sadhasivam <redacted> --- drivers/clk/actions/Kconfig | 1 + drivers/clk/actions/Makefile | 1 + drivers/clk/actions/owl-common.h | 2 + drivers/clk/actions/owl-reset.c | 72 ++++++++++++++++++++++++++++++++ drivers/clk/actions/owl-reset.h | 32 ++++++++++++++ 5 files changed, 108 insertions(+) create mode 100644 drivers/clk/actions/owl-reset.c create mode 100644 drivers/clk/actions/owl-reset.hdiff --git a/drivers/clk/actions/Kconfig b/drivers/clk/actions/Kconfig index dc38c85a4833..04f0a6355726 100644 --- a/drivers/clk/actions/Kconfig +++ b/drivers/clk/actions/Kconfig@@ -2,6 +2,7 @@ config CLK_ACTIONS bool "Clock driver for Actions Semi SoCs" depends on ARCH_ACTIONS || COMPILE_TEST select REGMAP_MMIO + select RESET_CONTROLLER default ARCH_ACTIONS if CLK_ACTIONSdiff --git a/drivers/clk/actions/Makefile b/drivers/clk/actions/Makefile index 78c17d56f991..ccfdf9781cef 100644 --- a/drivers/clk/actions/Makefile +++ b/drivers/clk/actions/Makefile@@ -7,6 +7,7 @@ clk-owl-y += owl-divider.o clk-owl-y += owl-factor.o clk-owl-y += owl-composite.o clk-owl-y += owl-pll.o +clk-owl-y += owl-reset.o # SoC support obj-$(CONFIG_CLK_OWL_S700) += owl-s700.odiff --git a/drivers/clk/actions/owl-common.h b/drivers/clk/actions/owl-common.h index 56f01f7774aa..4dc7f286831f 100644 --- a/drivers/clk/actions/owl-common.h +++ b/drivers/clk/actions/owl-common.h@@ -26,6 +26,8 @@ struct owl_clk_desc { struct owl_clk_common **clks; unsigned long num_clks; struct clk_hw_onecell_data *hw_clks; + struct owl_reset_map *resets;
Could this be: const struct owl_reset_map *resets; ?
quoted hunk ↗ jump to hunk
+ unsigned long num_resets; struct regmap *regmap; };diff --git a/drivers/clk/actions/owl-reset.c b/drivers/clk/actions/owl-reset.c new file mode 100644 index 000000000000..91b161cc68de --- /dev/null +++ b/drivers/clk/actions/owl-reset.c@@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// Actions Semi Owl SoCs Reset Management Unit driver +// +// Copyright (c) 2018 Linaro Ltd. +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> + +#include <linux/delay.h> +#include <linux/io.h>
This seems unnecessary, since register access is done via regmap.
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+
+#include "owl-reset.h"
+
+static int owl_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct owl_reset *reset = to_owl_reset(rcdev);
+ const struct owl_reset_map *map = &reset->reset_map[id];
+ u32 reg;
+
+ regmap_read(reset->regmap, map->reg, ®);
+ regmap_write(reset->regmap, map->reg, reg & ~map->bit);This read-modify-write sequence needs locking against concurrent register access. Better use regmap_update_bits(): return regmap_update_bits(reset->regmap, map->reg, map->bit, 0);
+
+ return 0;
+}
+
+static int owl_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct owl_reset *reset = to_owl_reset(rcdev);
+ const struct owl_reset_map *map = &reset->reset_map[id];
+ u32 reg;
+
+ regmap_read(reset->regmap, map->reg, ®);
+ regmap_write(reset->regmap, map->reg, reg | map->bit);Better: return regmap_update_bits(reset->regmap, map->reg, map->bit, map->bit);
+
+ return 0;
+}
+
+static int owl_reset_reset(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ owl_reset_assert(rcdev, id);
+ udelay(1);Is the delay valid for all IP cores on all SoCs variants?
+ owl_reset_deassert(rcdev, id);
+
+ return 0;
+}
+
+static int owl_reset_status(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct owl_reset *reset = to_owl_reset(rcdev);
+ const struct owl_reset_map *map = &reset->reset_map[id];
+ u32 reg;
+
+ regmap_read(reset->regmap, map->reg, ®);If this could return an error, better report it.
quoted hunk ↗ jump to hunk
+ + /* + * The reset control API expects 0 if reset is not asserted, + * which is the opposite of what our hardware uses. + */ + return !(map->bit & reg); +} + +const struct reset_control_ops owl_reset_ops = { + .assert = owl_reset_assert, + .deassert = owl_reset_deassert, + .reset = owl_reset_reset, + .status = owl_reset_status, +};diff --git a/drivers/clk/actions/owl-reset.h b/drivers/clk/actions/owl-reset.h new file mode 100644 index 000000000000..1a5e987ba99b --- /dev/null +++ b/drivers/clk/actions/owl-reset.h@@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// Actions Semi Owl SoCs Reset Management Unit driver +// +// Copyright (c) 2018 Linaro Ltd. +// Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> + +#ifndef _OWL_RESET_H_ +#define _OWL_RESET_H_ + +#include <linux/reset-controller.h> +#include <linux/spinlock.h>
spinlock?
+
+struct owl_reset_map {
+ u16 reg;Note that this will be aligned to 32-bits. If the intention was to save space, consider storing an u8 bit index instead of the mask.
+ u32 bit;
+};
+
+struct owl_reset {
+ struct reset_controller_dev rcdev;
+ struct owl_reset_map *reset_map;Could this be const struct owl_reset_map *reset_map; ?
+ struct regmap *regmap;
+};
+
+static inline struct owl_reset *to_owl_reset(struct reset_controller_dev *rcdev)
+{
+ return container_of(rcdev, struct owl_reset, rcdev);
+}
+
+extern const struct reset_control_ops owl_reset_ops;
+
+#endif /* _OWL_RESET_H_ */regards Philipp