| Message ID | 1541414117-27864-1-git-send-email-tdas@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | arm64: dts: sdm845: Add videocc node | expand |
Quoting Taniya Das (2018-11-05 02:35:17) > This adds the video clock controller node to sdm845 based on the examples > in the bindings. > > Signed-off-by: Taniya Das <tdas@codeaurora.org> > --- Did you mean to send "To:" Andy? Clk tree doesn't take these sorts of dts patches.
Thanks Stephen for adding Andy. On 11/6/2018 4:51 AM, Stephen Boyd wrote: > Quoting Taniya Das (2018-11-05 02:35:17) >> This adds the video clock controller node to sdm845 based on the examples >> in the bindings. >> >> Signed-off-by: Taniya Das <tdas@codeaurora.org> >> --- > > Did you mean to send "To:" Andy? Clk tree doesn't take these sorts of > dts patches. >
Hi, On Mon, Nov 5, 2018 at 2:35 AM Taniya Das <tdas@codeaurora.org> wrote: > > This adds the video clock controller node to sdm845 based on the examples > in the bindings. > > Signed-off-by: Taniya Das <tdas@codeaurora.org> > --- > 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. > + }; > + > tsens0: thermal-sensor@c263000 { Please sort your new node by unit address. Specifically "ab00000" comes before "af00000", thus I would expect you to have your node right before the dispcc. -Doug
Quoting Doug Anderson (2018-11-26 16:35:50) > Hi, > > On Mon, Nov 5, 2018 at 2:35 AM Taniya Das <tdas@codeaurora.org> wrote: > > > > This adds the video clock controller node to sdm845 based on the examples > > in the bindings. > > > > Signed-off-by: Taniya Das <tdas@codeaurora.org> > > --- > > 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.
Hi, On Mon, Nov 26, 2018 at 10:57 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > + 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. Patch sent for the bindings change. https://lkml.kernel.org/r/20181127192443.136158-1-dianders@chromium.org
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>; + }; + tsens0: thermal-sensor@c263000 { compatible = "qcom,sdm845-tsens", "qcom,tsens-v2"; reg = <0xc263000 0x1ff>, /* TM */
This adds the video clock controller node to sdm845 based on the examples in the bindings. Signed-off-by: Taniya Das <tdas@codeaurora.org> --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+) -- Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.