[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ghpmbmfjps24x7xvojk4gbkl55wjcuufd4v6mz6ws5htv35g2b@ugqsbet7t73p>
Date: Fri, 27 Oct 2023 14:04:15 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>
Cc: Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
davem@...emloft.net, kuba@...nel.org, linux-kernel@...r.kernel.org,
andrew@...n.ch, Jose.Abreu@...opsys.com,
UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH net-next V3] net: pcs: xpcs: Add 2500BASE-X case in get
state for XPCS drivers
Cc += Russell
* It's a good practice to add all the reviewers to Cc in the new patch
* revisions.
On Fri, Oct 27, 2023 at 10:13:06AM +0530, Raju Lakkaraju wrote:
> Add DW_2500BASEX case in xpcs_get_state( ) to update speed, duplex and pause
>
> Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@...rochip.com>
With a nitpick below clarified, feel free to add:
Reviewed-by: Serge Semin <fancer.lancer@...il.com>
> ---
> drivers/net/pcs/pcs-xpcs.c | 29 +++++++++++++++++++++++++++++
> drivers/net/pcs/pcs-xpcs.h | 2 ++
> 2 files changed, 31 insertions(+)
>
> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> index 4dbc21f604f2..31f0beba638a 100644
> --- a/drivers/net/pcs/pcs-xpcs.c
> +++ b/drivers/net/pcs/pcs-xpcs.c
> @@ -1090,6 +1090,28 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
> return 0;
> }
>
> +static int xpcs_get_state_2500basex(struct dw_xpcs *xpcs,
> + struct phylink_link_state *state)
> +{
> + int ret;
> +
> + ret = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_STS);
> + if (ret < 0) {
> + state->link = 0;
> + return ret;
> + }
> +
> + state->link = !!(ret & DW_VR_MII_MMD_STS_LINK_STS);
> + if (!state->link)
> + return 0;
> +
> + state->speed = SPEED_2500;
> + state->pause |= MLO_PAUSE_TX | MLO_PAUSE_RX;
Why is it '|=' instead of just '='? Is it possible to have the 'pause'
field having some additional flags set which would be required to
preserve?
-Serge(y)
> + state->duplex = DUPLEX_FULL;
> +
> + return 0;
> +}
> +
> static void xpcs_get_state(struct phylink_pcs *pcs,
> struct phylink_link_state *state)
> {
> @@ -1127,6 +1149,13 @@ static void xpcs_get_state(struct phylink_pcs *pcs,
> ERR_PTR(ret));
> }
> break;
> + case DW_2500BASEX:
> + ret = xpcs_get_state_2500basex(xpcs, state);
> + if (ret) {
> + pr_err("xpcs_get_state_2500basex returned %pe\n",
> + ERR_PTR(ret));
> + }
> + break;
> default:
> return;
> }
> diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
> index 39a90417e535..96c36b32ca99 100644
> --- a/drivers/net/pcs/pcs-xpcs.h
> +++ b/drivers/net/pcs/pcs-xpcs.h
> @@ -55,6 +55,8 @@
> /* Clause 37 Defines */
> /* VR MII MMD registers offsets */
> #define DW_VR_MII_MMD_CTRL 0x0000
> +#define DW_VR_MII_MMD_STS 0x0001
> +#define DW_VR_MII_MMD_STS_LINK_STS BIT(2)
> #define DW_VR_MII_DIG_CTRL1 0x8000
> #define DW_VR_MII_AN_CTRL 0x8001
> #define DW_VR_MII_AN_INTR_STS 0x8002
> --
> 2.34.1
>
Powered by blists - more mailing lists