[PATCH net-next v4 0/3] net: phy: dp83822: Add support for changing the transmit amplitude voltage

[PATCH net-next v4 0/3] net: phy: dp83822: Add support for changing the transmit amplitude voltage

[Dimitri Fedrau via B4 Relay]

Add support for changing the transmit amplitude voltage in 100BASE-TX mode.
Add support for configuration via DT.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
Changes in v4:
- Remove type $ref from binding
- Remove '|' from description in binding
- Change helper function from:
    static int phy_get_int_delay_property(struct device *dev, const char *name)
  to:
    static int phy_get_u32_property(struct device *dev, const char *name, u32 *val)
- Apply helper function to phy_get_internal_delay
- Link to v3: https://lore.kernel.org/r/20250204-dp83822-tx-swing-v3-0-9798e96500d9@liebherr.com

Changes in v3:
- Switch to tx-amplitude-100base-tx-percent in bindings
- Link to v2: https://lore.kernel.org/r/20250120-dp83822-tx-swing-v2-0-07c99dc42627@liebherr.com

Changes in v2:
- Remove binding ti,tx-amplitude-100base-tx-millivolt from ti,dp83822.yaml
- Add binding tx-amplitude-100base-tx-gain-milli to ethernet-phy.yaml
- Add helper to get tx amplitude gain from DT
- Link to v1: https://lore.kernel.org/r/20250113-dp83822-tx-swing-v1-0-7ed5a9d80010@liebherr.com

---
Dimitri Fedrau (3):
      dt-bindings: net: ethernet-phy: add property tx-amplitude-100base-tx-percent
      net: phy: Add helper for getting tx amplitude gain
      net: phy: dp83822: Add support for changing the transmit amplitude voltage

 .../devicetree/bindings/net/ethernet-phy.yaml      |  6 +++
 drivers/net/phy/dp83822.c                          | 38 +++++++++++++++++++
 drivers/net/phy/phy_device.c                       | 44 +++++++++++++---------
 include/linux/phy.h                                |  4 ++
 4 files changed, 74 insertions(+), 18 deletions(-)
---
base-commit: c2933b2befe25309f4c5cfbea0ca80909735fd76
change-id: 20241213-dp83822-tx-swing-5ba6c1e9b065

Best regards,
-- 
Dimitri Fedrau <dimitri.fedrau@liebherr.com>

[PATCH net-next v4 1/3] dt-bindings: net: ethernet-phy: add property tx-amplitude-100base-tx-percent

[Dimitri Fedrau via B4 Relay]

From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

Add property tx-amplitude-100base-tx-percent in the device tree bindings
for configuring the tx amplitude of 100BASE-TX PHYs. Modifying it can be
necessary to compensate losses on the PCB and connector, so the voltages
measured on the RJ45 pins are conforming.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 2c71454ae8e362e7032e44712949e12da6826070..e0c001f1690c1eb9b0386438f2d5558fd8c94eca 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -232,6 +232,12 @@ properties:
       PHY's that have configurable TX internal delays. If this property is
       present then the PHY applies the TX delay.
 
+  tx-amplitude-100base-tx-percent:
+    description:
+      Transmit amplitude gain applied for 100BASE-TX. When omitted, the PHYs
+      default will be left as is.
+    default: 100
+
   leds:
     type: object
 

-- 
2.39.5

[PATCH net-next v4 2/3] net: phy: Add helper for getting tx amplitude gain

[Dimitri Fedrau via B4 Relay]

From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

Add helper which returns the tx amplitude gain defined in device tree.
Modifying it can be necessary to compensate losses on the PCB and
connector, so the voltages measured on the RJ45 pins are conforming.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 drivers/net/phy/phy_device.c | 44 ++++++++++++++++++++++++++------------------
 include/linux/phy.h          |  4 ++++
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 46713d27412b76077d2e51e29b8d84f4f8f0a86d..25ee085816b711c8a90ebf93001d892488935575 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3096,19 +3096,12 @@ void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause)
 EXPORT_SYMBOL(phy_get_pause);
 
 #if IS_ENABLED(CONFIG_OF_MDIO)
