* [RESEND PATCH net-next v2] net: phylink: Add helpers for c22 registers without MDIO
@ 2021-11-18 16:14 Sean Anderson
2021-11-18 21:07 ` kernel test robot
2021-11-20 0:32 ` kernel test robot
0 siblings, 2 replies; 4+ messages in thread
From: Sean Anderson @ 2021-11-18 16:14 UTC (permalink / raw)
To: netdev, Russell King
Cc: Heiner Kallweit, Andrew Lunn, linux-kernel, David S . Miller,
Jakub Kicinski, Sean Anderson, Russell King
Some devices expose memory-mapped c22-compliant PHYs. Because these
devices do not have an MDIO bus, we cannot use the existing helpers.
Refactor the existing helpers to allow supplying the values for c22
registers directly, instead of using MDIO to access them. Only get_state
and set_advertisement are converted, since they contain the most complex
logic. Because set_advertisement is never actually used outside
phylink_mii_c22_pcs_config, move the MDIO-writing part into that
function. Because some modes do not need the advertisement register set
at all, we use -EINVAL for this purpose.
Additionally, a new function phylink_pcs_enable_an is provided to
determine whether to enable autonegotiation.
Signed-off-by: Sean Anderson <sean.anderson@seco.com>
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
This series was originally submitted as [1]. Although does not include
its intended user (macb), I have submitted it separately at the behest
of Russel.
[1] https://lore.kernel.org/netdev/YVtypfZJfivfDnu7@lunn.ch/T/#m50877e4daf344ac0b5efced38c79246ad2b9cb6e
Changes in v2:
- Add phylink_pcs_enable_an
- Also remove set_advertisement
- Use mdiobus_modify_changed
drivers/net/phy/phylink.c | 120 +++++++++++++++++++++-----------------
include/linux/phylink.h | 7 ++-
2 files changed, 72 insertions(+), 55 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 33462fdc7add..36d7784e7a17 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2813,6 +2813,52 @@ void phylink_decode_usxgmii_word(struct phylink_link_state *state,
}
EXPORT_SYMBOL_GPL(phylink_decode_usxgmii_word);
+/**
+ * phylink_mii_c22_pcs_decode_state() - Decode MAC PCS state from MII registers
+ * @state: a pointer to a &struct phylink_link_state.
+ * @bmsr: The value of the %MII_BMSR register
+ * @lpa: The value of the %MII_LPA register
+ *
+ * Helper for MAC PCS supporting the 802.3 clause 22 register set for
+ * clause 37 negotiation and/or SGMII control.
+ *
+ * Parse the Clause 37 or Cisco SGMII link partner negotiation word into
+ * the phylink @state structure. This is suitable to be used for implementing
+ * the mac_pcs_get_state() member of the struct phylink_mac_ops structure if
+ * accessing @bmsr and @lpa cannot be done with MDIO directly.
+ */
+void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
+ u16 bmsr, u16 lpa)
+{
+ state->link = !!(bmsr & BMSR_LSTATUS);
+ state->an_complete = !!(bmsr & BMSR_ANEGCOMPLETE);
+ /* If there is no link or autonegotiation is disabled, the LP advertisement
+ * data is not meaningful, so don't go any further.
+ */
+ if (!state->link || !state->an_enabled)
+ return;
+
+ switch (state->interface) {
+ case PHY_INTERFACE_MODE_1000BASEX:
+ phylink_decode_c37_word(state, lpa, SPEED_1000);
+ break;
+
+ case PHY_INTERFACE_MODE_2500BASEX:
+ phylink_decode_c37_word(state, lpa, SPEED_2500);
+ break;
+
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_QSGMII:
+ phylink_decode_sgmii_word(state, lpa);
+ break;
+
+ default:
+ state->link = false;
+ break;
+ }
+}
+EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_decode_state);
+
/**
* phylink_mii_c22_pcs_get_state() - read the MAC PCS state
* @pcs: a pointer to a &struct mdio_device.
@@ -2839,55 +2885,26 @@ void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs,
return;
}
- state->link = !!(bmsr & BMSR_LSTATUS);
- state->an_complete = !!(bmsr & BMSR_ANEGCOMPLETE);
- /* If there is no link or autonegotiation is disabled, the LP advertisement
- * data is not meaningful, so don't go any further.
- */
- if (!state->link || !state->an_enabled)
- return;
-
- switch (state->interface) {
- case PHY_INTERFACE_MODE_1000BASEX:
- phylink_decode_c37_word(state, lpa, SPEED_1000);
- break;
-
- case PHY_INTERFACE_MODE_2500BASEX:
- phylink_decode_c37_word(state, lpa, SPEED_2500);
- break;
-
- case PHY_INTERFACE_MODE_SGMII:
- case PHY_INTERFACE_MODE_QSGMII:
- phylink_decode_sgmii_word(state, lpa);
- break;
-
- default:
- state->link = false;
- break;
- }
+ phylink_mii_c22_pcs_decode_state(state, bmsr, lpa);
}
EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_get_state);
/**
- * phylink_mii_c22_pcs_set_advertisement() - configure the clause 37 PCS
+ * phylink_mii_c22_pcs_encode_advertisement() - configure the clause 37 PCS
* advertisement
- * @pcs: a pointer to a &struct mdio_device.
* @interface: the PHY interface mode being configured
* @advertising: the ethtool advertisement mask
*
* Helper for MAC PCS supporting the 802.3 clause 22 register set for
* clause 37 negotiation and/or SGMII control.
*
- * Configure the clause 37 PCS advertisement as specified by @state. This
- * does not trigger a renegotiation; phylink will do that via the
- * mac_an_restart() method of the struct phylink_mac_ops structure.
+ * Encode the clause 37 PCS advertisement as specified by @interface and
+ * @advertising.
*
- * Returns negative error code on failure to configure the advertisement,
- * zero if no change has been made, or one if the advertisement has changed.
+ * Return: The new value for @adv, or ``-EINVAL`` if it should not be changed.
*/
-int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs,
- phy_interface_t interface,
- const unsigned long *advertising)
+int phylink_mii_c22_pcs_encode_advertisement(phy_interface_t interface,
+ const unsigned long *advertising)
{
u16 adv;
@@ -2901,18 +2918,15 @@ int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs,
if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
advertising))
adv |= ADVERTISE_1000XPSE_ASYM;
-
- return mdiodev_modify_changed(pcs, MII_ADVERTISE, 0xffff, adv);
-
+ return adv;
case PHY_INTERFACE_MODE_SGMII:
- return mdiodev_modify_changed(pcs, MII_ADVERTISE, 0xffff, 0x0001);
-
+ return 0x0001;
default:
/* Nothing to do for other modes */
- return 0;
+ return -EINVAL;
}
}
-EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_set_advertisement);
+EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_encode_advertisement);
/**
* phylink_mii_c22_pcs_config() - configure clause 22 PCS
@@ -2930,16 +2944,18 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
phy_interface_t interface,
const unsigned long *advertising)
{
- bool changed;
- u16 bmcr;
+ bool changed = 0;
+ u16 adv, bmcr;
int ret;
- ret = phylink_mii_c22_pcs_set_advertisement(pcs, interface,
- advertising);
- if (ret < 0)
- return ret;
-
- changed = ret > 0;
+ adv = phylink_mii_c22_pcs_encode_advertisement(interface, advertising);
+ if (adv != -EINVAL) {
+ ret = mdiobus_modify_changed(pcs->bus, pcs->addr,
+ MII_ADVERTISE, 0xffff, adv);
+ if (ret < 0)
+ return ret;
+ changed = ret;
+ }
/* Ensure ISOLATE bit is disabled */
if (mode == MLO_AN_INBAND &&
@@ -2952,7 +2968,7 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
if (ret < 0)
return ret;
- return changed ? 1 : 0;
+ return changed;
}
EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_config);
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 3563820a1765..01224235df0f 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -527,11 +527,12 @@ void phylink_set_port_modes(unsigned long *bits);
void phylink_set_10g_modes(unsigned long *mask);
void phylink_helper_basex_speed(struct phylink_link_state *state);
+void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
+ u16 bmsr, u16 lpa);
void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs,
struct phylink_link_state *state);
-int phylink_mii_c22_pcs_set_advertisement(struct mdio_device *pcs,
- phy_interface_t interface,
- const unsigned long *advertising);
+int phylink_mii_c22_pcs_encode_advertisement(phy_interface_t interface,
+ const unsigned long *advertising);
int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
phy_interface_t interface,
const unsigned long *advertising);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RESEND PATCH net-next v2] net: phylink: Add helpers for c22 registers without MDIO
2021-11-18 16:14 [RESEND PATCH net-next v2] net: phylink: Add helpers for c22 registers without MDIO Sean Anderson
@ 2021-11-18 21:07 ` kernel test robot
2021-11-18 21:16 ` Sean Anderson
2021-11-20 0:32 ` kernel test robot
1 sibling, 1 reply; 4+ messages in thread
From: kernel test robot @ 2021-11-18 21:07 UTC (permalink / raw)
To: Sean Anderson, netdev, Russell King
Cc: llvm, kbuild-all, Heiner Kallweit, Andrew Lunn, linux-kernel,
David S . Miller, Jakub Kicinski, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 3298 bytes --]
Hi Sean,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Sean-Anderson/net-phylink-Add-helpers-for-c22-registers-without-MDIO/20211119-002726
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git bb8cecf8ba127abca8ccd102207a59c55fdae515
config: hexagon-randconfig-r045-20211118 (attached as .config)
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/0day-ci/linux/commit/1af77aa70fbd03602e8db3d621fdaf4e8a301e98
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sean-Anderson/net-phylink-Add-helpers-for-c22-registers-without-MDIO/20211119-002726
git checkout 1af77aa70fbd03602e8db3d621fdaf4e8a301e98
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/net/phy/phylink.c:2952:10: warning: result of comparison of constant -22 with expression of type 'u16' (aka 'unsigned short') is always true [-Wtautological-constant-out-of-range-compare]
if (adv != -EINVAL) {
~~~ ^ ~~~~~~~
1 warning generated.
vim +2952 drivers/net/phy/phylink.c
2930
2931 /**
2932 * phylink_mii_c22_pcs_config() - configure clause 22 PCS
2933 * @pcs: a pointer to a &struct mdio_device.
2934 * @mode: link autonegotiation mode
2935 * @interface: the PHY interface mode being configured
2936 * @advertising: the ethtool advertisement mask
2937 *
2938 * Configure a Clause 22 PCS PHY with the appropriate negotiation
2939 * parameters for the @mode, @interface and @advertising parameters.
2940 * Returns negative error number on failure, zero if the advertisement
2941 * has not changed, or positive if there is a change.
2942 */
2943 int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
2944 phy_interface_t interface,
2945 const unsigned long *advertising)
2946 {
2947 bool changed = 0;
2948 u16 adv, bmcr;
2949 int ret;
2950
2951 adv = phylink_mii_c22_pcs_encode_advertisement(interface, advertising);
> 2952 if (adv != -EINVAL) {
2953 ret = mdiobus_modify_changed(pcs->bus, pcs->addr,
2954 MII_ADVERTISE, 0xffff, adv);
2955 if (ret < 0)
2956 return ret;
2957 changed = ret;
2958 }
2959
2960 /* Ensure ISOLATE bit is disabled */
2961 if (mode == MLO_AN_INBAND &&
2962 linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, advertising))
2963 bmcr = BMCR_ANENABLE;
2964 else
2965 bmcr = 0;
2966
2967 ret = mdiodev_modify(pcs, MII_BMCR, BMCR_ANENABLE | BMCR_ISOLATE, bmcr);
2968 if (ret < 0)
2969 return ret;
2970
2971 return changed;
2972 }
2973 EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_config);
2974
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26052 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RESEND PATCH net-next v2] net: phylink: Add helpers for c22 registers without MDIO
2021-11-18 21:07 ` kernel test robot
@ 2021-11-18 21:16 ` Sean Anderson
0 siblings, 0 replies; 4+ messages in thread
From: Sean Anderson @ 2021-11-18 21:16 UTC (permalink / raw)
To: kernel test robot, netdev, Russell King
Cc: llvm, kbuild-all, Heiner Kallweit, Andrew Lunn, linux-kernel,
David S . Miller, Jakub Kicinski
On 11/18/21 4:07 PM, kernel test robot wrote:
> Hi Sean,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on net-next/master]
>
> url: https://github.com/0day-ci/linux/commits/Sean-Anderson/net-phylink-Add-helpers-for-c22-registers-without-MDIO/20211119-002726
> base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git bb8cecf8ba127abca8ccd102207a59c55fdae515
> config: hexagon-randconfig-r045-20211118 (attached as .config)
> 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/0day-ci/linux/commit/1af77aa70fbd03602e8db3d621fdaf4e8a301e98
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Sean-Anderson/net-phylink-Add-helpers-for-c22-registers-without-MDIO/20211119-002726
> git checkout 1af77aa70fbd03602e8db3d621fdaf4e8a301e98
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>>> drivers/net/phy/phylink.c:2952:10: warning: result of comparison of constant -22 with expression of type 'u16' (aka 'unsigned short') is always true [-Wtautological-constant-out-of-range-compare]
> if (adv != -EINVAL) {
> ~~~ ^ ~~~~~~~
> 1 warning generated.
Hmm, looks like this should be
ret = phylink_mii_c22_pcs_encode_advertisement();
if (ret > 0) {
...
}
and just eliminate adv
--Sean
>
> vim +2952 drivers/net/phy/phylink.c
>
> 2930
> 2931 /**
> 2932 * phylink_mii_c22_pcs_config() - configure clause 22 PCS
> 2933 * @pcs: a pointer to a &struct mdio_device.
> 2934 * @mode: link autonegotiation mode
> 2935 * @interface: the PHY interface mode being configured
> 2936 * @advertising: the ethtool advertisement mask
> 2937 *
> 2938 * Configure a Clause 22 PCS PHY with the appropriate negotiation
> 2939 * parameters for the @mode, @interface and @advertising parameters.
> 2940 * Returns negative error number on failure, zero if the advertisement
> 2941 * has not changed, or positive if there is a change.
> 2942 */
> 2943 int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
> 2944 phy_interface_t interface,
> 2945 const unsigned long *advertising)
> 2946 {
> 2947 bool changed = 0;
> 2948 u16 adv, bmcr;
> 2949 int ret;
> 2950
> 2951 adv = phylink_mii_c22_pcs_encode_advertisement(interface, advertising);
>> 2952 if (adv != -EINVAL) {
> 2953 ret = mdiobus_modify_changed(pcs->bus, pcs->addr,
> 2954 MII_ADVERTISE, 0xffff, adv);
> 2955 if (ret < 0)
> 2956 return ret;
> 2957 changed = ret;
> 2958 }
> 2959
> 2960 /* Ensure ISOLATE bit is disabled */
> 2961 if (mode == MLO_AN_INBAND &&
> 2962 linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, advertising))
> 2963 bmcr = BMCR_ANENABLE;
> 2964 else
> 2965 bmcr = 0;
> 2966
> 2967 ret = mdiodev_modify(pcs, MII_BMCR, BMCR_ANENABLE | BMCR_ISOLATE, bmcr);
> 2968 if (ret < 0)
> 2969 return ret;
> 2970
> 2971 return changed;
> 2972 }
> 2973 EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_config);
> 2974
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RESEND PATCH net-next v2] net: phylink: Add helpers for c22 registers without MDIO
2021-11-18 16:14 [RESEND PATCH net-next v2] net: phylink: Add helpers for c22 registers without MDIO Sean Anderson
2021-11-18 21:07 ` kernel test robot
@ 2021-11-20 0:32 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-11-20 0:32 UTC (permalink / raw)
To: Sean Anderson, netdev, Russell King
Cc: kbuild-all, Heiner Kallweit, Andrew Lunn, linux-kernel,
David S . Miller, Jakub Kicinski, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 2498 bytes --]
Hi Sean,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Sean-Anderson/net-phylink-Add-helpers-for-c22-registers-without-MDIO/20211119-002726
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git bb8cecf8ba127abca8ccd102207a59c55fdae515
config: microblaze-randconfig-m031-20211118 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
drivers/net/phy/phylink.c:2952 phylink_mii_c22_pcs_config() warn: always true condition '(adv != -22) => (0-u16max != (-22))'
vim +2952 drivers/net/phy/phylink.c
2930
2931 /**
2932 * phylink_mii_c22_pcs_config() - configure clause 22 PCS
2933 * @pcs: a pointer to a &struct mdio_device.
2934 * @mode: link autonegotiation mode
2935 * @interface: the PHY interface mode being configured
2936 * @advertising: the ethtool advertisement mask
2937 *
2938 * Configure a Clause 22 PCS PHY with the appropriate negotiation
2939 * parameters for the @mode, @interface and @advertising parameters.
2940 * Returns negative error number on failure, zero if the advertisement
2941 * has not changed, or positive if there is a change.
2942 */
2943 int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
2944 phy_interface_t interface,
2945 const unsigned long *advertising)
2946 {
2947 bool changed = 0;
2948 u16 adv, bmcr;
2949 int ret;
2950
2951 adv = phylink_mii_c22_pcs_encode_advertisement(interface, advertising);
> 2952 if (adv != -EINVAL) {
2953 ret = mdiobus_modify_changed(pcs->bus, pcs->addr,
2954 MII_ADVERTISE, 0xffff, adv);
2955 if (ret < 0)
2956 return ret;
2957 changed = ret;
2958 }
2959
2960 /* Ensure ISOLATE bit is disabled */
2961 if (mode == MLO_AN_INBAND &&
2962 linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, advertising))
2963 bmcr = BMCR_ANENABLE;
2964 else
2965 bmcr = 0;
2966
2967 ret = mdiodev_modify(pcs, MII_BMCR, BMCR_ANENABLE | BMCR_ISOLATE, bmcr);
2968 if (ret < 0)
2969 return ret;
2970
2971 return changed;
2972 }
2973 EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_config);
2974
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38555 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-20 0:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18 16:14 [RESEND PATCH net-next v2] net: phylink: Add helpers for c22 registers without MDIO Sean Anderson
2021-11-18 21:07 ` kernel test robot
2021-11-18 21:16 ` Sean Anderson
2021-11-20 0:32 ` kernel test robot
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).