clk: intel: Add a new driver for a new clock controller IP

PATCH Superseded Rahul Tanwar 5 participants 8 messages Dec 20, 2019 – Dec 24, 2019

Subsystems (2)

Device tree bindings Common clk

lore

Share & tools

lore

Rahul Tanwar

author

2019-12-20 03:31 UTC #0 lore raw

v2 cover clk: intel: Add a new driver for a new clock controller IP

Hi,

This series adds clock driver for Clock Generation Unit(CGU) of Lightning Mountain(LGM) SoC.

Patch 1 adds common clock framework based clock driver for CGU. Patch 2 adds bindings document & include file for CGU.

These patches are baselined upon Linux 5.5-rc1 at below Git link:
git git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git

v2:
- Move the driver to x86 folder.
- Remove syscon usage.
- Remove regmap based access. Use direct readl()/write() instead. Add spinlocks.
- Change all enum values to capitals.
- Rename all data structures & functions from intel_* to lgm_*.
- Remove multiple header files. Keep only one header file.
- Make probe fail when any of the clk/pll registration fails.
- Fix few bugs with clk_init_data assignement.
- Address review concerns for code quality/style/convention.

v1:
- Initial version.

Rahul Tanwar (1):
dt-bindings: clk: intel: Add bindings document & header file for CGU

rtanwar (1):
clk: intel: Add CGU clock driver for a new SoC

 .../devicetree/bindings/clock/intel,cgu-lgm.yaml   |  43 ++
 drivers/clk/Kconfig                                |   8 +
 drivers/clk/x86/Makefile                           |   1 +
 drivers/clk/x86/clk-cgu-pll.c                      | 194 +++++++
 drivers/clk/x86/clk-cgu.c                          | 559 +++++++++++++++++++++
 drivers/clk/x86/clk-cgu.h                          | 296 +++++++++++
 drivers/clk/x86/clk-lgm.c                          | 351 +++++++++++++
 include/dt-bindings/clock/intel,lgm-clk.h          | 150 ++++++
 8 files changed, 1602 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml
 create mode 100644 drivers/clk/x86/clk-cgu-pll.c
 create mode 100644 drivers/clk/x86/clk-cgu.c
 create mode 100644 drivers/clk/x86/clk-cgu.h
 create mode 100644 drivers/clk/x86/clk-lgm.c
 create mode 100644 include/dt-bindings/clock/intel,lgm-clk.h

Signature · 1 line

2.11.0

Rahul Tanwar

author

2019-12-20 03:31 UTC #1 lore raw

PATCH v2 1/2 clk: intel: Add CGU clock driver for a new SoC

From: rtanwar <[email protected]>

Clock Generation Unit(CGU) is a new clock controller IP of a forthcoming Intel network processor SoC. It provides programming interfaces to control & configure all CPU & peripheral clocks. Add common clock framework based clock controller driver for CGU.

Signed-off-by Rahul Tanwar <[email protected]>

---
 drivers/clk/Kconfig           |   8 +
 drivers/clk/x86/Makefile      |   1 +
 drivers/clk/x86/clk-cgu-pll.c | 194 +++++++++++++++
 drivers/clk/x86/clk-cgu.c     | 559 ++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/x86/clk-cgu.h     | 296 ++++++++++++++++++++++
 drivers/clk/x86/clk-lgm.c     | 351 ++++++++++++++++++++++++++
 6 files changed, 1409 insertions(+)
 create mode 100644 drivers/clk/x86/clk-cgu-pll.c
 create mode 100644 drivers/clk/x86/clk-cgu.c
 create mode 100644 drivers/clk/x86/clk-cgu.h
 create mode 100644 drivers/clk/x86/clk-lgm.c

drivers/clk/Kconfig +8 -0

index 45653a0e6ecd..be5225b31de8 100644
@@ -39,6 +39,14 @@ config CLK_HSDK
 	  This driver supports the HSDK core, system, ddr, tunnel and hdmi PLLs
 	  control.
 
