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

From: Jakub Kicinski <kuba@kernel.org>
Sent: Tuesday, August 15, 2023 4:44 AM

On Fri, 11 Aug 2023 21:03:33 +0100 Vadim Fedorenko wrote:

quoted

+attribute-sets:
+  -
+    name: dpll
+    enum-name: dpll_a
+    attributes:
+      -
+        name: id
+        type: u32
+        value: 1

value: 1 is the default

quoted

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

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

quoted

+      -
+        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.

quoted

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

Same.

quoted

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

And here.

quoted

+        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?

I agree, but there is an issue with this, currently:

name: pin-parent-device
subset-of: dpll
attributes:
  -
    name: id
    type: u32
  -
    name: pin-direction
    type: u32
  -
    name: pin-prio
    type: u32
  -
    name: pin-state
    type: u32

Where "id" is a part of device space, rest attrs would be a pin space..
Shall we have another argument for device id in a pin space?

Thank you!
Arkadiusz

quoted

+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

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