-static int phy_get_int_delay_property(struct device *dev, const char *name)
+static int phy_get_u32_property(struct device *dev, const char *name, u32 *val)
 {
-	s32 int_delay;
-	int ret;
-
-	ret = device_property_read_u32(dev, name, &int_delay);
-	if (ret)
-		return ret;
-
-	return int_delay;
+	return device_property_read_u32(dev, name, val);
 }
 #else
-static int phy_get_int_delay_property(struct device *dev, const char *name)
+static int phy_get_u32_property(struct device *dev, const char *name, u32 *val)
 {
 	return -EINVAL;
 }
@@ -3133,12 +3126,12 @@ static int phy_get_int_delay_property(struct device *dev, const char *name)
 s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev,
 			   const int *delay_values, int size, bool is_rx)
 {
-	s32 delay;
-	int i;
+	u32 delay;
+	int i, ret;
 
 	if (is_rx) {
-		delay = phy_get_int_delay_property(dev, "rx-internal-delay-ps");
-		if (delay < 0 && size == 0) {
+		ret = phy_get_u32_property(dev, "rx-internal-delay-ps", &delay);
+		if (ret < 0 && size == 0) {
 			if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
 			    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
 				return 1;
@@ -3147,8 +3140,8 @@ s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev,
 		}
 
 	} else {
-		delay = phy_get_int_delay_property(dev, "tx-internal-delay-ps");
-		if (delay < 0 && size == 0) {
+		ret = phy_get_u32_property(dev, "tx-internal-delay-ps", &delay);
+		if (ret < 0 && size == 0) {
 			if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
 			    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
 				return 1;
@@ -3157,8 +3150,8 @@ s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev,
 		}
 	}
 
-	if (delay < 0)
-		return delay;
+	if (ret < 0)
+		return ret;
 
 	if (size == 0)
 		return delay;
@@ -3193,6 +3186,21 @@ s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev,
 }
 EXPORT_SYMBOL(phy_get_internal_delay);
 
+int phy_get_tx_amplitude_gain(struct phy_device *phydev, struct device *dev,
+			      enum ethtool_link_mode_bit_indices linkmode,
+			      u32 *val)
+{
+	switch (linkmode) {
+	case ETHTOOL_LINK_MODE_100baseT_Full_BIT:
+		return phy_get_u32_property(dev,
+					    "tx-amplitude-100base-tx-percent",
+					    val);
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL(phy_get_tx_amplitude_gain);
+
 static int phy_led_set_brightness(struct led_classdev *led_cdev,
 				  enum led_brightness value)
 {
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 19f076a71f9462cd37588a5da240a1d54df0fe0f..7c9da26145d30e6659bf6c664e77a63b5d668a5c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -2114,6 +2114,10 @@ void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause);
 s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev,
 			   const int *delay_values, int size, bool is_rx);
 
+int phy_get_tx_amplitude_gain(struct phy_device *phydev, struct device *dev,
+			      enum ethtool_link_mode_bit_indices linkmode,
+			      u32 *val);
+
 void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv,
 		       bool *tx_pause, bool *rx_pause);
 

-- 
2.39.5

[PATCH net-next v4 3/3] net: phy: dp83822: Add support for changing the transmit amplitude voltage

[Dimitri Fedrau via B4 Relay]

From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

Add support for changing the transmit amplitude voltage in 100BASE-TX mode.
Modifying it can be necessary to compensate losses on the PCB and
connector, so the voltages measured on the RJ45 pins are conforming.

Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 drivers/net/phy/dp83822.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 6599feca1967d705331d6e354205a2485ea962f2..3662f3905d5ade8ad933608fcaeabb714a588418 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -31,6 +31,7 @@
 #define MII_DP83822_RCSR	0x17
 #define MII_DP83822_RESET_CTRL	0x1f
 #define MII_DP83822_MLEDCR	0x25
+#define MII_DP83822_LDCTRL	0x403
 #define MII_DP83822_LEDCFG1	0x460
 #define MII_DP83822_IOCTRL1	0x462
 #define MII_DP83822_IOCTRL2	0x463
@@ -123,6 +124,9 @@
 #define DP83822_IOCTRL1_GPIO1_CTRL		GENMASK(2, 0)
 #define DP83822_IOCTRL1_GPIO1_CTRL_LED_1	BIT(0)
 
+/* LDCTRL bits */
+#define DP83822_100BASE_TX_LINE_DRIVER_SWING	GENMASK(7, 4)
+
 /* IOCTRL2 bits */
 #define DP83822_IOCTRL2_GPIO2_CLK_SRC		GENMASK(6, 4)
 #define DP83822_IOCTRL2_GPIO2_CTRL		GENMASK(2, 0)
@@ -197,6 +201,7 @@ struct dp83822_private {
 	bool set_gpio2_clk_out;
 	u32 gpio2_clk_out;
 	bool led_pin_enable[DP83822_MAX_LED_PINS];
+	int tx_amplitude_100base_tx_index;
 };
 
 static int dp83822_config_wol(struct phy_device *phydev,
@@ -522,6 +527,12 @@ static int dp83822_config_init(struct phy_device *phydev)
 			       FIELD_PREP(DP83822_IOCTRL2_GPIO2_CLK_SRC,
 					  dp83822->gpio2_clk_out));
 
+	if (dp83822->tx_amplitude_100base_tx_index >= 0)
+		phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_LDCTRL,
+			       DP83822_100BASE_TX_LINE_DRIVER_SWING,
+			       FIELD_PREP(DP83822_100BASE_TX_LINE_DRIVER_SWING,
+					  dp83822->tx_amplitude_100base_tx_index));
+
 	err = dp83822_config_init_leds(phydev);
 	if (err)
 		return err;
@@ -720,6 +731,11 @@ static int dp83822_phy_reset(struct phy_device *phydev)
 }
 
 #ifdef CONFIG_OF_MDIO
