[PATCH v2 0/2] pmdomain: rockchip: fix the probe error and deferral paths

[PATCH v2 0/2] pmdomain: rockchip: fix the probe error and deferral paths

[Arun Iyengar]

Two fixes for what happens when a Rockchip power domain clock is not
available yet and rockchip_pm_domain_probe() defers. They are based on
patch 1/2 "pmdomain: rockchip: Only put clocks that have been
successfully obtained" of Yao Zi's series [1], which fixes the
WARN_ON_ONCE() in __clk_put() on that path and has to be applied first.

1. The error cleanup removes the parent/child links before the domains.
   pm_genpd_remove() refuses a domain that is still a parent, and on
   RK3588 (RKVDEC0/1 are children of both VCODEC and VDPU) no ordering
   avoids that, so devm freed domains that were still on gpd_list.
2. Errors are reported once, where they happen, with dev_err_probe().
   A deferral is no longer printed as an error, once per probe attempt
   by rockchip_pm_add_one_domain() and once or twice by its callers,
   its reason is recorded for devices_deferred, and the paths that
   relied on the callers' message (clk_bulk_prepare(), pm_qos) now say
   what failed.

Patch 2 supersedes patch 2/2 "pmdomain: rockchip: Don't log when
of_clk_get() fails with -EPROBE_DEFER" of [1]. That patch keeps the
callers' messages and skips them for -EPROBE_DEFER, as v1 of this patch
did; in the review of v1 Sebastian asked to drop the callers' messages
and use dev_err_probe() in the error paths instead, which is what patch
2 does, including the of_clk_get() path that 2/2 of [1] converts.

Tested on an Orange Pi 5 (RK3588S) with v1 of this series on a v7.3
based kernel, with a test device tree that gives PD_FEC a clock which
only appears about 26 s into boot. PD_FEC is nested under PD_VI and is
reached after 18 domains and their links were registered, so every
deferral runs the cleanup with nested domains present. devices_deferred
shows "failed to get clk at index 5", there is no "failed to remove
domain", list or slab corruption report (booted with page_poison=1
init_on_free=1), no "-517" error lines, and the controller binds later
with each of the 26 domains listed once in pm_genpd_summary. The same
late clock on PD_NPU (the first domain, so nothing is registered yet
when it defers) was only booted without these two patches; those boots
are where the "-517" lines quoted in patch 2 come from. A host-side
model of the probe and cleanup on the board's DTB, deferring at each
domain in turn, leaves freed domains on gpd_list at 24 of 26 failure
points without patch 1 and at none with it.

[1] https://lore.kernel.org/r/20261105001257.1368266-1-me@ziyao.cc

This series was developed with Claude (Anthropic) as a coding assistant:
it wrote the code of both patches and the commit messages, and I
reviewed every change and tested it as described above.

Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
Changes in v2:
- Dropped 1/3 "pmdomain: rockchip: put only the clocks that were got on
  error" (Igor Pecovnik) in favour of Yao Zi's earlier fix for the same
  bug, 1/2 of [1] (same Fixes tag, the same clocks are put). The series
  is now based on that patch, and the dev_err_probe() for a failed
  of_clk_get() that was part of the dropped patch is now in 2/2.
- 2/2 (was 3/3): drop the "failed to handle (subdomain) node" messages
  from the callers instead of skipping them for -EPROBE_DEFER, and
  report each error once where it happens with dev_err_probe(),
  including the clk_bulk_prepare() and pm_qos paths that relied on the
  callers' message; new subject (Sebastian Reichel)
- 2/2: a failed syscon_node_to_regmap() for a pm_qos node now returns
  its own error (it can be -EPROBE_DEFER) instead of -ENODEV
- 1/2 (was 2/3): unchanged
- Cc Yao Zi
- Not re-tested on the board. For the tested case (-EPROBE_DEFER from
  of_clk_get()) v2 differs from what was tested only in where the
  clocks are put (the err_put_clocks label, 1/2 of [1], instead of next
  to of_clk_get()) and in the callers' messages, which v1 skipped for
  -EPROBE_DEFER and v2 no longer has. The other changes in 2/2 are on
  error paths the test did not reach. v2 is build-tested (W=1, arm64).
- Based on v7.4-rc2; with 1/2 of [1] the series applies to the pmdomain
  fixes branch as is. On the next branch, 1/2 of [1] conflicts with
  "pmdomain: rockchip: cycle an optional power-domain reset on power-on"
  (a reset_control_put() added after the changed line), and 2/2
  conflicts with it in context only; git am -3 resolves the latter.
- Link to v1: https://patch.msgid.link/20261110-pmdomain-rockchip-v1-0-6b9fa6ebaa86@gmail.com

---
Arun Iyengar (2):
      pmdomain: rockchip: remove subdomain links before the domains on error
      pmdomain: rockchip: report domain errors once, with dev_err_probe()

 drivers/pmdomain/rockchip/pm-domains.c | 144 ++++++++++++++++++++-------------
 1 file changed, 89 insertions(+), 55 deletions(-)
---
base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
change-id: 20261110-pmdomain-rockchip-3497a2e7fdf5
prerequisite-patch-id: c2f0c5eb6651baa744f66342db63485a6685fb71

Best regards,
--  
Arun Iyengar <aiyengar1990@gmail.com>

[PATCH v2 0/2] pmdomain: rockchip: fix the probe error and deferral paths

[Arun Iyengar via B4 Relay]

Two fixes for what happens when a Rockchip power domain clock is not
available yet and rockchip_pm_domain_probe() defers. They are based on
patch 1/2 "pmdomain: rockchip: Only put clocks that have been
successfully obtained" of Yao Zi's series [1], which fixes the
WARN_ON_ONCE() in __clk_put() on that path and has to be applied first.

1. The error cleanup removes the parent/child links before the domains.
   pm_genpd_remove() refuses a domain that is still a parent, and on
   RK3588 (RKVDEC0/1 are children of both VCODEC and VDPU) no ordering
   avoids that, so devm freed domains that were still on gpd_list.
2. Errors are reported once, where they happen, with dev_err_probe().
   A deferral is no longer printed as an error, once per probe attempt
   by rockchip_pm_add_one_domain() and once or twice by its callers,
   its reason is recorded for devices_deferred, and the paths that
   relied on the callers' message (clk_bulk_prepare(), pm_qos) now say
   what failed.

Patch 2 supersedes patch 2/2 "pmdomain: rockchip: Don't log when
of_clk_get() fails with -EPROBE_DEFER" of [1]. That patch keeps the
callers' messages and skips them for -EPROBE_DEFER, as v1 of this patch
did; in the review of v1 Sebastian asked to drop the callers' messages
and use dev_err_probe() in the error paths instead, which is what patch
2 does, including the of_clk_get() path that 2/2 of [1] converts.

Tested on an Orange Pi 5 (RK3588S) with v1 of this series on a v7.3
based kernel, with a test device tree that gives PD_FEC a clock which
only appears about 26 s into boot. PD_FEC is nested under PD_VI and is
reached after 18 domains and their links were registered, so every
deferral runs the cleanup with nested domains present. devices_deferred
shows "failed to get clk at index 5", there is no "failed to remove
domain", list or slab corruption report (booted with page_poison=1
init_on_free=1), no "-517" error lines, and the controller binds later
with each of the 26 domains listed once in pm_genpd_summary. The same
late clock on PD_NPU (the first domain, so nothing is registered yet
when it defers) was only booted without these two patches; those boots
are where the "-517" lines quoted in patch 2 come from. A host-side
model of the probe and cleanup on the board's DTB, deferring at each
domain in turn, leaves freed domains on gpd_list at 24 of 26 failure
points without patch 1 and at none with it.

[1] https://lore.kernel.org/r/20261105001257.1368266-1-me@ziyao.cc

This series was developed with Claude (Anthropic) as a coding assistant:
it wrote the code of both patches and the commit messages, and I
reviewed every change and tested it as described above.

Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
Changes in v2:
- Dropped 1/3 "pmdomain: rockchip: put only the clocks that were got on
  error" (Igor Pecovnik) in favour of Yao Zi's earlier fix for the same
  bug, 1/2 of [1] (same Fixes tag, the same clocks are put). The series
  is now based on that patch, and the dev_err_probe() for a failed
  of_clk_get() that was part of the dropped patch is now in 2/2.
- 2/2 (was 3/3): drop the "failed to handle (subdomain) node" messages
  from the callers instead of skipping them for -EPROBE_DEFER, and
  report each error once where it happens with dev_err_probe(),
  including the clk_bulk_prepare() and pm_qos paths that relied on the
  callers' message; new subject (Sebastian Reichel)
- 2/2: a failed syscon_node_to_regmap() for a pm_qos node now returns
  its own error (it can be -EPROBE_DEFER) instead of -ENODEV
- 1/2 (was 2/3): unchanged
- Cc Yao Zi
- Not re-tested on the board. For the tested case (-EPROBE_DEFER from
  of_clk_get()) v2 differs from what was tested only in where the
  clocks are put (the err_put_clocks label, 1/2 of [1], instead of next
  to of_clk_get()) and in the callers' messages, which v1 skipped for
  -EPROBE_DEFER and v2 no longer has. The other changes in 2/2 are on
  error paths the test did not reach. v2 is build-tested (W=1, arm64).
