Re: [EXTERNAL]Re: [PATCH 2/2] dt-bindings: net: pse-pd: Add bindings for Si3474 PSE controller

[Piotr Kubik <piotr.kubik@adtran.com>]

On 4/16/25 12:58, Krzysztof Kozlowski wrote:
> [Nie otrzymujesz często wiadomości e-mail z krzk@kernel.org. Dowiedz się, dlaczego jest to ważne, na stronie https://aka.ms/LearnAboutSenderIdentification ]
>
> On 16/04/2025 12:47, Piotr Kubik wrote:
>> From: Piotr Kubik <piotr.kubik@adtran.com>
>>
>> Add the Si3474 I2C Power Sourcing Equipment controller device tree
>> bindings documentation.
>>
>> Signed-off-by: Piotr Kubik <piotr.kubik@adtran.com>
>> ---
>>  .../bindings/net/pse-pd/skyworks,si3474.yaml  | 154 ++++++++++++++++++
>>  1 file changed, 154 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/net/pse-pd/skyworks,si3474.yaml
>
> Also looks like corrupted patch.
>
>>
>> diff --git
>> a/Documentation/devicetree/bindings/net/pse-pd/skyworks,si3474.yaml
>> b/Documentation/devicetree/bindings/net/pse-pd/skyworks,si3474.yaml
>> new file mode 100644
>> index 000000000000..fd48eeb2f79b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/pse-pd/skyworks,si3474.yaml
>> @@ -0,0 +1,154 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/pse-pd/skyworks,si3474.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Skyworks Si3474 Power Sourcing Equipment controller
>> +
>> +maintainers:
>> +  - Kory Maincent <kory.maincent@bootlin.com>
>
> This should be someone interested in this hardware, not subsystem
> maintainer.
>
>> +
>> +allOf:
>> +  - $ref: pse-controller.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - skyworks,si347
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  '#pse-cells':
>> +    const: 1
>> +
>> +  channels:
>> +    description: Each Si3474 is divided into two quad PoE controllers
>> +      accessible on different i2c addresses. Each set of quad ports can be
>> +      assigned to two physical channels (currently 4p support only).
>
> What this "currently" means? Limitation of hardware or Linux? If the
> latter, then drop.
>
>> +      This parameter describes the configuration of the ports conversion
>> +      matrix that establishes relationship between the logical ports and
>> +      the physical channels.
>> +    type: object
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      "#address-cells":
>> +        const: 1
>> +
>> +      "#size-cells":
>> +        const: 0
>> +
>> +    patternProperties:
>> +      '^channel@[0-3]$':
>> +        type: object
>> +        additionalProperties: false
>> +
>> +        properties:
>> +          reg:
>> +            maxItems: 1
>> +
>> +        required:
>> +          - reg
>> +
>> +    required:
>> +      - "#address-cells"
>> +      - "#size-cells"
>> +
>> +unevaluatedProperties: false
>
> This goes after required: block.
>
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +examples:
>> +  - |
>> +    i2c {
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +
>> +      ethernet-pse@26 {
>> +        compatible = "skyworks,si3474";
>> +        reg = <0x26>;
>> +
>> +        channels {
>> +          #address-cells = <1>;
>> +          #size-cells = <0>;
>> +          phys0_0: channel@0 {
>> +            reg = <0>;
>> +          };
>> +          phys0_1: channel@1 {
>> +            reg = <1>;
>> +          };
>> +          phys0_2: channel@2 {
>> +            reg = <2>;
>> +          };
>> +          phys0_3: channel@3 {
>> +            reg = <3>;
>> +          };
>> +        };
>> +        pse-pis {
>> +          #address-cells = <1>;
>> +          #size-cells = <0>;
>> +          pse_pi2: pse-pi@2 {
>> +            reg = <2>;
>> +            #pse-cells = <0>;
>> +            pairset-names = "alternative-a", "alternative-b";
>> +            pairsets = <&phys0_0>, <&phys0_1>;
>> +            polarity-supported = "MDI-X", "S";
>> +            vpwr-supply = <&reg_pse>;
>> +          };
>> +          pse_pi3: pse-pi@3 {
>> +            reg = <3>;
>> +            #pse-cells = <0>;
>> +            pairset-names = "alternative-a", "alternative-b";
>> +            pairsets = <&phys0_2>, <&phys0_3>;
>> +            polarity-supported = "MDI-X", "S";
>> +            vpwr-supply = <&reg_pse>;
>> +          };
>> +        };
>> +      };
>> +
>> +      ethernet-pse@27 {
>> +        compatible = "skyworks,si3474";
>
>
> This is the same as other example, so drop and keep only one.

Right, but Si3474 is specific, like it has two i2c addresses, one for each quad port.
That's why I kept both here to show how the full config for the IC looks like.
I agree it's almost the same and one will easily figure out how to configure the second one.
Anyway, if I update the driver according to Oleksij's comment regarding one driver instance
for both quads, this issue will be gone.

Regards,
Piotr