[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZR//rQ7WGmHeRBOP@nanopsycho>
Date: Fri, 6 Oct 2023 14:38:05 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
Cc: netdev@...r.kernel.org, vadim.fedorenko@...ux.dev, corbet@....net,
davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
jesse.brandeburg@...el.com, anthony.l.nguyen@...el.com,
linux-doc@...r.kernel.org, intel-wired-lan@...ts.osuosl.org
Subject: Re: [PATCH net-next v3 3/5] dpll: netlink/core: add support for
pin-dpll signal phase offset/adjust
Fri, Oct 06, 2023 at 01:40:59PM CEST, arkadiusz.kubalewski@...el.com wrote:
>Add callback ops for pin-dpll phase measurment.
>Add callback for pin signal phase adjustment.
>Add min and max phase adjustment values to pin proprties.
>Invoke callbacks in dpll_netlink.c when filling the pin details to
>provide user with phase related attribute values.
>
>Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
>---
> drivers/dpll/dpll_netlink.c | 130 +++++++++++++++++++++++++++++++++++-
> include/linux/dpll.h | 18 +++++
> 2 files changed, 147 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
>index e20daba6896a..97319a9e4667 100644
>--- a/drivers/dpll/dpll_netlink.c
>+++ b/drivers/dpll/dpll_netlink.c
>@@ -212,6 +212,53 @@ dpll_msg_add_pin_direction(struct sk_buff *msg, struct dpll_pin *pin,
> return 0;
> }
>
>+static int
>+dpll_msg_add_pin_phase_adjust(struct sk_buff *msg, struct dpll_pin *pin,
>+ struct dpll_pin_ref *ref,
>+ struct netlink_ext_ack *extack)
>+{
>+ const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
>+ struct dpll_device *dpll = ref->dpll;
>+ s32 phase_adjust;
>+ int ret;
>+
>+ if (!ops->phase_adjust_get)
>+ return 0;
>+ ret = ops->phase_adjust_get(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+ dpll, dpll_priv(dpll),
>+ &phase_adjust, extack);
>+ if (ret)
>+ return ret;
>+ if (nla_put_s32(msg, DPLL_A_PIN_PHASE_ADJUST, phase_adjust))
>+ return -EMSGSIZE;
>+
>+ return 0;
>+}
>+
>+static int
>+dpll_msg_add_phase_offset(struct sk_buff *msg, struct dpll_pin *pin,
>+ struct dpll_pin_ref *ref,
>+ struct netlink_ext_ack *extack)
>+{
>+ const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
>+ struct dpll_device *dpll = ref->dpll;
>+ s64 phase_offset;
>+ int ret;
>+
>+ if (!ops->phase_offset_get)
>+ return 0;
>+ ret = ops->phase_offset_get(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+ dpll, dpll_priv(dpll), &phase_offset,
>+ extack);
>+ if (ret)
>+ return ret;
>+ if (nla_put_64bit(msg, DPLL_A_PIN_PHASE_OFFSET, sizeof(phase_offset),
>+ &phase_offset, DPLL_A_PIN_PAD))
>+ return -EMSGSIZE;
>+
>+ return 0;
>+}
>+
> static int
> dpll_msg_add_pin_freq(struct sk_buff *msg, struct dpll_pin *pin,
> struct dpll_pin_ref *ref, struct netlink_ext_ack *extack)
>@@ -330,6 +377,9 @@ dpll_msg_add_pin_dplls(struct sk_buff *msg, struct dpll_pin *pin,
> if (ret)
> goto nest_cancel;
> ret = dpll_msg_add_pin_direction(msg, pin, ref, extack);
>+ if (ret)
>+ goto nest_cancel;
>+ ret = dpll_msg_add_phase_offset(msg, pin, ref, extack);
> if (ret)
> goto nest_cancel;
> nla_nest_end(msg, attr);
>@@ -377,6 +427,15 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
> if (nla_put_u32(msg, DPLL_A_PIN_CAPABILITIES, prop->capabilities))
> return -EMSGSIZE;
> ret = dpll_msg_add_pin_freq(msg, pin, ref, extack);
>+ if (ret)
>+ return ret;
>+ if (nla_put_s32(msg, DPLL_A_PIN_PHASE_ADJUST_MIN,
>+ prop->phase_range.min))
>+ return -EMSGSIZE;
>+ if (nla_put_s32(msg, DPLL_A_PIN_PHASE_ADJUST_MAX,
>+ prop->phase_range.max))
>+ return -EMSGSIZE;
>+ ret = dpll_msg_add_pin_phase_adjust(msg, pin, ref, extack);
> if (ret)
> return ret;
> if (xa_empty(&pin->parent_refs))
>@@ -416,7 +475,7 @@ dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg,
> if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type))
> return -EMSGSIZE;
>
>- return ret;
>+ return 0;
> }
>
> static int
>@@ -705,6 +764,70 @@ dpll_pin_direction_set(struct dpll_pin *pin, struct dpll_device *dpll,
> return 0;
> }
>
>+static int
>+dpll_pin_phase_adj_set(struct dpll_pin *pin, struct nlattr *phase_adj_attr,
>+ struct netlink_ext_ack *extack)
>+{
>+ struct dpll_pin_ref *ref, *failed;
>+ const struct dpll_pin_ops *ops;
>+ s32 phase_adj, old_phase_adj;
>+ struct dpll_device *dpll;
>+ unsigned long i;
>+ int ret;
>+
>+ phase_adj = nla_get_s32(phase_adj_attr);
>+ if (phase_adj > pin->prop->phase_range.max ||
>+ phase_adj < pin->prop->phase_range.min) {
>+ NL_SET_ERR_MSG(extack, "phase adjust value not supported");
>+ return -EINVAL;
>+ }
>+ xa_for_each(&pin->dpll_refs, i, ref) {
>+ ops = dpll_pin_ops(ref);
>+ if (!ops->phase_adjust_set || !ops->phase_adjust_get)
Extack msg.
>+ return -EOPNOTSUPP;
>+ }
>+ ref = dpll_xa_ref_dpll_first(&pin->dpll_refs);
>+ ops = dpll_pin_ops(ref);
>+ dpll = ref->dpll;
>+ ret = ops->phase_adjust_get(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+ dpll, dpll_priv(dpll), &old_phase_adj,
>+ extack);
>+ if (ret) {
>+ NL_SET_ERR_MSG(extack, "unable to get old phase adjust value");
>+ return ret;
>+ }
>+ if (phase_adj == old_phase_adj)
>+ return 0;
>+ xa_for_each(&pin->dpll_refs, i, ref) {
>+ ops = dpll_pin_ops(ref);
>+ dpll = ref->dpll;
>+ ret = ops->phase_adjust_set(pin,
>+ dpll_pin_on_dpll_priv(dpll, pin),
>+ dpll, dpll_priv(dpll), phase_adj,
>+ extack);
>+ if (ret) {
>+ failed = ref;
Extack msg.
>+ goto rollback;
>+ }
>+ }
>+ __dpll_pin_change_ntf(pin);
>+
>+ return 0;
>+
>+rollback:
>+ xa_for_each(&pin->dpll_refs, i, ref) {
>+ if (ref == failed)
>+ break;
>+ ops = dpll_pin_ops(ref);
>+ dpll = ref->dpll;
>+ if (ops->phase_adjust_set(pin, dpll_pin_on_dpll_priv(dpll, pin),
>+ dpll, dpll_priv(dpll), old_phase_adj,
>+ extack))
>+ NL_SET_ERR_MSG(extack, "set phase adjust rollback failed");
>+ }
>+ return ret;
>+}
>+
> static int
> dpll_pin_parent_device_set(struct dpll_pin *pin, struct nlattr *parent_nest,
> struct netlink_ext_ack *extack)
>@@ -793,6 +916,11 @@ dpll_pin_set_from_nlattr(struct dpll_pin *pin, struct genl_info *info)
> if (ret)
> return ret;
> break;
>+ case DPLL_A_PIN_PHASE_ADJUST:
>+ ret = dpll_pin_phase_adj_set(pin, a, info->extack);
>+ if (ret)
>+ return ret;
>+ break;
> case DPLL_A_PIN_PARENT_DEVICE:
> ret = dpll_pin_parent_device_set(pin, a, info->extack);
> if (ret)
>diff --git a/include/linux/dpll.h b/include/linux/dpll.h
>index bbc480cd2932..578fc5fa3750 100644
>--- a/include/linux/dpll.h
>+++ b/include/linux/dpll.h
>@@ -68,6 +68,18 @@ struct dpll_pin_ops {
> int (*prio_set)(const struct dpll_pin *pin, void *pin_priv,
> const struct dpll_device *dpll, void *dpll_priv,
> const u32 prio, struct netlink_ext_ack *extack);
>+ int (*phase_offset_get)(const struct dpll_pin *pin, void *pin_priv,
>+ const struct dpll_device *dpll, void *dpll_priv,
>+ s64 *phase_offset,
>+ struct netlink_ext_ack *extack);
>+ int (*phase_adjust_get)(const struct dpll_pin *pin, void *pin_priv,
>+ const struct dpll_device *dpll, void *dpll_priv,
>+ s32 *phase_adjust,
>+ struct netlink_ext_ack *extack);
>+ int (*phase_adjust_set)(const struct dpll_pin *pin, void *pin_priv,
>+ const struct dpll_device *dpll, void *dpll_priv,
>+ const s32 phase_adjust,
>+ struct netlink_ext_ack *extack);
> };
>
> struct dpll_pin_frequency {
>@@ -91,6 +103,11 @@ struct dpll_pin_frequency {
> #define DPLL_PIN_FREQUENCY_DCF77 \
> DPLL_PIN_FREQUENCY(DPLL_PIN_FREQUENCY_77_5_KHZ)
>
>+struct dpll_pin_phase_adjust_range {
>+ s32 min;
>+ s32 max;
>+};
>+
> struct dpll_pin_properties {
> const char *board_label;
> const char *panel_label;
>@@ -99,6 +116,7 @@ struct dpll_pin_properties {
> unsigned long capabilities;
> u32 freq_supported_num;
> struct dpll_pin_frequency *freq_supported;
>+ struct dpll_pin_phase_adjust_range phase_range;
> };
>
> #if IS_ENABLED(CONFIG_DPLL)
>--
>2.38.1
>
Powered by blists - more mailing lists