2023-11-15 14:09:40

by Luo Jie

[permalink] [raw]
Subject: [PATCH v3 0/6] add qca8084 ethernet phy driver

QCA8084 is four-port PHY with maximum link capability 2.5G,
which supports the interface mode qusgmii and sgmii mode,
there are two PCSs available to connected with ethernet port.

QCA8084 can work in switch mode or PHY mode.
For switch mode, both PCS0 and PCS1 work on sgmii mode.
For PHY mode, PCS1 works on qusgmii mode, the last port
(the fourth port) works on sgmii mode.

Besides this PHY driver patches, the PCS driver is also needed
to bring up the qca8084 device, which mainly configurs PCS
and clocks.

Changes in v3:
1. pick the two patches to introduce the interface mode
10g-qxgmii from Vladimir Oltean([email protected]).

2. add the function phydev_id_is_qca808x to identify the
PHY qca8081 and qca8084.

3. update the interface mode name PHY_INTERFACE_MODE_QUSGMII
to PHY_INTERFACE_MODE_10G_QXGMII.

Luo Jie (4):
net: phy: at803x: add QCA8084 ethernet phy support
net: phy: at803x: add the function phydev_id_is_qca808x
net: phy: at803x: Add qca8084_config_init function
net: phy: qca8084: add qca8084_link_change_notify

Vladimir Oltean (2):
net: phylink: move phylink_pcs_neg_mode() to phylink.c
net: phy: introduce core support for phy-mode = "10g-qxgmii"

.../bindings/net/ethernet-controller.yaml | 1 +
Documentation/networking/phy.rst | 6 +
drivers/net/phy/at803x.c | 130 +++++++++++++++++-
drivers/net/phy/phy-core.c | 1 +
drivers/net/phy/phylink.c | 77 ++++++++++-
include/linux/phy.h | 4 +
include/linux/phylink.h | 67 +--------
7 files changed, 212 insertions(+), 74 deletions(-)


base-commit: bc962b35b139dd52319e6fc0f4bab00593bf38c9
--
2.42.0


2023-11-15 14:09:52

by Luo Jie

[permalink] [raw]
Subject: [PATCH v3 1/6] net: phylink: move phylink_pcs_neg_mode() to phylink.c

From: Vladimir Oltean <[email protected]>

Russell points out that there is no user of phylink_pcs_neg_mode()
outside of phylink.c, nor is there planned to be any, so we can just
move it there.

Suggested-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
Signed-off-by: Luo Jie <[email protected]>
---
drivers/net/phy/phylink.c | 65 ++++++++++++++++++++++++++++++++++++++
include/linux/phylink.h | 66 ---------------------------------------
2 files changed, 65 insertions(+), 66 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 25c19496a336..162f51b0986a 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -162,6 +162,71 @@ static const char *phylink_an_mode_str(unsigned int mode)
return mode < ARRAY_SIZE(modestr) ? modestr[mode] : "unknown";
}

