[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DM6PR11MB46573B295F2AE013DA758D979B442@DM6PR11MB4657.namprd11.prod.outlook.com>
Date: Mon, 14 Oct 2024 08:54:52 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: Jiri Pirko <jiri@...nulli.us>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>
CC: "davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>, "donald.hunter@...il.com"
<donald.hunter@...il.com>, "vadim.fedorenko@...ux.dev"
<vadim.fedorenko@...ux.dev>, "saeedm@...dia.com" <saeedm@...dia.com>,
"leon@...nel.org" <leon@...nel.org>, "tariqt@...dia.com" <tariqt@...dia.com>
Subject: RE: [PATCH net-next v3 1/2] dpll: add clock quality level attribute
and op
>From: Jiri Pirko <jiri@...nulli.us>
>Sent: Monday, October 14, 2024 10:12 AM
>To: netdev@...r.kernel.org
>
>From: Jiri Pirko <jiri@...dia.com>
>
>In order to allow driver expose quality level of the clock it is
>running, introduce a new netlink attr with enum to carry it to the
>userspace. Also, introduce an op the dpll netlink code calls into the
>driver to obtain the value.
>
>Signed-off-by: Jiri Pirko <jiri@...dia.com>
>---
>v2->v3:
>- changed "itu" prefix to "itu-opt1"
>- changed driver op to pass bitmap to allow to set multiple qualities
> and pass it to user over multiple attrs
>- enhanced the documentation a bit
>v1->v2:
>- extended quality enum documentation
>- added "itu" prefix to the enum values
>---
> Documentation/netlink/specs/dpll.yaml | 35 +++++++++++++++++++++++++++
> drivers/dpll/dpll_netlink.c | 24 ++++++++++++++++++
> include/linux/dpll.h | 4 +++
> include/uapi/linux/dpll.h | 24 ++++++++++++++++++
> 4 files changed, 87 insertions(+)
>
>diff --git a/Documentation/netlink/specs/dpll.yaml
>b/Documentation/netlink/specs/dpll.yaml
>index f2894ca35de8..0bd708e136ff 100644
>--- a/Documentation/netlink/specs/dpll.yaml
>+++ b/Documentation/netlink/specs/dpll.yaml
>@@ -85,6 +85,36 @@ definitions:
> This may happen for example if dpll device was previously
> locked on an input pin of type PIN_TYPE_SYNCE_ETH_PORT.
> render-max: true
>+ -
>+ type: enum
>+ name: clock-quality-level
>+ doc: |
>+ level of quality of a clock device. This mainly applies when
>+ the dpll lock-status is not DPLL_LOCK_STATUS_LOCKED.
>+ The current list is defined according to the table 11-7 contained
>+ in ITU-T G.8264/Y.1364 document. One may extend this list freely
>+ by other ITU-T defined clock qualities, or different ones defined
>+ by another standardization body (for those, please use
>+ different prefix).
>+ entries:
>+ -
>+ name: itu-opt1-prc
>+ value: 1
>+ -
>+ name: itu-opt1-ssu-a
>+ -
>+ name: itu-opt1-ssu-b
>+ -
>+ name: itu-opt1-eec1
>+ -
>+ name: itu-opt1-prtc
>+ -
>+ name: itu-opt1-eprtc
>+ -
>+ name: itu-opt1-eeec
>+ -
>+ name: itu-opt1-eprc
>+ render-max: true
> -
> type: const
> name: temp-divider
>@@ -252,6 +282,11 @@ attribute-sets:
> name: lock-status-error
> type: u32
> enum: lock-status-error
>+ -
>+ name: clock-quality-level
>+ type: u32
>+ enum: clock-quality-level
>+ multi-attr: true
> -
> name: pin
> enum-name: dpll_a_pin
>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>index fc0280dcddd1..c130f87147fa 100644
>--- a/drivers/dpll/dpll_netlink.c
>+++ b/drivers/dpll/dpll_netlink.c
>@@ -169,6 +169,27 @@ dpll_msg_add_temp(struct sk_buff *msg, struct dpll_device
>*dpll,
> return 0;
> }
>
>+static int
>+dpll_msg_add_clock_quality_level(struct sk_buff *msg, struct dpll_device
>*dpll,
>+ struct netlink_ext_ack *extack)
>+{
>+ const struct dpll_device_ops *ops = dpll_device_ops(dpll);
>+ DECLARE_BITMAP(qls, DPLL_CLOCK_QUALITY_LEVEL_MAX) = { 0 };
>+ enum dpll_clock_quality_level ql;
>+ int ret;
>+
>+ if (!ops->clock_quality_level_get)
>+ return 0;
>+ ret = ops->clock_quality_level_get(dpll, dpll_priv(dpll), qls, extack);
>+ if (ret)
>+ return ret;
>+ for_each_set_bit(ql, qls, DPLL_CLOCK_QUALITY_LEVEL_MAX)
>+ if (nla_put_u32(msg, DPLL_A_CLOCK_QUALITY_LEVEL, ql))
>+ return -EMSGSIZE;
>+
>+ return 0;
>+}
>+
> static int
> dpll_msg_add_pin_prio(struct sk_buff *msg, struct dpll_pin *pin,
> struct dpll_pin_ref *ref,
>@@ -557,6 +578,9 @@ dpll_device_get_one(struct dpll_device *dpll, struct
>sk_buff *msg,
> if (ret)
> return ret;
> ret = dpll_msg_add_lock_status(msg, dpll, extack);
>+ if (ret)
>+ return ret;
>+ ret = dpll_msg_add_clock_quality_level(msg, dpll, extack);
> if (ret)
> return ret;
> ret = dpll_msg_add_mode(msg, dpll, extack);
>diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>index 81f7b623d0ba..5e4f9ab1cf75 100644
>--- a/include/linux/dpll.h
>+++ b/include/linux/dpll.h
>@@ -26,6 +26,10 @@ struct dpll_device_ops {
> struct netlink_ext_ack *extack);
> int (*temp_get)(const struct dpll_device *dpll, void *dpll_priv,
> s32 *temp, struct netlink_ext_ack *extack);
>+ int (*clock_quality_level_get)(const struct dpll_device *dpll,
>+ void *dpll_priv,
>+ unsigned long *qls,
>+ struct netlink_ext_ack *extack);
> };
>
> struct dpll_pin_ops {
>diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
>index b0654ade7b7e..4b37542eace3 100644
>--- a/include/uapi/linux/dpll.h
>+++ b/include/uapi/linux/dpll.h
>@@ -79,6 +79,29 @@ enum dpll_lock_status_error {
> DPLL_LOCK_STATUS_ERROR_MAX = (__DPLL_LOCK_STATUS_ERROR_MAX - 1)
> };
>
>+/**
>+ * enum dpll_clock_quality_level - level of quality of a clock device. This
>+ * mainly applies when the dpll lock-status is not DPLL_LOCK_STATUS_LOCKED.
>+ * The current list is defined according to the table 11-7 contained in
>ITU-T
>+ * G.8264/Y.1364 document. One may extend this list freely by other ITU-T
>+ * defined clock qualities, or different ones defined by another
>+ * standardization body (for those, please use different prefix).
>+ */
>+enum dpll_clock_quality_level {
>+ DPLL_CLOCK_QUALITY_LEVEL_ITU_OPT1_PRC = 1,
>+ DPLL_CLOCK_QUALITY_LEVEL_ITU_OPT1_SSU_A,
>+ DPLL_CLOCK_QUALITY_LEVEL_ITU_OPT1_SSU_B,
>+ DPLL_CLOCK_QUALITY_LEVEL_ITU_OPT1_EEC1,
>+ DPLL_CLOCK_QUALITY_LEVEL_ITU_OPT1_PRTC,
>+ DPLL_CLOCK_QUALITY_LEVEL_ITU_OPT1_EPRTC,
>+ DPLL_CLOCK_QUALITY_LEVEL_ITU_OPT1_EEEC,
>+ DPLL_CLOCK_QUALITY_LEVEL_ITU_OPT1_EPRC,
>+
>+ /* private: */
>+ __DPLL_CLOCK_QUALITY_LEVEL_MAX,
>+ DPLL_CLOCK_QUALITY_LEVEL_MAX = (__DPLL_CLOCK_QUALITY_LEVEL_MAX - 1)
>+};
>+
> #define DPLL_TEMP_DIVIDER 1000
>
> /**
>@@ -180,6 +203,7 @@ enum dpll_a {
> DPLL_A_TEMP,
> DPLL_A_TYPE,
> DPLL_A_LOCK_STATUS_ERROR,
>+ DPLL_A_CLOCK_QUALITY_LEVEL,
>
> __DPLL_A_MAX,
> DPLL_A_MAX = (__DPLL_A_MAX - 1)
>--
>2.47.0
LGTM, Thank you!
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
Powered by blists - more mailing lists