Re: [PATCH 1/5] dt-bindings: clock: Add STM32MP25 SYSCFG clock controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dt-bindings: clock: stm32mp25-syscfg-clk duplicates parent
  syscfg clock provider
--

commit 56dc8d49308b7af249ab11bbe59e951afa0c0726
Author: Gabriel Fernandez <gabriel.fernandez@foss.st.com>

dt-bindings: clock: Add STM32MP25 SYSCFG clock controller

This commit adds the devicetree binding and clock identifier header for
the STM32MP23x and STM32MP25x SYSCFG display pixel clock controller.

> diff --git a/Documentation/devicetree/bindings/clock/st,stm32mp25-syscfg-clk.yaml b/Documentation/devicetree/bindings/clock/st,stm32mp25-syscfg-clk.yaml
> new file mode 100644
> index 0000000000000..027d14f28b0cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/st,stm32mp25-syscfg-clk.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/st,stm32mp25-syscfg-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: STMicroelectronics STM32MP2 SYSCFG clock controller
> +
> +maintainers:
> +  - Gabriel Fernandez <gabriel.fernandez@foss.st.com>
> +
> +description: |
> +  The SYSCFG of the STM32MP23x and STM32MP25x SoCs provides clocks, described
> +  by a clock controller sub-node of the SYSCFG node.
> +
> +  The provided clocks are identified by the macros defined in
> +  include/dt-bindings/clock/st,stm32mp25-syscfg-clk.h:
> +    - CK_SYSCFG_PIX_LTDC: LTDC pixel clock, selected in the DISPLAYCLKCR
> +      register among the LTDC kernel clock, used by the RGB parallel output,
> +      the LVDS PHY pixel clock and the DSI PHY byte lane clock.
> +
> +properties:
> +  compatible:
> +    const: st,stm32mp25-syscfg-clk
> +
> +  "#clock-cells":
> +    const: 1

[Severity: Medium]
Does this st,stm32mp25-syscfg-clk child node duplicate the clock provider
already defined on the parent SYSCFG node?

Commit 1b9482f8dfce ("dt-bindings: arm: stm32: add required #clock-cells
property") already added #clock-cells = <0> as a required property on
st,stm32mp23-syscfg and st,stm32mp25-syscfg in
Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml for this
display clock multiplexer, and the existing STM32MP23 and STM32MP25 dtsi
files already wire <&syscfg> as the LTDC reference clock:

Documentation/devicetree/bindings/arm/stm32/st,stm32-syscon.yaml:
  - if:
      properties:
        compatible:
          contains:
            enum:
              - st,stm32mp23-syscfg
              - st,stm32mp25-syscfg
    then:
      required:
        - "#address-cells"
        - "#clock-cells"
        - "#size-cells"
        - phy@2400
        - phy@2800

Adding a separate reg-less child node just to instantiate a platform
driver goes against Documentation/devicetree/bindings/writing-bindings.rst
and leaves two conflicting clock provider bindings on the same SYSCFG
block.  Could the parent SYSCFG node remain the clock provider directly
(for example by registering the clock or an auxiliary device from the
syscfg driver)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-ups_mp25_clk_syscfg-v1-0-667829cd290a@foss.st.com?part=1