diff mbox series

[RFC,v2,01/19] dt-bindings: clock: Add VO subsystem clocks and update address requirements

Message ID 20241223125553.3527812-2-m.wilczynski@samsung.com (mailing list archive)
State New
Headers show
Series [RFC,v2,01/19] dt-bindings: clock: Add VO subsystem clocks and update address requirements | expand

Commit Message

Michal Wilczynski Dec. 23, 2024, 12:55 p.m. UTC
The T-Head TH1520 SoC’s AP clock controller now needs two address ranges
to manage both the Application Processor (AP) and Video Output (VO)
subsystem clocks. Update the device tree bindings to require two `reg`
entries, one for the AP clocks and one for the VO clocks.

Additionally, introduce new VO subsystem clock constants in the header
file. These constants will be used by the driver to control VO-related
components such as display and graphics units.

Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
---
 .../bindings/clock/thead,th1520-clk-ap.yaml   | 15 +++++++--
 .../dt-bindings/clock/thead,th1520-clk-ap.h   | 33 +++++++++++++++++++
 2 files changed, 45 insertions(+), 3 deletions(-)

Comments

Krzysztof Kozlowski Dec. 23, 2024, 4:05 p.m. UTC | #1
On 23/12/2024 13:55, Michal Wilczynski wrote:

>  description: |
>    The T-HEAD TH1520 AP sub-system clock controller configures the
> -  CPU, DPU, GMAC and TEE PLLs.
> +  CPU, DPU, GMAC and TEE PLLs. Additionally the VO subsystem configures
> +  the clock gates for the HDMI, MIPI and the GPU.
>  
>    SoC reference manual
>    https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf
> @@ -23,7 +24,13 @@ properties:
>      const: thead,th1520-clk-ap
>  
>    reg:
> -    maxItems: 1
> +    minItems: 2

You can drop minItems in this case.

> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: ap-clks
> +      - const: vo-clks


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof
Stephen Boyd Dec. 23, 2024, 8:50 p.m. UTC | #2
Quoting Michal Wilczynski (2024-12-23 04:55:35)
> The T-Head TH1520 SoC’s AP clock controller now needs two address ranges
> to manage both the Application Processor (AP) and Video Output (VO)
> subsystem clocks. Update the device tree bindings to require two `reg`
> entries, one for the AP clocks and one for the VO clocks.
> 
> Additionally, introduce new VO subsystem clock constants in the header
> file. These constants will be used by the driver to control VO-related
> components such as display and graphics units.
> 
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> ---
[...]
> diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> index 0129bd0ba4b3..f0df97a450ef 100644
> --- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> +++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> @@ -47,7 +54,9 @@ examples:
>      #include <dt-bindings/clock/thead,th1520-clk-ap.h>
>      clock-controller@ef010000 {
>          compatible = "thead,th1520-clk-ap";
> -        reg = <0xef010000 0x1000>;
> +        reg = <0xef010000 0x1000>,
> +              <0xff010000 0x1000>;

I don't get it. Why not have two nodes and two devices? They have
different register regions so likely they're different devices on the
internal SoC bus. They may have the same input clks, but otherwise I
don't see how they're the same node.

> +        reg-names = "ap-clks", "vo-clks";
>          clocks = <&osc>;
>          #clock-cells = <1>;
>      };
Krzysztof Kozlowski Dec. 24, 2024, 8:53 a.m. UTC | #3
On Mon, Dec 23, 2024 at 12:50:59PM -0800, Stephen Boyd wrote:
> Quoting Michal Wilczynski (2024-12-23 04:55:35)
> > The T-Head TH1520 SoC’s AP clock controller now needs two address ranges
> > to manage both the Application Processor (AP) and Video Output (VO)
> > subsystem clocks. Update the device tree bindings to require two `reg`
> > entries, one for the AP clocks and one for the VO clocks.
> > 
> > Additionally, introduce new VO subsystem clock constants in the header
> > file. These constants will be used by the driver to control VO-related
> > components such as display and graphics units.
> > 
> > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> > ---
> [...]
> > diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> > index 0129bd0ba4b3..f0df97a450ef 100644
> > --- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> > +++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
> > @@ -47,7 +54,9 @@ examples:
> >      #include <dt-bindings/clock/thead,th1520-clk-ap.h>
> >      clock-controller@ef010000 {
> >          compatible = "thead,th1520-clk-ap";
> > -        reg = <0xef010000 0x1000>;
> > +        reg = <0xef010000 0x1000>,
> > +              <0xff010000 0x1000>;
> 
> I don't get it. Why not have two nodes and two devices? They have
> different register regions so likely they're different devices on the
> internal SoC bus. They may have the same input clks, but otherwise I
> don't see how they're the same node.

That's a good point. Aren't here simply two different clock controllers?

Best regards,
Krzysztof
Michal Wilczynski Dec. 24, 2024, 9:23 a.m. UTC | #4
On 12/24/24 09:53, Krzysztof Kozlowski wrote:
> On Mon, Dec 23, 2024 at 12:50:59PM -0800, Stephen Boyd wrote:
>> Quoting Michal Wilczynski (2024-12-23 04:55:35)
>>> The T-Head TH1520 SoC’s AP clock controller now needs two address ranges
>>> to manage both the Application Processor (AP) and Video Output (VO)
>>> subsystem clocks. Update the device tree bindings to require two `reg`
>>> entries, one for the AP clocks and one for the VO clocks.
>>>
>>> Additionally, introduce new VO subsystem clock constants in the header
>>> file. These constants will be used by the driver to control VO-related
>>> components such as display and graphics units.
>>>
>>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>>> ---
>> [...]
>>> diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>>> index 0129bd0ba4b3..f0df97a450ef 100644
>>> --- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
>>> @@ -47,7 +54,9 @@ examples:
>>>      #include <dt-bindings/clock/thead,th1520-clk-ap.h>
>>>      clock-controller@ef010000 {
>>>          compatible = "thead,th1520-clk-ap";
>>> -        reg = <0xef010000 0x1000>;
>>> +        reg = <0xef010000 0x1000>,
>>> +              <0xff010000 0x1000>;
>>
>> I don't get it. Why not have two nodes and two devices? They have
>> different register regions so likely they're different devices on the
>> internal SoC bus. They may have the same input clks, but otherwise I
>> don't see how they're the same node.
> 
> That's a good point. Aren't here simply two different clock controllers?

Yeah there are two clock controllers, based on the review comments I was
trying to re-use the driver, but the driver can also be re-used to serve
multiple nodes and multiple compatible and .data properties, if that's
fine with you that's how it will look like in v3.

Thanks,
Michał
> 
> Best regards,
> Krzysztof
> 
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
index 0129bd0ba4b3..f0df97a450ef 100644
--- a/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
+++ b/Documentation/devicetree/bindings/clock/thead,th1520-clk-ap.yaml
@@ -8,7 +8,8 @@  title: T-HEAD TH1520 AP sub-system clock controller
 
 description: |
   The T-HEAD TH1520 AP sub-system clock controller configures the
-  CPU, DPU, GMAC and TEE PLLs.
+  CPU, DPU, GMAC and TEE PLLs. Additionally the VO subsystem configures
+  the clock gates for the HDMI, MIPI and the GPU.
 
   SoC reference manual
   https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf
@@ -23,7 +24,13 @@  properties:
     const: thead,th1520-clk-ap
 
   reg:
-    maxItems: 1
+    minItems: 2
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: ap-clks
+      - const: vo-clks
 
   clocks:
     items:
@@ -47,7 +54,9 @@  examples:
     #include <dt-bindings/clock/thead,th1520-clk-ap.h>
     clock-controller@ef010000 {
         compatible = "thead,th1520-clk-ap";
-        reg = <0xef010000 0x1000>;
+        reg = <0xef010000 0x1000>,
+              <0xff010000 0x1000>;
+        reg-names = "ap-clks", "vo-clks";
         clocks = <&osc>;
         #clock-cells = <1>;
     };
diff --git a/include/dt-bindings/clock/thead,th1520-clk-ap.h b/include/dt-bindings/clock/thead,th1520-clk-ap.h
index a199784b3512..8b9a98f878a6 100644
--- a/include/dt-bindings/clock/thead,th1520-clk-ap.h
+++ b/include/dt-bindings/clock/thead,th1520-clk-ap.h
@@ -93,4 +93,37 @@ 
 #define CLK_SRAM3		83
 #define CLK_PLL_GMAC_100M	84
 #define CLK_UART_SCLK		85
+
+/* VO clocks */
+#define CLK_AXI4_VO_ACLK		86
+#define CLK_GPU_CORE			87
+#define CLK_GPU_CFG_ACLK		88
+#define CLK_DPU_PIXELCLK0		89
+#define CLK_DPU_PIXELCLK1		90
+#define CLK_DPU_HCLK			91
+#define CLK_DPU_ACLK			92
+#define CLK_DPU_CCLK			93
+#define CLK_HDMI_SFR			94
+#define CLK_HDMI_PCLK			95
+#define CLK_HDMI_CEC			96
+#define CLK_MIPI_DSI0_PCLK		97
+#define CLK_MIPI_DSI1_PCLK		98
+#define CLK_MIPI_DSI0_CFG		99
+#define CLK_MIPI_DSI1_CFG		100
+#define CLK_MIPI_DSI0_REFCLK		101
+#define CLK_MIPI_DSI1_REFCLK		102
+#define CLK_HDMI_I2S			103
+#define CLK_X2H_DPU1_ACLK		104
+#define CLK_X2H_DPU_ACLK		105
+#define CLK_AXI4_VO_PCLK		106
+#define CLK_IOPMP_VOSYS_DPU_PCLK	107
+#define CLK_IOPMP_VOSYS_DPU1_PCLK	108
+#define CLK_IOPMP_VOSYS_GPU_PCLK	109
+#define CLK_IOPMP_DPU1_ACLK		110
+#define CLK_IOPMP_DPU_ACLK		111
+#define CLK_IOPMP_GPU_ACLK		112
+#define CLK_MIPIDSI0_PIXCLK		113
+#define CLK_MIPIDSI1_PIXCLK		114
+#define CLK_HDMI_PIXCLK			115
+
 #endif