* [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals
@ 2025-11-10 11:05 Horatiu Vultur
2025-11-10 11:05 ` [PATCH 1/2] phy: microchip: lan966x: Add support for inverting the rx/tx lanes Horatiu Vultur
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Horatiu Vultur @ 2025-11-10 11:05 UTC (permalink / raw)
To: vkoul, kishon, robh, krzk+dt, conor+dt
Cc: linux-phy, devicetree, linux-kernel, Horatiu Vultur
Allow to invert the N and P signals of the Serdes for both RX and TX. This
is used to allow the board designer to trace more easily the signals.
Horatiu Vultur (2):
phy: microchip: lan966x: Add support for inverting the rx/tx lanes
dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
.../phy/microchip,lan966x-serdes.yaml | 24 +++++++++++++++++++
drivers/phy/microchip/lan966x_serdes.c | 23 ++++++++++++++++++
2 files changed, 47 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] phy: microchip: lan966x: Add support for inverting the rx/tx lanes
2025-11-10 11:05 [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals Horatiu Vultur
@ 2025-11-10 11:05 ` Horatiu Vultur
2025-11-10 11:05 ` [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted Horatiu Vultur
2025-11-10 11:42 ` [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals Vladimir Oltean
2 siblings, 0 replies; 16+ messages in thread
From: Horatiu Vultur @ 2025-11-10 11:05 UTC (permalink / raw)
To: vkoul, kishon, robh, krzk+dt, conor+dt
Cc: linux-phy, devicetree, linux-kernel, Horatiu Vultur
The lan966x has 3 SerDes and they have the capability to invert the P
and N signal for the TX and RX. Add this configuration option, which is
done through device tree ans the board designer will decide if the
serdes signals to be inverted.
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
drivers/phy/microchip/lan966x_serdes.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/phy/microchip/lan966x_serdes.c b/drivers/phy/microchip/lan966x_serdes.c
index 835e369cdfc5f..5faf2001a100b 100644
--- a/drivers/phy/microchip/lan966x_serdes.c
+++ b/drivers/phy/microchip/lan966x_serdes.c
@@ -17,6 +17,11 @@
#define PLL_CONF_SERDES_125MHZ 2
#define PLL_CONF_BYPASS 3
+#define LAN966X_SERDES_MAX 3
+#define LAN966X_SERDES_DIR 2
+#define LAN966X_SERDES_TX 0
+#define LAN966X_SERDES_RX 1
+
#define lan_offset_(id, tinst, tcnt, \
gbase, ginst, gcnt, gwidth, \
raddr, rinst, rcnt, rwidth) \
@@ -129,6 +134,7 @@ struct serdes_ctrl {
struct device *dev;
struct phy *phys[SERDES_MAX];
int ref125;
+ bool inverted[LAN966X_SERDES_MAX][LAN966X_SERDES_DIR];
};
struct serdes_macro {
@@ -387,6 +393,11 @@ static int lan966x_sd6g40_setup(struct serdes_macro *macro, u32 idx, int mode)
conf.refclk125M = macro->ctrl->ref125;
+ if (macro->ctrl->inverted[idx][LAN966X_SERDES_TX])
+ conf.txinvert = 1;
+ if (macro->ctrl->inverted[idx][LAN966X_SERDES_RX])
+ conf.rxinvert = 1;
+
if (mode == PHY_INTERFACE_MODE_QSGMII)
conf.mode = LAN966X_SD6G40_MODE_QSGMII;
else
@@ -596,6 +607,18 @@ static int serdes_probe(struct platform_device *pdev)
ctrl->ref125 = (val == PLL_CONF_125MHZ ||
val == PLL_CONF_SERDES_125MHZ);
+ for (i = 0; i < LAN966X_SERDES_MAX; ++i) {
+ u8 prop[25];
+
+ sprintf(prop, "microchip,s%d-tx-inverted", i);
+ if (device_property_read_bool(ctrl->dev, prop))
+ ctrl->inverted[i][LAN966X_SERDES_TX] = true;
+
+ sprintf(prop, "microchip,s%d-rx-inverted", i);
+ if (device_property_read_bool(ctrl->dev, prop))
+ ctrl->inverted[i][LAN966X_SERDES_RX] = true;
+ }
+
dev_set_drvdata(&pdev->dev, ctrl);
provider = devm_of_phy_provider_register(ctrl->dev,
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
2025-11-10 11:05 [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals Horatiu Vultur
2025-11-10 11:05 ` [PATCH 1/2] phy: microchip: lan966x: Add support for inverting the rx/tx lanes Horatiu Vultur
@ 2025-11-10 11:05 ` Horatiu Vultur
2025-11-10 18:43 ` Conor Dooley
2025-11-10 11:42 ` [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals Vladimir Oltean
2 siblings, 1 reply; 16+ messages in thread
From: Horatiu Vultur @ 2025-11-10 11:05 UTC (permalink / raw)
To: vkoul, kishon, robh, krzk+dt, conor+dt
Cc: linux-phy, devicetree, linux-kernel, Horatiu Vultur
This allows to invert the N and P signals of the RX and TX Serdes
signals. This option allows the board designer to trace their signals
easier on the boards.
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
.../phy/microchip,lan966x-serdes.yaml | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml b/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml
index 6e914fbbac567..21b19e06a75aa 100644
--- a/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml
+++ b/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml
@@ -41,6 +41,30 @@ properties:
- The macro to be used. The macros are defined in
dt-bindings/phy/phy-lan966x-serdes.
+ microchip,s0-tx-inverted:
+ type: boolean
+ description: Invert the TX N and P signals for Serdes 0
+
+ microchip,s1-tx-inverted:
+ type: boolean
+ description: Invert the TX N and P signals for Serdes 1
+
+ microchip,s2-tx-inverted:
+ type: boolean
+ description: Invert the TX N and P signals for Serdes 2
+
+ microchip,s0-rx-inverted:
+ type: boolean
+ description: Invert the RX N and P signals for Serdes 0
+
+ microchip,s1-rx-inverted:
+ type: boolean
+ description: Invert the RX N and P signals for Serdes 1
+
+ microchip,s2-rx-inverted:
+ type: boolean
+ description: Invert the RX N and P signals for Serdes 2
+
required:
- compatible
- reg
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals
2025-11-10 11:05 [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals Horatiu Vultur
2025-11-10 11:05 ` [PATCH 1/2] phy: microchip: lan966x: Add support for inverting the rx/tx lanes Horatiu Vultur
2025-11-10 11:05 ` [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted Horatiu Vultur
@ 2025-11-10 11:42 ` Vladimir Oltean
2025-11-11 9:50 ` Horatiu Vultur
2 siblings, 1 reply; 16+ messages in thread
From: Vladimir Oltean @ 2025-11-10 11:42 UTC (permalink / raw)
To: Horatiu Vultur
Cc: vkoul, kishon, robh, krzk+dt, conor+dt, linux-phy, devicetree,
linux-kernel, Daniel Golle
Hi Horatiu,
On Mon, Nov 10, 2025 at 12:05:34PM +0100, Horatiu Vultur wrote:
> Allow to invert the N and P signals of the Serdes for both RX and TX. This
> is used to allow the board designer to trace more easily the signals.
>
> Horatiu Vultur (2):
> phy: microchip: lan966x: Add support for inverting the rx/tx lanes
> dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
>
> .../phy/microchip,lan966x-serdes.yaml | 24 +++++++++++++++++++
> drivers/phy/microchip/lan966x_serdes.c | 23 ++++++++++++++++++
> 2 files changed, 47 insertions(+)
>
> --
> 2.34.1
For context, I am trying to describe the lane polarity property
generically, and I've already blocked Daniel Golle's attempt to
introduce the similar in intent "maxlinear,rx-inverted" and
"maxlinear,tx-inverted".
https://lore.kernel.org/netdev/20251028000959.3kiac5kwo5pcl4ft@skbuf/
I am trying to find out all there is to know in order about this
feature, and I just noticed your patch, so I have to ask some questions
in order to understand, had a generic property existed, whether you
would have used it.
So I see that you don't have OF nodes for individual SerDes lanes, so
this makes your device tree structure incompatible with simple
"tx-polarity"/"rx-polarity" properties. Are those something you're not
willing to introduce? What about other stuff that's in
Documentation/devicetree/bindings/phy/transmit-amplitude.yaml?
You also won't be able to make use of the existing device tree
properties if you don't have OF node containers for each lane.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
2025-11-10 11:05 ` [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted Horatiu Vultur
@ 2025-11-10 18:43 ` Conor Dooley
2025-11-11 9:58 ` Horatiu Vultur
0 siblings, 1 reply; 16+ messages in thread
From: Conor Dooley @ 2025-11-10 18:43 UTC (permalink / raw)
To: Horatiu Vultur
Cc: vkoul, kishon, robh, krzk+dt, conor+dt, linux-phy, devicetree,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]
On Mon, Nov 10, 2025 at 12:05:36PM +0100, Horatiu Vultur wrote:
> This allows to invert the N and P signals of the RX and TX Serdes
> signals. This option allows the board designer to trace their signals
> easier on the boards.
Why can't this just be done in software, debugfs or something like that?
Maybe it's just your description is poor, but sounds like the intention
here is to just switch things around for debug purposes.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
> .../phy/microchip,lan966x-serdes.yaml | 24 +++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml b/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml
> index 6e914fbbac567..21b19e06a75aa 100644
> --- a/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml
> +++ b/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml
> @@ -41,6 +41,30 @@ properties:
> - The macro to be used. The macros are defined in
> dt-bindings/phy/phy-lan966x-serdes.
>
> + microchip,s0-tx-inverted:
> + type: boolean
> + description: Invert the TX N and P signals for Serdes 0
> +
> + microchip,s1-tx-inverted:
> + type: boolean
> + description: Invert the TX N and P signals for Serdes 1
> +
> + microchip,s2-tx-inverted:
> + type: boolean
> + description: Invert the TX N and P signals for Serdes 2
> +
> + microchip,s0-rx-inverted:
> + type: boolean
> + description: Invert the RX N and P signals for Serdes 0
> +
> + microchip,s1-rx-inverted:
> + type: boolean
> + description: Invert the RX N and P signals for Serdes 1
> +
> + microchip,s2-rx-inverted:
> + type: boolean
> + description: Invert the RX N and P signals for Serdes 2
> +
> required:
> - compatible
> - reg
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals
2025-11-10 11:42 ` [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals Vladimir Oltean
@ 2025-11-11 9:50 ` Horatiu Vultur
2025-11-13 16:30 ` Vladimir Oltean
0 siblings, 1 reply; 16+ messages in thread
From: Horatiu Vultur @ 2025-11-11 9:50 UTC (permalink / raw)
To: Vladimir Oltean
Cc: vkoul, kishon, robh, krzk+dt, conor+dt, linux-phy, devicetree,
linux-kernel, Daniel Golle
The 11/10/2025 13:42, Vladimir Oltean wrote:
>
> Hi Horatiu,
Hi Vladimir,
>
> On Mon, Nov 10, 2025 at 12:05:34PM +0100, Horatiu Vultur wrote:
> > Allow to invert the N and P signals of the Serdes for both RX and TX. This
> > is used to allow the board designer to trace more easily the signals.
> >
> > Horatiu Vultur (2):
> > phy: microchip: lan966x: Add support for inverting the rx/tx lanes
> > dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
> >
> > .../phy/microchip,lan966x-serdes.yaml | 24 +++++++++++++++++++
> > drivers/phy/microchip/lan966x_serdes.c | 23 ++++++++++++++++++
> > 2 files changed, 47 insertions(+)
> >
> > --
> > 2.34.1
>
> For context, I am trying to describe the lane polarity property
> generically, and I've already blocked Daniel Golle's attempt to
> introduce the similar in intent "maxlinear,rx-inverted" and
> "maxlinear,tx-inverted".
> https://lore.kernel.org/netdev/20251028000959.3kiac5kwo5pcl4ft@skbuf/
>
> I am trying to find out all there is to know in order about this
> feature, and I just noticed your patch, so I have to ask some questions
> in order to understand, had a generic property existed, whether you
> would have used it.
Yes, if there was something generic that would fit, I would like to use it.
>
> So I see that you don't have OF nodes for individual SerDes lanes, so
> this makes your device tree structure incompatible with simple
> "tx-polarity"/"rx-polarity" properties. Are those something you're not
> willing to introduce?
Do you propose to change the device tree to describe each SerDes lane
individualy?
Apparently in the lan966x_serdes we have also the port muxing which I am
not sure it should be there as it should be in the switch. I have done
it this way because I have use the phy-ocelot-serdes.c as an example.
If I change the device tree to describe each lane, then first I need to
take the port muxing which is fine for me. But there might be a problem,
if someone will use a newer kernel with an older device tree, it would
break the things?
> What about other stuff that's in
> Documentation/devicetree/bindings/phy/transmit-amplitude.yaml?
> You also won't be able to make use of the existing device tree
> properties if you don't have OF node containers for each lane.
To be honest, I haven't look at transmit-amplitude.yaml yet.
--
/Horatiu
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
2025-11-10 18:43 ` Conor Dooley
@ 2025-11-11 9:58 ` Horatiu Vultur
2025-11-11 10:06 ` Krzysztof Kozlowski
0 siblings, 1 reply; 16+ messages in thread
From: Horatiu Vultur @ 2025-11-11 9:58 UTC (permalink / raw)
To: Conor Dooley
Cc: vkoul, kishon, robh, krzk+dt, conor+dt, linux-phy, devicetree,
linux-kernel
The 11/10/2025 18:43, Conor Dooley wrote:
Hi Conor,
> On Mon, Nov 10, 2025 at 12:05:36PM +0100, Horatiu Vultur wrote:
> > This allows to invert the N and P signals of the RX and TX Serdes
> > signals. This option allows the board designer to trace their signals
> > easier on the boards.
>
> Why can't this just be done in software, debugfs or something like that?
> Maybe it's just your description is poor, but sounds like the intention
> here is to just switch things around for debug purposes.
I don't think it should be done through debugfs. As this describes the
board layout and I don't think someone will want to change it at
runtime to see how things behave. So maybe the description is poor.
>
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> > .../phy/microchip,lan966x-serdes.yaml | 24 +++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml b/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml
> > index 6e914fbbac567..21b19e06a75aa 100644
> > --- a/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml
> > +++ b/Documentation/devicetree/bindings/phy/microchip,lan966x-serdes.yaml
> > @@ -41,6 +41,30 @@ properties:
> > - The macro to be used. The macros are defined in
> > dt-bindings/phy/phy-lan966x-serdes.
> >
> > + microchip,s0-tx-inverted:
> > + type: boolean
> > + description: Invert the TX N and P signals for Serdes 0
> > +
> > + microchip,s1-tx-inverted:
> > + type: boolean
> > + description: Invert the TX N and P signals for Serdes 1
> > +
> > + microchip,s2-tx-inverted:
> > + type: boolean
> > + description: Invert the TX N and P signals for Serdes 2
> > +
> > + microchip,s0-rx-inverted:
> > + type: boolean
> > + description: Invert the RX N and P signals for Serdes 0
> > +
> > + microchip,s1-rx-inverted:
> > + type: boolean
> > + description: Invert the RX N and P signals for Serdes 1
> > +
> > + microchip,s2-rx-inverted:
> > + type: boolean
> > + description: Invert the RX N and P signals for Serdes 2
> > +
> > required:
> > - compatible
> > - reg
> > --
> > 2.34.1
> >
--
/Horatiu
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
2025-11-11 9:58 ` Horatiu Vultur
@ 2025-11-11 10:06 ` Krzysztof Kozlowski
2025-11-11 17:39 ` Conor Dooley
0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-11 10:06 UTC (permalink / raw)
To: Horatiu Vultur, Conor Dooley
Cc: vkoul, kishon, robh, krzk+dt, conor+dt, linux-phy, devicetree,
linux-kernel
On 11/11/2025 10:58, Horatiu Vultur wrote:
> The 11/10/2025 18:43, Conor Dooley wrote:
>
> Hi Conor,
>
>> On Mon, Nov 10, 2025 at 12:05:36PM +0100, Horatiu Vultur wrote:
>>> This allows to invert the N and P signals of the RX and TX Serdes
>>> signals. This option allows the board designer to trace their signals
>>> easier on the boards.
>>
>> Why can't this just be done in software, debugfs or something like that?
>> Maybe it's just your description is poor, but sounds like the intention
>> here is to just switch things around for debug purposes.
>
> I don't think it should be done through debugfs. As this describes the
> board layout and I don't think someone will want to change it at
> runtime to see how things behave. So maybe the description is poor.
You said it is purely for hardware designer to trace signals, so sorry,
but that's not DTs purpose.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
2025-11-11 10:06 ` Krzysztof Kozlowski
@ 2025-11-11 17:39 ` Conor Dooley
2025-11-12 8:02 ` Horatiu Vultur
0 siblings, 1 reply; 16+ messages in thread
From: Conor Dooley @ 2025-11-11 17:39 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Horatiu Vultur, vkoul, kishon, robh, krzk+dt, conor+dt, linux-phy,
devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]
On Tue, Nov 11, 2025 at 11:06:02AM +0100, Krzysztof Kozlowski wrote:
> On 11/11/2025 10:58, Horatiu Vultur wrote:
> > The 11/10/2025 18:43, Conor Dooley wrote:
> >
> > Hi Conor,
> >
> >> On Mon, Nov 10, 2025 at 12:05:36PM +0100, Horatiu Vultur wrote:
> >>> This allows to invert the N and P signals of the RX and TX Serdes
> >>> signals. This option allows the board designer to trace their signals
> >>> easier on the boards.
> >>
> >> Why can't this just be done in software, debugfs or something like that?
> >> Maybe it's just your description is poor, but sounds like the intention
> >> here is to just switch things around for debug purposes.
> >
> > I don't think it should be done through debugfs. As this describes the
> > board layout and I don't think someone will want to change it at
> > runtime to see how things behave. So maybe the description is poor.
>
> You said it is purely for hardware designer to trace signals, so sorry,
> but that's not DTs purpose.
If it is not purely some sort of debug helper, then please explain
better in your commit message.
pw-bot: changes-requested
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
2025-11-11 17:39 ` Conor Dooley
@ 2025-11-12 8:02 ` Horatiu Vultur
2025-11-12 18:32 ` Conor Dooley
0 siblings, 1 reply; 16+ messages in thread
From: Horatiu Vultur @ 2025-11-12 8:02 UTC (permalink / raw)
To: Conor Dooley
Cc: Krzysztof Kozlowski, vkoul, kishon, robh, krzk+dt, conor+dt,
linux-phy, devicetree, linux-kernel
The 11/11/2025 17:39, Conor Dooley wrote:
> On Tue, Nov 11, 2025 at 11:06:02AM +0100, Krzysztof Kozlowski wrote:
> > On 11/11/2025 10:58, Horatiu Vultur wrote:
> > > The 11/10/2025 18:43, Conor Dooley wrote:
> > >
> > > Hi Conor,
> > >
> > >> On Mon, Nov 10, 2025 at 12:05:36PM +0100, Horatiu Vultur wrote:
> > >>> This allows to invert the N and P signals of the RX and TX Serdes
> > >>> signals. This option allows the board designer to trace their signals
> > >>> easier on the boards.
> > >>
> > >> Why can't this just be done in software, debugfs or something like that?
> > >> Maybe it's just your description is poor, but sounds like the intention
> > >> here is to just switch things around for debug purposes.
> > >
> > > I don't think it should be done through debugfs. As this describes the
> > > board layout and I don't think someone will want to change it at
> > > runtime to see how things behave. So maybe the description is poor.
> >
> > You said it is purely for hardware designer to trace signals, so sorry,
> > but that's not DTs purpose.
>
> If it is not purely some sort of debug helper, then please explain
> better in your commit message.
Yes, I will do so because I don't see how this is a debug helper
functionality. I see it as changing the polarity of some pins and there
are few examples in the devicetree bindings where pins change the
polarity. Why I see it as changing the polarity is because the
N(negative) will become P(positive) and the P(positive) will become the
N(negative), so we just invert the signals.
> pw-bot: changes-requested
--
/Horatiu
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
2025-11-12 8:02 ` Horatiu Vultur
@ 2025-11-12 18:32 ` Conor Dooley
2025-11-13 11:56 ` Horatiu Vultur
0 siblings, 1 reply; 16+ messages in thread
From: Conor Dooley @ 2025-11-12 18:32 UTC (permalink / raw)
To: Horatiu Vultur
Cc: Krzysztof Kozlowski, vkoul, kishon, robh, krzk+dt, conor+dt,
linux-phy, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2023 bytes --]
On Wed, Nov 12, 2025 at 09:02:35AM +0100, Horatiu Vultur wrote:
> The 11/11/2025 17:39, Conor Dooley wrote:
> > On Tue, Nov 11, 2025 at 11:06:02AM +0100, Krzysztof Kozlowski wrote:
> > > On 11/11/2025 10:58, Horatiu Vultur wrote:
> > > > The 11/10/2025 18:43, Conor Dooley wrote:
> > > >
> > > > Hi Conor,
> > > >
> > > >> On Mon, Nov 10, 2025 at 12:05:36PM +0100, Horatiu Vultur wrote:
> > > >>> This allows to invert the N and P signals of the RX and TX Serdes
> > > >>> signals. This option allows the board designer to trace their signals
> > > >>> easier on the boards.
> > > >>
> > > >> Why can't this just be done in software, debugfs or something like that?
> > > >> Maybe it's just your description is poor, but sounds like the intention
> > > >> here is to just switch things around for debug purposes.
> > > >
> > > > I don't think it should be done through debugfs. As this describes the
> > > > board layout and I don't think someone will want to change it at
> > > > runtime to see how things behave. So maybe the description is poor.
> > >
> > > You said it is purely for hardware designer to trace signals, so sorry,
> > > but that's not DTs purpose.
> >
> > If it is not purely some sort of debug helper, then please explain
> > better in your commit message.
>
> Yes, I will do so because I don't see how this is a debug helper
> functionality. I see it as changing the polarity of some pins and there
The word "trace" here might be problematic? Maybe you meant something
like "lay out", but all of the use of the word tracing in electronics
that I have ever seen refers to troubleshooting - be that physically
following signals to see if there's degradation or things like the
trace framework in linux.
> are few examples in the devicetree bindings where pins change the
> polarity. Why I see it as changing the polarity is because the
> N(negative) will become P(positive) and the P(positive) will become the
> N(negative), so we just invert the signals.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
2025-11-12 18:32 ` Conor Dooley
@ 2025-11-13 11:56 ` Horatiu Vultur
2025-11-13 19:13 ` Conor Dooley
0 siblings, 1 reply; 16+ messages in thread
From: Horatiu Vultur @ 2025-11-13 11:56 UTC (permalink / raw)
To: Conor Dooley
Cc: Krzysztof Kozlowski, vkoul, kishon, robh, krzk+dt, conor+dt,
linux-phy, devicetree, linux-kernel
The 11/12/2025 18:32, Conor Dooley wrote:
> On Wed, Nov 12, 2025 at 09:02:35AM +0100, Horatiu Vultur wrote:
> > The 11/11/2025 17:39, Conor Dooley wrote:
> > > On Tue, Nov 11, 2025 at 11:06:02AM +0100, Krzysztof Kozlowski wrote:
> > > > On 11/11/2025 10:58, Horatiu Vultur wrote:
> > > > > The 11/10/2025 18:43, Conor Dooley wrote:
> > > > >
> > > > > Hi Conor,
> > > > >
> > > > >> On Mon, Nov 10, 2025 at 12:05:36PM +0100, Horatiu Vultur wrote:
> > > > >>> This allows to invert the N and P signals of the RX and TX Serdes
> > > > >>> signals. This option allows the board designer to trace their signals
> > > > >>> easier on the boards.
> > > > >>
> > > > >> Why can't this just be done in software, debugfs or something like that?
> > > > >> Maybe it's just your description is poor, but sounds like the intention
> > > > >> here is to just switch things around for debug purposes.
> > > > >
> > > > > I don't think it should be done through debugfs. As this describes the
> > > > > board layout and I don't think someone will want to change it at
> > > > > runtime to see how things behave. So maybe the description is poor.
> > > >
> > > > You said it is purely for hardware designer to trace signals, so sorry,
> > > > but that's not DTs purpose.
> > >
> > > If it is not purely some sort of debug helper, then please explain
> > > better in your commit message.
> >
> > Yes, I will do so because I don't see how this is a debug helper
> > functionality. I see it as changing the polarity of some pins and there
>
> The word "trace" here might be problematic? Maybe you meant something
> like "lay out", but all of the use of the word tracing in electronics
> that I have ever seen refers to troubleshooting - be that physically
> following signals to see if there's degradation or things like the
> trace framework in linux.
I understand, by trace I meant "lay out" the signals on the board.
What do you think if I say something like this:
---
dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
The lan966x has 3 integrated SerDess and for each of them it is possible
to change the polarity of the P(possitive) and N(Negative) pins Serdes.
By changing the polarity of both pins then the functionality of the pins
will be inverted.
---
I have tried not to mention any 'lay out' or 'trace' not to make it
confusing.
>
> > are few examples in the devicetree bindings where pins change the
> > polarity. Why I see it as changing the polarity is because the
> > N(negative) will become P(positive) and the P(positive) will become the
> > N(negative), so we just invert the signals.
--
/Horatiu
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals
2025-11-11 9:50 ` Horatiu Vultur
@ 2025-11-13 16:30 ` Vladimir Oltean
2025-11-14 10:34 ` Horatiu Vultur
0 siblings, 1 reply; 16+ messages in thread
From: Vladimir Oltean @ 2025-11-13 16:30 UTC (permalink / raw)
To: Horatiu Vultur
Cc: Vladimir Oltean, vkoul, kishon, robh, krzk+dt, conor+dt,
linux-phy, devicetree, linux-kernel, Daniel Golle
Hi Horatiu,
On Tue, Nov 11, 2025 at 10:50:16AM +0100, Horatiu Vultur wrote:
> The 11/10/2025 13:42, Vladimir Oltean wrote:
> >
> > Hi Horatiu,
>
> Hi Vladimir,
>
> >
> > On Mon, Nov 10, 2025 at 12:05:34PM +0100, Horatiu Vultur wrote:
> > > Allow to invert the N and P signals of the Serdes for both RX and TX. This
> > > is used to allow the board designer to trace more easily the signals.
> > >
> > > Horatiu Vultur (2):
> > > phy: microchip: lan966x: Add support for inverting the rx/tx lanes
> > > dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
> > >
> > > .../phy/microchip,lan966x-serdes.yaml | 24 +++++++++++++++++++
> > > drivers/phy/microchip/lan966x_serdes.c | 23 ++++++++++++++++++
> > > 2 files changed, 47 insertions(+)
> > >
> > > --
> > > 2.34.1
> >
> > For context, I am trying to describe the lane polarity property
> > generically, and I've already blocked Daniel Golle's attempt to
> > introduce the similar in intent "maxlinear,rx-inverted" and
> > "maxlinear,tx-inverted".
> > https://lore.kernel.org/netdev/20251028000959.3kiac5kwo5pcl4ft@skbuf/
> >
> > I am trying to find out all there is to know in order about this
> > feature, and I just noticed your patch, so I have to ask some questions
> > in order to understand, had a generic property existed, whether you
> > would have used it.
>
> Yes, if there was something generic that would fit, I would like to use it.
>
> >
> > So I see that you don't have OF nodes for individual SerDes lanes, so
> > this makes your device tree structure incompatible with simple
> > "tx-polarity"/"rx-polarity" properties. Are those something you're not
> > willing to introduce?
>
> Do you propose to change the device tree to describe each SerDes lane
> individualy?
> Apparently in the lan966x_serdes we have also the port muxing which I am
> not sure it should be there as it should be in the switch. I have done
> it this way because I have use the phy-ocelot-serdes.c as an example.
> If I change the device tree to describe each lane, then first I need to
> take the port muxing which is fine for me. But there might be a problem,
> if someone will use a newer kernel with an older device tree, it would
> break the things?
>
> > What about other stuff that's in
> > Documentation/devicetree/bindings/phy/transmit-amplitude.yaml?
> > You also won't be able to make use of the existing device tree
> > properties if you don't have OF node containers for each lane.
>
> To be honest, I haven't look at transmit-amplitude.yaml yet.
>
> --
> /Horatiu
>
ffs :-/
The radioactive piece of #### that is my work inbox moved your reply to
the Junk folder, _even though_ you were already in the list of safe
senders and domains. I just checked this thread to see what was going on
and why you didn't respond...
Yeah, the device tree binding I want to propose is per lane, so there
needs to be an OF node for each lane.
I can't easily parse the lan966x_serdes_muxes[] macros, assuming this is
what you are talking about.
Would it be possible to leave the SerDes muxing alone (with its
#phy-cells = <2>) and just add the lane OF nodes as an extra? You can
add new support for phys = <&phandle_directly_to_lane>, but that
wouldn't remove the existing support.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
2025-11-13 11:56 ` Horatiu Vultur
@ 2025-11-13 19:13 ` Conor Dooley
0 siblings, 0 replies; 16+ messages in thread
From: Conor Dooley @ 2025-11-13 19:13 UTC (permalink / raw)
To: Horatiu Vultur
Cc: Krzysztof Kozlowski, vkoul, kishon, robh, krzk+dt, conor+dt,
linux-phy, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2623 bytes --]
On Thu, Nov 13, 2025 at 12:56:50PM +0100, Horatiu Vultur wrote:
> The 11/12/2025 18:32, Conor Dooley wrote:
> > On Wed, Nov 12, 2025 at 09:02:35AM +0100, Horatiu Vultur wrote:
> > > The 11/11/2025 17:39, Conor Dooley wrote:
> > > > On Tue, Nov 11, 2025 at 11:06:02AM +0100, Krzysztof Kozlowski wrote:
> > > > > On 11/11/2025 10:58, Horatiu Vultur wrote:
> > > > > > The 11/10/2025 18:43, Conor Dooley wrote:
> > > > > >
> > > > > > Hi Conor,
> > > > > >
> > > > > >> On Mon, Nov 10, 2025 at 12:05:36PM +0100, Horatiu Vultur wrote:
> > > > > >>> This allows to invert the N and P signals of the RX and TX Serdes
> > > > > >>> signals. This option allows the board designer to trace their signals
> > > > > >>> easier on the boards.
> > > > > >>
> > > > > >> Why can't this just be done in software, debugfs or something like that?
> > > > > >> Maybe it's just your description is poor, but sounds like the intention
> > > > > >> here is to just switch things around for debug purposes.
> > > > > >
> > > > > > I don't think it should be done through debugfs. As this describes the
> > > > > > board layout and I don't think someone will want to change it at
> > > > > > runtime to see how things behave. So maybe the description is poor.
> > > > >
> > > > > You said it is purely for hardware designer to trace signals, so sorry,
> > > > > but that's not DTs purpose.
> > > >
> > > > If it is not purely some sort of debug helper, then please explain
> > > > better in your commit message.
> > >
> > > Yes, I will do so because I don't see how this is a debug helper
> > > functionality. I see it as changing the polarity of some pins and there
> >
> > The word "trace" here might be problematic? Maybe you meant something
> > like "lay out", but all of the use of the word tracing in electronics
> > that I have ever seen refers to troubleshooting - be that physically
> > following signals to see if there's degradation or things like the
> > trace framework in linux.
>
> I understand, by trace I meant "lay out" the signals on the board.
> What do you think if I say something like this:
>
> ---
> dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
>
> The lan966x has 3 integrated SerDess and for each of them it is possible
> to change the polarity of the P(possitive) and N(Negative) pins Serdes.
> By changing the polarity of both pins then the functionality of the pins
> will be inverted.
> ---
>
> I have tried not to mention any 'lay out' or 'trace' not to make it
> confusing.
Yeah, I think that that is substantially better.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals
2025-11-13 16:30 ` Vladimir Oltean
@ 2025-11-14 10:34 ` Horatiu Vultur
2025-11-19 19:23 ` Vladimir Oltean
0 siblings, 1 reply; 16+ messages in thread
From: Horatiu Vultur @ 2025-11-14 10:34 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Vladimir Oltean, vkoul, kishon, robh, krzk+dt, conor+dt,
linux-phy, devicetree, linux-kernel, Daniel Golle
The 11/13/2025 18:30, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Horatiu,
>
> On Tue, Nov 11, 2025 at 10:50:16AM +0100, Horatiu Vultur wrote:
> > The 11/10/2025 13:42, Vladimir Oltean wrote:
> > >
> > > Hi Horatiu,
> >
> > Hi Vladimir,
> >
> > >
> > > On Mon, Nov 10, 2025 at 12:05:34PM +0100, Horatiu Vultur wrote:
> > > > Allow to invert the N and P signals of the Serdes for both RX and TX. This
> > > > is used to allow the board designer to trace more easily the signals.
> > > >
> > > > Horatiu Vultur (2):
> > > > phy: microchip: lan966x: Add support for inverting the rx/tx lanes
> > > > dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted
> > > >
> > > > .../phy/microchip,lan966x-serdes.yaml | 24 +++++++++++++++++++
> > > > drivers/phy/microchip/lan966x_serdes.c | 23 ++++++++++++++++++
> > > > 2 files changed, 47 insertions(+)
> > > >
> > > > --
> > > > 2.34.1
> > >
> > > For context, I am trying to describe the lane polarity property
> > > generically, and I've already blocked Daniel Golle's attempt to
> > > introduce the similar in intent "maxlinear,rx-inverted" and
> > > "maxlinear,tx-inverted".
> > > https://lore.kernel.org/netdev/20251028000959.3kiac5kwo5pcl4ft@skbuf/
> > >
> > > I am trying to find out all there is to know in order about this
> > > feature, and I just noticed your patch, so I have to ask some questions
> > > in order to understand, had a generic property existed, whether you
> > > would have used it.
> >
> > Yes, if there was something generic that would fit, I would like to use it.
> >
> > >
> > > So I see that you don't have OF nodes for individual SerDes lanes, so
> > > this makes your device tree structure incompatible with simple
> > > "tx-polarity"/"rx-polarity" properties. Are those something you're not
> > > willing to introduce?
> >
> > Do you propose to change the device tree to describe each SerDes lane
> > individualy?
> > Apparently in the lan966x_serdes we have also the port muxing which I am
> > not sure it should be there as it should be in the switch. I have done
> > it this way because I have use the phy-ocelot-serdes.c as an example.
> > If I change the device tree to describe each lane, then first I need to
> > take the port muxing which is fine for me. But there might be a problem,
> > if someone will use a newer kernel with an older device tree, it would
> > break the things?
> >
> > > What about other stuff that's in
> > > Documentation/devicetree/bindings/phy/transmit-amplitude.yaml?
> > > You also won't be able to make use of the existing device tree
> > > properties if you don't have OF node containers for each lane.
> >
> > To be honest, I haven't look at transmit-amplitude.yaml yet.
> >
> > --
> > /Horatiu
> >
>
> ffs :-/
>
> The radioactive piece of #### that is my work inbox moved your reply to
> the Junk folder, _even though_ you were already in the list of safe
> senders and domains. I just checked this thread to see what was going on
> and why you didn't respond...
No worries.
>
> Yeah, the device tree binding I want to propose is per lane, so there
> needs to be an OF node for each lane.
>
> I can't easily parse the lan966x_serdes_muxes[] macros, assuming this is
> what you are talking about.
Yes, I was talking about lan966x_serdes_muxes and I totally understand
that is not that easy to parse it.
>
> Would it be possible to leave the SerDes muxing alone (with its
> #phy-cells = <2>) and just add the lane OF nodes as an extra? You can
> add new support for phys = <&phandle_directly_to_lane>, but that
> wouldn't remove the existing support.
So you were thinking something like this
---
serdes: serdes@e202c000 {
compatible = "microchip,lan966x-serdes";
reg = <0xe202c000 0x9c>,
<0xe2004010 0x4>;
#phy-cells = <2>;
serdes_lane_0 {
reg = <0>;
};
};
port0 {
phys = <&serdes_lane_0>;
};
---
Maybe it is just a silly idea but what about doing like this:
---
serdes: serdes@e202c000 {
compatible = "microchip,lan966x-serdes";
reg = <0xe202c000 0x9c>,
<0xe2004010 0x4>;
#phy-cells = <2>;
status = "disabled";
serdes_lane_0 {
serdes-properties
};
};
---
Then there is no change to the ports and then in the lan966x-serdes I
will iterate over all the child nodes and read the properties for each
lane.
Anyway I can wait with this patch series until you get your changes in.
--
/Horatiu
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals
2025-11-14 10:34 ` Horatiu Vultur
@ 2025-11-19 19:23 ` Vladimir Oltean
0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Oltean @ 2025-11-19 19:23 UTC (permalink / raw)
To: Horatiu Vultur
Cc: Vladimir Oltean, vkoul, kishon, robh, krzk+dt, conor+dt,
linux-phy, devicetree, linux-kernel, Daniel Golle
Sorry for the delay. This time I got it but I forgot to answer.
On Fri, Nov 14, 2025 at 11:34:11AM +0100, Horatiu Vultur wrote:
> > Would it be possible to leave the SerDes muxing alone (with its
> > #phy-cells = <2>) and just add the lane OF nodes as an extra? You can
> > add new support for phys = <&phandle_directly_to_lane>, but that
> > wouldn't remove the existing support.
>
> So you were thinking something like this
> ---
> serdes: serdes@e202c000 {
> compatible = "microchip,lan966x-serdes";
> reg = <0xe202c000 0x9c>,
> <0xe2004010 0x4>;
> #phy-cells = <2>;
>
> serdes_lane_0 {
> reg = <0>;
> };
> };
>
> port0 {
> phys = <&serdes_lane_0>;
> };
> ---
>
> Maybe it is just a silly idea but what about doing like this:
> ---
> serdes: serdes@e202c000 {
> compatible = "microchip,lan966x-serdes";
> reg = <0xe202c000 0x9c>,
> <0xe2004010 0x4>;
> #phy-cells = <2>;
> status = "disabled";
>
> serdes_lane_0 {
> serdes-properties
> };
> };
> ---
> Then there is no change to the ports and then in the lan966x-serdes I
> will iterate over all the child nodes and read the properties for each
> lane.
Well, if you re-read my message I think we are saying the same thing,
but in reverse order.
"Would it be possible to leave the SerDes muxing alone ... and just add
the lane OF nodes as an extra?" <- this would be your "silly" idea where
serdes_lane_0 just holds reg = <0> and the polarity properties.
"You can add new support for phys = <&phandle_directly_to_lane>, but
that wouldn't remove the existing support." <- this would correspond
to the first example you gave, presented as "So you were thinking
something like this".
Actually, I wasn't saying you have to implement the first way, just that
you can optionally do that as well.
To expand on your example. The base SerDes device tree node would look
like this:
serdes: serdes@e202c000 {
compatible = "microchip,lan966x-serdes";
reg = <0xe202c000 0x9c>,
<0xe2004010 0x4>;
#phy-cells = <2>;
serdes_lane_0: phy@0 {
reg = <0>;
#phy-cells = <1>;
tx-polarity = <PHY_POL_INV>;
};
};
and you could reference it like this (i.e. keep everything the same in
the consumer as until now, to avoid breaking compatibility):
port0 {
phys = <&serdes 0 CU(0)>;
};
or like this (following the principle - if you have an OF node for the
lane, why not allow it be the PHY provider):
port0 {
phys = <&serdes_lane_0 CU(0)>;
};
Just be careful that transitioning existing boards from one phandle
format to the other is a breaking change (old kernels won't understand
the "phys" with just 1 #phy-cell).
> Anyway I can wait with this patch series until you get your changes in.
I will keep you copied to the patch set which I hope to send later today.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2025-11-19 19:23 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-10 11:05 [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals Horatiu Vultur
2025-11-10 11:05 ` [PATCH 1/2] phy: microchip: lan966x: Add support for inverting the rx/tx lanes Horatiu Vultur
2025-11-10 11:05 ` [PATCH 2/2] dt-bindings: phy: lan966x: Add optional microchip,sx-tx/rx-inverted Horatiu Vultur
2025-11-10 18:43 ` Conor Dooley
2025-11-11 9:58 ` Horatiu Vultur
2025-11-11 10:06 ` Krzysztof Kozlowski
2025-11-11 17:39 ` Conor Dooley
2025-11-12 8:02 ` Horatiu Vultur
2025-11-12 18:32 ` Conor Dooley
2025-11-13 11:56 ` Horatiu Vultur
2025-11-13 19:13 ` Conor Dooley
2025-11-10 11:42 ` [PATCH 0/2] phy: microchip: lan966x: Allow to invert N and P signals Vladimir Oltean
2025-11-11 9:50 ` Horatiu Vultur
2025-11-13 16:30 ` Vladimir Oltean
2025-11-14 10:34 ` Horatiu Vultur
2025-11-19 19:23 ` Vladimir Oltean
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).