+config CLK_LGM_CGU
+	depends on OF && X86 && HAS_IOMEM
+	select OF_EARLY_FLATTREE
+	bool "Clock driver for Lightning Mountain(LGM) platform"
+	help
+	  Clock Generation Unit(CGU) driver for Intel Lightning Mountain(LGM)
+	  network processor SoC.
+
 config COMMON_CLK_MAX77686
 	tristate "Clock driver for Maxim 77620/77686/77802 MFD"
 	depends on MFD_MAX77686 || MFD_MAX77620 || COMPILE_TEST

drivers/clk/x86/Makefile +1 -0

index e3ec81e2a1c2..7c774ea7ddeb 100644
@@ -3,3 +3,4 @@ obj-$(CONFIG_PMC_ATOM)		+= clk-pmc-atom.o
 obj-$(CONFIG_X86_AMD_PLATFORM_DEVICE)	+= clk-st.o
 clk-x86-lpss-objs		:= clk-lpt.o
 obj-$(CONFIG_X86_INTEL_LPSS)	+= clk-x86-lpss.o
+obj-$(CONFIG_CLK_LGM_CGU)	+= clk-cgu.o clk-cgu-pll.o clk-lgm.o

drivers/clk/x86/clk-cgu-pll.c new file +194 -0

194 lines not on this page · raw message

drivers/clk/x86/clk-cgu.c new file +559 -0

559 lines not on this page · raw message

drivers/clk/x86/clk-cgu.h new file +296 -0

296 lines not on this page · raw message

drivers/clk/x86/clk-lgm.c new file +351 -0

351 lines not on this page · raw message

Signature · 1 line

2.11.0

Rahul Tanwar

author

2019-12-20 03:31 UTC #6 lore raw

PATCH v2 2/2 dt-bindings: clk: intel: Add bindings document & header file for CGU

In reply to Rahul Tanwar

Clock generation unit(CGU) is a clock controller IP of Intel's Lightning Mountain(LGM) SoC. Add DT bindings include file and document for CGU clock controller driver of LGM.

Signed-off-by Rahul Tanwar <[email protected]>

---
 .../devicetree/bindings/clock/intel,cgu-lgm.yaml   |  43 ++++++
 include/dt-bindings/clock/intel,lgm-clk.h          | 150 +++++++++++++++++++++
 2 files changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml
 create mode 100644 include/dt-bindings/clock/intel,lgm-clk.h

Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml new file +43 -0

new file mode 100644
index 000000000000..2c9edabe0490
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/clock/intel,cgu-lgm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel Lightning Mountain SoC's Clock Controller(CGU) Binding
+
+maintainers:
+  - Rahul Tanwar <[email protected]>
+
+description: |
+  Lightning Mountain(LGM) SoC's Clock Generation Unit(CGU) driver provides
+  all means to access the CGU hardware module in order to generate a series
+  of clocks for the whole system and individual peripherals.
+
+  This binding uses the common clock bindings
+  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+properties:
+  compatible:
+    const: intel,cgu-lgm
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+
+examples:
+  - |
+    cgu: cgu@e0200000 {
+        compatible = "intel,cgu-lgm";
+        reg = <0xe0200000 0x33c>;
+        #clock-cells = <1>;
+    };
+
+...

include/dt-bindings/clock/intel,lgm-clk.h new file +150 -0

150 lines not on this page · raw message

Signature · 1 line

2.11.0

Andy Shevchenko

2019-12-20 10:13 UTC #2 lore raw

Re: [PATCH v2 1/2] clk: intel: Add CGU clock driver for a new SoC

In reply to Rahul Tanwar

Quoted 6 lines · Rahul Tanwar

From: rtanwar <[email protected]>
Clock Generation Unit(CGU) is a new clock controller IP of a forthcoming Intel network processor SoC. It provides programming interfaces to control & configure all CPU & peripheral clocks. Add common clock framework based clock controller driver for CGU.

