On 04.05.21 13:22, Peng Fan (OSS) wrote:

quoted

Subject: Re: [PATCH V2 3/4] soc: imx: Add generic blk-ctl driver

On 30.04.21 07:27, Peng Fan (OSS) wrote:

quoted

From: Peng Fan <peng.fan@nxp.com>

The i.MX8MM introduces an IP named BLK_CTL and usually is comprised of
some GPRs.

The GPRs has some clock bits and reset bits, but here we take it as
virtual PDs, because of the clock and power domain A/B lock issue when
taking it as a clock controller.

For some bits, it might be good to also make it as a reset controller,
but to i.MX8MM, we not add that support for now.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
  drivers/soc/imx/Makefile  |   2 +-
  drivers/soc/imx/blk-ctl.c | 303

++++++++++++++++++++++++++++++++++++++

quoted

  drivers/soc/imx/blk-ctl.h |  76 ++++++++++
  3 files changed, 380 insertions(+), 1 deletion(-)
  create mode 100644 drivers/soc/imx/blk-ctl.c
  create mode 100644 drivers/soc/imx/blk-ctl.h

[...]

quoted

quoted

+
+static int imx_blk_ctl_attach_pd(struct device *dev, struct device **devs,

char **pd_names,

quoted

+				 u32 num_pds)
+{
+	int i, ret;
+
+	if (!pd_names)
+		return 0;
+
+	if (dev->pm_domain) {
+		devs[0] = dev;
+		pm_runtime_enable(dev);
+		return 0;
+	}
+
+	for (i = 0; i < num_pds; i++) {
+		devs[i] = dev_pm_domain_attach_by_name(dev, pd_names[i]);
+		if (IS_ERR_OR_NULL(devs[i])) {
+			ret = PTR_ERR(devs[i]) ? : -ENODATA;
+			goto detach_pm;
+		}
+	}
+
+	return 0;
+
+detach_pm:
+	for (i--; i >= 0; i--)
+		dev_pm_domain_detach(devs[i], false);

It looks like you should add pm_runtime_disable() in this error path to not
leave the pm_runtime_enable() unmatched.

I might need to remove pm runtime, since no the ops callback here does nothing.

Anyway, my comment is nonsense as you return success right after pm_runtime_enable().