+static const u32 tx_amplitude_100base_tx_gain[] = {
+	80, 82, 83, 85, 87, 88, 90, 92,
+	93, 95, 97, 98, 100, 102, 103, 105,
+};
+
 static int dp83822_of_init_leds(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
@@ -780,6 +796,8 @@ static int dp83822_of_init(struct phy_device *phydev)
 	struct dp83822_private *dp83822 = phydev->priv;
 	struct device *dev = &phydev->mdio.dev;
 	const char *of_val;
+	int i, ret;
+	u32 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
@@ -815,6 +833,26 @@ static int dp83822_of_init(struct phy_device *phydev)
 		dp83822->set_gpio2_clk_out = true;
 	}
 
+	dp83822->tx_amplitude_100base_tx_index = -1;
+	ret = phy_get_tx_amplitude_gain(phydev, dev,
+					ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+					&val);
+	if (!ret) {
+		for (i = 0; i < ARRAY_SIZE(tx_amplitude_100base_tx_gain); i++) {
+			if (tx_amplitude_100base_tx_gain[i] == val) {
+				dp83822->tx_amplitude_100base_tx_index = i;
+				break;
+			}
+		}
+
+		if (dp83822->tx_amplitude_100base_tx_index < 0) {
+			phydev_err(phydev,
+				   "Invalid value for tx-amplitude-100base-tx-percent property (%u)\n",
+				   val);
+			return -EINVAL;
+		}
+	}
+
 	return dp83822_of_init_leds(phydev);
 }
 

-- 
2.39.5

Re: [PATCH net-next v4 1/3] dt-bindings: net: ethernet-phy: add property tx-amplitude-100base-tx-percent

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]

On Tue, Feb 11, 2025 at 09:33:47AM +0100, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> 
> Add property tx-amplitude-100base-tx-percent in the device tree bindings
> for configuring the tx amplitude of 100BASE-TX PHYs. Modifying it can be
> necessary to compensate losses on the PCB and connector, so the voltages
> measured on the RJ45 pins are conforming.
> 
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

> ---
>  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 2c71454ae8e362e7032e44712949e12da6826070..e0c001f1690c1eb9b0386438f2d5558fd8c94eca 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -232,6 +232,12 @@ properties:
>        PHY's that have configurable TX internal delays. If this property is
>        present then the PHY applies the TX delay.
>  
> +  tx-amplitude-100base-tx-percent:
> +    description:
> +      Transmit amplitude gain applied for 100BASE-TX. When omitted, the PHYs
> +      default will be left as is.
> +    default: 100
> +
>    leds:
>      type: object
>  
> 
> -- 
> 2.39.5
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next v4 1/3] dt-bindings: net: ethernet-phy: add property tx-amplitude-100base-tx-percent

[Andrew Lunn]

