Re: [PATCH 1/2] dt-bindings: phy: realtek: Add Realtek DHC RTD SoC PCIe PHY

[Conor Dooley <conor@xxxxxxxxxx>]

On Fri, Dec 01, 2023 at 06:52:06PM +0800, Tzuyi Chang wrote:
> Add the device tree bindings for the Realtek DHC(Digital Home Center)
> RTD SoCs PCIe PHY.
> 
> Signed-off-by: Tzuyi Chang <tychang@xxxxxxxxxxx>
> ---
>  .../bindings/phy/realtek,rtd-pcie-phy.yaml    | 61 +++++++++++++++++++
>  1 file changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml
> new file mode 100644
> index 000000000000..44ff23f698e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,rtd-pcie-phy.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Realtek Semiconductor Corporation
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/realtek,rtd-pcie-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek DHC PCIe PHY
> +
> +maintainers:
> +  - Tzuyi Chang <tychang@xxxxxxxxxxx>
> +
> +description:
> +  Realtek PCIe PHY supports the DHC(Digital Home Center) RTD series SoCs.
> +  The PCIe PHY driver is designed to support physical layer functionality
> +  of the PCIe controller.
> +
> +properties:
> +  compatible:
> +    enum:

> +      - realtek,rtd1319-pcie0-phy
> +      - realtek,rtd1319-pcie1-phy
> +      - realtek,rtd1319-pcie2-phy
> +      - realtek,rtd1619b-pcie1-phy
> +      - realtek,rtd1619b-pcie2-phy

Please explain why different PHYs on the same SoC need different
compatibles.

> +      - realtek,rtd1319d-pcie1-phy
> +      - realtek,rtd1315e-pcie1-phy

And why bother with the 1 here given there is no 0 or 2?

This looks suspiciously like abuse of the compatible - especially since
most of the ops are the same despite the differing compatibles. The case
where that does not apply, it looks like the issue is down to the
portion of the nvmem cell corresponding to the PHY, which has nothing to
do with the programming model of the PHY itself IMO.

Cheers,
Conor.

> +
> +  "#phy-cells":
> +    const: 0
> +
> +  nvmem-cells:
> +    maxItems: 1
> +    description:
> +      Phandle to nvmem cell that contains 'Tx swing trim'
> +      tuning parameter value for PCIe phy.
> +
> +  nvmem-cell-names:
> +    items:
> +      - const: tx_swing_trim
> +
> +  realtek,pcie-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle of syscon used to control PCIe MDIO register.
> +
> +required:
> +  - compatible
> +  - realtek,pcie-syscon
> +  - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pcie1_phy {
> +        compatible = "realtek,rtd1319d-pcie1-phy";
> +        realtek,pcie-syscon = <&pcie1>;
> +        #phy-cells = <0>;
> +        nvmem-cells = <&otp_pcie_tx_swing_trim>;
> +        nvmem-cell-names = "tx_swing_trim";
> +    };
> -- 
> 2.43.0
>

Attachment: signature.asc
Description: PGP signature