[PATCH 00/26] MIPS generic kernels, SEAD-3 & Boston support

[PATCH 00/26] MIPS generic kernels, SEAD-3 & Boston support

[Paul Burton]

This series introduces some infrastructure for building generic kernels
which will run on multiple boards depending upon the device tree
provided to them by the bootloader. It converts SEAD-3 to make use of
this, and adds support for the MIPS Boston development platform.

The Boston support can be tested in QEMU with this patchset applied:

  https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg03419.html

To do so, configure the kernel for the generic 64r6el_defconfig & run
QEMU like so:

  $ make ARCH=mips 64r6el_defconfig
  $ make ARCH=mips CROSS_COMPILE=my-toolchain-
  $ qemu-system-mips64el -M boston \
      -kernel arch/mips/boot/vmlinux.gz.itb \
      serial stdio

The same kernel binary will also boot on a SEAD-3 if using a bootloader
capable of loading the FIT image format (ie. U-Boot). These 2 boards
form the starting point for the generic kernels, with Ci20 & Ci40 able
to be added easily. Malta will require further work, but I've got most
peripherals converted to probe using device tree as a starting point &
will submit that separately.

This series applies atop v4.8-rc3 with my "MIPS: SEAD3 device tree
conversion" series applied first.

Paul Burton (26):
  MIPS: PCI: Support for CONFIG_PCI_DOMAINS_GENERIC
  MIPS: PCI: Make pcibios_set_cache_line_size an initcall
  MIPS: PCI: Inline pcibios_assign_all_busses
  MIPS: PCI: Split pci.c into pci.c & pci-legacy.c
  MIPS: PCI: Introduce CONFIG_PCI_DRIVERS_LEGACY
  MIPS: PCI: Support generic drivers
  MIPS: Sanitise coherentio semantics
  MIPS: dma-default: Don't check hw_coherentio if device is non-coherent
  MIPS: Support per-device DMA coherence
  MIPS: Print CM error reports upon bus errors
  dt-bindings: Document mti,mips-cpc binding
  MIPS: CPC: Provide a default mips_cpc_default_phys_base
  dt-bindings: Document mti,mips-cdmm binding
  MIPS: CDMM: Allow CDMM base address to be specified via DT
  irqchip: mips-cpu: Replace magic 0x100 with IE_SW0
  irqchip: mips-cpu: Prepare for non-legacy IRQ domains
  irqchip: mips-cpu: Introduce IPI IRQ domain support
  MIPS: smp-mt: Use CPU interrupt controller IPI IRQ domain support
  MIPS: Stengthen IPI IRQ domain sanity check
  MIPS: Adjust MIPS64 CAC_BASE to reflect Config.K0
  MIPS: Support generating Flattened Image Trees (.itb)
  MIPS: generic: Introduce generic DT-based board support
  MIPS: generic: Convert SEAD-3 to a generic board
  dt-bindings: Document img,boston-clock binding
  clk: boston: Add a driver for MIPS Boston board clocks
  MIPS: generic: Support MIPS Boston development boards

 .../devicetree/bindings/clock/img,boston-clock.txt |  27 ++
 .../devicetree/bindings/misc/mti,mips-cdmm.txt     |   8 +
 .../devicetree/bindings/misc/mti,mips-cpc.txt      |   8 +
 arch/mips/Kbuild.platforms                         |   2 +-
 arch/mips/Kconfig                                  | 104 ++++---
 arch/mips/Makefile                                 |  72 ++++-
 arch/mips/alchemy/common/setup.c                   |   6 +-
 arch/mips/boot/Makefile                            |  66 +++++
 arch/mips/boot/dts/Makefile                        |   1 +
 arch/mips/boot/dts/img/Makefile                    |   7 +
 arch/mips/boot/dts/img/boston.dts                  | 230 ++++++++++++++++
 arch/mips/boot/dts/mti/Makefile                    |   2 +-
 arch/mips/boot/dts/mti/sead3.dts                   |   1 +
 arch/mips/configs/generic/32r1.config              |   2 +
 arch/mips/configs/generic/32r2.config              |   3 +
 arch/mips/configs/generic/32r6.config              |   2 +
 arch/mips/configs/generic/64r1.config              |   1 +
 arch/mips/configs/generic/64r2.config              |   2 +
 arch/mips/configs/generic/64r6.config              |   1 +
 arch/mips/configs/generic/board-boston.config      |  46 ++++
 arch/mips/configs/generic/board-sead-3.config      |  32 +++
 arch/mips/configs/generic/eb.config                |   1 +
 arch/mips/configs/generic/el.config                |   1 +
 arch/mips/configs/generic/micro32r2.config         |   4 +
 arch/mips/configs/generic_defconfig                |   2 +
 arch/mips/configs/sead3_defconfig                  | 129 ---------
 arch/mips/configs/sead3micro_defconfig             | 122 ---------
 arch/mips/generic/Kconfig                          |  27 ++
 arch/mips/generic/Makefile                         |  15 +
 arch/mips/generic/Platform                         |  14 +
 .../sead3-dtshim.c => generic/board-sead3.c}       | 106 +++++++-
 arch/mips/generic/init.c                           | 174 ++++++++++++
 arch/mips/generic/irq.c                            |  64 +++++
 arch/mips/generic/proc.c                           |  29 ++
 arch/mips/generic/vmlinux.its.S                    |  56 ++++
 arch/mips/include/asm/addrspace.h                  |   3 +-
 arch/mips/include/asm/device.h                     |   5 +
 arch/mips/include/asm/dma-coherence.h              |  16 +-
 arch/mips/include/asm/dma-mapping.h                |  10 +
 arch/mips/include/asm/mach-generic/dma-coherence.h |  14 +-
 arch/mips/include/asm/mach-generic/spaces.h        |   8 +-
 arch/mips/include/asm/mach-ip27/spaces.h           |   1 +
 .../include/asm/mach-sead3/cpu-feature-overrides.h |  72 -----
 arch/mips/include/asm/mach-sead3/irq.h             |   9 -
 .../include/asm/mach-sead3/kernel-entry-init.h     |  21 --
 arch/mips/include/asm/mach-sead3/sead3-dtshim.h    |  29 --
 arch/mips/include/asm/mach-sead3/war.h             |  24 --
 arch/mips/include/asm/machine.h                    |  63 +++++
 arch/mips/include/asm/pci.h                        |  57 +++-
 arch/mips/kernel/mips-cpc.c                        |  18 ++
 arch/mips/kernel/smp-mt.c                          |  49 +---
 arch/mips/kernel/smp.c                             |  20 +-
 arch/mips/kernel/traps.c                           |   3 +
 arch/mips/lantiq/irq.c                             |  52 ----
 arch/mips/lib/iomap-pci.c                          |   4 +
 arch/mips/mm/c-r4k.c                               |   7 +-
 arch/mips/mm/dma-default.c                         |  16 +-
 arch/mips/mti-malta/malta-int.c                    |  90 +-----
 arch/mips/mti-malta/malta-setup.c                  |  10 +-
 arch/mips/mti-sead3/Makefile                       |  15 -
 arch/mips/mti-sead3/Platform                       |   7 -
 arch/mips/mti-sead3/sead3-init.c                   | 100 -------
 arch/mips/mti-sead3/sead3-int.c                    |  23 --
 arch/mips/mti-sead3/sead3-setup.c                  |  39 ---
 arch/mips/mti-sead3/sead3-time.c                   |  91 -------
 arch/mips/pci/Makefile                             |   2 +
 arch/mips/pci/pci-alchemy.c                        |   3 +-
 arch/mips/pci/pci-generic.c                        |  52 ++++
 arch/mips/pci/pci-legacy.c                         | 302 +++++++++++++++++++++
 arch/mips/pci/pci.c                                | 296 +-------------------
 drivers/bus/mips_cdmm.c                            |  13 +
 drivers/clk/Kconfig                                |   9 +
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-boston.c                           | 131 +++++++++
 drivers/irqchip/Kconfig                            |   2 +
 drivers/irqchip/irq-mips-cpu.c                     | 149 ++++++++--
 include/dt-bindings/clock/boston-clock.h           |  13 +
 77 files changed, 1937 insertions(+), 1279 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/img,boston-clock.txt
 create mode 100644 Documentation/devicetree/bindings/misc/mti,mips-cdmm.txt
 create mode 100644 Documentation/devicetree/bindings/misc/mti,mips-cpc.txt
 create mode 100644 arch/mips/boot/dts/img/Makefile
 create mode 100644 arch/mips/boot/dts/img/boston.dts
 create mode 100644 arch/mips/configs/generic/32r1.config
 create mode 100644 arch/mips/configs/generic/32r2.config
 create mode 100644 arch/mips/configs/generic/32r6.config
 create mode 100644 arch/mips/configs/generic/64r1.config
 create mode 100644 arch/mips/configs/generic/64r2.config
 create mode 100644 arch/mips/configs/generic/64r6.config
 create mode 100644 arch/mips/configs/generic/board-boston.config
 create mode 100644 arch/mips/configs/generic/board-sead-3.config
 create mode 100644 arch/mips/configs/generic/eb.config
 create mode 100644 arch/mips/configs/generic/el.config
 create mode 100644 arch/mips/configs/generic/micro32r2.config
 create mode 100644 arch/mips/configs/generic_defconfig
 delete mode 100644 arch/mips/configs/sead3_defconfig
 delete mode 100644 arch/mips/configs/sead3micro_defconfig
 create mode 100644 arch/mips/generic/Kconfig
 create mode 100644 arch/mips/generic/Makefile
 create mode 100644 arch/mips/generic/Platform
 rename arch/mips/{mti-sead3/sead3-dtshim.c => generic/board-sead3.c} (72%)
 create mode 100644 arch/mips/generic/init.c
 create mode 100644 arch/mips/generic/irq.c
 create mode 100644 arch/mips/generic/proc.c
 create mode 100644 arch/mips/generic/vmlinux.its.S
 delete mode 100644 arch/mips/include/asm/mach-sead3/cpu-feature-overrides.h
 delete mode 100644 arch/mips/include/asm/mach-sead3/irq.h
 delete mode 100644 arch/mips/include/asm/mach-sead3/kernel-entry-init.h
 delete mode 100644 arch/mips/include/asm/mach-sead3/sead3-dtshim.h
 delete mode 100644 arch/mips/include/asm/mach-sead3/war.h
 create mode 100644 arch/mips/include/asm/machine.h
 delete mode 100644 arch/mips/mti-sead3/Makefile
 delete mode 100644 arch/mips/mti-sead3/Platform
 delete mode 100644 arch/mips/mti-sead3/sead3-init.c
 delete mode 100644 arch/mips/mti-sead3/sead3-int.c
 delete mode 100644 arch/mips/mti-sead3/sead3-setup.c
 delete mode 100644 arch/mips/mti-sead3/sead3-time.c
 create mode 100644 arch/mips/pci/pci-generic.c
 create mode 100644 arch/mips/pci/pci-legacy.c
 create mode 100644 drivers/clk/clk-boston.c
 create mode 100644 include/dt-bindings/clock/boston-clock.h

-- 
2.9.3

[PATCH 24/26] dt-bindings: Document img,boston-clock binding

[Paul Burton]

Add device tree binding documentation for the clocks provided by the
MIPS Boston development board from Imagination Technologies, and a
header file describing the available clocks for use by device trees &
driver.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 .../devicetree/bindings/clock/img,boston-clock.txt | 27 ++++++++++++++++++++++
 include/dt-bindings/clock/boston-clock.h           | 13 +++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/img,boston-clock.txt
 create mode 100644 include/dt-bindings/clock/boston-clock.h

diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
new file mode 100644
index 0000000..c01ea60
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
@@ -0,0 +1,27 @@
+Binding for Imagination Technologies MIPS Boston clock sources.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : Should be "img,boston-clock".
+- #clock-cells : Should be set to 1.
+  Values available for clock consumers can be found in the header file:
+    <dt-bindings/clock/boston-clock.h>
+- regmap : Phandle to the Boston platform register system controller.
+  This should contain a phandle to the system controller node covering the
+  platform registers provided by the Boston board.
+
+Example:
+
+	clk_boston: clock {
+		compatible = "img,boston-clock";
+		#clock-cells = <1>;
+		regmap = <&plat_regs>;
+	};
+
+	uart0: uart@17ffe000 {
+		/* ... */
+		clocks = <&clk_boston BOSTON_CLK_SYS>;
+	};
diff --git a/include/dt-bindings/clock/boston-clock.h b/include/dt-bindings/clock/boston-clock.h
new file mode 100644
index 0000000..25f9cd2
--- /dev/null
+++ b/include/dt-bindings/clock/boston-clock.h
@@ -0,0 +1,13 @@
+/*
+ * Copyright (C) 2016 Imagination Technologies
+ *
+ * SPDX-License-Identifier:	GPL-2.0
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_BOSTON_CLOCK_H__
+#define __DT_BINDINGS_CLOCK_BOSTON_CLOCK_H__
+
+#define BOSTON_CLK_SYS 0
+#define BOSTON_CLK_CPU 1
+
+#endif /* __DT_BINDINGS_CLOCK_BOSTON_CLOCK_H__ */
-- 
2.9.3

[PATCH 25/26] clk: boston: Add a driver for MIPS Boston board clocks

[Paul Burton]

Add a driver for the clocks provided by the MIPS Boston board from
Imagination Technologies. 2 clocks are provided - the system clock & the
CPU clock - and each is a simple fixed rate clock whose frequency can be
determined by reading a register provided by the board.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 drivers/clk/Kconfig      |   9 ++++
 drivers/clk/Makefile     |   1 +
 drivers/clk/clk-boston.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+)
 create mode 100644 drivers/clk/clk-boston.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index e2d9bd7..2680343 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -208,6 +208,15 @@ config COMMON_CLK_OXNAS
 	---help---
 	  Support for the OXNAS SoC Family clocks.
 
+config COMMON_CLK_BOSTON
+	bool "Clock driver for MIPS Boston boards"
+	select MFD_SYSCON
+	---help---
+	  Enable this to support the system & CPU clocks on the MIPS Boston
+	  development board from Imagination Technologies. These are simple
+	  fixed rate clocks whose rate is determined by reading a platform
+	  provided register.
+
 source "drivers/clk/bcm/Kconfig"
 source "drivers/clk/hisilicon/Kconfig"
 source "drivers/clk/meson/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 3b6f9cf..3b78e515 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -20,6 +20,7 @@ endif
 obj-$(CONFIG_MACH_ASM9260)		+= clk-asm9260.o
 obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)	+= clk-axi-clkgen.o
 obj-$(CONFIG_ARCH_AXXIA)		+= clk-axm5516.o
