All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Manaf Meethalavalappu Pallikunhi" <manaf.pallikunhi@oss.qualcomm.com>
Cc: conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org
Subject: Re: [RFC PATCH] dt-bindings: power: Add power-limit-controller schema
Date: Thu, 09 Jul 2026 18:14:29 +0000	[thread overview]
Message-ID: <20260709181430.509131F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260709180727.4015267-1-manaf.pallikunhi@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Node name regexes for domains and limits incorrectly restrict unit addresses to decimal digits, breaking on standard hex addresses.
- [Medium] Missing `#address-cells` and `#size-cells` properties for nodes with `reg`-bearing children, causing dtc warnings and schema validation failures.
- [Medium] Use of non-standard `-microsecond` suffix prevents automatic schema typing.
--

--- Patch [1]: [RFC PATCH] dt-bindings: power: Add power-limit-controller schema ---
commit 04e63fa5b3b701b0306a74eba5521673337890b4
Author: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>

dt-bindings: power: Add power-limit-controller schema
    
This RFC proposes a new device tree binding schema for power limit
controllers that manage SoC power domains with hardware-enforced power
capping capabilities.

> diff --git a/Documentation/devicetree/bindings/power/limits/power-limit-controller.yaml b/Documentation/devicetree/bindings/power/limits/power-limit-controller.yaml
> new file mode 100644
> index 0000000000000..9cd4d9d6414d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/limits/power-limit-controller.yaml
> @@ -0,0 +1,238 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/limits/power-limit-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

[ ... ]

> +properties:
> +  $nodename:
> +    pattern: "^power-limits(@.*)?$"
> +
> +  '#power-limit-domain-cells':
> +    description: |
> +      Number of cells in a power limit domain specifier for child domains.
> +      Typically 1, representing the domain index.
> +    const: 1

[Severity: Medium]
Will this schema cause dtc warnings for missing #address-cells and #size-cells?

Since the child nodes like power-limit-domain@0 require a reg property,
wouldn't the root node need to define #address-cells and #size-cells
properties here in the schema?

> +
> +patternProperties:
> +  "^power-limit-domain@[0-9]+$":

[Severity: Medium]
Does this regular expression restrict unit addresses to only decimal digits?

Standard device trees use hexadecimal for unit addresses, so a node like
power-limit-domain@a would fail to match this pattern. Could this be
updated to [0-9a-f]+ instead?

> +    type: object
> +    description: |
> +      Individual power limit domain/zone under this controller.
> +      Each domain can either:
> +        - Have power limit constraints (with power-limits child node)
> +        - Be monitoring-only (without power-limits child node)

[ ... ]

> +      power-limits:
> +        type: object
> +        description: |
> +          Container node for power limit constraints within this domain.
> +          Each child node represents a power limit constraint index.
> +
> +          This node is optional. If omitted (or if monitoring-only is set),
> +          the domain provides only power/energy measurement without limits.
> +
> +        patternProperties:
> +          "^power-limit@[0-9]+$":

[Severity: Medium]
Similarly, does this constraint node pattern also improperly restrict unit
addresses to decimal digits instead of hexadecimal?

> +            type: object
> +            description: |
> +              Individual power limit constraint configuration.

[ ... ]

> +              power-limit-microwatt:
> +                description: |
> +                  Default power limit value for this constraint at boot/reset.
> +                  This is the initial value that will be programmed.
> +
> +              time-window-min-microsecond:

[Severity: Medium]
Does the -microsecond suffix prevent dtschema from automatically inferring
property types?

The standard suffix recognized by the core schema for microseconds is -us.
Without the standard suffix, the schema validator cannot automatically infer
the property types (such as uint32), which could cause missing type definition
errors during dt_binding_check.

> +                description: |
> +                  Minimum time window for power averaging.
> +                  Shorter windows allow faster response to power excursions.

[ ... ]

> +            required:
> +              - reg
> +
> +            additionalProperties: true
> +
> +        additionalProperties: false

[Severity: Medium]
Will this strict additionalProperties: false prevent adding #address-cells
and #size-cells to the power-limits container?

Because the power-limit@N child nodes require reg properties, this container
node will need these cell-count properties, but they aren't explicitly defined
in this schema block.

> +
> +    required:
> +      - reg
> +
> +    additionalProperties: true
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    // Multi-domain power limit controller with mixed capabilities
> +    // Demonstrates multiple domains with and without power limit constraints
> +    power-limits@ef3b000 {
> +        compatible = "qcom,glymur-spel";
> +        reg = <0x0ef3b000 0x1000>;
> +        #power-limit-domain-cells = <1>;

[Severity: Medium]
Could this example fail dt_binding_check due to the missing #address-cells
and #size-cells properties in the power-limits@ef3b000 node?

> +
> +        // Domain 0: System domain with full power limit control (PL1/PL2)
> +        sys_domain: power-limit-domain@0 {
> +            reg = <0>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709180727.4015267-1-manaf.pallikunhi@oss.qualcomm.com?part=1

  reply	other threads:[~2026-07-09 18:14 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-02 17:22 [PATCH v3 0/4] Add Qualcomm SPEL powercap driver Manaf Meethalavalappu Pallikunhi
2026-07-02 17:22 ` [PATCH v3 1/4] dt-bindings: power: Add common power limit controller schema Manaf Meethalavalappu Pallikunhi
2026-07-02 17:44   ` sashiko-bot
2026-07-03  6:20   ` Krzysztof Kozlowski
2026-07-09 18:07     ` [RFC PATCH] dt-bindings: power: Add power-limit-controller schema Manaf Meethalavalappu Pallikunhi
2026-07-09 18:14       ` sashiko-bot [this message]
2026-07-02 17:22 ` [PATCH v3 2/4] dt-bindings: power: limits: Describe Qualcomm SPEL hardware Manaf Meethalavalappu Pallikunhi
2026-07-02 17:30   ` sashiko-bot
2026-07-03  6:21   ` Krzysztof Kozlowski
2026-07-02 17:22 ` [PATCH v3 3/4] powercap: qcom: Add SPEL powercap driver Manaf Meethalavalappu Pallikunhi
2026-07-02 17:36   ` sashiko-bot
2026-07-03  6:24   ` Krzysztof Kozlowski
2026-07-06 11:57   ` Konrad Dybcio
2026-07-09 12:58     ` Konrad Dybcio
2026-07-06 13:52   ` Daniel Lezcano
2026-07-07 14:17   ` Uwe Kleine-König
2026-07-02 17:22 ` [PATCH v3 4/4] arm64: dts: qcom: glymur: Enable " Manaf Meethalavalappu Pallikunhi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260709181430.509131F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=manaf.pallikunhi@oss.qualcomm.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.