- Based on v7.4-rc2; with 1/2 of [1] the series applies to the pmdomain
  fixes branch as is. On the next branch, 1/2 of [1] conflicts with
  "pmdomain: rockchip: cycle an optional power-domain reset on power-on"
  (a reset_control_put() added after the changed line), and 2/2
  conflicts with it in context only; git am -3 resolves the latter.
- Link to v1: https://patch.msgid.link/20261110-pmdomain-rockchip-v1-0-6b9fa6ebaa86@gmail.com

---
Arun Iyengar (2):
      pmdomain: rockchip: remove subdomain links before the domains on error
      pmdomain: rockchip: report domain errors once, with dev_err_probe()

 drivers/pmdomain/rockchip/pm-domains.c | 144 ++++++++++++++++++++-------------
 1 file changed, 89 insertions(+), 55 deletions(-)
---
base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
change-id: 20261110-pmdomain-rockchip-3497a2e7fdf5
prerequisite-patch-id: c2f0c5eb6651baa744f66342db63485a6685fb71

Best regards,
--  
Arun Iyengar <aiyengar1990@gmail.com>



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 0/2] pmdomain: rockchip: fix the probe error and deferral paths

[Arun Iyengar via B4 Relay]

Two fixes for what happens when a Rockchip power domain clock is not
available yet and rockchip_pm_domain_probe() defers. They are based on
patch 1/2 "pmdomain: rockchip: Only put clocks that have been
successfully obtained" of Yao Zi's series [1], which fixes the
WARN_ON_ONCE() in __clk_put() on that path and has to be applied first.

1. The error cleanup removes the parent/child links before the domains.
   pm_genpd_remove() refuses a domain that is still a parent, and on
   RK3588 (RKVDEC0/1 are children of both VCODEC and VDPU) no ordering
   avoids that, so devm freed domains that were still on gpd_list.
2. Errors are reported once, where they happen, with dev_err_probe().
   A deferral is no longer printed as an error, once per probe attempt
   by rockchip_pm_add_one_domain() and once or twice by its callers,
   its reason is recorded for devices_deferred, and the paths that
   relied on the callers' message (clk_bulk_prepare(), pm_qos) now say
   what failed.

Patch 2 supersedes patch 2/2 "pmdomain: rockchip: Don't log when
of_clk_get() fails with -EPROBE_DEFER" of [1]. That patch keeps the
callers' messages and skips them for -EPROBE_DEFER, as v1 of this patch
did; in the review of v1 Sebastian asked to drop the callers' messages
and use dev_err_probe() in the error paths instead, which is what patch
2 does, including the of_clk_get() path that 2/2 of [1] converts.

Tested on an Orange Pi 5 (RK3588S) with v1 of this series on a v7.3
based kernel, with a test device tree that gives PD_FEC a clock which
only appears about 26 s into boot. PD_FEC is nested under PD_VI and is
reached after 18 domains and their links were registered, so every
deferral runs the cleanup with nested domains present. devices_deferred
shows "failed to get clk at index 5", there is no "failed to remove
domain", list or slab corruption report (booted with page_poison=1
init_on_free=1), no "-517" error lines, and the controller binds later
with each of the 26 domains listed once in pm_genpd_summary. The same
late clock on PD_NPU (the first domain, so nothing is registered yet
when it defers) was only booted without these two patches; those boots
are where the "-517" lines quoted in patch 2 come from. A host-side
model of the probe and cleanup on the board's DTB, deferring at each
domain in turn, leaves freed domains on gpd_list at 24 of 26 failure
points without patch 1 and at none with it.

[1] https://lore.kernel.org/r/20261105001257.1368266-1-me@ziyao.cc

This series was developed with Claude (Anthropic) as a coding assistant:
it wrote the code of both patches and the commit messages, and I
reviewed every change and tested it as described above.

Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
Changes in v2:
- Dropped 1/3 "pmdomain: rockchip: put only the clocks that were got on
  error" (Igor Pecovnik) in favour of Yao Zi's earlier fix for the same
  bug, 1/2 of [1] (same Fixes tag, the same clocks are put). The series
  is now based on that patch, and the dev_err_probe() for a failed
  of_clk_get() that was part of the dropped patch is now in 2/2.
- 2/2 (was 3/3): drop the "failed to handle (subdomain) node" messages
  from the callers instead of skipping them for -EPROBE_DEFER, and
  report each error once where it happens with dev_err_probe(),
  including the clk_bulk_prepare() and pm_qos paths that relied on the
  callers' message; new subject (Sebastian Reichel)
- 2/2: a failed syscon_node_to_regmap() for a pm_qos node now returns
  its own error (it can be -EPROBE_DEFER) instead of -ENODEV
- 1/2 (was 2/3): unchanged
- Cc Yao Zi
- Not re-tested on the board. For the tested case (-EPROBE_DEFER from
  of_clk_get()) v2 differs from what was tested only in where the
  clocks are put (the err_put_clocks label, 1/2 of [1], instead of next
  to of_clk_get()) and in the callers' messages, which v1 skipped for
  -EPROBE_DEFER and v2 no longer has. The other changes in 2/2 are on
  error paths the test did not reach. v2 is build-tested (W=1, arm64).
- Based on v7.4-rc2; with 1/2 of [1] the series applies to the pmdomain
  fixes branch as is. On the next branch, 1/2 of [1] conflicts with
  "pmdomain: rockchip: cycle an optional power-domain reset on power-on"
  (a reset_control_put() added after the changed line), and 2/2
  conflicts with it in context only; git am -3 resolves the latter.
- Link to v1: https://patch.msgid.link/20261110-pmdomain-rockchip-v1-0-6b9fa6ebaa86@gmail.com

---
Arun Iyengar (2):
      pmdomain: rockchip: remove subdomain links before the domains on error
      pmdomain: rockchip: report domain errors once, with dev_err_probe()

 drivers/pmdomain/rockchip/pm-domains.c | 144 ++++++++++++++++++++-------------
 1 file changed, 89 insertions(+), 55 deletions(-)
---
base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
change-id: 20261110-pmdomain-rockchip-3497a2e7fdf5
prerequisite-patch-id: c2f0c5eb6651baa744f66342db63485a6685fb71

Best regards,
--  
Arun Iyengar <aiyengar1990@gmail.com>

[PATCH v2 1/2] pmdomain: rockchip: remove subdomain links before the domains on error

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

When rockchip_pm_domain_probe() fails after some domains were
registered, for example because a clock of a nested domain is not
available yet and returns -EPROBE_DEFER, rockchip_pm_domain_cleanup()
calls pm_genpd_remove() for every registered domain in domain id order.

pm_genpd_remove() refuses a domain that is still the parent of another
domain and returns -EBUSY. The cleanup walks parents before their
children in id order, and on RK3588 RKVDEC0/RKVDEC1 (ids 14 and 15) are
also children of VDPU (id 21), so no ordering of the domain array lets
every removal succeed. Each refused parent only gets the "state may be
inconsistent" message, and on return devm frees its struct
rockchip_pm_domain, the embedded generic_pm_domain included, while it
is still linked on the global gpd_list. The next registration, the
unused domain power off at late_initcall and the genpd debugfs files
then walk freed memory.

Remove the parent/child links first, walking the power domain nodes
depth first in the same way rockchip_pm_add_subdomain() added them, so
that each child no longer has subdomains of its own when its link is
removed. With no links left, and no provider or devices attached in the
probe error path, pm_genpd_remove() succeeds for every domain before
devm frees it. pm_genpd_remove_subdomain() also drops the parent's
subdomain count, which removing the child with pm_genpd_remove() alone
does not.

Fixes: 79d12b7a7f22 ("soc: rockchip: power-domain: use pm_genpd_remove in error cleanup")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 46 ++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 86b5074b0469..00c7e78f89b5 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -955,12 +955,58 @@ static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
 	/* devm will free our memory */
 }
 
