[PATCH 0/5] Add support for Mobileye EyeQ5 clock controller

[PATCH 0/5] Add support for Mobileye EyeQ5 clock controller

[Théo Lebrun]

Hi,

We replace fixed clocks as declared in the initial platform support
series [1] by read-only clocks exposed by the clock driver implemented
here. Write-ability is supported by the hardware but not implemented,
it could be added later-on if the need appears.

We expose ten PLLs that derive directly from the main crystal. Also, a divider
clock is exposed as a child clock of one of the PLLs.

The platform devicetree has many more clock nodes but those are
fixed-factors that are not hardware controllable; we therefore do not
deal with them.

This series starts with a fix for a fixed-rate clock registering macro that is
broken. We are the first upstream users which explains why we found the typo.

We end our patch series by adding the clock controller node to the
platform devicetree & by replacing hardcoded devicetree clocks by the
ones exposed by this driver. The controller driver addition is split in
two commits to simplify reviewing.

[1]: https://lore.kernel.org/lkml/20231212163459.1923041-1-gregory.clement@bootlin.com/

Have a nice day,
Théo Lebrun

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Théo Lebrun (5):
      clk: fixed-rate: fix clk_hw_register_fixed_rate_with_accuracy_parent_hw
      dt-bindings: clock: mobileye,eyeq5-clk: add bindings
      clk: eyeq5: add controller
      clk: eyeq5: add OSPI table-based divider clock
      MIPS: mobileye: eyeq5: add OLB clocks controller node & pinmux nodes

 .../bindings/clock/mobileye,eyeq5-clk.yaml         |  83 +++++
 MAINTAINERS                                        |   3 +
 .../{eyeq5-fixed-clocks.dtsi => eyeq5-clocks.dtsi} |  56 +---
 arch/mips/boot/dts/mobileye/eyeq5.dtsi             |   9 +-
 drivers/clk/Kconfig                                |  11 +
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-eyeq5.c                            | 346 +++++++++++++++++++++
 include/dt-bindings/clock/mobileye,eyeq5-clk.h     |  22 ++
 include/linux/clk-provider.h                       |   4 +-
 9 files changed, 493 insertions(+), 42 deletions(-)
---
base-commit: 0bb6b85cadabf93a754df740bd1b6c56ef41ac2c
change-id: 20231023-mbly-clk-87ce5c241f08

Best regards,
-- 
Théo Lebrun <theo.lebrun@bootlin.com>

[PATCH 1/5] clk: fixed-rate: fix clk_hw_register_fixed_rate_with_accuracy_parent_hw

[Théo Lebrun]

Add missing comma and remove extraneous NULL argument. The macro is
currently used by no one which explains why the typo slipped by.

Fixes: 2d34f09e79c9 ("clk: fixed-rate: Add support for specifying parents via DT/pointers")
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 include/linux/clk-provider.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index ace3a4ce2fc9..1293c38ddb7f 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -448,8 +448,8 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
  */
 #define clk_hw_register_fixed_rate_with_accuracy_parent_hw(dev, name,	      \
 		parent_hw, flags, fixed_rate, fixed_accuracy)		      \