On Tue, Feb 11, 2025 at 09:33:47AM +0100, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> 
> Add property tx-amplitude-100base-tx-percent in the device tree bindings
> for configuring the tx amplitude of 100BASE-TX PHYs. Modifying it can be
> necessary to compensate losses on the PCB and connector, so the voltages
> measured on the RJ45 pins are conforming.
> 
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> ---
>  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 2c71454ae8e362e7032e44712949e12da6826070..e0c001f1690c1eb9b0386438f2d5558fd8c94eca 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -232,6 +232,12 @@ properties:
>        PHY's that have configurable TX internal delays. If this property is
>        present then the PHY applies the TX delay.
>  
> +  tx-amplitude-100base-tx-percent:
> +    description:
> +      Transmit amplitude gain applied for 100BASE-TX. When omitted, the PHYs
> +      default will be left as is.
> +    default: 100

Doesn't having a default statement contradict the text? Maybe the
bootloader has set it to 110%, the text suggests it will be left at
that, but the default value of 100 means it will get set back to 100%?

What do your driver changes actually do?

	Andrew

Re: [PATCH net-next v4 2/3] net: phy: Add helper for getting tx amplitude gain

[Andrew Lunn]

> @@ -3133,12 +3126,12 @@ static int phy_get_int_delay_property(struct device *dev, const char *name)
>  s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev,
>  			   const int *delay_values, int size, bool is_rx)
>  {
> -	s32 delay;
> -	int i;
> +	u32 delay;
> +	int i, ret;

Networking uses reverse christmass tree. So you need to sort these two
longest first.

> +int phy_get_tx_amplitude_gain(struct phy_device *phydev, struct device *dev,
> +			      enum ethtool_link_mode_bit_indices linkmode,
> +			      u32 *val)

Since this is an exported symbol, it would be nice to have some
kerneldoc for it.

> +{
> +	switch (linkmode) {
> +	case ETHTOOL_LINK_MODE_100baseT_Full_BIT:
> +		return phy_get_u32_property(dev,
> +					    "tx-amplitude-100base-tx-percent",
> +					    val);

So no handling of the default value here. This would be the logical
place to have the 100 if the value is not in device tree.

> +	default:
> +		return -EINVAL;
> +	}
> +}
> +EXPORT_SYMBOL(phy_get_tx_amplitude_gain);

I would prefer EXPORT_SYMBOL_GPL, but up to you.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next v4 3/3] net: phy: dp83822: Add support for changing the transmit amplitude voltage

[Andrew Lunn]

> +	if (dp83822->tx_amplitude_100base_tx_index >= 0)
> +		phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_LDCTRL,
> +			       DP83822_100BASE_TX_LINE_DRIVER_SWING,
> +			       FIELD_PREP(DP83822_100BASE_TX_LINE_DRIVER_SWING,
> +					  dp83822->tx_amplitude_100base_tx_index));

So the driver leaves the value unchanged by default. Please make the
code and the binding match.

	Andrew

Re: [PATCH net-next v4 1/3] dt-bindings: net: ethernet-phy: add property tx-amplitude-100base-tx-percent

[Russell King (Oracle)]

On Tue, Feb 11, 2025 at 09:33:47AM +0100, Dimitri Fedrau via B4 Relay wrote:
> @@ -232,6 +232,12 @@ properties:
>        PHY's that have configurable TX internal delays. If this property is
>        present then the PHY applies the TX delay.
>  
> +  tx-amplitude-100base-tx-percent:
> +    description:
> +      Transmit amplitude gain applied for 100BASE-TX. When omitted, the PHYs
> +      default will be left as is.
> +    default: 100
> +

This should mention what the reference is - so 100% is 100% of what (it
would be the 802.3 specified 100BASE-TX level, but it should make that
clear.)

I'm having a hard time trying to find its specification in 802.3, so
maybe a reference to where it can be found would be useful, otherwise
it's unclear what one gets for "100%".

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v4 1/3] dt-bindings: net: ethernet-phy: add property tx-amplitude-100base-tx-percent

[Dimitri Fedrau]