+obj-$(CONFIG_COMMON_CLK_BOSTON)		+= clk-boston.o
 obj-$(CONFIG_COMMON_CLK_CDCE706)	+= clk-cdce706.o
 obj-$(CONFIG_COMMON_CLK_CDCE925)	+= clk-cdce925.o
 obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
diff --git a/drivers/clk/clk-boston.c b/drivers/clk/clk-boston.c
new file mode 100644
index 0000000..4fa3fad
--- /dev/null
+++ b/drivers/clk/clk-boston.c
@@ -0,0 +1,131 @@
+/*
+ * Copyright (C) 2016 Imagination Technologies
+ * Author: Paul Burton <paul.burton@imgtec.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/mfd/syscon.h>
+
+#include <dt-bindings/clock/boston-clock.h>
+
+#define BOSTON_CLK_COUNT 2
+
+struct clk_boston {
+	struct clk_hw hw;
+	struct regmap *regmap;
+	unsigned int id;
+};
+
+struct clk_boston_state {
+	struct clk *clk[BOSTON_CLK_COUNT];
+	struct clk_boston clk_boston[BOSTON_CLK_COUNT];
+	struct clk_onecell_data onecell_data[BOSTON_CLK_COUNT];
+};
+
+static const char *clk_names[BOSTON_CLK_COUNT] = {
+	[BOSTON_CLK_SYS] = "sys",
+	[BOSTON_CLK_CPU] = "cpu",
+};
+
+#define BOSTON_PLAT_MMCMDIV		0x30
+# define BOSTON_PLAT_MMCMDIV_CLK0DIV	(0xff << 0)
+# define BOSTON_PLAT_MMCMDIV_INPUT	(0xff << 8)
+# define BOSTON_PLAT_MMCMDIV_MUL	(0xff << 16)
+# define BOSTON_PLAT_MMCMDIV_CLK1DIV	(0xff << 24)
+
+static struct clk_boston *to_clk_boston(struct clk_hw *hw)
+{
+	return container_of(hw, struct clk_boston, hw);
+}
+
+static uint32_t ext_field(uint32_t val, uint32_t mask)
+{
+	return (val & mask) >> (ffs(mask) - 1);
+}
+
+static unsigned long clk_boston_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	struct clk_boston *state = to_clk_boston(hw);
+	uint32_t in_rate, mul, div;
+	uint mmcmdiv;
+	int err;
+
+	err = regmap_read(state->regmap, BOSTON_PLAT_MMCMDIV, &mmcmdiv);
+	if (err)
+		return 0;
+
+	in_rate = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_INPUT);
+	mul = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_MUL);
+
+	switch (state->id) {
+	case BOSTON_CLK_SYS:
+		div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK0DIV);
+		break;
+	case BOSTON_CLK_CPU:
+		div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK1DIV);
+		break;
+	default:
+		return 0;
+	}
+
+	return (in_rate * mul * 1000000) / div;
+}
+
+static const struct clk_ops clk_boston_ops = {
+	.recalc_rate = clk_boston_recalc_rate,
+};
+
+static void __init clk_boston_setup(struct device_node *np)
+{
+	struct clk_boston_state *state;
+	struct clk_init_data init;
+	struct regmap *regmap;
+	int i, err;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return;
+
+	regmap = syscon_regmap_lookup_by_phandle(np, "regmap");
+	if (IS_ERR(regmap)) {
+		pr_err("failed to find regmap\n");
+		return;
+	}
+
+	for (i = 0; i < BOSTON_CLK_COUNT; i++) {
+		memset(&init, 0, sizeof(init));
+		init.flags = CLK_IS_BASIC;
+		init.name = clk_names[i];
+		init.ops = &clk_boston_ops;
+
+		state->clk_boston[i].hw.init = &init;
+		state->clk_boston[i].id = i;
+		state->clk_boston[i].regmap = regmap;
+
+		state->clk[i] = clk_register(NULL, &state->clk_boston[i].hw);
+		if (IS_ERR(state->clk[i])) {
+			pr_err("failed to register clock: %ld\n",
+			       PTR_ERR(state->clk[i]));
+			return;
+		}
+	}
+
+	state->onecell_data->clks = state->clk;
+	state->onecell_data->clk_num = BOSTON_CLK_COUNT;
+
+	err = of_clk_add_provider(np, of_clk_src_onecell_get,
+				  state->onecell_data);
+	if (err)
+		pr_err("failed to add DT provider: %d\n", err);
+}
+CLK_OF_DECLARE(clk_boston, "img,boston-clock", clk_boston_setup);
-- 
2.9.3

Re: [PATCH 25/26] clk: boston: Add a driver for MIPS Boston board clocks

[Stephen Boyd]

On 08/26, Paul Burton wrote:
> 
>  drivers/clk/Kconfig      |   9 ++++
>  drivers/clk/Makefile     |   1 +
>  drivers/clk/clk-boston.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++

Maybe a vendor subdirectory is appropriate? imgtec?

> +
> +struct clk_boston_state {
> +	struct clk *clk[BOSTON_CLK_COUNT];
> +	struct clk_boston clk_boston[BOSTON_CLK_COUNT];
> +	struct clk_onecell_data onecell_data[BOSTON_CLK_COUNT];
> +};
> +
> +static const char *clk_names[BOSTON_CLK_COUNT] = {

const char * const?

> +	[BOSTON_CLK_SYS] = "sys",
> +	[BOSTON_CLK_CPU] = "cpu",
> +};
> +
> +#define BOSTON_PLAT_MMCMDIV		0x30
> +# define BOSTON_PLAT_MMCMDIV_CLK0DIV	(0xff << 0)
> +# define BOSTON_PLAT_MMCMDIV_INPUT	(0xff << 8)
> +# define BOSTON_PLAT_MMCMDIV_MUL	(0xff << 16)
> +# define BOSTON_PLAT_MMCMDIV_CLK1DIV	(0xff << 24)
> +
> +static struct clk_boston *to_clk_boston(struct clk_hw *hw)
> +{
> +	return container_of(hw, struct clk_boston, hw);
> +}
> +
> +static uint32_t ext_field(uint32_t val, uint32_t mask)

Please use u32 instead of uint32_t in drivers.

> +{
> +	return (val & mask) >> (ffs(mask) - 1);
> +}
> +
> +static unsigned long clk_boston_recalc_rate(struct clk_hw *hw,
> +					    unsigned long parent_rate)
> +{
> +	struct clk_boston *state = to_clk_boston(hw);
> +	uint32_t in_rate, mul, div;
> +	uint mmcmdiv;

unsigned int?

> +	int err;
> +
> +	err = regmap_read(state->regmap, BOSTON_PLAT_MMCMDIV, &mmcmdiv);
> +	if (err)
> +		return 0;
> +
> +	in_rate = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_INPUT);

This sounds like a parent rate? Should there be another clk
created for that so that parent_rate in this function is useful?

> +	mul = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_MUL);
> +
> +	switch (state->id) {
> +	case BOSTON_CLK_SYS:
> +		div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK0DIV);
> +		break;
> +	case BOSTON_CLK_CPU:
> +		div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK1DIV);

Why not put the CLK0DIV or CLK1DIV offset in state->id instead?
That way this function just read in_rate, mul, and div and then
does the math?

> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return (in_rate * mul * 1000000) / div;

Is this always fixed at boot? It may be easier to populate fixed
rate clks during probe with the rate calculated there. Then there
aren't any clk_ops to implement.

> +}
> +
> +static const struct clk_ops clk_boston_ops = {
> +	.recalc_rate = clk_boston_recalc_rate,
> +};
> +
> +static void __init clk_boston_setup(struct device_node *np)
> +{
> +	struct clk_boston_state *state;
> +	struct clk_init_data init;
> +	struct regmap *regmap;
> +	int i, err;
> +
> +	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return;
> +
> +	regmap = syscon_regmap_lookup_by_phandle(np, "regmap");
> +	if (IS_ERR(regmap)) {
> +		pr_err("failed to find regmap\n");
> +		return;
> +	}
> +
> +	for (i = 0; i < BOSTON_CLK_COUNT; i++) {
> +		memset(&init, 0, sizeof(init));
> +		init.flags = CLK_IS_BASIC;

Please drop this flag unless you really need it for something. As
far as I know CLK_IS_BASIC is just for OMAP code.

> +		init.name = clk_names[i];
> +		init.ops = &clk_boston_ops;
> +
> +		state->clk_boston[i].hw.init = &init;
> +		state->clk_boston[i].id = i;
> +		state->clk_boston[i].regmap = regmap;
> +
> +		state->clk[i] = clk_register(NULL, &state->clk_boston[i].hw);

Please use clk_hw_register() instead.

> +		if (IS_ERR(state->clk[i])) {
> +			pr_err("failed to register clock: %ld\n",
> +			       PTR_ERR(state->clk[i]));
> +			return;
> +		}
> +	}
> +
> +	state->onecell_data->clks = state->clk;
> +	state->onecell_data->clk_num = BOSTON_CLK_COUNT;
> +
> +	err = of_clk_add_provider(np, of_clk_src_onecell_get,
> +				  state->onecell_data);

Please use of_clk_add_hw_provider() instead.

> +	if (err)
> +		pr_err("failed to add DT provider: %d\n", err);
> +}
> +CLK_OF_DECLARE(clk_boston, "img,boston-clock", clk_boston_setup);

Please make this into a platform driver.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 24/26] dt-bindings: Document img,boston-clock binding

[Stephen Boyd]

On 08/26, Paul Burton wrote:
> diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
> new file mode 100644
> index 0000000..c01ea60
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
> @@ -0,0 +1,27 @@
> +Binding for Imagination Technologies MIPS Boston clock sources.
> +
> +This binding uses the common clock binding[1].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible : Should be "img,boston-clock".
> +- #clock-cells : Should be set to 1.
> +  Values available for clock consumers can be found in the header file:
> +    <dt-bindings/clock/boston-clock.h>
> +- regmap : Phandle to the Boston platform register system controller.
> +  This should contain a phandle to the system controller node covering the
> +  platform registers provided by the Boston board.
> +
> +Example:
> +
> +	clk_boston: clock {
> +		compatible = "img,boston-clock";
> +		#clock-cells = <1>;
> +		regmap = <&plat_regs>;

Isn't syscon more standard than regmap as the property name? Is
there a binding for the plat_regs device? Is there any reason the
clks can't be populated in that syscon driver?

> +	};
> +
> +	uart0: uart@17ffe000 {
> +		/* ... */
> +		clocks = <&clk_boston BOSTON_CLK_SYS>;
> +	};

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 25/26] clk: boston: Add a driver for MIPS Boston board clocks

[Paul Burton]

On 26/08/16 18:41, Stephen Boyd wrote:
>> +	if (err)
>> +		pr_err("failed to add DT provider: %d\n", err);
>> +}
>> +CLK_OF_DECLARE(clk_boston, "img,boston-clock", clk_boston_setup);
> 
> Please make this into a platform driver.

Hi Stephen,

The problem with this would be that we need to obtain the CPU clock rate
fairly early during boot in order to set up the clocksource & delay loop
etc. Using CLK_OF_DECLARE allows for that but if this were a platform
driver my understanding is that the clocks wouldn't become available
until some point later in boot. If I'm wrong & there's a way to avoid
that please let me know.

Also: why? If CLK_OF_DECLARE isn't liked, shouldn't that be documented
somewhere (ideally next to the declaration of CLK_OF_DECLARE in
include/linux/clk-provider.h)?

Thanks,
    Paul

Re: [PATCH 24/26] dt-bindings: Document img,boston-clock binding

[Paul Burton]

On 26/08/16 18:44, Stephen Boyd wrote:
> On 08/26, Paul Burton wrote:
>> diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
>> new file mode 100644
>> index 0000000..c01ea60
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
>> @@ -0,0 +1,27 @@
>> +Binding for Imagination Technologies MIPS Boston clock sources.
>> +
>> +This binding uses the common clock binding[1].
>> +
>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +
>> +Required properties:
>> +- compatible : Should be "img,boston-clock".
>> +- #clock-cells : Should be set to 1.
>> +  Values available for clock consumers can be found in the header file:
>> +    <dt-bindings/clock/boston-clock.h>
>> +- regmap : Phandle to the Boston platform register system controller.
>> +  This should contain a phandle to the system controller node covering the
>> +  platform registers provided by the Boston board.
>> +
>> +Example:
>> +
>> +	clk_boston: clock {
>> +		compatible = "img,boston-clock";
>> +		#clock-cells = <1>;
>> +		regmap = <&plat_regs>;
> 
> Isn't syscon more standard than regmap as the property name? Is
> there a binding for the plat_regs device? Is there any reason the
> clks can't be populated in that syscon driver?

Hi Stephen,

The plat_regs device doesn't have a custom driver, it simply makes use
of the generic "syscon" driver which can provide a regmap.

It would be possible to register the clocks from a register for the
plat_regs device, but I don't think it would make much sense. The
platform registers in question are essentially just a convenient place
where various bits of information about the system are exposed,
including the clock frequencies but also other bits & pieces like
connectivity of PCIe controllers or I/O coherence units, the RTL
revision of the CPU or the wrapper RTL that runs on this FPGA-based
board, a register that allows for resetting the board, etc. It's not a
single piece of hardware, more a dumping ground for miscellanea. So in
my opinion using the syscon approach works best here, and drivers for
well defined pieces of hardware or functionality can reference that
syscon to retrieve the regmap.

As for whether "syscon" is a more standard property name than "regmap",
both seem to be used based on a grep of
Documentation/devicetree/bindings/. I believe I picked up use of
"regmap" from the generic syscon-poweroff & syscon-reboot drivers, which
both use "regmap" as a property name.

Thanks,
    Paul

> 
>> +	};
>> +
>> +	uart0: uart@17ffe000 {
>> +		/* ... */
>> +		clocks = <&clk_boston BOSTON_CLK_SYS>;
>> +	};
> 

