Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: Prabhakar <prabhakar.csengg@gmail.com>
Cc: robh@kernel.org, krzk+dt@kernel.org, devicetree@vger.kernel.org,
	conor+dt@kernel.org, wsa+renesas@sang-engineering.com
Subject: Re: [PATCH v4 1/2] dt-bindings: memory: renesas,rzg3e-xspi: Add RZ/T2H and RZ/N2H support
Date: Fri, 15 May 2026 12:10:48 +0000	[thread overview]
Message-ID: <20260515121049.18568C2BCB0@smtp.kernel.org> (raw)
In-Reply-To: <20260515115202.1515577-2-prabhakar.mahadev-lad.rj@bp.renesas.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The new SoCs claim fallback compatibility with `renesas,r9a09g047-xspi` but provide strictly fewer hardware resources than the fallback schema previously mandated.
- [Medium] The `else` block enforces a known-incorrect `minItems: 4` constraint on the base `renesas,r9a09g047-xspi` compatible, contradicting the hardware reality described in the commit message.
--

commit 53ce1d0b1afc85c1adf4f55123b53c081bc04767
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

dt-bindings: memory: renesas,rzg3e-xspi: Add RZ/T2H and RZ/N2H support

Document the xSPI controller found on the Renesas RZ/T2H and RZ/N2H SoCs.
The patch updates clock and reset constraints to handle SoC differences,
noting that the RZ/G3E hardware actually only supports a single bridge
channel despite manual references.

> diff --git a/Documentation/devicetree/bindings/memory-controllers/renesas,rzg3e-xspi.yaml b/Documentation/devicetree/bindings/memory-controllers/renesas,rzg3e-xspi.yaml
> index 7a84f5bb7284e..cdeca4c795f3a 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/renesas,rzg3e-xspi.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/renesas,rzg3e-xspi.yaml
> @@ -30,6 +30,8 @@ properties:
>            - enum:
>                - renesas,r9a09g056-xspi  # RZ/V2N
>                - renesas,r9a09g057-xspi  # RZ/V2H(P)
> +              - renesas,r9a09g077-xspi  # RZ/T2H
> +              - renesas,r9a09g087-xspi  # RZ/N2H
>            - const: renesas,r9a09g047-xspi

Could this cause a regression since the new SoCs claim fallback
compatibility with renesas,r9a09g047-xspi, but provide strictly fewer
hardware resources than the fallback schema previously mandated?

A fallback compatible string asserts that an unmodified legacy OS driver for
the fallback SoC can successfully drive the new hardware. Prior to this patch,
the schema for renesas,r9a09g047-xspi mandated 4 clocks and 2 resets. The new
if/then block restricts the new SoCs to only 2 clocks and 1 reset.

While the current Linux driver handles this gracefully by using
devm_clk_get_optional_enabled(), Device Tree bindings are strictly
OS-agnostic. An older OS driver written strictly to the fallback's original
schema might attempt to acquire the previously mandatory axi/spix2 clocks and
aresetn reset, resulting in a probe failure.

Could the fallback's constraints be relaxed instead, or should SoCs providing
fewer resources avoid claiming fallback compatibility with a more
resource-demanding SoC?

[ ... ]

> @@ -109,6 +121,34 @@ required:
>    - '#address-cells'
>    - '#size-cells'
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - renesas,r9a09g077-xspi
> +          - renesas,r9a09g087-xspi
> +then:
> +  properties:
> +    clocks:
> +      maxItems: 2
> +    clock-names:
> +      maxItems: 2
> +    resets:
> +      maxItems: 1
> +    reset-names:
> +      maxItems: 1
> +else:
> +  properties:
> +    clocks:
> +      minItems: 4
> +    clock-names:
> +      minItems: 4
> +    resets:
> +      minItems: 2
> +    reset-names:
> +      minItems: 2

Does this else block introduce a regression by enforcing a known-incorrect
minItems constraint on the base renesas,r9a09g047-xspi compatible?

The commit message explicitly states that the RZ/G3E hardware actually only
supports a single bridge channel, matching the RZ/T2H design. However, the
patch adds an else block that strictly enforces minItems: 4 for clocks and
minItems: 2 for resets on all variants not listed in the if block. This
cements a known-incorrect hardware description into the schema.

When the RZ/G3E devicetrees are eventually corrected to reflect the 2-clock
hardware reality as promised, will they immediately fail schema validation?
Would relaxing the else block correctly allow RZ/G3E to use either the legacy
4-item or the correct 2-item configurations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515115202.1515577-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=1

  reply	other threads:[~2026-05-15 12:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15 11:52 [PATCH v4 0/2] Add xSPI support for RZ/T2H and RZ/N2H SoCs Prabhakar
2026-05-15 11:52 ` [PATCH v4 1/2] dt-bindings: memory: renesas,rzg3e-xspi: Add RZ/T2H and RZ/N2H support Prabhakar
2026-05-15 12:10   ` sashiko-bot [this message]
2026-05-15 11:52 ` [PATCH v4 2/2] memory: renesas-rpc-if: Fix duplicate device name on multi-instance platforms Prabhakar

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=20260515121049.18568C2BCB0@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=prabhakar.csengg@gmail.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=wsa+renesas@sang-engineering.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox