Re: [PATCH 3/4] dt-bindings: clock: Add Siflower SF19A2890 peripheral clock/reset managers

[Conor Dooley]

On Wed, Nov 04, 2026 at 02:21:00AM -0500, Joseph Telaak wrote:
> Each peripheral block of the Siflower SF19A2890 has a clock and reset
> manager of its own, with a reset register, a clock gate register and a
> bus output enable. The blocks differ in which gate and reset bits they
> use, so each has its own compatible, and clock N of a manager is its
> input clock N, gated.
> 
> Based on Chuanhong Guo's binding, which described every block with one
> compatible and its bit layout in properties.
> 
> Signed-off-by: Joseph Telaak <joseph@xxxxxxxxxx>
> ---
>  .../clock/siflower,sf19a2890-periph-crm.yaml       | 169 +++++++++++++++++++++
>  1 file changed, 169 insertions(+)
> 
> 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 000000000..7f0bce603
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/siflower,sf19a2890-periph-crm.yaml
> @@ -0,0 +1,169 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/siflower,sf19a2890-periph-crm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Siflower SF19A2890 peripheral clock and reset manager
> +
> +maintainers:
> +  - Joseph Telaak <joseph@xxxxxxxxxx>
> +
> +description: |
> +  Each peripheral, or group of peripherals, of the Siflower SF19A2890 has
> +  a clock and reset manager of its own: a reset register, a clock gate
> +  register, and a bus output enable that the peripherals need as well as
> +  their clocks to answer bus requests.
> +
> +  Clock N of the manager is input clock N, gated. The reset lines are:
> +
> +    gmac-crm:  0 GMAC
> +    usb-crm:   0 USB controller, 1 USB PHY power-on, 2 USB PHY
> +    uart-crm:  0 UART0, 1 UART1, 2 UART2
> +    the others: 0 the block

I think you should put these in a binding header file, rather than just
put them in a description like this.
I think you really need one for the clocks too, since you've got things
like
		uart0: serial@18300000 {

			clocks = <&uart_crm 3>, <&uart_crm 0>;
			clock-names = "uartclk", "apb_pclk";

		};

		uart1: serial@18301000 {

			clocks = <&uart_crm 4>, <&uart_crm 1>;
			clock-names = "uartclk", "apb_pclk";

		};
and it's not some simple case of 0 is always apb and 1 is always some
ancillary clock, the meanings depend entirely on what which crm node
we're looking at.

The rest of this seems fine though.
pw-bot: changes-requested

Thanks,
Conor.

> +
> +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
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 6
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  "#reset-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - "#clock-cells"
> +  - "#reset-cells"
> +
> +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
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/siflower,sf19a2890-clk.h>
> +
> +    clock-controller@19e24c00 {
> +        compatible = "siflower,sf19a2890-uart-crm";
> +        reg = <0x19e24c00 0x10>;
> +        clocks = <&clk CLK_MUXDIV_PBUS>, <&clk CLK_MUXDIV_PBUS>,
> +                 <&clk CLK_MUXDIV_PBUS>, <&clk CLK_MUXDIV_UART>,
> +                 <&clk CLK_MUXDIV_UART>, <&clk CLK_MUXDIV_UART>;
> +        #clock-cells = <1>;
> +        #reset-cells = <1>;
> +    };
> 
> -- 
> Git-155)
>

Attachment: signature.asc
Description: PGP signature