Message ID | 20221010011804.23716-5-vfedorenko@novek.ru (mailing list archive) |
---|---|
State | RFC |
Headers | show |
Series | Create common DPLL/clock configuration API | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Guessed tree name to be net-next |
netdev/fixes_present | success | Fixes tag not required for -next series |
netdev/subject_prefix | success | Link |
netdev/cover_letter | success | Series has a cover letter |
netdev/patch_count | success | Link |
netdev/header_inline | success | No static functions without inline keyword in header files |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/cc_maintainers | success | CCed 3 of 3 maintainers |
netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
netdev/module_param | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
netdev/check_selftest | success | No net selftest shell script |
netdev/verify_fixes | success | No Fixes tag |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 81 lines checked |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/source_inline | success | Was 0 now: 0 |
Mon, Oct 10, 2022 at 03:18:02AM CEST, vfedorenko@novek.ru wrote: >From: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> > >Dump names of sources and outputs in response to DPLL_CMD_DEVICE_GET dump >request. > >Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> >--- > drivers/dpll/dpll_netlink.c | 24 ++++++++++++++++++++++++ > include/linux/dpll.h | 2 ++ > include/uapi/linux/dpll.h | 2 ++ > 3 files changed, 28 insertions(+) > >diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c >index a5779871537a..e3604c10b59e 100644 >--- a/drivers/dpll/dpll_netlink.c >+++ b/drivers/dpll/dpll_netlink.c >@@ -31,12 +31,16 @@ static const struct nla_policy dpll_genl_set_source_policy[] = { > [DPLLA_DEVICE_ID] = { .type = NLA_U32 }, > [DPLLA_SOURCE_ID] = { .type = NLA_U32 }, > [DPLLA_SOURCE_TYPE] = { .type = NLA_U32 }, >+ [DPLLA_SOURCE_NAME] = { .type = NLA_STRING, >+ .len = DPLL_NAME_LENGTH }, > }; > > static const struct nla_policy dpll_genl_set_output_policy[] = { > [DPLLA_DEVICE_ID] = { .type = NLA_U32 }, > [DPLLA_OUTPUT_ID] = { .type = NLA_U32 }, > [DPLLA_OUTPUT_TYPE] = { .type = NLA_U32 }, >+ [DPLLA_OUTPUT_NAME] = { .type = NLA_STRING, >+ .len = DPLL_NAME_LENGTH }, > }; > > static const struct nla_policy dpll_genl_set_src_select_mode_policy[] = { >@@ -100,6 +104,7 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll, > { > int i, ret = 0, type, prio; > struct nlattr *src_attr; >+ const char *name; > > for (i = 0; i < dpll->sources_count; i++) { > src_attr = nla_nest_start(msg, DPLLA_SOURCE); >@@ -132,6 +137,15 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll, > break; > } > } >+ if (dpll->ops->get_source_name) { >+ name = dpll->ops->get_source_name(dpll, i); >+ if (name && nla_put_string(msg, DPLLA_SOURCE_NAME, >+ name)) { >+ nla_nest_cancel(msg, src_attr); >+ ret = -EMSGSIZE; >+ break; >+ } >+ } > nla_nest_end(msg, src_attr); > } > >@@ -143,6 +157,7 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll, > { > struct nlattr *out_attr; > int i, ret = 0, type; >+ const char *name; > > for (i = 0; i < dpll->outputs_count; i++) { > out_attr = nla_nest_start(msg, DPLLA_OUTPUT); >@@ -167,6 +182,15 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll, > } > ret = 0; > } >+ if (dpll->ops->get_output_name) { >+ name = dpll->ops->get_output_name(dpll, i); >+ if (name && nla_put_string(msg, DPLLA_OUTPUT_NAME, >+ name)) { >+ nla_nest_cancel(msg, out_attr); >+ ret = -EMSGSIZE; >+ break; >+ } >+ } > nla_nest_end(msg, out_attr); > } > >diff --git a/include/linux/dpll.h b/include/linux/dpll.h >index 3fe957a06b90..2f4964dc28f0 100644 >--- a/include/linux/dpll.h >+++ b/include/linux/dpll.h >@@ -23,6 +23,8 @@ struct dpll_device_ops { > int (*set_output_type)(struct dpll_device *dpll, int id, int val); > int (*set_source_select_mode)(struct dpll_device *dpll, int mode); > int (*set_source_prio)(struct dpll_device *dpll, int id, int prio); >+ const char *(*get_source_name)(struct dpll_device *dpll, int id); >+ const char *(*get_output_name)(struct dpll_device *dpll, int id); Hmm, why you exactly need the name for? > }; > > struct dpll_device *dpll_device_alloc(struct dpll_device_ops *ops, const char *name, >diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h >index f6b674e5cf01..8782d3425aae 100644 >--- a/include/uapi/linux/dpll.h >+++ b/include/uapi/linux/dpll.h >@@ -26,11 +26,13 @@ enum dpll_genl_attr { > DPLLA_SOURCE, > DPLLA_SOURCE_ID, > DPLLA_SOURCE_TYPE, >+ DPLLA_SOURCE_NAME, > DPLLA_SOURCE_SUPPORTED, > DPLLA_SOURCE_PRIO, > DPLLA_OUTPUT, > DPLLA_OUTPUT_ID, > DPLLA_OUTPUT_TYPE, >+ DPLLA_OUTPUT_NAME, > DPLLA_OUTPUT_SUPPORTED, > DPLLA_STATUS, > DPLLA_TEMP, >-- >2.27.0 >
On 10.10.2022 10:45, Jiri Pirko wrote: > Mon, Oct 10, 2022 at 03:18:02AM CEST, vfedorenko@novek.ru wrote: >> From: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> >> >> Dump names of sources and outputs in response to DPLL_CMD_DEVICE_GET dump >> request. >> >> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> >> --- >> drivers/dpll/dpll_netlink.c | 24 ++++++++++++++++++++++++ >> include/linux/dpll.h | 2 ++ >> include/uapi/linux/dpll.h | 2 ++ >> 3 files changed, 28 insertions(+) >> >> diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c >> index a5779871537a..e3604c10b59e 100644 >> --- a/drivers/dpll/dpll_netlink.c >> +++ b/drivers/dpll/dpll_netlink.c >> @@ -31,12 +31,16 @@ static const struct nla_policy dpll_genl_set_source_policy[] = { >> [DPLLA_DEVICE_ID] = { .type = NLA_U32 }, >> [DPLLA_SOURCE_ID] = { .type = NLA_U32 }, >> [DPLLA_SOURCE_TYPE] = { .type = NLA_U32 }, >> + [DPLLA_SOURCE_NAME] = { .type = NLA_STRING, >> + .len = DPLL_NAME_LENGTH }, >> }; >> >> static const struct nla_policy dpll_genl_set_output_policy[] = { >> [DPLLA_DEVICE_ID] = { .type = NLA_U32 }, >> [DPLLA_OUTPUT_ID] = { .type = NLA_U32 }, >> [DPLLA_OUTPUT_TYPE] = { .type = NLA_U32 }, >> + [DPLLA_OUTPUT_NAME] = { .type = NLA_STRING, >> + .len = DPLL_NAME_LENGTH }, >> }; >> >> static const struct nla_policy dpll_genl_set_src_select_mode_policy[] = { >> @@ -100,6 +104,7 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll, >> { >> int i, ret = 0, type, prio; >> struct nlattr *src_attr; >> + const char *name; >> >> for (i = 0; i < dpll->sources_count; i++) { >> src_attr = nla_nest_start(msg, DPLLA_SOURCE); >> @@ -132,6 +137,15 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll, >> break; >> } >> } >> + if (dpll->ops->get_source_name) { >> + name = dpll->ops->get_source_name(dpll, i); >> + if (name && nla_put_string(msg, DPLLA_SOURCE_NAME, >> + name)) { >> + nla_nest_cancel(msg, src_attr); >> + ret = -EMSGSIZE; >> + break; >> + } >> + } >> nla_nest_end(msg, src_attr); >> } >> >> @@ -143,6 +157,7 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll, >> { >> struct nlattr *out_attr; >> int i, ret = 0, type; >> + const char *name; >> >> for (i = 0; i < dpll->outputs_count; i++) { >> out_attr = nla_nest_start(msg, DPLLA_OUTPUT); >> @@ -167,6 +182,15 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll, >> } >> ret = 0; >> } >> + if (dpll->ops->get_output_name) { >> + name = dpll->ops->get_output_name(dpll, i); >> + if (name && nla_put_string(msg, DPLLA_OUTPUT_NAME, >> + name)) { >> + nla_nest_cancel(msg, out_attr); >> + ret = -EMSGSIZE; >> + break; >> + } >> + } >> nla_nest_end(msg, out_attr); >> } >> >> diff --git a/include/linux/dpll.h b/include/linux/dpll.h >> index 3fe957a06b90..2f4964dc28f0 100644 >> --- a/include/linux/dpll.h >> +++ b/include/linux/dpll.h >> @@ -23,6 +23,8 @@ struct dpll_device_ops { >> int (*set_output_type)(struct dpll_device *dpll, int id, int val); >> int (*set_source_select_mode)(struct dpll_device *dpll, int mode); >> int (*set_source_prio)(struct dpll_device *dpll, int id, int prio); >> + const char *(*get_source_name)(struct dpll_device *dpll, int id); >> + const char *(*get_output_name)(struct dpll_device *dpll, int id); > > Hmm, why you exactly need the name for? > As with device name, user-space app can use source/output name to easily select one using configuration value, for example. > >> }; >> >> struct dpll_device *dpll_device_alloc(struct dpll_device_ops *ops, const char *name, >> diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h >> index f6b674e5cf01..8782d3425aae 100644 >> --- a/include/uapi/linux/dpll.h >> +++ b/include/uapi/linux/dpll.h >> @@ -26,11 +26,13 @@ enum dpll_genl_attr { >> DPLLA_SOURCE, >> DPLLA_SOURCE_ID, >> DPLLA_SOURCE_TYPE, >> + DPLLA_SOURCE_NAME, >> DPLLA_SOURCE_SUPPORTED, >> DPLLA_SOURCE_PRIO, >> DPLLA_OUTPUT, >> DPLLA_OUTPUT_ID, >> DPLLA_OUTPUT_TYPE, >> + DPLLA_OUTPUT_NAME, >> DPLLA_OUTPUT_SUPPORTED, >> DPLLA_STATUS, >> DPLLA_TEMP, >> -- >> 2.27.0 >>
Mon, Oct 10, 2022 at 09:55:57PM CEST, vfedorenko@novek.ru wrote: >On 10.10.2022 10:45, Jiri Pirko wrote: >> Mon, Oct 10, 2022 at 03:18:02AM CEST, vfedorenko@novek.ru wrote: >> > From: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> >> > >> > Dump names of sources and outputs in response to DPLL_CMD_DEVICE_GET dump >> > request. >> > >> > Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> >> > --- >> > drivers/dpll/dpll_netlink.c | 24 ++++++++++++++++++++++++ >> > include/linux/dpll.h | 2 ++ >> > include/uapi/linux/dpll.h | 2 ++ >> > 3 files changed, 28 insertions(+) >> > >> > diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c >> > index a5779871537a..e3604c10b59e 100644 >> > --- a/drivers/dpll/dpll_netlink.c >> > +++ b/drivers/dpll/dpll_netlink.c >> > @@ -31,12 +31,16 @@ static const struct nla_policy dpll_genl_set_source_policy[] = { >> > [DPLLA_DEVICE_ID] = { .type = NLA_U32 }, >> > [DPLLA_SOURCE_ID] = { .type = NLA_U32 }, >> > [DPLLA_SOURCE_TYPE] = { .type = NLA_U32 }, >> > + [DPLLA_SOURCE_NAME] = { .type = NLA_STRING, >> > + .len = DPLL_NAME_LENGTH }, >> > }; >> > >> > static const struct nla_policy dpll_genl_set_output_policy[] = { >> > [DPLLA_DEVICE_ID] = { .type = NLA_U32 }, >> > [DPLLA_OUTPUT_ID] = { .type = NLA_U32 }, >> > [DPLLA_OUTPUT_TYPE] = { .type = NLA_U32 }, >> > + [DPLLA_OUTPUT_NAME] = { .type = NLA_STRING, >> > + .len = DPLL_NAME_LENGTH }, >> > }; >> > >> > static const struct nla_policy dpll_genl_set_src_select_mode_policy[] = { >> > @@ -100,6 +104,7 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll, >> > { >> > int i, ret = 0, type, prio; >> > struct nlattr *src_attr; >> > + const char *name; >> > >> > for (i = 0; i < dpll->sources_count; i++) { >> > src_attr = nla_nest_start(msg, DPLLA_SOURCE); >> > @@ -132,6 +137,15 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll, >> > break; >> > } >> > } >> > + if (dpll->ops->get_source_name) { >> > + name = dpll->ops->get_source_name(dpll, i); >> > + if (name && nla_put_string(msg, DPLLA_SOURCE_NAME, >> > + name)) { >> > + nla_nest_cancel(msg, src_attr); >> > + ret = -EMSGSIZE; >> > + break; >> > + } >> > + } >> > nla_nest_end(msg, src_attr); >> > } >> > >> > @@ -143,6 +157,7 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll, >> > { >> > struct nlattr *out_attr; >> > int i, ret = 0, type; >> > + const char *name; >> > >> > for (i = 0; i < dpll->outputs_count; i++) { >> > out_attr = nla_nest_start(msg, DPLLA_OUTPUT); >> > @@ -167,6 +182,15 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll, >> > } >> > ret = 0; >> > } >> > + if (dpll->ops->get_output_name) { >> > + name = dpll->ops->get_output_name(dpll, i); >> > + if (name && nla_put_string(msg, DPLLA_OUTPUT_NAME, >> > + name)) { >> > + nla_nest_cancel(msg, out_attr); >> > + ret = -EMSGSIZE; >> > + break; >> > + } >> > + } >> > nla_nest_end(msg, out_attr); >> > } >> > >> > diff --git a/include/linux/dpll.h b/include/linux/dpll.h >> > index 3fe957a06b90..2f4964dc28f0 100644 >> > --- a/include/linux/dpll.h >> > +++ b/include/linux/dpll.h >> > @@ -23,6 +23,8 @@ struct dpll_device_ops { >> > int (*set_output_type)(struct dpll_device *dpll, int id, int val); >> > int (*set_source_select_mode)(struct dpll_device *dpll, int mode); >> > int (*set_source_prio)(struct dpll_device *dpll, int id, int prio); >> > + const char *(*get_source_name)(struct dpll_device *dpll, int id); >> > + const char *(*get_output_name)(struct dpll_device *dpll, int id); >> >> Hmm, why you exactly need the name for? >> >As with device name, user-space app can use source/output name to easily >select one using configuration value, for example. Can you give me some examples? I can't imagine how it can be named differently than containing the "type string". My point is to avoid drivers to come up with original names that basically have the same meaning creating a mess. Why index (which is predictable and could be used in scripts) isn't enough? Or, alternatively, we can leave the "resolve" up to the userspace app. Example: index 0 type 1PPS index 1 type 10MHZ index 2 type SyncE ifindex 20 index 3 type SyncE ifindex 30 Now when user does: 1) dpll X source set index 0 could be equivalent to dpll X source set 1pps dpll app would do dump of all sources and if there is only one of type 1pps, it will resolve it to index 0 which it is going to send via ATTR_INDEX to kernel. 2) dpll X source set index 2 could be equivalent to dpll X source set eth1 dpll app would use rtnetlink to get ifindex 20 for eth1. Then it will do dump of all sources and see if there is SyncE source with ifindex 30. This will resolve to index 2 which it is going to send via ATTR_INDEX to kernel. >> >> > }; >> > >> > struct dpll_device *dpll_device_alloc(struct dpll_device_ops *ops, const char *name, >> > diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h >> > index f6b674e5cf01..8782d3425aae 100644 >> > --- a/include/uapi/linux/dpll.h >> > +++ b/include/uapi/linux/dpll.h >> > @@ -26,11 +26,13 @@ enum dpll_genl_attr { >> > DPLLA_SOURCE, >> > DPLLA_SOURCE_ID, >> > DPLLA_SOURCE_TYPE, >> > + DPLLA_SOURCE_NAME, >> > DPLLA_SOURCE_SUPPORTED, >> > DPLLA_SOURCE_PRIO, >> > DPLLA_OUTPUT, >> > DPLLA_OUTPUT_ID, >> > DPLLA_OUTPUT_TYPE, >> > + DPLLA_OUTPUT_NAME, >> > DPLLA_OUTPUT_SUPPORTED, >> > DPLLA_STATUS, >> > DPLLA_TEMP, >> > -- >> > 2.27.0 >> > >
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c index a5779871537a..e3604c10b59e 100644 --- a/drivers/dpll/dpll_netlink.c +++ b/drivers/dpll/dpll_netlink.c @@ -31,12 +31,16 @@ static const struct nla_policy dpll_genl_set_source_policy[] = { [DPLLA_DEVICE_ID] = { .type = NLA_U32 }, [DPLLA_SOURCE_ID] = { .type = NLA_U32 }, [DPLLA_SOURCE_TYPE] = { .type = NLA_U32 }, + [DPLLA_SOURCE_NAME] = { .type = NLA_STRING, + .len = DPLL_NAME_LENGTH }, }; static const struct nla_policy dpll_genl_set_output_policy[] = { [DPLLA_DEVICE_ID] = { .type = NLA_U32 }, [DPLLA_OUTPUT_ID] = { .type = NLA_U32 }, [DPLLA_OUTPUT_TYPE] = { .type = NLA_U32 }, + [DPLLA_OUTPUT_NAME] = { .type = NLA_STRING, + .len = DPLL_NAME_LENGTH }, }; static const struct nla_policy dpll_genl_set_src_select_mode_policy[] = { @@ -100,6 +104,7 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll, { int i, ret = 0, type, prio; struct nlattr *src_attr; + const char *name; for (i = 0; i < dpll->sources_count; i++) { src_attr = nla_nest_start(msg, DPLLA_SOURCE); @@ -132,6 +137,15 @@ static int __dpll_cmd_dump_sources(struct dpll_device *dpll, break; } } + if (dpll->ops->get_source_name) { + name = dpll->ops->get_source_name(dpll, i); + if (name && nla_put_string(msg, DPLLA_SOURCE_NAME, + name)) { + nla_nest_cancel(msg, src_attr); + ret = -EMSGSIZE; + break; + } + } nla_nest_end(msg, src_attr); } @@ -143,6 +157,7 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll, { struct nlattr *out_attr; int i, ret = 0, type; + const char *name; for (i = 0; i < dpll->outputs_count; i++) { out_attr = nla_nest_start(msg, DPLLA_OUTPUT); @@ -167,6 +182,15 @@ static int __dpll_cmd_dump_outputs(struct dpll_device *dpll, } ret = 0; } + if (dpll->ops->get_output_name) { + name = dpll->ops->get_output_name(dpll, i); + if (name && nla_put_string(msg, DPLLA_OUTPUT_NAME, + name)) { + nla_nest_cancel(msg, out_attr); + ret = -EMSGSIZE; + break; + } + } nla_nest_end(msg, out_attr); } diff --git a/include/linux/dpll.h b/include/linux/dpll.h index 3fe957a06b90..2f4964dc28f0 100644 --- a/include/linux/dpll.h +++ b/include/linux/dpll.h @@ -23,6 +23,8 @@ struct dpll_device_ops { int (*set_output_type)(struct dpll_device *dpll, int id, int val); int (*set_source_select_mode)(struct dpll_device *dpll, int mode); int (*set_source_prio)(struct dpll_device *dpll, int id, int prio); + const char *(*get_source_name)(struct dpll_device *dpll, int id); + const char *(*get_output_name)(struct dpll_device *dpll, int id); }; struct dpll_device *dpll_device_alloc(struct dpll_device_ops *ops, const char *name, diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h index f6b674e5cf01..8782d3425aae 100644 --- a/include/uapi/linux/dpll.h +++ b/include/uapi/linux/dpll.h @@ -26,11 +26,13 @@ enum dpll_genl_attr { DPLLA_SOURCE, DPLLA_SOURCE_ID, DPLLA_SOURCE_TYPE, + DPLLA_SOURCE_NAME, DPLLA_SOURCE_SUPPORTED, DPLLA_SOURCE_PRIO, DPLLA_OUTPUT, DPLLA_OUTPUT_ID, DPLLA_OUTPUT_TYPE, + DPLLA_OUTPUT_NAME, DPLLA_OUTPUT_SUPPORTED, DPLLA_STATUS, DPLLA_TEMP,