-	__clk_hw_register_fixed_rate((dev), NULL, (name), NULL, (parent_hw)   \
-				     NULL, NULL, (flags), (fixed_rate),	      \
+	__clk_hw_register_fixed_rate((dev), NULL, (name), NULL, (parent_hw),  \
+				     NULL, (flags), (fixed_rate),	      \
 				     (fixed_accuracy), 0, false)
 /**
  * clk_hw_register_fixed_rate_with_accuracy_parent_data - register fixed-rate

-- 
2.43.0

[PATCH 2/5] dt-bindings: clock: mobileye,eyeq5-clk: add bindings

[Théo Lebrun]

Add DT schema bindings for the EyeQ5 clock controller driver.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../bindings/clock/mobileye,eyeq5-clk.yaml         | 83 ++++++++++++++++++++++
 MAINTAINERS                                        |  2 +
 include/dt-bindings/clock/mobileye,eyeq5-clk.h     | 22 ++++++
 3 files changed, 107 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
new file mode 100644
index 000000000000..d56482a06bf1
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/mobileye,eyeq5-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mobileye EyeQ5 clock controller
+
+description:
+  The EyeQ5 clock controller handles 10 read-only PLLs derived from the main
+  crystal clock. It also exposes one divider clock, a child of one of the PLLs.
+  It is custom to this platform, its registers live in a shared region called
+  OLB.
+
+maintainers:
+  - Grégory Clement <gregory.clement@bootlin.com>
+  - Théo Lebrun <theo.lebrun@bootlin.com>
+  - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
+
+properties:
+  $nodename:
+    pattern: "^clocks$"
+    description:
+      We have no unique address, we rely on OLB.
+
+  compatible:
+    const: mobileye,eyeq5-clk
+
+  "#clock-cells":
+    const: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      Input parent clock to all PLLs. Expected to be the main crystal.
+
+  clock-names:
+    items:
+      - const: ref
+
+  mobileye,olb:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      A phandle to the OLB syscon. This is a fallback to using the parent as
+      syscon node.
+
+required:
+  - compatible
+  - "#clock-cells"
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    olb@e00000 {
+      compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
+      reg = <0xe00000 0x400>;
+      reg-io-width = <4>;
+
+      clocks {
+        compatible = "mobileye,eyeq5-clk";
+        #clock-cells = <1>;
+        clocks = <&xtal>;
+        clock-names = "ref";
+      };
+    };
+
+  - |
+    olb: olb@e00000 {
+      compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
+      reg = <0xe00000 0x400>;
+      reg-io-width = <4>;
+    };
+
+    clocks {
+      compatible = "mobileye,eyeq5-clk";
+      mobileye,olb = <&olb>;
+      #clock-cells = <1>;
+      clocks = <&xtal>;
+      clock-names = "ref";
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 4a7bd6b40d74..7f04fa760a4d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14552,10 +14552,12 @@ M:	Gregory CLEMENT <gregory.clement@bootlin.com>
 M:	Théo Lebrun <theo.lebrun@bootlin.com>
 L:	linux-mips@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
 F:	Documentation/devicetree/bindings/mips/mobileye.yaml
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/generic/board-eyeq5.config
 F:	arch/mips/generic/board-epm5.its.S
+F:	include/dt-bindings/clock/mobileye,eyeq5-clk.h
 F:	include/dt-bindings/soc/mobileye,eyeq5.h
 
 MODULE SUPPORT
diff --git a/include/dt-bindings/clock/mobileye,eyeq5-clk.h b/include/dt-bindings/clock/mobileye,eyeq5-clk.h
new file mode 100644
index 000000000000..7aa974354bb6
--- /dev/null
+++ b/include/dt-bindings/clock/mobileye,eyeq5-clk.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) 2023 Mobileye Vision Technologies Ltd.
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_MOBILEYE_EYEQ5_CLK_H
+#define _DT_BINDINGS_CLOCK_MOBILEYE_EYEQ5_CLK_H
+
+#define EQ5C_PLL_CPU	0
+#define EQ5C_PLL_VMP	1
+#define EQ5C_PLL_PMA	2
+#define EQ5C_PLL_VDI	3
+#define EQ5C_PLL_DDR0	4
+#define EQ5C_PLL_PCI	5
+#define EQ5C_PLL_PER	6
+#define EQ5C_PLL_PMAC	7
+#define EQ5C_PLL_MPC	8
+#define EQ5C_PLL_DDR1	9
+
+#define EQ5C_DIV_OSPI	10
+
+#endif

-- 
2.43.0

[PATCH 3/5] clk: eyeq5: add controller

[Théo Lebrun]

Add the Mobileye EyeQ5 clock controller driver. See the header comment
for more information on how it works. This driver is specific to this
platform; it might grow to add later support of other platforms from
Mobileye.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 MAINTAINERS             |   1 +
 drivers/clk/Kconfig     |  11 +++
 drivers/clk/Makefile    |   1 +
 drivers/clk/clk-eyeq5.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 224 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7f04fa760a4d..c75c7de1d507 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14557,6 +14557,7 @@ F:	Documentation/devicetree/bindings/mips/mobileye.yaml
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/generic/board-eyeq5.config
 F:	arch/mips/generic/board-epm5.its.S
+F:	drivers/clk/clk-eyeq5.c
 F:	include/dt-bindings/clock/mobileye,eyeq5-clk.h
 F:	include/dt-bindings/soc/mobileye,eyeq5.h
 
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index c30d0d396f7a..84fe0a89b8df 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -218,6 +218,17 @@ config COMMON_CLK_EN7523
 	  This driver provides the fixed clocks and gates present on Airoha
 	  ARM silicon.
 
+config COMMON_CLK_EYEQ5
+	bool "Clock driver for the Mobileye EyeQ5 platform"
+	depends on OF
+	depends on SOC_EYEQ5 || COMPILE_TEST
+	default SOC_EYEQ5
+	help
+		This drivers provides the fixed clocks found on the Mobileye EyeQ5
+		SoC. Its registers live in a shared register region called OLB.
+		It provides 10 read-only PLLs derived from the main crystal clock which
+		must be constant.
+
 config COMMON_CLK_FSL_FLEXSPI
 	tristate "Clock driver for FlexSPI on Layerscape SoCs"
 	depends on ARCH_LAYERSCAPE || COMPILE_TEST
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index ed71f2e0ee36..0df0dc6dbcae 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
 obj-$(CONFIG_COMMON_CLK_CS2000_CP)	+= clk-cs2000-cp.o
 obj-$(CONFIG_ARCH_SPARX5)		+= clk-sparx5.o
 obj-$(CONFIG_COMMON_CLK_EN7523)		+= clk-en7523.o
+obj-$(CONFIG_COMMON_CLK_EYEQ5)		+= clk-eyeq5.o
 obj-$(CONFIG_COMMON_CLK_FIXED_MMIO)	+= clk-fixed-mmio.o
 obj-$(CONFIG_COMMON_CLK_FSL_FLEXSPI)	+= clk-fsl-flexspi.o
 obj-$(CONFIG_COMMON_CLK_FSL_SAI)	+= clk-fsl-sai.o
diff --git a/drivers/clk/clk-eyeq5.c b/drivers/clk/clk-eyeq5.c
new file mode 100644
index 000000000000..74bcb8cec5c1
--- /dev/null
+++ b/drivers/clk/clk-eyeq5.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PLL clock driver for the Mobileye EyeQ5 platform.
+ *
+ * This controller handles 10 read-only PLLs, all derived from the same main
+ * crystal clock. The parent clock is expected to be constant. This driver is
+ * custom to this platform, its registers live in a shared region called OLB.
+ *
+ * We use eq5c_ as prefix, as-in "EyeQ5 Clock", but way shorter.
+ *
+ * Copyright (C) 2023 Mobileye Vision Technologies Ltd.
+ */
+#include <linux/bits.h>
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+/*
+ * PLL control & status registers, n=0..1
+ * 0x02c..0x078
+ */
+#define OLB_PCSR_CPU(n)				(0x02C + (n) * 4) /* CPU */
+#define OLB_PCSR_VMP(n)				(0x034 + (n) * 4) /* VMP */
+#define OLB_PCSR_PMA(n)				(0x03C + (n) * 4) /* PMA */
+#define OLB_PCSR_VDI(n)				(0x044 + (n) * 4) /* VDI */
+#define OLB_PCSR_DDR0(n)			(0x04C + (n) * 4) /* DDR0 */
+#define OLB_PCSR_PCI(n)				(0x054 + (n) * 4) /* PCI */
+#define OLB_PCSR_PER(n)				(0x05C + (n) * 4) /* PER */
+#define OLB_PCSR_PMAC(n)			(0x064 + (n) * 4) /* PMAC */
+#define OLB_PCSR_MPC(n)				(0x06c + (n) * 4) /* MPC */
+#define OLB_PCSR_DDR1(n)			(0x074 + (n) * 4) /* DDR1 */
+
+/* In frac mode, it enables fractional noise canceling DAC. Else, no function. */
+#define OLB_PCSR0_DAC_EN			BIT(0)
+/* Fractional or integer mode */
+#define OLB_PCSR0_DSM_EN			BIT(1)
+#define OLB_PCSR0_PLL_EN			BIT(2)
+/* All clocks output held at 0 */
+#define OLB_PCSR0_FOUTPOSTDIV_EN		BIT(3)
+#define OLB_PCSR0_POST_DIV1			GENMASK(6, 4)
+#define OLB_PCSR0_POST_DIV2			GENMASK(9, 7)
+#define OLB_PCSR0_REF_DIV			GENMASK(15, 10)
+#define OLB_PCSR0_INTIN				GENMASK(27, 16)
+#define OLB_PCSR0_BYPASS			BIT(28)
+/* Bits 30..29 are reserved */
+#define OLB_PCSR0_PLL_LOCKED			BIT(31)
+
+#define OLB_PCSR1_RESET				BIT(0)
+#define OLB_PCSR1_SSGC_DIV			GENMASK(4, 1)
+/* Spread amplitude (% = 0.1 * SPREAD[4:0]) */
+#define OLB_PCSR1_SPREAD			GENMASK(9, 5)
+#define OLB_PCSR1_DIS_SSCG			BIT(10)
+/* Down-spread or center-spread */
+#define OLB_PCSR1_DOWN_SPREAD			BIT(11)
+#define OLB_PCSR1_FRAC_IN			GENMASK(31, 12)
+
+static const struct eq5c_pll {
+	const char *name;
+	u32 reg;
+} eq5c_plls[] = {
+	[EQ5C_PLL_CPU] =  { .name = "pll-cpu",  .reg = OLB_PCSR_CPU(0),  },
+	[EQ5C_PLL_VMP] =  { .name = "pll-vmp",  .reg = OLB_PCSR_VMP(0),  },
+	[EQ5C_PLL_PMA] =  { .name = "pll-pma",  .reg = OLB_PCSR_PMA(0),  },
+	[EQ5C_PLL_VDI] =  { .name = "pll-vdi",  .reg = OLB_PCSR_VDI(0),  },
+	[EQ5C_PLL_DDR0] = { .name = "pll-ddr0", .reg = OLB_PCSR_DDR0(0), },
+	[EQ5C_PLL_PCI] =  { .name = "pll-pci",  .reg = OLB_PCSR_PCI(0),  },
+	[EQ5C_PLL_PER] =  { .name = "pll-per",  .reg = OLB_PCSR_PER(0),  },
+	[EQ5C_PLL_PMAC] = { .name = "pll-pmac", .reg = OLB_PCSR_PMAC(0), },
+	[EQ5C_PLL_MPC] =  { .name = "pll-mpc",  .reg = OLB_PCSR_MPC(0),  },
+	[EQ5C_PLL_DDR1] = { .name = "pll-ddr1", .reg = OLB_PCSR_DDR1(0), },
+};
+
+static int eq5c_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
+				    unsigned long *div, unsigned long *acc)
+{
+	if (!mult || !div || !acc)
+		return -EINVAL;
+
+	if (r0 & OLB_PCSR0_BYPASS) {
+		*mult = 1;
+		*div = 1;
+		*acc = 0;
+		return 0;
+	}
+
+	if (!(r0 & OLB_PCSR0_PLL_LOCKED))
+		return -EINVAL;
+
+	*mult = FIELD_GET(OLB_PCSR0_INTIN, r0);
+	*div = FIELD_GET(OLB_PCSR0_REF_DIV, r0);
+	if (r0 & OLB_PCSR0_FOUTPOSTDIV_EN)
+		*div *= FIELD_GET(OLB_PCSR0_POST_DIV1, r0) *
+			FIELD_GET(OLB_PCSR0_POST_DIV2, r0);
+
+	/* Fractional mode, in 2^20 (0x100000) parts. */
+	if (r0 & OLB_PCSR0_DSM_EN) {
+		*div *= 0x100000;
+		*mult = *mult * 0x100000 + FIELD_GET(OLB_PCSR1_FRAC_IN, r1);
+	}
+
+	if (!*mult || !*div)
+		return -EINVAL;
+
+	/* Spread spectrum. */
+	if (!(r1 & (OLB_PCSR1_RESET | OLB_PCSR1_DIS_SSCG))) {
+		/*
+		 * Spread is 1/1000 parts of frequency, accuracy is half of
+		 * that. To get accuracy, convert to ppb (parts per billion).
+		 */
+		u32 spread = FIELD_GET(OLB_PCSR1_SPREAD, r1);
+		*acc = spread * 500000;
+		if (r1 & OLB_PCSR1_DOWN_SPREAD) {
+			/*
+			 * Downspreading: the central frequency is half a
+			 * spread lower.
+			 */
+			*mult *= 2000 - spread;
+			*div *= 2000;
+		}
+	} else {
+		*acc = 0;
+	}
+
+	return 0;
+}
+
+static void eq5c_init(struct device_node *np)
+{
+	struct device_node *parent_np = of_get_parent(np);
+	struct clk_hw_onecell_data *data;
+	unsigned long parent_clk_rate;
+	struct clk_hw *parent_clk_hw;
+	struct clk *parent_clk;
+	struct regmap *olb;
+	int i;
+
+	data = kzalloc(struct_size(data, hws, ARRAY_SIZE(eq5c_plls)), GFP_KERNEL);
+	if (!data)
+		return;
+
+	data->num = ARRAY_SIZE(eq5c_plls);
+
+	/*
+	 * TODO: currently, if OLB is not available, we log an error and early
+	 * return. We might want to change this behavior and assume all clocks
+	 * are in bypass mode; that is what is being done in the vendor driver.
+	 *
+	 * It is still unclear if there are valid situations where the OLB
+	 * region would be inaccessible.
+	 */
+	olb = ERR_PTR(-ENODEV);
+	if (parent_np)
+		olb = syscon_node_to_regmap(parent_np);
+	if (IS_ERR(olb))
+		olb = syscon_regmap_lookup_by_phandle(np, "mobileye,olb");
+	if (IS_ERR(olb)) {
+		pr_err("failed getting regmap: %ld\n", PTR_ERR(olb));
+		return;
+	}
+
+	parent_clk = of_clk_get_by_name(np, "ref");
+	if (IS_ERR_OR_NULL(parent_clk)) {
+		pr_err("no parent clock found\n");
+		return;
+	}
+	parent_clk_hw = __clk_get_hw(parent_clk);
+	parent_clk_rate = clk_get_rate(parent_clk);
+	clk_put(parent_clk);
+
+	for (i = 0; i < ARRAY_SIZE(eq5c_plls); i++) {
+		const struct eq5c_pll *pll = &eq5c_plls[i];
+		unsigned long mult, div, acc;
+		u32 r0, r1;
+		int ret;
+
+		regmap_read(olb, pll->reg, &r0);
+		regmap_read(olb, pll->reg + sizeof(r0), &r1);
+
+		ret = eq5c_pll_parse_registers(r0, r1, &mult, &div, &acc);
+		if (ret) {
+			pr_warn("failed parsing state of %s\n", pll->name);
+			continue;
+		}
+
+		/* We use fixed_rate and not fixed_factor because the latter
+		 * does not allow reporting accuracy. The alternative is to
+		 * create a custom clk implementation but that adds too many
+		 * lines to the kernel for not much benefit; our parent clock
+		 * rate won't change anyway.
+		 */
+		data->hws[i] = clk_hw_register_fixed_rate_with_accuracy_parent_hw(
+				NULL, pll->name, parent_clk_hw, 0,
+				parent_clk_rate * mult / div, acc);
+		if (IS_ERR_OR_NULL(data->hws[i])) {
+			pr_err("failed registering %s: %ld\n",
+			       pll->name, PTR_ERR(data->hws[i]));
+			data->hws[i] = ERR_PTR(-ENOENT);
+		}
+	}
+
+	of_clk_add_hw_provider(np, of_clk_hw_onecell_get, data);
+}
+
+CLK_OF_DECLARE_DRIVER(eq5c, "mobileye,eyeq5-clk", eq5c_init);

