| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 6 Sep 2023 11:45:01 +0000
From: "Drewek, Wojciech" <wojciech.drewek@...el.com>
To: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>,
"kuba@...nel.org" <kuba@...nel.org>, "jiri@...nulli.us" <jiri@...nulli.us>,
"jonathan.lemon@...il.com" <jonathan.lemon@...il.com>, "pabeni@...hat.com"
<pabeni@...hat.com>, "vadim.fedorenko@...ux.dev" <vadim.fedorenko@...ux.dev>
CC: "bvanassche@....org" <bvanassche@....org>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "intel-wired-lan@...ts.osuosl.org"
<intel-wired-lan@...ts.osuosl.org>, "linux-clk@...r.kernel.org"
<linux-clk@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [Intel-wired-lan] [PATCH net-next 3/4] dpll: netlink/core: add
support for pin-dpll signal phase offset/adjust
> -----Original Message-----
> From: Kubalewski, Arkadiusz <arkadiusz.kubalewski@...el.com>
> Sent: Wednesday, September 6, 2023 12:26 PM
> To: Drewek, Wojciech <wojciech.drewek@...el.com>; kuba@...nel.org;
> jiri@...nulli.us; jonathan.lemon@...il.com; pabeni@...hat.com;
> vadim.fedorenko@...ux.dev
> Cc: bvanassche@....org; netdev@...r.kernel.org; intel-wired-
> lan@...ts.osuosl.org; linux-clk@...r.kernel.org; linux-arm-
> kernel@...ts.infradead.org
> Subject: RE: [Intel-wired-lan] [PATCH net-next 3/4] dpll: netlink/core: add
> support for pin-dpll signal phase offset/adjust
>
> >From: Drewek, Wojciech <wojciech.drewek@...el.com>
> >Sent: Wednesday, September 6, 2023 10:02 AM
> >
> >> -----Original Message-----
> >> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of
> >> Arkadiusz Kubalewski
> >> Sent: Wednesday, September 6, 2023 1:26 AM
> >> To: kuba@...nel.org; jiri@...nulli.us; jonathan.lemon@...il.com;
> >> pabeni@...hat.com; vadim.fedorenko@...ux.dev
> >> Cc: bvanassche@....org; netdev@...r.kernel.org; intel-wired-
> >> lan@...ts.osuosl.org; linux-clk@...r.kernel.org; linux-arm-
> >> kernel@...ts.infradead.org
> >> Subject: [Intel-wired-lan] [PATCH net-next 3/4] dpll: netlink/core: add
> >> support
> >> for pin-dpll signal phase offset/adjust
> >>
> >> 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 | 99
> >> ++++++++++++++++++++++++++++++++++++-
> >> include/linux/dpll.h | 18 +++++++
> >> 2 files changed, 116 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
> >> index 764437a0661b..548517d9ca4c 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;
> >
> >Why 0 is returned here? If it's intended, I would put a comment stating
> >why.
> >Same thing in dpll_msg_add_phase_offset.
>
> The callback is optional, any driver implementing dpll interface doesn't
> have to implement this callback and it must not be seen as an error.
> Callback that are required are pointed out in documentation:
> Documentation/driver-api/dpll.rst
>
> All the optional callbacks are returning this way, I don't see a point
> in adding extra comment here.
>
> Thank you!
> Arkadiusz
I see,
Thanks
>
> >
> >> + 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,39 @@ 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;
> >> + unsigned long i;
> >> + s32 phase_adj;
> >> + 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) {
> >> + const struct dpll_pin_ops *ops = dpll_pin_ops(ref);
> >> + struct dpll_device *dpll = ref->dpll;
> >> +
> >> + if (!ops->phase_adjust_set)
> >> + return -EOPNOTSUPP;
> >> + ret = ops->phase_adjust_set(pin,
> >> + dpll_pin_on_dpll_priv(dpll, pin),
> >> + dpll, dpll_priv(dpll), phase_adj,
> >> + extack);
> >> + if (ret)
> >> + return ret;
> >> + }
> >> + __dpll_pin_change_ntf(pin);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static int
> >> dpll_pin_parent_device_set(struct dpll_pin *pin, struct nlattr
> >> *parent_nest,
> >> struct netlink_ext_ack *extack)
> >> @@ -793,6 +885,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
> >>
> >> _______________________________________________
> >> Intel-wired-lan mailing list
> >> Intel-wired-lan@...osl.org
> >> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Powered by blists - more mailing lists