Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs

2018-08-08 Thread yixin zhu




On 8/8/2018 1:50 PM, Stephen Boyd wrote:

Quoting Songjun Wu (2018-08-02 20:02:21)

From: Yixin Zhu 

This driver provides PLL clock registration as well as various clock
branches, e.g. MUX clock, gate clock, divider clock and so on.

PLLs that provide clock to DDR, CPU and peripherals are shown 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-->
  +-+

Thanks for the picture!

Thanks for the review.




Signed-off-by: Yixin Zhu 
Signed-off-by: Songjun Wu 
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 0bb25dd009d1..d929ca4607cf 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -72,6 +72,9 @@ obj-$(CONFIG_ARCH_HISI)   += hisilicon/
  obj-y  += imgtec/
  obj-$(CONFIG_ARCH_MXC) += imx/
  obj-$(CONFIG_MACH_INGENIC) += ingenic/
+ifeq ($(CONFIG_COMMON_CLK), y)
+obj-y  +=intel/
+endif

Why not obj-$(CONFIG_INTEL_CCF) or something like that?

Will use obj-$(CONFIG_INTEL_CCF)


  obj-$(CONFIG_ARCH_KEYSTONE)+= keystone/
  obj-$(CONFIG_MACH_LOONGSON32)  += loongson1/
  obj-y  += mediatek/
diff --git a/drivers/clk/intel/Kconfig b/drivers/clk/intel/Kconfig
new file mode 100644
index ..c7d3fb1721fa
--- /dev/null
+++ b/drivers/clk/intel/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0
+config INTEL_CGU_CLK
+   depends on COMMON_CLK
+   depends on INTEL_MIPS || COMPILE_TEST
+   select MFD_SYSCON
+   bool "Intel clock controller support"
+   help
+ This driver support Intel CGU (Clock Generation Unit).

Is it really called a clock generation unit? Or that's just copied from
sunxi driver?

Yes,  It's called clock generation unit(CGU) in our HW chip spec.


+
+choice
+   prompt "SoC platform selection"
+   depends on INTEL_CGU_CLK
+   default INTEL_GRX500_CGU_CLK
+
+config INTEL_GRX500_CGU_CLK
+   bool "GRX500 CLK"
+   help
+ Clock driver of GRX500 platform.
+
+endchoice
diff --git a/drivers/clk/intel/Makefile b/drivers/clk/intel/Makefile
new file mode 100644
index ..16a0138e52c2
--- /dev/null
+++ b/drivers/clk/intel/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for intel specific clk
+
+obj-$(CONFIG_INTEL_CGU_CLK) += clk-cgu.o clk-cgu-pll.o
+ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),)
+   obj-y += clk-grx500.o
+endif
diff --git a/drivers/clk/intel/clk-cgu-pll.c b/drivers/clk/intel/clk-cgu-pll.c
new file mode 100644
index ..20759bc27e95
--- /dev/null
+++ b/drivers/clk/intel/clk-cgu-pll.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2018 Intel Corporation.
+ *  Zhu YiXin 
+ */
+
+#include 

Is this include used?

Not used. Will remove it.


+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "clk-cgu-pll.h"
+#include "clk-cgu.h"
+
+#define to_intel_clk_pll(_hw)  container_of(_hw, struct intel_clk_pll, hw)
+
+/*
+ * Calculate formula:
+ * rate = (prate * mult + (prate * frac) / frac_div) / div
+ */
+static unsigned long
+intel_pll_calc_rate(unsigned long prate, unsigned int mult,
+   unsigned int div, unsigned int frac,
+   unsigned int frac_div)
+{
+   u64 crate, frate, rate64;
+
+   rate64 = prate;
+   crate = rate64 * mult;
+
+   if (frac) {
+   frate = rate64 * frac;
+   do_div(frate, frac_div);
+   crate += frate;
+   }
+   do_div(crate, div);
+
+   return (unsigned long)crate;
+}
+
+static void
+grx500_pll_get_params(struct intel_clk_pll *pll, unsigned int *mult,
+ unsigned int *frac)
+{
+   *mult = intel_get_clk_val(pll->map, pll->reg, 2, 7);
+   *frac = intel_get_clk_val(pll->map, pll->reg, 9, 21);
+}
+
+static int intel_wait_pll_lock(struct intel_clk_pll *pll, int bit_idx)
+{
+   unsigned int val;
+
+   return regmap_read_poll_timeout(pll->map, pll->reg, val,
+   val & BIT(bit_idx), 10, 1000);
+}
+
+static unsigned long
+intel_grx500_pll_recalc_rate(struct clk_hw *hw,

Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs

2018-08-08 Thread yixin zhu




On 8/8/2018 1:50 PM, Stephen Boyd wrote:

Quoting Songjun Wu (2018-08-02 20:02:21)

From: Yixin Zhu 

This driver provides PLL clock registration as well as various clock
branches, e.g. MUX clock, gate clock, divider clock and so on.

PLLs that provide clock to DDR, CPU and peripherals are shown 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-->
  +-+

Thanks for the picture!

Thanks for the review.




Signed-off-by: Yixin Zhu 
Signed-off-by: Songjun Wu 
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 0bb25dd009d1..d929ca4607cf 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -72,6 +72,9 @@ obj-$(CONFIG_ARCH_HISI)   += hisilicon/
  obj-y  += imgtec/
  obj-$(CONFIG_ARCH_MXC) += imx/
  obj-$(CONFIG_MACH_INGENIC) += ingenic/
+ifeq ($(CONFIG_COMMON_CLK), y)
+obj-y  +=intel/
+endif

Why not obj-$(CONFIG_INTEL_CCF) or something like that?

Will use obj-$(CONFIG_INTEL_CCF)


  obj-$(CONFIG_ARCH_KEYSTONE)+= keystone/
  obj-$(CONFIG_MACH_LOONGSON32)  += loongson1/
  obj-y  += mediatek/
diff --git a/drivers/clk/intel/Kconfig b/drivers/clk/intel/Kconfig
new file mode 100644
index ..c7d3fb1721fa
--- /dev/null
+++ b/drivers/clk/intel/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0
+config INTEL_CGU_CLK
+   depends on COMMON_CLK
+   depends on INTEL_MIPS || COMPILE_TEST
+   select MFD_SYSCON
+   bool "Intel clock controller support"
+   help
+ This driver support Intel CGU (Clock Generation Unit).

Is it really called a clock generation unit? Or that's just copied from
sunxi driver?

Yes,  It's called clock generation unit(CGU) in our HW chip spec.


+
+choice
+   prompt "SoC platform selection"
+   depends on INTEL_CGU_CLK
+   default INTEL_GRX500_CGU_CLK
+
+config INTEL_GRX500_CGU_CLK
+   bool "GRX500 CLK"
+   help
+ Clock driver of GRX500 platform.
+
+endchoice
diff --git a/drivers/clk/intel/Makefile b/drivers/clk/intel/Makefile
new file mode 100644
index ..16a0138e52c2
--- /dev/null
+++ b/drivers/clk/intel/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for intel specific clk
+
+obj-$(CONFIG_INTEL_CGU_CLK) += clk-cgu.o clk-cgu-pll.o
+ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),)
+   obj-y += clk-grx500.o
+endif
diff --git a/drivers/clk/intel/clk-cgu-pll.c b/drivers/clk/intel/clk-cgu-pll.c
new file mode 100644
index ..20759bc27e95
--- /dev/null
+++ b/drivers/clk/intel/clk-cgu-pll.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2018 Intel Corporation.
+ *  Zhu YiXin 
+ */
+
+#include 

Is this include used?

Not used. Will remove it.