+/**
+ * phylink_pcs_neg_mode() - helper to determine PCS inband mode
+ * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
+ * @interface: interface mode to be used
+ * @advertising: adertisement ethtool link mode mask
+ *
+ * Determines the negotiation mode to be used by the PCS, and returns
+ * one of:
+ *
+ * - %PHYLINK_PCS_NEG_NONE: interface mode does not support inband
+ * - %PHYLINK_PCS_NEG_OUTBAND: an out of band mode (e.g. reading the PHY)
+ * will be used.
+ * - %PHYLINK_PCS_NEG_INBAND_DISABLED: inband mode selected but autoneg
+ * disabled
+ * - %PHYLINK_PCS_NEG_INBAND_ENABLED: inband mode selected and autoneg enabled
+ *
+ * Note: this is for cases where the PCS itself is involved in negotiation
+ * (e.g. Clause 37, SGMII and similar) not Clause 73.
+ */
+static unsigned int phylink_pcs_neg_mode(unsigned int mode, phy_interface_t interface,
+ const unsigned long *advertising)
+{
+ unsigned int neg_mode;
+
+ switch (interface) {
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_QSGMII:
+ case PHY_INTERFACE_MODE_QUSGMII:
+ case PHY_INTERFACE_MODE_USXGMII:
+ /* These protocols are designed for use with a PHY which
+ * communicates its negotiation result back to the MAC via
+ * inband communication. Note: there exist PHYs that run
+ * with SGMII but do not send the inband data.
+ */
+ if (!phylink_autoneg_inband(mode))
+ neg_mode = PHYLINK_PCS_NEG_OUTBAND;
+ else
+ neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
+ break;
+
+ case PHY_INTERFACE_MODE_1000BASEX:
+ case PHY_INTERFACE_MODE_2500BASEX:
+ /* 1000base-X is designed for use media-side for Fibre
+ * connections, and thus the Autoneg bit needs to be
+ * taken into account. We also do this for 2500base-X
+ * as well, but drivers may not support this, so may
+ * need to override this.
+ */
+ if (!phylink_autoneg_inband(mode))
+ neg_mode = PHYLINK_PCS_NEG_OUTBAND;
+ else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+ advertising))
+ neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
+ else
+ neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED;
+ break;
+
+ default:
+ neg_mode = PHYLINK_PCS_NEG_NONE;
+ break;
+ }
+
+ return neg_mode;
+}
+
static unsigned int phylink_interface_signal_rate(phy_interface_t interface)
{
switch (interface) {
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 875439ab45de..d589f89c612c 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -98,72 +98,6 @@ static inline bool phylink_autoneg_inband(unsigned int mode)
return mode == MLO_AN_INBAND;
}

-/**
- * phylink_pcs_neg_mode() - helper to determine PCS inband mode
- * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
- * @interface: interface mode to be used
- * @advertising: adertisement ethtool link mode mask
- *
- * Determines the negotiation mode to be used by the PCS, and returns
- * one of:
- *
- * - %PHYLINK_PCS_NEG_NONE: interface mode does not support inband
- * - %PHYLINK_PCS_NEG_OUTBAND: an out of band mode (e.g. reading the PHY)
- * will be used.
- * - %PHYLINK_PCS_NEG_INBAND_DISABLED: inband mode selected but autoneg
- * disabled
- * - %PHYLINK_PCS_NEG_INBAND_ENABLED: inband mode selected and autoneg enabled
- *
- * Note: this is for cases where the PCS itself is involved in negotiation
- * (e.g. Clause 37, SGMII and similar) not Clause 73.
- */
-static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
- phy_interface_t interface,
- const unsigned long *advertising)
-{
- unsigned int neg_mode;
-
- switch (interface) {
- case PHY_INTERFACE_MODE_SGMII:
- case PHY_INTERFACE_MODE_QSGMII:
- case PHY_INTERFACE_MODE_QUSGMII:
- case PHY_INTERFACE_MODE_USXGMII:
- /* These protocols are designed for use with a PHY which
- * communicates its negotiation result back to the MAC via
- * inband communication. Note: there exist PHYs that run
- * with SGMII but do not send the inband data.
- */
- if (!phylink_autoneg_inband(mode))
- neg_mode = PHYLINK_PCS_NEG_OUTBAND;
- else
- neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
- break;
-
- case PHY_INTERFACE_MODE_1000BASEX:
- case PHY_INTERFACE_MODE_2500BASEX:
- /* 1000base-X is designed for use media-side for Fibre
- * connections, and thus the Autoneg bit needs to be
- * taken into account. We also do this for 2500base-X
- * as well, but drivers may not support this, so may
- * need to override this.
- */
- if (!phylink_autoneg_inband(mode))
- neg_mode = PHYLINK_PCS_NEG_OUTBAND;
- else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
- advertising))
- neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
- else
- neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED;
- break;
-
- default:
- neg_mode = PHYLINK_PCS_NEG_NONE;
- break;
- }
-
- return neg_mode;
-}
-
/**
* struct phylink_link_state - link state structure
* @advertising: ethtool bitmask containing advertised link modes
--
2.42.0

2023-11-15 14:09:52

by Luo Jie

[permalink] [raw]
Subject: [PATCH v3 3/6] net: phy: at803x: add QCA8084 ethernet phy support

Add qca8084 PHY support, which is four-port PHY with maximum
link capability 2.5G, the features of each port is almost same
as QCA8081 and slave seed config is not needed.

Three kind of interface modes supported by qca8084.
PHY_INTERFACE_MODE_10G_QXGMII, PHY_INTERFACE_MODE_2500BASEX and
PHY_INTERFACE_MODE_SGMII.

The PCS(serdes) and clock are also needed to be configured to
bringup qca8084 PHY, which will be added in the pcs driver.

The additional CDT configurations used for qca8084.

Signed-off-by: Luo Jie <[email protected]>
---
drivers/net/phy/at803x.c | 48 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 37fb033e1c29..471d5c13d76d 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -176,6 +176,7 @@
#define AT8030_PHY_ID_MASK 0xffffffef

#define QCA8081_PHY_ID 0x004dd101
+#define QCA8084_PHY_ID 0x004dd180

#define QCA8327_A_PHY_ID 0x004dd033
#define QCA8327_B_PHY_ID 0x004dd034
@@ -1760,6 +1761,9 @@ static bool qca808x_is_prefer_master(struct phy_device *phydev)

static bool qca808x_has_fast_retrain_or_slave_seed(struct phy_device *phydev)
{
+ if (phydev_id_compare(phydev, QCA8084_PHY_ID))
+ return false;
+
return linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, phydev->supported);
}

@@ -1824,6 +1828,21 @@ static int qca808x_read_status(struct phy_device *phydev)
return ret;

if (phydev->link) {
+ /* There are two PCSs available for QCA8084, which support the following
+ * interface modes.
+ *
+ * 1. PHY_INTERFACE_MODE_10G_QXGMII utilizes PCS1 for all available 4 ports,
+ * which is for all link speeds.
+ *
+ * 2. PHY_INTERFACE_MODE_2500BASEX utilizes PCS0 for the fourth port,
+ * which is only for the link speed 2500M same as QCA8081.
+ *
+ * 3. PHY_INTERFACE_MODE_SGMII utilizes PCS0 for the fourth port,
+ * which is for the link speed 10M, 100M and 1000M same as QCA8081.
+ */
+ if (phydev->interface == PHY_INTERFACE_MODE_10G_QXGMII)
+ return 0;
+
if (phydev->speed == SPEED_2500)
phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
else
@@ -1958,6 +1977,14 @@ static int qca808x_cable_test_start(struct phy_device *phydev)
phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807a, 0xc060);
phy_write_mmd(phydev, MDIO_MMD_PCS, 0x807e, 0xb060);

