Re: [PATCH v9 1/3] dt-bindings: clock: airoha: Document support for AN7583 clock

[Brian Masney]

Hi Christian,

> Document support for Airoha AN7583 clock. This is based on the EN7523
> clock schema with the new requirement of the "airoha,chip-scu".
> 
> The register for clock and reset for normal system and PCIe are scattered
> between the "SCU" and "chip SCU" registers (they are 2 different register
> block, the name is taken from the programming guide)
> 
> - Chip SCU provide register access to some clk gate and clk rate
> - SCU provide register for reset, PCIe clock, other clock gate and PHY
>   SERDES.
> 
> Upcoming and current airoha clock driver all follow this pattern of
> declaring one of the 2 register and use a sysconf for the other as the 2
> register block are tighlty coupled.
> 
> EN7581 internally handle the syscon with a direct reference by the use of
> syscon_regmap_lookup_by_compatible while AN7583 wants to make this
> explicit in DT with the new "airoha,chip-scu" phandle.
> 
> Similar to EN7581, also AN7583 permit the selection of the serdes line.
> 
> Add additional binding for additional clock and reset lines.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
> Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> ---
>  .../bindings/clock/airoha,en7523-scu.yaml     | 23 +++++++
>  include/dt-bindings/clock/en7523-clk.h        |  3 +
>  .../dt-bindings/reset/airoha,an7583-reset.h   | 65 +++++++++++++++++++
>  3 files changed, 91 insertions(+)
>  create mode 100644 include/dt-bindings/reset/airoha,an7583-reset.h
>
> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> index fc37786ac7ed..8b94113d00ab 100644
> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> @@ -31,6 +31,7 @@ properties:
>    compatible:
>      items:
>        - enum:
> +          - airoha,an7583-scu
>            - airoha,en7523-scu
>            - airoha,en7581-scu
>            - econet,en751221-scu
> @@ -57,12 +58,34 @@ properties:
>        available serdes port.
>      const: 1
>  
> +  airoha,chip-scu:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the Chip SCU providing the registers required
> +      for configuring the PCIe related clocks and resets.

How about this?

    description: phandle to the Chip SCU syscon providing the registers
      used to read the base clock source and divider settings.

> +
>  required:
>    - compatible
>    - reg
>    - '#clock-cells'
>  
>  allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: airoha,an7583-scu
> +
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 1
> +
> +      required:
> +        - airoha,chip-scu

The Sashiko feedback about including #reset-cells looks legit here as
well.

> +
> +    else:
> +      properties:
> +        airoha,chip-scu: false
> +
>    - if:
>        properties:
>          compatible:
> diff --git a/include/dt-bindings/clock/en7523-clk.h b/include/dt-bindings/clock/en7523-clk.h
> index edfa64045f52..0fbbcb7b1b25 100644
> --- a/include/dt-bindings/clock/en7523-clk.h
> +++ b/include/dt-bindings/clock/en7523-clk.h
> @@ -14,4 +14,7 @@
>  
>  #define EN7581_CLK_EMMC		8
>  
> +#define AN7583_CLK_MDIO0	9
> +#define AN7583_CLK_MDIO1	10
> +
>  #endif /* _DT_BINDINGS_CLOCK_AIROHA_EN7523_H_ */
> diff --git a/include/dt-bindings/reset/airoha,an7583-reset.h b/include/dt-bindings/reset/airoha,an7583-reset.h
> new file mode 100644
> index 000000000000..410e19802cf7
> --- /dev/null
> +++ b/include/dt-bindings/reset/airoha,an7583-reset.h
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2024 AIROHA Inc
> + * Author: Christian Marangi <ansuelsmth@xxxxxxxxx>
> + */
> +
> +#ifndef __DT_BINDINGS_RESET_CONTROLLER_AIROHA_AN7583_H_
> +#define __DT_BINDINGS_RESET_CONTROLLER_AIROHA_AN7583_H_
> +
> +/* RST_CTRL2 */
> +#define AN7583_XPON_PHY_RST		 0
> +#define AN7583_GPON_OLT_RST		 1
> +#define AN7583_CPU_TIMER2_RST		 2
> +#define AN7583_HSUART_RST		 3
> +#define AN7583_UART4_RST		 4
> +#define AN7583_UART5_RST		 5
> +#define AN7583_I2C2_RST			 6
> +#define AN7583_XSI_MAC_RST		 7
> +#define AN7583_XSI_PHY_RST		 8
> +#define AN7583_NPU_RST			 9
> +#define AN7583_TRNG_MSTART_RST		10
> +#define AN7583_DUAL_HSI0_RST		11
> +#define AN7583_DUAL_HSI1_RST		12
> +#define AN7583_DUAL_HSI0_MAC_RST	13
> +#define AN7583_DUAL_HSI1_MAC_RST	14
> +#define AN7583_XPON_XFI_RST             15

This line uses spaces before the value while the rest of the file
uses tabs.

Brian

-- 
Brian Masney <masneyb@xxxxxxxxxx>