For CL37 1000BASE-X AN, DW xPCS does not support C22 method but offers
C45 vendor-specific MII MMD for programming.
We also add the ability to disable Autoneg (through ethtool for certain
network switch that supports 1000BASE-X (1000Mbps and Full-Duplex) but
not Autoneg capability.
Tested-by: Emilio Riva <[email protected]>
Signed-off-by: Ong Boon Leong <[email protected]>
---
drivers/net/pcs/pcs-xpcs.c | 174 ++++++++++++++++++++++++++++++++++-
include/linux/pcs/pcs-xpcs.h | 3 +-
2 files changed, 173 insertions(+), 4 deletions(-)
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 61418d4dc0c..7ba60944ba0 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -77,6 +77,14 @@ static const int xpcs_sgmii_features[] = {
__ETHTOOL_LINK_MODE_MASK_NBITS,
};
+static const int xpcs_1000basex_features[] = {
+ ETHTOOL_LINK_MODE_Pause_BIT,
+ ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+ ETHTOOL_LINK_MODE_Autoneg_BIT,
+ ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+ __ETHTOOL_LINK_MODE_MASK_NBITS,
+};
+
static const int xpcs_2500basex_features[] = {
ETHTOOL_LINK_MODE_Pause_BIT,
ETHTOOL_LINK_MODE_Asym_Pause_BIT,
@@ -102,6 +110,10 @@ static const phy_interface_t xpcs_sgmii_interfaces[] = {
PHY_INTERFACE_MODE_SGMII,
};
+static const phy_interface_t xpcs_1000basex_interfaces[] = {
+ PHY_INTERFACE_MODE_1000BASEX,
+};
+
static const phy_interface_t xpcs_2500basex_interfaces[] = {
PHY_INTERFACE_MODE_2500BASEX,
PHY_INTERFACE_MODE_MAX,
@@ -112,6 +124,7 @@ enum {
DW_XPCS_10GKR,
DW_XPCS_XLGMII,
DW_XPCS_SGMII,
+ DW_XPCS_1000BASEX,
DW_XPCS_2500BASEX,
DW_XPCS_INTERFACE_MAX,
};
@@ -239,6 +252,7 @@ static int xpcs_soft_reset(struct dw_xpcs *xpcs,
break;
case DW_AN_C37_SGMII:
case DW_2500BASEX:
+ case DW_AN_C37_1000BASEX:
dev = MDIO_MMD_VEND2;
break;
default:
@@ -774,6 +788,58 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs, unsigned int mode)
return ret;
}
+static int xpcs_config_aneg_c37_1000basex(struct dw_xpcs *xpcs, unsigned int mode,
+ const unsigned long *advertising)
+{
+ int ret, mdio_ctrl;
+
+ /* For AN for 1000BASE-X mode, the settings are :-
+ * 1) VR_MII_MMD_CTRL Bit(12) [AN_ENABLE] = 0b (Disable C37 AN in case
+ * it is already enabled)
+ * 2) VR_MII_AN_CTRL Bit(2:1)[PCS_MODE] = 00b (1000BASE-X C37)
+ * 3) SR_MII_AN_ADV Bit(6)[FD] = 1b (Full Duplex)
+ * Note: Half Duplex is rarely used, so don't advertise.
+ * 4) VR_MII_MMD_CTRL Bit(12) [AN_ENABLE] = 1b (Enable C37 AN)
+ */
+ mdio_ctrl = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL);
+ if (mdio_ctrl < 0)
+ return mdio_ctrl;
+
+ if (mdio_ctrl & AN_CL37_EN) {
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL,
+ mdio_ctrl & ~AN_CL37_EN);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL);
+ if (ret < 0)
+ return ret;
+
+ ret &= ~DW_VR_MII_PCS_MODE_MASK;
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL, ret);
+ if (ret < 0)
+ return ret;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE);
+ ret |= ADVERTISE_1000XFULL;
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE, ret);
+ if (ret < 0)
+ return ret;
+
+ /* Clear CL37 AN complete status */
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
+ if (ret < 0)
+ return ret;
+
+ if (phylink_autoneg_inband(mode) &&
+ linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, advertising))
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL,
+ mdio_ctrl | AN_CL37_EN);
+
+ return ret;
+}
+
static int xpcs_config_2500basex(struct dw_xpcs *xpcs)
{
int ret;
@@ -797,7 +863,7 @@ static int xpcs_config_2500basex(struct dw_xpcs *xpcs)
}
int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
- unsigned int mode)
+ unsigned int mode, const unsigned long *advertising)
{
const struct xpcs_compat *compat;
int ret;
@@ -819,6 +885,12 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
if (ret)
return ret;
break;
+ case DW_AN_C37_1000BASEX:
+ ret = xpcs_config_aneg_c37_1000basex(xpcs, mode,
+ advertising);
+ if (ret)
+ return ret;
+ break;
case DW_2500BASEX:
ret = xpcs_config_2500basex(xpcs);
if (ret)
@@ -845,7 +917,7 @@ static int xpcs_config(struct phylink_pcs *pcs, unsigned int mode,
{
struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
- return xpcs_do_config(xpcs, interface, mode);
+ return xpcs_do_config(xpcs, interface, mode, advertising);
}
static int xpcs_get_state_c73(struct dw_xpcs *xpcs,
@@ -866,7 +938,7 @@ static int xpcs_get_state_c73(struct dw_xpcs *xpcs,
state->link = 0;
- return xpcs_do_config(xpcs, state->interface, MLO_AN_INBAND);
+ return xpcs_do_config(xpcs, state->interface, MLO_AN_INBAND, NULL);
}
if (state->an_enabled && xpcs_aneg_done_c73(xpcs, state, compat)) {
@@ -923,6 +995,50 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs,
return 0;
}
+static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
+ struct phylink_link_state *state)
+{
+ int lpa, adv;
+ int ret;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL);
+ if (ret < 0)
+ return ret;
+
+ if (ret & AN_CL37_EN) {
+ /* Reset link_state */
+ state->link = false;
+ state->speed = SPEED_UNKNOWN;
+ state->duplex = DUPLEX_UNKNOWN;
+ state->pause = 0;
+
+ lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_LPA);
+ if (lpa < 0 || lpa & LPA_RFAULT)
+ return false;
+
+ adv = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE);
+ if (adv < 0)
+ return false;
+
+ if (lpa & ADVERTISE_1000XFULL &&
+ adv & ADVERTISE_1000XFULL) {
+ state->speed = SPEED_1000;
+ state->duplex = DUPLEX_FULL;
+ state->link = true;
+ }
+
+ /* Clear CL37 AN complete status */
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
+ } else {
+ state->link = true;
+ state->speed = SPEED_1000;
+ state->duplex = DUPLEX_FULL;
+ state->pause = 0;
+ }
+
+ return 0;
+}
+
static void xpcs_get_state(struct phylink_pcs *pcs,
struct phylink_link_state *state)
{
@@ -950,6 +1066,13 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
ERR_PTR(ret));
}
break;
+ case DW_AN_C37_1000BASEX:
+ ret = xpcs_get_state_c37_1000basex(xpcs, state);
+ if (ret) {
+ pr_err("xpcs_get_state_c37_1000basex returned %pe\n",
+ ERR_PTR(ret));
+ }
+ break;
default:
return;
}
@@ -985,6 +1108,32 @@ static void xpcs_link_up_sgmii(struct dw_xpcs *xpcs, unsigned int mode,
pr_err("%s: xpcs_write returned %pe\n", __func__, ERR_PTR(ret));
}
+static void xpcs_link_up_1000basex(struct dw_xpcs *xpcs, int speed,
+ int duplex)
+{
+ int val, ret;
+
+ switch (speed) {
+ case SPEED_1000:
+ val = BMCR_SPEED1000;
+ break;
+ case SPEED_100:
+ case SPEED_10:
+ default:
+ pr_err("%s: speed = %d\n", __func__, speed);
+ return;
+ }
+
+ if (duplex == DUPLEX_FULL)
+ val |= BMCR_FULLDPLX;
+ else
+ pr_err("%s: half duplex not supported\n", __func__);
+
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1, val);
+ if (ret)
+ pr_err("%s: xpcs_write returned %pe\n", __func__, ERR_PTR(ret));
+}
+
void xpcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
phy_interface_t interface, int speed, int duplex)
{
@@ -994,9 +1143,21 @@ void xpcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
return xpcs_config_usxgmii(xpcs, speed);
if (interface == PHY_INTERFACE_MODE_SGMII)
return xpcs_link_up_sgmii(xpcs, mode, speed, duplex);
+ if (interface == PHY_INTERFACE_MODE_1000BASEX)
+ return xpcs_link_up_1000basex(xpcs, speed, duplex);
}
EXPORT_SYMBOL_GPL(xpcs_link_up);
+static void xpcs_an_restart(struct phylink_pcs *pcs)
+{
+ struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
+ int ret;
+
+ ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1);
+ ret |= BMCR_ANRESTART;
+ ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1, ret);
+}
+
static u32 xpcs_get_id(struct dw_xpcs *xpcs)
{
int ret;
@@ -1062,6 +1223,12 @@ static const struct xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = {
.num_interfaces = ARRAY_SIZE(xpcs_sgmii_interfaces),
.an_mode = DW_AN_C37_SGMII,
},
+ [DW_XPCS_1000BASEX] = {
+ .supported = xpcs_1000basex_features,
+ .interface = xpcs_1000basex_interfaces,
+ .num_interfaces = ARRAY_SIZE(xpcs_1000basex_interfaces),
+ .an_mode = DW_AN_C37_1000BASEX,
+ },
[DW_XPCS_2500BASEX] = {
.supported = xpcs_2500basex_features,
.interface = xpcs_2500basex_interfaces,
@@ -1117,6 +1284,7 @@ static const struct phylink_pcs_ops xpcs_phylink_ops = {
.pcs_validate = xpcs_validate,
.pcs_config = xpcs_config,
.pcs_get_state = xpcs_get_state,
+ .pcs_an_restart = xpcs_an_restart,
.pcs_link_up = xpcs_link_up,
};
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index 266eb26fb02..d2da1e0b4a9 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -17,6 +17,7 @@
#define DW_AN_C73 1
#define DW_AN_C37_SGMII 2
#define DW_2500BASEX 3
+#define DW_AN_C37_1000BASEX 4
struct xpcs_id;
@@ -30,7 +31,7 @@ int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface);
void xpcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
phy_interface_t interface, int speed, int duplex);
int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
- unsigned int mode);
+ unsigned int mode, const unsigned long *advertising);
void xpcs_get_interfaces(struct dw_xpcs *xpcs, unsigned long *interfaces);
int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns,
int enable);
--
2.25.1
Hi,
On Fri, Apr 22, 2022 at 03:35:02PM +0800, Ong Boon Leong wrote:
> @@ -774,6 +788,58 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs, unsigned int mode)
> return ret;
> }
>
> +static int xpcs_config_aneg_c37_1000basex(struct dw_xpcs *xpcs, unsigned int mode,
> + const unsigned long *advertising)
> +{
> + int ret, mdio_ctrl;
> +
> + /* For AN for 1000BASE-X mode, the settings are :-
> + * 1) VR_MII_MMD_CTRL Bit(12) [AN_ENABLE] = 0b (Disable C37 AN in case
> + * it is already enabled)
> + * 2) VR_MII_AN_CTRL Bit(2:1)[PCS_MODE] = 00b (1000BASE-X C37)
> + * 3) SR_MII_AN_ADV Bit(6)[FD] = 1b (Full Duplex)
> + * Note: Half Duplex is rarely used, so don't advertise.
> + * 4) VR_MII_MMD_CTRL Bit(12) [AN_ENABLE] = 1b (Enable C37 AN)
So if this function gets called to update the advertisement - even if
there is no actual change - we go through a AN-disable..AN-enable
dance and cause the link to re-negotiate. That doesn't sound like nice
behaviour.
> + */
> + mdio_ctrl = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL);
> + if (mdio_ctrl < 0)
> + return mdio_ctrl;
> +
> + if (mdio_ctrl & AN_CL37_EN) {
> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL,
> + mdio_ctrl & ~AN_CL37_EN);
> + if (ret < 0)
> + return ret;
> + }
> +
> + ret = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL);
> + if (ret < 0)
> + return ret;
> +
> + ret &= ~DW_VR_MII_PCS_MODE_MASK;
> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL, ret);
> + if (ret < 0)
> + return ret;
> +
> + ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE);
> + ret |= ADVERTISE_1000XFULL;
> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE, ret);
What if other bits are already set in the MII_ADVERTISE register?
Maybe consider using phylink_mii_c22_pcs_encode_advertisement()?
The pcs_config() method is also supposed to return either a negative
error, 0 for no advertisement change, or positive for an advertisement
change, in which case phylink will trigger a call to pcs_an_restart().
> +static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
> + struct phylink_link_state *state)
> +{
> + int lpa, adv;
> + int ret;
> +
> + ret = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL);
> + if (ret < 0)
> + return ret;
> +
> + if (ret & AN_CL37_EN) {
> + /* Reset link_state */
> + state->link = false;
> + state->speed = SPEED_UNKNOWN;
> + state->duplex = DUPLEX_UNKNOWN;
> + state->pause = 0;
Phylink guarantees that speed, duplex and pause are set to something
sensible - please remove these. The only one you probably need here
is state->link.
> +
> + lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_LPA);
> + if (lpa < 0 || lpa & LPA_RFAULT)
> + return false;
This function does not return a boolean. Returning "false" is the same
as returning 0, which means "no error" but an error has occurred.
> +
> + adv = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE);
> + if (adv < 0)
> + return false;
Ditto.
> +
> + if (lpa & ADVERTISE_1000XFULL &&
> + adv & ADVERTISE_1000XFULL) {
> + state->speed = SPEED_1000;
> + state->duplex = DUPLEX_FULL;
> + state->link = true;
> + }
> +
> + /* Clear CL37 AN complete status */
> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
> + } else {
> + state->link = true;
> + state->speed = SPEED_1000;
> + state->duplex = DUPLEX_FULL;
> + state->pause = 0;
If we're in AN-disabled mode, phylink will set state->speed and
state->duplex according to the user's parameters, so there should be no
need to do it here.
> @@ -994,9 +1143,21 @@ void xpcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
> return xpcs_config_usxgmii(xpcs, speed);
> if (interface == PHY_INTERFACE_MODE_SGMII)
> return xpcs_link_up_sgmii(xpcs, mode, speed, duplex);
> + if (interface == PHY_INTERFACE_MODE_1000BASEX)
> + return xpcs_link_up_1000basex(xpcs, speed, duplex);
> }
> EXPORT_SYMBOL_GPL(xpcs_link_up);
>
> +static void xpcs_an_restart(struct phylink_pcs *pcs)
> +{
> + struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
> + int ret;
> +
> + ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1);
> + ret |= BMCR_ANRESTART;
> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1, ret);
If xpcs_read() returns an error, we try to write the error back to
the control register? Is that a good idea/
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Hi Ong,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Ong-Boon-Leong/pcs-xpcs-stmmac-add-1000BASE-X-AN-for-network-switch/20220422-154446
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 9c8774e629a1950c24b44e3c8fb93d76fb644b49
config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220423/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5bd87350a5ae429baf8f373cb226a57b62f87280)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/dc88c5b7c183eeaff9db0e88d7b0d1d7f73e830b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ong-Boon-Leong/pcs-xpcs-stmmac-add-1000BASE-X-AN-for-network-switch/20220422-154446
git checkout dc88c5b7c183eeaff9db0e88d7b0d1d7f73e830b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/dsa/sja1105/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/net/dsa/sja1105/sja1105_main.c:2334:52: error: too few arguments to function call, expected 4, have 3
rc = xpcs_do_config(xpcs, priv->phy_mode[i], mode);
~~~~~~~~~~~~~~ ^
include/linux/pcs/pcs-xpcs.h:33:5: note: 'xpcs_do_config' declared here
int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
^
1 error generated.
vim +2334 drivers/net/dsa/sja1105/sja1105_main.c
2eea1fa82f681b Vladimir Oltean 2019-11-12 2224
6666cebc5e306f Vladimir Oltean 2019-05-02 2225 /* For situations where we need to change a setting at runtime that is only
6666cebc5e306f Vladimir Oltean 2019-05-02 2226 * available through the static configuration, resetting the switch in order
6666cebc5e306f Vladimir Oltean 2019-05-02 2227 * to upload the new static config is unavoidable. Back up the settings we
6666cebc5e306f Vladimir Oltean 2019-05-02 2228 * modify at runtime (currently only MAC) and restore them after uploading,
6666cebc5e306f Vladimir Oltean 2019-05-02 2229 * such that this operation is relatively seamless.
6666cebc5e306f Vladimir Oltean 2019-05-02 2230 */
2eea1fa82f681b Vladimir Oltean 2019-11-12 2231 int sja1105_static_config_reload(struct sja1105_private *priv,
2eea1fa82f681b Vladimir Oltean 2019-11-12 2232 enum sja1105_reset_reason reason)
6666cebc5e306f Vladimir Oltean 2019-05-02 2233 {
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2234 struct ptp_system_timestamp ptp_sts_before;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2235 struct ptp_system_timestamp ptp_sts_after;
82760d7f2ea638 Vladimir Oltean 2021-05-24 2236 int speed_mbps[SJA1105_MAX_NUM_PORTS];
84db00f2c04338 Vladimir Oltean 2021-05-31 2237 u16 bmcr[SJA1105_MAX_NUM_PORTS] = {0};
6666cebc5e306f Vladimir Oltean 2019-05-02 2238 struct sja1105_mac_config_entry *mac;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2239 struct dsa_switch *ds = priv->ds;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2240 s64 t1, t2, t3, t4;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2241 s64 t12, t34;
6666cebc5e306f Vladimir Oltean 2019-05-02 2242 int rc, i;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2243 s64 now;
6666cebc5e306f Vladimir Oltean 2019-05-02 2244
af580ae2dcb250 Vladimir Oltean 2019-11-09 2245 mutex_lock(&priv->mgmt_lock);
af580ae2dcb250 Vladimir Oltean 2019-11-09 2246
6666cebc5e306f Vladimir Oltean 2019-05-02 2247 mac = priv->static_config.tables[BLK_IDX_MAC_CONFIG].entries;
6666cebc5e306f Vladimir Oltean 2019-05-02 2248
8400cff60b472c Vladimir Oltean 2019-06-08 2249 /* Back up the dynamic link speed changed by sja1105_adjust_port_config
8400cff60b472c Vladimir Oltean 2019-06-08 2250 * in order to temporarily restore it to SJA1105_SPEED_AUTO - which the
8400cff60b472c Vladimir Oltean 2019-06-08 2251 * switch wants to see in the static config in order to allow us to
8400cff60b472c Vladimir Oltean 2019-06-08 2252 * change it through the dynamic interface later.
6666cebc5e306f Vladimir Oltean 2019-05-02 2253 */
542043e91df452 Vladimir Oltean 2021-05-24 2254 for (i = 0; i < ds->num_ports; i++) {
3ad1d171548e85 Vladimir Oltean 2021-06-11 2255 u32 reg_addr = mdiobus_c45_addr(MDIO_MMD_VEND2, MDIO_CTRL1);
3ad1d171548e85 Vladimir Oltean 2021-06-11 2256
41fed17fdbe531 Vladimir Oltean 2021-05-31 2257 speed_mbps[i] = sja1105_port_speed_to_ethtool(priv,
41fed17fdbe531 Vladimir Oltean 2021-05-31 2258 mac[i].speed);
41fed17fdbe531 Vladimir Oltean 2021-05-31 2259 mac[i].speed = priv->info->port_speed[SJA1105_SPEED_AUTO];
6666cebc5e306f Vladimir Oltean 2019-05-02 2260
3ad1d171548e85 Vladimir Oltean 2021-06-11 2261 if (priv->xpcs[i])
3ad1d171548e85 Vladimir Oltean 2021-06-11 2262 bmcr[i] = mdiobus_read(priv->mdio_pcs, i, reg_addr);
84db00f2c04338 Vladimir Oltean 2021-05-31 2263 }
ffe10e679cec9a Vladimir Oltean 2020-03-20 2264
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2265 /* No PTP operations can run right now */
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2266 mutex_lock(&priv->ptp_data.lock);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2267
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2268 rc = __sja1105_ptp_gettimex(ds, &now, &ptp_sts_before);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2269 if (rc < 0) {
61c77533b82ba8 Vladimir Oltean 2021-06-18 2270 mutex_unlock(&priv->ptp_data.lock);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2271 goto out;
61c77533b82ba8 Vladimir Oltean 2021-06-18 2272 }
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2273
6666cebc5e306f Vladimir Oltean 2019-05-02 2274 /* Reset switch and send updated static configuration */
6666cebc5e306f Vladimir Oltean 2019-05-02 2275 rc = sja1105_static_config_upload(priv);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2276 if (rc < 0) {
61c77533b82ba8 Vladimir Oltean 2021-06-18 2277 mutex_unlock(&priv->ptp_data.lock);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2278 goto out;
61c77533b82ba8 Vladimir Oltean 2021-06-18 2279 }
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2280
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2281 rc = __sja1105_ptp_settime(ds, 0, &ptp_sts_after);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2282 if (rc < 0) {
61c77533b82ba8 Vladimir Oltean 2021-06-18 2283 mutex_unlock(&priv->ptp_data.lock);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2284 goto out;
61c77533b82ba8 Vladimir Oltean 2021-06-18 2285 }
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2286
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2287 t1 = timespec64_to_ns(&ptp_sts_before.pre_ts);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2288 t2 = timespec64_to_ns(&ptp_sts_before.post_ts);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2289 t3 = timespec64_to_ns(&ptp_sts_after.pre_ts);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2290 t4 = timespec64_to_ns(&ptp_sts_after.post_ts);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2291 /* Mid point, corresponds to pre-reset PTPCLKVAL */
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2292 t12 = t1 + (t2 - t1) / 2;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2293 /* Mid point, corresponds to post-reset PTPCLKVAL, aka 0 */
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2294 t34 = t3 + (t4 - t3) / 2;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2295 /* Advance PTPCLKVAL by the time it took since its readout */
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2296 now += (t34 - t12);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2297
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2298 __sja1105_ptp_adjtime(ds, now);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2299
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2300 mutex_unlock(&priv->ptp_data.lock);
6666cebc5e306f Vladimir Oltean 2019-05-02 2301
2eea1fa82f681b Vladimir Oltean 2019-11-12 2302 dev_info(priv->ds->dev,
2eea1fa82f681b Vladimir Oltean 2019-11-12 2303 "Reset switch and programmed static config. Reason: %s\n",
2eea1fa82f681b Vladimir Oltean 2019-11-12 2304 sja1105_reset_reasons[reason]);
2eea1fa82f681b Vladimir Oltean 2019-11-12 2305
6666cebc5e306f Vladimir Oltean 2019-05-02 2306 /* Configure the CGU (PLLs) for MII and RMII PHYs.
6666cebc5e306f Vladimir Oltean 2019-05-02 2307 * For these interfaces there is no dynamic configuration
6666cebc5e306f Vladimir Oltean 2019-05-02 2308 * needed, since PLLs have same settings at all speeds.
6666cebc5e306f Vladimir Oltean 2019-05-02 2309 */
cb5a82d2b9aaca Vladimir Oltean 2021-06-18 2310 if (priv->info->clocking_setup) {
c50376783f23ff Vladimir Oltean 2021-05-24 2311 rc = priv->info->clocking_setup(priv);
6666cebc5e306f Vladimir Oltean 2019-05-02 2312 if (rc < 0)
6666cebc5e306f Vladimir Oltean 2019-05-02 2313 goto out;
cb5a82d2b9aaca Vladimir Oltean 2021-06-18 2314 }
6666cebc5e306f Vladimir Oltean 2019-05-02 2315
542043e91df452 Vladimir Oltean 2021-05-24 2316 for (i = 0; i < ds->num_ports; i++) {
3ad1d171548e85 Vladimir Oltean 2021-06-11 2317 struct dw_xpcs *xpcs = priv->xpcs[i];
3ad1d171548e85 Vladimir Oltean 2021-06-11 2318 unsigned int mode;
84db00f2c04338 Vladimir Oltean 2021-05-31 2319
8400cff60b472c Vladimir Oltean 2019-06-08 2320 rc = sja1105_adjust_port_config(priv, i, speed_mbps[i]);
6666cebc5e306f Vladimir Oltean 2019-05-02 2321 if (rc < 0)
6666cebc5e306f Vladimir Oltean 2019-05-02 2322 goto out;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2323
3ad1d171548e85 Vladimir Oltean 2021-06-11 2324 if (!xpcs)
84db00f2c04338 Vladimir Oltean 2021-05-31 2325 continue;
84db00f2c04338 Vladimir Oltean 2021-05-31 2326
3ad1d171548e85 Vladimir Oltean 2021-06-11 2327 if (bmcr[i] & BMCR_ANENABLE)
3ad1d171548e85 Vladimir Oltean 2021-06-11 2328 mode = MLO_AN_INBAND;
3ad1d171548e85 Vladimir Oltean 2021-06-11 2329 else if (priv->fixed_link[i])
3ad1d171548e85 Vladimir Oltean 2021-06-11 2330 mode = MLO_AN_FIXED;
3ad1d171548e85 Vladimir Oltean 2021-06-11 2331 else
3ad1d171548e85 Vladimir Oltean 2021-06-11 2332 mode = MLO_AN_PHY;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2333
3ad1d171548e85 Vladimir Oltean 2021-06-11 @2334 rc = xpcs_do_config(xpcs, priv->phy_mode[i], mode);
3ad1d171548e85 Vladimir Oltean 2021-06-11 2335 if (rc < 0)
3ad1d171548e85 Vladimir Oltean 2021-06-11 2336 goto out;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2337
3ad1d171548e85 Vladimir Oltean 2021-06-11 2338 if (!phylink_autoneg_inband(mode)) {
ffe10e679cec9a Vladimir Oltean 2020-03-20 2339 int speed = SPEED_UNKNOWN;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2340
56b63466333b25 Vladimir Oltean 2021-06-11 2341 if (priv->phy_mode[i] == PHY_INTERFACE_MODE_2500BASEX)
56b63466333b25 Vladimir Oltean 2021-06-11 2342 speed = SPEED_2500;
56b63466333b25 Vladimir Oltean 2021-06-11 2343 else if (bmcr[i] & BMCR_SPEED1000)
ffe10e679cec9a Vladimir Oltean 2020-03-20 2344 speed = SPEED_1000;
84db00f2c04338 Vladimir Oltean 2021-05-31 2345 else if (bmcr[i] & BMCR_SPEED100)
ffe10e679cec9a Vladimir Oltean 2020-03-20 2346 speed = SPEED_100;
053d8ad10d585a Vladimir Oltean 2021-03-04 2347 else
ffe10e679cec9a Vladimir Oltean 2020-03-20 2348 speed = SPEED_10;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2349
3ad1d171548e85 Vladimir Oltean 2021-06-11 2350 xpcs_link_up(&xpcs->pcs, mode, priv->phy_mode[i],
3ad1d171548e85 Vladimir Oltean 2021-06-11 2351 speed, DUPLEX_FULL);
ffe10e679cec9a Vladimir Oltean 2020-03-20 2352 }
ffe10e679cec9a Vladimir Oltean 2020-03-20 2353 }
4d7525085a9ba8 Vladimir Oltean 2020-05-28 2354
4d7525085a9ba8 Vladimir Oltean 2020-05-28 2355 rc = sja1105_reload_cbs(priv);
4d7525085a9ba8 Vladimir Oltean 2020-05-28 2356 if (rc < 0)
4d7525085a9ba8 Vladimir Oltean 2020-05-28 2357 goto out;
6666cebc5e306f Vladimir Oltean 2019-05-02 2358 out:
af580ae2dcb250 Vladimir Oltean 2019-11-09 2359 mutex_unlock(&priv->mgmt_lock);
af580ae2dcb250 Vladimir Oltean 2019-11-09 2360
6666cebc5e306f Vladimir Oltean 2019-05-02 2361 return rc;
6666cebc5e306f Vladimir Oltean 2019-05-02 2362 }
6666cebc5e306f Vladimir Oltean 2019-05-02 2363
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Ong,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Ong-Boon-Leong/pcs-xpcs-stmmac-add-1000BASE-X-AN-for-network-switch/20220422-154446
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 9c8774e629a1950c24b44e3c8fb93d76fb644b49
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220423/[email protected]/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/dc88c5b7c183eeaff9db0e88d7b0d1d7f73e830b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Ong-Boon-Leong/pcs-xpcs-stmmac-add-1000BASE-X-AN-for-network-switch/20220422-154446
git checkout dc88c5b7c183eeaff9db0e88d7b0d1d7f73e830b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers/net/dsa/sja1105/sja1105_main.c: In function 'sja1105_static_config_reload':
>> drivers/net/dsa/sja1105/sja1105_main.c:2334:22: error: too few arguments to function 'xpcs_do_config'
2334 | rc = xpcs_do_config(xpcs, priv->phy_mode[i], mode);
| ^~~~~~~~~~~~~~
In file included from drivers/net/dsa/sja1105/sja1105_main.c:19:
include/linux/pcs/pcs-xpcs.h:33:5: note: declared here
33 | int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
| ^~~~~~~~~~~~~~
vim +/xpcs_do_config +2334 drivers/net/dsa/sja1105/sja1105_main.c
2eea1fa82f681b Vladimir Oltean 2019-11-12 2224
6666cebc5e306f Vladimir Oltean 2019-05-02 2225 /* For situations where we need to change a setting at runtime that is only
6666cebc5e306f Vladimir Oltean 2019-05-02 2226 * available through the static configuration, resetting the switch in order
6666cebc5e306f Vladimir Oltean 2019-05-02 2227 * to upload the new static config is unavoidable. Back up the settings we
6666cebc5e306f Vladimir Oltean 2019-05-02 2228 * modify at runtime (currently only MAC) and restore them after uploading,
6666cebc5e306f Vladimir Oltean 2019-05-02 2229 * such that this operation is relatively seamless.
6666cebc5e306f Vladimir Oltean 2019-05-02 2230 */
2eea1fa82f681b Vladimir Oltean 2019-11-12 2231 int sja1105_static_config_reload(struct sja1105_private *priv,
2eea1fa82f681b Vladimir Oltean 2019-11-12 2232 enum sja1105_reset_reason reason)
6666cebc5e306f Vladimir Oltean 2019-05-02 2233 {
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2234 struct ptp_system_timestamp ptp_sts_before;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2235 struct ptp_system_timestamp ptp_sts_after;
82760d7f2ea638 Vladimir Oltean 2021-05-24 2236 int speed_mbps[SJA1105_MAX_NUM_PORTS];
84db00f2c04338 Vladimir Oltean 2021-05-31 2237 u16 bmcr[SJA1105_MAX_NUM_PORTS] = {0};
6666cebc5e306f Vladimir Oltean 2019-05-02 2238 struct sja1105_mac_config_entry *mac;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2239 struct dsa_switch *ds = priv->ds;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2240 s64 t1, t2, t3, t4;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2241 s64 t12, t34;
6666cebc5e306f Vladimir Oltean 2019-05-02 2242 int rc, i;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2243 s64 now;
6666cebc5e306f Vladimir Oltean 2019-05-02 2244
af580ae2dcb250 Vladimir Oltean 2019-11-09 2245 mutex_lock(&priv->mgmt_lock);
af580ae2dcb250 Vladimir Oltean 2019-11-09 2246
6666cebc5e306f Vladimir Oltean 2019-05-02 2247 mac = priv->static_config.tables[BLK_IDX_MAC_CONFIG].entries;
6666cebc5e306f Vladimir Oltean 2019-05-02 2248
8400cff60b472c Vladimir Oltean 2019-06-08 2249 /* Back up the dynamic link speed changed by sja1105_adjust_port_config
8400cff60b472c Vladimir Oltean 2019-06-08 2250 * in order to temporarily restore it to SJA1105_SPEED_AUTO - which the
8400cff60b472c Vladimir Oltean 2019-06-08 2251 * switch wants to see in the static config in order to allow us to
8400cff60b472c Vladimir Oltean 2019-06-08 2252 * change it through the dynamic interface later.
6666cebc5e306f Vladimir Oltean 2019-05-02 2253 */
542043e91df452 Vladimir Oltean 2021-05-24 2254 for (i = 0; i < ds->num_ports; i++) {
3ad1d171548e85 Vladimir Oltean 2021-06-11 2255 u32 reg_addr = mdiobus_c45_addr(MDIO_MMD_VEND2, MDIO_CTRL1);
3ad1d171548e85 Vladimir Oltean 2021-06-11 2256
41fed17fdbe531 Vladimir Oltean 2021-05-31 2257 speed_mbps[i] = sja1105_port_speed_to_ethtool(priv,
41fed17fdbe531 Vladimir Oltean 2021-05-31 2258 mac[i].speed);
41fed17fdbe531 Vladimir Oltean 2021-05-31 2259 mac[i].speed = priv->info->port_speed[SJA1105_SPEED_AUTO];
6666cebc5e306f Vladimir Oltean 2019-05-02 2260
3ad1d171548e85 Vladimir Oltean 2021-06-11 2261 if (priv->xpcs[i])
3ad1d171548e85 Vladimir Oltean 2021-06-11 2262 bmcr[i] = mdiobus_read(priv->mdio_pcs, i, reg_addr);
84db00f2c04338 Vladimir Oltean 2021-05-31 2263 }
ffe10e679cec9a Vladimir Oltean 2020-03-20 2264
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2265 /* No PTP operations can run right now */
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2266 mutex_lock(&priv->ptp_data.lock);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2267
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2268 rc = __sja1105_ptp_gettimex(ds, &now, &ptp_sts_before);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2269 if (rc < 0) {
61c77533b82ba8 Vladimir Oltean 2021-06-18 2270 mutex_unlock(&priv->ptp_data.lock);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2271 goto out;
61c77533b82ba8 Vladimir Oltean 2021-06-18 2272 }
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2273
6666cebc5e306f Vladimir Oltean 2019-05-02 2274 /* Reset switch and send updated static configuration */
6666cebc5e306f Vladimir Oltean 2019-05-02 2275 rc = sja1105_static_config_upload(priv);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2276 if (rc < 0) {
61c77533b82ba8 Vladimir Oltean 2021-06-18 2277 mutex_unlock(&priv->ptp_data.lock);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2278 goto out;
61c77533b82ba8 Vladimir Oltean 2021-06-18 2279 }
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2280
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2281 rc = __sja1105_ptp_settime(ds, 0, &ptp_sts_after);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2282 if (rc < 0) {
61c77533b82ba8 Vladimir Oltean 2021-06-18 2283 mutex_unlock(&priv->ptp_data.lock);
61c77533b82ba8 Vladimir Oltean 2021-06-18 2284 goto out;
61c77533b82ba8 Vladimir Oltean 2021-06-18 2285 }
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2286
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2287 t1 = timespec64_to_ns(&ptp_sts_before.pre_ts);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2288 t2 = timespec64_to_ns(&ptp_sts_before.post_ts);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2289 t3 = timespec64_to_ns(&ptp_sts_after.pre_ts);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2290 t4 = timespec64_to_ns(&ptp_sts_after.post_ts);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2291 /* Mid point, corresponds to pre-reset PTPCLKVAL */
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2292 t12 = t1 + (t2 - t1) / 2;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2293 /* Mid point, corresponds to post-reset PTPCLKVAL, aka 0 */
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2294 t34 = t3 + (t4 - t3) / 2;
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2295 /* Advance PTPCLKVAL by the time it took since its readout */
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2296 now += (t34 - t12);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2297
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2298 __sja1105_ptp_adjtime(ds, now);
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2299
6cf99c13ea07b5 Vladimir Oltean 2019-11-09 2300 mutex_unlock(&priv->ptp_data.lock);
6666cebc5e306f Vladimir Oltean 2019-05-02 2301
2eea1fa82f681b Vladimir Oltean 2019-11-12 2302 dev_info(priv->ds->dev,
2eea1fa82f681b Vladimir Oltean 2019-11-12 2303 "Reset switch and programmed static config. Reason: %s\n",
2eea1fa82f681b Vladimir Oltean 2019-11-12 2304 sja1105_reset_reasons[reason]);
2eea1fa82f681b Vladimir Oltean 2019-11-12 2305
6666cebc5e306f Vladimir Oltean 2019-05-02 2306 /* Configure the CGU (PLLs) for MII and RMII PHYs.
6666cebc5e306f Vladimir Oltean 2019-05-02 2307 * For these interfaces there is no dynamic configuration
6666cebc5e306f Vladimir Oltean 2019-05-02 2308 * needed, since PLLs have same settings at all speeds.
6666cebc5e306f Vladimir Oltean 2019-05-02 2309 */
cb5a82d2b9aaca Vladimir Oltean 2021-06-18 2310 if (priv->info->clocking_setup) {
c50376783f23ff Vladimir Oltean 2021-05-24 2311 rc = priv->info->clocking_setup(priv);
6666cebc5e306f Vladimir Oltean 2019-05-02 2312 if (rc < 0)
6666cebc5e306f Vladimir Oltean 2019-05-02 2313 goto out;
cb5a82d2b9aaca Vladimir Oltean 2021-06-18 2314 }
6666cebc5e306f Vladimir Oltean 2019-05-02 2315
542043e91df452 Vladimir Oltean 2021-05-24 2316 for (i = 0; i < ds->num_ports; i++) {
3ad1d171548e85 Vladimir Oltean 2021-06-11 2317 struct dw_xpcs *xpcs = priv->xpcs[i];
3ad1d171548e85 Vladimir Oltean 2021-06-11 2318 unsigned int mode;
84db00f2c04338 Vladimir Oltean 2021-05-31 2319
8400cff60b472c Vladimir Oltean 2019-06-08 2320 rc = sja1105_adjust_port_config(priv, i, speed_mbps[i]);
6666cebc5e306f Vladimir Oltean 2019-05-02 2321 if (rc < 0)
6666cebc5e306f Vladimir Oltean 2019-05-02 2322 goto out;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2323
3ad1d171548e85 Vladimir Oltean 2021-06-11 2324 if (!xpcs)
84db00f2c04338 Vladimir Oltean 2021-05-31 2325 continue;
84db00f2c04338 Vladimir Oltean 2021-05-31 2326
3ad1d171548e85 Vladimir Oltean 2021-06-11 2327 if (bmcr[i] & BMCR_ANENABLE)
3ad1d171548e85 Vladimir Oltean 2021-06-11 2328 mode = MLO_AN_INBAND;
3ad1d171548e85 Vladimir Oltean 2021-06-11 2329 else if (priv->fixed_link[i])
3ad1d171548e85 Vladimir Oltean 2021-06-11 2330 mode = MLO_AN_FIXED;
3ad1d171548e85 Vladimir Oltean 2021-06-11 2331 else
3ad1d171548e85 Vladimir Oltean 2021-06-11 2332 mode = MLO_AN_PHY;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2333
3ad1d171548e85 Vladimir Oltean 2021-06-11 @2334 rc = xpcs_do_config(xpcs, priv->phy_mode[i], mode);
3ad1d171548e85 Vladimir Oltean 2021-06-11 2335 if (rc < 0)
3ad1d171548e85 Vladimir Oltean 2021-06-11 2336 goto out;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2337
3ad1d171548e85 Vladimir Oltean 2021-06-11 2338 if (!phylink_autoneg_inband(mode)) {
ffe10e679cec9a Vladimir Oltean 2020-03-20 2339 int speed = SPEED_UNKNOWN;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2340
56b63466333b25 Vladimir Oltean 2021-06-11 2341 if (priv->phy_mode[i] == PHY_INTERFACE_MODE_2500BASEX)
56b63466333b25 Vladimir Oltean 2021-06-11 2342 speed = SPEED_2500;
56b63466333b25 Vladimir Oltean 2021-06-11 2343 else if (bmcr[i] & BMCR_SPEED1000)
ffe10e679cec9a Vladimir Oltean 2020-03-20 2344 speed = SPEED_1000;
84db00f2c04338 Vladimir Oltean 2021-05-31 2345 else if (bmcr[i] & BMCR_SPEED100)
ffe10e679cec9a Vladimir Oltean 2020-03-20 2346 speed = SPEED_100;
053d8ad10d585a Vladimir Oltean 2021-03-04 2347 else
ffe10e679cec9a Vladimir Oltean 2020-03-20 2348 speed = SPEED_10;
ffe10e679cec9a Vladimir Oltean 2020-03-20 2349
3ad1d171548e85 Vladimir Oltean 2021-06-11 2350 xpcs_link_up(&xpcs->pcs, mode, priv->phy_mode[i],
3ad1d171548e85 Vladimir Oltean 2021-06-11 2351 speed, DUPLEX_FULL);
ffe10e679cec9a Vladimir Oltean 2020-03-20 2352 }
ffe10e679cec9a Vladimir Oltean 2020-03-20 2353 }
4d7525085a9ba8 Vladimir Oltean 2020-05-28 2354
4d7525085a9ba8 Vladimir Oltean 2020-05-28 2355 rc = sja1105_reload_cbs(priv);
4d7525085a9ba8 Vladimir Oltean 2020-05-28 2356 if (rc < 0)
4d7525085a9ba8 Vladimir Oltean 2020-05-28 2357 goto out;
6666cebc5e306f Vladimir Oltean 2019-05-02 2358 out:
af580ae2dcb250 Vladimir Oltean 2019-11-09 2359 mutex_unlock(&priv->mgmt_lock);
af580ae2dcb250 Vladimir Oltean 2019-11-09 2360
6666cebc5e306f Vladimir Oltean 2019-05-02 2361 return rc;
6666cebc5e306f Vladimir Oltean 2019-05-02 2362 }
6666cebc5e306f Vladimir Oltean 2019-05-02 2363
--
0-DAY CI Kernel Test Service
https://01.org/lkp
>On Fri, Apr 22, 2022 at 03:35:02PM +0800, Ong Boon Leong wrote:
>> @@ -774,6 +788,58 @@ static int xpcs_config_aneg_c37_sgmii(struct
>dw_xpcs *xpcs, unsigned int mode)
>> return ret;
>> }
>>
>> +static int xpcs_config_aneg_c37_1000basex(struct dw_xpcs *xpcs, unsigned
>int mode,
>> + const unsigned long *advertising)
>> +{
>> + int ret, mdio_ctrl;
>> +
>> + /* For AN for 1000BASE-X mode, the settings are :-
>> + * 1) VR_MII_MMD_CTRL Bit(12) [AN_ENABLE] = 0b (Disable C37 AN in
>case
>> + * it is already enabled)
>> + * 2) VR_MII_AN_CTRL Bit(2:1)[PCS_MODE] = 00b (1000BASE-X C37)
>> + * 3) SR_MII_AN_ADV Bit(6)[FD] = 1b (Full Duplex)
>> + * Note: Half Duplex is rarely used, so don't advertise.
>> + * 4) VR_MII_MMD_CTRL Bit(12) [AN_ENABLE] = 1b (Enable C37 AN)
>
>So if this function gets called to update the advertisement - even if
>there is no actual change - we go through a AN-disable..AN-enable
>dance and cause the link to re-negotiate. That doesn't sound like nice
>behaviour.
Good feedback. Will look into this part.
>
>> + */
>> + mdio_ctrl = xpcs_read(xpcs, MDIO_MMD_VEND2,
>DW_VR_MII_MMD_CTRL);
>> + if (mdio_ctrl < 0)
>> + return mdio_ctrl;
>> +
>> + if (mdio_ctrl & AN_CL37_EN) {
>> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2,
>DW_VR_MII_MMD_CTRL,
>> + mdio_ctrl & ~AN_CL37_EN);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + ret = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret &= ~DW_VR_MII_PCS_MODE_MASK;
>> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL,
>ret);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE);
>> + ret |= ADVERTISE_1000XFULL;
>> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE, ret);
>
>What if other bits are already set in the MII_ADVERTISE register?
>Maybe consider using phylink_mii_c22_pcs_encode_advertisement()?
The IP supports C45 MII MMD access and not C22. Your feedback does
make sense to strengthen the logics here. Thanks
>
>The pcs_config() method is also supposed to return either a negative
>error, 0 for no advertisement change, or positive for an advertisement
>change, in which case phylink will trigger a call to pcs_an_restart().
>
>> +static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
>> + struct phylink_link_state *state)
>> +{
>> + int lpa, adv;
>> + int ret;
>> +
>> + ret = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (ret & AN_CL37_EN) {
>> + /* Reset link_state */
>> + state->link = false;
>> + state->speed = SPEED_UNKNOWN;
>> + state->duplex = DUPLEX_UNKNOWN;
>> + state->pause = 0;
>
>Phylink guarantees that speed, duplex and pause are set to something
>sensible - please remove these. The only one you probably need here
>is state->link.
Thanks for the input here.
>
>> +
>> + lpa = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_LPA);
>> + if (lpa < 0 || lpa & LPA_RFAULT)
>> + return false;
>
>This function does not return a boolean. Returning "false" is the same
>as returning 0, which means "no error" but an error has occurred.
Good catch.
>
>> +
>> + adv = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE);
>> + if (adv < 0)
>> + return false;
>
>Ditto.
>
>> +
>> + if (lpa & ADVERTISE_1000XFULL &&
>> + adv & ADVERTISE_1000XFULL) {
>> + state->speed = SPEED_1000;
>> + state->duplex = DUPLEX_FULL;
>> + state->link = true;
>> + }
>> +
>> + /* Clear CL37 AN complete status */
>> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2,
>DW_VR_MII_AN_INTR_STS, 0);
>> + } else {
>> + state->link = true;
>> + state->speed = SPEED_1000;
>> + state->duplex = DUPLEX_FULL;
>> + state->pause = 0;
>
>If we're in AN-disabled mode, phylink will set state->speed and
>state->duplex according to the user's parameters, so there should be no
>need to do it here.
Thanks for the input.
>
>> @@ -994,9 +1143,21 @@ void xpcs_link_up(struct phylink_pcs *pcs,
>unsigned int mode,
>> return xpcs_config_usxgmii(xpcs, speed);
>> if (interface == PHY_INTERFACE_MODE_SGMII)
>> return xpcs_link_up_sgmii(xpcs, mode, speed, duplex);
>> + if (interface == PHY_INTERFACE_MODE_1000BASEX)
>> + return xpcs_link_up_1000basex(xpcs, speed, duplex);
>> }
>> EXPORT_SYMBOL_GPL(xpcs_link_up);
>>
>> +static void xpcs_an_restart(struct phylink_pcs *pcs)
>> +{
>> + struct dw_xpcs *xpcs = phylink_pcs_to_xpcs(pcs);
>> + int ret;
>> +
>> + ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1);
>> + ret |= BMCR_ANRESTART;
>> + ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1, ret);
>
>If xpcs_read() returns an error, we try to write the error back to
>the control register? Is that a good idea/
Thanks! I will fix this.