On 17/01/2023 11:31, Kelvin Cheung wrote: >>> + "#clock-cells": >>> + const: 0 >>> + >>> + compatible: >>> + enum: >>> + - loongson,ls1b-clk-pll >>> + - loongson,ls1b-clk-cpu >>> + - loongson,ls1b-clk-ahb >>> + - loongson,ls1c-clk-pll >>> + - loongson,ls1c-clk-cpu >>> + - loongson,ls1c-clk-ahb >> >> Are you registering single clocks? It looks like. No, make a proper >> clock controller. > > This binding contains two types of clock, pll-clk and div-clk. > Should I split the binding to two bindings files? No, you should register rather one clock controller. Why this have to be 3 separate clock controllers? >> >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> +required: >>> + - "#clock-cells" >>> + - compatible >>> + - clocks >>> + - reg >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + clocks { >> >> No, not really related to the binding. > > Should I remove the "clocks" section? Yes. >> >>> + #address-cells = <1>; >>> + #size-cells = <1>; >>> + ranges; >>> + >>> + xtal: xtal { >> >> Incorrect in this context. Missing unit address. > > XTAL doesn't have reg property. Yeah, but DTS is not correct now, is it? If you doubt, build your DTB with W=1. >> >>> + compatible = "fixed-clock"; >>> + #clock-cells = <0>; >>> + clock-frequency = <33000000>; >>> + }; >>> + Best regards, Krzysztof