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

[Kubalewski, Arkadiusz]

>From: Jiri Pirko <jiri at resnulli.us>
>Sent: Friday, August 18, 2023 9:24 AM
>
>Fri, Aug 18, 2023 at 01:36:40AM CEST, kuba at kernel.org wrote:
>>On Thu, 17 Aug 2023 18:40:00 +0000 Kubalewski, Arkadiusz wrote:
>>> >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?
>>
>>Why would pin and device not have separate spaces?
>>
>>When referring to a pin from a "device mostly" command you can
>>usually wrap the pin attributes in a nest, and vice versa.
>>But it may not be needed at all here? Let's look at the commands:
>>
>>+    -
>>+      name: device-id-get
>>+        request:
>>+          attributes:
>>+            - module-name
>>+            - clock-id
>>+            - type
>>+        reply:
>>+          attributes:
>>+            - id
>>
>>All attributes are in "device" space, no mixing.
>>
>>+      name: device-get
>>+        request:
>>+          attributes:
>>+            - id
>>+        reply: &dev-attrs
>>+          attributes:
>>+            - id
>>+            - module-name
>>+            - mode
>>+            - mode-supported
>>+            - lock-status
>>+            - temp
>>+            - clock-id
>>+            - type
>>
>>Again, no pin attributes, so pin can be separate?
>>
>>+    -
>>+      name: device-set
>>+        request:
>>+          attributes:
>>+            - id
>>
>>Herm, this one looks like it's missing attrs :S
>>
>>+    -
>>+      name: pin-id-get
>>+        request:
>>+          attributes:
>>+            - module-name
>>+            - clock-id
>>+            - pin-board-label
>>+            - pin-panel-label
>>+            - pin-package-label
>>+            - pin-type
>>+        reply:
>>+          attributes:
>>+            - pin-id
>>
>>Mostly pin stuff. I guess the module-name and clock-id attrs can be
>>copy/pasted between device and pin, or put them in a separate set
>>and add that set as an attr here. Copy paste is likely much simpler.
>
>Agreed for the copy.
>
>Honestly, I wound thing that shared ATTR space is fine for DPLL,
>the split is an overkill here. But up to you Jakub :)
>

I prepared some POC's and it seems most convenient way to do the
split was to add new argument as proposed on the previous mail.
After all the spec generated diff for uAPI header like this:

--- a/include/uapi/linux/dpll.h
+++ b/include/uapi/linux/dpll.h
@@ -148,7 +148,17 @@ enum dpll_a {
        DPLL_A_LOCK_STATUS,
        DPLL_A_TEMP,
        DPLL_A_TYPE,
-       DPLL_A_PIN_ID,
+
+       __DPLL_A_MAX,
+       DPLL_A_MAX = (__DPLL_A_MAX - 1)
+};
+
+enum dpll_a_pin {
+       DPLL_A_PIN_ID = 1,
+       DPLL_A_PIN_PARENT_ID,
+       DPLL_A_PIN_MODULE_NAME,
+       DPLL_A_PIN_PAD,
+       DPLL_A_PIN_CLOCK_ID,
        DPLL_A_PIN_BOARD_LABEL,
        DPLL_A_PIN_PANEL_LABEL,
        DPLL_A_PIN_PACKAGE_LABEL,
@@ -164,8 +174,8 @@ enum dpll_a {
        DPLL_A_PIN_PARENT_DEVICE,
        DPLL_A_PIN_PARENT_PIN,

-       __DPLL_A_MAX,
-       DPLL_A_MAX = (__DPLL_A_MAX - 1)
+       __DPLL_A_PIN_MAX,
+       DPLL_A_PIN_MAX = (__DPLL_A_PIN_MAX - 1)
 };

So we have additional attribute for targeting either a pin or device
DPLL_A_PIN_PARENT_ID (u32) - which would be enclosed in the nests as
previously:
- DPLL_A_PIN_PARENT_DEVICE (if parent is a device)
- DPLL_A_PIN_PARENT_PIN (if parent is a pin)


I will adapt the docs and send this to Vadim's repo for review today,
if that is ok for us.

Thank you!
Arkadiusz

>
>>
>>+    -
>>+      name: pin-get
>>+        request:
>>+          attributes:
>>+            - pin-id
>>+        reply: &pin-attrs
>>+          attributes:
>>+            - pin-id
>>+            - pin-board-label
>>+            - pin-panel-label
>>+            - pin-package-label
>>+            - pin-type
>>+            - pin-frequency
>>+            - pin-frequency-supported
>>+            - pin-dpll-caps
>>+            - pin-parent-device
>
>The ID of device is inside this nest.
>
>
>>+            - pin-parent-pin
>>
>>All pin.
>>
>>+    -
>>+      name: pin-set
>>+        request:
>>+          attributes:
>>+            - pin-id
>>+            - pin-frequency
>>+            - pin-direction
>>+            - pin-prio
>>+            - pin-state
>>+            - pin-parent-device
>
>Same here.
>
>
>>+            - pin-parent-pin
>>
>>And all pin.