Messages in this thread Patch in this message | | | From | <> | Subject | Reset controller within a clock driver | Date | Fri, 27 May 2022 18:40:59 +0000 |
| |
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; }
| |