From: Vladimir Oltean <olteanv@gmail.com>
To: davem@davemloft.net, netdev@vger.kernel.org
Cc: andrew@lunn.ch, f.fainelli@gmail.com, vivien.didelot@gmail.com,
claudiu.manoil@nxp.com, alexandru.marginean@nxp.com,
ioana.ciornei@nxp.com, linux@armlinux.org.uk
Subject: [PATCH v2 net-next 1/6] net: dsa: felix: clarify the intention of writes to MII_BMCR
Date: Sat, 4 Jul 2020 15:45:02 +0300 [thread overview]
Message-ID: <20200704124507.3336497-2-olteanv@gmail.com> (raw)
In-Reply-To: <20200704124507.3336497-1-olteanv@gmail.com>
From: Vladimir Oltean <vladimir.oltean@nxp.com>
The driver appears to write to BMCR_SPEED and BMCR_DUPLEX, fields which
are read-only, since they are actually configured through the
vendor-specific IF_MODE (0x14) register.
But the reason we're writing back the read-only values of MII_BMCR is to
alter these writable fields:
BMCR_RESET
BMCR_LOOPBACK
BMCR_ANENABLE
BMCR_PDOWN
BMCR_ISOLATE
BMCR_ANRESTART
In particular, the only field which is really relevant to this driver is
BMCR_ANENABLE. Clarify that intention by spelling it out, using
phy_set_bits and phy_clear_bits.
The driver also made a few writes to BMCR_RESET and BMCR_ANRESTART which
are unnecessary and may temporarily disrupt the link to the PHY. Remove
them.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
Patch is new.
drivers/net/dsa/ocelot/felix_vsc9959.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 2067776773f7..9f4c8343652f 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -815,7 +815,7 @@ static void vsc9959_pcs_init_sgmii(struct phy_device *pcs,
phy_write(pcs, ENETC_PCS_LINK_TIMER2,
ENETC_PCS_LINK_TIMER2_VAL);
- phy_write(pcs, MII_BMCR, BMCR_ANRESTART | BMCR_ANENABLE);
+ phy_set_bits(pcs, MII_BMCR, BMCR_ANENABLE);
} else {
int speed;
@@ -845,10 +845,7 @@ static void vsc9959_pcs_init_sgmii(struct phy_device *pcs,
ENETC_PCS_IF_MODE_SGMII_EN |
ENETC_PCS_IF_MODE_SGMII_SPEED(speed));
- /* Yes, not a mistake: speed is given by IF_MODE. */
- phy_write(pcs, MII_BMCR, BMCR_RESET |
- BMCR_SPEED1000 |
- BMCR_FULLDPLX);
+ phy_clear_bits(pcs, MII_BMCR, BMCR_ANENABLE);
}
}
@@ -882,9 +879,7 @@ static void vsc9959_pcs_init_2500basex(struct phy_device *pcs,
ENETC_PCS_IF_MODE_SGMII_EN |
ENETC_PCS_IF_MODE_SGMII_SPEED(ENETC_PCS_SPEED_2500));
- phy_write(pcs, MII_BMCR, BMCR_SPEED1000 |
- BMCR_FULLDPLX |
- BMCR_RESET);
+ phy_clear_bits(pcs, MII_BMCR, BMCR_ANENABLE);
}
static void vsc9959_pcs_init_usxgmii(struct phy_device *pcs,
--
2.25.1
next prev parent reply other threads:[~2020-07-04 12:45 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-04 12:45 [PATCH v2 net-next 0/6] PHYLINK integration improvements for Felix DSA driver Vladimir Oltean
2020-07-04 12:45 ` Vladimir Oltean [this message]
2020-07-04 12:45 ` [PATCH v2 net-next 2/6] net: dsa: felix: support half-duplex link modes Vladimir Oltean
2020-07-04 12:45 ` [PATCH v2 net-next 3/6] net: dsa: felix: unconditionally configure MAC speed to 1000Mbps Vladimir Oltean
2020-07-04 12:45 ` [PATCH v2 net-next 4/6] net: dsa: felix: set proper pause frame timers based on link speed Vladimir Oltean
2020-07-04 12:45 ` [PATCH v2 net-next 5/6] net: dsa: felix: delete .phylink_mac_an_restart code Vladimir Oltean
2020-07-04 14:56 ` Russell King - ARM Linux admin
2020-07-04 15:50 ` Vladimir Oltean
2020-07-04 18:14 ` Russell King - ARM Linux admin
2020-07-04 20:29 ` Vladimir Oltean
2020-07-04 21:55 ` Russell King - ARM Linux admin
2020-07-04 12:45 ` [PATCH v2 net-next 6/6] net: dsa: felix: use resolved link config in mac_link_up() Vladimir Oltean
2020-07-05 22:26 ` [PATCH v2 net-next 0/6] PHYLINK integration improvements for Felix DSA driver David Miller
2020-07-06 8:45 ` Russell King - ARM Linux admin
2020-07-06 19:54 ` David Miller
2020-07-06 20:39 ` Russell King - ARM Linux admin
2020-07-06 20:46 ` David Miller
2020-07-06 20:53 ` Florian Fainelli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200704124507.3336497-2-olteanv@gmail.com \
--to=olteanv@gmail.com \
--cc=alexandru.marginean@nxp.com \
--cc=andrew@lunn.ch \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=ioana.ciornei@nxp.com \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=vivien.didelot@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).