+ if (phydev_id_compare(phydev, QCA8084_PHY_ID)) {
+ /* Adjust the positive and negative pulse thereshold of CDT */
+ phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8075, 0xa060);
+
+ /* Disable the near echo bypass */
+ phy_modify_mmd(phydev, MDIO_MMD_PCS, 0x807f, BIT(15), 0);
+ }
+
return 0;
}

@@ -2227,6 +2254,26 @@ static struct phy_driver at803x_driver[] = {
.cable_test_start = qca808x_cable_test_start,
.cable_test_get_status = qca808x_cable_test_get_status,
.link_change_notify = qca808x_link_change_notify,
+}, {
+ /* Qualcomm QCA8084 */
+ PHY_ID_MATCH_MODEL(QCA8084_PHY_ID),
+ .name = "Qualcomm QCA8084",
+ .flags = PHY_POLL_CABLE_TEST,
+ .probe = at803x_probe,
+ .config_intr = at803x_config_intr,
+ .handle_interrupt = at803x_handle_interrupt,
+ .get_tunable = at803x_get_tunable,
+ .set_tunable = at803x_set_tunable,
+ .set_wol = at803x_set_wol,
+ .get_wol = at803x_get_wol,
+ .get_features = qca808x_get_features,
+ .config_aneg = at803x_config_aneg,
+ .suspend = genphy_suspend,
+ .resume = genphy_resume,
+ .read_status = qca808x_read_status,
+ .soft_reset = qca808x_soft_reset,
+ .cable_test_start = qca808x_cable_test_start,
+ .cable_test_get_status = qca808x_cable_test_get_status,
}, };

module_phy_driver(at803x_driver);
@@ -2242,6 +2289,7 @@ static struct mdio_device_id __maybe_unused atheros_tbl[] = {
{ PHY_ID_MATCH_EXACT(QCA8327_B_PHY_ID) },
{ PHY_ID_MATCH_EXACT(QCA9561_PHY_ID) },
{ PHY_ID_MATCH_EXACT(QCA8081_PHY_ID) },
+ { PHY_ID_MATCH_MODEL(QCA8084_PHY_ID) },
{ }
};

--
2.42.0

2023-11-15 14:11:36

by Luo Jie

[permalink] [raw]
Subject: [PATCH v3 5/6] net: phy: at803x: Add qca8084_config_init function

