| Message ID | 20250211-dp83822-tx-swing-v4-2-1e8ebd71ad54@liebherr.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: dp83822: Add support for changing the transmit amplitude voltage | expand |
> @@ -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
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
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);