On 07/09/2024 16:27, Junhao Xie wrote:

quoted

quoted

+

[...]

quoted

quoted

+  local-address:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 127
+    default: 1
+    description: CPU board address

Address of what? In which notation? It's part of this hardware.

Photonicat's MCU protocol documentation says it supports multiple hosts.
But Photonicat only uses one.
Is it necessary to remove local-address and use a fixed address?

I don't understand what this "address" is for.

quoted

quoted

+
+  remote-address:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 127
+    default: 1
+    description: PMU board address

Eee, no. Your board knows its address. You do not have to tell it.

I will remove remote-address.

quoted

[...]

quoted

quoted

+
+patternProperties:
+  '^leds-(status)':

That's not a pattern.

I originally wanted to keep it for extensions, but it didn't seem like a good idea.
I will move it to properties.

quoted

quoted

+    $ref: /schemas/leds/ariaboard,photonicat-pmu-leds.yaml
+
+  '^supply-(battery|charger)$':
+    $ref: /schemas/power/supply/ariaboard,photonicat-pmu-supply.yaml

Why two nodes?

quoted

+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+      serial {
+          photonicat-pmu {
+              compatible = "ariaboard,photonicat-pmu";
+              current-speed = <115200>;
+              local-address = <1>;
+              remote-address = <1>;
+
+              supply-battery {
+                  compatible = "ariaboard,photonicat-pmu-supply";
+                  label = "battery";

Nope, drop label.

quoted

+                  type = "battery";

No, there is no type property.

There are two supplies here, one is the charger and the other is the battery.
Is it necessary to change to use different compatible ones like
ariaboard,photonicat-pmu-battery and ariaboard,photonicat-pmu-charger?

Are the devices different? Why do you even need the type?

quoted

Missing monitored battery.

I will add it.

quoted

quoted

+              };
+

[...]

quoted

quoted

+
+              watchdog {
+                  compatible = "ariaboard,photonicat-pmu-watchdog";
+              };

These are seriously redundant and useless nodes.  There is nothing
beneficial from the nodes above - they are all empty, without resources.
Drop all of them.

How should I describe these devices? Using mfd_cell?

You mean drivers? MFD or auxiliary bus.

Best regards,
Krzysztof