Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation

[Kory Maincent <kory.maincent@bootlin.com>]

On Sun, 22 Dec 2024 19:54:37 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

>         transformer {
>             model = "ABC123"; /* Transformer model number */
>             manufacturer = "TransformerCo"; /* Manufacturer name */
> 
>             pairs {
>                 pair@0 {
>                     name = "A"; /* Pair A */
>                     pins = <1 2>; /* Connector pins */
>                     phy-mapping = <PHY_TX0_P PHY_TX0_N>; /* PHY pin mapping */
>                     center-tap = "CT0"; /* Central tap identifier */
>                     /* if pse-positive and pse-negative are present -
> polarity is configurable */ pse-positive = <PSE_OUT0_0>; /* PSE-controlled
> positive pin -> CT0 */ pse-negative = <PSE_OUT0_1>; /* PSE-controlled
> negative pin -> CT0 */ };
>                 pair@1 {
>                     name = "B"; /* Pair B */
>                     pins = <3 6>; /* Connector pins */
>                     phy-mapping = <PHY_RX0_P PHY_RX0_N>;
>                     center-tap = "CT1"; /* Central tap identifier */
>                     pse-positive = <PSE_OUT1_0>;
>                     pse-negative = <PSE_OUT1_1>;
>                 };
>                 pair@2 {
>                     name = "C"; /* Pair C */
>                     pins = <4 5>; /* Connector pins */
>                     phy-mapping = <PHY_TXRX1_P PHY_TXRX1_N>; /* PHY
> connection only */ center-tap = "CT2"; /* Central tap identifier */
>                     pse-positive = <PSE_OUT2_0>;
>                     pse-negative = <PSE_OUT2_1>;
>                 };
>                 pair@3 {
>                     name = "D"; /* Pair D */
>                     pins = <7 8>; /* Connector pins */
>                     phy-mapping = <PHY_TXRX2_P PHY_TXRX2_N>; /* PHY
> connection only */ center-tap = "CT3"; /* Central tap identifier */
>                     pse-positive = <PSE_OUT3_0>;
>                     pse-negative = <PSE_OUT3_1>;
>                 };
>             };
>         };
> 
>         pse = <&pse1>; /* Reference to the attached PSE controller */

The PSE pairset and polarity are already described in the PSE bindings.
https://elixir.bootlin.com/linux/v6.12.6/source/Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml

I am not sure it is a good idea to have PSE information at two different places.

>         leds {
>             ethernet-leds {
>                 link = <&eth_led0>; /* Link status LED */
>                 activity = <&eth_led1>; /* Activity LED */
>                 speed = <&eth_led2>; /* Speed indication LED */
>             };
> 
>             poe-leds {
>                 power = <&poe_led0>; /* PoE power status LED */
>                 fault = <&poe_led1>; /* PoE fault indication LED */
>                 budget = <&poe_led2>; /* PoE budget usage LED */
>             };
>         };

Maybe the PoE leds should also land in our pse-pis binding.

> In case of PoDL, we will have something like this:
> 
> pair@0 {
>     name = "A"; /* Single pair for 10BaseT1L */
>     pins = <1 2>; /* Connector pins */
>     phy-mapping = <PHY_TXRX0_P PHY_TXRX0_N>; /* PHY pin mapping */
>     podl-mapping = <PODL_OUT0_P PODL_OUT0_N>; /* PoDL mapping: Positive and
> negative outputs */ };

We should do the same for PoDL. Put all information in the same place, the PSE
bindings.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com