-- 
2.43.0

[PATCH 4/5] clk: eyeq5: add OSPI table-based divider clock

[Théo Lebrun]

The driver supports PLLs on the platform. Add the single divider clock
of the platform.

Helpers from include/linux/clk-provider.h could have been used if it was
not for the use of regmap to access the register.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/clk/Kconfig     |   2 +-
 drivers/clk/clk-eyeq5.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 140 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 84fe0a89b8df..63cc354f41ab 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -227,7 +227,7 @@ config COMMON_CLK_EYEQ5
 		This drivers provides the fixed clocks found on the Mobileye EyeQ5
 		SoC. Its registers live in a shared register region called OLB.
 		It provides 10 read-only PLLs derived from the main crystal clock which
-		must be constant.
+		must be constant and one divider clock based on one PLLs.
 
 config COMMON_CLK_FSL_FLEXSPI
 	tristate "Clock driver for FlexSPI on Layerscape SoCs"
diff --git a/drivers/clk/clk-eyeq5.c b/drivers/clk/clk-eyeq5.c
index 74bcb8cec5c1..3382f4d870d7 100644
--- a/drivers/clk/clk-eyeq5.c
+++ b/drivers/clk/clk-eyeq5.c
@@ -3,8 +3,9 @@
  * PLL clock driver for the Mobileye EyeQ5 platform.
  *
  * This controller handles 10 read-only PLLs, all derived from the same main
- * crystal clock. The parent clock is expected to be constant. This driver is
- * custom to this platform, its registers live in a shared region called OLB.
+ * crystal clock. It also exposes one divider clock, a child of one of the
+ * PLLs. The parent clock is expected to be constant. This driver is custom to
+ * this platform, its registers live in a shared region called OLB.
  *
  * We use eq5c_ as prefix, as-in "EyeQ5 Clock", but way shorter.
  *
@@ -77,6 +78,8 @@ static const struct eq5c_pll {
 	[EQ5C_PLL_DDR1] = { .name = "pll-ddr1", .reg = OLB_PCSR_DDR1(0), },
 };
 
+#define EQ5C_OSPI_DIV_CLK_NAME	"div-ospi"
+
 static int eq5c_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
 				    unsigned long *div, unsigned long *acc)
 {
@@ -131,6 +134,128 @@ static int eq5c_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
 	return 0;
 }
 