+static struct generic_pm_domain *
+rockchip_pm_node_to_genpd(struct rockchip_pmu *pmu, struct device_node *node)
+{
+	u32 id;
+
+	if (of_property_read_u32(node, "reg", &id) ||
+	    id >= pmu->genpd_data.num_domains)
+		return NULL;
+
+	return pmu->genpd_data.domains[id];
+}
+
+/*
+ * Undo the pm_genpd_add_subdomain() calls of rockchip_pm_add_subdomain().
+ * Links are only ever added between a DT node and its children, so walking
+ * the tree depth first and removing the links of the children before the
+ * link to their parent always finds the subdomain without subdomains of its
+ * own, as pm_genpd_remove_subdomain() requires. This also covers domains with
+ * more than one parent (RKVDEC0/RKVDEC1 on RK3588), which a walk in domain id
+ * or registration order does not.
+ */
+static void rockchip_pm_remove_subdomains(struct rockchip_pmu *pmu,
+					  struct device_node *parent)
+{
+	struct generic_pm_domain *parent_domain, *child_domain;
+
+	parent_domain = rockchip_pm_node_to_genpd(pmu, parent);
+
+	for_each_child_of_node_scoped(parent, np) {
+		rockchip_pm_remove_subdomains(pmu, np);
+
+		child_domain = rockchip_pm_node_to_genpd(pmu, np);
+		if (parent_domain && child_domain)
+			pm_genpd_remove_subdomain(parent_domain, child_domain);
+	}
+}
+
 static void rockchip_pm_domain_cleanup(struct rockchip_pmu *pmu)
 {
 	struct generic_pm_domain *genpd;
 	struct rockchip_pm_domain *pd;
 	int i;
 
+	/*
+	 * pm_genpd_remove() refuses a domain that still has subdomains, and
+	 * devm frees the domains on return whether or not they were removed,
+	 * which would leave freed memory on the global genpd list. Remove all
+	 * parent/child links first, so every domain can be removed.
+	 */
+	for_each_available_child_of_node_scoped(pmu->dev->of_node, np)
+		rockchip_pm_remove_subdomains(pmu, np);
+
 	for (i = 0; i < pmu->genpd_data.num_domains; i++) {
 		genpd = pmu->genpd_data.domains[i];
 		if (genpd) {

-- 
2.55.0

[PATCH v2 1/2] pmdomain: rockchip: remove subdomain links before the domains on error

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

When rockchip_pm_domain_probe() fails after some domains were
registered, for example because a clock of a nested domain is not
available yet and returns -EPROBE_DEFER, rockchip_pm_domain_cleanup()
calls pm_genpd_remove() for every registered domain in domain id order.

pm_genpd_remove() refuses a domain that is still the parent of another
domain and returns -EBUSY. The cleanup walks parents before their
children in id order, and on RK3588 RKVDEC0/RKVDEC1 (ids 14 and 15) are
also children of VDPU (id 21), so no ordering of the domain array lets
every removal succeed. Each refused parent only gets the "state may be
inconsistent" message, and on return devm frees its struct
rockchip_pm_domain, the embedded generic_pm_domain included, while it
is still linked on the global gpd_list. The next registration, the
unused domain power off at late_initcall and the genpd debugfs files
then walk freed memory.

Remove the parent/child links first, walking the power domain nodes
depth first in the same way rockchip_pm_add_subdomain() added them, so
that each child no longer has subdomains of its own when its link is
removed. With no links left, and no provider or devices attached in the
probe error path, pm_genpd_remove() succeeds for every domain before
devm frees it. pm_genpd_remove_subdomain() also drops the parent's
subdomain count, which removing the child with pm_genpd_remove() alone
does not.

Fixes: 79d12b7a7f22 ("soc: rockchip: power-domain: use pm_genpd_remove in error cleanup")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 46 ++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 86b5074b0469..00c7e78f89b5 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -955,12 +955,58 @@ static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
 	/* devm will free our memory */
 }
 
+static struct generic_pm_domain *
+rockchip_pm_node_to_genpd(struct rockchip_pmu *pmu, struct device_node *node)
+{
+	u32 id;
+
+	if (of_property_read_u32(node, "reg", &id) ||
+	    id >= pmu->genpd_data.num_domains)
+		return NULL;
+
+	return pmu->genpd_data.domains[id];
+}
+
+/*
+ * Undo the pm_genpd_add_subdomain() calls of rockchip_pm_add_subdomain().
+ * Links are only ever added between a DT node and its children, so walking
+ * the tree depth first and removing the links of the children before the
+ * link to their parent always finds the subdomain without subdomains of its
+ * own, as pm_genpd_remove_subdomain() requires. This also covers domains with
+ * more than one parent (RKVDEC0/RKVDEC1 on RK3588), which a walk in domain id
+ * or registration order does not.
+ */
+static void rockchip_pm_remove_subdomains(struct rockchip_pmu *pmu,
+					  struct device_node *parent)
+{
+	struct generic_pm_domain *parent_domain, *child_domain;
+
+	parent_domain = rockchip_pm_node_to_genpd(pmu, parent);
+
+	for_each_child_of_node_scoped(parent, np) {
+		rockchip_pm_remove_subdomains(pmu, np);
+
+		child_domain = rockchip_pm_node_to_genpd(pmu, np);
+		if (parent_domain && child_domain)
+			pm_genpd_remove_subdomain(parent_domain, child_domain);
+	}
+}
+
 static void rockchip_pm_domain_cleanup(struct rockchip_pmu *pmu)
 {
 	struct generic_pm_domain *genpd;
 	struct rockchip_pm_domain *pd;
 	int i;
 
+	/*
+	 * pm_genpd_remove() refuses a domain that still has subdomains, and
+	 * devm frees the domains on return whether or not they were removed,
+	 * which would leave freed memory on the global genpd list. Remove all
+	 * parent/child links first, so every domain can be removed.
+	 */
+	for_each_available_child_of_node_scoped(pmu->dev->of_node, np)
+		rockchip_pm_remove_subdomains(pmu, np);
+
 	for (i = 0; i < pmu->genpd_data.num_domains; i++) {
 		genpd = pmu->genpd_data.domains[i];
 		if (genpd) {

-- 
2.55.0



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 1/2] pmdomain: rockchip: remove subdomain links before the domains on error

[Arun Iyengar]

When rockchip_pm_domain_probe() fails after some domains were
registered, for example because a clock of a nested domain is not
available yet and returns -EPROBE_DEFER, rockchip_pm_domain_cleanup()
calls pm_genpd_remove() for every registered domain in domain id order.

pm_genpd_remove() refuses a domain that is still the parent of another
domain and returns -EBUSY. The cleanup walks parents before their
children in id order, and on RK3588 RKVDEC0/RKVDEC1 (ids 14 and 15) are
also children of VDPU (id 21), so no ordering of the domain array lets
every removal succeed. Each refused parent only gets the "state may be
inconsistent" message, and on return devm frees its struct
rockchip_pm_domain, the embedded generic_pm_domain included, while it
is still linked on the global gpd_list. The next registration, the
unused domain power off at late_initcall and the genpd debugfs files
then walk freed memory.

Remove the parent/child links first, walking the power domain nodes
depth first in the same way rockchip_pm_add_subdomain() added them, so
that each child no longer has subdomains of its own when its link is
removed. With no links left, and no provider or devices attached in the
probe error path, pm_genpd_remove() succeeds for every domain before
devm frees it. pm_genpd_remove_subdomain() also drops the parent's
subdomain count, which removing the child with pm_genpd_remove() alone
does not.

Fixes: 79d12b7a7f22 ("soc: rockchip: power-domain: use pm_genpd_remove in error cleanup")
Cc: stable@vger.kernel.org
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 46 ++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 86b5074b0469..00c7e78f89b5 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -955,12 +955,58 @@ static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
 	/* devm will free our memory */
 }
 
+static struct generic_pm_domain *
+rockchip_pm_node_to_genpd(struct rockchip_pmu *pmu, struct device_node *node)
+{
+	u32 id;
+
+	if (of_property_read_u32(node, "reg", &id) ||
+	    id >= pmu->genpd_data.num_domains)
+		return NULL;
+
+	return pmu->genpd_data.domains[id];
+}
+
+/*
+ * Undo the pm_genpd_add_subdomain() calls of rockchip_pm_add_subdomain().
+ * Links are only ever added between a DT node and its children, so walking
+ * the tree depth first and removing the links of the children before the
+ * link to their parent always finds the subdomain without subdomains of its
+ * own, as pm_genpd_remove_subdomain() requires. This also covers domains with
+ * more than one parent (RKVDEC0/RKVDEC1 on RK3588), which a walk in domain id
+ * or registration order does not.
+ */
+static void rockchip_pm_remove_subdomains(struct rockchip_pmu *pmu,
+					  struct device_node *parent)
+{
+	struct generic_pm_domain *parent_domain, *child_domain;
+
+	parent_domain = rockchip_pm_node_to_genpd(pmu, parent);
+
+	for_each_child_of_node_scoped(parent, np) {
+		rockchip_pm_remove_subdomains(pmu, np);
+
+		child_domain = rockchip_pm_node_to_genpd(pmu, np);
+		if (parent_domain && child_domain)
+			pm_genpd_remove_subdomain(parent_domain, child_domain);
+	}
+}
+
 static void rockchip_pm_domain_cleanup(struct rockchip_pmu *pmu)
 {
 	struct generic_pm_domain *genpd;
 	struct rockchip_pm_domain *pd;
 	int i;
 
+	/*
+	 * pm_genpd_remove() refuses a domain that still has subdomains, and
+	 * devm frees the domains on return whether or not they were removed,
+	 * which would leave freed memory on the global genpd list. Remove all
+	 * parent/child links first, so every domain can be removed.
+	 */
+	for_each_available_child_of_node_scoped(pmu->dev->of_node, np)
+		rockchip_pm_remove_subdomains(pmu, np);
+
 	for (i = 0; i < pmu->genpd_data.num_domains; i++) {
 		genpd = pmu->genpd_data.domains[i];
 		if (genpd) {

-- 
2.55.0

[PATCH v2 2/2] pmdomain: rockchip: report domain errors once, with dev_err_probe()

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

When a power domain clock is not available yet,
rockchip_pm_add_one_domain() returns -EPROBE_DEFER, and both it and its
callers rockchip_pm_domain_probe() and rockchip_pm_add_subdomain() print
that with dev_err():

  rockchip-pm-domain ...: power-domain: failed to get clk at index 1: -517
  rockchip-pm-domain ...: failed to handle node power-domain: -517

once per probe attempt, and the second line twice for a nested domain.
On an RK3588 board whose NPU domain clock appears late this is more than
20 error lines before the domain finally probes, and the reason of the
deferral is not recorded. Other failures are reported twice (once where
they happen and once by the callers), or only by the callers, which name
the node but not what failed: a missing pm_qos phandle and a pm_qos node
without a regmap print nothing in rockchip_pm_add_one_domain(), and a
failed clk_bulk_prepare() only has the message of the clk core, which
names neither the device nor the domain.

Report every error once, where it happens, with dev_err_probe(), and
drop the "failed to handle (subdomain) node" messages from the callers.
A deferral is then quiet and keeps its specific reason in
/sys/kernel/debug/devices_deferred, such as "failed to get clk at index
1", and -ENOMEM, which the callers used to print, is quiet as well. The
return values are unchanged, except that a failed
syscon_node_to_regmap() for a pm_qos node now returns its own error
instead of -ENODEV: it can be -EPROBE_DEFER (a syscon regmap that is
not registered yet, or a syscon clock or hwlock that is not available
yet), which must not turn into a probe failure.

Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 98 +++++++++++++++-------------------
 1 file changed, 43 insertions(+), 55 deletions(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 00c7e78f89b5..be077fb52f40 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -804,28 +804,22 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	int error;
 
 	error = of_property_read_u32(node, "reg", &id);
-	if (error) {
-		dev_err(pmu->dev,
-			"%pOFn: failed to retrieve domain id (reg): %d\n",
-			node, error);
-		return -EINVAL;
-	}
+	if (error)
+		return dev_err_probe(pmu->dev, -EINVAL,
+				     "%pOFn: failed to retrieve domain id (reg)\n",
+				     node);
 
-	if (id >= pmu->info->num_domains) {
-		dev_err(pmu->dev, "%pOFn: invalid domain id %d\n",
-			node, id);
-		return -EINVAL;
-	}
+	if (id >= pmu->info->num_domains)
+		return dev_err_probe(pmu->dev, -EINVAL,
+				     "%pOFn: invalid domain id %d\n", node, id);
 	/* RK3588 has domains with two parents (RKVDEC0/RKVDEC1) */
 	if (pmu->genpd_data.domains[id])
 		return 0;
 
 	pd_info = &pmu->info->domain_info[id];
-	if (!pd_info->pwr_mask && !pd_info->req_mask) {
-		dev_err(pmu->dev, "%pOFn: undefined domain id %d\n",
-			node, id);
-		return -EINVAL;
-	}
+	if (!pd_info->pwr_mask && !pd_info->req_mask)
+		return dev_err_probe(pmu->dev, -EINVAL,
+				     "%pOFn: undefined domain id %d\n", node, id);
 
 	pd = devm_kzalloc_obj(pmu->dev, *pd);
 	if (!pd)
@@ -849,17 +843,20 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	for (i = 0; i < pd->num_clks; i++) {
 		pd->clks[i].clk = of_clk_get(node, i);
 		if (IS_ERR(pd->clks[i].clk)) {
-			error = PTR_ERR(pd->clks[i].clk);
-			dev_err(pmu->dev,
-				"%pOFn: failed to get clk at index %d: %d\n",
-				node, i, error);
+			error = dev_err_probe(pmu->dev,
+					      PTR_ERR(pd->clks[i].clk),
+					      "%pOFn: failed to get clk at index %d\n",
+					      node, i);
 			goto err_put_clocks;
 		}
 	}
 
 	error = clk_bulk_prepare(pd->num_clks, pd->clks);
-	if (error)
+	if (error) {
+		dev_err_probe(pmu->dev, error,
+			      "%pOFn: failed to prepare clocks\n", node);
 		goto err_put_clocks;
+	}
 
 	pd->num_qos = of_count_phandle_with_args(node, "pm_qos",
 						 NULL);
@@ -884,13 +881,18 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 		for (j = 0; j < pd->num_qos; j++) {
 			qos_node = of_parse_phandle(node, "pm_qos", j);
 			if (!qos_node) {
-				error = -ENODEV;
+				error = dev_err_probe(pmu->dev, -ENODEV,
+						      "%pOFn: failed to get pm_qos %d\n",
+						      node, j);
 				goto err_unprepare_clocks;
 			}
 			pd->qos_regmap[j] = syscon_node_to_regmap(qos_node);
 			of_node_put(qos_node);
 			if (IS_ERR(pd->qos_regmap[j])) {
-				error = -ENODEV;
+				error = dev_err_probe(pmu->dev,
+						      PTR_ERR(pd->qos_regmap[j]),
+						      "%pOFn: failed to get pm_qos %d regmap\n",
+						      node, j);
 				goto err_unprepare_clocks;
 			}
 		}
@@ -1038,39 +1040,31 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
 		u32 idx;
 
 		error = of_property_read_u32(parent, "reg", &idx);
-		if (error) {
-			dev_err(pmu->dev,
-				"%pOFn: failed to retrieve domain id (reg): %d\n",
-				parent, error);
-			return error;
-		}
+		if (error)
+			return dev_err_probe(pmu->dev, error,
+					     "%pOFn: failed to retrieve domain id (reg)\n",
+					     parent);
 		parent_domain = pmu->genpd_data.domains[idx];
 
 		error = rockchip_pm_add_one_domain(pmu, np);
-		if (error) {
-			dev_err(pmu->dev, "failed to handle node %pOFn: %d\n",
-				np, error);
+		if (error)
 			return error;
-		}
 
 		error = of_property_read_u32(np, "reg", &idx);
-		if (error) {
-			dev_err(pmu->dev,
-				"%pOFn: failed to retrieve domain id (reg): %d\n",
-				np, error);
-			return error;
-		}
+		if (error)
+			return dev_err_probe(pmu->dev, error,
+					     "%pOFn: failed to retrieve domain id (reg)\n",
+					     np);
 		child_domain = pmu->genpd_data.domains[idx];
 
 		error = pm_genpd_add_subdomain(parent_domain, child_domain);
-		if (error) {
-			dev_err(pmu->dev, "%s failed to add subdomain %s: %d\n",
-				parent_domain->name, child_domain->name, error);
-			return error;
-		} else {
-			dev_dbg(pmu->dev, "%s add subdomain: %s\n",
-				parent_domain->name, child_domain->name);
-		}
+		if (error)
+			return dev_err_probe(pmu->dev, error,
+					     "%s failed to add subdomain %s\n",
+					     parent_domain->name, child_domain->name);
+
+		dev_dbg(pmu->dev, "%s add subdomain: %s\n",
+			parent_domain->name, child_domain->name);
 
 		error = rockchip_pm_add_subdomain(pmu, np);
 		if (error)
@@ -1141,18 +1135,12 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
 
 	for_each_available_child_of_node_scoped(np, node) {
 		error = rockchip_pm_add_one_domain(pmu, node);
-		if (error) {
-			dev_err(dev, "failed to handle node %pOFn: %d\n",
-				node, error);
+		if (error)
 			goto err_out;
-		}
 
 		error = rockchip_pm_add_subdomain(pmu, node);
-		if (error < 0) {
-			dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
-				node, error);
+		if (error < 0)
 			goto err_out;
-		}
 	}
 
 	if (error) {

-- 
2.55.0

[PATCH v2 2/2] pmdomain: rockchip: report domain errors once, with dev_err_probe()

[Arun Iyengar via B4 Relay]

From: Arun Iyengar <aiyengar1990@gmail.com>

When a power domain clock is not available yet,
rockchip_pm_add_one_domain() returns -EPROBE_DEFER, and both it and its
callers rockchip_pm_domain_probe() and rockchip_pm_add_subdomain() print
that with dev_err():

  rockchip-pm-domain ...: power-domain: failed to get clk at index 1: -517
  rockchip-pm-domain ...: failed to handle node power-domain: -517

once per probe attempt, and the second line twice for a nested domain.
On an RK3588 board whose NPU domain clock appears late this is more than
20 error lines before the domain finally probes, and the reason of the
deferral is not recorded. Other failures are reported twice (once where
they happen and once by the callers), or only by the callers, which name
the node but not what failed: a missing pm_qos phandle and a pm_qos node
without a regmap print nothing in rockchip_pm_add_one_domain(), and a
failed clk_bulk_prepare() only has the message of the clk core, which
names neither the device nor the domain.

Report every error once, where it happens, with dev_err_probe(), and
drop the "failed to handle (subdomain) node" messages from the callers.
A deferral is then quiet and keeps its specific reason in
/sys/kernel/debug/devices_deferred, such as "failed to get clk at index
1", and -ENOMEM, which the callers used to print, is quiet as well. The
return values are unchanged, except that a failed
syscon_node_to_regmap() for a pm_qos node now returns its own error
instead of -ENODEV: it can be -EPROBE_DEFER (a syscon regmap that is
not registered yet, or a syscon clock or hwlock that is not available
yet), which must not turn into a probe failure.

Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 98 +++++++++++++++-------------------
 1 file changed, 43 insertions(+), 55 deletions(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 00c7e78f89b5..be077fb52f40 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -804,28 +804,22 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	int error;
 
 	error = of_property_read_u32(node, "reg", &id);
-	if (error) {
-		dev_err(pmu->dev,
-			"%pOFn: failed to retrieve domain id (reg): %d\n",
-			node, error);
-		return -EINVAL;
-	}
+	if (error)
+		return dev_err_probe(pmu->dev, -EINVAL,
+				     "%pOFn: failed to retrieve domain id (reg)\n",
+				     node);
 
-	if (id >= pmu->info->num_domains) {
-		dev_err(pmu->dev, "%pOFn: invalid domain id %d\n",
-			node, id);
-		return -EINVAL;
-	}
+	if (id >= pmu->info->num_domains)
+		return dev_err_probe(pmu->dev, -EINVAL,
+				     "%pOFn: invalid domain id %d\n", node, id);
 	/* RK3588 has domains with two parents (RKVDEC0/RKVDEC1) */
 	if (pmu->genpd_data.domains[id])
 		return 0;
 
 	pd_info = &pmu->info->domain_info[id];
-	if (!pd_info->pwr_mask && !pd_info->req_mask) {
-		dev_err(pmu->dev, "%pOFn: undefined domain id %d\n",
-			node, id);
-		return -EINVAL;
-	}
+	if (!pd_info->pwr_mask && !pd_info->req_mask)
+		return dev_err_probe(pmu->dev, -EINVAL,
+				     "%pOFn: undefined domain id %d\n", node, id);
 
 	pd = devm_kzalloc_obj(pmu->dev, *pd);
 	if (!pd)
@@ -849,17 +843,20 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	for (i = 0; i < pd->num_clks; i++) {
 		pd->clks[i].clk = of_clk_get(node, i);
 		if (IS_ERR(pd->clks[i].clk)) {
-			error = PTR_ERR(pd->clks[i].clk);
-			dev_err(pmu->dev,
-				"%pOFn: failed to get clk at index %d: %d\n",
-				node, i, error);
+			error = dev_err_probe(pmu->dev,
+					      PTR_ERR(pd->clks[i].clk),
+					      "%pOFn: failed to get clk at index %d\n",
+					      node, i);
 			goto err_put_clocks;
 		}
 	}
 
 	error = clk_bulk_prepare(pd->num_clks, pd->clks);
-	if (error)
+	if (error) {
+		dev_err_probe(pmu->dev, error,
+			      "%pOFn: failed to prepare clocks\n", node);
 		goto err_put_clocks;
+	}
 
 	pd->num_qos = of_count_phandle_with_args(node, "pm_qos",
 						 NULL);
@@ -884,13 +881,18 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 		for (j = 0; j < pd->num_qos; j++) {
 			qos_node = of_parse_phandle(node, "pm_qos", j);
 			if (!qos_node) {
-				error = -ENODEV;
+				error = dev_err_probe(pmu->dev, -ENODEV,
+						      "%pOFn: failed to get pm_qos %d\n",
+						      node, j);
 				goto err_unprepare_clocks;
 			}
 			pd->qos_regmap[j] = syscon_node_to_regmap(qos_node);
 			of_node_put(qos_node);
 			if (IS_ERR(pd->qos_regmap[j])) {
-				error = -ENODEV;
+				error = dev_err_probe(pmu->dev,
+						      PTR_ERR(pd->qos_regmap[j]),
+						      "%pOFn: failed to get pm_qos %d regmap\n",
+						      node, j);
 				goto err_unprepare_clocks;
 			}
 		}
@@ -1038,39 +1040,31 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
 		u32 idx;
 
 		error = of_property_read_u32(parent, "reg", &idx);
-		if (error) {
-			dev_err(pmu->dev,
-				"%pOFn: failed to retrieve domain id (reg): %d\n",
-				parent, error);
-			return error;
-		}
+		if (error)
+			return dev_err_probe(pmu->dev, error,
+					     "%pOFn: failed to retrieve domain id (reg)\n",
+					     parent);
 		parent_domain = pmu->genpd_data.domains[idx];
 
 		error = rockchip_pm_add_one_domain(pmu, np);
-		if (error) {
-			dev_err(pmu->dev, "failed to handle node %pOFn: %d\n",
-				np, error);
+		if (error)
 			return error;
-		}
 
 		error = of_property_read_u32(np, "reg", &idx);
-		if (error) {
-			dev_err(pmu->dev,
-				"%pOFn: failed to retrieve domain id (reg): %d\n",
-				np, error);
-			return error;
-		}
+		if (error)
+			return dev_err_probe(pmu->dev, error,
+					     "%pOFn: failed to retrieve domain id (reg)\n",
+					     np);
 		child_domain = pmu->genpd_data.domains[idx];
 
 		error = pm_genpd_add_subdomain(parent_domain, child_domain);
-		if (error) {
-			dev_err(pmu->dev, "%s failed to add subdomain %s: %d\n",
-				parent_domain->name, child_domain->name, error);
-			return error;
-		} else {
-			dev_dbg(pmu->dev, "%s add subdomain: %s\n",
-				parent_domain->name, child_domain->name);
-		}
+		if (error)
+			return dev_err_probe(pmu->dev, error,
+					     "%s failed to add subdomain %s\n",
+					     parent_domain->name, child_domain->name);
+
+		dev_dbg(pmu->dev, "%s add subdomain: %s\n",
+			parent_domain->name, child_domain->name);
 
 		error = rockchip_pm_add_subdomain(pmu, np);
 		if (error)
@@ -1141,18 +1135,12 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
 
 	for_each_available_child_of_node_scoped(np, node) {
 		error = rockchip_pm_add_one_domain(pmu, node);
-		if (error) {
-			dev_err(dev, "failed to handle node %pOFn: %d\n",
-				node, error);
+		if (error)
 			goto err_out;
-		}
 
 		error = rockchip_pm_add_subdomain(pmu, node);
-		if (error < 0) {
-			dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
-				node, error);
+		if (error < 0)
 			goto err_out;
-		}
 	}
 
 	if (error) {

-- 
2.55.0



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2 2/2] pmdomain: rockchip: report domain errors once, with dev_err_probe()

[Arun Iyengar]

When a power domain clock is not available yet,
rockchip_pm_add_one_domain() returns -EPROBE_DEFER, and both it and its
callers rockchip_pm_domain_probe() and rockchip_pm_add_subdomain() print
that with dev_err():

  rockchip-pm-domain ...: power-domain: failed to get clk at index 1: -517
  rockchip-pm-domain ...: failed to handle node power-domain: -517

once per probe attempt, and the second line twice for a nested domain.
On an RK3588 board whose NPU domain clock appears late this is more than
20 error lines before the domain finally probes, and the reason of the
deferral is not recorded. Other failures are reported twice (once where
they happen and once by the callers), or only by the callers, which name
the node but not what failed: a missing pm_qos phandle and a pm_qos node
without a regmap print nothing in rockchip_pm_add_one_domain(), and a
failed clk_bulk_prepare() only has the message of the clk core, which
names neither the device nor the domain.

Report every error once, where it happens, with dev_err_probe(), and
drop the "failed to handle (subdomain) node" messages from the callers.
A deferral is then quiet and keeps its specific reason in
/sys/kernel/debug/devices_deferred, such as "failed to get clk at index
1", and -ENOMEM, which the callers used to print, is quiet as well. The
return values are unchanged, except that a failed
syscon_node_to_regmap() for a pm_qos node now returns its own error
instead of -ENODEV: it can be -EPROBE_DEFER (a syscon regmap that is
not registered yet, or a syscon clock or hwlock that is not available
yet), which must not turn into a probe failure.

Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 98 +++++++++++++++-------------------
 1 file changed, 43 insertions(+), 55 deletions(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 00c7e78f89b5..be077fb52f40 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -804,28 +804,22 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	int error;
 
 	error = of_property_read_u32(node, "reg", &id);
-	if (error) {
-		dev_err(pmu->dev,
-			"%pOFn: failed to retrieve domain id (reg): %d\n",
-			node, error);
-		return -EINVAL;
-	}
+	if (error)
+		return dev_err_probe(pmu->dev, -EINVAL,
+				     "%pOFn: failed to retrieve domain id (reg)\n",
+				     node);
 
-	if (id >= pmu->info->num_domains) {
-		dev_err(pmu->dev, "%pOFn: invalid domain id %d\n",
-			node, id);
-		return -EINVAL;
-	}
+	if (id >= pmu->info->num_domains)
+		return dev_err_probe(pmu->dev, -EINVAL,
+				     "%pOFn: invalid domain id %d\n", node, id);
 	/* RK3588 has domains with two parents (RKVDEC0/RKVDEC1) */
 	if (pmu->genpd_data.domains[id])
 		return 0;
 
 	pd_info = &pmu->info->domain_info[id];
-	if (!pd_info->pwr_mask && !pd_info->req_mask) {
-		dev_err(pmu->dev, "%pOFn: undefined domain id %d\n",
-			node, id);
-		return -EINVAL;
-	}
+	if (!pd_info->pwr_mask && !pd_info->req_mask)
+		return dev_err_probe(pmu->dev, -EINVAL,
+				     "%pOFn: undefined domain id %d\n", node, id);
 
 	pd = devm_kzalloc_obj(pmu->dev, *pd);
 	if (!pd)
@@ -849,17 +843,20 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	for (i = 0; i < pd->num_clks; i++) {
 		pd->clks[i].clk = of_clk_get(node, i);
 		if (IS_ERR(pd->clks[i].clk)) {
-			error = PTR_ERR(pd->clks[i].clk);
-			dev_err(pmu->dev,
-				"%pOFn: failed to get clk at index %d: %d\n",
-				node, i, error);
+			error = dev_err_probe(pmu->dev,
+					      PTR_ERR(pd->clks[i].clk),
+					      "%pOFn: failed to get clk at index %d\n",
+					      node, i);
 			goto err_put_clocks;
 		}
 	}
 
 	error = clk_bulk_prepare(pd->num_clks, pd->clks);
-	if (error)
+	if (error) {
+		dev_err_probe(pmu->dev, error,
+			      "%pOFn: failed to prepare clocks\n", node);
 		goto err_put_clocks;
+	}
 
 	pd->num_qos = of_count_phandle_with_args(node, "pm_qos",
 						 NULL);
@@ -884,13 +881,18 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 		for (j = 0; j < pd->num_qos; j++) {
 			qos_node = of_parse_phandle(node, "pm_qos", j);
 			if (!qos_node) {
-				error = -ENODEV;
+				error = dev_err_probe(pmu->dev, -ENODEV,
+						      "%pOFn: failed to get pm_qos %d\n",
+						      node, j);
 				goto err_unprepare_clocks;
 			}
 			pd->qos_regmap[j] = syscon_node_to_regmap(qos_node);
 			of_node_put(qos_node);
 			if (IS_ERR(pd->qos_regmap[j])) {
-				error = -ENODEV;
+				error = dev_err_probe(pmu->dev,
+						      PTR_ERR(pd->qos_regmap[j]),
+						      "%pOFn: failed to get pm_qos %d regmap\n",
+						      node, j);
 				goto err_unprepare_clocks;
 			}
 		}
@@ -1038,39 +1040,31 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
 		u32 idx;
 
 		error = of_property_read_u32(parent, "reg", &idx);
-		if (error) {
-			dev_err(pmu->dev,
-				"%pOFn: failed to retrieve domain id (reg): %d\n",
-				parent, error);
-			return error;
-		}
+		if (error)
+			return dev_err_probe(pmu->dev, error,
+					     "%pOFn: failed to retrieve domain id (reg)\n",
+					     parent);
 		parent_domain = pmu->genpd_data.domains[idx];
 
 		error = rockchip_pm_add_one_domain(pmu, np);
-		if (error) {
-			dev_err(pmu->dev, "failed to handle node %pOFn: %d\n",
-				np, error);
+		if (error)
 			return error;
-		}
 
 		error = of_property_read_u32(np, "reg", &idx);
-		if (error) {
-			dev_err(pmu->dev,
-				"%pOFn: failed to retrieve domain id (reg): %d\n",
-				np, error);
-			return error;
-		}
+		if (error)
+			return dev_err_probe(pmu->dev, error,
+					     "%pOFn: failed to retrieve domain id (reg)\n",
+					     np);
 		child_domain = pmu->genpd_data.domains[idx];
 
 		error = pm_genpd_add_subdomain(parent_domain, child_domain);
-		if (error) {
-			dev_err(pmu->dev, "%s failed to add subdomain %s: %d\n",
-				parent_domain->name, child_domain->name, error);
-			return error;
-		} else {
-			dev_dbg(pmu->dev, "%s add subdomain: %s\n",
-				parent_domain->name, child_domain->name);
-		}
+		if (error)
+			return dev_err_probe(pmu->dev, error,
+					     "%s failed to add subdomain %s\n",
+					     parent_domain->name, child_domain->name);
+
+		dev_dbg(pmu->dev, "%s add subdomain: %s\n",
+			parent_domain->name, child_domain->name);
 
 		error = rockchip_pm_add_subdomain(pmu, np);
 		if (error)
@@ -1141,18 +1135,12 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
 
 	for_each_available_child_of_node_scoped(np, node) {
 		error = rockchip_pm_add_one_domain(pmu, node);
-		if (error) {
-			dev_err(dev, "failed to handle node %pOFn: %d\n",
-				node, error);
+		if (error)
 			goto err_out;
-		}
 
 		error = rockchip_pm_add_subdomain(pmu, node);
-		if (error < 0) {
-			dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
-				node, error);
+		if (error < 0)
 			goto err_out;
-		}
 	}
 
 	if (error) {

-- 
2.55.0

Re: [PATCH v2 2/2] pmdomain: rockchip: report domain errors once, with dev_err_probe()

[Heiko Stübner]

Am Donnerstag, 12. November 2026, 09:23:30 Mitteleuropäische Normalzeit schrieb Arun Iyengar via B4 Relay:
> From: Arun Iyengar <aiyengar1990@gmail.com>
> 
> When a power domain clock is not available yet,
> rockchip_pm_add_one_domain() returns -EPROBE_DEFER, and both it and its
> callers rockchip_pm_domain_probe() and rockchip_pm_add_subdomain() print
> that with dev_err():
> 
>   rockchip-pm-domain ...: power-domain: failed to get clk at index 1: -517
>   rockchip-pm-domain ...: failed to handle node power-domain: -517
> 
> once per probe attempt, and the second line twice for a nested domain.
> On an RK3588 board whose NPU domain clock appears late this is more than
> 20 error lines before the domain finally probes, and the reason of the
> deferral is not recorded. Other failures are reported twice (once where
> they happen and once by the callers), or only by the callers, which name
> the node but not what failed: a missing pm_qos phandle and a pm_qos node
> without a regmap print nothing in rockchip_pm_add_one_domain(), and a
> failed clk_bulk_prepare() only has the message of the clk core, which
> names neither the device nor the domain.
> 
> Report every error once, where it happens, with dev_err_probe(), and
> drop the "failed to handle (subdomain) node" messages from the callers.
> A deferral is then quiet and keeps its specific reason in
> /sys/kernel/debug/devices_deferred, such as "failed to get clk at index
> 1", and -ENOMEM, which the callers used to print, is quiet as well. The
> return values are unchanged, except that a failed
> syscon_node_to_regmap() for a pm_qos node now returns its own error
> instead of -ENODEV: it can be -EPROBE_DEFER (a syscon regmap that is
> not registered yet, or a syscon clock or hwlock that is not available
> yet), which must not turn into a probe failure.
> 
> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
> Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>

code change itself looks fine, but pretty please cut down the text length
in all your patches a lot.

You're simply converting to dev_err_probe here to not show defer messages.
This does definitly not need 3 quite long paragraphs of text.

This applys for all patches you sent recently :-) .

Same is true for the big'ish comment blocks you have in patch1 of this
series (and other series too). They should be short and to the point.

If needed people can get deeper information reading the actual code.


Thanks
Heiko


> ---
>  drivers/pmdomain/rockchip/pm-domains.c | 98 +++++++++++++++-------------------
>  1 file changed, 43 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 00c7e78f89b5..be077fb52f40 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -804,28 +804,22 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  	int error;
>  
>  	error = of_property_read_u32(node, "reg", &id);
> -	if (error) {
> -		dev_err(pmu->dev,
> -			"%pOFn: failed to retrieve domain id (reg): %d\n",
> -			node, error);
> -		return -EINVAL;
> -	}
> +	if (error)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: failed to retrieve domain id (reg)\n",
> +				     node);
>  
> -	if (id >= pmu->info->num_domains) {
> -		dev_err(pmu->dev, "%pOFn: invalid domain id %d\n",
> -			node, id);
> -		return -EINVAL;
> -	}
> +	if (id >= pmu->info->num_domains)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: invalid domain id %d\n", node, id);
>  	/* RK3588 has domains with two parents (RKVDEC0/RKVDEC1) */
>  	if (pmu->genpd_data.domains[id])
>  		return 0;
>  
>  	pd_info = &pmu->info->domain_info[id];
> -	if (!pd_info->pwr_mask && !pd_info->req_mask) {
> -		dev_err(pmu->dev, "%pOFn: undefined domain id %d\n",
> -			node, id);
> -		return -EINVAL;
> -	}
> +	if (!pd_info->pwr_mask && !pd_info->req_mask)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: undefined domain id %d\n", node, id);
>  
>  	pd = devm_kzalloc_obj(pmu->dev, *pd);
>  	if (!pd)
> @@ -849,17 +843,20 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  	for (i = 0; i < pd->num_clks; i++) {
>  		pd->clks[i].clk = of_clk_get(node, i);
>  		if (IS_ERR(pd->clks[i].clk)) {
> -			error = PTR_ERR(pd->clks[i].clk);
> -			dev_err(pmu->dev,
> -				"%pOFn: failed to get clk at index %d: %d\n",
> -				node, i, error);
> +			error = dev_err_probe(pmu->dev,
> +					      PTR_ERR(pd->clks[i].clk),
> +					      "%pOFn: failed to get clk at index %d\n",
> +					      node, i);
>  			goto err_put_clocks;
>  		}
>  	}
>  
>  	error = clk_bulk_prepare(pd->num_clks, pd->clks);
> -	if (error)
> +	if (error) {
> +		dev_err_probe(pmu->dev, error,
> +			      "%pOFn: failed to prepare clocks\n", node);
>  		goto err_put_clocks;
> +	}
>  
>  	pd->num_qos = of_count_phandle_with_args(node, "pm_qos",
>  						 NULL);
> @@ -884,13 +881,18 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  		for (j = 0; j < pd->num_qos; j++) {
>  			qos_node = of_parse_phandle(node, "pm_qos", j);
>  			if (!qos_node) {
> -				error = -ENODEV;
> +				error = dev_err_probe(pmu->dev, -ENODEV,
> +						      "%pOFn: failed to get pm_qos %d\n",
> +						      node, j);
>  				goto err_unprepare_clocks;
>  			}
>  			pd->qos_regmap[j] = syscon_node_to_regmap(qos_node);
>  			of_node_put(qos_node);
>  			if (IS_ERR(pd->qos_regmap[j])) {
> -				error = -ENODEV;
> +				error = dev_err_probe(pmu->dev,
> +						      PTR_ERR(pd->qos_regmap[j]),
> +						      "%pOFn: failed to get pm_qos %d regmap\n",
> +						      node, j);
>  				goto err_unprepare_clocks;
>  			}
>  		}
> @@ -1038,39 +1040,31 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
>  		u32 idx;
>  
>  		error = of_property_read_u32(parent, "reg", &idx);
> -		if (error) {
> -			dev_err(pmu->dev,
> -				"%pOFn: failed to retrieve domain id (reg): %d\n",
> -				parent, error);
> -			return error;
> -		}
> +		if (error)
> +			return dev_err_probe(pmu->dev, error,
> +					     "%pOFn: failed to retrieve domain id (reg)\n",
> +					     parent);
>  		parent_domain = pmu->genpd_data.domains[idx];
>  
>  		error = rockchip_pm_add_one_domain(pmu, np);
> -		if (error) {
> -			dev_err(pmu->dev, "failed to handle node %pOFn: %d\n",
> -				np, error);
> +		if (error)
>  			return error;
> -		}
>  
>  		error = of_property_read_u32(np, "reg", &idx);
> -		if (error) {
> -			dev_err(pmu->dev,
> -				"%pOFn: failed to retrieve domain id (reg): %d\n",
> -				np, error);
> -			return error;
> -		}
> +		if (error)
> +			return dev_err_probe(pmu->dev, error,
> +					     "%pOFn: failed to retrieve domain id (reg)\n",
> +					     np);
>  		child_domain = pmu->genpd_data.domains[idx];
>  
>  		error = pm_genpd_add_subdomain(parent_domain, child_domain);
> -		if (error) {
> -			dev_err(pmu->dev, "%s failed to add subdomain %s: %d\n",
> -				parent_domain->name, child_domain->name, error);
> -			return error;
> -		} else {
> -			dev_dbg(pmu->dev, "%s add subdomain: %s\n",
> -				parent_domain->name, child_domain->name);
> -		}
> +		if (error)
> +			return dev_err_probe(pmu->dev, error,
> +					     "%s failed to add subdomain %s\n",
> +					     parent_domain->name, child_domain->name);
> +
> +		dev_dbg(pmu->dev, "%s add subdomain: %s\n",
> +			parent_domain->name, child_domain->name);
>  
>  		error = rockchip_pm_add_subdomain(pmu, np);
>  		if (error)
> @@ -1141,18 +1135,12 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
>  
>  	for_each_available_child_of_node_scoped(np, node) {
>  		error = rockchip_pm_add_one_domain(pmu, node);
> -		if (error) {
> -			dev_err(dev, "failed to handle node %pOFn: %d\n",
> -				node, error);
> +		if (error)
>  			goto err_out;
> -		}
>  
>  		error = rockchip_pm_add_subdomain(pmu, node);
> -		if (error < 0) {
> -			dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
> -				node, error);
> +		if (error < 0)
>  			goto err_out;
> -		}
>  	}
>  
>  	if (error) {
> 
> 

Re: [PATCH v2 2/2] pmdomain: rockchip: report domain errors once, with dev_err_probe()

[Heiko Stübner]

Am Donnerstag, 12. November 2026, 09:23:30 Mitteleuropäische Normalzeit schrieb Arun Iyengar via B4 Relay:
> From: Arun Iyengar <aiyengar1990@gmail.com>
> 
> When a power domain clock is not available yet,
> rockchip_pm_add_one_domain() returns -EPROBE_DEFER, and both it and its
> callers rockchip_pm_domain_probe() and rockchip_pm_add_subdomain() print
> that with dev_err():
> 
>   rockchip-pm-domain ...: power-domain: failed to get clk at index 1: -517
>   rockchip-pm-domain ...: failed to handle node power-domain: -517
> 
> once per probe attempt, and the second line twice for a nested domain.
> On an RK3588 board whose NPU domain clock appears late this is more than
> 20 error lines before the domain finally probes, and the reason of the
> deferral is not recorded. Other failures are reported twice (once where
> they happen and once by the callers), or only by the callers, which name
> the node but not what failed: a missing pm_qos phandle and a pm_qos node
> without a regmap print nothing in rockchip_pm_add_one_domain(), and a
> failed clk_bulk_prepare() only has the message of the clk core, which
> names neither the device nor the domain.
> 
> Report every error once, where it happens, with dev_err_probe(), and
> drop the "failed to handle (subdomain) node" messages from the callers.
> A deferral is then quiet and keeps its specific reason in
> /sys/kernel/debug/devices_deferred, such as "failed to get clk at index
> 1", and -ENOMEM, which the callers used to print, is quiet as well. The
> return values are unchanged, except that a failed
> syscon_node_to_regmap() for a pm_qos node now returns its own error
> instead of -ENODEV: it can be -EPROBE_DEFER (a syscon regmap that is
> not registered yet, or a syscon clock or hwlock that is not available
> yet), which must not turn into a probe failure.
> 
> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
> Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>

code change itself looks fine, but pretty please cut down the text length
in all your patches a lot.

You're simply converting to dev_err_probe here to not show defer messages.
This does definitly not need 3 quite long paragraphs of text.

This applys for all patches you sent recently :-) .

Same is true for the big'ish comment blocks you have in patch1 of this
series (and other series too). They should be short and to the point.

If needed people can get deeper information reading the actual code.


Thanks
Heiko


> ---
>  drivers/pmdomain/rockchip/pm-domains.c | 98 +++++++++++++++-------------------
>  1 file changed, 43 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 00c7e78f89b5..be077fb52f40 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -804,28 +804,22 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  	int error;
>  
>  	error = of_property_read_u32(node, "reg", &id);
> -	if (error) {
> -		dev_err(pmu->dev,
> -			"%pOFn: failed to retrieve domain id (reg): %d\n",
> -			node, error);
> -		return -EINVAL;
> -	}
> +	if (error)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: failed to retrieve domain id (reg)\n",
> +				     node);
>  
> -	if (id >= pmu->info->num_domains) {
> -		dev_err(pmu->dev, "%pOFn: invalid domain id %d\n",
> -			node, id);
> -		return -EINVAL;
> -	}
> +	if (id >= pmu->info->num_domains)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: invalid domain id %d\n", node, id);
>  	/* RK3588 has domains with two parents (RKVDEC0/RKVDEC1) */
>  	if (pmu->genpd_data.domains[id])
>  		return 0;
>  
>  	pd_info = &pmu->info->domain_info[id];
> -	if (!pd_info->pwr_mask && !pd_info->req_mask) {
> -		dev_err(pmu->dev, "%pOFn: undefined domain id %d\n",
> -			node, id);
> -		return -EINVAL;
> -	}
> +	if (!pd_info->pwr_mask && !pd_info->req_mask)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: undefined domain id %d\n", node, id);
>  
>  	pd = devm_kzalloc_obj(pmu->dev, *pd);
>  	if (!pd)
> @@ -849,17 +843,20 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  	for (i = 0; i < pd->num_clks; i++) {
>  		pd->clks[i].clk = of_clk_get(node, i);
>  		if (IS_ERR(pd->clks[i].clk)) {
> -			error = PTR_ERR(pd->clks[i].clk);
> -			dev_err(pmu->dev,
> -				"%pOFn: failed to get clk at index %d: %d\n",
> -				node, i, error);
> +			error = dev_err_probe(pmu->dev,
> +					      PTR_ERR(pd->clks[i].clk),
> +					      "%pOFn: failed to get clk at index %d\n",
> +					      node, i);
>  			goto err_put_clocks;
>  		}
>  	}
>  
>  	error = clk_bulk_prepare(pd->num_clks, pd->clks);
> -	if (error)
> +	if (error) {
> +		dev_err_probe(pmu->dev, error,
> +			      "%pOFn: failed to prepare clocks\n", node);
>  		goto err_put_clocks;
> +	}
>  
>  	pd->num_qos = of_count_phandle_with_args(node, "pm_qos",
>  						 NULL);
> @@ -884,13 +881,18 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  		for (j = 0; j < pd->num_qos; j++) {
>  			qos_node = of_parse_phandle(node, "pm_qos", j);
>  			if (!qos_node) {
> -				error = -ENODEV;
> +				error = dev_err_probe(pmu->dev, -ENODEV,
> +						      "%pOFn: failed to get pm_qos %d\n",
> +						      node, j);
>  				goto err_unprepare_clocks;
>  			}
>  			pd->qos_regmap[j] = syscon_node_to_regmap(qos_node);
>  			of_node_put(qos_node);
>  			if (IS_ERR(pd->qos_regmap[j])) {
> -				error = -ENODEV;
> +				error = dev_err_probe(pmu->dev,
> +						      PTR_ERR(pd->qos_regmap[j]),
> +						      "%pOFn: failed to get pm_qos %d regmap\n",
> +						      node, j);
>  				goto err_unprepare_clocks;
>  			}
>  		}
> @@ -1038,39 +1040,31 @@ static int rockchip_pm_add_subdomain(struct rockchip_pmu *pmu,
>  		u32 idx;
>  
>  		error = of_property_read_u32(parent, "reg", &idx);
> -		if (error) {
> -			dev_err(pmu->dev,
> -				"%pOFn: failed to retrieve domain id (reg): %d\n",
> -				parent, error);
> -			return error;
> -		}
> +		if (error)
> +			return dev_err_probe(pmu->dev, error,
> +					     "%pOFn: failed to retrieve domain id (reg)\n",
> +					     parent);
>  		parent_domain = pmu->genpd_data.domains[idx];
>  
>  		error = rockchip_pm_add_one_domain(pmu, np);
> -		if (error) {
> -			dev_err(pmu->dev, "failed to handle node %pOFn: %d\n",
> -				np, error);
> +		if (error)
>  			return error;
> -		}
>  
>  		error = of_property_read_u32(np, "reg", &idx);
> -		if (error) {
> -			dev_err(pmu->dev,
> -				"%pOFn: failed to retrieve domain id (reg): %d\n",
> -				np, error);
> -			return error;
> -		}
> +		if (error)
> +			return dev_err_probe(pmu->dev, error,
> +					     "%pOFn: failed to retrieve domain id (reg)\n",
> +					     np);
>  		child_domain = pmu->genpd_data.domains[idx];
>  
>  		error = pm_genpd_add_subdomain(parent_domain, child_domain);
> -		if (error) {
> -			dev_err(pmu->dev, "%s failed to add subdomain %s: %d\n",
> -				parent_domain->name, child_domain->name, error);
> -			return error;
> -		} else {
> -			dev_dbg(pmu->dev, "%s add subdomain: %s\n",
> -				parent_domain->name, child_domain->name);
> -		}
> +		if (error)
> +			return dev_err_probe(pmu->dev, error,
> +					     "%s failed to add subdomain %s\n",
> +					     parent_domain->name, child_domain->name);
> +
> +		dev_dbg(pmu->dev, "%s add subdomain: %s\n",
> +			parent_domain->name, child_domain->name);
>  
>  		error = rockchip_pm_add_subdomain(pmu, np);
>  		if (error)
> @@ -1141,18 +1135,12 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
>  
>  	for_each_available_child_of_node_scoped(np, node) {
>  		error = rockchip_pm_add_one_domain(pmu, node);
> -		if (error) {
> -			dev_err(dev, "failed to handle node %pOFn: %d\n",
> -				node, error);
> +		if (error)
>  			goto err_out;
> -		}
>  
>  		error = rockchip_pm_add_subdomain(pmu, node);
> -		if (error < 0) {
> -			dev_err(dev, "failed to handle subdomain node %pOFn: %d\n",
> -				node, error);
> +		if (error < 0)
>  			goto err_out;
> -		}
>  	}
>  
>  	if (error) {
> 
> 





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v2 2/2] pmdomain: rockchip: report domain errors once, with dev_err_probe()

[Yao Zi]

On Thu, Nov 12, 2026 at 08:23:30AM +0000, Arun Iyengar via B4 Relay wrote:
> From: Arun Iyengar <aiyengar1990@gmail.com>
> 
> When a power domain clock is not available yet,
> rockchip_pm_add_one_domain() returns -EPROBE_DEFER, and both it and its
> callers rockchip_pm_domain_probe() and rockchip_pm_add_subdomain() print
> that with dev_err():
> 
>   rockchip-pm-domain ...: power-domain: failed to get clk at index 1: -517
>   rockchip-pm-domain ...: failed to handle node power-domain: -517
> 
> once per probe attempt, and the second line twice for a nested domain.
> On an RK3588 board whose NPU domain clock appears late this is more than
> 20 error lines before the domain finally probes, and the reason of the
> deferral is not recorded. Other failures are reported twice (once where
> they happen and once by the callers), or only by the callers, which name
> the node but not what failed: a missing pm_qos phandle and a pm_qos node
> without a regmap print nothing in rockchip_pm_add_one_domain(), and a
> failed clk_bulk_prepare() only has the message of the clk core, which
> names neither the device nor the domain.
> 
> Report every error once, where it happens, with dev_err_probe(), and
> drop the "failed to handle (subdomain) node" messages from the callers.
> A deferral is then quiet and keeps its specific reason in
> /sys/kernel/debug/devices_deferred, such as "failed to get clk at index
> 1", and -ENOMEM, which the callers used to print, is quiet as well. The
> return values are unchanged, except that a failed
> syscon_node_to_regmap() for a pm_qos node now returns its own error
> instead of -ENODEV: it can be -EPROBE_DEFER (a syscon regmap that is
> not registered yet, or a syscon clock or hwlock that is not available
> yet), which must not turn into a probe failure.
> 
> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
> Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
> ---
>  drivers/pmdomain/rockchip/pm-domains.c | 98 +++++++++++++++-------------------
>  1 file changed, 43 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 00c7e78f89b5..be077fb52f40 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -804,28 +804,22 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  	int error;
>  
>  	error = of_property_read_u32(node, "reg", &id);
> -	if (error) {
> -		dev_err(pmu->dev,
> -			"%pOFn: failed to retrieve domain id (reg): %d\n",
> -			node, error);
> -		return -EINVAL;
> -	}
> +	if (error)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: failed to retrieve domain id (reg)\n",
> +				     node);
>  
> -	if (id >= pmu->info->num_domains) {
> -		dev_err(pmu->dev, "%pOFn: invalid domain id %d\n",
> -			node, id);
> -		return -EINVAL;
> -	}
> +	if (id >= pmu->info->num_domains)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: invalid domain id %d\n", node, id);

These look like code clean-up instead of fixes, thus should be split
into a separate patch, especially when we probably want to backport the
real fixes. This applies for many places below.

Aside these unnecessary code changes, this patch seems in a better shape
than the very first version[1] I submitted a week ago, which seems,
unfortunately, being ignored.

I'll drop the second patch in my series and use yours instead. But
please search before sending patches next time to avoid duplicated work.

-- 
Thanks,
Yao Zi

Meow-mrrp-mrow, I'm a cat.

[1]: https://lore.kernel.org/linux-rockchip/20261105001257.1368266-1-me@ziyao.cc/

Re: [PATCH v2 2/2] pmdomain: rockchip: report domain errors once, with dev_err_probe()

[Yao Zi]

On Thu, Nov 12, 2026 at 08:23:30AM +0000, Arun Iyengar via B4 Relay wrote:
> From: Arun Iyengar <aiyengar1990@gmail.com>
> 
> When a power domain clock is not available yet,
> rockchip_pm_add_one_domain() returns -EPROBE_DEFER, and both it and its
> callers rockchip_pm_domain_probe() and rockchip_pm_add_subdomain() print
> that with dev_err():
> 
>   rockchip-pm-domain ...: power-domain: failed to get clk at index 1: -517
>   rockchip-pm-domain ...: failed to handle node power-domain: -517
> 
> once per probe attempt, and the second line twice for a nested domain.
> On an RK3588 board whose NPU domain clock appears late this is more than
> 20 error lines before the domain finally probes, and the reason of the
> deferral is not recorded. Other failures are reported twice (once where
> they happen and once by the callers), or only by the callers, which name
> the node but not what failed: a missing pm_qos phandle and a pm_qos node
> without a regmap print nothing in rockchip_pm_add_one_domain(), and a
> failed clk_bulk_prepare() only has the message of the clk core, which
> names neither the device nor the domain.
> 
> Report every error once, where it happens, with dev_err_probe(), and
> drop the "failed to handle (subdomain) node" messages from the callers.
> A deferral is then quiet and keeps its specific reason in
> /sys/kernel/debug/devices_deferred, such as "failed to get clk at index
> 1", and -ENOMEM, which the callers used to print, is quiet as well. The
> return values are unchanged, except that a failed
> syscon_node_to_regmap() for a pm_qos node now returns its own error
> instead of -ENODEV: it can be -EPROBE_DEFER (a syscon regmap that is
> not registered yet, or a syscon clock or hwlock that is not available
> yet), which must not turn into a probe failure.
> 
> Fixes: 7c696693a4f5 ("soc: rockchip: power-domain: Add power domain driver")
> Fixes: 6be05b5ec161 ("soc: rockchip: power-domain: add support for sub-power domains")
> Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Signed-off-by: Arun Iyengar <aiyengar1990@gmail.com>
> ---
>  drivers/pmdomain/rockchip/pm-domains.c | 98 +++++++++++++++-------------------
>  1 file changed, 43 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 00c7e78f89b5..be077fb52f40 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c
> @@ -804,28 +804,22 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  	int error;
>  
>  	error = of_property_read_u32(node, "reg", &id);
> -	if (error) {
> -		dev_err(pmu->dev,
> -			"%pOFn: failed to retrieve domain id (reg): %d\n",
> -			node, error);
> -		return -EINVAL;
> -	}
> +	if (error)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: failed to retrieve domain id (reg)\n",
> +				     node);
>  
> -	if (id >= pmu->info->num_domains) {
> -		dev_err(pmu->dev, "%pOFn: invalid domain id %d\n",
> -			node, id);
> -		return -EINVAL;
> -	}
> +	if (id >= pmu->info->num_domains)
> +		return dev_err_probe(pmu->dev, -EINVAL,
> +				     "%pOFn: invalid domain id %d\n", node, id);

These look like code clean-up instead of fixes, thus should be split
into a separate patch, especially when we probably want to backport the
real fixes. This applies for many places below.

Aside these unnecessary code changes, this patch seems in a better shape
than the very first version[1] I submitted a week ago, which seems,
unfortunately, being ignored.

I'll drop the second patch in my series and use yours instead. But
please search before sending patches next time to avoid duplicated work.

-- 
Thanks,
Yao Zi

Meow-mrrp-mrow, I'm a cat.

[1]: https://lore.kernel.org/linux-rockchip/20261105001257.1368266-1-me@ziyao.cc/

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip