Re: [Intel-wired-lan] [PATCH net-next v3 2/3] dpll: add phase_offset_monitor_get/set callback ops
Fri, May 23, 2025 at 09:45:29AM +0200, [email protected] wrote: >>From: Jiri Pirko >>Sent: Friday, May 9, 2025 8:15 AM >> >>Thu, May 08, 2025 at 05:20:24PM +0200, [email protected] >>wrote: From: Jiri Pirko Sent: Thursday, May 8, 2025 4:31 PM Thu, May 08, 2025 at 02:21:27PM +0200, [email protected] wrote: >Add new callback operations for a dpll device: >- phase_offset_monitor_get(..) - to obtain current state of phase offset > monitor feature from dpll device, >- phase_offset_monitor_set(..) - to allow feature configuration. > >Obtain the feature state value using the get callback and provide it to >the user if the device driver implements callbacks. > >Execute the set callback upon user requests. > >Reviewed-by: Milena Olech >Signed-off-by: Arkadiusz Kubalewski >--- >v3: >- remove feature flags and capabilities, >- add separated (per feature) callback ops, >- use callback ops to determine feature availability. >--- > drivers/dpll/dpll_netlink.c | 76 - > include/linux/dpll.h| 8 > 2 files changed, 82 insertions(+), 2 deletions(-) > >diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c >index c130f87147fa..6d2980455a46 100644 >--- a/drivers/dpll/dpll_netlink.c >+++ b/drivers/dpll/dpll_netlink.c >@@ -126,6 +126,26 @@ dpll_msg_add_mode_supported(struct sk_buff *msg, >struct dpll_device *dpll, > return 0; > } > >+static int >+dpll_msg_add_phase_offset_monitor(struct sk_buff *msg, struct >dpll_device >*dpll, >+struct netlink_ext_ack *extack) >+{ >+ const struct dpll_device_ops *ops = dpll_device_ops(dpll); >+ enum dpll_feature_state state; >+ int ret; >+ >+ if (ops->phase_offset_monitor_set && ops->phase_offset_monitor_get) { >+ ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), >+ &state, extack); >+ if (ret) >+ return -EINVAL; Why you don't propagate "ret"? >>> >>>My bad, will fix that. >>> >+ if (nla_put_u32(msg, DPLL_A_PHASE_OFFSET_MONITOR, state)) >+ return -EMSGSIZE; >+ } >+ >+ return 0; >+} >+ > static int > dpll_msg_add_lock_status(struct sk_buff *msg, struct dpll_device *dpll, >struct netlink_ext_ack *extack) >@@ -591,6 +611,9 @@ dpll_device_get_one(struct dpll_device *dpll, struct >sk_buff *msg, > return ret; > if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type)) > return -EMSGSIZE; >+ ret = dpll_msg_add_phase_offset_monitor(msg, dpll, extack); >+ if (ret) >+ return ret; > > return 0; > } >@@ -746,6 +769,31 @@ int dpll_pin_change_ntf(struct dpll_pin *pin) > } > EXPORT_SYMBOL_GPL(dpll_pin_change_ntf); > >+static int >+dpll_phase_offset_monitor_set(struct dpll_device *dpll, struct nlattr >*a, >+struct netlink_ext_ack *extack) >+{ >+ const struct dpll_device_ops *ops = dpll_device_ops(dpll); >+ enum dpll_feature_state state = nla_get_u32(a), old_state; >+ int ret; >+ >+ if (!(ops->phase_offset_monitor_set && ops- >phase_offset_monitor_get)) { >+ NL_SET_ERR_MSG_ATTR(extack, a, "dpll device not capable of >phase offset monitor"); >+ return -EOPNOTSUPP; >+ } >+ ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), >&old_state, >+ extack); >+ if (ret) { >+ NL_SET_ERR_MSG(extack, "unable to get current state of phase >offset monitor"); >+ return -EINVAL; >> >>Propagate ret. >> > >Sure, will do. > >> >+ } >+ if (state == old_state) >+ return 0; >+ >+ return ops->phase_offset_monitor_set(dpll, dpll_priv(dpll), state, >+ extack); Why you need to do this get/set dance? I mean, just call the driver set() op and let it do what is needed there, no? >>> >>>We did it this way from the beginning (during various pin-set related >>>flows). >> >>Hmm, idk if it is absolutelly necessary to stick with this pattern all >>the time. I mean, what's the benefit here? I don't see any. >> > >Driver implementing callback could do that, or can be done here. Here is >earlier/better, right? > >Why would we remove this pattern for one flow, and use different for >other flows? Doesn't make much sense to me, we could change for all to >make it consistent. Fair. > >> >>> >+} >+ > static int > dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a, > struct netlink_ext_ack *extack) >@@ -1533
Re: [Intel-wired-lan] [PATCH net-next v3 2/3] dpll: add phase_offset_monitor_get/set callback ops
>From: Jiri Pirko >Sent: Friday, May 9, 2025 8:15 AM > >Thu, May 08, 2025 at 05:20:24PM +0200, [email protected] >wrote: >>>From: Jiri Pirko >>>Sent: Thursday, May 8, 2025 4:31 PM >>> >>>Thu, May 08, 2025 at 02:21:27PM +0200, [email protected] >>>wrote: Add new callback operations for a dpll device: - phase_offset_monitor_get(..) - to obtain current state of phase offset monitor feature from dpll device, - phase_offset_monitor_set(..) - to allow feature configuration. Obtain the feature state value using the get callback and provide it to the user if the device driver implements callbacks. Execute the set callback upon user requests. Reviewed-by: Milena Olech Signed-off-by: Arkadiusz Kubalewski --- v3: - remove feature flags and capabilities, - add separated (per feature) callback ops, - use callback ops to determine feature availability. --- drivers/dpll/dpll_netlink.c | 76 - include/linux/dpll.h| 8 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c index c130f87147fa..6d2980455a46 100644 --- a/drivers/dpll/dpll_netlink.c +++ b/drivers/dpll/dpll_netlink.c @@ -126,6 +126,26 @@ dpll_msg_add_mode_supported(struct sk_buff *msg, struct dpll_device *dpll, return 0; } +static int +dpll_msg_add_phase_offset_monitor(struct sk_buff *msg, struct dpll_device *dpll, + struct netlink_ext_ack *extack) +{ + const struct dpll_device_ops *ops = dpll_device_ops(dpll); + enum dpll_feature_state state; + int ret; + + if (ops->phase_offset_monitor_set && ops->phase_offset_monitor_get) { + ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), + &state, extack); + if (ret) + return -EINVAL; >>> >>>Why you don't propagate "ret"? >>> >> >>My bad, will fix that. >> >>> + if (nla_put_u32(msg, DPLL_A_PHASE_OFFSET_MONITOR, state)) + return -EMSGSIZE; + } + + return 0; +} + static int dpll_msg_add_lock_status(struct sk_buff *msg, struct dpll_device *dpll, struct netlink_ext_ack *extack) @@ -591,6 +611,9 @@ dpll_device_get_one(struct dpll_device *dpll, struct sk_buff *msg, return ret; if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type)) return -EMSGSIZE; + ret = dpll_msg_add_phase_offset_monitor(msg, dpll, extack); + if (ret) + return ret; return 0; } @@ -746,6 +769,31 @@ int dpll_pin_change_ntf(struct dpll_pin *pin) } EXPORT_SYMBOL_GPL(dpll_pin_change_ntf); +static int +dpll_phase_offset_monitor_set(struct dpll_device *dpll, struct nlattr *a, + struct netlink_ext_ack *extack) +{ + const struct dpll_device_ops *ops = dpll_device_ops(dpll); + enum dpll_feature_state state = nla_get_u32(a), old_state; + int ret; + + if (!(ops->phase_offset_monitor_set && ops- phase_offset_monitor_get)) { + NL_SET_ERR_MSG_ATTR(extack, a, "dpll device not capable of phase offset monitor"); + return -EOPNOTSUPP; + } + ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), &old_state, + extack); + if (ret) { + NL_SET_ERR_MSG(extack, "unable to get current state of phase offset monitor"); + return -EINVAL; > >Propagate ret. > Sure, will do. > + } + if (state == old_state) + return 0; + + return ops->phase_offset_monitor_set(dpll, dpll_priv(dpll), state, +extack); >>> >>>Why you need to do this get/set dance? I mean, just call the driver >>>set() op and let it do what is needed there, no? >>> >> >>We did it this way from the beginning (during various pin-set related >>flows). > >Hmm, idk if it is absolutelly necessary to stick with this pattern all >the time. I mean, what's the benefit here? I don't see any. > Driver implementing callback could do that, or can be done here. Here is earlier/better, right? Why would we remove this pattern for one flow, and use different for other flows? Doesn't make much sense to me, we could change for all to make it consistent. > >> >>> +} + static int dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a, struct netlink_ext_ack *extack) @@ -1533,10 +1581,34 @@ int dpll_nl_device_get_doit(struct sk_buff *skb, struct genl_info *info) return genlmsg_reply(msg, info); } +static int +dpll_set_from_nlattr(struct
Re: [Intel-wired-lan] [PATCH net-next v3 2/3] dpll: add phase_offset_monitor_get/set callback ops
Thu, May 08, 2025 at 05:20:24PM +0200, [email protected] wrote: >>From: Jiri Pirko >>Sent: Thursday, May 8, 2025 4:31 PM >> >>Thu, May 08, 2025 at 02:21:27PM +0200, [email protected] >>wrote: >>>Add new callback operations for a dpll device: >>>- phase_offset_monitor_get(..) - to obtain current state of phase offset >>> monitor feature from dpll device, >>>- phase_offset_monitor_set(..) - to allow feature configuration. >>> >>>Obtain the feature state value using the get callback and provide it to >>>the user if the device driver implements callbacks. >>> >>>Execute the set callback upon user requests. >>> >>>Reviewed-by: Milena Olech >>>Signed-off-by: Arkadiusz Kubalewski >>>--- >>>v3: >>>- remove feature flags and capabilities, >>>- add separated (per feature) callback ops, >>>- use callback ops to determine feature availability. >>>--- >>> drivers/dpll/dpll_netlink.c | 76 - >>> include/linux/dpll.h| 8 >>> 2 files changed, 82 insertions(+), 2 deletions(-) >>> >>>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c >>>index c130f87147fa..6d2980455a46 100644 >>>--- a/drivers/dpll/dpll_netlink.c >>>+++ b/drivers/dpll/dpll_netlink.c >>>@@ -126,6 +126,26 @@ dpll_msg_add_mode_supported(struct sk_buff *msg, >>>struct dpll_device *dpll, >>> return 0; >>> } >>> >>>+static int >>>+dpll_msg_add_phase_offset_monitor(struct sk_buff *msg, struct dpll_device >>>*dpll, >>>+ struct netlink_ext_ack *extack) >>>+{ >>>+const struct dpll_device_ops *ops = dpll_device_ops(dpll); >>>+enum dpll_feature_state state; >>>+int ret; >>>+ >>>+if (ops->phase_offset_monitor_set && ops->phase_offset_monitor_get) { >>>+ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), >>>+&state, extack); >>>+if (ret) >>>+return -EINVAL; >> >>Why you don't propagate "ret"? >> > >My bad, will fix that. > >> >>>+if (nla_put_u32(msg, DPLL_A_PHASE_OFFSET_MONITOR, state)) >>>+return -EMSGSIZE; >>>+} >>>+ >>>+return 0; >>>+} >>>+ >>> static int >>> dpll_msg_add_lock_status(struct sk_buff *msg, struct dpll_device *dpll, >>> struct netlink_ext_ack *extack) >>>@@ -591,6 +611,9 @@ dpll_device_get_one(struct dpll_device *dpll, struct >>>sk_buff *msg, >>> return ret; >>> if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type)) >>> return -EMSGSIZE; >>>+ret = dpll_msg_add_phase_offset_monitor(msg, dpll, extack); >>>+if (ret) >>>+return ret; >>> >>> return 0; >>> } >>>@@ -746,6 +769,31 @@ int dpll_pin_change_ntf(struct dpll_pin *pin) >>> } >>> EXPORT_SYMBOL_GPL(dpll_pin_change_ntf); >>> >>>+static int >>>+dpll_phase_offset_monitor_set(struct dpll_device *dpll, struct nlattr *a, >>>+ struct netlink_ext_ack *extack) >>>+{ >>>+const struct dpll_device_ops *ops = dpll_device_ops(dpll); >>>+enum dpll_feature_state state = nla_get_u32(a), old_state; >>>+int ret; >>>+ >>>+if (!(ops->phase_offset_monitor_set && ops- >>>phase_offset_monitor_get)) { >>>+NL_SET_ERR_MSG_ATTR(extack, a, "dpll device not capable of >>>phase offset monitor"); >>>+return -EOPNOTSUPP; >>>+} >>>+ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), >>>&old_state, >>>+extack); >>>+if (ret) { >>>+NL_SET_ERR_MSG(extack, "unable to get current state of phase >>>offset monitor"); >>>+return -EINVAL; Propagate ret. >>>+} >>>+if (state == old_state) >>>+return 0; >>>+ >>>+return ops->phase_offset_monitor_set(dpll, dpll_priv(dpll), state, >>>+ extack); >> >>Why you need to do this get/set dance? I mean, just call the driver >>set() op and let it do what is needed there, no? >> > >We did it this way from the beginning (during various pin-set related flows). Hmm, idk if it is absolutelly necessary to stick with this pattern all the time. I mean, what's the benefit here? I don't see any. > >> >>>+} >>>+ >>> static int >>> dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a, >>> struct netlink_ext_ack *extack) >>>@@ -1533,10 +1581,34 @@ int dpll_nl_device_get_doit(struct sk_buff *skb, >>>struct genl_info *info) >>> return genlmsg_reply(msg, info); >>> } >>> >>>+static int >>>+dpll_set_from_nlattr(struct dpll_device *dpll, struct genl_info *info) >>>+{ >>>+struct nlattr *a; >>>+int rem, ret; >>>+ >>>+nla_for_each_attr(a, genlmsg_data(info->genlhdr), >>>+ genlmsg_len(info->genlhdr), rem) { >> >>Hmm, why you iterate? Why you just don't parse to attr array, as it is >>usually done? >> > >Hmm, AFAIR there are issues when you parse nested stuff with the array >approach, here nothing is nested, but we already
Re: [Intel-wired-lan] [PATCH net-next v3 2/3] dpll: add phase_offset_monitor_get/set callback ops
>From: Jiri Pirko >Sent: Thursday, May 8, 2025 4:31 PM > >Thu, May 08, 2025 at 02:21:27PM +0200, [email protected] >wrote: >>Add new callback operations for a dpll device: >>- phase_offset_monitor_get(..) - to obtain current state of phase offset >> monitor feature from dpll device, >>- phase_offset_monitor_set(..) - to allow feature configuration. >> >>Obtain the feature state value using the get callback and provide it to >>the user if the device driver implements callbacks. >> >>Execute the set callback upon user requests. >> >>Reviewed-by: Milena Olech >>Signed-off-by: Arkadiusz Kubalewski >>--- >>v3: >>- remove feature flags and capabilities, >>- add separated (per feature) callback ops, >>- use callback ops to determine feature availability. >>--- >> drivers/dpll/dpll_netlink.c | 76 - >> include/linux/dpll.h| 8 >> 2 files changed, 82 insertions(+), 2 deletions(-) >> >>diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c >>index c130f87147fa..6d2980455a46 100644 >>--- a/drivers/dpll/dpll_netlink.c >>+++ b/drivers/dpll/dpll_netlink.c >>@@ -126,6 +126,26 @@ dpll_msg_add_mode_supported(struct sk_buff *msg, >>struct dpll_device *dpll, >> return 0; >> } >> >>+static int >>+dpll_msg_add_phase_offset_monitor(struct sk_buff *msg, struct dpll_device >>*dpll, >>+ struct netlink_ext_ack *extack) >>+{ >>+ const struct dpll_device_ops *ops = dpll_device_ops(dpll); >>+ enum dpll_feature_state state; >>+ int ret; >>+ >>+ if (ops->phase_offset_monitor_set && ops->phase_offset_monitor_get) { >>+ ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), >>+ &state, extack); >>+ if (ret) >>+ return -EINVAL; > >Why you don't propagate "ret"? > My bad, will fix that. > >>+ if (nla_put_u32(msg, DPLL_A_PHASE_OFFSET_MONITOR, state)) >>+ return -EMSGSIZE; >>+ } >>+ >>+ return 0; >>+} >>+ >> static int >> dpll_msg_add_lock_status(struct sk_buff *msg, struct dpll_device *dpll, >> struct netlink_ext_ack *extack) >>@@ -591,6 +611,9 @@ dpll_device_get_one(struct dpll_device *dpll, struct >>sk_buff *msg, >> return ret; >> if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type)) >> return -EMSGSIZE; >>+ ret = dpll_msg_add_phase_offset_monitor(msg, dpll, extack); >>+ if (ret) >>+ return ret; >> >> return 0; >> } >>@@ -746,6 +769,31 @@ int dpll_pin_change_ntf(struct dpll_pin *pin) >> } >> EXPORT_SYMBOL_GPL(dpll_pin_change_ntf); >> >>+static int >>+dpll_phase_offset_monitor_set(struct dpll_device *dpll, struct nlattr *a, >>+ struct netlink_ext_ack *extack) >>+{ >>+ const struct dpll_device_ops *ops = dpll_device_ops(dpll); >>+ enum dpll_feature_state state = nla_get_u32(a), old_state; >>+ int ret; >>+ >>+ if (!(ops->phase_offset_monitor_set && ops- >>phase_offset_monitor_get)) { >>+ NL_SET_ERR_MSG_ATTR(extack, a, "dpll device not capable of >>phase offset monitor"); >>+ return -EOPNOTSUPP; >>+ } >>+ ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), >>&old_state, >>+ extack); >>+ if (ret) { >>+ NL_SET_ERR_MSG(extack, "unable to get current state of phase >>offset monitor"); >>+ return -EINVAL; >>+ } >>+ if (state == old_state) >>+ return 0; >>+ >>+ return ops->phase_offset_monitor_set(dpll, dpll_priv(dpll), state, >>+ extack); > >Why you need to do this get/set dance? I mean, just call the driver >set() op and let it do what is needed there, no? > We did it this way from the beginning (during various pin-set related flows). > >>+} >>+ >> static int >> dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a, >>struct netlink_ext_ack *extack) >>@@ -1533,10 +1581,34 @@ int dpll_nl_device_get_doit(struct sk_buff *skb, >>struct genl_info *info) >> return genlmsg_reply(msg, info); >> } >> >>+static int >>+dpll_set_from_nlattr(struct dpll_device *dpll, struct genl_info *info) >>+{ >>+ struct nlattr *a; >>+ int rem, ret; >>+ >>+ nla_for_each_attr(a, genlmsg_data(info->genlhdr), >>+ genlmsg_len(info->genlhdr), rem) { > >Hmm, why you iterate? Why you just don't parse to attr array, as it is >usually done? > Hmm, AFAIR there are issues when you parse nested stuff with the array approach, here nothing is nested, but we already have the same approach on parsing pin related stuff (dpll_pin_set_from_nlattr(..)), just did the same here. Thank you! Arkadiusz > >>+ switch (nla_type(a)) { >>+ case DPLL_A_PHASE_OFFSET_MONITOR: >>+ ret = dpll_phase_offset_monitor_set(dpll, a, >>+
Re: [Intel-wired-lan] [PATCH net-next v3 2/3] dpll: add phase_offset_monitor_get/set callback ops
Thu, May 08, 2025 at 02:21:27PM +0200, [email protected] wrote: >Add new callback operations for a dpll device: >- phase_offset_monitor_get(..) - to obtain current state of phase offset > monitor feature from dpll device, >- phase_offset_monitor_set(..) - to allow feature configuration. > >Obtain the feature state value using the get callback and provide it to >the user if the device driver implements callbacks. > >Execute the set callback upon user requests. > >Reviewed-by: Milena Olech >Signed-off-by: Arkadiusz Kubalewski >--- >v3: >- remove feature flags and capabilities, >- add separated (per feature) callback ops, >- use callback ops to determine feature availability. >--- > drivers/dpll/dpll_netlink.c | 76 - > include/linux/dpll.h| 8 > 2 files changed, 82 insertions(+), 2 deletions(-) > >diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c >index c130f87147fa..6d2980455a46 100644 >--- a/drivers/dpll/dpll_netlink.c >+++ b/drivers/dpll/dpll_netlink.c >@@ -126,6 +126,26 @@ dpll_msg_add_mode_supported(struct sk_buff *msg, struct >dpll_device *dpll, > return 0; > } > >+static int >+dpll_msg_add_phase_offset_monitor(struct sk_buff *msg, struct dpll_device >*dpll, >+struct netlink_ext_ack *extack) >+{ >+ const struct dpll_device_ops *ops = dpll_device_ops(dpll); >+ enum dpll_feature_state state; >+ int ret; >+ >+ if (ops->phase_offset_monitor_set && ops->phase_offset_monitor_get) { >+ ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), >+ &state, extack); >+ if (ret) >+ return -EINVAL; Why you don't propagate "ret"? >+ if (nla_put_u32(msg, DPLL_A_PHASE_OFFSET_MONITOR, state)) >+ return -EMSGSIZE; >+ } >+ >+ return 0; >+} >+ > static int > dpll_msg_add_lock_status(struct sk_buff *msg, struct dpll_device *dpll, >struct netlink_ext_ack *extack) >@@ -591,6 +611,9 @@ dpll_device_get_one(struct dpll_device *dpll, struct >sk_buff *msg, > return ret; > if (nla_put_u32(msg, DPLL_A_TYPE, dpll->type)) > return -EMSGSIZE; >+ ret = dpll_msg_add_phase_offset_monitor(msg, dpll, extack); >+ if (ret) >+ return ret; > > return 0; > } >@@ -746,6 +769,31 @@ int dpll_pin_change_ntf(struct dpll_pin *pin) > } > EXPORT_SYMBOL_GPL(dpll_pin_change_ntf); > >+static int >+dpll_phase_offset_monitor_set(struct dpll_device *dpll, struct nlattr *a, >+struct netlink_ext_ack *extack) >+{ >+ const struct dpll_device_ops *ops = dpll_device_ops(dpll); >+ enum dpll_feature_state state = nla_get_u32(a), old_state; >+ int ret; >+ >+ if (!(ops->phase_offset_monitor_set && ops->phase_offset_monitor_get)) { >+ NL_SET_ERR_MSG_ATTR(extack, a, "dpll device not capable of >phase offset monitor"); >+ return -EOPNOTSUPP; >+ } >+ ret = ops->phase_offset_monitor_get(dpll, dpll_priv(dpll), &old_state, >+ extack); >+ if (ret) { >+ NL_SET_ERR_MSG(extack, "unable to get current state of phase >offset monitor"); >+ return -EINVAL; >+ } >+ if (state == old_state) >+ return 0; >+ >+ return ops->phase_offset_monitor_set(dpll, dpll_priv(dpll), state, >+ extack); Why you need to do this get/set dance? I mean, just call the driver set() op and let it do what is needed there, no? >+} >+ > static int > dpll_pin_freq_set(struct dpll_pin *pin, struct nlattr *a, > struct netlink_ext_ack *extack) >@@ -1533,10 +1581,34 @@ int dpll_nl_device_get_doit(struct sk_buff *skb, >struct genl_info *info) > return genlmsg_reply(msg, info); > } > >+static int >+dpll_set_from_nlattr(struct dpll_device *dpll, struct genl_info *info) >+{ >+ struct nlattr *a; >+ int rem, ret; >+ >+ nla_for_each_attr(a, genlmsg_data(info->genlhdr), >+genlmsg_len(info->genlhdr), rem) { Hmm, why you iterate? Why you just don't parse to attr array, as it is usually done? >+ switch (nla_type(a)) { >+ case DPLL_A_PHASE_OFFSET_MONITOR: >+ ret = dpll_phase_offset_monitor_set(dpll, a, >+ info->extack); >+ if (ret) >+ return ret; >+ break; >+ default: >+ break; >+ } >+ } >+ >+ return 0; >+} >+ > int dpll_nl_device_set_doit(struct sk_buff *skb, struct genl_info *info) > { >- /* placeholder for set command */ >- return 0; >+ struct dpll_device *dpll = info->user_ptr[0]; >+ >+ return dpll_set_from_nlattr
