diff mbox series

[06/14] dt-bindings: clock: milbeaut: add Milbeaut clock description

Message ID 1542589274-13878-7-git-send-email-sugaya.taichi@socionext.com (mailing list archive)
State New, archived
Headers show
Series Add basic support for Socionext Milbeaut M10V SoCs | expand

Commit Message

Sugaya Taichi Nov. 19, 2018, 1:01 a.m. UTC
Add DT bindings document for Milbeaut clock.

Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
---
 .../devicetree/bindings/clock/milbeaut-clock.txt   | 93 ++++++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/milbeaut-clock.txt

Comments

Stephen Boyd Nov. 30, 2018, 8:19 a.m. UTC | #1
Quoting Sugaya Taichi (2018-11-18 17:01:11)
> Add DT bindings document for Milbeaut clock.
> 
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
>  .../devicetree/bindings/clock/milbeaut-clock.txt   | 93 ++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/milbeaut-clock.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/milbeaut-clock.txt b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
> new file mode 100644
> index 0000000..5c093c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
> @@ -0,0 +1,93 @@
> +Milbeaut M10V Clock Controller Binding
> +----------------------------------------
> +Milbeaut clock controller is consists of few oscillators, PLL, multiplexer
> +and few divider modules
> +
> +This binding uses common clock bindings
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible: shall be "socionext,milbeaut-m10v-clk-regs"
> +- reg: shall contain base address and length of clock registers
> +- #clock-cells: shall be 0
> +
> +Example:
> +       m10v-clk-tree@ {
> +               compatible = "socionext,milbeaut-m10v-clk-regs";
> +               reg = <0x1d021000 0x4000>;
> +
> +               clocks {
> +                       #address-cells = <0>;
> +                       #size-cells = <0>;
> +
> +                       uclk40xi: uclk40xi {
> +                               compatible = "fixed-clock";
> +                               #clock-cells = <0>;
> +                               clock-frequency = <40000000>;
> +                       };
> +               };

This style of binding is highly discouraged. We don't describe each and
every clk in DT, we describe clk controllers and their outputs and
inputs in DT. The driver is the place where the clock controller
describes the internal clk topology of that controller. Also, fixed
frequency clks are typically oscillators and those would come from the
board dts file, but otherwise I wouldn't expect to see fixed frequency
clks in DT.

> +       }
> +
> +The clock consumer shall specify the desired clock-output of the clock
> +controller (as defined in [2]) by specifying output-id in its "clock"
> +phandle cell
> +[2] arch/arm/boot/dts/milbeaut-m10v-clk.h
> +
[...]
> +
> +Example
> +       piclk_mux_0: spiclk_mux_0 {
> +               compatible = "socionext,m10v-clk-div";
> +               #clock-cells = <0>;
> +               clocks = <&pll10_div_1_2>;
> +               offset = <bSPICLK>;
> +               mask = <0x3>;
> +               ratios = <4 0x5 2 0x4>;
> +       };
> +
> +       pll10: pll10 {
> +               compatible = "socionext,m10v-pll-fixed-factor";
> +               #clock-cells = <0>;
> +               clocks = <&uclk40xi>;
> +               offset = <10>;
> +               clock-div = <5>;
> +               clock-mult = <108>;
> +       };
> +
> +       emmcclk: emmcclk {
> +               compatible = "socionext,m10v-clk-div";
> +               #clock-cells = <0>;
> +               clocks = <&pll11>;
> +               offset = <bEMMCCLK>;
> +               mask = <0x3>;
> +               ratios = <15 0x7 10 0x6 9 0x5 8 0x4>;

Yeah, please no. This whole binding needs a rewrite to not have one node
per clk.
Sugaya Taichi Dec. 3, 2018, 8:08 a.m. UTC | #2
Hi,

Thank you for your comments.

On 2018/11/30 17:19, Stephen Boyd wrote:
> Quoting Sugaya Taichi (2018-11-18 17:01:11)
>> Add DT bindings document for Milbeaut clock.
>>
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
>> ---
>>   .../devicetree/bindings/clock/milbeaut-clock.txt   | 93 ++++++++++++++++++++++
>>   1 file changed, 93 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/milbeaut-clock.txt
>>
>> diff --git a/Documentation/devicetree/bindings/clock/milbeaut-clock.txt b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
>> new file mode 100644
>> index 0000000..5c093c8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
>> @@ -0,0 +1,93 @@
>> +Milbeaut M10V Clock Controller Binding
>> +----------------------------------------
>> +Milbeaut clock controller is consists of few oscillators, PLL, multiplexer
>> +and few divider modules
>> +
>> +This binding uses common clock bindings
>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +
>> +Required properties:
>> +- compatible: shall be "socionext,milbeaut-m10v-clk-regs"
>> +- reg: shall contain base address and length of clock registers
>> +- #clock-cells: shall be 0
>> +
>> +Example:
>> +       m10v-clk-tree@ {
>> +               compatible = "socionext,milbeaut-m10v-clk-regs";
>> +               reg = <0x1d021000 0x4000>;
>> +
>> +               clocks {
>> +                       #address-cells = <0>;
>> +                       #size-cells = <0>;
>> +
>> +                       uclk40xi: uclk40xi {
>> +                               compatible = "fixed-clock";
>> +                               #clock-cells = <0>;
>> +                               clock-frequency = <40000000>;
>> +                       };
>> +               };
> 
> This style of binding is highly discouraged. We don't describe each and
> every clk in DT, we describe clk controllers and their outputs and
> inputs in DT. The driver is the place where the clock controller
> describes the internal clk topology of that controller. Also, fixed
> frequency clks are typically oscillators and those would come from the
> board dts file, but otherwise I wouldn't expect to see fixed frequency
> clks in DT.

I understand. Received the similar comment from Rob in the DT part also.

> 
>> +       }
>> +
>> +The clock consumer shall specify the desired clock-output of the clock
>> +controller (as defined in [2]) by specifying output-id in its "clock"
>> +phandle cell
>> +[2] arch/arm/boot/dts/milbeaut-m10v-clk.h
>> +
> [...]
>> +
>> +Example
>> +       piclk_mux_0: spiclk_mux_0 {
>> +               compatible = "socionext,m10v-clk-div";
>> +               #clock-cells = <0>;
>> +               clocks = <&pll10_div_1_2>;
>> +               offset = <bSPICLK>;
>> +               mask = <0x3>;
>> +               ratios = <4 0x5 2 0x4>;
>> +       };
>> +
>> +       pll10: pll10 {
>> +               compatible = "socionext,m10v-pll-fixed-factor";
>> +               #clock-cells = <0>;
>> +               clocks = <&uclk40xi>;
>> +               offset = <10>;
>> +               clock-div = <5>;
>> +               clock-mult = <108>;
>> +       };
>> +
>> +       emmcclk: emmcclk {
>> +               compatible = "socionext,m10v-clk-div";
>> +               #clock-cells = <0>;
>> +               clocks = <&pll11>;
>> +               offset = <bEMMCCLK>;
>> +               mask = <0x3>;
>> +               ratios = <15 0x7 10 0x6 9 0x5 8 0x4>;
> 
> Yeah, please no. This whole binding needs a rewrite to not have one node
> per clk.

OK, I will renew the binding.

Thanks.
Sugaya Taichi

>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/milbeaut-clock.txt b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
new file mode 100644
index 0000000..5c093c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/milbeaut-clock.txt
@@ -0,0 +1,93 @@ 
+Milbeaut M10V Clock Controller Binding
+----------------------------------------
+Milbeaut clock controller is consists of few oscillators, PLL, multiplexer
+and few divider modules
+
+This binding uses common clock bindings
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible: shall be "socionext,milbeaut-m10v-clk-regs"
+- reg: shall contain base address and length of clock registers
+- #clock-cells: shall be 0
+
+Example:
+	m10v-clk-tree@ {
+		compatible = "socionext,milbeaut-m10v-clk-regs";
+		reg = <0x1d021000 0x4000>;
+
+		clocks {
+			#address-cells = <0>;
+			#size-cells = <0>;
+
+			uclk40xi: uclk40xi {
+				compatible = "fixed-clock";
+				#clock-cells = <0>;
+				clock-frequency = <40000000>;
+			};
+		};
+	}
+
+The clock consumer shall specify the desired clock-output of the clock
+controller (as defined in [2]) by specifying output-id in its "clock"
+phandle cell
+[2] arch/arm/boot/dts/milbeaut-m10v-clk.h
+
+For example for UART1:
+	usio1:  usio_uart@1e700010 {
+		index = <0>;
+		compatible = "socionext,milbeaut-m10v-usio-uart";
+		reg = <0x1e700010 0x10>;
+		interrupts = <0 141 0x4>, <0 149 0x4>;
+		interrupt-names = "rx", "tx";
+		clocks = <&hclk>;
+	};
+
+
+
+
+Required properties:
+- compatible:
+	"socionext,milbeaut-m10v-clk-mux"
+		-clock-cells: should be 0
+		-clocks: should be two factor
+	"socionext,milbeaut-m10v-pll-fixed-factor"
+		-clock-cells: should be 0
+		-clocks: should be one factor
+		-offset: offset
+		-clock-div: div number
+		-clock-mult: multiple number
+	"socionext,milbeaut-m10v-clk-div"
+		-clock-cells: should be 0
+		-clocks: should be one factor
+		-offset: offset
+		-mask: mask bit
+		-ratios: div ratio
+
+Example
+	piclk_mux_0: spiclk_mux_0 {
+		compatible = "socionext,m10v-clk-div";
+		#clock-cells = <0>;
+		clocks = <&pll10_div_1_2>;
+		offset = <bSPICLK>;
+		mask = <0x3>;
+		ratios = <4 0x5 2 0x4>;
+	};
+
+	pll10: pll10 {
+		compatible = "socionext,m10v-pll-fixed-factor";
+		#clock-cells = <0>;
+		clocks = <&uclk40xi>;
+		offset = <10>;
+		clock-div = <5>;
+		clock-mult = <108>;
+	};
+
+	emmcclk: emmcclk {
+		compatible = "socionext,m10v-clk-div";
+		#clock-cells = <0>;
+		clocks = <&pll11>;
+		offset = <bEMMCCLK>;
+		mask = <0x3>;
+		ratios = <15 0x7 10 0x6 9 0x5 8 0x4>;
+	};