...

Quoted 1 line

 drivers/clk/Kconfig           |   8 +

This should be in x86 folder and you perhaps need to add source "drivers/clk/x86/Kconfig" here.

Also drivers/clk/Makefile should be updated accordingly.

...

Quoted 7 lines

drivers/clk/x86/clk-cgu-pll.c new file +7 -0

+static int lgm_pll_wait_for_lock(struct lgm_clk_pll *pll)
+{
+	int max_loop_cnt = 100;
+	unsigned long flags;
+	unsigned int val;
+
+	while (max_loop_cnt > 0) {

drivers/clk/x86/clk-cgu-pll.c line 57

Quoted 10 lines

drivers/clk/x86/clk-cgu-pll.c new file +10 -0

+		raw_spin_lock_irqsave(&pll->lock, flags);
+		val = lgm_get_clk_val(pll->membase, pll->reg, 0, 1);
+		raw_spin_unlock_irqrestore(&pll->lock, flags);
+
+		if (val)
+			return 0;
+
+		udelay(1);
+		max_loop_cnt--;
+	}

Looks like a repetition of iopoll.h.
Even without that the waiting loops like this more natural in a form of

	unsigned int count = ...;
	...
	do {
		...
	} while (--count);

drivers/clk/x86/clk-cgu-pll.c line 60

Quoted 3 lines

drivers/clk/x86/clk-cgu-pll.c new file +3 -0
+
+	return -EIO;
+}

I think I told you that during internal review.

...

drivers/clk/x86/clk-cgu.c line 27

Quoted 2 lines

drivers/clk/x86/clk-cgu.c new file +2 -0

+void lgm_set_clk_val(void *membase, u32 reg,
+		     u8 shift, u8 width, u32 set_val)

There is plenty of space, and to be precise it would take exactly 80, on the previous line.

Quoted 8 lines

drivers/clk/x86/clk-cgu.c new file +8 -0

+{
+	u32 mask = (GENMASK(width - 1, 0) << shift);
+	u32 regval;
+
+	regval = readl(membase + reg);
+	regval = (regval & ~mask) | ((set_val << shift) & mask);
+	writel(regval, membase + reg);
+}

...

drivers/clk/x86/clk-cgu.c line 48

Quoted 2 lines

drivers/clk/x86/clk-cgu.c new file +2 -0

+void lgm_clk_add_lookup(struct lgm_clk_provider *ctx,
+			struct clk_hw *hw, unsigned int id)

Ditto.

...

drivers/clk/x86/clk-cgu.c line 304

Quoted 9 lines

drivers/clk/x86/clk-cgu.c new file +9 -0

+	if (gate->flags & GATE_CLK_HW)
+		reg = GATE_HW_REG_EN(gate->reg);
+	else if (gate->flags & GATE_CLK_SW)
+		reg = gate->reg;
+	else {
+		dev_err(gate->dev, "%s has unsupported flags 0x%lx\n",
+			clk_hw_get_name(hw), gate->flags);
+		return 0;
+	}

Missed curly braces in first two conditionals.

...

drivers/clk/x86/clk-cgu.c line 351

Quoted 9 lines

drivers/clk/x86/clk-cgu.c new file +9 -0

+	if (gate->flags & GATE_CLK_HW)
+		reg = GATE_HW_REG_STAT(gate->reg);
+	else if (gate->flags & GATE_CLK_SW)
+		reg = gate->reg;
+	else {
+		dev_err(gate->dev, "%s has unsupported flags 0x%lx\n",
+			clk_hw_get_name(hw), gate->flags);
+		return 0;
+	}

Ditto.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

kbuild test robot

build bot

2019-12-23 18:42 UTC #3 lore raw

Re: [PATCH v2 1/2] clk: intel: Add CGU clock driver for a new SoC

In reply to Rahul Tanwar

Thank you for the patch! Perhaps something to improve:

Hi Rahul,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on clk/clk-next]
[also build test WARNING on robh/for-next v5.5-rc3 next-20191220] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Rahul-Tanwar/clk-intel-Add-a-new-driver-for-a-new-clock-controller-IP/20191223-110300
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-129-g341daf20-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag

Reported-by kbuild test robot <[email protected]>

sparse warnings: (new ones prefixed by >>)

>> drivers/clk/x86/clk-cgu.c:32:32: sparse: sparse: incorrect type in argument 1 (different address spaces)
>> drivers/clk/x86/clk-cgu.c:32:32: sparse:    expected void const volatile [noderef] <asn:2> *addr
>> drivers/clk/x86/clk-cgu.c:32:32: sparse:    got void *
   drivers/clk/x86/clk-cgu.c:34:32: sparse: sparse: incorrect type in argument 2 (different address spaces)
>> drivers/clk/x86/clk-cgu.c:34:32: sparse:    expected void volatile [noderef] <asn:2> *addr
   drivers/clk/x86/clk-cgu.c:34:32: sparse:    got void *
   drivers/clk/x86/clk-cgu.c:41:29: sparse: sparse: incorrect type in argument 1 (different address spaces)
   drivers/clk/x86/clk-cgu.c:41:29: sparse:    expected void const volatile [noderef] <asn:2> *addr
   drivers/clk/x86/clk-cgu.c:41:29: sparse:    got void *
   drivers/clk/x86/clk-cgu.c:61:36: sparse: sparse: incorrect type in argument 1 (different address spaces)
>> drivers/clk/x86/clk-cgu.c:61:36: sparse:    expected void *membase
>> drivers/clk/x86/clk-cgu.c:61:36: sparse:    got void [noderef] <asn:2> *membase

drivers/clk/x86/clk-cgu.c:78:34: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:78:34: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:78:34: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:91:28: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:91:28: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:91:28: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:155:36: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:155:36: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:155:36: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:170:38: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:170:38: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:170:38: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:202:32: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:202:32: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:202:32: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:260:36: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:260:36: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:260:36: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:282:36: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:282:36: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:282:36: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:307:29: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:307:29: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:307:29: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:333:29: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:333:29: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:333:29: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:354:35: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:354:35: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:354:35: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:409:37: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:409:37: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:409:37: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:466:36: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:466:36: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:466:36: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:468:36: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:468:36: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:468:36: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu.c:470:37: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu.c:470:37: sparse: expected void *membase
drivers/clk/x86/clk-cgu.c:470:37: sparse: got void [noderef] <asn:2> *membase
--

>> drivers/clk/x86/clk-cgu-pll.c:49:42: sparse: sparse: incorrect type in argument 1 (different address spaces)
>> drivers/clk/x86/clk-cgu-pll.c:49:42: sparse:    expected void *membase
>> drivers/clk/x86/clk-cgu-pll.c:49:42: sparse:    got void [noderef] <asn:2> *membase

drivers/clk/x86/clk-cgu-pll.c:69:36: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu-pll.c:69:36: sparse: expected void *membase
drivers/clk/x86/clk-cgu-pll.c:69:36: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu-pll.c:70:35: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu-pll.c:70:35: sparse: expected void *membase
drivers/clk/x86/clk-cgu-pll.c:70:35: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu-pll.c:71:36: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu-pll.c:71:36: sparse: expected void *membase
drivers/clk/x86/clk-cgu-pll.c:71:36: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu-pll.c:94:34: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu-pll.c:94:34: sparse: expected void *membase
drivers/clk/x86/clk-cgu-pll.c:94:34: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu-pll.c:106:28: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu-pll.c:106:28: sparse: expected void *membase
drivers/clk/x86/clk-cgu-pll.c:106:28: sparse: got void [noderef] <asn:2> *membase
drivers/clk/x86/clk-cgu-pll.c:118:28: sparse: sparse: incorrect type in argument 1 (different address spaces)
drivers/clk/x86/clk-cgu-pll.c:118:28: sparse: expected void *membase
drivers/clk/x86/clk-cgu-pll.c:118:28: sparse: got void [noderef] <asn:2> *membase

