Messages in this thread | | | Date | Mon, 20 Nov 2023 11:09:44 +0100 | From | Köry Maincent <> | Subject | Re: [PATCH net-next 2/9] ethtool: Expand Ethernet Power Equipment with PoE alongside PoDL |
| |
+Oleksij
Sorry forgot to CC you the series. Maybe you should add yourself to the MAINTAINERS of pse-pd drivers subsystem?
On Sat, 18 Nov 2023 18:38:43 +0100 Andrew Lunn <andrew@lunn.ch> wrote:
> On Thu, Nov 16, 2023 at 03:01:34PM +0100, Kory Maincent wrote: > > In the current PSE interface for Ethernet Power Equipment, support is > > limited to PoDL. This patch extends the interface to accommodate the > > objects specified in IEEE 802.3-2022 145.2 for Power sourcing > > Equipment (PSE). > > Sorry for taking a while getting to these patches. Plumbers and other > patches have been keeping me busy.
Don't worry you are doing a great job as a net maintainer and I won't raise any remarks on delay considering how you are doing your job. Thanks again for your review!!
> I'm trying to get my head around naming... Is there some sort of > hierarchy? Is PSE the generic concept for putting power down the > cable? Then you have the sub-type PoDL, and the sub-type PoE?
In fact as we discussed with Oleksij I decided to keep the naming as close as possible to the IEEE 802.3 standard. On the standard the PODL is naming like this aPoDLPSE* (ex: aPoDLPSEAdminState) and the PSE is naming like this aPSE* (ex: aPSEAdminState) without any PoE prefix. Maybe it is due to PoE being supported before PoDL and they didn't expect the PoDL part.
> > struct pse_control_config { > > enum ethtool_podl_pse_admin_state podl_admin_control; > > + enum ethtool_pse_admin_state admin_control; > > When i look at this, it seems to me admin_control should be generic > across all schemes which put power down the cable, and > podl_admin_control is specific to how PoDL puts power down the cable. > > Since you appear to be adding support for a second way to put power > down the cable, i would expect something like poe_admin_control being > added here. But maybe that is in a later patch?
No as said above admin_control is for PoE and podl_admin_control is for PoDL. Maybe you prefer to use poe_admin_control, and add poe prefix in the poe variables. It will differ a bit from the IEEE standard naming but I agreed that it would be more understandable in the development part.
I am open to the change. Oleksij do you agree?
Regards, -- Köry Maincent, Bootlin Embedded Linux and kernel engineering https://bootlin.com
| |