[v8] add Amlogic A1 clock controller drivers

[PATCH v8 11/11] arm64: dts: meson: a1: introduce PLL and Peripherals clk controllers

Posted by Dmitry Rokosov 2 years, 9 months ago

This patch adds clkc_periphs and clkc_pll dts nodes to A1 SoC main dtsi.
The first one clk controller is responsible for all SoC peripherals
clocks excluding audio clocks. The second one clk controller is used by
A1 SoC PLLs. Actually, there are two different APB heads, so we have two
different drivers.

Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 27 ++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index b4000cf65a9a..38e6517c603c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -6,6 +6,8 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/gpio/meson-a1-gpio.h>
+#include <dt-bindings/clock/a1-pll-clkc.h>
+#include <dt-bindings/clock/a1-clkc.h>
 
 / {
 	compatible = "amlogic,a1";
@@ -81,7 +83,6 @@ apb: bus@fe000000 {
 			#size-cells = <2>;
 			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x1000000>;
 
-
 			reset: reset-controller@0 {
 				compatible = "amlogic,meson-a1-reset";
 				reg = <0x0 0x0 0x0 0x8c>;
@@ -124,6 +125,30 @@ uart_AO_B: serial@2000 {
 				clock-names = "xtal", "pclk", "baud";
 				status = "disabled";
 			};
+
+			clkc_periphs: periphs-clock-controller@800 {
+				compatible = "amlogic,a1-periphs-clkc";
+				reg = <0 0x800 0 0x104>;
+				#clock-cells = <1>;
+				clocks = <&clkc_pll CLKID_FCLK_DIV2>,
+					 <&clkc_pll CLKID_FCLK_DIV3>,
+					 <&clkc_pll CLKID_FCLK_DIV5>,
+					 <&clkc_pll CLKID_FCLK_DIV7>,
+					 <&clkc_pll CLKID_HIFI_PLL>,
+					 <&xtal>;
+				clock-names = "fclk_div2", "fclk_div3",
+					      "fclk_div5", "fclk_div7",
+					      "hifi_pll", "xtal";
+			};
+
+			clkc_pll: pll-clock-controller@7c80 {
+				compatible = "amlogic,a1-pll-clkc";
+				reg = <0 0x7c80 0 0x18c>;
+				#clock-cells = <1>;
+				clocks = <&clkc_periphs CLKID_XTAL_FIXPLL>,
+					 <&clkc_periphs CLKID_XTAL_HIFIPLL>;
+				clock-names = "xtal_fixpll", "xtal_hifipll";
+			};
 		};
 
 		gic: interrupt-controller@ff901000 {
-- 
2.36.0

Re: [PATCH v8 11/11] arm64: dts: meson: a1: introduce PLL and Peripherals clk controllers

Posted by Jerome Brunet 2 years, 9 months ago

On Fri 02 Dec 2022 at 01:57, Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:

> This patch adds clkc_periphs and clkc_pll dts nodes to A1 SoC main dtsi.
> The first one clk controller is responsible for all SoC peripherals
> clocks excluding audio clocks. The second one clk controller is used by
> A1 SoC PLLs. Actually, there are two different APB heads, so we have two
> different drivers.

Please send this change through a separate patcheset.

One patcheset/series for clk (and bindings)
Another one for the DTS (usually sent after the first one is accepted)

>
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 27 ++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> index b4000cf65a9a..38e6517c603c 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> @@ -6,6 +6,8 @@
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/gpio/meson-a1-gpio.h>
> +#include <dt-bindings/clock/a1-pll-clkc.h>
> +#include <dt-bindings/clock/a1-clkc.h>
>  
>  / {
>  	compatible = "amlogic,a1";
> @@ -81,7 +83,6 @@ apb: bus@fe000000 {
>  			#size-cells = <2>;
>  			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x1000000>;
>  
> -
>  			reset: reset-controller@0 {
>  				compatible = "amlogic,meson-a1-reset";
>  				reg = <0x0 0x0 0x0 0x8c>;
> @@ -124,6 +125,30 @@ uart_AO_B: serial@2000 {
>  				clock-names = "xtal", "pclk", "baud";
>  				status = "disabled";
>  			};
> +
> +			clkc_periphs: periphs-clock-controller@800 {

device name should be generic so

clkc_periphs: clock-controller@800 would be better

> +				compatible = "amlogic,a1-periphs-clkc";
> +				reg = <0 0x800 0 0x104>;
> +				#clock-cells = <1>;
> +				clocks = <&clkc_pll CLKID_FCLK_DIV2>,
> +					 <&clkc_pll CLKID_FCLK_DIV3>,
> +					 <&clkc_pll CLKID_FCLK_DIV5>,
> +					 <&clkc_pll CLKID_FCLK_DIV7>,
> +					 <&clkc_pll CLKID_HIFI_PLL>,
> +					 <&xtal>;
> +				clock-names = "fclk_div2", "fclk_div3",
> +					      "fclk_div5", "fclk_div7",
> +					      "hifi_pll", "xtal";
> +			};
> +
> +			clkc_pll: pll-clock-controller@7c80 {

Same here

> +				compatible = "amlogic,a1-pll-clkc";
> +				reg = <0 0x7c80 0 0x18c>;
> +				#clock-cells = <1>;
> +				clocks = <&clkc_periphs CLKID_XTAL_FIXPLL>,
> +					 <&clkc_periphs CLKID_XTAL_HIFIPLL>;
> +				clock-names = "xtal_fixpll", "xtal_hifipll";
> +			};
>  		};
>  
>  		gic: interrupt-controller@ff901000 {

Re: [PATCH v8 11/11] arm64: dts: meson: a1: introduce PLL and Peripherals clk controllers

Posted by neil.armstrong@linaro.org 2 years, 9 months ago

On 02/12/2022 13:03, Jerome Brunet wrote:
> 
> On Fri 02 Dec 2022 at 01:57, Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
> 
>> This patch adds clkc_periphs and clkc_pll dts nodes to A1 SoC main dtsi.
>> The first one clk controller is responsible for all SoC peripherals
>> clocks excluding audio clocks. The second one clk controller is used by
>> A1 SoC PLLs. Actually, there are two different APB heads, so we have two
>> different drivers.
> 
> Please send this change through a separate patcheset.
> 
> One patcheset/series for clk (and bindings)
> Another one for the DTS (usually sent after the first one is accepted)

Yes please split out the DT in a separate patchset, but only send then once
the bindings are fully reviewed and accepted.

Start from v1 for the DT patchset, no need to continue the current numbering.

Thanks,
Neil

> 
>>
>> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
>> ---
>>   arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 27 ++++++++++++++++++++++-
>>   1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> index b4000cf65a9a..38e6517c603c 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> @@ -6,6 +6,8 @@
>>   #include <dt-bindings/interrupt-controller/irq.h>
>>   #include <dt-bindings/interrupt-controller/arm-gic.h>
>>   #include <dt-bindings/gpio/meson-a1-gpio.h>
>> +#include <dt-bindings/clock/a1-pll-clkc.h>
>> +#include <dt-bindings/clock/a1-clkc.h>
>>   
>>   / {
>>   	compatible = "amlogic,a1";
>> @@ -81,7 +83,6 @@ apb: bus@fe000000 {
>>   			#size-cells = <2>;
>>   			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x1000000>;
>>   
>> -
>>   			reset: reset-controller@0 {
>>   				compatible = "amlogic,meson-a1-reset";
>>   				reg = <0x0 0x0 0x0 0x8c>;
>> @@ -124,6 +125,30 @@ uart_AO_B: serial@2000 {
>>   				clock-names = "xtal", "pclk", "baud";
>>   				status = "disabled";
>>   			};
>> +
>> +			clkc_periphs: periphs-clock-controller@800 {
> 
> device name should be generic so
> 
> clkc_periphs: clock-controller@800 would be better
> 
>> +				compatible = "amlogic,a1-periphs-clkc";
>> +				reg = <0 0x800 0 0x104>;
>> +				#clock-cells = <1>;
>> +				clocks = <&clkc_pll CLKID_FCLK_DIV2>,
>> +					 <&clkc_pll CLKID_FCLK_DIV3>,
>> +					 <&clkc_pll CLKID_FCLK_DIV5>,
>> +					 <&clkc_pll CLKID_FCLK_DIV7>,
>> +					 <&clkc_pll CLKID_HIFI_PLL>,
>> +					 <&xtal>;
>> +				clock-names = "fclk_div2", "fclk_div3",
>> +					      "fclk_div5", "fclk_div7",
>> +					      "hifi_pll", "xtal";
>> +			};
>> +
>> +			clkc_pll: pll-clock-controller@7c80 {
> 
> Same here
> 
>> +				compatible = "amlogic,a1-pll-clkc";
>> +				reg = <0 0x7c80 0 0x18c>;
>> +				#clock-cells = <1>;
>> +				clocks = <&clkc_periphs CLKID_XTAL_FIXPLL>,
>> +					 <&clkc_periphs CLKID_XTAL_HIFIPLL>;
>> +				clock-names = "xtal_fixpll", "xtal_hifipll";
>> +			};
>>   		};
>>   
>>   		gic: interrupt-controller@ff901000 {
>

Re: [PATCH v8 11/11] arm64: dts: meson: a1: introduce PLL and Peripherals clk controllers

Posted by Krzysztof Kozlowski 2 years, 9 months ago

On 01/12/2022 23:57, Dmitry Rokosov wrote:
> This patch adds clkc_periphs and clkc_pll dts nodes to A1 SoC main dtsi.
> The first one clk controller is responsible for all SoC peripherals
> clocks excluding audio clocks. The second one clk controller is used by
> A1 SoC PLLs. Actually, there are two different APB heads, so we have two
> different drivers.
> 
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
>  arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 27 ++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> index b4000cf65a9a..38e6517c603c 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> @@ -6,6 +6,8 @@
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/gpio/meson-a1-gpio.h>
> +#include <dt-bindings/clock/a1-pll-clkc.h>
> +#include <dt-bindings/clock/a1-clkc.h>
>  
>  / {
>  	compatible = "amlogic,a1";
> @@ -81,7 +83,6 @@ apb: bus@fe000000 {
>  			#size-cells = <2>;
>  			ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x1000000>;
>  
> -
>  			reset: reset-controller@0 {
>  				compatible = "amlogic,meson-a1-reset";
>  				reg = <0x0 0x0 0x0 0x8c>;
> @@ -124,6 +125,30 @@ uart_AO_B: serial@2000 {
>  				clock-names = "xtal", "pclk", "baud";
>  				status = "disabled";
>  			};
> +
> +			clkc_periphs: periphs-clock-controller@800 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +				compatible = "amlogic,a1-periphs-clkc";
> +				reg = <0 0x800 0 0x104>;
> +				#clock-cells = <1>;
> +				clocks = <&clkc_pll CLKID_FCLK_DIV2>,
> +					 <&clkc_pll CLKID_FCLK_DIV3>,
> +					 <&clkc_pll CLKID_FCLK_DIV5>,
> +					 <&clkc_pll CLKID_FCLK_DIV7>,
> +					 <&clkc_pll CLKID_HIFI_PLL>,
> +					 <&xtal>;
> +				clock-names = "fclk_div2", "fclk_div3",
> +					      "fclk_div5", "fclk_div7",
> +					      "hifi_pll", "xtal";
> +			};
> +
> +			clkc_pll: pll-clock-controller@7c80 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +				compatible = "amlogic,a1-pll-clkc";


Best regards,
Krzysztof