+#define OLB_OSPI_REG		0x11C
+#define OLB_OSPI_DIV_MASK	GENMASK(3, 0)
+#define OLB_OSPI_DIV_MASK_WIDTH	4
+
+static const struct clk_div_table eq5c_ospi_div_table[] = {
+	{ .val = 0, .div = 2 },
+	{ .val = 1, .div = 4 },
+	{ .val = 2, .div = 6 },
+	{ .val = 3, .div = 8 },
+	{ .val = 4, .div = 10 },
+	{ .val = 5, .div = 12 },
+	{ .val = 6, .div = 14 },
+	{ .val = 7, .div = 16 },
+	{} /* sentinel */
+};
+
+struct eq5c_ospi_div {
+	struct clk_hw	hw;
+	struct regmap	*olb;
+};
+
+static struct eq5c_ospi_div *clk_hw_to_ospi_priv(struct clk_hw *hw)
+{
+	return container_of(hw, struct eq5c_ospi_div, hw);
+}
+
+static unsigned long eq5c_ospi_div_recalc_rate(struct clk_hw *hw,
+					       unsigned long parent_rate)
+{
+	struct eq5c_ospi_div *div = clk_hw_to_ospi_priv(hw);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(div->olb, OLB_OSPI_REG, &val);
+
+	if (ret) {
+		pr_err("%s: regmap_read failed: %d\n", __func__, ret);
+		return 0;
+	}
+
+	val = FIELD_GET(OLB_OSPI_DIV_MASK, val);
+
+	return divider_recalc_rate(hw, parent_rate, val,
+				   eq5c_ospi_div_table, 0,
+				   OLB_OSPI_DIV_MASK_WIDTH);
+}
+
+static long eq5c_ospi_div_round_rate(struct clk_hw *hw,
+				     unsigned long rate, unsigned long *prate)
+{
+	return divider_round_rate(hw, rate, prate, eq5c_ospi_div_table,
+				  OLB_OSPI_DIV_MASK_WIDTH, 0);
+}
+
+static int eq5c_ospi_div_determine_rate(struct clk_hw *hw,
+					struct clk_rate_request *req)
+{
+	return divider_determine_rate(hw, req, eq5c_ospi_div_table,
+				      OLB_OSPI_DIV_MASK_WIDTH, 0);
+}
+
+static int eq5c_ospi_div_set_rate(struct clk_hw *hw,
+				  unsigned long rate, unsigned long parent_rate)
+{
+	struct eq5c_ospi_div *div = clk_hw_to_ospi_priv(hw);
+	unsigned int val;
+	int value, ret;
+
+	value = divider_get_val(rate, parent_rate, eq5c_ospi_div_table,
+				OLB_OSPI_DIV_MASK_WIDTH, 0);
+	if (value < 0)
+		return value;
+
+	ret = regmap_read(div->olb, OLB_OSPI_REG, &val);
+	if (ret) {
+		pr_err("%s: regmap_read failed: %d\n", __func__, ret);
+		return -ret;
+	}
+
+	val &= ~OLB_OSPI_DIV_MASK;
+	val |= FIELD_PREP(OLB_OSPI_DIV_MASK, value);
+
+	ret = regmap_write(div->olb, OLB_OSPI_REG, val);
+	if (ret) {
+		pr_err("%s: regmap_write failed: %d\n", __func__, ret);
+		return -ret;
+	}
+
+	return 0;
+}
+
+const struct clk_ops eq5c_ospi_div_ops = {
+	.recalc_rate = eq5c_ospi_div_recalc_rate,
+	.round_rate = eq5c_ospi_div_round_rate,
+	.determine_rate = eq5c_ospi_div_determine_rate,
+	.set_rate = eq5c_ospi_div_set_rate,
+};
+
+static struct clk_hw *eq5c_init_ospi_div(const struct clk_hw *parent,
+					 struct regmap *olb)
+{
+	struct eq5c_ospi_div *div;
+	int ret;
+
+	div = kzalloc(sizeof(*div), GFP_KERNEL);
+	if (!div)
+		return ERR_PTR(-ENOENT);
+
+	div->olb = olb;
+	div->hw.init = CLK_HW_INIT_HW(EQ5C_OSPI_DIV_CLK_NAME, parent,
+				      &eq5c_ospi_div_ops, 0);
+
+	ret = clk_hw_register(NULL, &div->hw);
+	if (ret) {
+		pr_err("failed registering div_ospi: %d\n", ret);
+		kfree(div);
+		return ERR_PTR(-ENOENT);
+	}
+
+	return &div->hw;
+}
+
 static void eq5c_init(struct device_node *np)
 {
 	struct device_node *parent_np = of_get_parent(np);
@@ -139,13 +264,15 @@ static void eq5c_init(struct device_node *np)
 	struct clk_hw *parent_clk_hw;
 	struct clk *parent_clk;
 	struct regmap *olb;
+	size_t nb_clks;
 	int i;
 
-	data = kzalloc(struct_size(data, hws, ARRAY_SIZE(eq5c_plls)), GFP_KERNEL);
+	nb_clks = ARRAY_SIZE(eq5c_plls) + 1;
+	data = kzalloc(struct_size(data, hws, nb_clks), GFP_KERNEL);
 	if (!data)
 		return;
 
-	data->num = ARRAY_SIZE(eq5c_plls);
+	data->num = nb_clks;
 
 	/*
 	 * TODO: currently, if OLB is not available, we log an error and early
@@ -205,6 +332,14 @@ static void eq5c_init(struct device_node *np)
 		}
 	}
 
+	/*
+	 * Register the OSPI table-based divider clock manually. This is
+	 * equivalent to drivers/clk/clk-divider.c, but using regmap to access
+	 * its register.
+	 */
+	i = ARRAY_SIZE(eq5c_plls);
+	data->hws[i] = eq5c_init_ospi_div(data->hws[EQ5C_PLL_PER], olb);
+
 	of_clk_add_hw_provider(np, of_clk_hw_onecell_get, data);
 }
 

-- 
2.43.0

[PATCH 5/5] MIPS: mobileye: eyeq5: add OLB clocks controller node & pinmux nodes

[Théo Lebrun]

We both add the PLL controller (read-only PLLs) node inside the OLB
memory region and add pinmux function nodes.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../{eyeq5-fixed-clocks.dtsi => eyeq5-clocks.dtsi} | 56 +++++++---------------
 arch/mips/boot/dts/mobileye/eyeq5.dtsi             |  9 +++-
 2 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi b/arch/mips/boot/dts/mobileye/eyeq5-clocks.dtsi
similarity index 88%
rename from arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi
rename to arch/mips/boot/dts/mobileye/eyeq5-clocks.dtsi
index 78f5533a95c6..d024e6968396 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5-clocks.dtsi
@@ -3,42 +3,20 @@
  * Copyright 2023 Mobileye Vision Technologies Ltd.
  */
 
