Fri, Jun 23, 2023 at 02:56:24AM CEST, arkadiusz.kubalewski@intel.com wrote:

quoted

From: Jiri Pirko <jiri@resnulli.us>
Sent: Wednesday, June 21, 2023 3:08 PM

Wed, Jun 21, 2023 at 01:53:24PM CEST, jiri@resnulli.us wrote:

quoted

Wed, Jun 21, 2023 at 01:18:59PM CEST, poros@redhat.com wrote:

quoted

Arkadiusz Kubalewski píše v Pá 09. 06. 2023 v 14:18 +0200:

quoted

From: Vadim Fedorenko <vadim.fedorenko@linux.dev>

[...]

Could you perhaps cut out the text you don't comment? Saves some time
finding your reply.

quoted

quoted

+static int
+dpll_set_from_nlattr(struct dpll_device *dpll, struct genl_info
*info)
+{
+       const struct dpll_device_ops *ops = dpll_device_ops(dpll);
+       struct nlattr *tb[DPLL_A_MAX + 1];
+       int ret = 0;
+
+       nla_parse(tb, DPLL_A_MAX, genlmsg_data(info->genlhdr),
+                 genlmsg_len(info->genlhdr), NULL, info->extack);
+       if (tb[DPLL_A_MODE]) {

Hi,

Here should be something like:
              if (!ops->mode_set)
                      return -EOPNOTSUPP;

Why? All drivers implement that.
I believe that it's actullaly better that way. For a called setting up
the same mode it is the dpll in, there should be 0 return by the driver.
Note that driver holds this value. I'd like to keep this code as it is.

Actually, you are correct Petr, my mistake. Actually, no driver
implements this. Arkadiusz, could you please remove this op and
possibly any other unused  op? It will be added when needed.

Thanks!

Sorry, didn't have time for such change, added only check as suggested by
Petr.
If you think this is a big issue, we could change it for next version.

It's odd to carry on ops which are unused. I would prefer that to be
removed now and only introduced when they are actually needed.

Thank you!
Arkadiusz

quoted

quoted

[...]