| 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 |
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
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>; > };
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
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 > >
On 24/12/2024 10:23, Michal Wilczynski wrote: > > > 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. Yeah, please drop my review tag and rework it to have two different devices. Driver design should not influence DTS. Best regards, Krzysztof
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
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(-)