Also, add the index based lookup support and update the device tree
bindings as per latest convention.
Signed-off-by: Satya Priya Kakitapalli <[email protected]>
---
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,
--
Satya Priya Kakitapalli <[email protected]>
Update the videocc device tree bindings for sm8150 to align with the
latest convention.
Fixes: 35d26e9292e2 ("dt-bindings: clock: Add YAML schemas for the QCOM VIDEOCC clock bindings")
Signed-off-by: Satya Priya Kakitapalli <[email protected]>
---
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
reg:
maxItems: 1
diff --git a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
index 6999e36ace1b..28d134ad9517 100644
--- a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
@@ -17,7 +17,6 @@ description: |
include/dt-bindings/clock/qcom,videocc-sc7180.h
include/dt-bindings/clock/qcom,videocc-sc7280.h
include/dt-bindings/clock/qcom,videocc-sdm845.h
- include/dt-bindings/clock/qcom,videocc-sm8150.h
include/dt-bindings/clock/qcom,videocc-sm8250.h
properties:
@@ -26,7 +25,6 @@ properties:
- qcom,sc7180-videocc
- qcom,sc7280-videocc
- qcom,sdm845-videocc
- - qcom,sm8150-videocc
- qcom,sm8250-videocc
clocks:
@@ -75,7 +73,6 @@ allOf:
enum:
- qcom,sc7180-videocc
- qcom,sdm845-videocc
- - qcom,sm8150-videocc
then:
properties:
clocks:
--
2.25.1
Add device node for video clock controller on Qualcomm
SM8150 platform.
Signed-off-by: Satya Priya Kakitapalli <[email protected]>
---
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>;
+ #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
Add support to look up for clocks using index instead of fw_name.
Signed-off-by: Satya Priya Kakitapalli <[email protected]>
---
drivers/clk/qcom/videocc-sm8150.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
index a0329260157a..2b788a03c5ed 100644
--- a/drivers/clk/qcom/videocc-sm8150.c
+++ b/drivers/clk/qcom/videocc-sm8150.c
@@ -19,6 +19,10 @@
#include "reset.h"
#include "gdsc.h"
+enum {
+ DT_BI_TCXO,
+};
+
enum {
P_BI_TCXO,
P_VIDEO_PLL0_OUT_MAIN,
@@ -49,7 +53,7 @@ static struct clk_alpha_pll video_pll0 = {
.hw.init = &(struct clk_init_data){
.name = "video_pll0",
.parent_data = &(const struct clk_parent_data){
- .fw_name = "bi_tcxo",
+ .index = DT_BI_TCXO,
},
.num_parents = 1,
.ops = &clk_alpha_pll_trion_ops,
@@ -63,7 +67,7 @@ static const struct parent_map video_cc_parent_map_0[] = {
};
static const struct clk_parent_data video_cc_parent_data_0[] = {
- { .fw_name = "bi_tcxo" },
+ { .index = DT_BI_TCXO },
{ .hw = &video_pll0.clkr.hw },
};
--
2.25.1
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?
>
> Fixes: 35d26e9292e2 ("dt-bindings: clock: Add YAML schemas for the QCOM VIDEOCC clock bindings")
What is the bug being fixed here?
> Signed-off-by: Satya Priya Kakitapalli <[email protected]>
> ---
> 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.
Best regards,
Krzysztof
On Wed, 13 Mar 2024 at 13:11, Satya Priya Kakitapalli
<[email protected]> wrote:
>
> Add support to look up for clocks using index instead of fw_name.
Why? You are breaking compatibility with existing bindings.
Also the commit message is incorrect. You are not _adding_ support.
You are changing name-based lookup to index-based one.
>
> Signed-off-by: Satya Priya Kakitapalli <[email protected]>
> ---
> drivers/clk/qcom/videocc-sm8150.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
> index a0329260157a..2b788a03c5ed 100644
> --- a/drivers/clk/qcom/videocc-sm8150.c
> +++ b/drivers/clk/qcom/videocc-sm8150.c
> @@ -19,6 +19,10 @@
> #include "reset.h"
> #include "gdsc.h"
>
> +enum {
> + DT_BI_TCXO,
> +};
> +
> enum {
> P_BI_TCXO,
> P_VIDEO_PLL0_OUT_MAIN,
> @@ -49,7 +53,7 @@ static struct clk_alpha_pll video_pll0 = {
> .hw.init = &(struct clk_init_data){
> .name = "video_pll0",
> .parent_data = &(const struct clk_parent_data){
> - .fw_name = "bi_tcxo",
> + .index = DT_BI_TCXO,
> },
> .num_parents = 1,
> .ops = &clk_alpha_pll_trion_ops,
> @@ -63,7 +67,7 @@ static const struct parent_map video_cc_parent_map_0[] = {
> };
>
> static const struct clk_parent_data video_cc_parent_data_0[] = {
> - { .fw_name = "bi_tcxo" },
> + { .index = DT_BI_TCXO },
> { .hw = &video_pll0.clkr.hw },
> };
>
>
> --
> 2.25.1
>
>
--
With best wishes
Dmitry
On Wed, 13 Mar 2024 at 13:11, Satya Priya Kakitapalli
<[email protected]> wrote:
>
> Update the videocc device tree bindings for sm8150 to align with the
> latest convention.
But why? Bindings already exist. There is nothing wrong with them. And
sm8150 platform in general uses name-based lookup.
>
> Fixes: 35d26e9292e2 ("dt-bindings: clock: Add YAML schemas for the QCOM VIDEOCC clock bindings")
It is not a fix, there is no bug that this commit is fixing.
> Signed-off-by: Satya Priya Kakitapalli <[email protected]>
> ---
> 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
>
> reg:
> maxItems: 1
> diff --git a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> index 6999e36ace1b..28d134ad9517 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> @@ -17,7 +17,6 @@ description: |
> include/dt-bindings/clock/qcom,videocc-sc7180.h
> include/dt-bindings/clock/qcom,videocc-sc7280.h
> include/dt-bindings/clock/qcom,videocc-sdm845.h
> - include/dt-bindings/clock/qcom,videocc-sm8150.h
> include/dt-bindings/clock/qcom,videocc-sm8250.h
>
> properties:
> @@ -26,7 +25,6 @@ properties:
> - qcom,sc7180-videocc
> - qcom,sc7280-videocc
> - qcom,sdm845-videocc
> - - qcom,sm8150-videocc
> - qcom,sm8250-videocc
>
> clocks:
> @@ -75,7 +73,6 @@ allOf:
> enum:
> - qcom,sc7180-videocc
> - qcom,sdm845-videocc
> - - qcom,sm8150-videocc
> then:
> properties:
> clocks:
>
> --
> 2.25.1
>
>
--
With best wishes
Dmitry
On Wed, 13 Mar 2024 at 13:11, Satya Priya Kakitapalli
<[email protected]> wrote:
>
> Add device node for video clock controller on Qualcomm
> SM8150 platform.
>
> Signed-off-by: Satya Priya Kakitapalli <[email protected]>
> ---
> 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.
> + #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
>
>
--
With best wishes
Dmitry
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 <[email protected]>
>> ---
>> 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
>
On 3/14/2024 12:46 AM, Dmitry Baryshkov wrote:
> On Wed, 13 Mar 2024 at 13:11, Satya Priya Kakitapalli
> <[email protected]> wrote:
>> Add device node for video clock controller on Qualcomm
>> SM8150 platform.
>>
>> Signed-off-by: Satya Priya Kakitapalli <[email protected]>
>> ---
>> 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?
>> + #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
>>
>>
>
On 3/14/2024 12:48 AM, Dmitry Baryshkov wrote:
> On Wed, 13 Mar 2024 at 13:11, Satya Priya Kakitapalli
> <[email protected]> wrote:
>> Add support to look up for clocks using index instead of fw_name.
> Why? You are breaking compatibility with existing bindings.
> Also the commit message is incorrect. You are not _adding_ support.
> You are changing name-based lookup to index-based one.
As per the recent upstream discussions, I see that the clock names is
not being allowed to use, it is recommended to use the index based
lookup. Hence I updated this before adding the DT node.
Will update the commit text accordingly.
>> Signed-off-by: Satya Priya Kakitapalli <[email protected]>
>> ---
>> drivers/clk/qcom/videocc-sm8150.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/videocc-sm8150.c b/drivers/clk/qcom/videocc-sm8150.c
>> index a0329260157a..2b788a03c5ed 100644
>> --- a/drivers/clk/qcom/videocc-sm8150.c
>> +++ b/drivers/clk/qcom/videocc-sm8150.c
>> @@ -19,6 +19,10 @@
>> #include "reset.h"
>> #include "gdsc.h"
>>
>> +enum {
>> + DT_BI_TCXO,
>> +};
>> +
>> enum {
>> P_BI_TCXO,
>> P_VIDEO_PLL0_OUT_MAIN,
>> @@ -49,7 +53,7 @@ static struct clk_alpha_pll video_pll0 = {
>> .hw.init = &(struct clk_init_data){
>> .name = "video_pll0",
>> .parent_data = &(const struct clk_parent_data){
>> - .fw_name = "bi_tcxo",
>> + .index = DT_BI_TCXO,
>> },
>> .num_parents = 1,
>> .ops = &clk_alpha_pll_trion_ops,
>> @@ -63,7 +67,7 @@ static const struct parent_map video_cc_parent_map_0[] = {
>> };
>>
>> static const struct clk_parent_data video_cc_parent_data_0[] = {
>> - { .fw_name = "bi_tcxo" },
>> + { .index = DT_BI_TCXO },
>> { .hw = &video_pll0.clkr.hw },
>> };
>>
>>
>> --
>> 2.25.1
>>
>>
>
On 3/14/2024 12:50 AM, Dmitry Baryshkov wrote:
> On Wed, 13 Mar 2024 at 13:11, Satya Priya Kakitapalli
> <[email protected]> wrote:
>> Update the videocc device tree bindings for sm8150 to align with the
>> latest convention.
> But why? Bindings already exist. There is nothing wrong with them. And
> sm8150 platform in general uses name-based lookup.
With the new index based lookup introduced we cannot use this bindings,
hence I moved to the sm8450-videocc bindings.
>> Fixes: 35d26e9292e2 ("dt-bindings: clock: Add YAML schemas for the QCOM VIDEOCC clock bindings")
> It is not a fix, there is no bug that this commit is fixing.
The clocks list needs to be fixed to add both XO and AHB clocks, and we
are adding required-opps property.
>> Signed-off-by: Satya Priya Kakitapalli <[email protected]>
>> ---
>> 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
>>
>> reg:
>> maxItems: 1
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
>> index 6999e36ace1b..28d134ad9517 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
>> @@ -17,7 +17,6 @@ description: |
>> include/dt-bindings/clock/qcom,videocc-sc7180.h
>> include/dt-bindings/clock/qcom,videocc-sc7280.h
>> include/dt-bindings/clock/qcom,videocc-sdm845.h
>> - include/dt-bindings/clock/qcom,videocc-sm8150.h
>> include/dt-bindings/clock/qcom,videocc-sm8250.h
>>
>> properties:
>> @@ -26,7 +25,6 @@ properties:
>> - qcom,sc7180-videocc
>> - qcom,sc7280-videocc
>> - qcom,sdm845-videocc
>> - - qcom,sm8150-videocc
>> - qcom,sm8250-videocc
>>
>> clocks:
>> @@ -75,7 +73,6 @@ allOf:
>> enum:
>> - qcom,sc7180-videocc
>> - qcom,sdm845-videocc
>> - - qcom,sm8150-videocc
>> then:
>> properties:
>> clocks:
>>
>> --
>> 2.25.1
>>
>>
>
On 14/03/2024 10:13, Satya Priya Kakitapalli (Temp) wrote:
>
> 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.
Yes, please.
>
>
>>> 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.
This should be in the commit msg.
Best regards,
Krzysztof
On Thu, 14 Mar 2024 at 11:14, Satya Priya Kakitapalli (Temp)
<[email protected]> wrote:
>
>
> On 3/14/2024 12:46 AM, Dmitry Baryshkov wrote:
> > On Wed, 13 Mar 2024 at 13:11, Satya Priya Kakitapalli
> > <[email protected]> wrote:
> >> Add device node for video clock controller on Qualcomm
> >> SM8150 platform.
> >>
> >> Signed-off-by: Satya Priya Kakitapalli <[email protected]>
> >> ---
> >> 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/[email protected]/
>
>
> >> + #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
> >>
> >>
> >
--
With best wishes
Dmitry
On Thu, 14 Mar 2024 at 11:19, Satya Priya Kakitapalli (Temp)
<[email protected]> wrote:
>
>
> On 3/14/2024 12:50 AM, Dmitry Baryshkov wrote:
> > On Wed, 13 Mar 2024 at 13:11, Satya Priya Kakitapalli
> > <[email protected]> wrote:
> >> Update the videocc device tree bindings for sm8150 to align with the
> >> latest convention.
> > But why? Bindings already exist. There is nothing wrong with them. And
> > sm8150 platform in general uses name-based lookup.
>
>
> With the new index based lookup introduced we cannot use this bindings,
> hence I moved to the sm8450-videocc bindings.
This is true for _new_ drivers. However you have a driver already. And
the driver has bindings. If you check, existing drivers were updated
from parent_names to fw_name / parent_hw lookups. However none of the
drivers was _updated_ to use index-based lookups.
> >> Fixes: 35d26e9292e2 ("dt-bindings: clock: Add YAML schemas for the QCOM VIDEOCC clock bindings")
> > It is not a fix, there is no bug that this commit is fixing.
>
>
> The clocks list needs to be fixed to add both XO and AHB clocks, and we
> are adding required-opps property.
Oh, so you have mixed two unrelated changes without telling anybody.
Please don't do this.
>
>
> >> Signed-off-by: Satya Priya Kakitapalli <[email protected]>
> >> ---
> >> 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
> >>
> >> reg:
> >> maxItems: 1
> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> >> index 6999e36ace1b..28d134ad9517 100644
> >> --- a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> >> +++ b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> >> @@ -17,7 +17,6 @@ description: |
> >> include/dt-bindings/clock/qcom,videocc-sc7180.h
> >> include/dt-bindings/clock/qcom,videocc-sc7280.h
> >> include/dt-bindings/clock/qcom,videocc-sdm845.h
> >> - include/dt-bindings/clock/qcom,videocc-sm8150.h
> >> include/dt-bindings/clock/qcom,videocc-sm8250.h
> >>
> >> properties:
> >> @@ -26,7 +25,6 @@ properties:
> >> - qcom,sc7180-videocc
> >> - qcom,sc7280-videocc
> >> - qcom,sdm845-videocc
> >> - - qcom,sm8150-videocc
> >> - qcom,sm8250-videocc
> >>
> >> clocks:
> >> @@ -75,7 +73,6 @@ allOf:
> >> enum:
> >> - qcom,sc7180-videocc
> >> - qcom,sdm845-videocc
> >> - - qcom,sm8150-videocc
> >> then:
> >> properties:
> >> clocks:
> >>
> >> --
> >> 2.25.1
> >>
> >>
> >
--
With best wishes
Dmitry