Re: [PATCH net-next 00/11] Create common DPLL configuration API

Jiri Pirko
2023-07-21T11:15:07Z

There are couple of issues that came up during our internal ci run:

10:16:04 error: drivers/dpll/dpll_netlink.c:452:5: error: no previous prototype for '__dpll_device_change_ntf' [-Werror=missing-prototypes]
10:16:04 error: drivers/dpll/dpll_netlink.c:1283:13: error: no previous prototype for 'dpll_netlink_fini' [-Werror=missing-prototypes]
10:16:04 error: drivers/dpll/dpll_core.c:221:1: error: no previous prototype for 'dpll_xa_ref_dpll_find' [-Werror=missing-prototypes]

10:27:31 error: drivers/dpll/dpll_core.c:220:21: warning: symbol 'dpll_xa_ref_dpll_find' was not declared. Should it be static?
10:27:31 error: drivers/dpll/dpll_netlink.c:452:5: warning: symbol '__dpll_device_change_ntf' was not declared. Should it be static?
10:27:31 error: drivers/dpll/dpll_netlink.c:1283:13: warning: symbol 'dpll_netlink_fini' was not declared. Should it be static?
10:27:41 error: drivers/net/ethernet/intel/ice/ice_dpll.c:461:3: error: a label can only be part of a statement and a declaration is not a statement

I believe that you didn't run make with C=2, otherwise you would hit
these.

Checkpatch issue:
10:29:30 CHECK: struct mutex definition without comment
10:29:30 #6581: FILE: drivers/net/ethernet/intel/ice/ice_dpll.h:85:
10:29:30 + struct mutex lock;

Spelling errors:
10:45:08 error: Documentation/netlink/specs/dpll.yaml:165: prority ==> priority
10:45:08 error: include/uapi/linux/dpll.h:128: prority ==> priority
10:45:08 error: drivers/net/ethernet/intel/ice/ice_dpll.c:2008: userpsace ==> userspace
10:45:08 error: drivers/net/ethernet/intel/ice/ice_dpll.h:20: properities ==> properties

Thu, Jul 20, 2023 at 11:18:52AM CEST, vadim.fedorenko@linux.dev wrote:
>Implement common API for clock/DPLL configuration and status reporting.
>The API utilises netlink interface as transport for commands and event
>notifications. This API aim to extend current pin configuration and

s/API aim/API aims/

>make it flexible and easy to cover special configurations.

I don't follow. How this could "aim to extend current pin configuration" ?
This is a new thing. Could you re-phrase?

What's "special configuration"? Sounds odd.

>
>Netlink interface is based on ynl spec, it allows use of in-kernel
>tools/net/ynl/cli.py application to control the interface with properly
>formated command and json attribute strings. Here are few command
>examples of how it works with `ice` driver on supported NIC:
>
>- dump dpll devices
>$# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \

"$#" looks a bit odd. Just "$" with "sudo" when you want to emphasize
root is needed to perform the command.

>--dump device-get
>[{'clock-id': 282574471561216,
> 'id': 0,
> 'lock-status': 'unlocked',
> 'mode': 'automatic',
> 'module-name': 'ice',
> 'type': 'eec'},
> {'clock-id': 282574471561216,
> 'id': 1,
> 'lock-status': 'unlocked',
> 'mode': 'automatic',
> 'module-name': 'ice',
> 'type': 'pps'}]
>
>- get single pin info:
>$# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
>--do pin-get --json '{"pin-id":2}'
>{'clock-id': 282574471561216,
> 'module-name': 'ice',
> 'pin-board-label': 'C827_0-RCLKA',
> 'pin-dpll-caps': 6,
> 'pin-frequency': 1953125,
> 'pin-id': 2,
> 'pin-parenti-device': [{'id': 0,

This looks like manual edit went wrong :)
s/parenti/parent/

> 'pin-direction': 'input',
> 'pin-prio': 11,
> 'pin-state': 'selectable'},
> {'id': 1,
> 'pin-direction': 'input',
> 'pin-prio': 9,
> 'pin-state': 'selectable'}],
> 'pin-type': 'mux'}
>
>- set pin's state on dpll:
>$# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
>--do pin-set --json '{"pin-id":2, "pin-parent-device":{"id":1, "pin-state":2}}'
>
>- set pin's prio on dpll:
>$# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
>--do pin-set --json '{"pin-id":2, "pin-parent-device":{"id":1, "pin-prio":4}}'
>
>- set pin's state on parent pin:
>$# ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/dpll.yaml \
>--do pin-set --json '{"pin-id":13, \
> "pin-parent-pin":{"pin-id":2, "pin-state":1}}'
> …