Quoting Doug Anderson (2018-11-26 16:35:50) > Hi, > > On Mon, Nov 5, 2018 at 2:35 AM Taniya Das <tdas@xxxxxxxxxxxxxx> wrote: > > > > This adds the video clock controller node to sdm845 based on the examples > > in the bindings. > > > > Signed-off-by: Taniya Das <tdas@xxxxxxxxxxxxxx> > > --- > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index b72bdb0..91a281b 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -8,6 +8,7 @@ > > #include <dt-bindings/clock/qcom,dispcc-sdm845.h> > > #include <dt-bindings/clock/qcom,gcc-sdm845.h> > > #include <dt-bindings/clock/qcom,rpmh.h> > > +#include <dt-bindings/clock/qcom,videocc-sdm845.h> > > #include <dt-bindings/interrupt-controller/arm-gic.h> > > #include <dt-bindings/phy/phy-qcom-qusb2.h> > > #include <dt-bindings/reset/qcom,sdm845-aoss.h> > > @@ -1256,6 +1257,13 @@ > > #power-domain-cells = <1>; > > }; > > > > + videocc: clock-controller@ab00000 { > > + compatible = "qcom,sdm845-videocc"; > > + reg = <0xab00000 0x10000>; > > + #clock-cells = <1>; > > + #power-domain-cells = <1>; > > Any reason not to include "#reset-cells = <1>;" here? The bindings > list it as optional but I see no reason why we should leave it off. > The file "include/dt-bindings/clock/qcom,videocc-sdm845.h" seems to > include some #defines for resets. Even though the driver doesn't seem > like it supports it yet, it still should be fine to list it here. We should update the binding to make it a required property. It doesn't make any sense why that property would be optional given that the hardware either has support for resets or it doesn't.