Configure MSE detect threshold and ADC clock edge invert.

Signed-off-by: Luo Jie <[email protected]>
---
drivers/net/phy/at803x.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index f56202f5944d..06a068ca5539 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -280,6 +280,15 @@
#define QCA8081_PHY_SERDES_MMD1_FIFO_CTRL 0x9072
#define QCA8081_PHY_FIFO_RSTN BIT(11)

+/* QCA8084 ADC clock edge */
+#define QCA8084_ADC_CLK_SEL 0x8b80
+#define QCA8084_ADC_CLK_SEL_ACLK GENMASK(7, 4)
+#define QCA8084_ADC_CLK_SEL_ACLK_FALL 0xf
+#define QCA8084_ADC_CLK_SEL_ACLK_RISE 0x0
+
+#define QCA8084_MSE_THRESHOLD 0x800a
+#define QCA8084_MSE_THRESHOLD_2P5G_VAL 0x51c6
+
MODULE_DESCRIPTION("Qualcomm Atheros AR803x and QCA808X PHY driver");
MODULE_AUTHOR("Matus Ujhelyi");
MODULE_LICENSE("GPL");
@@ -2083,6 +2092,23 @@ static void qca808x_link_change_notify(struct phy_device *phydev)
QCA8081_PHY_FIFO_RSTN, phydev->link ? QCA8081_PHY_FIFO_RSTN : 0);
}

+static int qca8084_config_init(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Invert ADC clock edge */
+ ret = at803x_debug_reg_mask(phydev, QCA8084_ADC_CLK_SEL,
+ QCA8084_ADC_CLK_SEL_ACLK,
+ FIELD_PREP(QCA8084_ADC_CLK_SEL_ACLK,
+ QCA8084_ADC_CLK_SEL_ACLK_FALL));
+ if (ret < 0)
+ return ret;
+
+ /* Adjust MSE threshold value to avoid link issue with some link partner */
+ return phy_write_mmd(phydev, MDIO_MMD_PMAPMD,
+ QCA8084_MSE_THRESHOLD, QCA8084_MSE_THRESHOLD_2P5G_VAL);
+}
+
static struct phy_driver at803x_driver[] = {
{
/* Qualcomm Atheros AR8035 */
@@ -2280,6 +2306,7 @@ static struct phy_driver at803x_driver[] = {
.soft_reset = qca808x_soft_reset,
.cable_test_start = qca808x_cable_test_start,
.cable_test_get_status = qca808x_cable_test_get_status,
+ .config_init = qca8084_config_init,
}, };

module_phy_driver(at803x_driver);
--
2.42.0

2023-11-15 14:15:41

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] net: phylink: move phylink_pcs_neg_mode() to phylink.c

Hi,

You don't need this patch for your series, and you're bypassing my
ability to decide when this patch should be merged (which is not yet,
I want things to remain as-is for another cycle.)

In theory, looking at past history, 6.7 will probably be a LTS kernel,
but until that is known for certain, I don't want to commit to moving
this function in case LTS gets delayed by a cycle.

Please drop it from your series.

Thanks.

