Don't report SQI values for 10 ethernet, since the datasheet
says MSE values are only valid for 100/1000 ethernet
Signed-off-by: João Rodrigues <[email protected]>
---
drivers/net/phy/dp83867.c | 51 +++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 4120385c5a79..5741f09e29cb 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -53,6 +53,7 @@
#define DP83867_RXFSOP2 0x013A
#define DP83867_RXFSOP3 0x013B
#define DP83867_IO_MUX_CFG 0x0170
+#define DP83867_MSE_REG_1 0x0225
#define DP83867_SGMIICTL 0x00D3
#define DP83867_10M_SGMII_CFG 0x016F
#define DP83867_10M_SGMII_RATE_ADAPT_MASK BIT(7)
@@ -153,6 +154,9 @@
/* FLD_THR_CFG */
#define DP83867_FLD_THR_CFG_ENERGY_LOST_THR_MASK 0x7
+/* SQI */
+#define DP83867_MAX_SQI 0x07
+
#define DP83867_LED_COUNT 4
/* LED_DRV bits */
@@ -196,6 +200,24 @@ struct dp83867_private {
bool sgmii_ref_clk_en;
};
+/* Register values are converted to SNR(dB) as suggested by
+ * "How to utilize diagnostic test suite of Ethernet PHY":
+ * SNR(dB) = -10 * log10 (VAL/2^17) - 3 dB.
+ * SQI ranges are implemented according to "OPEN ALLIANCE - Advanced diagnostic
+ * features for 100BASE-T1 automotive Ethernet PHYs"
+ */
+
+static const u16 dp83867_mse_sqi_map[] = {
+ 0x0411, /* < 18dB */
+ 0x033b, /* 18dB =< SNR < 19dB */
+ 0x0290, /* 19dB =< SNR < 20dB */
+ 0x0209, /* 20dB =< SNR < 21dB */
+ 0x019e, /* 21dB =< SNR < 22dB */
+ 0x0149, /* 22dB =< SNR < 23dB */
+ 0x0105, /* 23dB =< SNR < 24dB */
+ 0x0000 /* 24dB =< SNR */
+};
+
static int dp83867_ack_interrupt(struct phy_device *phydev)
{
int err = phy_read(phydev, MII_DP83867_ISR);
@@ -1015,6 +1037,32 @@ static int dp83867_loopback(struct phy_device *phydev, bool enable)
enable ? BMCR_LOOPBACK : 0);
}
+static int dp83867_get_sqi(struct phy_device *phydev)
+{
+ u16 mse_val;
+ int sqi;
+ int ret;
+
+ if (phydev->speed == SPEED_10)
+ return -EOPNOTSUPP;
+
+ ret = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_MSE_REG_1);
+ if (ret < 0)
+ return ret;
+
+ mse_val = 0xFFFF & ret;
+ for (sqi = 0; sqi < ARRAY_SIZE(dp83867_mse_sqi_map); sqi++) {
+ if (mse_val >= dp83867_mse_sqi_map[sqi])
+ return sqi;
+ }
+ return -EINVAL;
+}
+
+static int dp83867_get_sqi_max(struct phy_device *phydev)
+{
+ return DP83867_MAX_SQI;
+}
+
static int
dp83867_led_brightness_set(struct phy_device *phydev,
u8 index, enum led_brightness brightness)
@@ -1195,6 +1243,9 @@ static struct phy_driver dp83867_driver[] = {
.config_intr = dp83867_config_intr,
.handle_interrupt = dp83867_handle_interrupt,
+ .get_sqi = dp83867_get_sqi,
+ .get_sqi_max = dp83867_get_sqi_max,
+
.suspend = dp83867_suspend,
.resume = dp83867_resume,
--
2.25.1
On Thu, Jun 13, 2024 at 04:51:51PM +0200, Jo?o Rodrigues wrote:
> Don't report SQI values for 10 ethernet, since the datasheet
> says MSE values are only valid for 100/1000 ethernet
The commit message could be better. Something like:
Don't report the SQI value when the link speed is 10Mbps, since the
datasheet says MSE values are only valid for 100/1000 links.
> +static int dp83867_get_sqi(struct phy_device *phydev)
> +{
> + u16 mse_val;
> + int sqi;
> + int ret;
> +
> + if (phydev->speed == SPEED_10)
> + return -EOPNOTSUPP;
What does the datasheet say about MSE where there is no link at all?
Maybe you need to expand this test to include SPEED_UNKNOWN?
Andrew
---
pw-bot: cr
On Thu, 13 Jun 2024 19:13:27 +0200
Andrew Lunn <[email protected]> wrote:
> On Thu, Jun 13, 2024 at 04:51:51PM +0200, João Rodrigues wrote:
> > Don't report SQI values for 10 ethernet, since the datasheet
> > says MSE values are only valid for 100/1000 ethernet
>
> The commit message could be better. Something like:
>
> Don't report the SQI value when the link speed is 10Mbps, since the
> datasheet says MSE values are only valid for 100/1000 links.
>
Thank you, I will use your wording on the next version.
> > +static int dp83867_get_sqi(struct phy_device *phydev)
> > +{
> > + u16 mse_val;
> > + int sqi;
> > + int ret;
> > +
> > + if (phydev->speed == SPEED_10)
> > + return -EOPNOTSUPP;
>
> What does the datasheet say about MSE where there is no link at all?
> Maybe you need to expand this test to include SPEED_UNKNOWN?
>
The datasheet does not have any information regarding this register
(or the related 0x265, 0x2A5 and 0x2E5, for the other pairs).
The information from these registers come from the "DP83867
Troubleshooting Guide (Rev. B)".
I will add the additional check for SPEED_UNKNOWN in the next
version.
João