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

[Jiri Pirko <jiri@resnulli.us>]

Fri, Jul 21, 2023 at 04:42:46PM CEST, vadim.fedorenko@linux.dev wrote:
>On 21.07.2023 12:14, Jiri Pirko wrote:
>> 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.
>
>Yeah, I'll re-run the set patch-by-patch with C=2 next time.
>
>> 
>> 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;
>
>Arkadiusz will take care of "ice" part.
>
>
>> 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
>> 
>
>Will fix it.
>
>> 
>> 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?
>
>Not really new. PTP devices have already simple pin configurations, mlx5 is
>using it for some cards with external pins. The problem is that PTP subsystem
>covers only simple configuration of the pin and doesn't cover DPLL part at all.

Okay, please put the info in.


>
>> 
>> What's "special configuration"? Sounds odd.
>> 
>
>Yeah, "complex configurations" sounds better, will change it.
>
>> 
>> > 
>> > 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/
>> 
>
>Ahhh... yeah :)
>
>> 
>> >                          '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}}'
>> > 
>> 
>> [...]
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel