Linux clock framework development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Joseph Telaak" <joseph@telaak.dev>
Cc: linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH 3/4] dt-bindings: clock: Add Siflower SF19A2890 peripheral clock/reset managers
Date: Wed, 04 Nov 2026 07:27:11 +0000	[thread overview]
Message-ID: <sashiko-outbox-196792@kernel.org> (raw)
In-Reply-To: <20261104-sf19a2890-sysm-v1-3-a4fac07df823@telaak.dev>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: clock: unwieldy per-compatible if-then conditionals in schema
--

commit a648a6a258536238fc540454eb5ccb65637262cb
Author: Joseph Telaak <joseph@telaak.dev>

dt-bindings: clock: Add Siflower SF19A2890 peripheral clock/reset managers

This commit adds a Devicetree binding schema for the Siflower SF19A2890
peripheral clock and reset managers, defining twelve compatible strings
and their corresponding clock inputs and reset lines.

> diff --git a/Documentation/devicetree/bindings/clock/siflower,sf19a2890-periph-crm.yaml b/Documentation/devicetree/bindings/clock/siflower,sf19a2890-periph-crm.yaml
> new file mode 100644
> index 0000000000000..7f0bce603b423
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/siflower,sf19a2890-periph-crm.yaml
> @@ -0,0 +1,169 @@

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - siflower,sf19a2890-gmac-crm
> +      - siflower,sf19a2890-gpio-crm
> +      - siflower,sf19a2890-i2c-crm
> +      - siflower,sf19a2890-pwm-crm
> +      - siflower,sf19a2890-rf-crm
> +      - siflower,sf19a2890-spi-crm
> +      - siflower,sf19a2890-timer-crm
> +      - siflower,sf19a2890-uart-crm
> +      - siflower,sf19a2890-usb-crm
> +      - siflower,sf19a2890-wdt-crm
> +      - siflower,sf19a2890-wlan24-crm
> +      - siflower,sf19a2890-wlan5-crm

[ ... ]

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: siflower,sf19a2890-gmac-crm
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: GMAC bus clock
> +            - description: GMAC register clock
> +            - description: RGMII transmit reference clock
> +            - description: Timestamp unit clock
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - siflower,sf19a2890-wlan24-crm
> +              - siflower,sf19a2890-wlan5-crm
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: AXI slave clock
> +            - description: AXI master clock
> +            - description: Radio platform clock
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: siflower,sf19a2890-rf-crm
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Test clock
> +            - description: AXI slave clock
> +            - description: Boot clock
> +            - description: Low-power clock
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: siflower,sf19a2890-usb-crm
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: USB controller clock
> +            - description: AXI master clock
> +            - description: USB PHY reference clock
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: siflower,sf19a2890-spi-crm
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: APB clock
> +            - description: SSP clock
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: siflower,sf19a2890-uart-crm
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: UART0 APB clock
> +            - description: UART1 APB clock
> +            - description: UART2 APB clock
> +            - description: UART0 clock
> +            - description: UART1 clock
> +            - description: UART2 clock
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - siflower,sf19a2890-gpio-crm
> +              - siflower,sf19a2890-i2c-crm
> +              - siflower,sf19a2890-pwm-crm
> +              - siflower,sf19a2890-timer-crm
> +              - siflower,sf19a2890-wdt-crm
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 1

[Severity: Low]
This isn't a bug, but properties.compatible in
Documentation/devicetree/bindings/clock/siflower,sf19a2890-periph-crm.yaml
combines twelve peripheral CRM variants that have different clock counts
(from 1 to 6 clocks), clock roles, and reset assignments, requiring seven
separate per-compatible if-then conditionals under allOf to constrain the
clocks property.

Would it be cleaner to split this binding into separate schema documents
rather than piling these per-compatible if-then conditionals into a single
schema?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-sf19a2890-sysm-v1-0-a4fac07df823@telaak.dev?part=3

  reply	other threads:[~2026-11-04  7:27 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04  7:20 [PATCH 0/4] clk: Add the Siflower SF19A2890 clock manager and peripheral clock and reset managers Joseph Telaak via B4 Relay
2026-11-04  7:20 ` [PATCH 1/4] dt-bindings: clock: Add Siflower SF19A2890 clock manager Joseph Telaak via B4 Relay
2026-11-04  7:20 ` [PATCH 2/4] clk: siflower: Add SF19A2890 clock manager driver Joseph Telaak via B4 Relay
2026-11-04  7:34   ` sashiko-bot
2026-11-04  7:21 ` [PATCH 3/4] dt-bindings: clock: Add Siflower SF19A2890 peripheral clock/reset managers Joseph Telaak via B4 Relay
2026-11-04  7:27   ` sashiko-bot [this message]
2026-11-04  7:21 ` [PATCH 4/4] clk: siflower: Add SF19A2890 peripheral clock and reset manager driver Joseph Telaak via B4 Relay
2026-11-04  9:25   ` Philipp Zabel
2026-11-04 13:37     ` Chuanhong Guo
2026-11-04 20:10       ` Joseph Telaak
2026-11-04 13:32   ` Chuanhong Guo
2026-11-04 20:12     ` Joseph Telaak

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=sashiko-outbox-196792@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=joseph@telaak.dev \
    --cc=linux-clk@vger.kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox