lkml.org 
[lkml]   [2018]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/7] clk: intel: Add clock driver for GRX500 SoC
From
Date


On 6/13/2018 6:37 AM, Rob Herring wrote:
> On Tue, Jun 12, 2018 at 01:40:29PM +0800, Songjun Wu wrote:
>> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>>
>> PLL of GRX500 provide clock to DDR, CPU, and peripherals as show below
>>
>> +---------+
>> |--->| LCPLL3 0|--PCIe clk-->
>> XO | +---------+
>> +-----------|
>> | +---------+
>> | | 3|--PAE clk-->
>> |--->| PLL0B 2|--GSWIP clk-->
>> | | 1|--DDR clk-->DDR PHY clk-->
>> | | 0|--CPU1 clk--+ +-----+
>> | +---------+ |--->0 |
>> | | MUX |--CPU clk-->
>> | +---------+ |--->1 |
>> | | 0|--CPU0 clk--+ +-----+
>> |--->| PLLOA 1|--SSX4 clk-->
>> | 2|--NGI clk-->
>> | 3|--CBM clk-->
>> +---------+
>>
>> VCO of all PLLs of GRX500 is not supposed to be reprogrammed.
>> DDR PHY clock is created to show correct clock rate in software
>> point of view.
>> CPU clock of 1Ghz from PLL0B otherwise from PLL0A.
>> Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
>>
>> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
>
> Need a blank line before the SoB's and not one in the middle.
Thanks.
Blank line in the middle of sign-off will be removed.
A new blank line will be added before SoB's.

>
>> ---
>>
>> .../devicetree/bindings/clock/intel,grx500-clk.txt | 46 ++
>
> Please split bindings to separate patch.
>
>> drivers/clk/Kconfig | 1 +
>> drivers/clk/Makefile | 1 +
>> drivers/clk/intel/Kconfig | 21 +
>> drivers/clk/intel/Makefile | 7 +
>> drivers/clk/intel/clk-cgu-api.c | 676 +++++++++++++++++++++
>> drivers/clk/intel/clk-cgu-api.h | 120 ++++
>> drivers/clk/intel/clk-grx500.c | 236 +++++++
>> include/dt-bindings/clock/intel,grx500-clk.h | 61 ++
>> 9 files changed, 1169 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
>> create mode 100644 drivers/clk/intel/Kconfig
>> create mode 100644 drivers/clk/intel/Makefile
>> create mode 100644 drivers/clk/intel/clk-cgu-api.c
>> create mode 100644 drivers/clk/intel/clk-cgu-api.h
>> create mode 100644 drivers/clk/intel/clk-grx500.c
>> create mode 100644 include/dt-bindings/clock/intel,grx500-clk.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt b/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
>> new file mode 100644
>> index 000000000000..dd761d900dc9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
>> @@ -0,0 +1,46 @@
>> +Device Tree Clock bindings for GRX500 PLL controller.
>> +
>> +This binding uses the common clock binding:
>> + Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +
>> +This GRX500 PLL controller provides the 5 main clock domain of the SoC: CPU/DDR, XBAR,
>> +Voice, WLAN, PCIe and gate clocks for HW modules.
>> +
>> +Required properties for osc clock node
>> +- compatible: Should be intel,grx500-xxx-clk
>
> These would need to be enumerated with all possible values. However, see
> below.
All possible values will be listed.

>
>> +- reg: offset address of the controller memory area.
>> +- clocks: phandle of the external reference clock
>> +- #clock-cells: can be one or zero.
>> +- clock-output-names: Names of the output clocks.
>> +
>> +Example:
>> + pll0aclk: pll0aclk {
>> + #clock-cells = <1>;
>> + compatible = "intel,grx500-pll0a-clk";
>> + clocks = <&pll0a>;
>> + reg = <0x8>;
>> + clock-output-names = "cbmclk", "ngiclk", "ssx4clk", "cpu0clk";
>> + };
>> +
>> + cpuclk: cpuclk {
>> + #clock-cells = <0>;
>> + compatible = "intel,grx500-cpu-clk";
>> + clocks = <&pll0aclk CPU0_CLK>, <&pll0bclk CPU1_CLK>;
>> + reg = <0x8>;
>> + clock-output-names = "cpu";
>> + };
>> +
>> +Required properties for gate node:
>> +- compatible: Should be intel,grx500-gatex-clk
>> +- reg: offset address of the controller memory area.
>> +- #clock-cells: Should be <1>
>> +- clock-output-names: Names of the output clocks.
>> +
>> +Example:
>> + clkgate0: clkgate0 {
>> + #clock-cells = <1>;
>> + compatible = "intel,grx500-gate0-clk";
>> + reg = <0x114>;
>> + clock-output-names = "gate_xbar0", "gate_xbar1", "gate_xbar2",
>> + "gate_xbar3", "gate_xbar6", "gate_xbar7";
>> + };
>
> We generally don't do a clock node per clock or few clocks but rather 1
> clock node per clock controller block. See any recent clock bindings.
>
> Rob
Do you mean only one example is needed per clock controller block?
cpuclk is not needed in the document?

\
 
 \ /
  Last update: 2018-06-14 10:41    [W:0.082 / U:0.720 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site