Am Wed, Feb 12, 2025 at 02:04:02PM +0100 schrieb Andrew Lunn:
> On Tue, Feb 11, 2025 at 09:33:47AM +0100, Dimitri Fedrau via B4 Relay wrote:
> > From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > 
> > Add property tx-amplitude-100base-tx-percent in the device tree bindings
> > for configuring the tx amplitude of 100BASE-TX PHYs. Modifying it can be
> > necessary to compensate losses on the PCB and connector, so the voltages
> > measured on the RJ45 pins are conforming.
> > 
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> >  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > index 2c71454ae8e362e7032e44712949e12da6826070..e0c001f1690c1eb9b0386438f2d5558fd8c94eca 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > @@ -232,6 +232,12 @@ properties:
> >        PHY's that have configurable TX internal delays. If this property is
> >        present then the PHY applies the TX delay.
> >  
> > +  tx-amplitude-100base-tx-percent:
> > +    description:
> > +      Transmit amplitude gain applied for 100BASE-TX. When omitted, the PHYs
> > +      default will be left as is.
> > +    default: 100
> 
> Doesn't having a default statement contradict the text? Maybe the
> bootloader has set it to 110%, the text suggests it will be left at
> that, but the default value of 100 means it will get set back to 100%?
>
You are right, will remove the default statement.

> What do your driver changes actually do?

Only if tx-amplitude-100base-tx-percent is present, the amplitude should
be changed.

Best regards,
Dimitri Fedrau

Re: [PATCH net-next v4 1/3] dt-bindings: net: ethernet-phy: add property tx-amplitude-100base-tx-percent

[Dimitri Fedrau]

Am Wed, Feb 12, 2025 at 01:38:13PM +0000 schrieb Russell King (Oracle):
> On Tue, Feb 11, 2025 at 09:33:47AM +0100, Dimitri Fedrau via B4 Relay wrote:
> > @@ -232,6 +232,12 @@ properties:
> >        PHY's that have configurable TX internal delays. If this property is
> >        present then the PHY applies the TX delay.
> >  
> > +  tx-amplitude-100base-tx-percent:
> > +    description:
> > +      Transmit amplitude gain applied for 100BASE-TX. When omitted, the PHYs
> > +      default will be left as is.
> > +    default: 100
> > +
> 
> This should mention what the reference is - so 100% is 100% of what (it
> would be the 802.3 specified 100BASE-TX level, but it should make that
> clear.)
>
Yes, will add that to the description. 100% should match 2V peak to
peak.

> I'm having a hard time trying to find its specification in 802.3, so
> maybe a reference to where it can be found would be useful, otherwise
> it's unclear what one gets for "100%".
>
Compliance testing was done as described in:
https://download.tek.com/document/61W_17381_3.pdf

Didn't find the specification regarding the amplitude in 802.3, but
according to the document above it should then be part of ANSI X3.263.
Unfortunately I don't have access to ANSI X3.263, so I have to rely on
the information in the document above.

Best regards,
Dimitri Fedrau

Re: [PATCH net-next v4 2/3] net: phy: Add helper for getting tx amplitude gain

[Dimitri Fedrau]

Am Wed, Feb 12, 2025 at 02:15:08PM +0100 schrieb Andrew Lunn:
> > @@ -3133,12 +3126,12 @@ static int phy_get_int_delay_property(struct device *dev, const char *name)
> >  s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev,
> >  			   const int *delay_values, int size, bool is_rx)
> >  {
> > -	s32 delay;
> > -	int i;
> > +	u32 delay;
> > +	int i, ret;
> 
> Networking uses reverse christmass tree. So you need to sort these two
> longest first.
>
Will fix it.

> > +int phy_get_tx_amplitude_gain(struct phy_device *phydev, struct device *dev,
> > +			      enum ethtool_link_mode_bit_indices linkmode,
> > +			      u32 *val)
> 
> Since this is an exported symbol, it would be nice to have some
> kerneldoc for it.
>
Yes.

> > +{
> > +	switch (linkmode) {
> > +	case ETHTOOL_LINK_MODE_100baseT_Full_BIT:
> > +		return phy_get_u32_property(dev,
> > +					    "tx-amplitude-100base-tx-percent",
> > +					    val);
> 
> So no handling of the default value here. This would be the logical
> place to have the 100 if the value is not in device tree.
> 
I will get rid of the default value.

> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +EXPORT_SYMBOL(phy_get_tx_amplitude_gain);
> 
> I would prefer EXPORT_SYMBOL_GPL, but up to you.
>
Ok.

Best regards,
Dimitri Fedrau