All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Krzysztof Kozlowski" <krzysztof.kozlowski@oss.qualcomm.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org
Subject: Re: [PATCH 2/2] dt-bindings: rockchip: grf: Narrow allowed reboot modes
Date: Wed, 01 Jul 2026 11:10:41 +0000	[thread overview]
Message-ID: <20260701111041.A75C71F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260701105849.197086-4-krzysztof.kozlowski@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a new `patternProperties` constraint to tighten the `reboot-mode` schema, but places it inside a conditional `if` block that does not enumerate all relevant Rockchip platforms. Platforms such as `rockchip,rk3328-grf`, `rockchip,rk3562-pmu-grf`, `rockchip,rk3066-grf`, and `rockchip,rk3188-grf` use `reboot-mode` in their device trees but are missing from this `if` condition. As a result, they bypass the block entirely and evade the newly introduced constraint, making the schema tightening incomplete.
--

commit 16c4d82cda45faacd19f46f2d3336cc68b841769
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

dt-bindings: rockchip: grf: Narrow allowed reboot modes

syscon-reboot-mode schema allows arbitrary "mode-.* properties but only
a subset actually makes sense and is valid.  Provide negative look-ahead
pattern to disallow any modes not supported by the device, which
tightens the binding.

> diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
> index 2cc43742b8e3b..7bcb4e2f47ec5 100644
> --- a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
> +++ b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml
> @@ -239,11 +239,14 @@ allOf:
>        properties:
>          reboot-mode:
>            type: object
> -
>            $ref: /schemas/power/reset/syscon-reboot-mode.yaml#
> -
>            unevaluatedProperties: false
>  
> +          patternProperties:
> +            # Negative look-ahead to disallow unsupported modes. The '$' has to be
> +            # part of lookahead group to work, instead of trailing outside of ().
> +            "^mode-(?!(bootloader$|fastboot$|loader$|normal$|recovery$))": false
> +

[Severity: Medium]
Looking at the surrounding code, this constraint is added inside a conditional
block that only enumerates a subset of platforms:

  - if:
      properties:
        compatible:
          contains:
            enum:
              - rockchip,px30-pmugrf
              - rockchip,rk3036-grf
              - rockchip,rk3308-grf
              - rockchip,rk3368-pmugrf
              - rockchip,rv1103b-pmu-grf

Does this leave the schema tightening incomplete? Other platforms such as
rockchip,rk3328-grf, rockchip,rk3562-pmu-grf, rockchip,rk3066-grf, and
rockchip,rk3188-grf also use reboot-mode in their device trees.

Since they are missing from this conditional, will their reboot-mode
properties silently evade the new patternProperties validation?

>    - if:
>        properties:
>          compatible:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701105849.197086-3-krzysztof.kozlowski@oss.qualcomm.com?part=2

  reply	other threads:[~2026-07-01 11:10 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01 10:58 [PATCH 1/2] dt-bindings: rockchip: pmu: Narrow definition of reboot-mode Krzysztof Kozlowski
2026-07-01 10:58 ` Krzysztof Kozlowski
2026-07-01 10:58 ` [PATCH 2/2] dt-bindings: rockchip: grf: Narrow allowed reboot modes Krzysztof Kozlowski
2026-07-01 10:58   ` Krzysztof Kozlowski
2026-07-01 11:10   ` sashiko-bot [this message]
2026-07-01 11:19     ` Krzysztof Kozlowski
2026-07-01 19:56   ` Rob Herring (Arm)
2026-07-01 19:56     ` Rob Herring (Arm)
2026-07-01 19:55 ` [PATCH 1/2] dt-bindings: rockchip: pmu: Narrow definition of reboot-mode Rob Herring (Arm)
2026-07-01 19:55   ` Rob Herring (Arm)
2026-07-07 19:44 ` Heiko Stuebner
2026-07-07 19:44   ` Heiko Stuebner

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=20260701111041.A75C71F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski@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.