On Wed, Nov 15, 2023 at 10:06:25PM +0800, Luo Jie wrote:
> From: Vladimir Oltean <[email protected]>
>
> Russell points out that there is no user of phylink_pcs_neg_mode()
> outside of phylink.c, nor is there planned to be any, so we can just
> move it there.
>
> Suggested-by: Russell King (Oracle) <[email protected]>
> Signed-off-by: Vladimir Oltean <[email protected]>
> Signed-off-by: Luo Jie <[email protected]>
> ---
> drivers/net/phy/phylink.c | 65 ++++++++++++++++++++++++++++++++++++++
> include/linux/phylink.h | 66 ---------------------------------------
> 2 files changed, 65 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 25c19496a336..162f51b0986a 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -162,6 +162,71 @@ static const char *phylink_an_mode_str(unsigned int mode)
> return mode < ARRAY_SIZE(modestr) ? modestr[mode] : "unknown";
> }
>
> +/**
> + * phylink_pcs_neg_mode() - helper to determine PCS inband mode
> + * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
> + * @interface: interface mode to be used
> + * @advertising: adertisement ethtool link mode mask
> + *
> + * Determines the negotiation mode to be used by the PCS, and returns
> + * one of:
> + *
> + * - %PHYLINK_PCS_NEG_NONE: interface mode does not support inband
> + * - %PHYLINK_PCS_NEG_OUTBAND: an out of band mode (e.g. reading the PHY)
> + * will be used.
> + * - %PHYLINK_PCS_NEG_INBAND_DISABLED: inband mode selected but autoneg
> + * disabled
> + * - %PHYLINK_PCS_NEG_INBAND_ENABLED: inband mode selected and autoneg enabled
> + *
> + * Note: this is for cases where the PCS itself is involved in negotiation
> + * (e.g. Clause 37, SGMII and similar) not Clause 73.
> + */
> +static unsigned int phylink_pcs_neg_mode(unsigned int mode, phy_interface_t interface,
> + const unsigned long *advertising)
> +{
> + unsigned int neg_mode;
> +
> + switch (interface) {
> + case PHY_INTERFACE_MODE_SGMII:
> + case PHY_INTERFACE_MODE_QSGMII:
> + case PHY_INTERFACE_MODE_QUSGMII:
> + case PHY_INTERFACE_MODE_USXGMII:
> + /* These protocols are designed for use with a PHY which
> + * communicates its negotiation result back to the MAC via
> + * inband communication. Note: there exist PHYs that run
> + * with SGMII but do not send the inband data.
> + */
> + if (!phylink_autoneg_inband(mode))
> + neg_mode = PHYLINK_PCS_NEG_OUTBAND;
> + else
> + neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
> + break;
> +
> + case PHY_INTERFACE_MODE_1000BASEX:
> + case PHY_INTERFACE_MODE_2500BASEX:
> + /* 1000base-X is designed for use media-side for Fibre
> + * connections, and thus the Autoneg bit needs to be
> + * taken into account. We also do this for 2500base-X
> + * as well, but drivers may not support this, so may
> + * need to override this.
> + */
> + if (!phylink_autoneg_inband(mode))
> + neg_mode = PHYLINK_PCS_NEG_OUTBAND;
> + else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> + advertising))
> + neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
> + else
> + neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED;
> + break;
> +
> + default:
> + neg_mode = PHYLINK_PCS_NEG_NONE;
> + break;
> + }
> +
> + return neg_mode;
> +}
> +
> static unsigned int phylink_interface_signal_rate(phy_interface_t interface)
> {
> switch (interface) {
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index 875439ab45de..d589f89c612c 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -98,72 +98,6 @@ static inline bool phylink_autoneg_inband(unsigned int mode)
> return mode == MLO_AN_INBAND;
> }
>
> -/**
> - * phylink_pcs_neg_mode() - helper to determine PCS inband mode
> - * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
> - * @interface: interface mode to be used
> - * @advertising: adertisement ethtool link mode mask
> - *
> - * Determines the negotiation mode to be used by the PCS, and returns
> - * one of:
> - *
> - * - %PHYLINK_PCS_NEG_NONE: interface mode does not support inband
> - * - %PHYLINK_PCS_NEG_OUTBAND: an out of band mode (e.g. reading the PHY)
> - * will be used.
> - * - %PHYLINK_PCS_NEG_INBAND_DISABLED: inband mode selected but autoneg
> - * disabled
> - * - %PHYLINK_PCS_NEG_INBAND_ENABLED: inband mode selected and autoneg enabled
> - *
> - * Note: this is for cases where the PCS itself is involved in negotiation
> - * (e.g. Clause 37, SGMII and similar) not Clause 73.
> - */
> -static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
> - phy_interface_t interface,
> - const unsigned long *advertising)
> -{
> - unsigned int neg_mode;
> -
> - switch (interface) {
> - case PHY_INTERFACE_MODE_SGMII:
> - case PHY_INTERFACE_MODE_QSGMII:
> - case PHY_INTERFACE_MODE_QUSGMII:
> - case PHY_INTERFACE_MODE_USXGMII:
> - /* These protocols are designed for use with a PHY which
> - * communicates its negotiation result back to the MAC via
> - * inband communication. Note: there exist PHYs that run
> - * with SGMII but do not send the inband data.
> - */
> - if (!phylink_autoneg_inband(mode))
> - neg_mode = PHYLINK_PCS_NEG_OUTBAND;
> - else
> - neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
> - break;
> -
> - case PHY_INTERFACE_MODE_1000BASEX:
> - case PHY_INTERFACE_MODE_2500BASEX:
> - /* 1000base-X is designed for use media-side for Fibre
> - * connections, and thus the Autoneg bit needs to be
> - * taken into account. We also do this for 2500base-X
> - * as well, but drivers may not support this, so may
> - * need to override this.
> - */
> - if (!phylink_autoneg_inband(mode))
> - neg_mode = PHYLINK_PCS_NEG_OUTBAND;
> - else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> - advertising))
> - neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
> - else
> - neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED;
> - break;
> -
> - default:
> - neg_mode = PHYLINK_PCS_NEG_NONE;
> - break;
> - }
> -
> - return neg_mode;
> -}
> -
> /**
> * struct phylink_link_state - link state structure
> * @advertising: ethtool bitmask containing advertised link modes
> --
> 2.42.0
>
>

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

2023-11-16 07:33:22

by Luo Jie

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] net: phylink: move phylink_pcs_neg_mode() to phylink.c



On 11/15/2023 10:14 PM, Russell King (Oracle) wrote:
> Hi,
>
> You don't need this patch for your series, and you're bypassing my
> ability to decide when this patch should be merged (which is not yet,
> I want things to remain as-is for another cycle.)
>
> In theory, looking at past history, 6.7 will probably be a LTS kernel,
> but until that is known for certain, I don't want to commit to moving
> this function in case LTS gets delayed by a cycle.
>
> Please drop it from your series.
>
> Thanks.

Got it, Russell.
I will drop this patch in the next patch set, Thanks.

>
> On Wed, Nov 15, 2023 at 10:06:25PM +0800, Luo Jie wrote:
>> From: Vladimir Oltean <[email protected]>
>>
>> Russell points out that there is no user of phylink_pcs_neg_mode()
>> outside of phylink.c, nor is there planned to be any, so we can just
>> move it there.
>>
>> Suggested-by: Russell King (Oracle) <[email protected]>
>> Signed-off-by: Vladimir Oltean <[email protected]>
>> Signed-off-by: Luo Jie <[email protected]>
>> ---
>> drivers/net/phy/phylink.c | 65 ++++++++++++++++++++++++++++++++++++++
>> include/linux/phylink.h | 66 ---------------------------------------
>> 2 files changed, 65 insertions(+), 66 deletions(-)
>>
>> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
>> index 25c19496a336..162f51b0986a 100644
>> --- a/drivers/net/phy/phylink.c
>> +++ b/drivers/net/phy/phylink.c
>> @@ -162,6 +162,71 @@ static const char *phylink_an_mode_str(unsigned int mode)
>> return mode < ARRAY_SIZE(modestr) ? modestr[mode] : "unknown";
>> }
>>
>> +/**
>> + * phylink_pcs_neg_mode() - helper to determine PCS inband mode
>> + * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
>> + * @interface: interface mode to be used
>> + * @advertising: adertisement ethtool link mode mask
>> + *
>> + * Determines the negotiation mode to be used by the PCS, and returns
>> + * one of:
>> + *
>> + * - %PHYLINK_PCS_NEG_NONE: interface mode does not support inband
>> + * - %PHYLINK_PCS_NEG_OUTBAND: an out of band mode (e.g. reading the PHY)
>> + * will be used.
>> + * - %PHYLINK_PCS_NEG_INBAND_DISABLED: inband mode selected but autoneg
>> + * disabled
>> + * - %PHYLINK_PCS_NEG_INBAND_ENABLED: inband mode selected and autoneg enabled
>> + *
>> + * Note: this is for cases where the PCS itself is involved in negotiation
>> + * (e.g. Clause 37, SGMII and similar) not Clause 73.
>> + */
>> +static unsigned int phylink_pcs_neg_mode(unsigned int mode, phy_interface_t interface,
>> + const unsigned long *advertising)
>> +{
>> + unsigned int neg_mode;
>> +
>> + switch (interface) {
>> + case PHY_INTERFACE_MODE_SGMII:
>> + case PHY_INTERFACE_MODE_QSGMII:
>> + case PHY_INTERFACE_MODE_QUSGMII:
>> + case PHY_INTERFACE_MODE_USXGMII:
>> + /* These protocols are designed for use with a PHY which
>> + * communicates its negotiation result back to the MAC via
>> + * inband communication. Note: there exist PHYs that run
>> + * with SGMII but do not send the inband data.
>> + */
>> + if (!phylink_autoneg_inband(mode))
>> + neg_mode = PHYLINK_PCS_NEG_OUTBAND;
>> + else
>> + neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
>> + break;
>> +
>> + case PHY_INTERFACE_MODE_1000BASEX:
>> + case PHY_INTERFACE_MODE_2500BASEX:
>> + /* 1000base-X is designed for use media-side for Fibre
>> + * connections, and thus the Autoneg bit needs to be
>> + * taken into account. We also do this for 2500base-X
>> + * as well, but drivers may not support this, so may
>> + * need to override this.
>> + */
>> + if (!phylink_autoneg_inband(mode))
>> + neg_mode = PHYLINK_PCS_NEG_OUTBAND;
>> + else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
>> + advertising))
>> + neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
>> + else
>> + neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED;
>> + break;
>> +
>> + default:
>> + neg_mode = PHYLINK_PCS_NEG_NONE;
>> + break;
>> + }
>> +
>> + return neg_mode;
>> +}
>> +
>> static unsigned int phylink_interface_signal_rate(phy_interface_t interface)
>> {
>> switch (interface) {
>> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
>> index 875439ab45de..d589f89c612c 100644
>> --- a/include/linux/phylink.h
>> +++ b/include/linux/phylink.h
>> @@ -98,72 +98,6 @@ static inline bool phylink_autoneg_inband(unsigned int mode)
>> return mode == MLO_AN_INBAND;
>> }
>>
>> -/**
>> - * phylink_pcs_neg_mode() - helper to determine PCS inband mode
>> - * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
>> - * @interface: interface mode to be used
>> - * @advertising: adertisement ethtool link mode mask
>> - *
>> - * Determines the negotiation mode to be used by the PCS, and returns
>> - * one of:
>> - *
>> - * - %PHYLINK_PCS_NEG_NONE: interface mode does not support inband
>> - * - %PHYLINK_PCS_NEG_OUTBAND: an out of band mode (e.g. reading the PHY)
>> - * will be used.
>> - * - %PHYLINK_PCS_NEG_INBAND_DISABLED: inband mode selected but autoneg
>> - * disabled
>> - * - %PHYLINK_PCS_NEG_INBAND_ENABLED: inband mode selected and autoneg enabled
>> - *
>> - * Note: this is for cases where the PCS itself is involved in negotiation
>> - * (e.g. Clause 37, SGMII and similar) not Clause 73.
>> - */
>> -static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
>> - phy_interface_t interface,
>> - const unsigned long *advertising)
>> -{
>> - unsigned int neg_mode;
>> -
>> - switch (interface) {
>> - case PHY_INTERFACE_MODE_SGMII:
>> - case PHY_INTERFACE_MODE_QSGMII:
>> - case PHY_INTERFACE_MODE_QUSGMII:
>> - case PHY_INTERFACE_MODE_USXGMII:
>> - /* These protocols are designed for use with a PHY which
>> - * communicates its negotiation result back to the MAC via
>> - * inband communication. Note: there exist PHYs that run
>> - * with SGMII but do not send the inband data.
>> - */
>> - if (!phylink_autoneg_inband(mode))
>> - neg_mode = PHYLINK_PCS_NEG_OUTBAND;
>> - else
>> - neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
>> - break;
>> -
>> - case PHY_INTERFACE_MODE_1000BASEX:
>> - case PHY_INTERFACE_MODE_2500BASEX:
>> - /* 1000base-X is designed for use media-side for Fibre
>> - * connections, and thus the Autoneg bit needs to be
>> - * taken into account. We also do this for 2500base-X
>> - * as well, but drivers may not support this, so may
>> - * need to override this.
>> - */
>> - if (!phylink_autoneg_inband(mode))
>> - neg_mode = PHYLINK_PCS_NEG_OUTBAND;
>> - else if (linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
>> - advertising))
>> - neg_mode = PHYLINK_PCS_NEG_INBAND_ENABLED;
>> - else
>> - neg_mode = PHYLINK_PCS_NEG_INBAND_DISABLED;
>> - break;
>> -
>> - default:
>> - neg_mode = PHYLINK_PCS_NEG_NONE;
>> - break;
>> - }
>> -
>> - return neg_mode;
>> -}
>> -
>> /**
>> * struct phylink_link_state - link state structure
>> * @advertising: ethtool bitmask containing advertised link modes
>> --
>> 2.42.0
>>
>>
>