vim +32 drivers/clk/x86/clk-cgu.c

    25	
    26	void lgm_set_clk_val(void *membase, u32 reg,
    27			     u8 shift, u8 width, u32 set_val)
    28	{
    29		u32 mask = (GENMASK(width - 1, 0) << shift);
    30		u32 regval;
    31	
  > 32		regval = readl(membase + reg);
    33		regval = (regval & ~mask) | ((set_val << shift) & mask);
  > 34		writel(regval, membase + reg);
    35	}
    36	
    37	u32 lgm_get_clk_val(void *membase, u32 reg, u8 shift, u8 width)
    38	{
    39		u32 val;
    40	
    41		val = readl(membase + reg);
    42		val = (val >> shift) & (BIT(width) - 1);
    43	
    44		return val;
    45	}
    46	
    47	void lgm_clk_add_lookup(struct lgm_clk_provider *ctx,
    48				struct clk_hw *hw, unsigned int id)
    49	{
    50		if (ctx->clk_data.hws)
    51			ctx->clk_data.hws[id] = hw;
    52	}
    53	
    54	static struct clk_hw *lgm_clk_register_fixed(struct lgm_clk_provider *ctx,
    55						     const struct lgm_clk_branch *list)
    56	{
    57		unsigned long flags;
    58	
    59		if (list->div_flags & CLOCK_FLAG_VAL_INIT) {
    60			raw_spin_lock_irqsave(&ctx->lock, flags);
  > 61			lgm_set_clk_val(ctx->membase, list->div_off, list->div_shift,
    62					list->div_width, list->div_val);
    63			raw_spin_unlock_irqrestore(&ctx->lock, flags);
    64		}
    65	
    66		return clk_hw_register_fixed_rate(NULL, list->name,
    67						  list->parent_names[0],
    68						  list->flags, list->mux_flags);
    69	}
    70

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation

Stephen Boyd

maintainer

2019-12-23 19:38 UTC #7 lore raw

Re: [PATCH v2 2/2] dt-bindings: clk: intel: Add bindings document & header file for CGU

In reply to Rahul Tanwar

Quoting Rahul Tanwar (2019-12-19 19:31:08)

Quoted 14 lines

Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml new file +8 -0

new file mode 100644
index 000000000000..2c9edabe0490
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/clock/intel,cgu-lgm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel Lightning Mountain SoC's Clock Controller(CGU) Binding
+

Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml line 18

Quoted 10 lines

Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml new file +10 -0

+maintainers:
+  - Rahul Tanwar <[email protected]>
+
+description: |
+  Lightning Mountain(LGM) SoC's Clock Generation Unit(CGU) driver provides
+  all means to access the CGU hardware module in order to generate a series
+  of clocks for the whole system and individual peripherals.
+
+  This binding uses the common clock bindings
+  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt

I don't know if this second paragraph is very useful.

Quoted 9 lines

Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml new file +9 -0
+
+properties:
+  compatible:
+    const: intel,cgu-lgm
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':

Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml line 37

Quoted 10 lines

Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml new file +10 -0
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+
+examples:
+  - |
+    cgu: cgu@e0200000 {

Node name should be 'clock-controller'

include/dt-bindings/clock/intel,lgm-clk.h line 1

Quoted 13 lines

Documentation/devicetree/bindings/clock/intel,cgu-lgm.yaml new file +6 -0
+        compatible = "intel,cgu-lgm";
+        reg = <0xe0200000 0x33c>;
+        #clock-cells = <1>;
+    };
+
+...
include/dt-bindings/clock/intel,lgm-clk.h new file +1 -0
new file mode 100644
index 000000000000..09e5dc59ff5b
@@ -0,0 +1,150 @@
+/* SPDX-License-Identifier: GPL-2.0 */

Can this be more than just GPL? Maybe GPL or BSD?

[...]

