[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DM6PR11MB46578FE668325CDECCCCD3899B662@DM6PR11MB4657.namprd11.prod.outlook.com>
Date: Fri, 5 Jan 2024 13:31:02 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: Jiri Pirko <jiri@...nulli.us>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>
CC: "kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com"
<pabeni@...hat.com>, "davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>, "vadim.fedorenko@...ux.dev"
<vadim.fedorenko@...ux.dev>, "M, Saeed" <saeedm@...dia.com>,
"leon@...nel.org" <leon@...nel.org>, "Michalik, Michal"
<michal.michalik@...el.com>, "rrameshbabu@...dia.com"
<rrameshbabu@...dia.com>
Subject: RE: [patch net-next 2/3] net/mlx5: DPLL, Use struct to get values
from mlx5_dpll_synce_status_get()
>From: Jiri Pirko <jiri@...nulli.us>
>Sent: Wednesday, January 3, 2024 2:29 PM
>
>From: Jiri Pirko <jiri@...dia.com>
>
>Instead of passing separate args, introduce
>struct mlx5_dpll_synce_status to hold the values obtained by
>mlx5_dpll_synce_status_get().
>
>Signed-off-by: Jiri Pirko <jiri@...dia.com>
>---
> .../net/ethernet/mellanox/mlx5/core/dpll.c | 63 +++++++++----------
> 1 file changed, 28 insertions(+), 35 deletions(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>index a7ffd61fe248..dbe09d2f2069 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/dpll.c
>@@ -36,11 +36,15 @@ static int mlx5_dpll_clock_id_get(struct mlx5_core_dev
>*mdev, u64 *clock_id)
> return 0;
> }
>
>+struct mlx5_dpll_synce_status {
>+ enum mlx5_msees_admin_status admin_status;
>+ enum mlx5_msees_oper_status oper_status;
>+ bool ho_acq;
>+};
>+
> static int
> mlx5_dpll_synce_status_get(struct mlx5_core_dev *mdev,
>- enum mlx5_msees_admin_status *admin_status,
>- enum mlx5_msees_oper_status *oper_status,
>- bool *ho_acq)
>+ struct mlx5_dpll_synce_status *synce_status)
> {
> u32 out[MLX5_ST_SZ_DW(msees_reg)] = {};
> u32 in[MLX5_ST_SZ_DW(msees_reg)] = {};
>@@ -50,11 +54,9 @@ mlx5_dpll_synce_status_get(struct mlx5_core_dev *mdev,
> MLX5_REG_MSEES, 0, 0);
> if (err)
> return err;
>- if (admin_status)
>- *admin_status = MLX5_GET(msees_reg, out, admin_status);
>- *oper_status = MLX5_GET(msees_reg, out, oper_status);
>- if (ho_acq)
>- *ho_acq = MLX5_GET(msees_reg, out, ho_acq);
>+ synce_status->admin_status = MLX5_GET(msees_reg, out, admin_status);
>+ synce_status->oper_status = MLX5_GET(msees_reg, out, oper_status);
>+ synce_status->ho_acq = MLX5_GET(msees_reg, out, ho_acq);
> return 0;
> }
>
>@@ -74,14 +76,14 @@ mlx5_dpll_synce_status_set(struct mlx5_core_dev *mdev,
> }
>
> static enum dpll_lock_status
>-mlx5_dpll_lock_status_get(enum mlx5_msees_oper_status oper_status, bool
>ho_acq)
>+mlx5_dpll_lock_status_get(struct mlx5_dpll_synce_status *synce_status)
> {
>- switch (oper_status) {
>+ switch (synce_status->oper_status) {
> case MLX5_MSEES_OPER_STATUS_SELF_TRACK:
> fallthrough;
> case MLX5_MSEES_OPER_STATUS_OTHER_TRACK:
>- return ho_acq ? DPLL_LOCK_STATUS_LOCKED_HO_ACQ :
>- DPLL_LOCK_STATUS_LOCKED;
>+ return synce_status->ho_acq ? DPLL_LOCK_STATUS_LOCKED_HO_ACQ :
>+ DPLL_LOCK_STATUS_LOCKED;
> case MLX5_MSEES_OPER_STATUS_HOLDOVER:
> fallthrough;
> case MLX5_MSEES_OPER_STATUS_FAIL_HOLDOVER:
>@@ -92,12 +94,11 @@ mlx5_dpll_lock_status_get(enum mlx5_msees_oper_status
>oper_status, bool ho_acq)
> }
>
> static enum dpll_pin_state
>-mlx5_dpll_pin_state_get(enum mlx5_msees_admin_status admin_status,
>- enum mlx5_msees_oper_status oper_status)
>+mlx5_dpll_pin_state_get(struct mlx5_dpll_synce_status *synce_status)
> {
>- return (admin_status == MLX5_MSEES_ADMIN_STATUS_TRACK &&
>- (oper_status == MLX5_MSEES_OPER_STATUS_SELF_TRACK ||
>- oper_status == MLX5_MSEES_OPER_STATUS_OTHER_TRACK)) ?
>+ return (synce_status->admin_status == MLX5_MSEES_ADMIN_STATUS_TRACK
>&&
>+ (synce_status->oper_status == MLX5_MSEES_OPER_STATUS_SELF_TRACK
>||
>+ synce_status->oper_status ==
>MLX5_MSEES_OPER_STATUS_OTHER_TRACK)) ?
> DPLL_PIN_STATE_CONNECTED : DPLL_PIN_STATE_DISCONNECTED;
> }
>
>@@ -106,17 +107,14 @@ static int mlx5_dpll_device_lock_status_get(const
>struct dpll_device *dpll,
> enum dpll_lock_status *status,
> struct netlink_ext_ack *extack)
> {
>- enum mlx5_msees_oper_status oper_status;
>+ struct mlx5_dpll_synce_status synce_status;
> struct mlx5_dpll *mdpll = priv;
>- bool ho_acq;
> int err;
>
>- err = mlx5_dpll_synce_status_get(mdpll->mdev, NULL,
>- &oper_status, &ho_acq);
>+ err = mlx5_dpll_synce_status_get(mdpll->mdev, &synce_status);
> if (err)
> return err;
>-
>- *status = mlx5_dpll_lock_status_get(oper_status, ho_acq);
>+ *status = mlx5_dpll_lock_status_get(&synce_status);
> return 0;
> }
>
>@@ -151,16 +149,14 @@ static int mlx5_dpll_state_on_dpll_get(const struct
>dpll_pin *pin,
> enum dpll_pin_state *state,
> struct netlink_ext_ack *extack)
> {
>- enum mlx5_msees_admin_status admin_status;
>- enum mlx5_msees_oper_status oper_status;
>+ struct mlx5_dpll_synce_status synce_status;
> struct mlx5_dpll *mdpll = pin_priv;
> int err;
>
>- err = mlx5_dpll_synce_status_get(mdpll->mdev, &admin_status,
>- &oper_status, NULL);
>+ err = mlx5_dpll_synce_status_get(mdpll->mdev, &synce_status);
> if (err)
> return err;
>- *state = mlx5_dpll_pin_state_get(admin_status, oper_status);
>+ *state = mlx5_dpll_pin_state_get(&synce_status);
> return 0;
> }
>
>@@ -202,19 +198,16 @@ static void mlx5_dpll_periodic_work(struct
>work_struct *work)
> {
> struct mlx5_dpll *mdpll = container_of(work, struct mlx5_dpll,
> work.work);
>- enum mlx5_msees_admin_status admin_status;
>- enum mlx5_msees_oper_status oper_status;
>+ struct mlx5_dpll_synce_status synce_status;
> enum dpll_lock_status lock_status;
> enum dpll_pin_state pin_state;
>- bool ho_acq;
> int err;
>
>- err = mlx5_dpll_synce_status_get(mdpll->mdev, &admin_status,
>- &oper_status, &ho_acq);
>+ err = mlx5_dpll_synce_status_get(mdpll->mdev, &synce_status);
> if (err)
> goto err_out;
>- lock_status = mlx5_dpll_lock_status_get(oper_status, ho_acq);
>- pin_state = mlx5_dpll_pin_state_get(admin_status, oper_status);
>+ lock_status = mlx5_dpll_lock_status_get(&synce_status);
>+ pin_state = mlx5_dpll_pin_state_get(&synce_status);
>
> if (!mdpll->last.valid)
> goto invalid_out;
>--
>2.43.0
Hi Jiri,
Looks good to me.
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
Powered by blists - more mailing lists