mbox series

[0/3] Add DT support for video clock controller on SM8150

Message ID 20240313-videocc-sm8150-dt-node-v1-0-ae8ec3c822c2@quicinc.com
Headers show
Series Add DT support for video clock controller on SM8150 | expand

Message

Satya Priya Kakitapalli March 13, 2024, 11:08 a.m. UTC
Also, add the index based lookup support and update the device tree
bindings as per latest convention.

Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
---
Satya Priya Kakitapalli (3):
      dt-bindings: clock: qcom: Update SM8150 videocc bindings
      clk: qcom: videocc-sm8150: Add index based clk lookup
      arm64: dts: qcom: sm8150: Add video clock controller node

 .../devicetree/bindings/clock/qcom,sm8450-videocc.yaml      |  1 +
 Documentation/devicetree/bindings/clock/qcom,videocc.yaml   |  3 ---
 arch/arm64/boot/dts/qcom/sa8155p.dtsi                       |  4 ++++
 arch/arm64/boot/dts/qcom/sm8150.dtsi                        | 13 +++++++++++++
 drivers/clk/qcom/videocc-sm8150.c                           |  8 ++++++--
 5 files changed, 24 insertions(+), 5 deletions(-)
---
base-commit: 8ffc8b1bbd505e27e2c8439d326b6059c906c9dd
change-id: 20240308-videocc-sm8150-dt-node-6f163b492f7c

Best regards,

Comments

Satya Priya Kakitapalli March 14, 2024, 9:13 a.m. UTC | #1
On 3/13/2024 10:21 PM, Krzysztof Kozlowski wrote:
> On 13/03/2024 12:08, Satya Priya Kakitapalli wrote:
>> Update the videocc device tree bindings for sm8150 to align with the
>> latest convention.
> Everything is an update. Please explain what you did and why. The "why"
> part you tried to cover but I just don't understand what is "align with
> the latest convention". What convention?


As per the recent upstream discussions, it is recommended to use 
index-based lookup instead of using clock names. The current bindings is 
not aligned with this, hence updating. I'll add the details to commit text.


>> Fixes: 35d26e9292e2 ("dt-bindings: clock: Add YAML schemas for the QCOM VIDEOCC clock bindings")
> What is the bug being fixed here?


There are 2 clocks required for this, AHB and XO. Only one clock is 
mentioned in the bindings for SM8150, this is one of the reasons to move 
to latest sm8450 bindings apart from clock names. Hence added a Fixes tag.


>> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
>> ---
>>   Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml | 1 +
>>   Documentation/devicetree/bindings/clock/qcom,videocc.yaml        | 3 ---
>>   2 files changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> index bad8f019a8d3..e00fdc8ceaa4 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> @@ -20,6 +20,7 @@ properties:
>>       enum:
>>         - qcom,sm8450-videocc
>>         - qcom,sm8550-videocc
>> +      - qcom,sm8150-videocc
> Wrong order. Look at the place from where you copied it.


Sure, will correct it.


> Best regards,
> Krzysztof
>
Dmitry Baryshkov March 14, 2024, 12:50 p.m. UTC | #2
On Thu, 14 Mar 2024 at 11:14, Satya Priya Kakitapalli (Temp)
<quic_skakitap@quicinc.com> wrote:
>
>
> On 3/14/2024 12:46 AM, Dmitry Baryshkov wrote:
> > On Wed, 13 Mar 2024 at 13:11, Satya Priya Kakitapalli
> > <quic_skakitap@quicinc.com> wrote:
> >> Add device node for video clock controller on Qualcomm
> >> SM8150 platform.
> >>
> >> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> >> ---
> >>   arch/arm64/boot/dts/qcom/sa8155p.dtsi |  4 ++++
> >>   arch/arm64/boot/dts/qcom/sm8150.dtsi  | 13 +++++++++++++
> >>   2 files changed, 17 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/sa8155p.dtsi b/arch/arm64/boot/dts/qcom/sa8155p.dtsi
> >> index ffb7ab695213..9e70effc72e1 100644
> >> --- a/arch/arm64/boot/dts/qcom/sa8155p.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sa8155p.dtsi
> >> @@ -38,3 +38,7 @@ &rpmhpd {
> >>           */
> >>          compatible = "qcom,sa8155p-rpmhpd";
> >>   };
> >> +
> >> +&videocc {
> >> +       power-domains = <&rpmhpd SA8155P_CX>;
> >> +};
> >> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> >> index a35c0852b5a1..6573c907d7e2 100644
> >> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> >> @@ -14,6 +14,7 @@
> >>   #include <dt-bindings/clock/qcom,dispcc-sm8150.h>
> >>   #include <dt-bindings/clock/qcom,gcc-sm8150.h>
> >>   #include <dt-bindings/clock/qcom,gpucc-sm8150.h>
> >> +#include <dt-bindings/clock/qcom,videocc-sm8150.h>
> >>   #include <dt-bindings/interconnect/qcom,osm-l3.h>
> >>   #include <dt-bindings/interconnect/qcom,sm8150.h>
> >>   #include <dt-bindings/thermal/thermal.h>
> >> @@ -3715,6 +3716,18 @@ usb_2_dwc3: usb@a800000 {
> >>                          };
> >>                  };
> >>
> >> +               videocc: clock-controller@ab00000 {
> >> +                       compatible = "qcom,sm8150-videocc";
> >> +                       reg = <0 0x0ab00000 0 0x10000>;
> >> +                       clocks = <&rpmhcc RPMH_CXO_CLK>,
> >> +                                <&gcc GCC_VIDEO_AHB_CLK>;
> >> +                       power-domains = <&rpmhpd SM8150_MMCX>;
> >> +                       required-opps = <&rpmhpd_opp_low_svs>;
> > Should not be necessary anymore.
>
>
> Whenever the rail is turned on, we want to keep it in low_svs state
> instead of retention, hence added this property , please let me know why
> you think it is not needed?

See https://lore.kernel.org/linux-arm-msm/20240226-rpmhpd-enable-corner-fix-v1-1-68c004cec48c@quicinc.com/

>
>
> >> +                       #clock-cells = <1>;
> >> +                       #reset-cells = <1>;
> >> +                       #power-domain-cells = <1>;
> >> +               };
> >> +
> >>                  camnoc_virt: interconnect@ac00000 {
> >>                          compatible = "qcom,sm8150-camnoc-virt";
> >>                          reg = <0 0x0ac00000 0 0x1000>;
> >>
> >> --
> >> 2.25.1
> >>
> >>
> >