On Tue, 5 Dec 2023 16:23:21 +0100 Köry Maincent <kory.maincent@xxxxxxxxxxx> wrote: > On Tue, 5 Dec 2023 15:21:47 +0100 > Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote: > > > On Tue, Dec 05, 2023 at 02:31:23PM +0100, Köry Maincent wrote: > > > On Tue, 5 Dec 2023 11:15:01 +0100 > > > Köry Maincent <kory.maincent@xxxxxxxxxxx> wrote: > > > > > > > On Tue, 5 Dec 2023 07:36:06 +0100 > > > > Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote: > > > > > > > > I would expect a devicetree like this: > > > > > > > > > > ethernet-pse@3c { > > > > > // controller compatible should be precise > > > > > compatible = "microchip,pd69210"; > > > > > reg = <0x3c>; > > > > > #pse-cells = <1>; > > > > > > > > > > managers { > > > > > manager@0 { > > > > > // manager compatible should be included, since we are > > > > > // able to campare it with communication results > > > > > compatible = "microchip,pd69208t4" > > > > > // addressing corresponding to the chip select > > > > > addressing reg = <0>; > > > > > > > > > > physical-ports { > > > > > phys0: port@0 { > > > > > // each of physical ports is actually a regulator > > > > > > > > > > > If this phys0 is a regulator, which device will be the consumer of this > > > regulator? log_port0 as the phys0 regulator consumer seems a bit odd! > > > > Why? > > > > > A 8P8C node should be the consumer. > > > > PHY is not actual consumer of this regulator. State of the Ethernet PHY > > is not related to the power supply. We should deliver power independent > > of network interface state. There is no other local consumer we can > > use in this case. > > Just to be clear, are you saying we should use the regulator framework or is > it simply a way of speaking as it behaves like regulator? > > > > Finally, the devicetree would not know the matrix between logical port and > > > physical port, this would be cleaner. > > > > > > Did I miss something? > > > > In case different PSE suppliers are linked withing the PHY node, we > > loose most of information needed for PSE functionality. For example how > > we will know if our log_port supports PoE4 and PoE2 mode, or only PoE2. > > This information is vital for proper PSE configuration, this is why I > > suggested to have logica-ports subnodes. With the price of hawing huge > > DT on a switch with 48 ports. > > It could be known in the of_pse_control_get() function if there is two > phandles in the "pses" parameter. Then we add a new enum c33_pse_mode member > in the pse_control struct to store the mode. > PoE2 and PoE4 is not a parameter of the logical port, it depends of the number > of PSE ports wired to an 8P8C connector. > > In fact I am also working on the tps23881 driver which aimed to be added to > this series soon. In the tps23881 case the logical port can only be configured > to one physical port. Two physical ports (which mean two logical ports) can > still be used to have PoE4 mode. > For PoE4, in the pd692x0 driver we use one logical port (one pse_control->id) > configured to two physical ports but in the tps23881 we will need two logical > ports (two pse_control->id). > > So with the tps23881 driver we will need two phandle in the "pses" parameter > to have PoE4, that's why my proposition seems relevant. > > The same goes with your pse-regulator driver, you can't do PoE4 if two > regulators is needed for each two pairs group. Oleksij, what your thought for the binding I have proposed in the thread. For the PoE4 we could add a "pses-poe4" bool property alongside the two phandle in "pses" property. Here is the current binding proposition: ethernet-pse@3c { // controller compatible should be precise compatible = "microchip,pd69210"; reg = <0x3c>; #pse-cells = <1>; managers { manager@0 { // manager compatible should be included, since we are // able to compare it with communication results compatible = "microchip,pd69208t4" // addressing corresponding to the chip select addressing reg = <0>; physical-ports { phys_port0: port@0 { // each of physical ports is actually a regulator reg = <0>; }; phy_port1: port@1 { reg = <1>; }; phy_port2: port@2 { reg = <2>; }; ... } manager@1 { ... }; }; }; .... ethernet-phy@1 { reg = <1>; pses-poe4; pses = <&phy_port0, &phy_port1>; }; ethernet-phy@2 { reg = <2>; pses = <&phy_port2>; } Regards, -- Köry Maincent, Bootlin Embedded Linux and kernel engineering https://bootlin.com