[PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[Krzysztof Kozlowski]

On 07/10/2024 18:36, Markus Stockhausen wrote:
> Add bindings for the SerDes of the Realtek Otto platform. These are
> network Switch SoCs with up to 52 ports divided into four different
> model lines.
> 
> Changes in v2:
> - new subject
> - removed patch command sequences
> - renamed parameter controlled-ports to realtek,controlled-ports

Changelog goes under ---.


> 
> Signed-off-by: Markus Stockhausen <markus.stockhausen at gmx.de>
> ---
>  .../bindings/phy/realtek,otto-serdes.yaml     | 95 +++++++++++++++++++
>  1 file changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> new file mode 100644
> index 000000000000..a72ac206b35f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml

Nothing improved.

> @@ -0,0 +1,95 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/realtek,otto-serdes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek Otto SerDes controller
> +
> +maintainers:
> +  - Markus Stockhausen <markus.stockhausen at gmx.de>
> +
> +description:
> +  The MIPS based Realtek Switch SoCs of the Realtek RTL838x, RTL839x, RTL930x and RTL931x series
> +  have multiple SerDes built in. They are linked to single, quad or octa PHYs like the RTL8218B,
> +  RTL8218D or RTL8214FC and are one of the integral part of the up-to-52-port switch architecture.
> +
> +  Although these SerDes controllers have common basics they behave differently on the SoC families
> +  and rely on heavy register magic. To keep the driver clean it can load patch sequences from
> +  devictree and execute them during the controller actions like phy_init(), ...
> +
> +  The driver exposes the SerDes registers different from the hardware but instead gives a
> +  consistent view and programming interface. So the RTL838x series has 6 ports and 4 pages, the
> +  RTL839x has 14 ports and 12 pages, the RTL930x has 12 ports and 64 pages and the RTL931x has
> +  14 ports and 192 pages.

Totally messed wrapping. Please wrap your code as Linux coding style.

> +
> +properties:
> +  $nodename:
> +    pattern: "^serdes@[0-9a-f]+$"
> +
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,rtl8380-serdes
> +          - realtek,rtl8390-serdes
> +          - realtek,rtl9300-serdes
> +          - realtek,rtl9310-serdes
> +
> +  reg:
> +    items:
> +      description:
> +        The primary SerDes paged register memory location. Other SerDes control and management
> +        registers are distributed all over the I/O memory space and are identified by the driver.

What happened here? I asked only about |. Why are you adding unrelated
changes?

Anyway, still not tested and still does not look any other binding.

> +
> +  "#phy-cells":
> +    const: 4
> +    description:
> +      The first number defines the SerDes to use. The second number a linked SerDes. E.g. if a octa
> +      1G PHY is attached to two QSGMII SerDes. The third number is the first switch port this
> +      SerDes is working for, the fourth number is the last switch port the SerDes is working for.
> +
> +  realtek,controlled-ports:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      A bit mask defining the ports that are actively controlled by the driver. In case a bit is

Driver? Bindings are not about drivers. Drop.

I don't think you implemented my feedback.

> +      not set the driver will only process read operations on the SerDes. This is just in case the
> +      SerDes has been setup by U-Boot correctly and the driver malfunctions. If not set the driver
> +      will control all SerDes.



> +
> +reguired:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +
> +additionalProperties:
> +  false

Please open any existing binding and do it like there. Or start from
scratch from example-schema.

> +
> +examples:
> +  - |
> +    serdes: serdes at 1b00e780 {
> +      compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes";
> +      reg = <0x1b00e780 0x1200>;
> +      controlled-ports = <0x003f>;
> +      #phy-cells = <4>;
> +    };

One example is enough.

... and still not tested. Sending untested code is waste of our time.

Best regards,
Krzysztof