Re: [PATCH v2 03/11] dt-bindings: clock: en7523: add Documentation for Airoha AN7581 SCU SSR

[Rob Herring <robh@kernel.org>]

On Thu, Mar 20, 2025 at 02:00:26PM +0100, Christian Marangi wrote:
> The Airoha AN7581 SoC have in the SCU register space particular
> address that control how some peripheral are configured.
> 
> These are toggeled in the System Status Register and are used to
> toggle Serdes port for USB 3.0 mode or HSGMII, USB 3.0 mode or PCIe2
> or setup port for PCIe mode or Ethrnet mode (HSGMII/USXGMII).
> 
> Modes are mutually exclusive and selecting one mode cause the
> other feature to not work (example a mode in USB 3.0 cause PCIe
> port 2 to not work) This depends also on what is physically
> connected to the Hardware and needs to correctly reflect the
> System Status Register bits.
> 
> Special care is needed for PCIe port 0 in 2 line mode that
> requires both WiFi1 and WiFi2 Serdes port set to PCIe0 2 Line
> mode.
> 
> Expose these configuration as an enum of strings in the SCU node and
> also add dt-bindings header to reference each serdes port in DT.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../bindings/clock/airoha,en7523-scu.yaml     | 101 ++++++++++++++++--
>  MAINTAINERS                                   |   7 ++
>  include/dt-bindings/soc/airoha,scu-ssr.h      |  11 ++
>  3 files changed, 110 insertions(+), 9 deletions(-)
>  create mode 100644 include/dt-bindings/soc/airoha,scu-ssr.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> index fe2c5c1baf43..637ce0e06619 100644
> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> @@ -9,6 +9,7 @@ title: EN7523 Clock
>  maintainers:
>    - Felix Fietkau <nbd@nbd.name>
>    - John Crispin <nbd@nbd.name>
> +  - Christian Marangi <ansuelsmth@gmail.com>
>  
>  description: |
>    This node defines the System Control Unit of the EN7523 SoC,
> @@ -26,6 +27,23 @@ description: |
>  
>    The clocks are provided inside a system controller node.
>  
> +  The System Control Unit may also set different mode for the Serdes ports
> +  present on the SoC.
> +
> +  These are toggeled in the System Status Register and are used to
> +  toggle Serdes port for USB 3.0 mode or HSGMII, USB 3.0 mode or PCIe2
> +  or setup port for PCIe mode or Ethernet mode (HSGMII/USXGMII).
> +
> +  Modes are mutually exclusive and selecting one mode cause the
> +  other feature to not work (example a mode in USB 3.0 cause PCIe
> +  port 2 to not work) This depends also on what is physically
> +  connected to the Hardware and needs to correctly reflect the
> +  System Status Register bits.
> +
> +  Special care is needed for PCIe port 0 in 2 line mode that
> +  requires both WiFi1 and WiFi2 Serdes port set to PCIe0 2 Line
> +  mode.
> +
>  properties:
>    compatible:
>      items:
> @@ -49,6 +67,40 @@ properties:
>      description: ID of the controller reset line
>      const: 1
>  
> +  airoha,serdes-wifi1:
> +    description: Configure the WiFi1 Serdes port
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - pcie0_x2
> +      - pcie0_x1
> +      - ethernet
> +    default: pcie0_x1
> +
> +  airoha,serdes-wifi2:
> +    description: Configure the WiFi2 Serdes port
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - pcie0_x2
> +      - pcie1_x1
> +      - ethernet
> +    default: pcie1_x1
> +
> +  airoha,serdes-usb1:
> +    description: Configure the USB1 Serdes port
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - usb3
> +      - ethernet
> +    default: usb3
> +
> +  airoha,serdes-usb2:
> +    description: Configure the USB2 Serdes port
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - usb3
> +      - pcie2_x1
> +    default: usb3

Couldn't you make this a phy provider and use the mode flags in the 
phy cells?

> +
>  required:
>    - compatible
>    - reg
> @@ -64,6 +116,12 @@ allOf:
>          reg:
>            minItems: 2
>  
> +        airoha,serdes-wifi1: false
> +        airoha,serdes-wifi2: false
> +
> +        airoha,serdes-usb1: false
> +        airoha,serdes-usb2: false
> +
>          '#reset-cells': false
>  
>    - if:
> @@ -75,6 +133,24 @@ allOf:
>          reg:
>            maxItems: 1
>  
> +  - if:
> +      properties:
> +        airoha,serdes-wifi1:
> +          const: pcie0_x2

This is also true if airoha,serdes-wifi1 is not present. Probably not 
what you intended.

> +    then:
> +      properties:
> +        airoha,serdes-wifi2:
> +          const: pcie0_x2
> +
> +  - if:
> +      properties:
> +        airoha,serdes-wifi2:
> +          const: pcie0_x2
> +    then:
> +      properties:
> +        airoha,serdes-wifi1:
> +          const: pcie0_x2
> +
>  additionalProperties: false