+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "clk-cgu-pll.h"
+#include "clk-cgu.h"
+
+#define to_intel_clk_pll(_hw)  container_of(_hw, struct intel_clk_pll, hw)
+
+/*
+ * Calculate formula:
+ * rate = (prate * mult + (prate * frac) / frac_div) / div
+ */
+static unsigned long
+intel_pll_calc_rate(unsigned long prate, unsigned int mult,
+   unsigned int div, unsigned int frac,
+   unsigned int frac_div)
+{
+   u64 crate, frate, rate64;
+
+   rate64 = prate;
+   crate = rate64 * mult;
+
+   if (frac) {
+   frate = rate64 * frac;
+   do_div(frate, frac_div);
+   crate += frate;
+   }
+   do_div(crate, div);
+
+   return (unsigned long)crate;
+}
+
+static void
+grx500_pll_get_params(struct intel_clk_pll *pll, unsigned int *mult,
+ unsigned int *frac)
+{
+   *mult = intel_get_clk_val(pll->map, pll->reg, 2, 7);
+   *frac = intel_get_clk_val(pll->map, pll->reg, 9, 21);
+}
+
+static int intel_wait_pll_lock(struct intel_clk_pll *pll, int bit_idx)
+{
+   unsigned int val;
+
+   return regmap_read_poll_timeout(pll->map, pll->reg, val,
+   val & BIT(bit_idx), 10, 1000);
+}
+
+static unsigned long
+intel_grx500_pll_recalc_rate(struct clk_hw *hw,

Re: [PATCH v2 03/18] dt-bindings: clk: Add documentation of grx500 clock controller

2018-08-07 Thread yixin zhu




On 8/6/2018 11:18 PM, Rob Herring wrote:

On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu  wrote:

From: Yixin Zhu 

This patch adds binding documentation for grx500 clock controller.

Signed-off-by: YiXin Zhu 
Signed-off-by: Songjun Wu 
---

Changes in v2:
- Rewrite clock driver's dt-binding document according to Rob Herring's
   comments.
- Simplify device tree docoment, remove some clock description.

  .../devicetree/bindings/clock/intel,grx500-clk.txt | 39 ++

Please match the compatible string: intel,grx500-cgu.txt

Will update to use same name.




  1 file changed, 39 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt

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 ..e54e1dad9196
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
@@ -0,0 +1,39 @@
+Device Tree Clock bindings for grx500 PLL controller.
+
+This binding uses the common clock binding:
+   Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+The grx500 clock controller supplies clock to various controllers within the
+SoC.
+
+Required properties for clock node
+- compatible: Should be "intel,grx500-cgu".
+- reg: physical base address of the controller and length of memory range.
+- #clock-cells: should be 1.
+
+Optional Propteries:
+- intel,osc-frequency: frequency of the osc clock.
+if missing, driver will use clock rate defined in the driver.

This should use a fixed-clock node instead.

Yes, This is a fixed clock node registered in driver code.
The frequency of the fixed clock is designed to be overwritten by device 
tree in case some one verify
clock driver in the emulation platform or in some cases frequency other 
than driver defined one is preferred.
These kinds of cases are very rare. But I feel it would be better to 
have a way to use customized frequency.
The frequency defined in device tree will overwritten driver defined 
frequency before registering fixed-clock node.



+
+Example: Clock controller node:
+
+   cgu: cgu@1620 {
+compatible = "intel,grx500-cgu", "syscon";
+   reg = <0x1620 0x200>;
+   #clock-cells = <1>;
+   };
+
+
+Example: UART controller node that consumes the clock generated by clock
+   controller.
+
+   asc0: serial@1660 {
+   compatible = "lantiq,asc";
+   reg = <0x1660 0x10>;
+   interrupt-parent = <>;
+   interrupts = ,
+   ,
+   ;
+   clocks = < CLK_SSX4>, < GCLK_UART>;
+   clock-names = "freq", "asc";
+   };
--
2.11.0





Re: [PATCH v2 03/18] dt-bindings: clk: Add documentation of grx500 clock controller

2018-08-07 Thread yixin zhu




On 8/6/2018 11:18 PM, Rob Herring wrote:

On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu  wrote:

From: Yixin Zhu 

This patch adds binding documentation for grx500 clock controller.

Signed-off-by: YiXin Zhu 
Signed-off-by: Songjun Wu 
---

Changes in v2:
- Rewrite clock driver's dt-binding document according to Rob Herring's
   comments.
- Simplify device tree docoment, remove some clock description.

  .../devicetree/bindings/clock/intel,grx500-clk.txt | 39 ++

Please match the compatible string: intel,grx500-cgu.txt

Will update to use same name.




  1 file changed, 39 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt

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 ..e54e1dad9196
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
@@ -0,0 +1,39 @@
+Device Tree Clock bindings for grx500 PLL controller.
+
+This binding uses the common clock binding:
+   Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+The grx500 clock controller supplies clock to various controllers within the
+SoC.
+
+Required properties for clock node
+- compatible: Should be "intel,grx500-cgu".
+- reg: physical base address of the controller and length of memory range.
+- #clock-cells: should be 1.
+
+Optional Propteries:
+- intel,osc-frequency: frequency of the osc clock.
+if missing, driver will use clock rate defined in the driver.

This should use a fixed-clock node instead.

Yes, This is a fixed clock node registered in driver code.
The frequency of the fixed clock is designed to be overwritten by device 
tree in case some one verify
clock driver in the emulation platform or in some cases frequency other 
than driver defined one is preferred.
These kinds of cases are very rare. But I feel it would be better to 
have a way to use customized frequency.
The frequency defined in device tree will overwritten driver defined 
frequency before registering fixed-clock node.



+
+Example: Clock controller node:
+
+   cgu: cgu@1620 {
+compatible = "intel,grx500-cgu", "syscon";
+   reg = <0x1620 0x200>;
+   #clock-cells = <1>;
+   };
+
+
+Example: UART controller node that consumes the clock generated by clock
+   controller.
+
+   asc0: serial@1660 {
+   compatible = "lantiq,asc";
+   reg = <0x1660 0x10>;
+   interrupt-parent = <>;
+   interrupts = ,
+   ,
+   ;
+   clocks = < CLK_SSX4>, < GCLK_UART>;
+   clock-names = "freq", "asc";
+   };
--
2.11.0





Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs

2018-08-07 Thread yixin zhu




On 8/6/2018 11:19 PM, Rob Herring wrote:

On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu  wrote:

From: Yixin Zhu 

This driver provides PLL clock registration as well as various clock
branches, e.g. MUX clock, gate clock, divider clock and so on.

PLLs that provide clock to DDR, CPU and peripherals are shown 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-->
      +-+

Signed-off-by: Yixin Zhu 
Signed-off-by: Songjun Wu 
---

Changes in v2:
- Rewrite clock driver, add platform clock description details in
   clock driver.

  drivers/clk/Kconfig  |   1 +
  drivers/clk/Makefile |   3 +
  drivers/clk/intel/Kconfig|  20 ++
  drivers/clk/intel/Makefile   |   7 +
  drivers/clk/intel/clk-cgu-pll.c  | 166 ++
  drivers/clk/intel/clk-cgu-pll.h  |  34 ++
  drivers/clk/intel/clk-cgu.c  | 470 +++
  drivers/clk/intel/clk-cgu.h  | 259 +++
  drivers/clk/intel/clk-grx500.c   | 168 ++
  include/dt-bindings/clock/intel,grx500-clk.h |  69 

This belongs with the clk binding patch.

Rob

Thanks for review.
Will move it to clock binding patch.







Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs

2018-08-07 Thread yixin zhu




On 8/6/2018 11:19 PM, Rob Herring wrote:

On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu  wrote:

From: Yixin Zhu 

This driver provides PLL clock registration as well as various clock
branches, e.g. MUX clock, gate clock, divider clock and so on.

PLLs that provide clock to DDR, CPU and peripherals are shown 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-->
      +-+

Signed-off-by: Yixin Zhu 
Signed-off-by: Songjun Wu 
---

Changes in v2:
- Rewrite clock driver, add platform clock description details in
   clock driver.

  drivers/clk/Kconfig  |   1 +
  drivers/clk/Makefile |   3 +
  drivers/clk/intel/Kconfig|  20 ++
  drivers/clk/intel/Makefile   |   7 +
  drivers/clk/intel/clk-cgu-pll.c  | 166 ++
  drivers/clk/intel/clk-cgu-pll.h  |  34 ++
  drivers/clk/intel/clk-cgu.c  | 470 +++
  drivers/clk/intel/clk-cgu.h  | 259 +++
  drivers/clk/intel/clk-grx500.c   | 168 ++
  include/dt-bindings/clock/intel,grx500-clk.h |  69 

This belongs with the clk binding patch.

Rob

Thanks for review.
Will move it to clock binding patch.







Re: [PATCH 2/7] clk: intel: Add clock driver for GRX500 SoC

2018-06-18 Thread yixin zhu




On 6/14/2018 10:09 PM, Rob Herring wrote:

On Thu, Jun 14, 2018 at 2:40 AM, yixin zhu  wrote:



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 

PLL of GRX500 provide clock to DDR, CPU, and peripherals as show below


[...]


+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?


No, I mean generally we have 1 DT node for the h/w block with all the
clock control registers rather than nodes with a single register and 1
or a couple of clocks. Sometimes the clock registers are mixed with
other functions which complicates things a bit. But I can't tell that
here because you haven't documented what's in the rest of the register
space.

Rob


Thanks
Will update to use one DT node for the whole clock controller.


Re: [PATCH 2/7] clk: intel: Add clock driver for GRX500 SoC

2018-06-18 Thread yixin zhu




On 6/14/2018 10:09 PM, Rob Herring wrote:

On Thu, Jun 14, 2018 at 2:40 AM, yixin zhu  wrote:



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 

PLL of GRX500 provide clock to DDR, CPU, and peripherals as show below


[...]


+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?


No, I mean generally we have 1 DT node for the h/w block with all the
clock control registers rather than nodes with a single register and 1
or a couple of clocks. Sometimes the clock registers are mixed with
other functions which complicates things a bit. But I can't tell that
here because you haven't documented what's in the rest of the register
space.

Rob


Thanks
Will update to use one DT node for the whole clock controller.


Re: [PATCH 3/7] MIPS: intel: Add initial support for Intel MIPS SoCs

2018-06-14 Thread yixin zhu




On 6/12/2018 7:23 PM, James Hogan wrote:

Hi,

Good to see this patch!

On Tue, Jun 12, 2018 at 01:40:30PM +0800, Songjun Wu wrote:

diff --git a/arch/mips/Kbuild.platforms b/arch/mips/Kbuild.platforms
index ac7ad54f984f..bcd647060f3e 100644
--- a/arch/mips/Kbuild.platforms
+++ b/arch/mips/Kbuild.platforms
@@ -12,6 +12,7 @@ platforms += cobalt
  platforms += dec
  platforms += emma
  platforms += generic
+platforms += intel-mips


What are the main things preventing this from moving to the generic
platform? Is it mainly the use of EVA (which generic doesn't yet
support)?


Yes. It's mainly because of EVA.


diff --git a/arch/mips/include/asm/mach-intel-mips/kernel-entry-init.h 
b/arch/mips/include/asm/mach-intel-mips/kernel-entry-init.h
new file mode 100644
index ..3893855b60c6
--- /dev/null
+++ b/arch/mips/include/asm/mach-intel-mips/kernel-entry-init.h

...

+   /*
+* Get Config.K0 value and use it to program
+* the segmentation registers


Please can you describe (maybe with a table) the segment layout in human
readable terms so the reader doesn't need to decode the SegCtl registers
to understand where everything is in the virtual address space?


Will add detailed EVA mapping description in code comments.


diff --git a/arch/mips/boot/dts/intel-mips/Makefile 
b/arch/mips/boot/dts/intel-mips/Makefile
new file mode 100644
index ..b16c0081639c
--- /dev/null
+++ b/arch/mips/boot/dts/intel-mips/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+dtb-$(CONFIG_DTB_INTEL_MIPS_GRX500)+= easy350_anywan.dtb
+obj-y  += $(patsubst %.dtb, %.dtb.o, $(dtb-y))


This needs updating to obj-$(CONFIG_BUILTIN_DTB) as per commit
fca3aa166422 ("MIPS: dts: Avoid unneeded built-in.a in DTS dirs") in
linux-next.


diff --git a/arch/mips/intel-mips/Makefile b/arch/mips/intel-mips/Makefile
new file mode 100644
index ..9f272d06eecd
--- /dev/null
+++ b/arch/mips/intel-mips/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_INTEL_MIPS)   += prom.o irq.o time.o


You can use obj-y, since this Makefile is only included if
CONFIG_INTEL_MIPS=y (i.e. via the platform-$(CONFIG_INTEL_MIPS) below).

Also please split each file onto separate "obj-y += whatever.o" lines.


Will use obj-y.
Will split it into per line per .o.


diff --git a/arch/mips/intel-mips/Platform b/arch/mips/intel-mips/Platform
new file mode 100644
index ..b34750eeaeb0
--- /dev/null
+++ b/arch/mips/intel-mips/Platform
@@ -0,0 +1,11 @@
+#
+# MIPs SoC platform
+#
+
+platform-$(CONFIG_INTEL_MIPS)  += intel-mips/


^^^ (this is what ensures the Makefile is only included for this
platform)


diff --git a/arch/mips/intel-mips/irq.c b/arch/mips/intel-mips/irq.c
new file mode 100644
index ..00637a5cdd20
--- /dev/null
+++ b/arch/mips/intel-mips/irq.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016 Intel Corporation.
+ */
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+void __init arch_init_irq(void)
+{
+   struct device_node *intc_node;
+
+   pr_info("EIC is %s\n", cpu_has_veic ? "on" : "off");
+   pr_info("VINT is %s\n", cpu_has_vint ? "on" : "off");
+
+   intc_node = of_find_compatible_node(NULL, NULL,
+   "mti,cpu-interrupt-controller");
+   if (!cpu_has_veic && !intc_node)
+   mips_cpu_irq_init();
+
+   irqchip_init();
+}
+
+int get_c0_perfcount_int(void)
+{
+   return gic_get_c0_perfcount_int();
+}
+EXPORT_SYMBOL_GPL(get_c0_perfcount_int);
+
+unsigned int get_c0_compare_int(void)
+{
+   return gic_get_c0_compare_int();
+}


Worth having get_c0_fdc_int() too for the "Fast Debug Channel"?


FDC not used in our product.


diff --git a/arch/mips/intel-mips/prom.c b/arch/mips/intel-mips/prom.c
new file mode 100644
index ..9407858ddc94
--- /dev/null
+++ b/arch/mips/intel-mips/prom.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2014 Lei Chuanhua 
+ * Copyright (C) 2016 Intel Corporation.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IOPORT_RESOURCE_START   0x1000
+#define IOPORT_RESOURCE_END 0x
+#define IOMEM_RESOURCE_START0x1000
+#define IOMEM_RESOURCE_END  0x


The _END ones seem to be unused?


Will remove unused _END macros.


+static void __init prom_init_cmdline(void)
+{
+   int i;
+   int argc;
+   char **argv;
+
+   /*
+* If u-boot pass parameters, it is ok, however, if without u-boot
+* JTAG or other tool has to reset all register value before it goes
+* emulation most likely belongs to this category
+*/
+   if (fw_arg0 == 0 || fw_arg1 == 0)
+   return;
+
+   argc = fw_arg0;
+   argv = (char **)KSEG1ADDR(fw_arg1);
+
+   arcs_cmdline[0] = '\0';
+
+   for (i = 0; i < 

Re: [PATCH 3/7] MIPS: intel: Add initial support for Intel MIPS SoCs

2018-06-14 Thread yixin zhu




On 6/12/2018 7:23 PM, James Hogan wrote:

Hi,

Good to see this patch!

On Tue, Jun 12, 2018 at 01:40:30PM +0800, Songjun Wu wrote:

diff --git a/arch/mips/Kbuild.platforms b/arch/mips/Kbuild.platforms
index ac7ad54f984f..bcd647060f3e 100644
--- a/arch/mips/Kbuild.platforms
+++ b/arch/mips/Kbuild.platforms
@@ -12,6 +12,7 @@ platforms += cobalt
  platforms += dec
  platforms += emma
  platforms += generic
+platforms += intel-mips


What are the main things preventing this from moving to the generic
platform? Is it mainly the use of EVA (which generic doesn't yet
support)?


Yes. It's mainly because of EVA.


diff --git a/arch/mips/include/asm/mach-intel-mips/kernel-entry-init.h 
b/arch/mips/include/asm/mach-intel-mips/kernel-entry-init.h
new file mode 100644
index ..3893855b60c6
--- /dev/null
+++ b/arch/mips/include/asm/mach-intel-mips/kernel-entry-init.h

...

+   /*
+* Get Config.K0 value and use it to program
+* the segmentation registers


Please can you describe (maybe with a table) the segment layout in human
readable terms so the reader doesn't need to decode the SegCtl registers
to understand where everything is in the virtual address space?


Will add detailed EVA mapping description in code comments.


diff --git a/arch/mips/boot/dts/intel-mips/Makefile 
b/arch/mips/boot/dts/intel-mips/Makefile
new file mode 100644
index ..b16c0081639c
--- /dev/null
+++ b/arch/mips/boot/dts/intel-mips/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+dtb-$(CONFIG_DTB_INTEL_MIPS_GRX500)+= easy350_anywan.dtb
+obj-y  += $(patsubst %.dtb, %.dtb.o, $(dtb-y))


This needs updating to obj-$(CONFIG_BUILTIN_DTB) as per commit
fca3aa166422 ("MIPS: dts: Avoid unneeded built-in.a in DTS dirs") in
linux-next.


diff --git a/arch/mips/intel-mips/Makefile b/arch/mips/intel-mips/Makefile
new file mode 100644
index ..9f272d06eecd
--- /dev/null
+++ b/arch/mips/intel-mips/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_INTEL_MIPS)   += prom.o irq.o time.o


You can use obj-y, since this Makefile is only included if
CONFIG_INTEL_MIPS=y (i.e. via the platform-$(CONFIG_INTEL_MIPS) below).

Also please split each file onto separate "obj-y += whatever.o" lines.


Will use obj-y.
Will split it into per line per .o.


diff --git a/arch/mips/intel-mips/Platform b/arch/mips/intel-mips/Platform
new file mode 100644
index ..b34750eeaeb0
--- /dev/null
+++ b/arch/mips/intel-mips/Platform
@@ -0,0 +1,11 @@
+#
+# MIPs SoC platform
+#
+
+platform-$(CONFIG_INTEL_MIPS)  += intel-mips/


^^^ (this is what ensures the Makefile is only included for this
platform)


diff --git a/arch/mips/intel-mips/irq.c b/arch/mips/intel-mips/irq.c
new file mode 100644
index ..00637a5cdd20
--- /dev/null
+++ b/arch/mips/intel-mips/irq.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016 Intel Corporation.
+ */
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+void __init arch_init_irq(void)
+{
+   struct device_node *intc_node;
+
+   pr_info("EIC is %s\n", cpu_has_veic ? "on" : "off");
+   pr_info("VINT is %s\n", cpu_has_vint ? "on" : "off");
+
+   intc_node = of_find_compatible_node(NULL, NULL,
+   "mti,cpu-interrupt-controller");
+   if (!cpu_has_veic && !intc_node)
+   mips_cpu_irq_init();
+
+   irqchip_init();
+}
+
+int get_c0_perfcount_int(void)
+{
+   return gic_get_c0_perfcount_int();
+}
+EXPORT_SYMBOL_GPL(get_c0_perfcount_int);
+
+unsigned int get_c0_compare_int(void)
+{
+   return gic_get_c0_compare_int();
+}


Worth having get_c0_fdc_int() too for the "Fast Debug Channel"?


FDC not used in our product.


diff --git a/arch/mips/intel-mips/prom.c b/arch/mips/intel-mips/prom.c
new file mode 100644
index ..9407858ddc94
--- /dev/null
+++ b/arch/mips/intel-mips/prom.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2014 Lei Chuanhua 
+ * Copyright (C) 2016 Intel Corporation.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IOPORT_RESOURCE_START   0x1000
+#define IOPORT_RESOURCE_END 0x
+#define IOMEM_RESOURCE_START0x1000
+#define IOMEM_RESOURCE_END  0x


The _END ones seem to be unused?


Will remove unused _END macros.


+static void __init prom_init_cmdline(void)
+{
+   int i;
+   int argc;
+   char **argv;
+
+   /*
+* If u-boot pass parameters, it is ok, however, if without u-boot
+* JTAG or other tool has to reset all register value before it goes
+* emulation most likely belongs to this category
+*/
+   if (fw_arg0 == 0 || fw_arg1 == 0)
+   return;
+
+   argc = fw_arg0;
+   argv = (char **)KSEG1ADDR(fw_arg1);
+
+   arcs_cmdline[0] = '\0';
+
+   for (i = 0; i < 

Re: [PATCH 2/7] clk: intel: Add clock driver for GRX500 SoC

2018-06-14 Thread yixin zhu




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 

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 

Signed-off-by: Songjun Wu 


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 ..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 = <>;
+   reg = <0x8>;
+   clock-output-names = "cbmclk", "ngiclk", "ssx4clk", "cpu0clk";
+   };
+
+   cpuclk: cpuclk {
+   #clock-cells = <0>;
+   compatible = "intel,grx500-cpu-clk";
+   clocks = < CPU0_CLK>, < 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?


Re: [PATCH 2/7] clk: intel: Add clock driver for GRX500 SoC

2018-06-14 Thread yixin zhu




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 

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 

Signed-off-by: Songjun Wu 


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 ..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 = <>;
+   reg = <0x8>;
+   clock-output-names = "cbmclk", "ngiclk", "ssx4clk", "cpu0clk";
+   };
+
+   cpuclk: cpuclk {
+   #clock-cells = <0>;
+   compatible = "intel,grx500-cpu-clk";
+   clocks = < CPU0_CLK>, < 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?