Re: [PATCH 24/26] dt-bindings: Document img,boston-clock binding

[Rob Herring]

On Tue, Aug 30, 2016 at 04:53:01PM +0100, Paul Burton wrote:
> On 26/08/16 18:44, Stephen Boyd wrote:
> > On 08/26, Paul Burton wrote:
> >> diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
> >> new file mode 100644
> >> index 0000000..c01ea60
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
> >> @@ -0,0 +1,27 @@
> >> +Binding for Imagination Technologies MIPS Boston clock sources.
> >> +
> >> +This binding uses the common clock binding[1].
> >> +
> >> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> +
> >> +Required properties:
> >> +- compatible : Should be "img,boston-clock".
> >> +- #clock-cells : Should be set to 1.
> >> +  Values available for clock consumers can be found in the header file:
> >> +    <dt-bindings/clock/boston-clock.h>
> >> +- regmap : Phandle to the Boston platform register system controller.
> >> +  This should contain a phandle to the system controller node covering the
> >> +  platform registers provided by the Boston board.
> >> +
> >> +Example:
> >> +
> >> +	clk_boston: clock {
> >> +		compatible = "img,boston-clock";
> >> +		#clock-cells = <1>;
> >> +		regmap = <&plat_regs>;
> > 
> > Isn't syscon more standard than regmap as the property name? Is
> > there a binding for the plat_regs device? Is there any reason the
> > clks can't be populated in that syscon driver?
> 
> Hi Stephen,
> 
> The plat_regs device doesn't have a custom driver, it simply makes use
> of the generic "syscon" driver which can provide a regmap.
> 
> It would be possible to register the clocks from a register for the
> plat_regs device, but I don't think it would make much sense. The
> platform registers in question are essentially just a convenient place
> where various bits of information about the system are exposed,
> including the clock frequencies but also other bits & pieces like
> connectivity of PCIe controllers or I/O coherence units, the RTL
> revision of the CPU or the wrapper RTL that runs on this FPGA-based
> board, a register that allows for resetting the board, etc. It's not a
> single piece of hardware, more a dumping ground for miscellanea. So in
> my opinion using the syscon approach works best here, and drivers for
> well defined pieces of hardware or functionality can reference that
> syscon to retrieve the regmap.

That is all quite common for any SoC. Whether it's 2 nodes or 2 drivers 
are independent questions. You can easily have 1 node and 2 drivers. The 
decision factor is really how many registers we're dealing with. We 
don't want to end up with a node per register or register field. That's 
too fine grained.

> As for whether "syscon" is a more standard property name than "regmap",
> both seem to be used based on a grep of
> Documentation/devicetree/bindings/. I believe I picked up use of
> "regmap" from the generic syscon-poweroff & syscon-reboot drivers, which
> both use "regmap" as a property name.

syscon is much more common.

Avoid the phandle altogether and make this a child node.

Rob

Re: [PATCH 24/26] dt-bindings: Document img,boston-clock binding

[Paul Burton]

On 02/09/16 13:54, Rob Herring wrote:
> On Tue, Aug 30, 2016 at 04:53:01PM +0100, Paul Burton wrote:
>> On 26/08/16 18:44, Stephen Boyd wrote:
>>> On 08/26, Paul Burton wrote:
>>>> diff --git a/Documentation/devicetree/bindings/clock/img,boston-clock.txt b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
>>>> new file mode 100644
>>>> index 0000000..c01ea60
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/img,boston-clock.txt
>>>> @@ -0,0 +1,27 @@
>>>> +Binding for Imagination Technologies MIPS Boston clock sources.
>>>> +
>>>> +This binding uses the common clock binding[1].
>>>> +
>>>> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>> +
>>>> +Required properties:
>>>> +- compatible : Should be "img,boston-clock".
>>>> +- #clock-cells : Should be set to 1.
>>>> +  Values available for clock consumers can be found in the header file:
>>>> +    <dt-bindings/clock/boston-clock.h>
>>>> +- regmap : Phandle to the Boston platform register system controller.
>>>> +  This should contain a phandle to the system controller node covering the
>>>> +  platform registers provided by the Boston board.
>>>> +
>>>> +Example:
>>>> +
>>>> +	clk_boston: clock {
>>>> +		compatible = "img,boston-clock";
>>>> +		#clock-cells = <1>;
>>>> +		regmap = <&plat_regs>;
>>>
>>> Isn't syscon more standard than regmap as the property name? Is
>>> there a binding for the plat_regs device? Is there any reason the
>>> clks can't be populated in that syscon driver?
>>
>> Hi Stephen,
>>
>> The plat_regs device doesn't have a custom driver, it simply makes use
>> of the generic "syscon" driver which can provide a regmap.
>>
>> It would be possible to register the clocks from a register for the
>> plat_regs device, but I don't think it would make much sense. The
>> platform registers in question are essentially just a convenient place
>> where various bits of information about the system are exposed,
>> including the clock frequencies but also other bits & pieces like
>> connectivity of PCIe controllers or I/O coherence units, the RTL
>> revision of the CPU or the wrapper RTL that runs on this FPGA-based
>> board, a register that allows for resetting the board, etc. It's not a
>> single piece of hardware, more a dumping ground for miscellanea. So in
>> my opinion using the syscon approach works best here, and drivers for
>> well defined pieces of hardware or functionality can reference that
>> syscon to retrieve the regmap.
> 
> That is all quite common for any SoC. Whether it's 2 nodes or 2 drivers 
> are independent questions. You can easily have 1 node and 2 drivers. The 
> decision factor is really how many registers we're dealing with. We 
> don't want to end up with a node per register or register field. That's 
> too fine grained.

Absolutely, I don't think we disagree there.

>> As for whether "syscon" is a more standard property name than "regmap",
>> both seem to be used based on a grep of
>> Documentation/devicetree/bindings/. I believe I picked up use of
>> "regmap" from the generic syscon-poweroff & syscon-reboot drivers, which
>> both use "regmap" as a property name.
> 
> syscon is much more common.
> 
> Avoid the phandle altogether and make this a child node.

I could do that, but it would feel rather odd to describe the clock
hardware as a child of a bunch of miscellaneous registers that happen to
expose some information about those clocks. Is that really what you
prefer? I think as-is the DT is a better description of the hardware.

Thanks,
    Paul

> 
> Rob
>