include/dt-bindings/clock/intel,lgm-clk.h line 3

Quoted 2 lines

include/dt-bindings/clock/intel,lgm-clk.h new file +2 -0
+
+#define CLK_NR_CLKS            180

Can this be removed from the binding file? It's too generic of a name regardless and it doesn't describe something that is actually part of the binding.

Nathan Chancellor

2019-12-24 05:29 UTC #4 lore raw

Re: [PATCH v2 1/2] clk: intel: Add CGU clock driver for a new SoC

In reply to Rahul Tanwar

Quoted 8 lines · Rahul Tanwar

From: rtanwar <[email protected]>
Clock Generation Unit(CGU) is a new clock controller IP of a forthcoming Intel network processor SoC. It provides programming interfaces to control & configure all CPU & peripheral clocks. Add common clock framework based clock controller driver for CGU.
Signed-off-by Rahul Tanwar <[email protected]>

Hi Rahul,

The 0day bot reported this warning with clang with your patch, mind taking a look at it since it seems like you will need to do a v2 based on other comments?

It seems like the check either needs to be something different or the check should just be removed.

Cheers,
Nathan

Quoted 49 lines · kbuild test robot

CC: [email protected]
In-Reply-To: <ee8a8a0f0c882e22361895b2663870c8037c422f.1576811332.git.rahul.tanwar@linux.intel.com>
References: <ee8a8a0f0c882e22361895b2663870c8037c422f.1576811332.git.rahul.tanwar@linux.intel.com>
TO: Rahul Tanwar <[email protected]>
CC: [email protected], [email protected], [email protected], [email protected]
CC: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], rtanwar <[email protected]>, Rahul Tanwar <[email protected]>
Hi Rahul,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on clk/clk-next]
[also build test WARNING on robh/for-next v5.5-rc3 next-20191220] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:    https://github.com/0day-ci/linux/commits/Rahul-Tanwar/clk-intel-Add-a-new-driver-for-a-new-clock-controller-IP/20191223-110300
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 10.0.0 (git://gitmirror/llvm_project 891e25b02d760d0de18c7d46947913b3166047e7)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 
If you fix the issue, kindly add following tag
Reported-by kbuild test robot <[email protected]>
All warnings (new ones prefixed by >>):
Quoted 1 line
Quoted 1 line
drivers/clk/x86/clk-cgu.c:50:20: warning: address of array 'ctx->clk_data.hws' will always evaluate to 'true' [-Wpointer-bool-conversion]
           if (ctx->clk_data.hws)
           ~~  ~~~~~~~~~~~~~~^~~
1 warning generated.
vim +50 drivers/clk/x86/clk-cgu.c
46
47 void lgm_clk_add_lookup(struct lgm_clk_provider *ctx,
48 struct clk_hw *hw, unsigned int id)
49 {
Quoted 1 line
50 if (ctx->clk_data.hws)
    51			ctx->clk_data.hws[id] = hw;
    52	}
    53	
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

Tanwar, Rahul

author

2019-12-24 05:49 UTC #5 lore raw

Re: [PATCH v2 1/2] clk: intel: Add CGU clock driver for a new SoC

Quoted 15 lines · Nathan Chancellor

Quoted 8 lines · Rahul Tanwar
From: rtanwar <[email protected]>
Clock Generation Unit(CGU) is a new clock controller IP of a forthcoming Intel network processor SoC. It provides programming interfaces to control & configure all CPU & peripheral clocks. Add common clock framework based clock controller driver for CGU.
Signed-off-by Rahul Tanwar <[email protected]>
Hi Rahul,
The 0day bot reported this warning with clang with your patch, mind taking a look at it since it seems like you will need to do a v2 based on other comments?

Quoted 5 lines

It seems like the check either needs to be something different or the check should just be removed.
Cheers,
Nathan

Hi Nathan,

Yes sure, i will fix it in v3. I anyways need to post v3 to address review comments received from few reviewers. Thanks.

Regards,
Rahul