Re: [PATCH net-next v4 2/9] dpll: spec: Add Netlink spec in YAML

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 11 Aug 2023 21:03:33 +0100 Vadim Fedorenko wrote:
> +attribute-sets:
> +  -
> +    name: dpll
> +    enum-name: dpll_a
> +    attributes:
> +      -
> +        name: id
> +        type: u32
> +        value: 1

value: 1 is the default

> +      -
> +        name: module-name
> +        type: string
> +      -
> +        name: clock-id
> +        type: u64

I don't see a pad, you have a u64 without a pad?

> +      -
> +        name: mode
> +        type: u8

It's an enum, should always be u32 or bigger at protocol level.
Netlink pads to 4B, you're not saving any space.

> +        enum: mode
> +      -
> +        name: mode-supported
> +        type: u8

Same.

> +        enum: mode
> +        multi-attr: true
> +      -
> +        name: lock-status
> +        type: u8

And here.

> +        enum: lock-status
> +      -
> +        name: temp
> +        type: s32
> +      -
> +        name: type
> +        type: u8

And here, etc.

Why are all attributes in a single attr space? :(
More than half of them are prefixed with a pin- does it really 
not scream to you that they belong to a different space?

> +operations:
> +  enum-name: dpll_cmd
> +  list:
> +    -
> +      name: device-id-get
> +      doc: |
> +        Get id of dpll device that matches given attributes
> +      value: 1

is the default
-- 
pw-bot: cr