On 07/09/2024 16:27, Junhao Xie wrote: >>> + > [...] >>> + 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. > >> >>> + >>> + 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. > >> > [...] >>> + >>> +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. > >>> + $ref: /schemas/leds/ariaboard,photonicat-pmu-leds.yaml >>> + >>> + '^supply-(battery|charger)$': >>> + $ref: /schemas/power/supply/ariaboard,photonicat-pmu-supply.yaml >> >> Why two nodes? >> >>> + >>> +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. >> >>> + 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? > >> >> Missing monitored battery. >> > > I will add it. > >>> + }; >>> + > [...] >>> + >>> + 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