-/ {
-	/* Fixed clock */
-	pll_cpu: pll-cpu {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <1500000000>;
-	};
+#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
 
-	pll_vdi: pll-vdi {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <1280000000>;
-	};
-
-	pll_per: pll-per {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <2000000000>;
-	};
-
-	pll_ddr0: pll-ddr0 {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <1857210000>;
-	};
-
-	pll_ddr1: pll-ddr1 {
+/ {
+/* Fixed clock */
+	xtal: xtal {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
-		clock-frequency = <1857210000>;
+		clock-frequency = <30000000>;
 	};
 
 /* PLL_CPU derivatives */
 	occ_cpu: occ-cpu {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_cpu>;
+		clocks = <&clocks EQ5C_PLL_CPU>;
 		#clock-cells = <0>;
 		clock-div = <1>;
 		clock-mult = <1>;
@@ -101,7 +79,7 @@ mem_clk: mem-clk {
 	};
 	occ_isram: occ-isram {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_cpu>;
+		clocks = <&clocks EQ5C_PLL_CPU>;
 		#clock-cells = <0>;
 		clock-div = <2>;
 		clock-mult = <1>;
@@ -115,7 +93,7 @@ isram_clk: isram-clk { /* gate ClkRstGen_isram */
 	};
 	occ_dbu: occ-dbu {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_cpu>;
+		clocks = <&clocks EQ5C_PLL_CPU>;
 		#clock-cells = <0>;
 		clock-div = <10>;
 		clock-mult = <1>;
@@ -130,7 +108,7 @@ si_dbu_tp_pclk: si-dbu-tp-pclk { /* gate ClkRstGen_dbu */
 /* PLL_VDI derivatives */
 	occ_vdi: occ-vdi {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_vdi>;
+		clocks = <&clocks EQ5C_PLL_VDI>;
 		#clock-cells = <0>;
 		clock-div = <2>;
 		clock-mult = <1>;
@@ -144,7 +122,7 @@ vdi_clk: vdi-clk { /* gate ClkRstGen_vdi */
 	};
 	occ_can_ser: occ-can-ser {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_vdi>;
+		clocks = <&clocks EQ5C_PLL_VDI>;
 		#clock-cells = <0>;
 		clock-div = <16>;
 		clock-mult = <1>;
@@ -158,7 +136,7 @@ can_ser_clk: can-ser-clk { /* gate ClkRstGen_can_ser */
 	};
 	i2c_ser_clk: i2c-ser-clk {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_vdi>;
+		clocks = <&clocks EQ5C_PLL_VDI>;
 		#clock-cells = <0>;
 		clock-div = <20>;
 		clock-mult = <1>;
@@ -166,7 +144,7 @@ i2c_ser_clk: i2c-ser-clk {
 /* PLL_PER derivatives */
 	occ_periph: occ-periph {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <16>;
 		clock-mult = <1>;
@@ -225,7 +203,7 @@ gpio_clk: gpio-clk {
 	};
 	emmc_sys_clk: emmc-sys-clk {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <10>;
 		clock-mult = <1>;
@@ -233,7 +211,7 @@ emmc_sys_clk: emmc-sys-clk {
 	};
 	ccf_ctrl_clk: ccf-ctrl-clk {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <4>;
 		clock-mult = <1>;
@@ -241,7 +219,7 @@ ccf_ctrl_clk: ccf-ctrl-clk {
 	};
 	occ_mjpeg_core: occ-mjpeg-core {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <2>;
 		clock-mult = <1>;
@@ -265,7 +243,7 @@ mjpeg_core_clk: mjpeg-core-clk { /* gate ClkRstGen_mjpeg_gen */
 	};
 	fcmu_a_clk: fcmu-a-clk {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <20>;
 		clock-mult = <1>;
@@ -273,7 +251,7 @@ fcmu_a_clk: fcmu-a-clk {
 	};
 	occ_pci_sys: occ-pci-sys {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <8>;
 		clock-mult = <1>;
diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index d32da8fabe5a..76ec650631db 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -7,7 +7,7 @@
 
 /memreserve/ 0x40000000 0xc0000000; /* DDR32 */
 
-#include "eyeq5-fixed-clocks.dtsi"
+#include "eyeq5-clocks.dtsi"
 
 / {
 	#address-cells = <2>;
@@ -76,6 +76,13 @@ olb: olb@e00000 {
 			compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
 			reg = <0 0xe00000 0x0 0x400>;
 			reg-io-width = <4>;
+
+			clocks: clocks {
+				compatible = "mobileye,eyeq5-clk";
+				#clock-cells = <1>;
+				clocks = <&xtal>;
+				clock-names = "ref";
+			};
 		};
 
 		gic: interrupt-controller@140000 {

-- 
2.43.0

Re: [PATCH 2/5] dt-bindings: clock: mobileye,eyeq5-clk: add bindings

[Rob Herring]

On Mon, 18 Dec 2023 18:14:17 +0100, Théo Lebrun wrote:
> Add DT schema bindings for the EyeQ5 clock controller driver.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  .../bindings/clock/mobileye,eyeq5-clk.yaml         | 83 ++++++++++++++++++++++
>  MAINTAINERS                                        |  2 +
>  include/dt-bindings/clock/mobileye,eyeq5-clk.h     | 22 ++++++
>  3 files changed, 107 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.example.dtb: /example-0/olb@e00000: failed to match any schema with compatible: ['mobileye,eyeq5-olb', 'syscon', 'simple-mfd']
Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.example.dtb: /example-1/olb@e00000: failed to match any schema with compatible: ['mobileye,eyeq5-olb', 'syscon', 'simple-mfd']
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.example.dtb: clocks: 'mobileye,olb' does not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|gpio|gpmc|hdmi|i2c-gpio),.*', '^(keypad|m25p|max8952|max8997|max8998|mpmc),.*', '^(pinctrl-single|#pinctrl-single|PowerPC),.*', '^(pl022|pxa-mmc|rcar_sound|rotary-encoder|s5m8767|sdhci),.*', '^(simple-audio-card|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acbel,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^aldec,.*', '^alfa-network,.*', '^allegro,.*', '^all
 o,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^bytedance,.*', '^calamp
 ,.*', '^calaosystems,.*', '^calxeda,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^ceva,.*', '^chargebyte,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chongzhou,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^clockwork,.*', '^cloos,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^congatec,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^do
 mintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebbg,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edgeble,.*', '^edimax,.*', '^edt,.*', '^ees,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embedfire,.*', '^embest,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^evervision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairphone,.*', '^faraday,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^garden
 a,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goldelico,.*', '^goodix,.*', '^google,.*', '^goramo,.*', '^gplus,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hechuang,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.*', '^honestar,.*', '^honeywell,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^inanbo,.*', '^incircuit,.*', '^indiedroid,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^ingrasys,.*', '^i
 njoinic,.*', '^innocomm,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inventec,.*', '^inversepath,.*', '^iom,.*', '^irondevice,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jadard,.*', '^jasonic,.*', '^jdi,.*', '^jedec,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon,.*', '^
 litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marantec,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^maxlinear,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,.*', '^memsensing,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^methode,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '^mti,.*'
 , '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^nec,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^novatek,.*', '^novtech,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '^picochi
 p,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^polyhex,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^powkiddy,.*', '^primux,.*', '^probox2,.*', '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*', '^sens
 eair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^sifive,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^square,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*', '^starry,.*', '^startek,.*', '^starterkit,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*', '^sun
 chip,.*', '^sundance,.*', '^sunplus,.*', '^supermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^tcs,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^teejet,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vialab,.*
 ', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.*', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^vot,.*', '^vxt,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^yiming,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^zarlink,.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/vendor-prefixes.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231218-mbly-clk-v1-2-44ce54108f06@bootlin.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 2/5] dt-bindings: clock: mobileye,eyeq5-clk: add bindings

[Krzysztof Kozlowski]

On 18/12/2023 18:14, Théo Lebrun wrote:
> Add DT schema bindings for the EyeQ5 clock controller driver.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  .../bindings/clock/mobileye,eyeq5-clk.yaml         | 83 ++++++++++++++++++++++
>  MAINTAINERS                                        |  2 +
>  include/dt-bindings/clock/mobileye,eyeq5-clk.h     | 22 ++++++
>  3 files changed, 107 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
> new file mode 100644
> index 000000000000..d56482a06bf1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/mobileye,eyeq5-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mobileye EyeQ5 clock controller
> +
> +description:
> +  The EyeQ5 clock controller handles 10 read-only PLLs derived from the main
> +  crystal clock. It also exposes one divider clock, a child of one of the PLLs.
> +  It is custom to this platform, its registers live in a shared region called
> +  OLB.
> +
> +maintainers:
> +  - Grégory Clement <gregory.clement@bootlin.com>
> +  - Théo Lebrun <theo.lebrun@bootlin.com>
> +  - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
> +
> +properties:
> +  $nodename:
> +    pattern: "^clocks$"

No, that's not correct pattern.

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +    description:
> +      We have no unique address, we rely on OLB.

No.

I explained why in pinctrl patchset.

> +
> +  compatible:
> +    const: mobileye,eyeq5-clk
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description:
> +      Input parent clock to all PLLs. Expected to be the main crystal.
> +
> +  clock-names:
> +    items:
> +      - const: ref
> +
> +  mobileye,olb:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      A phandle to the OLB syscon. This is a fallback to using the parent as
> +      syscon node.

Drop.

> +
> +required:
> +  - compatible
> +  - "#clock-cells"
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    olb@e00000 {
> +      compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";

Drop, not related.

> +      reg = <0xe00000 0x400>;
> +      reg-io-width = <4>;
> +
> +      clocks {
> +        compatible = "mobileye,eyeq5-clk";
> +        #clock-cells = <1>;
> +        clocks = <&xtal>;
> +        clock-names = "ref";
> +      };
> +    };
> +
> +  - |
> +    olb: olb@e00000 {
> +      compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";

Drop, even less related. Still no explanation why you represent the same
hardware in two different ways.


Best regards,
Krzysztof

Re: [PATCH 3/5] clk: eyeq5: add controller

[Stephen Boyd]

Quoting Théo Lebrun (2023-12-18 09:14:18)
> Add the Mobileye EyeQ5 clock controller driver. See the header comment
> for more information on how it works.

"See the header" is like saying "Read the code" which is pretty obvious.
Remove this sentence and tell us why only the PLLs are supported at the
moment or something like that.

> This driver is specific to this
> platform; it might grow to add later support of other platforms from
> Mobileye.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  MAINTAINERS             |   1 +
>  drivers/clk/Kconfig     |  11 +++
>  drivers/clk/Makefile    |   1 +
>  drivers/clk/clk-eyeq5.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 224 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7f04fa760a4d..c75c7de1d507 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14557,6 +14557,7 @@ F:      Documentation/devicetree/bindings/mips/mobileye.yaml
>  F:     arch/mips/boot/dts/mobileye/
>  F:     arch/mips/configs/generic/board-eyeq5.config
>  F:     arch/mips/generic/board-epm5.its.S
> +F:     drivers/clk/clk-eyeq5.c
>  F:     include/dt-bindings/clock/mobileye,eyeq5-clk.h
>  F:     include/dt-bindings/soc/mobileye,eyeq5.h
>  
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index c30d0d396f7a..84fe0a89b8df 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -218,6 +218,17 @@ config COMMON_CLK_EN7523
>           This driver provides the fixed clocks and gates present on Airoha
>           ARM silicon.
>  
> +config COMMON_CLK_EYEQ5
> +       bool "Clock driver for the Mobileye EyeQ5 platform"
> +       depends on OF
> +       depends on SOC_EYEQ5 || COMPILE_TEST
> +       default SOC_EYEQ5
> +       help
> +               This drivers provides the fixed clocks found on the Mobileye EyeQ5

s/drivers/driver/

> +               SoC. Its registers live in a shared register region called OLB.
> +               It provides 10 read-only PLLs derived from the main crystal clock which
> +               must be constant.
> +
>  config COMMON_CLK_FSL_FLEXSPI
>         tristate "Clock driver for FlexSPI on Layerscape SoCs"
>         depends on ARCH_LAYERSCAPE || COMPILE_TEST
> diff --git a/drivers/clk/clk-eyeq5.c b/drivers/clk/clk-eyeq5.c
> new file mode 100644
> index 000000000000..74bcb8cec5c1
> --- /dev/null
> +++ b/drivers/clk/clk-eyeq5.c
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PLL clock driver for the Mobileye EyeQ5 platform.
> + *
> + * This controller handles 10 read-only PLLs, all derived from the same main
> + * crystal clock. The parent clock is expected to be constant. This driver is
> + * custom to this platform, its registers live in a shared region called OLB.
> + *
> + * We use eq5c_ as prefix, as-in "EyeQ5 Clock", but way shorter.
> + *
> + * Copyright (C) 2023 Mobileye Vision Technologies Ltd.
> + */
> +#include <linux/bits.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>

Please drop this include.

> +#include <linux/mfd/syscon.h>
> +#include <linux/of_device.h>

Should be mod_devicetable.h or nothing at all.

> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "%s: " fmt, __func__

Don't we get this already with dynamic debug?

> +
> +/*
> + * PLL control & status registers, n=0..1
> + * 0x02c..0x078
> + */
> +#define OLB_PCSR_CPU(n)                                (0x02C + (n) * 4) /* CPU */
> +#define OLB_PCSR_VMP(n)                                (0x034 + (n) * 4) /* VMP */
> +#define OLB_PCSR_PMA(n)                                (0x03C + (n) * 4) /* PMA */
> +#define OLB_PCSR_VDI(n)                                (0x044 + (n) * 4) /* VDI */
> +#define OLB_PCSR_DDR0(n)                       (0x04C + (n) * 4) /* DDR0 */
> +#define OLB_PCSR_PCI(n)                                (0x054 + (n) * 4) /* PCI */
> +#define OLB_PCSR_PER(n)                                (0x05C + (n) * 4) /* PER */
> +#define OLB_PCSR_PMAC(n)                       (0x064 + (n) * 4) /* PMAC */
> +#define OLB_PCSR_MPC(n)                                (0x06c + (n) * 4) /* MPC */
> +#define OLB_PCSR_DDR1(n)                       (0x074 + (n) * 4) /* DDR1 */
> +
> +/* In frac mode, it enables fractional noise canceling DAC. Else, no function. */
> +#define OLB_PCSR0_DAC_EN                       BIT(0)
> +/* Fractional or integer mode */
> +#define OLB_PCSR0_DSM_EN                       BIT(1)
> +#define OLB_PCSR0_PLL_EN                       BIT(2)
> +/* All clocks output held at 0 */
> +#define OLB_PCSR0_FOUTPOSTDIV_EN               BIT(3)
> +#define OLB_PCSR0_POST_DIV1                    GENMASK(6, 4)
> +#define OLB_PCSR0_POST_DIV2                    GENMASK(9, 7)
> +#define OLB_PCSR0_REF_DIV                      GENMASK(15, 10)
> +#define OLB_PCSR0_INTIN                                GENMASK(27, 16)
> +#define OLB_PCSR0_BYPASS                       BIT(28)
> +/* Bits 30..29 are reserved */
> +#define OLB_PCSR0_PLL_LOCKED                   BIT(31)
> +
> +#define OLB_PCSR1_RESET                                BIT(0)
> +#define OLB_PCSR1_SSGC_DIV                     GENMASK(4, 1)
> +/* Spread amplitude (% = 0.1 * SPREAD[4:0]) */
> +#define OLB_PCSR1_SPREAD                       GENMASK(9, 5)
> +#define OLB_PCSR1_DIS_SSCG                     BIT(10)
> +/* Down-spread or center-spread */
> +#define OLB_PCSR1_DOWN_SPREAD                  BIT(11)
> +#define OLB_PCSR1_FRAC_IN                      GENMASK(31, 12)
> +
> +static const struct eq5c_pll {
> +       const char *name;
> +       u32 reg;
> +} eq5c_plls[] = {
> +       [EQ5C_PLL_CPU] =  { .name = "pll-cpu",  .reg = OLB_PCSR_CPU(0),  },
> +       [EQ5C_PLL_VMP] =  { .name = "pll-vmp",  .reg = OLB_PCSR_VMP(0),  },
> +       [EQ5C_PLL_PMA] =  { .name = "pll-pma",  .reg = OLB_PCSR_PMA(0),  },
> +       [EQ5C_PLL_VDI] =  { .name = "pll-vdi",  .reg = OLB_PCSR_VDI(0),  },
> +       [EQ5C_PLL_DDR0] = { .name = "pll-ddr0", .reg = OLB_PCSR_DDR0(0), },
> +       [EQ5C_PLL_PCI] =  { .name = "pll-pci",  .reg = OLB_PCSR_PCI(0),  },
> +       [EQ5C_PLL_PER] =  { .name = "pll-per",  .reg = OLB_PCSR_PER(0),  },
> +       [EQ5C_PLL_PMAC] = { .name = "pll-pmac", .reg = OLB_PCSR_PMAC(0), },
> +       [EQ5C_PLL_MPC] =  { .name = "pll-mpc",  .reg = OLB_PCSR_MPC(0),  },
> +       [EQ5C_PLL_DDR1] = { .name = "pll-ddr1", .reg = OLB_PCSR_DDR1(0), },
> +};
> +
> +static int eq5c_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
> +                                   unsigned long *div, unsigned long *acc)
> +{
> +       if (!mult || !div || !acc)
> +               return -EINVAL;

Is this condition ever true? Please remove.

> +
> +       if (r0 & OLB_PCSR0_BYPASS) {
> +               *mult = 1;
> +               *div = 1;
> +               *acc = 0;
> +               return 0;
> +       }
> +
> +       if (!(r0 & OLB_PCSR0_PLL_LOCKED))
> +               return -EINVAL;
> +
> +       *mult = FIELD_GET(OLB_PCSR0_INTIN, r0);
> +       *div = FIELD_GET(OLB_PCSR0_REF_DIV, r0);
> +       if (r0 & OLB_PCSR0_FOUTPOSTDIV_EN)
> +               *div *= FIELD_GET(OLB_PCSR0_POST_DIV1, r0) *
> +                       FIELD_GET(OLB_PCSR0_POST_DIV2, r0);
> +
> +       /* Fractional mode, in 2^20 (0x100000) parts. */
> +       if (r0 & OLB_PCSR0_DSM_EN) {
> +               *div *= 0x100000;
> +               *mult = *mult * 0x100000 + FIELD_GET(OLB_PCSR1_FRAC_IN, r1);
> +       }
> +
> +       if (!*mult || !*div)
> +               return -EINVAL;
> +
> +       /* Spread spectrum. */
> +       if (!(r1 & (OLB_PCSR1_RESET | OLB_PCSR1_DIS_SSCG))) {
> +               /*
> +                * Spread is 1/1000 parts of frequency, accuracy is half of
> +                * that. To get accuracy, convert to ppb (parts per billion).
> +                */
> +               u32 spread = FIELD_GET(OLB_PCSR1_SPREAD, r1);
> +               *acc = spread * 500000;
> +               if (r1 & OLB_PCSR1_DOWN_SPREAD) {
> +                       /*
> +                        * Downspreading: the central frequency is half a
> +                        * spread lower.
> +                        */
> +                       *mult *= 2000 - spread;
> +                       *div *= 2000;
> +               }
> +       } else {
> +               *acc = 0;
> +       }
> +
> +       return 0;
> +}
> +
> +static void eq5c_init(struct device_node *np)
> +{
> +       struct device_node *parent_np = of_get_parent(np);
> +       struct clk_hw_onecell_data *data;
> +       unsigned long parent_clk_rate;
> +       struct clk_hw *parent_clk_hw;
> +       struct clk *parent_clk;
> +       struct regmap *olb;
> +       int i;
> +
> +       data = kzalloc(struct_size(data, hws, ARRAY_SIZE(eq5c_plls)), GFP_KERNEL);
> +       if (!data)
> +               return;
> +
> +       data->num = ARRAY_SIZE(eq5c_plls);
> +
> +       /*
> +        * TODO: currently, if OLB is not available, we log an error and early
> +        * return. We might want to change this behavior and assume all clocks
> +        * are in bypass mode; that is what is being done in the vendor driver.
> +        *
> +        * It is still unclear if there are valid situations where the OLB
> +        * region would be inaccessible.
> +        */
> +       olb = ERR_PTR(-ENODEV);
> +       if (parent_np)
> +               olb = syscon_node_to_regmap(parent_np);
> +       if (IS_ERR(olb))
> +               olb = syscon_regmap_lookup_by_phandle(np, "mobileye,olb");
> +       if (IS_ERR(olb)) {
> +               pr_err("failed getting regmap: %ld\n", PTR_ERR(olb));
> +               return;
> +       }
> +
> +       parent_clk = of_clk_get_by_name(np, "ref");
> +       if (IS_ERR_OR_NULL(parent_clk)) {
> +               pr_err("no parent clock found\n");
> +               return;
> +       }
> +       parent_clk_hw = __clk_get_hw(parent_clk);
> +       parent_clk_rate = clk_get_rate(parent_clk);
> +       clk_put(parent_clk);

No. Don't get the parent clk in a clk provider. Tell the clk framework
which clk is the parent with clk_parent_data.

> +
> +       for (i = 0; i < ARRAY_SIZE(eq5c_plls); i++) {
> +               const struct eq5c_pll *pll = &eq5c_plls[i];
> +               unsigned long mult, div, acc;
> +               u32 r0, r1;
> +               int ret;
> +
> +               regmap_read(olb, pll->reg, &r0);
> +               regmap_read(olb, pll->reg + sizeof(r0), &r1);
> +
> +               ret = eq5c_pll_parse_registers(r0, r1, &mult, &div, &acc);
> +               if (ret) {
> +                       pr_warn("failed parsing state of %s\n", pll->name);
> +                       continue;
> +               }
> +
> +               /* We use fixed_rate and not fixed_factor because the latter
> +                * does not allow reporting accuracy. The alternative is to
> +                * create a custom clk implementation but that adds too many
> +                * lines to the kernel for not much benefit; our parent clock
> +                * rate won't change anyway.

This comment complains about the lack of something. Add what you need
please and get rid of this comment.

> +                */
> +               data->hws[i] = clk_hw_register_fixed_rate_with_accuracy_parent_hw(
> +                               NULL, pll->name, parent_clk_hw, 0,
> +                               parent_clk_rate * mult / div, acc);
> +               if (IS_ERR_OR_NULL(data->hws[i])) {

NULL is not an error.

> +                       pr_err("failed registering %s: %ld\n",
> +                              pll->name, PTR_ERR(data->hws[i]));
> +                       data->hws[i] = ERR_PTR(-ENOENT);
> +               }
> +       }
> +
> +       of_clk_add_hw_provider(np, of_clk_hw_onecell_get, data);
> +}
> +
> +CLK_OF_DECLARE_DRIVER(eq5c, "mobileye,eyeq5-clk", eq5c_init);

Please use a platform driver.

Re: [PATCH 4/5] clk: eyeq5: add OSPI table-based divider clock

[Stephen Boyd]

Quoting Théo Lebrun (2023-12-18 09:14:19)
> The driver supports PLLs on the platform. Add the single divider clock
> of the platform.
> 
> Helpers from include/linux/clk-provider.h could have been used if it was
> not for the use of regmap to access the register.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---

This patch should be squashed with the previous one.

> diff --git a/drivers/clk/clk-eyeq5.c b/drivers/clk/clk-eyeq5.c
> index 74bcb8cec5c1..3382f4d870d7 100644
> --- a/drivers/clk/clk-eyeq5.c
> +++ b/drivers/clk/clk-eyeq5.c
> @@ -77,6 +78,8 @@ static const struct eq5c_pll {
[...]
> +
> +static int eq5c_ospi_div_set_rate(struct clk_hw *hw,
> +                                 unsigned long rate, unsigned long parent_rate)
> +{
> +       struct eq5c_ospi_div *div = clk_hw_to_ospi_priv(hw);
> +       unsigned int val;
> +       int value, ret;
> +
> +       value = divider_get_val(rate, parent_rate, eq5c_ospi_div_table,
> +                               OLB_OSPI_DIV_MASK_WIDTH, 0);
> +       if (value < 0)
> +               return value;
> +
> +       ret = regmap_read(div->olb, OLB_OSPI_REG, &val);
> +       if (ret) {
> +               pr_err("%s: regmap_read failed: %d\n", __func__, ret);
> +               return -ret;

Why negative ret?

> +       }
> +
> +       val &= ~OLB_OSPI_DIV_MASK;
> +       val |= FIELD_PREP(OLB_OSPI_DIV_MASK, value);
> +
> +       ret = regmap_write(div->olb, OLB_OSPI_REG, val);
> +       if (ret) {
> +               pr_err("%s: regmap_write failed: %d\n", __func__, ret);
> +               return -ret;

Why negative ret?

> +       }
> +
> +       return 0;
> +}
> +
> +const struct clk_ops eq5c_ospi_div_ops = {

static?

> +       .recalc_rate = eq5c_ospi_div_recalc_rate,
> +       .round_rate = eq5c_ospi_div_round_rate,
> +       .determine_rate = eq5c_ospi_div_determine_rate,
> +       .set_rate = eq5c_ospi_div_set_rate,
> +};
> +
> +static struct clk_hw *eq5c_init_ospi_div(const struct clk_hw *parent,
> +                                        struct regmap *olb)
> +{
> +       struct eq5c_ospi_div *div;
> +       int ret;
> +
> +       div = kzalloc(sizeof(*div), GFP_KERNEL);
> +       if (!div)
> +               return ERR_PTR(-ENOENT);
> +
> +       div->olb = olb;
> +       div->hw.init = CLK_HW_INIT_HW(EQ5C_OSPI_DIV_CLK_NAME, parent,
> +                                     &eq5c_ospi_div_ops, 0);
> +
> +       ret = clk_hw_register(NULL, &div->hw);
> +       if (ret) {
> +               pr_err("failed registering div_ospi: %d\n", ret);
> +               kfree(div);
> +               return ERR_PTR(-ENOENT);

return ERR_PTR(ret)

> +       }
> +
> +       return &div->hw;
> +}
> +
>  static void eq5c_init(struct device_node *np)
>  {
>         struct device_node *parent_np = of_get_parent(np);

Re: [PATCH 1/5] clk: fixed-rate: fix clk_hw_register_fixed_rate_with_accuracy_parent_hw

[Stephen Boyd]

Quoting Théo Lebrun (2023-12-18 09:14:16)
> Add missing comma and remove extraneous NULL argument. The macro is
> currently used by no one which explains why the typo slipped by.
> 
> Fixes: 2d34f09e79c9 ("clk: fixed-rate: Add support for specifying parents via DT/pointers")
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---

Applied to clk-next

Re: [PATCH 3/5] clk: eyeq5: add controller

[Théo Lebrun]

Hello,

I've seen all your comments, thanks for that. I have a follow up about
one:

On Wed Dec 20, 2023 at 12:09 AM CET, Stephen Boyd wrote:
> Quoting Théo Lebrun (2023-12-18 09:14:18)
> > Add the Mobileye EyeQ5 clock controller driver. See the header comment
> > for more information on how it works.
>
> "See the header" is like saying "Read the code" which is pretty obvious.
> Remove this sentence and tell us why only the PLLs are supported at the
> moment or something like that.
>
> > This driver is specific to this
> > platform; it might grow to add later support of other platforms from
> > Mobileye.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  MAINTAINERS             |   1 +
> >  drivers/clk/Kconfig     |  11 +++
> >  drivers/clk/Makefile    |   1 +
> >  drivers/clk/clk-eyeq5.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 224 insertions(+)
> > 

[...]

> > diff --git a/drivers/clk/clk-eyeq5.c b/drivers/clk/clk-eyeq5.c
> > new file mode 100644
> > index 000000000000..74bcb8cec5c1
> > --- /dev/null
> > +++ b/drivers/clk/clk-eyeq5.c

[...]

> > +       of_clk_add_hw_provider(np, of_clk_hw_onecell_get, data);
> > +}
> > +
> > +CLK_OF_DECLARE_DRIVER(eq5c, "mobileye,eyeq5-clk", eq5c_init);
>
> Please use a platform driver.

I've been trying to do that but I had a stall at boot. I initially
associated it with the UART driver acquiring a clock too early but
instead it is the CPU timer clocksource driver that consumes one of our
clock way earlier than any platform driver initialisation.

The clocksource driver we are talking about is this one for reference:
https://elixir.bootlin.com/linux/v6.6.8/source/drivers/clocksource/mips-gic-timer.c

Its usage of TIMER_OF_DECLARE means it gets probed by timer_probe ->
plat_time_init -> time_init -> start_kernel. This is way before any
initcalls. Our prior use of CLK_OF_DECLARE_DRIVER meant that we got
probed by of_clk_init -> plat_time_init.

I'm guessing we are not the first one in this situation; any advice on
how to deal with it?

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/5] clk: eyeq5: add controller

[Théo Lebrun]

Hello,

On Fri Dec 22, 2023 at 12:25 PM CET, Théo Lebrun wrote:
> Hello,
>
> I've seen all your comments, thanks for that. I have a follow up about
> one:
>
> On Wed Dec 20, 2023 at 12:09 AM CET, Stephen Boyd wrote:
> > Quoting Théo Lebrun (2023-12-18 09:14:18)
> > > Add the Mobileye EyeQ5 clock controller driver. See the header comment
> > > for more information on how it works.
> >
> > "See the header" is like saying "Read the code" which is pretty obvious.
> > Remove this sentence and tell us why only the PLLs are supported at the
> > moment or something like that.
> >
> > > This driver is specific to this
> > > platform; it might grow to add later support of other platforms from
> > > Mobileye.
> > > 
> > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > > ---
> > >  MAINTAINERS             |   1 +
> > >  drivers/clk/Kconfig     |  11 +++
> > >  drivers/clk/Makefile    |   1 +
> > >  drivers/clk/clk-eyeq5.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 224 insertions(+)
> > > 
>
> [...]
>
> > > diff --git a/drivers/clk/clk-eyeq5.c b/drivers/clk/clk-eyeq5.c
> > > new file mode 100644
> > > index 000000000000..74bcb8cec5c1
> > > --- /dev/null
> > > +++ b/drivers/clk/clk-eyeq5.c
>
> [...]
>
> > > +       of_clk_add_hw_provider(np, of_clk_hw_onecell_get, data);
> > > +}
> > > +
> > > +CLK_OF_DECLARE_DRIVER(eq5c, "mobileye,eyeq5-clk", eq5c_init);
> >
> > Please use a platform driver.
>
> I've been trying to do that but I had a stall at boot. I initially
> associated it with the UART driver acquiring a clock too early but
> instead it is the CPU timer clocksource driver that consumes one of our
> clock way earlier than any platform driver initialisation.
>
> The clocksource driver we are talking about is this one for reference:
> https://elixir.bootlin.com/linux/v6.6.8/source/drivers/clocksource/mips-gic-timer.c
>
> Its usage of TIMER_OF_DECLARE means it gets probed by timer_probe ->
> plat_time_init -> time_init -> start_kernel. This is way before any
> initcalls. Our prior use of CLK_OF_DECLARE_DRIVER meant that we got
> probed by of_clk_init -> plat_time_init.
>
> I'm guessing we are not the first one in this situation; any advice on
> how to deal with it?

I went ahead with a V2, feeling it would be more productive to come up
with something and gather comments on concrete stuff. There were many
other things to address anyway.

I've addressed this point by declaring a dummy fixed-clock in the
devicetree that gets fed to the GIC timer. It is pretty much the same
thing as using `clock-frequency` which this specific clocksource uses
if `of_clk_get(node, 0)` fails. With the sent approach we have the
timer appear in the clock tree as a consumer.

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/5] clk: eyeq5: add controller

[Stephen Boyd]

Quoting Théo Lebrun (2023-12-22 03:25:12)
> On Wed Dec 20, 2023 at 12:09 AM CET, Stephen Boyd wrote:
> > Quoting Théo Lebrun (2023-12-18 09:14:18)
> 
> > > +       of_clk_add_hw_provider(np, of_clk_hw_onecell_get, data);
> > > +}
> > > +
> > > +CLK_OF_DECLARE_DRIVER(eq5c, "mobileye,eyeq5-clk", eq5c_init);
> >
> > Please use a platform driver.
> 
> I've been trying to do that but I had a stall at boot. I initially
> associated it with the UART driver acquiring a clock too early but
> instead it is the CPU timer clocksource driver that consumes one of our
> clock way earlier than any platform driver initialisation.
> 
> The clocksource driver we are talking about is this one for reference:
> https://elixir.bootlin.com/linux/v6.6.8/source/drivers/clocksource/mips-gic-timer.c
> 
> Its usage of TIMER_OF_DECLARE means it gets probed by timer_probe ->
> plat_time_init -> time_init -> start_kernel. This is way before any
> initcalls. Our prior use of CLK_OF_DECLARE_DRIVER meant that we got
> probed by of_clk_init -> plat_time_init.
> 
> I'm guessing we are not the first one in this situation; any advice on
> how to deal with it?
> 

You guessed correctly. In that case, use CLK_OF_DECLARE_DRIVER() and
register the clk(s) needed for the timer in that function. Other clks
shall be registered with a platform driver.

Re: [PATCH 3/5] clk: eyeq5: add controller

[Stephen Boyd]

Quoting Théo Lebrun (2023-12-27 08:30:20)
> 
> I went ahead with a V2, feeling it would be more productive to come up
> with something and gather comments on concrete stuff. There were many
> other things to address anyway.
> 
> I've addressed this point by declaring a dummy fixed-clock in the
> devicetree that gets fed to the GIC timer. It is pretty much the same
> thing as using `clock-frequency` which this specific clocksource uses
> if `of_clk_get(node, 0)` fails. With the sent approach we have the
> timer appear in the clock tree as a consumer.
> 

Ok, please send another round then. I was away from my computer for a
week or two.

Re: [PATCH 3/5] clk: eyeq5: add controller

[Stephen Boyd]

Quoting Théo Lebrun (2023-12-27 08:30:20)
> 
> I went ahead with a V2, feeling it would be more productive to come up
> with something and gather comments on concrete stuff. There were many
> other things to address anyway.
> 
> I've addressed this point by declaring a dummy fixed-clock in the
> devicetree that gets fed to the GIC timer. It is pretty much the same
> thing as using `clock-frequency` which this specific clocksource uses
> if `of_clk_get(node, 0)` fails. With the sent approach we have the
> timer appear in the clock tree as a consumer.

If the frequency is fixed then this seems fine to do always.

Re: [PATCH 3/5] clk: eyeq5: add controller

[Théo Lebrun]

Hello,

On Wed Jan 3, 2024 at 12:43 AM CET, Stephen Boyd wrote:
> Quoting Théo Lebrun (2023-12-27 08:30:20)
> >
> > I went ahead with a V2, feeling it would be more productive to come up
> > with something and gather comments on concrete stuff. There were many
> > other things to address anyway.
> >
> > I've addressed this point by declaring a dummy fixed-clock in the
> > devicetree that gets fed to the GIC timer. It is pretty much the same
> > thing as using `clock-frequency` which this specific clocksource uses
> > if `of_clk_get(node, 0)` fails. With the sent approach we have the
> > timer appear in the clock tree as a consumer.
> >
>
> Ok, please send another round then. I was away from my computer for a
> week or two.

No worries, hope you had a nice time.

On Wed Jan 3, 2024 at 12:47 AM CET, Stephen Boyd wrote:
> If the frequency is fixed then this seems fine to do always.

It is the case. Can you confirm that the taken approach is fine for you?
One issue I see with my V2 is that I still expose the timer clk from
the clk driver, even though it is not consumed by anyone and it is
exposed as a fixed-rate from a devicetree node. That makes a duplicate.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com