Subject: Re: [PATCH v3 9/9] clk: imx: Add the pcc reset controller support on
imx8ulp

On 21-09-14 14:52:08, Jacky Bai wrote:

quoted

diff --git a/drivers/clk/imx/clk-imx8ulp.c

b/drivers/clk/imx/clk-imx8ulp.c index 6aad04114658..6699437e17b8
100644

--- a/drivers/clk/imx/clk-imx8ulp.c
+++ b/drivers/clk/imx/clk-imx8ulp.c

@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/reset-controller.h>
 #include <linux/slab.h>

 #include "clk.h"

@@ -48,6 +49,99 @@ static const char * const nic_per_divplat[] = {

"nic_per_divplat" };  static const char * const lpav_axi_div[] = {
"lpav_axi_div" };  static const char * const lpav_bus_div[] = {
"lpav_bus_div" };

+struct pcc_reset_dev {
+	void __iomem *base;
+	struct reset_controller_dev rcdev;
+	const u32 *resets;
+	/* Set to imx_ccm_lock to protect register access shared with clock

control */

quoted

+	spinlock_t *lock;
+};
+
+#define PCC_SW_RST	BIT(28)
+#define to_pcc_reset_dev(_rcdev)	container_of(_rcdev, struct

pcc_reset_dev, rcdev)

quoted

+
+static const u32 pcc3_resets[] = {
+	0xa8, 0xac, 0xc8, 0xcc, 0xd0,
+	0xd4, 0xd8, 0xdc, 0xe0, 0xe4,
+	0xe8, 0xec, 0xf0
+};
+
+static const u32 pcc4_resets[] = {
+	0x4, 0x8, 0xc, 0x10, 0x14,
+	0x18, 0x1c, 0x20, 0x24, 0x34,
+	0x38, 0x3c, 0x40, 0x44, 0x48,
+	0x4c, 0x54
+};
+
+static const u32 pcc5_resets[] = {
+	0xa0, 0xa4, 0xa8, 0xac, 0xb0,
+	0xb4, 0xbc, 0xc0, 0xc8, 0xcc,
+	0xd0, 0xf0, 0xf4, 0xf8
+};
+

I believe this could be avoided entirely by having something like:

PCCx_RESETS_OFFSET + (index * 4)

and then:

#define PCC3_RESETS_OFFSET	0xa8
#define PCC4_RESETS_OFFSET	0x4
#define PCC5_RESETS_OFFSET	0xa0

#define PCC3_RESETS_NUM		13
#define PCC4_RESETS_NUM		17
#define PCC5_RESETS_NUM		14

Then we could pass on the PCCx_RESETS_OFFSET and the
PCCx_RESETS_NUM and that's it.

As you can see that the PCC offset that has reset is not consecutive in the register region.
So it will introduce hole in the index. And we can NOT use linear formula to calculate the offset directly.

BR
Jacky Bai

quoted

+static int imx8ulp_pcc_assert(struct reset_controller_dev *rcdev,
+unsigned long id) {
+	struct pcc_reset_dev *pcc_reset = to_pcc_reset_dev(rcdev);
+	u32 offset = pcc_reset->resets[id];
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(pcc_reset->lock, flags);
+
+	val = readl(pcc_reset->base + offset);
+	val &= ~PCC_SW_RST;
+	writel(val, pcc_reset->base + offset);
+
+	spin_unlock_irqrestore(pcc_reset->lock, flags);
+
+	return 0;
+}
+
+static int imx8ulp_pcc_deassert(struct reset_controller_dev *rcdev,
+unsigned long id) {
+	struct pcc_reset_dev *pcc_reset = to_pcc_reset_dev(rcdev);
+	u32 offset = pcc_reset->resets[id];
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(pcc_reset->lock, flags);
+
+	val = readl(pcc_reset->base + offset);
+	val |= PCC_SW_RST;
+	writel(val, pcc_reset->base + offset);
+
+	spin_unlock_irqrestore(pcc_reset->lock, flags);
+
+	return 0;
+}
+
+static const struct reset_control_ops imx8ulp_pcc_reset_ops = {
+	.assert = imx8ulp_pcc_assert,
+	.deassert = imx8ulp_pcc_deassert,
+};
+
+static int imx8ulp_pcc_reset_init(struct platform_device *pdev, void

__iomem *base,

quoted

+	 const u32 *resets, unsigned int nr_resets) {
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct pcc_reset_dev *pcc_reset;
+
+	pcc_reset = devm_kzalloc(dev, sizeof(*pcc_reset), GFP_KERNEL);
+	if (!pcc_reset)
+		return -ENOMEM;
+
+	pcc_reset->base = base;
+	pcc_reset->lock = &imx_ccm_lock;
+	pcc_reset->resets = resets;
+	pcc_reset->rcdev.owner = THIS_MODULE;
+	pcc_reset->rcdev.nr_resets = nr_resets;
+	pcc_reset->rcdev.ops = &imx8ulp_pcc_reset_ops;
+	pcc_reset->rcdev.of_node = np;
+
+	return devm_reset_controller_register(dev, &pcc_reset->rcdev); }
+

...

quoted

--
2.26.2