| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <31b7293f-662d-4a94-1717-9c76d7f33840@microchip.com>
Date: Fri, 27 May 2022 18:40:59 +0000
From: <Conor.Dooley@...rochip.com>
To: <sboyd@...nel.org>
CC: <Daire.McNamara@...rochip.com>, <geert@...ux-m68k.org>,
<linux-clk@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<mturquette@...libre.com>, <p.zabel@...gutronix.de>
Subject: Reset controller within a clock driver
Hi Stephen,
After I sent the fix for the broken resets in clk/microchip/clk-mpfs.c,
[0] I started looking at making a proper reset controller driver a la
clk/renesas/{renesas-cpg-mssr,rzgl2l-cpg}.c where the reset controller
is part of the clock driver file.
I did it that way b/c the reset controller is just a single reg,
surrounded by registers used by clocks. It's roughly a +130,-10 line
change to the existing driver. A /very/ rough version that will not
apply without other cleanup is appended for context.
Before I got around to testing properly and cleaning it up for
submission, I saw a mail you had sent and wondered if I'd gone for the
wrong approach [1].
Should I instead have my clock driver create a device for the reset
controller to bind to, or is that overkill for a single register &
Serge's situation is different b/c he'd created a file purely for
a reset controller?
Thanks,
Conor.
0 - https://lore.kernel.org/linux-clk/20220411072340.740981-1-conor.dooley@microchip.com/
1 - https://lore.kernel.org/linux-clk/20220517073729.2FAE2C385B8@smtp.kernel.org/
diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index ce3a48472fba..d9d1a4d9f131 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -9,6 +9,7 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
#include <linux/slab.h>
#include <dt-bindings/clock/microchip,mpfs-clock.h>
@@ -29,7 +30,13 @@
#define MSSPLL_POSTDIV_WIDTH 0x07u
#define MSSPLL_FIXED_DIV 4u
+#define MPFS_PERIPH_OFFSET 3u
+
struct mpfs_clock_data {
+ struct device *dev;
+#ifdef CONFIG_RESET_CONTROLLER
+ struct reset_controller_dev rcdev;
+#endif
void __iomem *base;
void __iomem *msspll_base;
struct clk_hw_onecell_data hw_data;
@@ -344,10 +351,6 @@ static int mpfs_periph_clk_enable(struct clk_hw *hw)
spin_lock_irqsave(&mpfs_clk_lock, flags);
- reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
- val = reg & ~(1u << periph->shift);
- writel_relaxed(val, base_addr + REG_SUBBLK_RESET_CR);
-
reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
val = reg | (1u << periph->shift);
writel_relaxed(val, base_addr + REG_SUBBLK_CLOCK_CR);
@@ -381,12 +384,9 @@ static int mpfs_periph_clk_is_enabled(struct clk_hw *hw)
void __iomem *base_addr = periph_hw->base;
u32 reg;
- reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
- if ((reg & (1u << periph->shift)) == 0u) {
- reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
- if (reg & (1u << periph->shift))
- return 1;
- }
+ reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
+ if (reg & (1u << periph->shift))
+ return 1;
return 0;
}
@@ -472,6 +472,118 @@ static int mpfs_clk_register_periphs(struct device *dev, struct mpfs_periph_hw_c
return 0;
}
+/*
+ * Peripheral clock resets
+ *
+ * CLK_RESERVED does not map to a clock, but it does map to a reset line, so it
+ * has to be accounted for here.
+ *
+ */
+
+#ifdef CONFIG_RESET_CONTROLLER
+
+#define rcdev_to_clock_data(x) container_of((x), struct mpfs_clock_data, rcdev)
+
+// static int mpfs_reset(struct reset_controller_dev *rcdev, unsigned long id)
+// {
+// struct mpfs_clock_data *clk_data = rcdev_to_clock_data(rcdev);
+//
+//
+//
+//
+//
+//
+//
+//
+//
+//
+//
+//
+//
+//
+//
+// return 0;
+// }
+
+static int mpfs_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ struct mpfs_clock_data *clk_data = rcdev_to_clock_data(rcdev);
+ u32 reg, val;
+
+ reg = readl_relaxed(clk_data->base + REG_SUBBLK_RESET_CR);
+ val = reg | (1u << id);
+ writel_relaxed(val, clk_data->base + REG_SUBBLK_RESET_CR);
+
+ dev_dbg(clk_data->dev, "deassert reset: %02lu\n", id);
+ return 0;
+}
+
+static int mpfs_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ struct mpfs_clock_data *clk_data = rcdev_to_clock_data(rcdev);
+ u32 reg, val;
+
+ reg = readl_relaxed(clk_data->base + REG_SUBBLK_RESET_CR);
+ val = reg & ~(1u << id);
+ writel_relaxed(val, clk_data->base + REG_SUBBLK_RESET_CR);
+
+ dev_dbg(clk_data->dev, "deassert reset: %02lu\n", id);
+
+ return 0;
+}
+
+static int mpfs_status(struct reset_controller_dev *rcdev, unsigned long id)
+{
+ struct mpfs_clock_data *clk_data = rcdev_to_clock_data(rcdev);
+ u32 reg;
+
+ reg = readl_relaxed(clk_data->base + REG_SUBBLK_RESET_CR);
+ return (reg & (1u << id));
+}
+
+ // .reset = mpfs_reset,
+static const struct reset_control_ops mpfs_reset_ops = {
+ .assert = mpfs_assert,
+ .deassert = mpfs_deassert,
+ .status = mpfs_status,
+};
+
+//geert - does it make sense to reuse the clk_ indexes for the reset ctrlr?
+// -> they run from 3 to 32 but skip one
+//if yes, do i p much just subtract 3 in of_xlate & manipulate that bit?
+static int mpfs_reset_xlate(struct reset_controller_dev *rcdev,
+ const struct of_phandle_args *reset_spec)
+{
+ struct mpfs_clock_data *clk_data = rcdev_to_clock_data(rcdev);
+ unsigned int index = reset_spec->args[0];
+ /* account for reserved fpga fabric reset */
+ unsigned int num_resets = ARRAY_SIZE(mpfs_periph_clks) + 1;
+
+ if (index < MPFS_PERIPH_OFFSET || index > (MPFS_PERIPH_OFFSET + num_resets)) {
+ dev_err(clk_data->dev, "Invalid reset index %u\n", reset_spec->args[0]);
+ return -EINVAL;
+ }
+
+ return index - MPFS_PERIPH_OFFSET;
+}
+static int mpfs_reset_controller_register(struct mpfs_clock_data *clk_data)
+{
+ clk_data->rcdev.ops = &mpfs_reset_ops;
+ clk_data->rcdev.of_node = clk_data->dev->of_node;
+ clk_data->rcdev.of_reset_n_cells = 1;
+ clk_data->rcdev.of_xlate = mpfs_reset_xlate;
+ /* CLK_RESERVED is not part of mpfs_periph_clks, so add 1 */
+ clk_data->rcdev.nr_resets = ARRAY_SIZE(mpfs_periph_clks) + 1;
+ return devm_reset_controller_register(clk_data->dev, &clk_data->rcdev);
+}
+
+#else /* !CONFIG_RESET_CONTROLLER */
+static int mpfs_reset_controller_register(struct mpfs_clock_data *clk_data)
+{
+ return 0;
+}
+#endif /* !CONFIG_RESET_CONTROLLER */
+
static int mpfs_clk_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -496,6 +608,7 @@ static int mpfs_clk_probe(struct platform_device *pdev)
return PTR_ERR(clk_data->msspll_base);
clk_data->hw_data.num = num_clks;
+ clk_data->dev = dev;
ret = mpfs_clk_register_mssplls(dev, mpfs_msspll_clks, ARRAY_SIZE(mpfs_msspll_clks),
clk_data);
@@ -515,6 +628,10 @@ static int mpfs_clk_probe(struct platform_device *pdev)
if (ret)
return ret;
+ ret = mpfs_reset_controller_register(clk_data);
+ if (ret)
+ return ret;
+
return ret;
}
Powered by blists - more mailing lists