* [PATCH net-next RFC 0/5] net: phy: Introduce a port representation
@ 2024-12-20 20:14 Maxime Chevallier
2024-12-20 20:15 ` [PATCH net-next RFC 1/5] net: ethtool: common: Make BaseT a 4-lanes mode Maxime Chevallier
` (5 more replies)
0 siblings, 6 replies; 26+ messages in thread
From: Maxime Chevallier @ 2024-12-20 20:14 UTC (permalink / raw)
To: davem
Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina,
Florian Fainelli, Heiner Kallweit, Vladimir Oltean,
Köry Maincent, Marek Behún, Oleksij Rempel,
Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart
Hello everyone,
This is a long overdue series to kick-off the introduction of a better representation
of front-facing ports for ethernet interfaces.
First, a short disclaimer. This series is RFC, and there are quite a lot of
shortcomings :
- There's no DT binding although this series adds a generic representation of
ports
- The port representation is in a minimal form
- No SFP support is included, but it will be required for that series to come
out of RFC as we can't gracefully handle multi-port interfaces without it.
These shortcomigs come from timing constraints, but also because I'd like to
start discussing that topic with some code as a basis.
For now, the only representation we have about the physical ports of an interface
come from the 'port' field (PORT_FIBRE, PORT_TP, PORT_MII, etc.), the presence or
not of an SFP module and the linkmodes reported by the ethtol ksettings ops. This
isn't enough to get a good idea of what the actual interface with the outside world
looks like.
The end-goal of the work this series is a part of is to get support for multi-port
interfaces. My end use-case has 2 ports, each driven by a dedicated PHY, but this
also applies to dual-port PHYs.
The current series introduces the object "struct phy_port". The naming may be
improved, as I think we could consider supporting port representation without
depending on phylib (not the case in this RFC). For now, not only do we integrate
that work in phylib, but only PHY-driven ports are supported.
In some situations, having a good representation of the physical port in devicetree
proves to be quite useful. We're seeing vendor properties to address the lack of
port representation such as micrel,fiber-mode or ti,fiber-mode, but there are needs
for more (glitchy straps that detect fiber mode on a PHY connected to copper,
RJ45 ports connected with 2 lanes only, ...).
As I said this RFC has no binding, sorry about that, but let's take a look at
the proposed DT syntax :
Example 1 : PHY with RJ45 connected with 2 lanes only
&mdio {
ge_phy: ethernet-phy@0 {
reg = <0>;
mdi {
port@0 {
media = "BaseT",
lanes = <2>;
};
};
};
};
Example 2 : PHY with a 100BaseFX port, without SFP
&mdio {
fiber-phy: ethernet-phy@0 {
reg = <0>;
mdi {
port@0 {
media = "BaseF",
lanes = <1>;
};
};
};
};
These ports may even be used to specify PSE-PD information for PoE ports that
are drivern by a dedicated PoE controller sitting in-between the PHY and the
connector :
&mdio {
ge_phy: ethernet-phy@0 {
reg = <0>;
mdi {
port@0 {
media = "BaseT",
lanes = <4>;
pse = <&pse1>;
};
};
};
};
The ports are initialized using the following sequence :
1: The PHY driver's probe() function indicated the max number of ports the device
can control
2: We parse the devicetree to find generic port representations
3: If we don't have at least one port from DT, we create one
4: We call the phy's .attach_port() for each port created so far. This allows
the phy driver either to take action based on the generic port devicetree
indications, or to populate the port information based on straps and
vendor-specific DT properties (think micrel,fiber-mode and similar)
5: If the ports are still not initialized (no .attach_port, no generic DT), then
we use snesible default value from what the PHY's supported modes.
6: We reconstruct the PHY's supported field in case there are limitations from the
port (2 lanes on BaseT for example). This last step will need to be changed
when SFP gets added.
So, the current work is only about init. The next steps for that work are :
- Introduce phy_port_ops, including a .configure() and a .read_status() to get
proper support for multi-port PHYs. This also means maintaining a list of
advertising/lp_advertising modes for each port.
- Add SFP support. It's a tricky part, the way I see that and have prototyped is
by representing the SFP cage itself as a port, as well as the SFP module's port.
ports therefore become chainable.
- Add the ability to list the ports in userspace.
Prototype work for the above parts exist, but due to lack of time I couldn't get
them polished enough for inclusion in that RFC.
Let me know if you see this going in the right direction, I'm really all ears
for suggestions on this, it's quite difficult to make sure no current use-case
breaks and no heavy rework is needed in PHY drivers.
Patches 1, 2 and 3 are preparatory work for the mediums representation. Patch 4
introduces the phy_port and patch 5 shows an example of usage in the dp83822 phy.
Thanks,
Maxime
Maxime Chevallier (5):
net: ethtool: common: Make BaseT a 4-lanes mode
net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values
net: ethtool: Export the link_mode_params definitions
net: phy: Introduce PHY ports representation
net: phy: dp83822: Add support for phy_port representation
drivers/net/phy/Makefile | 2 +-
drivers/net/phy/dp83822.c | 60 +++++++----
drivers/net/phy/phy_device.c | 167 +++++++++++++++++++++++++++++
drivers/net/phy/phy_port.c | 159 ++++++++++++++++++++++++++++
include/linux/ethtool.h | 62 +++++++++++
include/linux/phy.h | 31 ++++++
include/linux/phy_port.h | 69 ++++++++++++
net/ethtool/common.c | 197 +++++++++++++++++++----------------
net/ethtool/common.h | 7 --
9 files changed, 633 insertions(+), 121 deletions(-)
create mode 100644 drivers/net/phy/phy_port.c
create mode 100644 include/linux/phy_port.h
--
2.47.1
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH net-next RFC 1/5] net: ethtool: common: Make BaseT a 4-lanes mode 2024-12-20 20:14 [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Maxime Chevallier @ 2024-12-20 20:15 ` Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 2/5] net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values Maxime Chevallier ` (4 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Maxime Chevallier @ 2024-12-20 20:15 UTC (permalink / raw) To: davem Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Köry Maincent, Marek Behún, Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart When referring to BaseT ethernet, we are most of the time thinking of BaseT4 ethernet on Cat5/6/7 cables. This is therefore BaseT4, although BaseT4 is also possible for 100BaseTX. This is even more true now that we have a special __LINK_MODE_LANES_T1 mode especially for Single Pair ethernet. Mark BaseT as being a 4-lanes mode. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- net/ethtool/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ethtool/common.c b/net/ethtool/common.c index 02f941f667dd..5a9c09ce57d5 100644 --- a/net/ethtool/common.c +++ b/net/ethtool/common.c @@ -244,7 +244,7 @@ static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS); #define __LINK_MODE_LANES_LR8_ER8_FR8 8 #define __LINK_MODE_LANES_LRM 1 #define __LINK_MODE_LANES_MLD2 2 -#define __LINK_MODE_LANES_T 1 +#define __LINK_MODE_LANES_T 4 #define __LINK_MODE_LANES_T1 1 #define __LINK_MODE_LANES_X 1 #define __LINK_MODE_LANES_FX 1 -- 2.47.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH net-next RFC 2/5] net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values 2024-12-20 20:14 [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 1/5] net: ethtool: common: Make BaseT a 4-lanes mode Maxime Chevallier @ 2024-12-20 20:15 ` Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 3/5] net: ethtool: Export the link_mode_params definitions Maxime Chevallier ` (3 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Maxime Chevallier @ 2024-12-20 20:15 UTC (permalink / raw) To: davem Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Köry Maincent, Marek Behún, Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart In an effort to have a better representation of Ethernet ports, introduce enumeration values representing the various ethernet Mediums. This is part of the 802.3 naming convention, for example : 1000 Base T 4 | | | | | | | \_ lanes (4) | | \___ Medium (T == Twisted Copper Pairs) | \_______ Baseband transmission \____________ Speed Other example : 10000 Base K X 4 | | \_ lanes (4) | \___ encoding (BaseX is 8b/10b while BaseR is 66b/64b) \_____ Medium (K is backplane ethernet) In the case of representing a physical port, only the medium and number of lanes should be relevant. One exception would be 1000BaseX, which is currently also used as a medium in what appears to be any of 1000BaseSX, 1000BaseFX, 1000BaseCX and 1000BaseLX. These mediums are set in the net/ethtool/common.c lookup table that maintains a list of all linkmodes with their number of lanes, medium, encoding, speed and duplex. One notable exception to this is 100M BaseT Ethernet. 100BaseTX is a 2-lanes protocol but it will also work on 4-lanes cables, so the lookup table contains 2 sets of lane numbers, indicating the min number of lanes for a protocol to work and the "nominal" number of lanes as well. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- include/linux/ethtool.h | 48 ++++++++++ net/ethtool/common.c | 194 +++++++++++++++++++++------------------- net/ethtool/common.h | 2 + 3 files changed, 153 insertions(+), 91 deletions(-) diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index f711bfd75c4d..dc02a1bef549 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -203,6 +203,54 @@ static inline u8 *ethtool_rxfh_context_key(struct ethtool_rxfh_context *ctx) void ethtool_rxfh_context_lost(struct net_device *dev, u32 context_id); +enum ethtool_link_medium { + ETHTOOL_LINK_MEDIUM_BASET = 0, + ETHTOOL_LINK_MEDIUM_BASEK, + ETHTOOL_LINK_MEDIUM_BASES, + ETHTOOL_LINK_MEDIUM_BASEC, + ETHTOOL_LINK_MEDIUM_BASEL, + ETHTOOL_LINK_MEDIUM_BASED, + ETHTOOL_LINK_MEDIUM_BASEE, + ETHTOOL_LINK_MEDIUM_BASEF, + ETHTOOL_LINK_MEDIUM_BASEV, + ETHTOOL_LINK_MEDIUM_BASEMLD, /* I don't know what that is */ + ETHTOOL_LINK_MEDIUM_BASEX, /* Not a true medium, but abused... */ + ETHTOOL_LINK_MEDIUM_NONE, + + __ETHTOOL_LINK_MEDIUM_LAST, +}; + +static inline const char *phy_mediums(enum ethtool_link_medium medium) +{ + switch (medium) { + case ETHTOOL_LINK_MEDIUM_BASET: + return "BaseT"; + case ETHTOOL_LINK_MEDIUM_BASEK: + return "BaseK"; + case ETHTOOL_LINK_MEDIUM_BASES: + return "BaseS"; + case ETHTOOL_LINK_MEDIUM_BASEC: + return "BaseC"; + case ETHTOOL_LINK_MEDIUM_BASEL: + return "BaseL"; + case ETHTOOL_LINK_MEDIUM_BASED: + return "BaseD"; + case ETHTOOL_LINK_MEDIUM_BASEE: + return "BaseE"; + case ETHTOOL_LINK_MEDIUM_BASEF: + return "BaseF"; + case ETHTOOL_LINK_MEDIUM_BASEV: + return "BaseV"; + case ETHTOOL_LINK_MEDIUM_BASEMLD: + return "BaseMLD"; + case ETHTOOL_LINK_MEDIUM_BASEX: + return "BaseX"; + case ETHTOOL_LINK_MEDIUM_NONE: + return "None"; + default: return "unknown"; + } +} + /* declare a link mode bitmap */ #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \ DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS) diff --git a/net/ethtool/common.c b/net/ethtool/common.c index 5a9c09ce57d5..41e7598f6ed7 100644 --- a/net/ethtool/common.c +++ b/net/ethtool/common.c @@ -255,11 +255,22 @@ static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS); #define __LINK_MODE_LANES_DR8_2 8 #define __LINK_MODE_LANES_T1BRR 1 -#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex) \ +#define __DEFINE_LINK_MODE_PARAMS_LANES(_speed, _type, _min_lanes, _lanes, _duplex, _medium) \ [ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \ .speed = SPEED_ ## _speed, \ + .min_lanes = _min_lanes, \ + .lanes = _lanes, \ + .duplex = __DUPLEX_ ## _duplex, \ + .medium = ETHTOOL_LINK_MEDIUM_BASE ## _medium \ + } + +#define __DEFINE_LINK_MODE_PARAMS(_speed, _type, _duplex, _medium) \ + [ETHTOOL_LINK_MODE(_speed, _type, _duplex)] = { \ + .speed = SPEED_ ## _speed, \ + .min_lanes = __LINK_MODE_LANES_ ## _type, \ .lanes = __LINK_MODE_LANES_ ## _type, \ - .duplex = __DUPLEX_ ## _duplex \ + .duplex = __DUPLEX_ ## _duplex, \ + .medium = ETHTOOL_LINK_MEDIUM_BASE ## _medium \ } #define __DUPLEX_Half DUPLEX_HALF #define __DUPLEX_Full DUPLEX_FULL @@ -268,116 +279,117 @@ static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS); .speed = SPEED_UNKNOWN, \ .lanes = 0, \ .duplex = DUPLEX_UNKNOWN, \ + .medium = ETHTOOL_LINK_MEDIUM_NONE, \ } const struct link_mode_info link_mode_params[] = { - __DEFINE_LINK_MODE_PARAMS(10, T, Half), - __DEFINE_LINK_MODE_PARAMS(10, T, Full), - __DEFINE_LINK_MODE_PARAMS(100, T, Half), - __DEFINE_LINK_MODE_PARAMS(100, T, Full), - __DEFINE_LINK_MODE_PARAMS(1000, T, Half), - __DEFINE_LINK_MODE_PARAMS(1000, T, Full), + __DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Half, T), + __DEFINE_LINK_MODE_PARAMS_LANES(10, T, 2, 4, Full, T), + __DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Half, T), + __DEFINE_LINK_MODE_PARAMS_LANES(100, T, 2, 4, Full, T), + __DEFINE_LINK_MODE_PARAMS(1000, T, Half, T), + __DEFINE_LINK_MODE_PARAMS(1000, T, Full, T), __DEFINE_SPECIAL_MODE_PARAMS(Autoneg), __DEFINE_SPECIAL_MODE_PARAMS(TP), __DEFINE_SPECIAL_MODE_PARAMS(AUI), __DEFINE_SPECIAL_MODE_PARAMS(MII), __DEFINE_SPECIAL_MODE_PARAMS(FIBRE), __DEFINE_SPECIAL_MODE_PARAMS(BNC), - __DEFINE_LINK_MODE_PARAMS(10000, T, Full), + __DEFINE_LINK_MODE_PARAMS(10000, T, Full, T), __DEFINE_SPECIAL_MODE_PARAMS(Pause), __DEFINE_SPECIAL_MODE_PARAMS(Asym_Pause), - __DEFINE_LINK_MODE_PARAMS(2500, X, Full), + __DEFINE_LINK_MODE_PARAMS(2500, X, Full, X), __DEFINE_SPECIAL_MODE_PARAMS(Backplane), - __DEFINE_LINK_MODE_PARAMS(1000, KX, Full), - __DEFINE_LINK_MODE_PARAMS(10000, KX4, Full), - __DEFINE_LINK_MODE_PARAMS(10000, KR, Full), + __DEFINE_LINK_MODE_PARAMS(1000, KX, Full, K), + __DEFINE_LINK_MODE_PARAMS(10000, KX4, Full, K), + __DEFINE_LINK_MODE_PARAMS(10000, KR, Full, K), [ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] = { .speed = SPEED_10000, .lanes = 1, .duplex = DUPLEX_FULL, }, - __DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full), - __DEFINE_LINK_MODE_PARAMS(20000, KR2, Full), - __DEFINE_LINK_MODE_PARAMS(40000, KR4, Full), - __DEFINE_LINK_MODE_PARAMS(40000, CR4, Full), - __DEFINE_LINK_MODE_PARAMS(40000, SR4, Full), - __DEFINE_LINK_MODE_PARAMS(40000, LR4, Full), - __DEFINE_LINK_MODE_PARAMS(56000, KR4, Full), - __DEFINE_LINK_MODE_PARAMS(56000, CR4, Full), - __DEFINE_LINK_MODE_PARAMS(56000, SR4, Full), - __DEFINE_LINK_MODE_PARAMS(56000, LR4, Full), - __DEFINE_LINK_MODE_PARAMS(25000, CR, Full), - __DEFINE_LINK_MODE_PARAMS(25000, KR, Full), - __DEFINE_LINK_MODE_PARAMS(25000, SR, Full), - __DEFINE_LINK_MODE_PARAMS(50000, CR2, Full), - __DEFINE_LINK_MODE_PARAMS(50000, KR2, Full), - __DEFINE_LINK_MODE_PARAMS(100000, KR4, Full), - __DEFINE_LINK_MODE_PARAMS(100000, SR4, Full), - __DEFINE_LINK_MODE_PARAMS(100000, CR4, Full), - __DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, Full), - __DEFINE_LINK_MODE_PARAMS(50000, SR2, Full), - __DEFINE_LINK_MODE_PARAMS(1000, X, Full), - __DEFINE_LINK_MODE_PARAMS(10000, CR, Full), - __DEFINE_LINK_MODE_PARAMS(10000, SR, Full), - __DEFINE_LINK_MODE_PARAMS(10000, LR, Full), - __DEFINE_LINK_MODE_PARAMS(10000, LRM, Full), - __DEFINE_LINK_MODE_PARAMS(10000, ER, Full), - __DEFINE_LINK_MODE_PARAMS(2500, T, Full), - __DEFINE_LINK_MODE_PARAMS(5000, T, Full), + __DEFINE_LINK_MODE_PARAMS(20000, MLD2, Full, MLD), + __DEFINE_LINK_MODE_PARAMS(20000, KR2, Full, K), + __DEFINE_LINK_MODE_PARAMS(40000, KR4, Full, K), + __DEFINE_LINK_MODE_PARAMS(40000, CR4, Full, C), + __DEFINE_LINK_MODE_PARAMS(40000, SR4, Full, S), + __DEFINE_LINK_MODE_PARAMS(40000, LR4, Full, L), + __DEFINE_LINK_MODE_PARAMS(56000, KR4, Full, K), + __DEFINE_LINK_MODE_PARAMS(56000, CR4, Full, C), + __DEFINE_LINK_MODE_PARAMS(56000, SR4, Full, S), + __DEFINE_LINK_MODE_PARAMS(56000, LR4, Full, L), + __DEFINE_LINK_MODE_PARAMS(25000, CR, Full, C), + __DEFINE_LINK_MODE_PARAMS(25000, KR, Full, K), + __DEFINE_LINK_MODE_PARAMS(25000, SR, Full, S), + __DEFINE_LINK_MODE_PARAMS(50000, CR2, Full, C), + __DEFINE_LINK_MODE_PARAMS(50000, KR2, Full, K), + __DEFINE_LINK_MODE_PARAMS(100000, KR4, Full, K), + __DEFINE_LINK_MODE_PARAMS(100000, SR4, Full, S), + __DEFINE_LINK_MODE_PARAMS(100000, CR4, Full, C), + __DEFINE_LINK_MODE_PARAMS(100000, LR4_ER4, Full, L), + __DEFINE_LINK_MODE_PARAMS(50000, SR2, Full, S), + __DEFINE_LINK_MODE_PARAMS(1000, X, Full, X), /* :( */ + __DEFINE_LINK_MODE_PARAMS(10000, CR, Full, C), + __DEFINE_LINK_MODE_PARAMS(10000, SR, Full, S), + __DEFINE_LINK_MODE_PARAMS(10000, LR, Full, L), + __DEFINE_LINK_MODE_PARAMS(10000, LRM, Full, L), + __DEFINE_LINK_MODE_PARAMS(10000, ER, Full, E), + __DEFINE_LINK_MODE_PARAMS(2500, T, Full, T), + __DEFINE_LINK_MODE_PARAMS(5000, T, Full, T), __DEFINE_SPECIAL_MODE_PARAMS(FEC_NONE), __DEFINE_SPECIAL_MODE_PARAMS(FEC_RS), __DEFINE_SPECIAL_MODE_PARAMS(FEC_BASER), - __DEFINE_LINK_MODE_PARAMS(50000, KR, Full), - __DEFINE_LINK_MODE_PARAMS(50000, SR, Full), - __DEFINE_LINK_MODE_PARAMS(50000, CR, Full), - __DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, Full), - __DEFINE_LINK_MODE_PARAMS(50000, DR, Full), - __DEFINE_LINK_MODE_PARAMS(100000, KR2, Full), - __DEFINE_LINK_MODE_PARAMS(100000, SR2, Full), - __DEFINE_LINK_MODE_PARAMS(100000, CR2, Full), - __DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, Full), - __DEFINE_LINK_MODE_PARAMS(100000, DR2, Full), - __DEFINE_LINK_MODE_PARAMS(200000, KR4, Full), - __DEFINE_LINK_MODE_PARAMS(200000, SR4, Full), - __DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, Full), - __DEFINE_LINK_MODE_PARAMS(200000, DR4, Full), - __DEFINE_LINK_MODE_PARAMS(200000, CR4, Full), - __DEFINE_LINK_MODE_PARAMS(100, T1, Full), - __DEFINE_LINK_MODE_PARAMS(1000, T1, Full), - __DEFINE_LINK_MODE_PARAMS(400000, KR8, Full), - __DEFINE_LINK_MODE_PARAMS(400000, SR8, Full), - __DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full), - __DEFINE_LINK_MODE_PARAMS(400000, DR8, Full), - __DEFINE_LINK_MODE_PARAMS(400000, CR8, Full), + __DEFINE_LINK_MODE_PARAMS(50000, KR, Full, K), + __DEFINE_LINK_MODE_PARAMS(50000, SR, Full, S), + __DEFINE_LINK_MODE_PARAMS(50000, CR, Full, C), + __DEFINE_LINK_MODE_PARAMS(50000, LR_ER_FR, Full, L), /* I guess ? */ + __DEFINE_LINK_MODE_PARAMS(50000, DR, Full, D), + __DEFINE_LINK_MODE_PARAMS(100000, KR2, Full, K), + __DEFINE_LINK_MODE_PARAMS(100000, SR2, Full, S), + __DEFINE_LINK_MODE_PARAMS(100000, CR2, Full, C), + __DEFINE_LINK_MODE_PARAMS(100000, LR2_ER2_FR2, Full, L), + __DEFINE_LINK_MODE_PARAMS(100000, DR2, Full, D), + __DEFINE_LINK_MODE_PARAMS(200000, KR4, Full, K), + __DEFINE_LINK_MODE_PARAMS(200000, SR4, Full, S), + __DEFINE_LINK_MODE_PARAMS(200000, LR4_ER4_FR4, Full, L), + __DEFINE_LINK_MODE_PARAMS(200000, DR4, Full, D), + __DEFINE_LINK_MODE_PARAMS(200000, CR4, Full, C), + __DEFINE_LINK_MODE_PARAMS(100, T1, Full, T), + __DEFINE_LINK_MODE_PARAMS(1000, T1, Full, T), + __DEFINE_LINK_MODE_PARAMS(400000, KR8, Full, K), + __DEFINE_LINK_MODE_PARAMS(400000, SR8, Full, S), + __DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full, L), + __DEFINE_LINK_MODE_PARAMS(400000, DR8, Full, D), + __DEFINE_LINK_MODE_PARAMS(400000, CR8, Full, C), __DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS), - __DEFINE_LINK_MODE_PARAMS(100000, KR, Full), - __DEFINE_LINK_MODE_PARAMS(100000, SR, Full), - __DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, Full), - __DEFINE_LINK_MODE_PARAMS(100000, DR, Full), - __DEFINE_LINK_MODE_PARAMS(100000, CR, Full), - __DEFINE_LINK_MODE_PARAMS(200000, KR2, Full), - __DEFINE_LINK_MODE_PARAMS(200000, SR2, Full), - __DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, Full), - __DEFINE_LINK_MODE_PARAMS(200000, DR2, Full), - __DEFINE_LINK_MODE_PARAMS(200000, CR2, Full), - __DEFINE_LINK_MODE_PARAMS(400000, KR4, Full), - __DEFINE_LINK_MODE_PARAMS(400000, SR4, Full), - __DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, Full), - __DEFINE_LINK_MODE_PARAMS(400000, DR4, Full), - __DEFINE_LINK_MODE_PARAMS(400000, CR4, Full), - __DEFINE_LINK_MODE_PARAMS(100, FX, Half), - __DEFINE_LINK_MODE_PARAMS(100, FX, Full), - __DEFINE_LINK_MODE_PARAMS(10, T1L, Full), - __DEFINE_LINK_MODE_PARAMS(800000, CR8, Full), - __DEFINE_LINK_MODE_PARAMS(800000, KR8, Full), - __DEFINE_LINK_MODE_PARAMS(800000, DR8, Full), - __DEFINE_LINK_MODE_PARAMS(800000, DR8_2, Full), - __DEFINE_LINK_MODE_PARAMS(800000, SR8, Full), - __DEFINE_LINK_MODE_PARAMS(800000, VR8, Full), - __DEFINE_LINK_MODE_PARAMS(10, T1S, Full), - __DEFINE_LINK_MODE_PARAMS(10, T1S, Half), - __DEFINE_LINK_MODE_PARAMS(10, T1S_P2MP, Half), - __DEFINE_LINK_MODE_PARAMS(10, T1BRR, Full), + __DEFINE_LINK_MODE_PARAMS(100000, KR, Full, K), + __DEFINE_LINK_MODE_PARAMS(100000, SR, Full, S), + __DEFINE_LINK_MODE_PARAMS(100000, LR_ER_FR, Full, L), + __DEFINE_LINK_MODE_PARAMS(100000, DR, Full, D), + __DEFINE_LINK_MODE_PARAMS(100000, CR, Full, C), + __DEFINE_LINK_MODE_PARAMS(200000, KR2, Full, K), + __DEFINE_LINK_MODE_PARAMS(200000, SR2, Full, S), + __DEFINE_LINK_MODE_PARAMS(200000, LR2_ER2_FR2, Full, L), + __DEFINE_LINK_MODE_PARAMS(200000, DR2, Full, D), + __DEFINE_LINK_MODE_PARAMS(200000, CR2, Full, C), + __DEFINE_LINK_MODE_PARAMS(400000, KR4, Full, K), + __DEFINE_LINK_MODE_PARAMS(400000, SR4, Full, S), + __DEFINE_LINK_MODE_PARAMS(400000, LR4_ER4_FR4, Full, L), + __DEFINE_LINK_MODE_PARAMS(400000, DR4, Full, D), + __DEFINE_LINK_MODE_PARAMS(400000, CR4, Full, C), + __DEFINE_LINK_MODE_PARAMS(100, FX, Half, F), + __DEFINE_LINK_MODE_PARAMS(100, FX, Full, F), + __DEFINE_LINK_MODE_PARAMS(10, T1L, Full, T), + __DEFINE_LINK_MODE_PARAMS(800000, CR8, Full, C), + __DEFINE_LINK_MODE_PARAMS(800000, KR8, Full, K), + __DEFINE_LINK_MODE_PARAMS(800000, DR8, Full, D), + __DEFINE_LINK_MODE_PARAMS(800000, DR8_2, Full, D), + __DEFINE_LINK_MODE_PARAMS(800000, SR8, Full, S), + __DEFINE_LINK_MODE_PARAMS(800000, VR8, Full, V), + __DEFINE_LINK_MODE_PARAMS(10, T1S, Full, T), + __DEFINE_LINK_MODE_PARAMS(10, T1S, Half, T), + __DEFINE_LINK_MODE_PARAMS(10, T1S_P2MP, Half, T), + __DEFINE_LINK_MODE_PARAMS(10, T1BRR, Full, T), }; static_assert(ARRAY_SIZE(link_mode_params) == __ETHTOOL_LINK_MODE_MASK_NBITS); diff --git a/net/ethtool/common.h b/net/ethtool/common.h index 850eadde4bfc..b25011a05fea 100644 --- a/net/ethtool/common.h +++ b/net/ethtool/common.h @@ -16,8 +16,10 @@ struct link_mode_info { int speed; + u8 min_lanes; u8 lanes; u8 duplex; + enum ethtool_link_medium medium; }; struct genl_info; -- 2.47.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH net-next RFC 3/5] net: ethtool: Export the link_mode_params definitions 2024-12-20 20:14 [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 1/5] net: ethtool: common: Make BaseT a 4-lanes mode Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 2/5] net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values Maxime Chevallier @ 2024-12-20 20:15 ` Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 4/5] net: phy: Introduce PHY ports representation Maxime Chevallier ` (2 subsequent siblings) 5 siblings, 0 replies; 26+ messages in thread From: Maxime Chevallier @ 2024-12-20 20:15 UTC (permalink / raw) To: davem Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Köry Maincent, Marek Behún, Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart link_mode_params contains a lookup table of all 802.3 link modes that are currently supported with structured data about each mode's speed, duplex, number of lanes and mediums. As a preparation for a port representation, export that table for the rest of the net stack to use. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- include/linux/ethtool.h | 10 ++++++++++ net/ethtool/common.c | 1 + net/ethtool/common.h | 9 --------- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index dc02a1bef549..51d110dfa8ef 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -251,6 +251,16 @@ static inline const char *phy_mediums(enum ethtool_link_medium medium) } } +struct link_mode_info { + int speed; + u8 min_lanes; + u8 lanes; + u8 duplex; + enum ethtool_link_medium medium; +}; + +extern const struct link_mode_info link_mode_params[]; + /* declare a link mode bitmap */ #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name) \ DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS) diff --git a/net/ethtool/common.c b/net/ethtool/common.c index 41e7598f6ed7..bded82c0bba4 100644 --- a/net/ethtool/common.c +++ b/net/ethtool/common.c @@ -392,6 +392,7 @@ const struct link_mode_info link_mode_params[] = { __DEFINE_LINK_MODE_PARAMS(10, T1BRR, Full, T), }; static_assert(ARRAY_SIZE(link_mode_params) == __ETHTOOL_LINK_MODE_MASK_NBITS); +EXPORT_SYMBOL_GPL(link_mode_params); const char netif_msg_class_names[][ETH_GSTRING_LEN] = { [NETIF_MSG_DRV_BIT] = "drv", diff --git a/net/ethtool/common.h b/net/ethtool/common.h index b25011a05fea..ee250a5c5364 100644 --- a/net/ethtool/common.h +++ b/net/ethtool/common.h @@ -14,14 +14,6 @@ #define __SOF_TIMESTAMPING_CNT (const_ilog2(SOF_TIMESTAMPING_LAST) + 1) -struct link_mode_info { - int speed; - u8 min_lanes; - u8 lanes; - u8 duplex; - enum ethtool_link_medium medium; -}; - struct genl_info; struct hwtstamp_provider_desc; @@ -34,7 +26,6 @@ tunable_strings[__ETHTOOL_TUNABLE_COUNT][ETH_GSTRING_LEN]; extern const char phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN]; extern const char link_mode_names[][ETH_GSTRING_LEN]; -extern const struct link_mode_info link_mode_params[]; extern const char netif_msg_class_names[][ETH_GSTRING_LEN]; extern const char wol_mode_names[][ETH_GSTRING_LEN]; extern const char sof_timestamping_names[][ETH_GSTRING_LEN]; -- 2.47.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH net-next RFC 4/5] net: phy: Introduce PHY ports representation 2024-12-20 20:14 [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Maxime Chevallier ` (2 preceding siblings ...) 2024-12-20 20:15 ` [PATCH net-next RFC 3/5] net: ethtool: Export the link_mode_params definitions Maxime Chevallier @ 2024-12-20 20:15 ` Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 5/5] net: phy: dp83822: Add support for phy_port representation Maxime Chevallier 2024-12-22 15:59 ` [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Oleksij Rempel 5 siblings, 0 replies; 26+ messages in thread From: Maxime Chevallier @ 2024-12-20 20:15 UTC (permalink / raw) To: davem Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Köry Maincent, Marek Behún, Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart Ethernet provides a wide variety of layer 1 protocols and standards for data transmission. The front-facing ports of an interface have their own complexity and configurability. Introduce a representation of these front-facing ports. The current code is minimalistic and only support ports controlled by PHY devices, but the plan is to extend that to SFP as well as raw Ethernet MACs that don't use PHY devices. This minimal port representation allows describing the media and number of lanes of a port. From that information, we can derive the linkmodes usable on the port, which can be used to limit the capabilities of an interface. For now, the port lanes and medium is derived from devicetree, defined by the PHY driver, or populated with default values (as we assume that all PHYs expose at least one port). The typical example is 100M ethernet. 100BaseT can work using only 2 lanes on a Cat 5 cables. However, in the situation where a 10/100/1000 capable PHY is wired to its RJ45 port through 2 lanes only, we have no way of detecting that. The "max-speed" DT property can be used, but a more accurate representation can be used : mdi { port@0 { media = "BaseT"; lanes = <2>; }; }; From that information, we can derive the max speed reachable on the port. Another benefit of having that is to avoid vendor-specific DT properties (micrel,fiber-mode or ti,fiber-mode). This basic representation is meant to be expanded, by the introduction of port ops, userspace listing of ports, and support for multi-port devices. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- drivers/net/phy/Makefile | 2 +- drivers/net/phy/phy_device.c | 167 +++++++++++++++++++++++++++++++++++ drivers/net/phy/phy_port.c | 159 +++++++++++++++++++++++++++++++++ include/linux/ethtool.h | 4 + include/linux/phy.h | 31 +++++++ include/linux/phy_port.h | 69 +++++++++++++++ 6 files changed, 431 insertions(+), 1 deletion(-) create mode 100644 drivers/net/phy/phy_port.c create mode 100644 include/linux/phy_port.h diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index e6145153e837..1e324d3ad839 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -2,7 +2,7 @@ # Makefile for Linux PHY drivers libphy-y := phy.o phy-c45.o phy-core.o phy_device.o \ - linkmode.o phy_link_topology.o + linkmode.o phy_link_topology.o phy_port.o mdio-bus-y += mdio_bus.o mdio_device.o ifdef CONFIG_MDIO_DEVICE diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 928dc3c509b6..42cce18970e2 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -706,6 +706,13 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id, dev->state = PHY_DOWN; INIT_LIST_HEAD(&dev->leds); + INIT_LIST_HEAD(&dev->ports); + + /* The driver's probe function must change that to the real number + * of ports possible on the PHY. We assume by default we are dealing + * with a single-port PHY + */ + dev->max_n_ports = 1; mutex_init(&dev->lock); INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine); @@ -3454,6 +3461,159 @@ static int of_phy_leds(struct phy_device *phydev) return 0; } +static int phy_add_port(struct phy_device *phydev, struct phy_port *port) +{ + int ret = 0; + + if (phydev->n_ports == phydev->max_n_ports) + return -EBUSY; + + /* We set all ports as active by default, PHY drivers may deactivate + * them (when unused) + */ + port->active = true; + + if (phydev->drv && phydev->drv->attach_port) + ret = phydev->drv->attach_port(phydev, port); + + if (ret) + return ret; + + /* The PHY driver might have added, removed or set medium/lanes info, + * so update the port supported accordingly. + */ + phy_port_update_supported(port); + + list_add(&port->head, &phydev->ports); + + phydev->n_ports++; + + return 0; +} + +static void phy_del_port(struct phy_device *phydev, struct phy_port *port) +{ + if (!phydev->n_ports) + return; + + list_del(&port->head); + + phydev->n_ports--; +} + +static void phy_cleanup_ports(struct phy_device *phydev) +{ + struct phy_port *tmp, *port; + + list_for_each_entry_safe(port, tmp, &phydev->ports, head) { + phy_del_port(phydev, port); + phy_port_destroy(port); + } +} + +static int phy_default_setup_single_port(struct phy_device *phydev) +{ + struct phy_port *port = phy_port_alloc(); + + if (!port) + return -ENOMEM; + + port->parent_type = PHY_PORT_PHY; + port->phy = phydev; + linkmode_copy(port->supported, phydev->supported); + + /* default medium is copper */ + if (!port->mediums) + port->mediums |= BIT(ETHTOOL_LINK_MEDIUM_BASET); + + phy_add_port(phydev, port); + + return 0; +} + +static int of_phy_ports(struct phy_device *phydev) +{ + struct device_node *node = phydev->mdio.dev.of_node; + struct device_node *mdi; + struct phy_port *port; + int err; + + if (!IS_ENABLED(CONFIG_OF_MDIO)) + return 0; + + if (!node) + return 0; + + mdi = of_get_child_by_name(node, "mdi"); + if (!mdi) + return 0; + + for_each_available_child_of_node_scoped(mdi, port_node) { + port = phy_of_parse_port(port_node); + if (IS_ERR(port)) { + err = PTR_ERR(port); + goto out_err; + } + + port->parent_type = PHY_PORT_PHY; + port->phy = phydev; + err = phy_add_port(phydev, port); + if (err) + goto out_err; + } + of_node_put(mdi); + + return 0; + +out_err: + phy_cleanup_ports(phydev); + of_node_put(mdi); + return err; +} + +static int phy_setup_ports(struct phy_device *phydev) +{ + __ETHTOOL_DECLARE_LINK_MODE_MASK(ports_supported); + struct phy_port *port; + int ret; + + ret = of_phy_ports(phydev); + if (ret) + return ret; + + if (phydev->n_ports < phydev->max_n_ports) { + ret = phy_default_setup_single_port(phydev); + if (ret) + goto out; + } + + linkmode_zero(ports_supported); + + /* Aggregate the supported modes, which are made-up of : + * - What the PHY itself supports + * - What the sum of all ports support + */ + list_for_each_entry(port, &phydev->ports, head) + if (port->active) + linkmode_or(ports_supported, ports_supported, + port->supported); + + if (!linkmode_empty(ports_supported)) + linkmode_and(phydev->supported, phydev->supported, + ports_supported); + + /* For now, the phy->port field is set as the first active port's type */ + list_for_each_entry(port, &phydev->ports, head) + if (port->active) + phydev->port = phy_port_get_type(port); + + return 0; + +out: + phy_cleanup_ports(phydev); + return ret; +} + /** * fwnode_mdio_find_device - Given a fwnode, find the mdio_device * @fwnode: pointer to the mdio_device's fwnode @@ -3603,6 +3763,11 @@ static int phy_probe(struct device *dev) phydev->is_gigabit_capable = 1; of_set_phy_supported(phydev); + + err = phy_setup_ports(phydev); + if (err) + goto out; + phy_advertise_supported(phydev); /* Get PHY default EEE advertising modes and handle them as potentially @@ -3679,6 +3844,8 @@ static int phy_remove(struct device *dev) phydev->state = PHY_DOWN; + phy_cleanup_ports(phydev); + sfp_bus_del_upstream(phydev->sfp_bus); phydev->sfp_bus = NULL; diff --git a/drivers/net/phy/phy_port.c b/drivers/net/phy/phy_port.c new file mode 100644 index 000000000000..d89c572623d7 --- /dev/null +++ b/drivers/net/phy/phy_port.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Framework to drive Ethernet ports + * + * Copyright (c) 2024 Maxime Chevallier <maxime.chevallier@bootlin.com> + */ + +#include <linux/linkmode.h> +#include <linux/of.h> +#include <linux/phy_port.h> + +/** + * phy_port_alloc: Allocate a new phy_port + * + * Returns a newly allocated struct phy_port, or NULL. + */ +struct phy_port *phy_port_alloc(void) +{ + struct phy_port *port; + + port = kzalloc(sizeof(*port), GFP_KERNEL); + if (!port) + return NULL; + + linkmode_zero(port->supported); + INIT_LIST_HEAD(&port->head); + + return port; +} +EXPORT_SYMBOL_GPL(phy_port_alloc); + +/** + * phy_port_destroy: Free a struct phy_port + */ +void phy_port_destroy(struct phy_port *port) +{ + kfree(port); +} +EXPORT_SYMBOL_GPL(phy_port_destroy); + +static void ethtool_medium_get_supported(unsigned long *supported, + enum ethtool_link_medium medium, + int lanes) +{ + int i; + + for (i = 0; i < __ETHTOOL_LINK_MODE_MASK_NBITS; i++) { + /* Special bits such as Autoneg, Pause, Asym_pause, etc. are + * set and will be masked away by the port parent. + */ + if (link_mode_params[i].medium == ETHTOOL_LINK_MEDIUM_NONE) { + linkmode_set_bit(i, supported); + continue; + } + + /* For most cases, min_lanes == lanes, except for 10/100BaseT that work + * on 2 lanes but are compatible with 4 lanes mediums + */ + if (link_mode_params[i].medium == medium && + link_mode_params[i].lanes >= lanes && + link_mode_params[i].min_lanes <= lanes) { + linkmode_set_bit(i, supported); + } + } +} + +static enum ethtool_link_medium ethtool_str_to_medium(const char *str) +{ + int i; + + for (i = 0; i < __ETHTOOL_LINK_MEDIUM_LAST; i++) + if (!strcmp(phy_mediums(i), str)) + return i; + + return ETHTOOL_LINK_MEDIUM_NONE; +} + +/** + * phy_of_parse_port: Create a phy_port from a firmware representation + * + * Returns a newly allocated and initialized phy_port pointer, or an ERR_PTR. + */ +struct phy_port *phy_of_parse_port(struct device_node *dn) +{ + struct fwnode_handle *fwnode = of_fwnode_handle(dn); + enum ethtool_link_medium medium; + struct phy_port *port; + struct property *prop; + const char *med_str; + u32 lanes, mediums = 0; + int ret; + + ret = fwnode_property_read_u32(fwnode, "lanes", &lanes); + if (ret) + return ERR_PTR(ret); + + ret = fwnode_property_read_string(fwnode, "media", &med_str); + if (ret) + return ERR_PTR(ret); + + of_property_for_each_string(to_of_node(fwnode), "media", prop, med_str) { + medium = ethtool_str_to_medium(med_str); + if (medium == ETHTOOL_LINK_MEDIUM_NONE) + return ERR_PTR(-EINVAL); + + mediums |= BIT(medium); + } + + if (!mediums) + return ERR_PTR(-EINVAL); + + port = phy_port_alloc(); + if (!port) + return ERR_PTR(-ENOMEM); + + port->lanes = lanes; + port->mediums = mediums; + + return port; +} +EXPORT_SYMBOL_GPL(phy_of_parse_port); + +/** + * phy_port_update_supported: Setup the port->supported field + * port: the port to update + * + * Once the port's medium list and number of lanes has been configured based + * on firmware, straps and vendor-specific properties, this function may be + * called to update the port's supported linkmodes list. + * + * Any mode that was manually set in the port's supported list remains set. + */ +void phy_port_update_supported(struct phy_port *port) +{ + __ETHTOOL_DECLARE_LINK_MODE_MASK(supported); + int i; + + for_each_set_bit(i, &port->mediums, __ETHTOOL_LINK_MEDIUM_LAST) { + linkmode_zero(supported); + ethtool_medium_get_supported(supported, i, port->lanes); + linkmode_or(port->supported, port->supported, supported); + } +} +EXPORT_SYMBOL_GPL(phy_port_update_supported); + +/** + * phy_port_get_type: get the PORT_* attribut for that port. + */ +int phy_port_get_type(struct phy_port *port) +{ + if (port->mediums & ETHTOOL_LINK_MEDIUM_BASET) + return PORT_TP; + + if (phy_port_is_fiber(port) || + (port->mediums & BIT(ETHTOOL_LINK_MEDIUM_BASEX))) + return PORT_FIBRE; + + return PORT_OTHER; +} +EXPORT_SYMBOL_GPL(phy_port_get_type); diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 51d110dfa8ef..fdb47cfd8c79 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -220,6 +220,10 @@ enum ethtool_link_medium { __ETHTOOL_LINK_MEDIUM_LAST, }; +#define ETHTOOL_MEDIUM_FIBER_BITS (BIT(ETHTOOL_LINK_MEDIUM_BASES) | \ + BIT(ETHTOOL_LINK_MEDIUM_BASEL) | \ + BIT(ETHTOOL_LINK_MEDIUM_BASEF)) + static inline const char *phy_mediums(enum ethtool_link_medium medium) { switch (medium) { diff --git a/include/linux/phy.h b/include/linux/phy.h index 5bc71d59910c..14ba3eca7382 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -21,6 +21,7 @@ #include <linux/mii.h> #include <linux/mii_timestamper.h> #include <linux/module.h> +#include <linux/phy_port.h> #include <linux/timer.h> #include <linux/workqueue.h> #include <linux/mod_devicetable.h> @@ -660,6 +661,9 @@ struct macsec_ops; * @master_slave_state: Current master/slave configuration * @mii_ts: Pointer to time stamper callbacks * @psec: Pointer to Power Sourcing Equipment control struct + * @ports: List of PHY ports structures + * n_ports: Number of ports currently attached to the PHY + * @max_n_ports: Max number of ports this PHY can expose * @lock: Mutex for serialization access to PHY * @state_queue: Work queue for state machine * @link_down_events: Number of times link was lost @@ -752,6 +756,7 @@ struct phy_device { /* Host supported PHY interface types. Should be ignored if empty. */ DECLARE_PHY_INTERFACE_MASK(host_interfaces); + DECLARE_PHY_INTERFACE_MASK(sfp_bus_interfaces); #ifdef CONFIG_LED_TRIGGER_PHY struct phy_led_trigger *phy_led_triggers; @@ -794,6 +799,10 @@ struct phy_device { struct mii_timestamper *mii_ts; struct pse_control *psec; + struct list_head ports; + int n_ports; + int max_n_ports; + u8 mdix; u8 mdix_ctrl; @@ -1244,6 +1253,27 @@ struct phy_driver { */ int (*led_polarity_set)(struct phy_device *dev, int index, unsigned long modes); + + /** + * @attach_port: Indicates to the PHY driver that a port is detected + * @dev: PHY device to notify + * @port: The port being added + * + * Called when a port that needs to be driven by the PHY is found. The + * number of time this will be called depends on phydev->max_n_ports, + * which the driver can change in .probe(). + * + * The port that is being passed may or may not be initialized. If it is + * already initialized, it is by the generic port representation from + * devicetree, which superseeds any strapping or vendor-specific + * properties. + * + * If the port isn't initialized, the port->mediums and port->lanes + * fields must be set, possibly according to stapping information. + * + * Returns 0, or an error code. + */ + int (*attach_port)(struct phy_device *dev, struct phy_port *port); }; #define to_phy_driver(d) container_of_const(to_mdio_common_driver(d), \ struct phy_driver, mdiodrv) @@ -2051,6 +2081,7 @@ void phy_trigger_machine(struct phy_device *phydev); void phy_mac_interrupt(struct phy_device *phydev); void phy_start_machine(struct phy_device *phydev); void phy_stop_machine(struct phy_device *phydev); + void phy_ethtool_ksettings_get(struct phy_device *phydev, struct ethtool_link_ksettings *cmd); int phy_ethtool_ksettings_set(struct phy_device *phydev, diff --git a/include/linux/phy_port.h b/include/linux/phy_port.h new file mode 100644 index 000000000000..b34c15523adc --- /dev/null +++ b/include/linux/phy_port.h @@ -0,0 +1,69 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <linux/ethtool.h> +#include <linux/types.h> + +#ifndef __PHY_PORT_H +#define __PHY_PORT_H + +struct phy_port; + +/** + * enum phy_port_parent - The device this port is attached to + * + * @PHY_PORT_PHY: Indicates that the port is driven by a PHY device + */ +enum phy_port_parent { + PHY_PORT_PHY, +}; + +/** + * struct phy_port - A representation of a network device physical interface + * + * @head: Used by the port's parent to list ports + * @parent_type: The type of device this port is directly connected to + * @phy: If the parent is PHY_PORT_PHYDEV, the PHY controlling that port + * @lanes: The number of lanes (diff pairs) this port has, 0 if not applicable + * @medium: The physical medium this port provides access to + * @supported: The link modes this port can expose + * @active: Indicates if the port is currently part of the active link. + */ +struct phy_port { + struct list_head head; + enum phy_port_parent parent_type; + union { + struct phy_device *phy; + }; + + int lanes; + unsigned long mediums; + __ETHTOOL_DECLARE_LINK_MODE_MASK(supported); + + bool active; +}; + +struct phy_port *phy_port_alloc(void); +void phy_port_destroy(struct phy_port *port); + +static inline struct phy_device *port_phydev(struct phy_port *port) +{ + return port->phy; +} + +struct phy_port *phy_of_parse_port(struct device_node *dn); + +static inline bool phy_port_is_copper(struct phy_port *port) +{ + return port->mediums == BIT(ETHTOOL_LINK_MEDIUM_BASET); +} + +static inline bool phy_port_is_fiber(struct phy_port *port) +{ + return !!(port->mediums & ETHTOOL_MEDIUM_FIBER_BITS); +} + +void phy_port_update_supported(struct phy_port *port); + +int phy_port_get_type(struct phy_port *port); + +#endif -- 2.47.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH net-next RFC 5/5] net: phy: dp83822: Add support for phy_port representation 2024-12-20 20:14 [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Maxime Chevallier ` (3 preceding siblings ...) 2024-12-20 20:15 ` [PATCH net-next RFC 4/5] net: phy: Introduce PHY ports representation Maxime Chevallier @ 2024-12-20 20:15 ` Maxime Chevallier 2024-12-22 15:59 ` [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Oleksij Rempel 5 siblings, 0 replies; 26+ messages in thread From: Maxime Chevallier @ 2024-12-20 20:15 UTC (permalink / raw) To: davem Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Köry Maincent, Marek Behún, Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart With the phy_port representation intrduced, we can use .attach_port to populate the port information based on either the straps or the ti,fiber-mode property. This allows simplifying the probe function and allow users to override the strapping configuration. Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- drivers/net/phy/dp83822.c | 60 +++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c index 334c17a68edd..96a0ad1a0e83 100644 --- a/drivers/net/phy/dp83822.c +++ b/drivers/net/phy/dp83822.c @@ -637,17 +637,6 @@ static int dp83822_of_init(struct phy_device *phydev) struct device *dev = &phydev->mdio.dev; const char *of_val; - /* Signal detection for the PHY is only enabled if the FX_EN and the - * SD_EN pins are strapped. Signal detection can only enabled if FX_EN - * is strapped otherwise signal detection is disabled for the PHY. - */ - if (dp83822->fx_enabled && dp83822->fx_sd_enable) - dp83822->fx_signal_det_low = device_property_present(dev, - "ti,link-loss-low"); - if (!dp83822->fx_enabled) - dp83822->fx_enabled = device_property_present(dev, - "ti,fiber-mode"); - if (!device_property_read_string(dev, "ti,gpio2-clk-out", &of_val)) { if (strcmp(of_val, "mac-if") == 0) { dp83822->gpio2_clk_out = DP83822_CLK_SRC_MAC_IF; @@ -740,6 +729,44 @@ static int dp83822_read_straps(struct phy_device *phydev) return 0; } +static int dp83822_attach_port(struct phy_device *phydev, struct phy_port *port) +{ + struct dp83822_private *dp83822 = phydev->priv; + struct device *dev = &phydev->mdio.dev; + int ret; + + if (port->mediums) { + if (phy_port_is_fiber(port) || + port->mediums & BIT(ETHTOOL_LINK_MEDIUM_BASEX)) + dp83822->fx_enabled = true; + } else { + ret = dp83822_read_straps(phydev); + if (ret) + return ret; + +#ifdef CONFIG_OF_MDIO + if (dp83822->fx_enabled && dp83822->fx_sd_enable) + dp83822->fx_signal_det_low = + device_property_present(dev, "ti,link-loss-low"); + if (!dp83822->fx_enabled) + dp83822->fx_enabled = + device_property_present(dev, "ti,fiber-mode"); +#endif + + if (dp83822->fx_enabled) { + port->lanes = 1; + port->mediums = BIT(ETHTOOL_LINK_MEDIUM_BASEF) | + BIT(ETHTOOL_LINK_MEDIUM_BASEX); + } else { + /* This PHY can only to 100BaseTX max, so on 2 lanes */ + port->lanes = 2; + port->mediums = BIT(ETHTOOL_LINK_MEDIUM_BASET); + } + } + + return 0; +} + static int dp8382x_probe(struct phy_device *phydev) { struct dp83822_private *dp83822; @@ -756,24 +783,14 @@ static int dp8382x_probe(struct phy_device *phydev) static int dp83822_probe(struct phy_device *phydev) { - struct dp83822_private *dp83822; int ret; ret = dp8382x_probe(phydev); if (ret) return ret; - dp83822 = phydev->priv; - - ret = dp83822_read_straps(phydev); - if (ret) - return ret; - dp83822_of_init(phydev); - if (dp83822->fx_enabled) - phydev->port = PORT_FIBRE; - return 0; } @@ -831,6 +848,7 @@ static int dp83822_resume(struct phy_device *phydev) .handle_interrupt = dp83822_handle_interrupt, \ .suspend = dp83822_suspend, \ .resume = dp83822_resume, \ + .attach_port = dp83822_attach_port \ } #define DP83825_PHY_DRIVER(_id, _name) \ -- 2.47.1 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2024-12-20 20:14 [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Maxime Chevallier ` (4 preceding siblings ...) 2024-12-20 20:15 ` [PATCH net-next RFC 5/5] net: phy: dp83822: Add support for phy_port representation Maxime Chevallier @ 2024-12-22 15:59 ` Oleksij Rempel 2024-12-22 18:54 ` Oleksij Rempel 5 siblings, 1 reply; 26+ messages in thread From: Oleksij Rempel @ 2024-12-22 15:59 UTC (permalink / raw) To: Maxime Chevallier Cc: davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Köry Maincent, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart Hi Maxime, On Fri, Dec 20, 2024 at 09:14:59PM +0100, Maxime Chevallier wrote: > Hello everyone, > > This is a long overdue series to kick-off the introduction of a better representation > of front-facing ports for ethernet interfaces. > > First, a short disclaimer. This series is RFC, and there are quite a lot of > shortcomings : > > - There's no DT binding although this series adds a generic representation of > ports > - The port representation is in a minimal form > - No SFP support is included, but it will be required for that series to come > out of RFC as we can't gracefully handle multi-port interfaces without it. > > These shortcomigs come from timing constraints, but also because I'd like to > start discussing that topic with some code as a basis. > > For now, the only representation we have about the physical ports of an interface > come from the 'port' field (PORT_FIBRE, PORT_TP, PORT_MII, etc.), the presence or > not of an SFP module and the linkmodes reported by the ethtol ksettings ops. This > isn't enough to get a good idea of what the actual interface with the outside world > looks like. > > The end-goal of the work this series is a part of is to get support for multi-port > interfaces. My end use-case has 2 ports, each driven by a dedicated PHY, but this > also applies to dual-port PHYs. > > The current series introduces the object "struct phy_port". The naming may be > improved, as I think we could consider supporting port representation without > depending on phylib (not the case in this RFC). For now, not only do we integrate > that work in phylib, but only PHY-driven ports are supported. > > In some situations, having a good representation of the physical port in devicetree > proves to be quite useful. We're seeing vendor properties to address the lack of > port representation such as micrel,fiber-mode or ti,fiber-mode, but there are needs > for more (glitchy straps that detect fiber mode on a PHY connected to copper, > RJ45 ports connected with 2 lanes only, ...). > > As I said this RFC has no binding, sorry about that, but let's take a look at > the proposed DT syntax : > > Example 1 : PHY with RJ45 connected with 2 lanes only > > &mdio { > > ge_phy: ethernet-phy@0 { > reg = <0>; > > mdi { > port@0 { > media = "BaseT", > lanes = <2>; > }; > }; > > }; > }; > > Example 2 : PHY with a 100BaseFX port, without SFP > > &mdio { > > fiber-phy: ethernet-phy@0 { > reg = <0>; > > mdi { > port@0 { > media = "BaseF", > lanes = <1>; > }; > }; > > }; > }; > > These ports may even be used to specify PSE-PD information for PoE ports that > are drivern by a dedicated PoE controller sitting in-between the PHY and the > connector : > > &mdio { > > ge_phy: ethernet-phy@0 { > reg = <0>; > > mdi { > port@0 { > media = "BaseT", > lanes = <4>; > pse = <&pse1>; > }; > }; > > }; > }; > > The ports are initialized using the following sequence : > > 1: The PHY driver's probe() function indicated the max number of ports the device > can control > > 2: We parse the devicetree to find generic port representations > > 3: If we don't have at least one port from DT, we create one > > 4: We call the phy's .attach_port() for each port created so far. This allows > the phy driver either to take action based on the generic port devicetree > indications, or to populate the port information based on straps and > vendor-specific DT properties (think micrel,fiber-mode and similar) > > 5: If the ports are still not initialized (no .attach_port, no generic DT), then > we use snesible default value from what the PHY's supported modes. > > 6: We reconstruct the PHY's supported field in case there are limitations from the > port (2 lanes on BaseT for example). This last step will need to be changed > when SFP gets added. > > So, the current work is only about init. The next steps for that work are : > > - Introduce phy_port_ops, including a .configure() and a .read_status() to get > proper support for multi-port PHYs. This also means maintaining a list of > advertising/lp_advertising modes for each port. > > - Add SFP support. It's a tricky part, the way I see that and have prototyped is > by representing the SFP cage itself as a port, as well as the SFP module's port. > ports therefore become chainable. > > - Add the ability to list the ports in userspace. > > Prototype work for the above parts exist, but due to lack of time I couldn't get > them polished enough for inclusion in that RFC. > > Let me know if you see this going in the right direction, I'm really all ears > for suggestions on this, it's quite difficult to make sure no current use-case > breaks and no heavy rework is needed in PHY drivers. > > Patches 1, 2 and 3 are preparatory work for the mediums representation. Patch 4 > introduces the phy_port and patch 5 shows an example of usage in the dp83822 phy. I love the idea of introducing a port description. It’s a step in the right. However, in the proposed schema, I see some weak points that could be improved for better flexibility and usability. ### Weak Points Identified 1. **`lanes` Do Not Provide Enough Information** The concept of "lanes" can be ambiguous, as it is context-dependent. For differential connections, a lane typically refers to a differential pair, but the schema doesn't clearly define how the lanes map to physical wires or pins. By describing explicit pin mappings and supported modes, the `lanes` property becomes obsolete. 2. **`media` Lacks Sufficient Detail** The `media` property currently doesn’t add meaningful information for describing the physical characteristics of a port. By defining the connector type (e.g., RJ45, BNC, LC) and supported modes explicitly, the need for the `media` property is removed. ### Proposed Port Description Schema Here’s how I imagine the port description could look to address these issues: #### **Device Tree Example** /* Ports should be in the root of DT */ ports { /* Twisted-Pair Example */ port0: ethernet-port@0 { reg = <0>; /* Port index */ label = "ETH0"; /* Physical label on the device */ connector = "RJ45"; /* Connector type */ supported-modes = <10BaseT 100BaseTX>; /* Supported modes */ pairs { pair@0 { name = "A"; /* Pair A */ pins = <1 2>; /* Connector pins */ phy-mapping = <PHY_TX0_P PHY_TX0_N>; /* PHY pin mapping */ pse-mapping = <PSE_OUT0_P PSE_OUT0_N>; /* PSE pin mapping */ }; pair@1 { name = "B"; /* Pair B */ pins = <3 6>; phy-mapping = <PHY_RX0_P PHY_RX0_N>; pse-mapping = <PSE_OUT1_P PSE_OUT1_N>; }; }; pse = <&pse1>; /* Reference to attached PSE controller */ leds { link = <&led0>; /* Link status LED */ activity = <&led1>; /* Activity LED */ }; }; /* Single-Pair Ethernet (SPE) Example */ port1: ethernet-port@1 { reg = <1>; label = "SPE1"; connector = "H-MTD"; /* Single-pair connector */ supported-modes = <1000BaseT1>; /* Supported SPE modes */ pairs { pair@0 { name = "A"; /* Single pair for SPE */ pins = <1 2>; phy-mapping = <PHY_TXRX0_P PHY_TXRX0_N>; pse-mapping = <PSE_OUT0_P PSE_OUT0_N>; }; }; pse = <&pse2>; /* Reference to a different PSE controller */ leds { link = <&led3>; activity = <&led4>; }; }; /* Coaxial Example */ port2: ethernet-port@2 { reg = <2>; label = "COAX0"; /* Physical label for coaxial port */ connector = "BNC"; /* Connector type for coaxial */ supported-modes = <10Base2>; /* Supported coaxial modes */ coaxial { pins = <1>; /* Signal pin for coaxial port */ phy-mapping = <PHY_SIG>; /* Single-ended PHY signal mapping */ pse-mapping = <PSE_OUT>; /* PSE mapping (if applicable) */ }; }; /* Fiber Example */ port3: ethernet-port@3 { reg = <3>; label = "FIBER0"; /* Physical label for fiber port */ connector = "LC"; /* Connector type for fiber */ supported-modes = <100BaseFX>; /* Supported fiber modes */ fiber { tx-pins = <1 2>; /* TX signal pins for fiber */ rx-pins = <3 4>; /* RX signal pins for fiber */ phy-mapping = <PHY_TX_P PHY_TX_N PHY_RX_P PHY_RX_N>; /* TX/RX PHY mappings */ }; }; /* SFP Port Example */ port4: ethernet-port@4 { reg = <4>; /* Port index */ label = "SFP0"; /* Physical label for the SFP port */ connector = "SFP"; /* Connector type for SFP */ supported-modes = <1000BaseX 10GBaseSR>; /* Supported SFP modes */ sfp { tx-pins = <1 2>; /* TX differential pair for SFP module */ rx-pins = <3 4>; /* RX differential pair for SFP module */ phy-mapping = <PHY_TX_P PHY_TX_N PHY_RX_P PHY_RX_N>; /* TX/RX PHY mappings */ i2c = <&i2c0>; /* Reference to the I2C bus for SFP module management */ mod-def0 = <&gpio1 5 GPIO_ACTIVE_HIGH>; /* GPIO for MOD-DEF0 */ tx-disable = <&gpio1 6 GPIO_ACTIVE_HIGH>; /* GPIO for TX_DISABLE */ los = <&gpio1 7 GPIO_ACTIVE_HIGH>; /* GPIO for Loss of Signal (LOS) */ presence = <&gpio1 8 GPIO_ACTIVE_HIGH>; /* GPIO for module presence */ }; leds { link = <&led5>; /* Link status LED */ activity = <&led6>; /* Activity LED */ }; }; }; ### How This Schema Addresses the Problems 1. **Precise Pair and Pin Mapping** Each pair is explicitly described with its physical connector pins and corresponding mappings to PHY and PSE pins. This eliminates ambiguity around lane definitions and allows the PHY to handle pair-specific configurations like polarity inversion or swapped pairs. 2. **Simplified Media Representation** By specifying `connector` (e.g., RJ45, LC, BNC), the schema provides practical information about the physical interface without requiring an abstract `media` property. 3. **Support for LEDs and Power Delivery** The schema integrates additional hardware elements like LED assignments and PSE references, making it easier to configure ports for diagnostics and power delivery (PoE/PoDL). I hope this feedback helps refine the port description schema. Let me know what you think, and I’m happy to discuss this further! Best regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2024-12-22 15:59 ` [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Oleksij Rempel @ 2024-12-22 18:54 ` Oleksij Rempel 2025-01-02 10:48 ` Russell King (Oracle) 2025-01-04 22:23 ` Kory Maincent 0 siblings, 2 replies; 26+ messages in thread From: Oleksij Rempel @ 2024-12-22 18:54 UTC (permalink / raw) To: Maxime Chevallier Cc: davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Köry Maincent, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Sun, Dec 22, 2024 at 04:59:43PM +0100, Oleksij Rempel wrote: > ### Proposed Port Description Schema > > Here’s how I imagine the port description could look to address these issues: > > #### **Device Tree Example** > /* Ports should be in the root of DT */ > ports { > /* Twisted-Pair Example */ > port0: ethernet-port@0 { > reg = <0>; /* Port index */ > label = "ETH0"; /* Physical label on the device */ > connector = "RJ45"; /* Connector type */ > supported-modes = <10BaseT 100BaseTX>; /* Supported modes */ > > pairs { > pair@0 { > name = "A"; /* Pair A */ > pins = <1 2>; /* Connector pins */ > phy-mapping = <PHY_TX0_P PHY_TX0_N>; /* PHY pin mapping */ > pse-mapping = <PSE_OUT0_P PSE_OUT0_N>; /* PSE pin mapping */ > }; > pair@1 { > name = "B"; /* Pair B */ > pins = <3 6>; > phy-mapping = <PHY_RX0_P PHY_RX0_N>; > pse-mapping = <PSE_OUT1_P PSE_OUT1_N>; > }; > }; > > pse = <&pse1>; /* Reference to attached PSE controller */ > > leds { > link = <&led0>; /* Link status LED */ > activity = <&led1>; /* Activity LED */ > }; > }; Here is updated version: ports { /* 1000BaseT Port with Ethernet and simple PoE */ port0: ethernet-port@0 { reg = <0>; /* Port index */ label = "ETH0"; /* Physical label on the device */ connector = "RJ45"; /* Connector type */ supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ 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 */ pse-negative = <PSE_GND>; /* CT0 connected to GND */ }; 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_OUT0>; /* CT1 connected to PSE_OUT0 */ }; 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 */ /* No power connection to CT2 */ }; 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 */ /* No power connection to CT3 */ }; }; }; pse = <&pse1>; /* Reference to the attached PSE controller */ leds { ethernet-leds { link = <ð_led0>; /* Link status LED */ activity = <ð_led1>; /* Activity LED */ speed = <ð_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 */ }; }; }; }; A port with fully configurable PoE support: ports { /* 1000BaseT Port with Fully Configurable PoE */ port0: ethernet-port@0 { reg = <0>; /* Port index */ label = "ETH0"; /* Physical label on the device */ connector = "RJ45"; /* Connector type */ supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ shielding = "grounded"; /* Indicates the connector is shielded */ /* grounded: Shield is connected to chassis or earth ground. floating: Shield is not electrically connected. capacitive: Shield is connected to ground via a capacitor. signal: Shield is connected to the signal ground. */ 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 */ thermal { temp-sensor = <&tsensor0>; /* Reference to temperature sensor */ /* or */ thermal-zone = <&thermal_zone0>; /* Reference to thermal zone */ } fuses { overcurrent-fuse { type = "resettable"; /* Resettable polyfuse */ max-current = <1000>; /* Maximum current in milliamps */ location = "data-pairs"; /* Fuse protects data pairs */ }; overvoltage-fuse { type = "tvs-diode"; /* TVS diode for surge protection */ clamp-voltage = <60>; /* Clamping voltage in volts */ location = "poe-pairs"; /* Fuse protects PoE pairs */ }; }; leds { ethernet-leds { link = <ð_led0>; /* Link status LED */ activity = <ð_led1>; /* Activity LED */ speed = <ð_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 */ }; }; }; }; 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 */ }; -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2024-12-22 18:54 ` Oleksij Rempel @ 2025-01-02 10:48 ` Russell King (Oracle) 2025-01-02 17:03 ` Oleksij Rempel 2025-01-04 22:23 ` Kory Maincent 1 sibling, 1 reply; 26+ messages in thread From: Russell King (Oracle) @ 2025-01-02 10:48 UTC (permalink / raw) To: Oleksij Rempel Cc: Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Köry Maincent, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Sun, Dec 22, 2024 at 07:54:37PM +0100, Oleksij Rempel wrote: > Here is updated version: > > ports { > /* 1000BaseT Port with Ethernet and simple PoE */ > port0: ethernet-port@0 { > reg = <0>; /* Port index */ > label = "ETH0"; /* Physical label on the device */ > connector = "RJ45"; /* Connector type */ > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ > > 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 */ > pse-negative = <PSE_GND>; /* CT0 connected to GND */ > }; > 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_OUT0>; /* CT1 connected to PSE_OUT0 */ > }; > 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 */ > /* No power connection to CT2 */ > }; > 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 */ > /* No power connection to CT3 */ > }; > }; > }; I'm sorry, but... what is the point of giving this much detail in the DT description. How much of this actually would get used by *any* code? Why does it matter what transformer is used - surely 802.3 defines the characteristics of the signal at the RJ45 connector and it's up to the hardware designer to ensure that those characteristics are met. That will depend on the transformer, connector and board layout. What does it matter what connector pins are used? This is standardised. You also at one point had a description for a SFP cage (I'm sorry, I can't be bothered to find it in the 3000+ emails that I've missed over the Christmas period), using pin numbers 1, 2, 3, and 4. That's nonsense, those aren't the pin numbers for the data pairs. You also are effectively redefining what already exists for SFP cages - we already have a DT description for that, and it's based around the standardised connector. Why do we need a new description for SFP cages? Are we going to start converting schematics into DT representations, including any resistors and capacitors that may be present in the data path? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-02 10:48 ` Russell King (Oracle) @ 2025-01-02 17:03 ` Oleksij Rempel 2025-01-07 13:26 ` Kory Maincent 0 siblings, 1 reply; 26+ messages in thread From: Oleksij Rempel @ 2025-01-02 17:03 UTC (permalink / raw) To: Russell King (Oracle) Cc: Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Köry Maincent, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Thu, Jan 02, 2025 at 10:48:05AM +0000, Russell King (Oracle) wrote: > On Sun, Dec 22, 2024 at 07:54:37PM +0100, Oleksij Rempel wrote: > > Here is updated version: > > > > ports { > > /* 1000BaseT Port with Ethernet and simple PoE */ > > port0: ethernet-port@0 { > > reg = <0>; /* Port index */ > > label = "ETH0"; /* Physical label on the device */ > > connector = "RJ45"; /* Connector type */ > > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ > > > > 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 */ > > pse-negative = <PSE_GND>; /* CT0 connected to GND */ > > }; > > 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_OUT0>; /* CT1 connected to PSE_OUT0 */ > > }; > > 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 */ > > /* No power connection to CT2 */ > > }; > > 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 */ > > /* No power connection to CT3 */ > > }; > > }; > > }; > > I'm sorry, but... what is the point of giving this much detail in the DT > description. How much of this actually would get used by *any* code? > > Why does it matter what transformer is used - surely 802.3 defines the > characteristics of the signal at the RJ45 connector and it's up to the > hardware designer to ensure that those characteristics are met. That > will depend on the transformer, connector and board layout. > > What does it matter what connector pins are used? This is standardised. > > You also at one point had a description for a SFP cage (I'm sorry, I > can't be bothered to find it in the 3000+ emails that I've missed over > the Christmas period), using pin numbers 1, 2, 3, and 4. That's > nonsense, those aren't the pin numbers for the data pairs. You also > are effectively redefining what already exists for SFP cages - we > already have a DT description for that, and it's based around the > standardised connector. Why do we need a new description for SFP > cages? > > Are we going to start converting schematics into DT representations, > including any resistors and capacitors that may be present in the > data path? First of all, Happy New Year! :) I also apologize for the SFP example provided earlier - it was vague, unclear, and wasted your time. The purpose was not to redefine existing standards but to demonstrate that even SFP might require separate consideration if we ever move towards more detailed descriptions. My current focus, however, is on twisted pair-based Ethernet**, and I’d like to discuss that further if you’re open to it. ### Why Describe Pair Layouts? While Ethernet ports often work seamlessly even with misaligned pair layouts due to PHY auto-correction mechanisms like **MDI-X** or **CD pair swap correction**, these misalignments can affect diagnostics. If pairs are misaligned but still functional in automatic mode, the diagnostic interface may fail to provide meaningful or accurate data. For example: - MDI/MDI-X Detection: Misaligned pairs can lead to unexpected results, such as both sides reporting the same status when using a non-crossover cable. - Debugging Pair Issues: Without proper correction data in the DT, it becomes challenging to identify and validate pair swaps (e.g., A<>B or C<>D) during diagnostics. Including pair layout information in the DT ensures that the diagnostic interface can apply necessary corrections, making diagnostics usable. ### Why Address Polarity? Polarity detection and correction are mandatory, and all modern PHYs handle it seamlessly. However, Open Alliance standards require polarity indication and the ability for manual correction, particularly for automotive-grade PHYs (e.g., 1000BaseT1, 100BaseT1). (See: 6.4 Polarity Detection and Correction (POL)) https://opensig.org/wp-content/uploads/2024/01/Advanced_PHY_features_for_automotive_Ethernet_V1.0.pdf (Example of non-automotive PHYs supporting polarity indication and disabling of auto correction) https://ww1.microchip.com/downloads/en/DeviceDoc/VMDS-10242.pdf When polarity within the pair becomes relevant for diagnostics, proper PCB layout becomes crucial. It’s not uncommon for the PCB connection to differ from the expected polarity at the physical port. Including polarity details in the DT allows for better alignment between hardware and software diagnostics, ensuring issues can be detected and corrected efficiently. ### Why Mention Shielding? Shielding impacts EMI performance and compatibility, and knowing the shielding type (e.g., grounded, floating, capacitive) helps in: - Cable Selection: Ensures compatibility between shielded/unshielded cables and port design. - EMI Troubleshooting: Identifies issues in noisy environments or mismatched configurations. - System Design: Prevents ground loops and ensures compliance with EMC standards. Even though this information should ideally be part of the product documentation, having it accessible through the interface makes software-supported diagnostics much more convenient. For example, software could guide users by stating: "This controller has floating shielding. Ensure the cable is unshielded, or if shielded, it must be properly terminated at least on one side." ### Why Mention Magnetics (Transformers)? We have to deal with different types of connections: with or without magnetics. Magnetics themselves come in various types and can be classified by their common-mode rejection elements, number of cores, and other characteristics. From a software or user perspective, knowing the type and model of the magnetics is useful for several reasons. For example: - The transformer model do affect Time Domain Reflectometry offsets, sometimes resulting in discrepancies of several meters. (See: TDR Offset Calibration) https://www.analog.com/en/resources/app-notes/an-2553.html https://ww1.microchip.com/downloads/en/Appnotes/VPPD-01740.pdf - Transformers influence insertion loss and exhibit varying noise characteristics, both of which impact the analog properties of signals. - Some PHYs support tuning their analog characteristics (e.g., VoD swing, impedance matching) based on the attached magnetics. Advanced PHYs with such capabilities are often found in industrial-grade PHYs. (See: How to Tune DP83825 VoD Swing) https://www.ti.com/lit/an/snla266a/snla266a.pdf Best regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-02 17:03 ` Oleksij Rempel @ 2025-01-07 13:26 ` Kory Maincent 2025-01-07 14:02 ` Oleksij Rempel 2025-01-07 15:12 ` Russell King (Oracle) 0 siblings, 2 replies; 26+ messages in thread From: Kory Maincent @ 2025-01-07 13:26 UTC (permalink / raw) To: Oleksij Rempel Cc: Russell King (Oracle), Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Thu, 2 Jan 2025 18:03:52 +0100 Oleksij Rempel <o.rempel@pengutronix.de> wrote: > On Thu, Jan 02, 2025 at 10:48:05AM +0000, Russell King (Oracle) wrote: > > On Sun, Dec 22, 2024 at 07:54:37PM +0100, Oleksij Rempel wrote: > > > Here is updated version: > > > > > > ports { > > > /* 1000BaseT Port with Ethernet and simple PoE */ > > > port0: ethernet-port@0 { > > > reg = <0>; /* Port index */ > > > label = "ETH0"; /* Physical label on the device */ > > > connector = "RJ45"; /* Connector type */ > > > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported > > > modes */ > > > > > > 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 */ > > > pse-negative = <PSE_GND>; /* CT0 connected to GND */ > > > }; > > > 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_OUT0>; /* CT1 connected to > > > PSE_OUT0 */ }; > > > 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 */ > > > /* No power connection to CT2 */ > > > }; > > > 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 */ > > > /* No power connection to CT3 */ > > > }; > > > }; > > > }; Couldn't we begin with something simple like the following and add all the transformers and pairs information as you described later if the community feels we need it? mdis { /* 1000BaseT Port with Ethernet and PoE */ mdi0: ethernet-mdi@0 { reg = <0>; /* Port index */ label = "ETH0"; /* Physical label on the device */ connector = "RJ45"; /* Connector type */ supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ lanes = <2>; variant = "MDI-X"; /* MDI or MDI-X */ pse = <&pse1>; }; }; We can also add led, thermal and fuse subnodes later. Let's begin with something simple for the initial support, considering that it has places for additional details in the future. Regards, -- Köry Maincent, Bootlin Embedded Linux and kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 13:26 ` Kory Maincent @ 2025-01-07 14:02 ` Oleksij Rempel 2025-01-07 14:43 ` Kory Maincent 2025-01-07 15:14 ` Russell King (Oracle) 2025-01-07 15:12 ` Russell King (Oracle) 1 sibling, 2 replies; 26+ messages in thread From: Oleksij Rempel @ 2025-01-07 14:02 UTC (permalink / raw) To: Kory Maincent Cc: Russell King (Oracle), Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Tue, Jan 07, 2025 at 02:26:05PM +0100, Kory Maincent wrote: > On Thu, 2 Jan 2025 18:03:52 +0100 > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > On Thu, Jan 02, 2025 at 10:48:05AM +0000, Russell King (Oracle) wrote: > > > On Sun, Dec 22, 2024 at 07:54:37PM +0100, Oleksij Rempel wrote: > > > > Here is updated version: > > > > > > > > ports { > > > > /* 1000BaseT Port with Ethernet and simple PoE */ > > > > port0: ethernet-port@0 { > > > > reg = <0>; /* Port index */ > > > > label = "ETH0"; /* Physical label on the device */ > > > > connector = "RJ45"; /* Connector type */ > > > > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported > > > > modes */ > > > > > > > > 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 */ > > > > pse-negative = <PSE_GND>; /* CT0 connected to GND */ > > > > }; > > > > 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_OUT0>; /* CT1 connected to > > > > PSE_OUT0 */ }; > > > > 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 */ > > > > /* No power connection to CT2 */ > > > > }; > > > > 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 */ > > > > /* No power connection to CT3 */ > > > > }; > > > > }; > > > > }; > > Couldn't we begin with something simple like the following and add all the > transformers and pairs information as you described later if the community feels > we need it? > > mdis { > > /* 1000BaseT Port with Ethernet and PoE */ > mdi0: ethernet-mdi@0 { > reg = <0>; /* Port index */ > label = "ETH0"; /* Physical label on the device */ > connector = "RJ45"; /* Connector type */ > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ > lanes = <2>; > variant = "MDI-X"; /* MDI or MDI-X */ > pse = <&pse1>; > }; > }; The problematic properties are lanes and variants. Lanes seems to not provide any additional information which is not provided by the supported-modes. We have at least following working variants, which are supported by (some?) microchip PHYs: https://microchip.my.site.com/s/article/1000Base-T-Differential-Pair-Swapping For swapping A and B pairs, we may use MDI/MDI-X. What is about swapped C and D pairs? The IEEE 802.3 - 2022 has following variants: 14.5.2 Crossover function - only A<>B swap is supported 40.4.4 Automatic MDI/MDI-X Configuration - only A<>B swap is supported? 55.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported 113.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported 126.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported This was only the pair swap. How to reflect the polarity swap withing the pairs? > We can also add led, thermal and fuse subnodes later. > Let's begin with something simple for the initial support, considering > that it has places for additional details in the future. Yes :) -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 14:02 ` Oleksij Rempel @ 2025-01-07 14:43 ` Kory Maincent 2025-01-07 14:53 ` Oleksij Rempel 2025-01-07 15:14 ` Russell King (Oracle) 1 sibling, 1 reply; 26+ messages in thread From: Kory Maincent @ 2025-01-07 14:43 UTC (permalink / raw) To: Oleksij Rempel Cc: Russell King (Oracle), Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Tue, 7 Jan 2025 15:02:46 +0100 Oleksij Rempel <o.rempel@pengutronix.de> wrote: > On Tue, Jan 07, 2025 at 02:26:05PM +0100, Kory Maincent wrote: > > On Thu, 2 Jan 2025 18:03:52 +0100 > > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > > > On Thu, Jan 02, 2025 at 10:48:05AM +0000, Russell King (Oracle) wrote: > [...] > [...] > > > > Couldn't we begin with something simple like the following and add all the > > transformers and pairs information as you described later if the community > > feels we need it? > > > > mdis { > > > > /* 1000BaseT Port with Ethernet and PoE */ > > mdi0: ethernet-mdi@0 { > > reg = <0>; /* Port index */ > > label = "ETH0"; /* Physical label on the device */ > > connector = "RJ45"; /* Connector type */ > > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes > > */ lanes = <2>; > > variant = "MDI-X"; /* MDI or MDI-X */ > > pse = <&pse1>; > > }; > > }; > > The problematic properties are lanes and variants. > > Lanes seems to not provide any additional information which is not > provided by the supported-modes. > > We have at least following working variants, which are supported by (some?) > microchip PHYs: > https://microchip.my.site.com/s/article/1000Base-T-Differential-Pair-Swapping > For swapping A and B pairs, we may use MDI/MDI-X. What is about swapped > C and D pairs? > > The IEEE 802.3 - 2022 has following variants: > 14.5.2 Crossover function - only A<>B swap is supported > 40.4.4 Automatic MDI/MDI-X Configuration - only A<>B swap is supported? > 55.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported > 113.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported > 126.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported > > This was only the pair swap. How to reflect the polarity swap withing > the pairs? Indeed I see what you mean. Maybe we could add it later as optional binding and only focus for now on the current needs. According to Maxime proposition he wants the connector types and the supported modes (or number of lanes). On my side I am interested in the PSE phandle. We could begin with this: mdis { /* 1000BaseT Port with Ethernet and PoE */ mdi0: ethernet-mdi@0 { reg = <0>; /* Port index */ label = "ETH0"; /* Physical label on the device */ connector = "RJ45"; /* Connector type */ supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ pse = <&pse1>; }; }; Your proposition will stay in our mind for future development on the subject. I think we currently don't have enough time available to develop the full package. What do you think? Regards, -- Köry Maincent, Bootlin Embedded Linux and kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 14:43 ` Kory Maincent @ 2025-01-07 14:53 ` Oleksij Rempel 0 siblings, 0 replies; 26+ messages in thread From: Oleksij Rempel @ 2025-01-07 14:53 UTC (permalink / raw) To: Kory Maincent Cc: Russell King (Oracle), Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Tue, Jan 07, 2025 at 03:43:02PM +0100, Kory Maincent wrote: > On Tue, 7 Jan 2025 15:02:46 +0100 > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > On Tue, Jan 07, 2025 at 02:26:05PM +0100, Kory Maincent wrote: > > > On Thu, 2 Jan 2025 18:03:52 +0100 > > > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > > > > > On Thu, Jan 02, 2025 at 10:48:05AM +0000, Russell King (Oracle) wrote: > > [...] > > [...] > > > > > > Couldn't we begin with something simple like the following and add all the > > > transformers and pairs information as you described later if the community > > > feels we need it? > > > > > > mdis { > > > > > > /* 1000BaseT Port with Ethernet and PoE */ > > > mdi0: ethernet-mdi@0 { > > > reg = <0>; /* Port index */ > > > label = "ETH0"; /* Physical label on the device */ > > > connector = "RJ45"; /* Connector type */ > > > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes > > > */ lanes = <2>; > > > variant = "MDI-X"; /* MDI or MDI-X */ > > > pse = <&pse1>; > > > }; > > > }; > > This was only the pair swap. How to reflect the polarity swap withing > > the pairs? > > Indeed I see what you mean. Maybe we could add it later as optional binding and > only focus for now on the current needs. > According to Maxime proposition he wants the connector types and the > supported modes (or number of lanes). On my side I am interested in the PSE > phandle. > > We could begin with this: > mdis { > /* 1000BaseT Port with Ethernet and PoE */ > mdi0: ethernet-mdi@0 { > reg = <0>; /* Port index */ > label = "ETH0"; /* Physical label on the device */ > connector = "RJ45"; /* Connector type */ > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ > pse = <&pse1>; > }; > }; > > Your proposition will stay in our mind for future development on the subject. Perfect :) > I think we currently don't have enough time available to develop the full > package. Yes, no one do. > What do you think? Sounds good! -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 14:02 ` Oleksij Rempel 2025-01-07 14:43 ` Kory Maincent @ 2025-01-07 15:14 ` Russell King (Oracle) 2025-01-07 15:54 ` Oleksij Rempel 1 sibling, 1 reply; 26+ messages in thread From: Russell King (Oracle) @ 2025-01-07 15:14 UTC (permalink / raw) To: Oleksij Rempel Cc: Kory Maincent, Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Tue, Jan 07, 2025 at 03:02:46PM +0100, Oleksij Rempel wrote: > On Tue, Jan 07, 2025 at 02:26:05PM +0100, Kory Maincent wrote: > > On Thu, 2 Jan 2025 18:03:52 +0100 > > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > > > On Thu, Jan 02, 2025 at 10:48:05AM +0000, Russell King (Oracle) wrote: > > > > On Sun, Dec 22, 2024 at 07:54:37PM +0100, Oleksij Rempel wrote: > > > > > Here is updated version: > > > > > > > > > > ports { > > > > > /* 1000BaseT Port with Ethernet and simple PoE */ > > > > > port0: ethernet-port@0 { > > > > > reg = <0>; /* Port index */ > > > > > label = "ETH0"; /* Physical label on the device */ > > > > > connector = "RJ45"; /* Connector type */ > > > > > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported > > > > > modes */ > > > > > > > > > > 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 */ > > > > > pse-negative = <PSE_GND>; /* CT0 connected to GND */ > > > > > }; > > > > > 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_OUT0>; /* CT1 connected to > > > > > PSE_OUT0 */ }; > > > > > 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 */ > > > > > /* No power connection to CT2 */ > > > > > }; > > > > > 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 */ > > > > > /* No power connection to CT3 */ > > > > > }; > > > > > }; > > > > > }; > > > > Couldn't we begin with something simple like the following and add all the > > transformers and pairs information as you described later if the community feels > > we need it? > > > > mdis { > > > > /* 1000BaseT Port with Ethernet and PoE */ > > mdi0: ethernet-mdi@0 { > > reg = <0>; /* Port index */ > > label = "ETH0"; /* Physical label on the device */ > > connector = "RJ45"; /* Connector type */ > > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ > > lanes = <2>; > > variant = "MDI-X"; /* MDI or MDI-X */ > > pse = <&pse1>; > > }; > > }; > > The problematic properties are lanes and variants. > > Lanes seems to not provide any additional information which is not > provided by the supported-modes. > > We have at least following working variants, which are supported by (some?) > microchip PHYs: > https://microchip.my.site.com/s/article/1000Base-T-Differential-Pair-Swapping > For swapping A and B pairs, we may use MDI/MDI-X. What is about swapped > C and D pairs? > > The IEEE 802.3 - 2022 has following variants: > 14.5.2 Crossover function - only A<>B swap is supported > 40.4.4 Automatic MDI/MDI-X Configuration - only A<>B swap is supported? > 55.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported > 113.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported > 126.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported > > This was only the pair swap. How to reflect the polarity swap withing > the pairs? 802.3 supports this because of the problems caused by miswired cables, which are typically a user thing. It's not really there to give freedom to designers to wire their sockets incorrectly. Do we have any real cases where a socket has been wired incorrectly? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 15:14 ` Russell King (Oracle) @ 2025-01-07 15:54 ` Oleksij Rempel 0 siblings, 0 replies; 26+ messages in thread From: Oleksij Rempel @ 2025-01-07 15:54 UTC (permalink / raw) To: Russell King (Oracle) Cc: Kory Maincent, Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Tue, Jan 07, 2025 at 03:14:43PM +0000, Russell King (Oracle) wrote: > On Tue, Jan 07, 2025 at 03:02:46PM +0100, Oleksij Rempel wrote: > > On Tue, Jan 07, 2025 at 02:26:05PM +0100, Kory Maincent wrote: > > > On Thu, 2 Jan 2025 18:03:52 +0100 > > > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > The IEEE 802.3 - 2022 has following variants: > > 14.5.2 Crossover function - only A<>B swap is supported > > 40.4.4 Automatic MDI/MDI-X Configuration - only A<>B swap is supported? > > 55.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported > > 113.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported > > 126.4.4 Automatic MDI/MDI-X configuration - 4 swap variants are supported > > > > This was only the pair swap. How to reflect the polarity swap withing > > the pairs? > > 802.3 supports this because of the problems caused by miswired cables, > which are typically a user thing. It's not really there to give freedom > to designers to wire their sockets incorrectly. > > Do we have any real cases where a socket has been wired incorrectly? Yes. I tested some low cost adapter using same driver but reporting incorrect results, depending on board. I can explain it only by incorrect PCB design. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 13:26 ` Kory Maincent 2025-01-07 14:02 ` Oleksij Rempel @ 2025-01-07 15:12 ` Russell King (Oracle) 2025-01-07 16:13 ` Andrew Lunn 2025-01-07 16:15 ` Maxime Chevallier 1 sibling, 2 replies; 26+ messages in thread From: Russell King (Oracle) @ 2025-01-07 15:12 UTC (permalink / raw) To: Kory Maincent Cc: Oleksij Rempel, Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Tue, Jan 07, 2025 at 02:26:05PM +0100, Kory Maincent wrote: > On Thu, 2 Jan 2025 18:03:52 +0100 > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > On Thu, Jan 02, 2025 at 10:48:05AM +0000, Russell King (Oracle) wrote: > > > On Sun, Dec 22, 2024 at 07:54:37PM +0100, Oleksij Rempel wrote: > > > > Here is updated version: > > > > > > > > ports { > > > > /* 1000BaseT Port with Ethernet and simple PoE */ > > > > port0: ethernet-port@0 { > > > > reg = <0>; /* Port index */ > > > > label = "ETH0"; /* Physical label on the device */ > > > > connector = "RJ45"; /* Connector type */ > > > > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported > > > > modes */ > > > > > > > > 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 */ > > > > pse-negative = <PSE_GND>; /* CT0 connected to GND */ > > > > }; > > > > 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_OUT0>; /* CT1 connected to > > > > PSE_OUT0 */ }; > > > > 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 */ > > > > /* No power connection to CT2 */ > > > > }; > > > > 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 */ > > > > /* No power connection to CT3 */ > > > > }; > > > > }; > > > > }; > > Couldn't we begin with something simple like the following and add all the > transformers and pairs information as you described later if the community feels > we need it? +1. > mdis { > > /* 1000BaseT Port with Ethernet and PoE */ > mdi0: ethernet-mdi@0 { > reg = <0>; /* Port index */ > label = "ETH0"; /* Physical label on the device */ > connector = "RJ45"; /* Connector type */ > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported modes */ > lanes = <2>; > variant = "MDI-X"; /* MDI or MDI-X */ > pse = <&pse1>; > }; > }; We already manage well enough without anything like this level of detail of a RJ45 socket, so why don't we start off with something very simple. I'm thinking that even giving the supported-modes argument is too much here - have we *ever* had the case where an ethernet port can't support all the speeds that it's associated PHY supports? > We can also add led, thermal and fuse subnodes later. > Let's begin with something simple for the initial support, considering > that it has places for additional details in the future. What I think we both fear is having a complex DT description of a port that the kernel mostly ignores. While we can come out with the "but DT describes the hardware" claptrap, it's no good trying to describe the hardware in a firmware description unless there is some way to validate that the firmware description is correct - which means there must be something that depends on it in order to work. If we describe stuff that doesn't get used, there's no way to know if it is actually correct. We then end up with a lot of buggy DT descriptions with properties that can't be relied upon to be correct, and that makes those properties utterly useless. I'm sure DT maintainers will disagree due to the "DT describes the hardware" but... I've said it here, and if we end up with stuff over-described wrongly creating a mess, I'll be able to point back at this! -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 15:12 ` Russell King (Oracle) @ 2025-01-07 16:13 ` Andrew Lunn 2025-01-07 16:15 ` Maxime Chevallier 1 sibling, 0 replies; 26+ messages in thread From: Andrew Lunn @ 2025-01-07 16:13 UTC (permalink / raw) To: Russell King (Oracle) Cc: Kory Maincent, Oleksij Rempel, Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart > What I think we both fear is having a complex DT description of a > port that the kernel mostly ignores. While we can come out with the > "but DT describes the hardware" claptrap, it's no good trying to > describe the hardware in a firmware description unless there is some > way to validate that the firmware description is correct - which > means there must be something that depends on it in order to work. > > If we describe stuff that doesn't get used, there's no way to know > if it is actually correct. We then end up with a lot of buggy DT > descriptions with properties that can't be relied upon to be > correct, and that makes those properties utterly useless. This is a real issue we have had in the past. A property which was ignored, until it was not ignored, and then lots of boards broke because they had the property wrong. I would strongly recommend than any property you define is always used, validated, and ideally will not work when wrong. Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 15:12 ` Russell King (Oracle) 2025-01-07 16:13 ` Andrew Lunn @ 2025-01-07 16:15 ` Maxime Chevallier 2025-01-07 16:22 ` Andrew Lunn 1 sibling, 1 reply; 26+ messages in thread From: Maxime Chevallier @ 2025-01-07 16:15 UTC (permalink / raw) To: Russell King (Oracle) Cc: Kory Maincent, Oleksij Rempel, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart Hi Oleksij, Köry, Russell, Sorry for the silence so far, i'm now back from a restful holiday break, and I'm catching-up on this discussion. I'm replying here, but I've read the discussion so far, and thanks a lot Oleksji for all the suggestions, it's nice to see that this is interesting for you ! On Tue, 7 Jan 2025 15:12:20 +0000 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > On Tue, Jan 07, 2025 at 02:26:05PM +0100, Kory Maincent wrote: > > On Thu, 2 Jan 2025 18:03:52 +0100 > > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > > > On Thu, Jan 02, 2025 at 10:48:05AM +0000, Russell King (Oracle) wrote: > > > > On Sun, Dec 22, 2024 at 07:54:37PM +0100, Oleksij Rempel wrote: > > > > > Here is updated version: > > > > > > > > > > ports { > > > > > /* 1000BaseT Port with Ethernet and simple PoE */ > > > > > port0: ethernet-port@0 { > > > > > reg = <0>; /* Port index */ > > > > > label = "ETH0"; /* Physical label on the device */ > > > > > connector = "RJ45"; /* Connector type */ > > > > > supported-modes = <10BaseT 100BaseTX 1000BaseT>; /* Supported > > > > > modes */ So for these supported modes, this is something I've tried to avoid actually, and for 2 reasons : - With the usual argument that DT is a HW description, supported-modes doesn't work here. What matters is : - The medium used (is it twisted copper pairs ? backplane ? One of the may flavors of fiber through a dedicated connector ?) and as a complement, the number of lanes. This in itself isn't strictly necessary. A BaseT1 PHY will obviously have only one lane on its MDI for example. The reason I have included the lanes is mainly for BaseT, where BaseT1 has one lane while the typical gigabit port has 4. I have however seen devices that have a 1G PHY connected to a RJ45 port with 2 lanes only, thus limiting the max achievable speed to 100M. Here, we would explicietly describe the port has having 2 lanes. - The supported modes that can be achieved on a port is a bit redundant considering that we already have a great deal of capability discovery implemented in phylib, phylink, the SFP stack, etc. > > > > > > > > > > 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 */ > > > > > pse-negative = <PSE_GND>; /* CT0 connected to GND */ > > > > > }; > > > > > 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_OUT0>; /* CT1 connected to > > > > > PSE_OUT0 */ }; > > > > > 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 */ > > > > > /* No power connection to CT2 */ > > > > > }; > > > > > 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 */ > > > > > /* No power connection to CT3 */ > > > > > }; > > > > > }; > > > > > }; > > > > Couldn't we begin with something simple like the following and add all the > > transformers and pairs information as you described later if the community feels > > we need it? > > +1. +1 as well. One thing is that I'd like to make so that describing the port is some last-resort solution and make it really not mandatory. That means, the internal port representation that the kernel maintains should be able to be populated with sane defaults based on whatever drives the port can do. As Russell says, things are working pretty well so far, especially for single-port PHYs that can already indicated pretty precisely what they can output. Having a port representation in DT is useful for corner cases such as : - For a single-port PHY, when the PHY can't figure-out by itself what the MDI is. For example, PHYs that can drive either copper or fiber and that can't reliably derive the mode to use from its straps - For weirdnesses in the way the PHY port is wired in HW. As I mentionned, the only case I encountered was 2 lanes on a 1G PHY, thus limiting the speed to 100M max. - As an "attachment point" for things like PSE, that are really about ports and not the PHY. One could argue that this description isn't even needed, but OTHO with the recent addition of vendor properties like "micrel,fiber-mode" or "ti,fiber-mode", it appears that there's a gap to fill. I do think that there's value in Oleksij's ideas, but this discussion doesn't even include DT people who could help draw a line somewhere. All that being said, if the status-quo from this discussion is "let's keep it simple", I have some things I'd still like to discuss. First, should we keep the lanes ? And second, I'm confused about the way we deal with BaseX right now, as we treat it both as a medium and as an MII mode. If we look at 10G in comparison, we have a clear difference between the phy_interface_mode "10GBaseR" and the linkmodes "10000BaseKR", "10000BaseSR", etc. We don't really have that for baseX, and it may already be too late, but should we introduce the "1000BaseXS / 1000BaseLX / 1000BaseCX" modes ? Thanks everyone, Maxime ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 16:15 ` Maxime Chevallier @ 2025-01-07 16:22 ` Andrew Lunn 2025-01-07 16:41 ` Oleksij Rempel 0 siblings, 1 reply; 26+ messages in thread From: Andrew Lunn @ 2025-01-07 16:22 UTC (permalink / raw) To: Maxime Chevallier Cc: Russell King (Oracle), Kory Maincent, Oleksij Rempel, davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart > I have however seen devices that have a 1G PHY connected to a RJ45 > port with 2 lanes only, thus limiting the max achievable speed to 100M. > Here, we would explicietly describe the port has having 2 lanes. Some PHYs would handle this via downshift, detecting that some pairs are broken, and then dropping down to 100M on their own. So it is not always necessary to have a board property, at least not for data. I've no idea how this affects power transfer. Can the link partners detect which pairs are actually wired? Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 16:22 ` Andrew Lunn @ 2025-01-07 16:41 ` Oleksij Rempel 2025-01-07 16:49 ` Oleksij Rempel 2025-01-08 7:25 ` Maxime Chevallier 0 siblings, 2 replies; 26+ messages in thread From: Oleksij Rempel @ 2025-01-07 16:41 UTC (permalink / raw) To: Andrew Lunn Cc: Maxime Chevallier, Russell King (Oracle), Kory Maincent, davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Tue, Jan 07, 2025 at 05:22:51PM +0100, Andrew Lunn wrote: > > I have however seen devices that have a 1G PHY connected to a RJ45 > > port with 2 lanes only, thus limiting the max achievable speed to 100M. > > Here, we would explicietly describe the port has having 2 lanes. I can confirm existence of this kind of designs. One industrial real life example: a SoC connected to 3 port Gigabit KSZ switch. One port is typical RJ45 connector. Other port is RJ11 connector. The speed can be reduced by using max-speed property. But i can't provide any user usable diagnostic information just by saying pair A or B is broken. This is one of the reasons why i propose detailed description. > Some PHYs would handle this via downshift, detecting that some pairs > are broken, and then dropping down to 100M on their own. So it is not > always necessary to have a board property, at least not for data. Ack, but it will work not for all setups. > I've no idea how this affects power transfer. Can the link partners > detect which pairs are actually wired? Not is usual simple implementation. The PSE PI devicetree properties provide enough information for a _standard_ use case. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 16:41 ` Oleksij Rempel @ 2025-01-07 16:49 ` Oleksij Rempel 2025-01-08 7:25 ` Maxime Chevallier 1 sibling, 0 replies; 26+ messages in thread From: Oleksij Rempel @ 2025-01-07 16:49 UTC (permalink / raw) To: Andrew Lunn Cc: Maxime Chevallier, Russell King (Oracle), Kory Maincent, davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Tue, Jan 07, 2025 at 05:41:30PM +0100, Oleksij Rempel wrote: > On Tue, Jan 07, 2025 at 05:22:51PM +0100, Andrew Lunn wrote: > > > I have however seen devices that have a 1G PHY connected to a RJ45 > > > port with 2 lanes only, thus limiting the max achievable speed to 100M. > > > Here, we would explicietly describe the port has having 2 lanes. > > I can confirm existence of this kind of designs. One industrial real life > example: a SoC connected to 3 port Gigabit KSZ switch. One port is > typical RJ45 connector. Other port is RJ11 connector. > > The speed can be reduced by using max-speed property. But i can't > provide any user usable diagnostic information just by saying pair A or > B is broken. > > This is one of the reasons why i propose detailed description. Here is one example: https://www.balluff.com/de-de/products/BNI004F?gad_source=1&gclid=Cj0KCQiAvvO7BhC-ARIsAGFyToUmOy6VpSZUszHQ9X4tuUlyLdtAZqkk3tKxggF5z7D1pk3CgH5IOvwaAhhUEALw_wcB -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-07 16:41 ` Oleksij Rempel 2025-01-07 16:49 ` Oleksij Rempel @ 2025-01-08 7:25 ` Maxime Chevallier 2025-01-08 8:12 ` Oleksij Rempel 1 sibling, 1 reply; 26+ messages in thread From: Maxime Chevallier @ 2025-01-08 7:25 UTC (permalink / raw) To: Oleksij Rempel Cc: Andrew Lunn, Russell King (Oracle), Kory Maincent, davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Tue, 7 Jan 2025 17:41:30 +0100 Oleksij Rempel <o.rempel@pengutronix.de> wrote: > On Tue, Jan 07, 2025 at 05:22:51PM +0100, Andrew Lunn wrote: > > > I have however seen devices that have a 1G PHY connected to a RJ45 > > > port with 2 lanes only, thus limiting the max achievable speed to 100M. > > > Here, we would explicietly describe the port has having 2 lanes. > > I can confirm existence of this kind of designs. One industrial real life > example: a SoC connected to 3 port Gigabit KSZ switch. One port is > typical RJ45 connector. Other port is RJ11 connector. > > The speed can be reduced by using max-speed property. But i can't > provide any user usable diagnostic information just by saying pair A or > B is broken. > > This is one of the reasons why i propose detailed description. While I get the point, I'm wondering if it's relevant to expose this diag information for the user. As this is a HW design feature we're representing, and it's described in devicetree, the information that the HW design is wrong or uncommon is already known. So, exposing this to the user ends-up being a pretty way to display plain devicetree data, without much added value from the PHY stack ? Or am I missing the point ? I would see some value if we could detect that pairs are miswired or disconnected at runtime, then report this to user. Here the information is useful. The minimal information needed by software is in that case "how many working pairs are connected between the PHY and the connector", and possibly "are they swapped ?" but I think we already have a DT property for that ? Maxime ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-08 7:25 ` Maxime Chevallier @ 2025-01-08 8:12 ` Oleksij Rempel 0 siblings, 0 replies; 26+ messages in thread From: Oleksij Rempel @ 2025-01-08 8:12 UTC (permalink / raw) To: Maxime Chevallier Cc: Andrew Lunn, Russell King (Oracle), Kory Maincent, davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski, Eric Dumazet, Paolo Abeni, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Wed, Jan 08, 2025 at 08:25:07AM +0100, Maxime Chevallier wrote: > On Tue, 7 Jan 2025 17:41:30 +0100 > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > On Tue, Jan 07, 2025 at 05:22:51PM +0100, Andrew Lunn wrote: > > > > I have however seen devices that have a 1G PHY connected to a RJ45 > > > > port with 2 lanes only, thus limiting the max achievable speed to 100M. > > > > Here, we would explicietly describe the port has having 2 lanes. > > > > I can confirm existence of this kind of designs. One industrial real life > > example: a SoC connected to 3 port Gigabit KSZ switch. One port is > > typical RJ45 connector. Other port is RJ11 connector. > > > > The speed can be reduced by using max-speed property. But i can't > > provide any user usable diagnostic information just by saying pair A or > > B is broken. > > > > This is one of the reasons why i propose detailed description. > > While I get the point, I'm wondering if it's relevant to expose this > diag information for the user. As this is a HW design feature we're > representing, and it's described in devicetree, the information that the > HW design is wrong or uncommon is already known. So, exposing this to > the user ends-up being a pretty way to display plain devicetree data, > without much added value from the PHY stack ? Or am I missing the point > ? Correct. The same kind of information, such as whether the connector is RJ45 or an industrial one with a proprietary layout, or what label this port will have on the box, is essential. This information helps the user to manage, repair, or connect devices in the field - even after 30 years of operation, when no paper manual has survived being eaten by animals and the vendor's websites no longer exist. Here is one example of an industrial switch with PoE support: https://www.westermo.com/-/media/Files/User-guides/westermo_ug_6641-22501_viper-x12a-poe_revo.pdf?rev=083148a9e565416a9044e9a4e379635f Please pay attention to the difference for Gbit and 100Mbit ports and signal layout within this ports. > I would see some value if we could detect that pairs are miswired or > disconnected at runtime, then report this to user. Here the information > is useful. Ack. > The minimal information needed by software is in that case "how many > working pairs are connected between the PHY and the connector", and > possibly "are they swapped ?" In case of home and enterprise type of devices - yes. In case of industrial - the devices can be certified to operate in some specific link mode. For example device certified for explosive environments. These device should operate in 10BaseT1L-Vpp1 mode only, and discard any link attempts to devices which advertise 10BaseT1L-Vpp2 support in addition to 10BaseT1L-Vpp1: https://www.pepperl-fuchs.com/germany/de/classid_260.htm?view=productdetails&prodid=118298 My point is, if we will need to set limit for the link modes, soon or later, we will need a supported linkmodes property and this property will partially duplicated the lanes property. At same time, if we use supported linkmodes instead of lanes, we solve the problem with multimode PHYs which support twisted pair and fiber modes. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2024-12-22 18:54 ` Oleksij Rempel 2025-01-02 10:48 ` Russell King (Oracle) @ 2025-01-04 22:23 ` Kory Maincent 2025-01-06 18:17 ` Oleksij Rempel 1 sibling, 1 reply; 26+ messages in thread From: Kory Maincent @ 2025-01-04 22:23 UTC (permalink / raw) To: Oleksij Rempel Cc: Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart 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 = <ð_led0>; /* Link status LED */ > activity = <ð_led1>; /* Activity LED */ > speed = <ð_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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH net-next RFC 0/5] net: phy: Introduce a port representation 2025-01-04 22:23 ` Kory Maincent @ 2025-01-06 18:17 ` Oleksij Rempel 0 siblings, 0 replies; 26+ messages in thread From: Oleksij Rempel @ 2025-01-06 18:17 UTC (permalink / raw) To: Kory Maincent Cc: Maxime Chevallier, davem, netdev, linux-kernel, thomas.petazzoni, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel, Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit, Vladimir Oltean, Marek Behún, Nicolò Veronese, Simon Horman, mwojtas, Antoine Tenart On Sat, Jan 04, 2025 at 11:23:59PM +0100, Kory Maincent wrote: > On Sun, 22 Dec 2024 19:54:37 +0100 > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > 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. You are right, the PSE PI node already covers the IEEE specifications. But there are still some missing pieces. These are not directly tied to the PSE but are important for port functionality, like LEDs, thermal sensors, and transformer characteristics. ### Why Link to the Port Node While LEDs, thermal zones, and other components can be described in the Device Tree, there’s currently no clear way to indicate which ones belong to a specific port. For single-port devices, this isn’t a big issue, but for devices like switches with multiple ports, it gets messy very quickly. For example: - LEDs: Each port may have multiple LEDs for link, activity, and PoE status. Without linking them to specific ports, the software cannot correctly map these LEDs to their respective functionalities. - Thermal Sensors: Sensors located near specific ports or magnetics are crucial for thermal management. If they aren’t linked to a port, it’s unclear which sensor data applies to which port. ### Why Transformer Characteristics Matter Transformers (magnetics) are critical for Ethernet and affect software in these ways: 1. Signal Integrity: - Transformers influence noise and insertion loss. - Some PHYs allow analog tuning for the connected transformer. Software needs this information to configure PHY registers for optimal performance. 2. Power Delivery: - Transformers have power and current limits. - Software must ensure the power budget stays within these limits and detect overcurrent conditions. 3. Thermal Management: - Transformers can overheat under load. - Software should monitor nearby sensors and reduce power if temperatures exceed safe levels. This functionality does not need to be added right now. However, I’ve encountered some of these issues and believe they may impact others sooner or later. It would be good if newly added specifications avoid conflicting with or blocking potential solutions to these known issues. > > 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. Ack. Since IEEE does not provide anything beyond pinout and polarity description for PSE PI specifications (at least for PoE variants), let's keep those details there. Magnetics, being a shared component, should be described as part of the port. Thermal sensors located near magnetics or physical ports are port-related and should also be linked to the port. LEDs, however, fall into different groups: some are connected to PHYs, others to PSE, and they may be controlled by PHYs, PSE, or external controllers with software. These LEDs need a clear linkage to their corresponding port functionality. What would be the best way to establish this linkage without introducing unnecessary complexity? Best regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2025-01-08 8:12 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-20 20:14 [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 1/5] net: ethtool: common: Make BaseT a 4-lanes mode Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 2/5] net: ethtool: Introduce ETHTOOL_LINK_MEDIUM_* values Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 3/5] net: ethtool: Export the link_mode_params definitions Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 4/5] net: phy: Introduce PHY ports representation Maxime Chevallier 2024-12-20 20:15 ` [PATCH net-next RFC 5/5] net: phy: dp83822: Add support for phy_port representation Maxime Chevallier 2024-12-22 15:59 ` [PATCH net-next RFC 0/5] net: phy: Introduce a port representation Oleksij Rempel 2024-12-22 18:54 ` Oleksij Rempel 2025-01-02 10:48 ` Russell King (Oracle) 2025-01-02 17:03 ` Oleksij Rempel 2025-01-07 13:26 ` Kory Maincent 2025-01-07 14:02 ` Oleksij Rempel 2025-01-07 14:43 ` Kory Maincent 2025-01-07 14:53 ` Oleksij Rempel 2025-01-07 15:14 ` Russell King (Oracle) 2025-01-07 15:54 ` Oleksij Rempel 2025-01-07 15:12 ` Russell King (Oracle) 2025-01-07 16:13 ` Andrew Lunn 2025-01-07 16:15 ` Maxime Chevallier 2025-01-07 16:22 ` Andrew Lunn 2025-01-07 16:41 ` Oleksij Rempel 2025-01-07 16:49 ` Oleksij Rempel 2025-01-08 7:25 ` Maxime Chevallier 2025-01-08 8:12 ` Oleksij Rempel 2025-01-04 22:23 ` Kory Maincent 2025-01-06 18:17 ` Oleksij Rempel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).