All of lore.kernel.org
 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: 6+ 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-18  8:07   ` Krzysztof Kozlowski
2026-05-15 11:52 ` [PATCH v4 2/2] memory: renesas-rpc-if: Fix duplicate device name on multi-instance platforms Prabhakar
2026-05-24 19:29 ` [PATCH v4 0/2] Add xSPI support for RZ/T2H and RZ/N2H SoCs Krzysztof Kozlowski

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 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.