[PATCH v5 0/4] Add Mobileye EyeQ clock support

[PATCH v5 0/4] Add Mobileye EyeQ clock support

[Théo Lebrun]

This series adds a platform driver dealing with read-only PLLs derived
from the main crystal, and some divider clocks based on those PLLs. It
also acts at the one instantiating reset and pinctrl auxiliary devices.

One special feature is that some clocks are required before platform
bus infrastructure is available; we therefore register some clocks at
of_clk_init() stage.

We support EyeQ5, EyeQ6L and EyeQ6H SoCs. The last one is special in
that there are seven instances of this system-controller. All of those
handle clocks.

The clock driver instantiates reset and pinctrl auxiliary devices
present in the same system controller. Those drivers are upstream
already, as well as the devicetree patches to switch from static clocks
to using this driver:
 - 487b1b32e317 ("reset: eyeq: add platform driver") since v6.12-rc1
 - 41795aa1f56a ("pinctrl: eyeq5: add platform driver") since v6.12-rc1
 - bde4b22dc526 ("dt-bindings: soc: mobileye: add EyeQ OLB system
   controller") since v6.11-rc1

I had a pending question [0], asking for confirmation that the static
linked list to inherit cells from of_clk_init() stage to platform
device probe is indeed the right solution. As -rc1 got released I sent
the new revision anyway.

This new revision only touches the clk-eyeq driver. We officially
declare that we do not support unbinding (using .suppress_bind_attrs)
and remove all devres calls.

Have a nice day,
Thanks,
Théo

[0]: https://lore.kernel.org/lkml/D4ELMFAUQYZ7.3LXGQZJSX68UF@bootlin.com/

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Changes in v5:
- Set `.suppress_bind_attrs = true`.
- Replace devm_platform_ioremap_resource()
  by platform_get_resource() + ioremap().
- Use non-devres version of devm_kzalloc().
- Use non-devres version of devm_of_clk_add_hw_provider().
- Remove the eqc_auxdev_unregister() devres action.
- Link to v4: https://lore.kernel.org/r/20241004-mbly-clk-v4-0-c72c2e348e1f@bootlin.com

Changes in v4:
- clk-divider:
  - Switch flags function arguments from u16 to unsigned long.
- clk-eyeq Kconfig:
  - Remove OF dependency that is not required.
- clk-eyeq driver:
  - eqc_pll_parse_registers():
    - Make clk accuracy computation more explicit using a comment.
    - Early return when not spread spectrum, to deindent code.
  - Rename eqc_init() to eqc_early_init().
  - Remove the early match table. Register a different function in each
    CLK_OF_DECLARE_DRIVER(), which calls eqc_early_init() with an
    additional match data argument.
  - Add __initconst to early match data structs.
  - Remove spinlock on static linked list.
- Link to v3: https://lore.kernel.org/r/20240708-mbly-clk-v3-0-f3fa1ee28fed@bootlin.com

Changes in v3:
- Kconfig: add "depends on 64BIT" because we use readq(). This removes
  the ability to COMPILE_TEST the driver on 32bit, which is fine as
  this is a SoC clk platform driver used on 64bit SoCs.
- driver: avoid `of_match_node(...)->data` because, if !CONFIG_OF,
  of_match_node(...) is resolved by the preprocessor to NULL.
  There is still a warning "eqc_early_match_table declared but unused"
  if !CONFIG_OF. We fix the <linux/of.h> header in a separate patch:
  https://lore.kernel.org/lkml/20240708-of-match-node-v1-1-90aaa7c2d21d@bootlin.com/
- Link to v2: see [1]

Changes in v2:
- bindings: take Acked-by: Krzysztof Kozlowski.
- driver: eqc_auxdev_create(): cast the `void __iomem *base` variable to
  (void __force *) before putting it in platform_data, to avoid sparse
  warning.
- Link to v1: see [1]

Changes since OLB v3 [0]:
 - MAINTAINERS: Move changes into a separate commit to avoid merge
   conflicts. This commit is in the MIPS series [3].
 - dt-bindings: split include/dt-bindings/ changes into its own commit.
   It is part of this clk series.
 - dt-bindings: Take Reviewed-by: Rob Herring. The include/dt-bindings/
   new commit has NOT inherited from it, just to make sure.

---
Théo Lebrun (4):
      Revert "dt-bindings: clock: mobileye,eyeq5-clk: add bindings"
      dt-bindings: clock: add Mobileye EyeQ6L/EyeQ6H clock indexes
      clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag
      clk: eyeq: add driver

 .../bindings/clock/mobileye,eyeq5-clk.yaml         |  51 --
 drivers/clk/Kconfig                                |  12 +
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-divider.c                          |  16 +-
 drivers/clk/clk-eyeq.c                             | 779 +++++++++++++++++++++
 include/dt-bindings/clock/mobileye,eyeq5-clk.h     |  21 +
 include/linux/clk-provider.h                       |  15 +-
 7 files changed, 835 insertions(+), 60 deletions(-)
---
base-commit: 31d10adec91d1b3e51d7eaea7fdf294f1ebe83df
change-id: 20240628-mbly-clk-4c6ebc716347

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

[PATCH v5 1/4] Revert "dt-bindings: clock: mobileye,eyeq5-clk: add bindings"

[Théo Lebrun]

Switch from one sub-node per functionality in the system-controller to a
single node representing the entire OLB instance. This is the
recommended approach for controllers handling many different
functionalities; it is a single controller and should be represented by
a single devicetree node.

The clock bindings is removed and all properties will be described by:
soc/mobileye/mobileye,eyeq5-olb.yaml

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../bindings/clock/mobileye,eyeq5-clk.yaml         | 51 ----------------------
 1 file changed, 51 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
deleted file mode 100644
index 2d4f2cde1e589100f19d84db09050a591c6b9644..0000000000000000000000000000000000000000
--- a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
+++ /dev/null
@@ -1,51 +0,0 @@
-# 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.
-  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:
-  compatible:
-    const: mobileye,eyeq5-clk
-
-  reg:
-    maxItems: 2
-
-  reg-names:
-    items:
-      - const: plls
-      - const: ospi
-
-  "#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
-
-required:
-  - compatible
-  - reg
-  - reg-names
-  - "#clock-cells"
-  - clocks
-  - clock-names
-
-additionalProperties: false

-- 
2.46.2

[PATCH v5 2/4] dt-bindings: clock: add Mobileye EyeQ6L/EyeQ6H clock indexes

[Théo Lebrun]

Add #defines for Mobileye EyeQ6L and EyeQ6H SoC clocks.

Constant prefixes are:
 - EQ6LC_PLL_: EyeQ6L clock PLLs
 - EQ6HC_SOUTH_PLL_: EyeQ6H south OLB PLLs
 - EQ6HC_SOUTH_DIV_: EyeQ6H south OLB divider clocks
 - EQ6HC_ACC_PLL_: EyeQ6H accelerator OLB PLLs

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 include/dt-bindings/clock/mobileye,eyeq5-clk.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/dt-bindings/clock/mobileye,eyeq5-clk.h b/include/dt-bindings/clock/mobileye,eyeq5-clk.h
index 26d8930335e4b113a74f47575957e39163f02766..b433c1772c28fae818b3a6ba428d1f89000f9206 100644
--- a/include/dt-bindings/clock/mobileye,eyeq5-clk.h
+++ b/include/dt-bindings/clock/mobileye,eyeq5-clk.h
@@ -19,4 +19,25 @@
 
 #define EQ5C_DIV_OSPI	10
 
+#define EQ6LC_PLL_DDR		0
+#define EQ6LC_PLL_CPU		1
+#define EQ6LC_PLL_PER		2
+#define EQ6LC_PLL_VDI		3
+
+#define EQ6HC_SOUTH_PLL_VDI		0
+#define EQ6HC_SOUTH_PLL_PCIE		1
+#define EQ6HC_SOUTH_PLL_PER		2
+#define EQ6HC_SOUTH_PLL_ISP		3
+
+#define EQ6HC_SOUTH_DIV_EMMC		4
+#define EQ6HC_SOUTH_DIV_OSPI_REF	5
+#define EQ6HC_SOUTH_DIV_OSPI_SYS	6
+#define EQ6HC_SOUTH_DIV_TSU		7
+
+#define EQ6HC_ACC_PLL_XNN		0
+#define EQ6HC_ACC_PLL_VMP		1
+#define EQ6HC_ACC_PLL_PMA		2
+#define EQ6HC_ACC_PLL_MPC		3
+#define EQ6HC_ACC_PLL_NOC		4
+
 #endif

-- 
2.46.2

[PATCH v5 3/4] clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag

[Théo Lebrun]

Add CLK_DIVIDER_EVEN_INTEGERS flag to support divisor of 2, 4, 6, etc.
The same divisor can be done using a table, which would be big and
wasteful for a clock dividor of width 8 (256 entries).

Require increasing flags size from u8 to u16 because
CLK_DIVIDER_EVEN_INTEGERS is the eighth flag. u16 is used inside struct
clk_divider; `unsigned long` is used for function arguments.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/clk/clk-divider.c    | 16 ++++++++++++----
 include/linux/clk-provider.h | 15 ++++++++++-----
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index a2c2b5203b0a95cf1cf651b1fb4e2c24d230d831..c1f426b8a5043cb5a1de08e1da385928ec54a2ed 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -72,6 +72,8 @@ static unsigned int _get_maxdiv(const struct clk_div_table *table, u8 width,
 		return clk_div_mask(width);
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		return 1 << clk_div_mask(width);
+	if (flags & CLK_DIVIDER_EVEN_INTEGERS)
+		return 2 * (clk_div_mask(width) + 1);
 	if (table)
 		return _get_table_maxdiv(table, width);
 	return clk_div_mask(width) + 1;
@@ -97,6 +99,8 @@ static unsigned int _get_div(const struct clk_div_table *table,
 		return 1 << val;
 	if (flags & CLK_DIVIDER_MAX_AT_ZERO)
 		return val ? val : clk_div_mask(width) + 1;
+	if (flags & CLK_DIVIDER_EVEN_INTEGERS)
+		return 2 * (val + 1);
 	if (table)
 		return _get_table_div(table, val);
 	return val + 1;
@@ -122,6 +126,8 @@ static unsigned int _get_val(const struct clk_div_table *table,
 		return __ffs(div);
 	if (flags & CLK_DIVIDER_MAX_AT_ZERO)
 		return (div == clk_div_mask(width) + 1) ? 0 : div;
+	if (flags & CLK_DIVIDER_EVEN_INTEGERS)
+		return (div >> 1) - 1;
 	if (table)
 		return  _get_table_val(table, div);
 	return div - 1;
@@ -538,7 +544,8 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width,
+		unsigned long clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock)
 {
 	struct clk_divider *div;
@@ -610,8 +617,8 @@ EXPORT_SYMBOL_GPL(__clk_hw_register_divider);
 struct clk *clk_register_divider_table(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
-		u8 clk_divider_flags, const struct clk_div_table *table,
-		spinlock_t *lock)
+		unsigned long clk_divider_flags,
+		const struct clk_div_table *table, spinlock_t *lock)
 {
 	struct clk_hw *hw;
 
@@ -664,7 +671,8 @@ struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width,
+		unsigned long clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock)
 {
 	struct clk_hw **ptr, *hw;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 7e43caabb54b901d68484b86c8349febfe12ba0f..dbe793964c24ca3ab3a7facd090dfb6ae9df7631 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -689,13 +689,15 @@ struct clk_div_table {
  * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
  *	for the divider register.  Setting this flag makes the register accesses
  *	big endian.
+ * CLK_DIVIDER_EVEN_INTEGERS - clock divisor is 2, 4, 6, 8, 10, etc.
+ *	Formula is 2 * (value read from hardware + 1).
  */
 struct clk_divider {
 	struct clk_hw	hw;
 	void __iomem	*reg;
 	u8		shift;
 	u8		width;
-	u8		flags;
+	u16		flags;
 	const struct clk_div_table	*table;
 	spinlock_t	*lock;
 };
@@ -711,6 +713,7 @@ struct clk_divider {
 #define CLK_DIVIDER_READ_ONLY		BIT(5)
 #define CLK_DIVIDER_MAX_AT_ZERO		BIT(6)
 #define CLK_DIVIDER_BIG_ENDIAN		BIT(7)
+#define CLK_DIVIDER_EVEN_INTEGERS	BIT(8)
 
 extern const struct clk_ops clk_divider_ops;
 extern const struct clk_ops clk_divider_ro_ops;
@@ -740,19 +743,21 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width,
+		unsigned long clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock);
 struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width,
+		unsigned long clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock);
 struct clk *clk_register_divider_table(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
-		u8 clk_divider_flags, const struct clk_div_table *table,
-		spinlock_t *lock);
+		unsigned long clk_divider_flags,
+		const struct clk_div_table *table, spinlock_t *lock);
 /**
  * clk_register_divider - register a divider clock with the clock framework
  * @dev: device registering this clock

-- 
2.46.2

[PATCH v5 4/4] clk: eyeq: add driver

[Théo Lebrun]

Add Mobileye EyeQ5, EyeQ6L and EyeQ6H clock controller driver. It is
both a platform driver and a hook onto of_clk_init() used for clocks
required early (GIC timer, UARTs).

For some compatible, it is both at the same time. eqc_early_init()
initialises early PLLs and stores clock array in a static linked list.
It marks other clocks as deferred. eqc_probe() retrieves the clock
array and adds all remaining clocks.

It exposes read-only PLLs derived from the main crystal on board.
It also exposes another type of clocks: divider clocks.
They always have even divisors and have one PLL as parent.

This driver also bears the responsability for optional reset and pinctrl
auxiliary devices. The match data attached to the devicetree node
compatible indicate if such devices should be created. They all get
passed a pointer to the start of the OLB region.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/clk/Kconfig    |  12 +
 drivers/clk/Makefile   |   1 +
 drivers/clk/clk-eyeq.c | 779 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 792 insertions(+)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 299bc678ed1b9fcd9110bb8c5937a1bd1ea60e23..ae7caa28985481ce7280421de7d3f2340f8f9ab3 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -226,6 +226,18 @@ config COMMON_CLK_EP93XX
 	help
 	  This driver supports the SoC clocks on the Cirrus Logic ep93xx.
 
+config COMMON_CLK_EYEQ
+	bool "Clock driver for the Mobileye EyeQ platform"
+	depends on 64BIT # for readq()
+	depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST
+	select AUXILIARY_BUS
+	default MACH_EYEQ5 || MACH_EYEQ6H
+	help
+	  This driver provides clocks found on Mobileye EyeQ5, EyeQ6L and Eye6H
+	  SoCs. Controllers live in shared register regions called OLB. Driver
+	  provides read-only PLLs, derived from the main crystal clock (which
+	  must be constant). It also exposes some divider clocks.
+
 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 fb8878a5d7d93da6bec487460cdf63f1f764a431..9818a9445254a129e33279ae37a01f87bb9e1196 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_COMMON_CLK_CS2000_CP)	+= clk-cs2000-cp.o
 obj-$(CONFIG_COMMON_CLK_EP93XX)		+= clk-ep93xx.o
 obj-$(CONFIG_ARCH_SPARX5)		+= clk-sparx5.o
 obj-$(CONFIG_COMMON_CLK_EN7523)		+= clk-en7523.o
+obj-$(CONFIG_COMMON_CLK_EYEQ)		+= clk-eyeq.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-eyeq.c b/drivers/clk/clk-eyeq.c
new file mode 100644
index 0000000000000000000000000000000000000000..762885333c0336e9ff1162a3677f5fb815fd461a
--- /dev/null
+++ b/drivers/clk/clk-eyeq.c
@@ -0,0 +1,779 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PLL clock driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
+ *
+ * This controller handles read-only PLLs, all derived from the same main
+ * crystal clock. It also exposes divider clocks, those are children to PLLs.
+ * Parent clock is expected to be constant. This driver's registers live in
+ * a shared region called OLB. Some PLLs are initialised early by of_clk_init().
+ *
+ * We use eqc_ as prefix, as-in "EyeQ Clock", but way shorter.
+ *
+ * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
+ */
+
+/*
+ * Set pr_fmt() for printing from eqc_early_init().
+ * It is called at of_clk_init() stage (read: really early).
+ */
+#define pr_fmt(fmt) "clk-eyeq: " fmt
+
+#include <linux/array_size.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/overflow.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
+
+#define EQC_MAX_DIV_COUNT		4
+
+/* In frac mode, it enables fractional noise canceling DAC. Else, no function. */
+#define PCSR0_DAC_EN			BIT(0)
+/* Fractional or integer mode */
+#define PCSR0_DSM_EN			BIT(1)
+#define PCSR0_PLL_EN			BIT(2)
+/* All clocks output held at 0 */
+#define PCSR0_FOUTPOSTDIV_EN		BIT(3)
+#define PCSR0_POST_DIV1			GENMASK(6, 4)
+#define PCSR0_POST_DIV2			GENMASK(9, 7)
+#define PCSR0_REF_DIV			GENMASK(15, 10)
+#define PCSR0_INTIN			GENMASK(27, 16)
+#define PCSR0_BYPASS			BIT(28)
+/* Bits 30..29 are reserved */
+#define PCSR0_PLL_LOCKED		BIT(31)
+
+#define PCSR1_RESET			BIT(0)
+#define PCSR1_SSGC_DIV			GENMASK(4, 1)
+/* Spread amplitude (% = 0.1 * SPREAD[4:0]) */
+#define PCSR1_SPREAD			GENMASK(9, 5)
+#define PCSR1_DIS_SSCG			BIT(10)
+/* Down-spread or center-spread */
+#define PCSR1_DOWN_SPREAD		BIT(11)
+#define PCSR1_FRAC_IN			GENMASK(31, 12)
+
+/*
+ * Driver might register clock provider from eqc_early_init() if PLLs are
+ * required early (before platform bus is ready). Store struct eqc_priv inside
+ * linked list to pass clock provider from eqc_early_init() to eqc_probe() and
+ * register remaining clocks from platform device probe.
+ *
+ * Clock provider is NOT created by eqc_early_init() if no early clock is
+ * required. Store as linked list because EyeQ6H has multiple clock controller
+ * instances. Matching is done based on devicetree node pointer.
+ */
+static LIST_HEAD(eqc_list);
+
+struct eqc_pll {
+	unsigned int	index;
+	const char	*name;
+	unsigned int	reg64;
+};
+
+/*
+ * Divider clock. Divider is 2*(v+1), with v the register value.
+ * Min divider is 2, max is 2*(2^width).
+ */
+struct eqc_div {
+	unsigned int	index;
+	const char	*name;
+	unsigned int	parent;
+	unsigned int	reg;
+	u8		shift;
+	u8		width;
+};
+
+struct eqc_match_data {
+	unsigned int		pll_count;
+	const struct eqc_pll	*plls;
+
+	unsigned int		div_count;
+	const struct eqc_div	*divs;
+
+	const char		*reset_auxdev_name;
+	const char		*pinctrl_auxdev_name;
+};
+
+struct eqc_early_match_data {
+	unsigned int		early_pll_count;
+	const struct eqc_pll	*early_plls;
+	/* Information required to init properly clk HW cells. */
+	unsigned int		nb_late_clks;
+};
+
+struct eqc_priv {
+	struct clk_hw_onecell_data	*cells;
+	const struct eqc_early_match_data *early_data;
+	const struct eqc_match_data	*data;
+	void __iomem			*base;
+	struct device_node		*np;
+	struct list_head		list;
+};
+
+/*
+ * Both factors (mult and div) must fit in 32 bits. When an operation overflows,
+ * this function throws away low bits so that factors still fit in 32 bits.
+ *
+ * Precision loss depends on amplitude of mult and div. Worst theorical
+ * loss is: (UINT_MAX+1) / UINT_MAX - 1 = 2.3e-10.
+ * This is 1Hz every 4.3GHz.
+ */
+static void eqc_pll_downshift_factors(unsigned long *mult, unsigned long *div)
+{
+	unsigned long biggest;
+	unsigned int shift;
+
+	/* This function can be removed if mult/div switch to unsigned long. */
+	static_assert(sizeof_field(struct clk_fixed_factor, mult) == sizeof(unsigned int));
+	static_assert(sizeof_field(struct clk_fixed_factor, div) == sizeof(unsigned int));
+
+	/* No overflow, nothing to be done. */
+	if (*mult <= UINT_MAX && *div <= UINT_MAX)
+		return;
+
+	/*
+	 * Compute the shift required to bring the biggest factor into unsigned
+	 * int range. That is, shift its highest set bit to the unsigned int
+	 * most significant bit.
+	 */
+	biggest = max(*mult, *div);
+	shift = __fls(biggest) - (BITS_PER_BYTE * sizeof(unsigned int)) + 1;
+
+	*mult >>= shift;
+	*div >>= shift;
+}
+
+static int eqc_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
+				   unsigned long *div, unsigned long *acc)
+{
+	u32 spread;
+
+	if (r0 & PCSR0_BYPASS) {
+		*mult = 1;
+		*div = 1;
+		*acc = 0;
+		return 0;
+	}
+
+	if (!(r0 & PCSR0_PLL_LOCKED))
+		return -EINVAL;
+
+	*mult = FIELD_GET(PCSR0_INTIN, r0);
+	*div = FIELD_GET(PCSR0_REF_DIV, r0);
+	if (r0 & PCSR0_FOUTPOSTDIV_EN)
+		*div *= FIELD_GET(PCSR0_POST_DIV1, r0) * FIELD_GET(PCSR0_POST_DIV2, r0);
+
+	/* Fractional mode, in 2^20 (0x100000) parts. */
+	if (r0 & PCSR0_DSM_EN) {
+		*div *= (1ULL << 20);
+		*mult = *mult * (1ULL << 20) + FIELD_GET(PCSR1_FRAC_IN, r1);
+	}
+
+	if (!*mult || !*div)
+		return -EINVAL;
+
+	if (r1 & (PCSR1_RESET | PCSR1_DIS_SSCG)) {
+		*acc = 0;
+		return 0;
+	}
+
+	/*
+	 * Spread spectrum.
+	 *
+	 * Spread is 1/1000 parts of frequency, accuracy is half of
+	 * that. To get accuracy, convert to ppb (parts per billion).
+	 *
+	 * acc = spread * 1e6 / 2
+	 *   with acc in parts per billion and,
+	 *        spread in parts per thousand.
+	 */
+	spread = FIELD_GET(PCSR1_SPREAD, r1);
+	*acc = spread * 500000;
+
+	if (r1 & PCSR1_DOWN_SPREAD) {
+		/*
+		 * Downspreading: the central frequency is half a
+		 * spread lower.
+		 */
+		*mult *= 2000 - spread;
+		*div *= 2000;
+
+		/*
+		 * Previous operation might overflow 32 bits. If it
+		 * does, throw away the least amount of low bits.
+		 */
+		eqc_pll_downshift_factors(mult, div);
+	}
+
+	return 0;
+}
+
+static unsigned int eqc_compute_clock_count(const struct eqc_early_match_data *early_data,
+					    const struct eqc_match_data *data)
+{
+	unsigned int i, nb_clks = 0, sum = 0;
+
+	if (early_data) {
+		sum += early_data->early_pll_count;
+
+		for (i = 0; i < early_data->early_pll_count; i++)
+			if (early_data->early_plls[i].index >= nb_clks)
+				nb_clks = early_data->early_plls[i].index + 1;
+	}
+
+	if (data) {
+		sum += data->pll_count + data->div_count;
+
+		for (i = 0; i < data->pll_count; i++)
+			if (data->plls[i].index >= nb_clks)
+				nb_clks = data->plls[i].index + 1;
+
+		for (i = 0; i < data->div_count; i++)
+			if (data->divs[i].index >= nb_clks)
+				nb_clks = data->divs[i].index + 1;
+	}
+
+	/* We expect the biggest clock index to be 1 below the clock count. */
+	WARN_ON(nb_clks != sum);
+
+	return nb_clks;
+}
+
+static void eqc_probe_init_plls(struct device *dev, struct eqc_priv *priv)
+{
+	const struct eqc_match_data *data = priv->data;
+	unsigned long mult, div, acc;
+	const struct eqc_pll *pll;
+	struct clk_hw *hw;
+	unsigned int i;
+	u32 r0, r1;
+	u64 val;
+	int ret;
+
+	for (i = 0; i < data->pll_count; i++) {
+		pll = &data->plls[i];
+
+		val = readq(priv->base + pll->reg64);
+		r0 = val;
+		r1 = val >> 32;
+
+		ret = eqc_pll_parse_registers(r0, r1, &mult, &div, &acc);
+		if (ret) {
+			dev_warn(dev, "failed parsing state of %s\n", pll->name);
+			priv->cells->hws[pll->index] = ERR_PTR(ret);
+			continue;
+		}
+
+		hw = clk_hw_register_fixed_factor_with_accuracy_fwname(dev,
+				dev->of_node, pll->name, "ref", 0, mult, div, acc);
+		priv->cells->hws[pll->index] = hw;
+		if (IS_ERR(hw))
+			dev_warn(dev, "failed registering %s: %pe\n", pll->name, hw);
+	}
+}
+
+static void eqc_probe_init_divs(struct platform_device *pdev, struct device *dev,
+				struct eqc_priv *priv)
+{
+	const struct eqc_match_data *data = priv->data;
+	const struct eqc_div *div;
+	struct clk_hw *parent;
+	void __iomem *reg;
+	struct clk_hw *hw;
+	unsigned int i;
+
+	for (i = 0; i < data->div_count; i++) {
+		div = &data->divs[i];
+		reg = priv->base + div->reg;
+		parent = priv->cells->hws[div->parent];
+
+		hw = clk_hw_register_divider_table_parent_hw(dev, div->name,
+				parent, 0, reg, div->shift, div->width,
+				CLK_DIVIDER_EVEN_INTEGERS, NULL, NULL);
+		priv->cells->hws[div->index] = hw;
+		if (IS_ERR(hw))
+			dev_warn(dev, "failed registering %s: %pe\n",
+				 div->name, hw);
+	}
+}
+
+static void eqc_auxdev_release(struct device *dev)
+{
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+	kfree(adev);
+}
+
+static int eqc_auxdev_create(struct device *dev, void __iomem *base,
+			     const char *name, u32 id)
+{
+	struct auxiliary_device *adev;
+	int ret;
+
+	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+	if (!adev)
+		return -ENOMEM;
+
+	adev->name = name;
+	adev->dev.parent = dev;
+	adev->dev.platform_data = (void __force *)base;
+	adev->dev.release = eqc_auxdev_release;
+	adev->id = id;
+
+	ret = auxiliary_device_init(adev);
+	if (ret)
+		return ret;
+
+	ret = auxiliary_device_add(adev);
+	if (ret)
+		auxiliary_device_uninit(adev);
+
+	return ret;
+}
+
+static int eqc_probe(struct platform_device *pdev)
+{
+	const struct eqc_match_data *data;
+	struct device *dev = &pdev->dev;
+	struct eqc_priv *priv = NULL;
+	struct eqc_priv *entry;
+	struct resource *res;
+	unsigned int nb_clks;
+	void __iomem *base;
+	int ret;
+
+	data = device_get_match_data(dev);
+	if (!data)
+		return 0; /* No clocks nor auxdevs, we are done. */
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	base = ioremap(res->start, resource_size(res));
+	if (!base)
+		return -ENOMEM;
+
+	/* Init optional reset auxiliary device. */
+	if (data->reset_auxdev_name) {
+		ret = eqc_auxdev_create(dev, base, data->reset_auxdev_name, 0);
+		if (ret)
+			dev_warn(dev, "failed creating auxiliary device %s.%s: %d\n",
+				 KBUILD_MODNAME, data->reset_auxdev_name, ret);
+	}
+
+	/* Init optional pinctrl auxiliary device. */
+	if (data->pinctrl_auxdev_name) {
+		ret = eqc_auxdev_create(dev, base, data->pinctrl_auxdev_name, 0);
+		if (ret)
+			dev_warn(dev, "failed creating auxiliary device %s.%s: %d\n",
+				 KBUILD_MODNAME, data->pinctrl_auxdev_name, ret);
+	}
+
+	if (data->pll_count + data->div_count == 0)
+		return 0; /* Zero clocks, we are done. */
+
+	/* Try retrieving early init private data. */
+	list_for_each_entry(entry, &eqc_list, list) {
+		if (entry->np == dev->of_node) {
+			priv = entry;
+			break;
+		}
+	}
+
+	if (!priv) {
+		/* Device did not get init early. Do it now. */
+
+		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+		if (!priv)
+			return -ENOMEM;
+
+		priv->np = dev->of_node;
+
+		nb_clks = eqc_compute_clock_count(NULL, data);
+		priv->cells = kzalloc(struct_size(priv->cells, hws, nb_clks),
+				      GFP_KERNEL);
+		if (!priv->cells)
+			return -ENOMEM;
+
+		priv->cells->num = nb_clks;
+	} else {
+		/*
+		 * Device got init early. Check clock count.
+		 *
+		 * eqc_early_init() should already know the exact clk count
+		 * using nb_late_clks field. We ensure computation was right
+		 * and fix clk cells if not.
+		 */
+		nb_clks = eqc_compute_clock_count(priv->early_data, data);
+		if (WARN_ON(nb_clks != priv->cells->num))
+			priv->cells->num = nb_clks;
+	}
+
+	priv->base = base;
+	priv->data = data;
+
+	eqc_probe_init_plls(dev, priv);
+
+	eqc_probe_init_divs(pdev, dev, priv);
+
+	/* Clock provider has not been registered by eqc_early_init(). Do it now. */
+	if (!priv->early_data) {
+		/* When providing a single clock, require no cell. */
+		if (priv->cells->num == 1)
+			ret = of_clk_add_hw_provider(priv->np, of_clk_hw_simple_get,
+						     priv->cells->hws[0]);
+		else
+			ret = of_clk_add_hw_provider(priv->np, of_clk_hw_onecell_get,
+						     priv->cells);
+
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct eqc_pll eqc_eyeq5_plls[] = {
+	{ .index = EQ5C_PLL_VMP,  .name = "pll-vmp",  .reg64 = 0x034 },
+	{ .index = EQ5C_PLL_PMA,  .name = "pll-pma",  .reg64 = 0x03C },
+	{ .index = EQ5C_PLL_VDI,  .name = "pll-vdi",  .reg64 = 0x044 },
+	{ .index = EQ5C_PLL_DDR0, .name = "pll-ddr0", .reg64 = 0x04C },
+	{ .index = EQ5C_PLL_PCI,  .name = "pll-pci",  .reg64 = 0x054 },
+	{ .index = EQ5C_PLL_PMAC, .name = "pll-pmac", .reg64 = 0x064 },
+	{ .index = EQ5C_PLL_MPC,  .name = "pll-mpc",  .reg64 = 0x06C },
+	{ .index = EQ5C_PLL_DDR1, .name = "pll-ddr1", .reg64 = 0x074 },
+};
+
+static const struct eqc_div eqc_eyeq5_divs[] = {
+	{
+		.index = EQ5C_DIV_OSPI,
+		.name = "div-ospi",
+		.parent = EQ5C_PLL_PER,
+		.reg = 0x11C,
+		.shift = 0,
+		.width = 4,
+	},
+};
+
+static const struct eqc_match_data eqc_eyeq5_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq5_plls),
+	.plls		= eqc_eyeq5_plls,
+
+	.div_count	= ARRAY_SIZE(eqc_eyeq5_divs),
+	.divs		= eqc_eyeq5_divs,
+
+	.reset_auxdev_name = "reset",
+	.pinctrl_auxdev_name = "pinctrl",
+};
+
+static const struct eqc_pll eqc_eyeq6l_plls[] = {
+	{ .index = EQ6LC_PLL_DDR, .name = "pll-ddr", .reg64 = 0x02C },
+	{ .index = EQ6LC_PLL_CPU, .name = "pll-cpu", .reg64 = 0x034 }, /* also acc */
+	{ .index = EQ6LC_PLL_PER, .name = "pll-per", .reg64 = 0x03C },
+	{ .index = EQ6LC_PLL_VDI, .name = "pll-vdi", .reg64 = 0x044 },
+};
+
+static const struct eqc_match_data eqc_eyeq6l_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6l_plls),
+	.plls		= eqc_eyeq6l_plls,
+
+	.reset_auxdev_name = "reset",
+};
+
+static const struct eqc_match_data eqc_eyeq6h_west_match_data = {
+	.reset_auxdev_name = "reset_west",
+};
+
+static const struct eqc_pll eqc_eyeq6h_east_plls[] = {
+	{ .index = 0, .name = "pll-east", .reg64 = 0x074 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_east_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_east_plls),
+	.plls		= eqc_eyeq6h_east_plls,
+
+	.reset_auxdev_name = "reset_east",
+};
+
+static const struct eqc_pll eqc_eyeq6h_south_plls[] = {
+	{ .index = EQ6HC_SOUTH_PLL_VDI,  .name = "pll-vdi",  .reg64 = 0x000 },
+	{ .index = EQ6HC_SOUTH_PLL_PCIE, .name = "pll-pcie", .reg64 = 0x008 },
+	{ .index = EQ6HC_SOUTH_PLL_PER,  .name = "pll-per",  .reg64 = 0x010 },
+	{ .index = EQ6HC_SOUTH_PLL_ISP,  .name = "pll-isp",  .reg64 = 0x018 },
+};
+
+static const struct eqc_div eqc_eyeq6h_south_divs[] = {
+	{
+		.index = EQ6HC_SOUTH_DIV_EMMC,
+		.name = "div-emmc",
+		.parent = EQ6HC_SOUTH_PLL_PER,
+		.reg = 0x070,
+		.shift = 4,
+		.width = 4,
+	},
+	{
+		.index = EQ6HC_SOUTH_DIV_OSPI_REF,
+		.name = "div-ospi-ref",
+		.parent = EQ6HC_SOUTH_PLL_PER,
+		.reg = 0x090,
+		.shift = 4,
+		.width = 4,
+	},
+	{
+		.index = EQ6HC_SOUTH_DIV_OSPI_SYS,
+		.name = "div-ospi-sys",
+		.parent = EQ6HC_SOUTH_PLL_PER,
+		.reg = 0x090,
+		.shift = 8,
+		.width = 1,
+	},
+	{
+		.index = EQ6HC_SOUTH_DIV_TSU,
+		.name = "div-tsu",
+		.parent = EQ6HC_SOUTH_PLL_PCIE,
+		.reg = 0x098,
+		.shift = 4,
+		.width = 8,
+	},
+};
+
+static const struct eqc_match_data eqc_eyeq6h_south_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_south_plls),
+	.plls		= eqc_eyeq6h_south_plls,
+
+	.div_count	= ARRAY_SIZE(eqc_eyeq6h_south_divs),
+	.divs		= eqc_eyeq6h_south_divs,
+};
+
+static const struct eqc_pll eqc_eyeq6h_ddr0_plls[] = {
+	{ .index = 0, .name = "pll-ddr0", .reg64 = 0x074 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_ddr0_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_ddr0_plls),
+	.plls		= eqc_eyeq6h_ddr0_plls,
+};
+
+static const struct eqc_pll eqc_eyeq6h_ddr1_plls[] = {
+	{ .index = 0, .name = "pll-ddr1", .reg64 = 0x074 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_ddr1_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_ddr1_plls),
+	.plls		= eqc_eyeq6h_ddr1_plls,
+};
+
+static const struct eqc_pll eqc_eyeq6h_acc_plls[] = {
+	{ .index = EQ6HC_ACC_PLL_XNN, .name = "pll-xnn", .reg64 = 0x040 },
+	{ .index = EQ6HC_ACC_PLL_VMP, .name = "pll-vmp", .reg64 = 0x050 },
+	{ .index = EQ6HC_ACC_PLL_PMA, .name = "pll-pma", .reg64 = 0x05C },
+	{ .index = EQ6HC_ACC_PLL_MPC, .name = "pll-mpc", .reg64 = 0x068 },
+	{ .index = EQ6HC_ACC_PLL_NOC, .name = "pll-noc", .reg64 = 0x070 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_acc_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_acc_plls),
+	.plls		= eqc_eyeq6h_acc_plls,
+
+	.reset_auxdev_name = "reset_acc",
+};
+
+static const struct of_device_id eqc_match_table[] = {
+	{ .compatible = "mobileye,eyeq5-olb", .data = &eqc_eyeq5_match_data },
+	{ .compatible = "mobileye,eyeq6l-olb", .data = &eqc_eyeq6l_match_data },
+	{ .compatible = "mobileye,eyeq6h-west-olb", .data = &eqc_eyeq6h_west_match_data },
+	{ .compatible = "mobileye,eyeq6h-east-olb", .data = &eqc_eyeq6h_east_match_data },
+	{ .compatible = "mobileye,eyeq6h-south-olb", .data = &eqc_eyeq6h_south_match_data },
+	{ .compatible = "mobileye,eyeq6h-ddr0-olb", .data = &eqc_eyeq6h_ddr0_match_data },
+	{ .compatible = "mobileye,eyeq6h-ddr1-olb", .data = &eqc_eyeq6h_ddr1_match_data },
+	{ .compatible = "mobileye,eyeq6h-acc-olb", .data = &eqc_eyeq6h_acc_match_data },
+	{}
+};
+
+static struct platform_driver eqc_driver = {
+	.probe = eqc_probe,
+	.driver = {
+		.name = "clk-eyeq",
+		.of_match_table = eqc_match_table,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(eqc_driver);
+
+/* Required early for GIC timer (pll-cpu) and UARTs (pll-per). */
+static const struct eqc_pll eqc_eyeq5_early_plls[] = {
+	{ .index = EQ5C_PLL_CPU, .name = "pll-cpu",  .reg64 = 0x02C },
+	{ .index = EQ5C_PLL_PER, .name = "pll-per",  .reg64 = 0x05C },
+};
+
+static const struct eqc_early_match_data eqc_eyeq5_early_match_data __initconst = {
+	.early_pll_count	= ARRAY_SIZE(eqc_eyeq5_early_plls),
+	.early_plls		= eqc_eyeq5_early_plls,
+	.nb_late_clks = eqc_eyeq5_match_data.pll_count + eqc_eyeq5_match_data.div_count,
+};
+
+/* Required early for GIC timer. */
+static const struct eqc_pll eqc_eyeq6h_central_early_plls[] = {
+	{ .index = 0, .name = "pll-cpu", .reg64 = 0x02C },
+};
+
+static const struct eqc_early_match_data eqc_eyeq6h_central_early_match_data __initconst = {
+	.early_pll_count	= ARRAY_SIZE(eqc_eyeq6h_central_early_plls),
+	.early_plls		= eqc_eyeq6h_central_early_plls,
+	.nb_late_clks = 0,
+};
+
+/* Required early for UART. */
+static const struct eqc_pll eqc_eyeq6h_west_early_plls[] = {
+	{ .index = 0, .name = "pll-west", .reg64 = 0x074 },
+};
+
+static const struct eqc_early_match_data eqc_eyeq6h_west_early_match_data __initconst = {
+	.early_pll_count	= ARRAY_SIZE(eqc_eyeq6h_west_early_plls),
+	.early_plls		= eqc_eyeq6h_west_early_plls,
+	.nb_late_clks = 0,
+};
+
+static void __init eqc_early_init(struct device_node *np,
+				  const struct eqc_early_match_data *early_data)
+{
+	unsigned int nb_clks;
+	struct eqc_priv *priv;
+	void __iomem *base;
+	unsigned int i;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->np = np;
+	priv->early_data = early_data;
+
+	nb_clks = early_data->early_pll_count + early_data->nb_late_clks;
+	priv->cells = kzalloc(struct_size(priv->cells, hws, nb_clks), GFP_KERNEL);
+	if (!priv->cells) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->cells->num = nb_clks;
+
+	/*
+	 * Mark all clocks as deferred; some are registered here, the rest at
+	 * platform device probe.
+	 */
+	for (i = 0; i < nb_clks; i++)
+		priv->cells->hws[i] = ERR_PTR(-EPROBE_DEFER);
+
+	/* Offsets (reg64) of early PLLs are relative to OLB block. */
+	base = of_iomap(np, 0);
+	if (!base) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	for (i = 0; i < early_data->early_pll_count; i++) {
+		const struct eqc_pll *pll = &early_data->early_plls[i];
+		unsigned long mult, div, acc;
+		struct clk_hw *hw;
+		u32 r0, r1;
+		u64 val;
+
+		val = readq(base + pll->reg64);
+		r0 = val;
+		r1 = val >> 32;
+
+		ret = eqc_pll_parse_registers(r0, r1, &mult, &div, &acc);
+		if (ret) {
+			pr_err("failed parsing state of %s\n", pll->name);
+			goto err;
+		}
+
+		hw = clk_hw_register_fixed_factor_with_accuracy_fwname(NULL,
+				np, pll->name, "ref", 0, mult, div, acc);
+		priv->cells->hws[pll->index] = hw;
+		if (IS_ERR(hw)) {
+			pr_err("failed registering %s: %pe\n", pll->name, hw);
+			ret = PTR_ERR(hw);
+			goto err;
+		}
+	}
+
+	/* When providing a single clock, require no cell. */
+	if (nb_clks == 1)
+		ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, priv->cells->hws[0]);
+	else
+		ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, priv->cells);
+	if (ret) {
+		pr_err("failed registering clk provider: %d\n", ret);
+		goto err;
+	}
+
+	list_add_tail(&priv->list, &eqc_list);
+	return;
+
+err:
+	/*
+	 * We are doomed. The system will not be able to boot.
+	 *
+	 * Let's still try to be good citizens by freeing resources and print
+	 * a last error message that might help debugging.
+	 */
+
+	if (priv && priv->cells) {
+		of_clk_del_provider(np);
+
+		for (i = 0; i < early_data->early_pll_count; i++) {
+			const struct eqc_pll *pll = &early_data->early_plls[i];
+			struct clk_hw *hw = priv->cells->hws[pll->index];
+
+			if (!IS_ERR_OR_NULL(hw))
+				clk_hw_unregister_fixed_factor(hw);
+		}
+
+		kfree(priv->cells);
+	}
+
+	kfree(priv);
+
+	pr_err("failed clk init: %d\n", ret);
+}
+
+static void __init eqc_eyeq5_early_init(struct device_node *np)
+{
+	eqc_early_init(np, &eqc_eyeq5_early_match_data);
+}
+CLK_OF_DECLARE_DRIVER(eqc_eyeq5, "mobileye,eyeq5-olb", eqc_eyeq5_early_init);
+
+static void __init eqc_eyeq6h_central_early_init(struct device_node *np)
+{
+	eqc_early_init(np, &eqc_eyeq6h_central_early_match_data);
+}
+CLK_OF_DECLARE_DRIVER(eqc_eyeq6h_central, "mobileye,eyeq6h-central-olb",
+		      eqc_eyeq6h_central_early_init);
+
+static void __init eqc_eyeq6h_west_early_init(struct device_node *np)
+{
+	eqc_early_init(np, &eqc_eyeq6h_west_early_match_data);
+}
+CLK_OF_DECLARE_DRIVER(eqc_eyeq6h_west, "mobileye,eyeq6h-west-olb",
+		      eqc_eyeq6h_west_early_init);

-- 
2.46.2

Re: [PATCH v5 0/4] Add Mobileye EyeQ clock support

[Théo Lebrun]

Hello Stephen,

On Mon Oct 7, 2024 at 3:49 PM CEST, Théo Lebrun wrote:
> This series adds a platform driver dealing with read-only PLLs derived
> from the main crystal, and some divider clocks based on those PLLs. It
> also acts at the one instantiating reset and pinctrl auxiliary devices.

I'd be curious to get feedback on this series?
Could it make it before the next merge window?

V4 fixed all your comments but one. You implied the linked list might be
useless, but I am not convinced:

> I had a pending question [0], asking for confirmation that the static
> linked list to inherit cells from of_clk_init() stage to platform
> device probe is indeed the right solution. As -rc1 got released I sent
> the new revision anyway.
>
> [0]: https://lore.kernel.org/lkml/D4ELMFAUQYZ7.3LXGQZJSX68UF@bootlin.com/

Quoting here the original email for full context:

On Tue Sep 24, 2024 at 4:53 PM CEST, Théo Lebrun wrote:
> On Wed Sep 18, 2024 at 7:28 AM CEST, Stephen Boyd wrote:
> > Quoting Théo Lebrun (2024-07-30 09:04:46)
> > > +       list_add_tail(&priv->list, &eqc_list);
> >
> > The list is also kind of unnecessary. Set a bool in the match_data and
> > move on? We could have some sort of static_assert() check to make sure
> > if there's a CLK_OF_DECLARE_DRIVER() then the bool is set in the
> > match_data for the driver. Such a design is cheaper than taking a lock,
> > adding to a list.
>
> This list's main goal is not to know what was early-inited. Its only
> reason for existence is that we want to get, at eqc_probe(), the cells
> pointer allocated at eqc_init().
>
> struct eqc_priv {
> 	/* this field is why we store priv inside a linked list: */
> 	struct clk_hw_onecell_data	*cells;
> 	/* the rest, we don't care much: */
> 	const struct eqc_early_match_data *early_data;
> 	const struct eqc_match_data	*data;
> 	void __iomem			*base;
> 	struct device_node		*np;
> 	struct list_head		list;
> };
>
> I do not see how to do that with a bool. We could put the pointer into
> the match data, but that would mean we'd have to make them writable
> (currently static const data). We are talking about a linked list with
> two items in the worst case (EyeQ6H), accessed twice.
>
> The reason we store the whole of priv: simpler code and we avoid mapping
> registers twice (once at eqc_init() and once at eqc_probe()).
>
> Can you confirm the current static linked list approach (without any
> spinlock) will be good for next revision?

Thanks,

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

Re: [PATCH v5 0/4] Add Mobileye EyeQ clock support

[Stephen Boyd]

Quoting Théo Lebrun (2024-10-15 01:50:26)
> Hello Stephen,
> 
> On Mon Oct 7, 2024 at 3:49 PM CEST, Théo Lebrun wrote:
> > This series adds a platform driver dealing with read-only PLLs derived
> > from the main crystal, and some divider clocks based on those PLLs. It
> > also acts at the one instantiating reset and pinctrl auxiliary devices.
> 
> I'd be curious to get feedback on this series?
> Could it make it before the next merge window?
> 
> V4 fixed all your comments but one. You implied the linked list might be
> useless, but I am not convinced:
> 
> > I had a pending question [0], asking for confirmation that the static
> > linked list to inherit cells from of_clk_init() stage to platform
> > device probe is indeed the right solution. As -rc1 got released I sent
> > the new revision anyway.
> >
> > [0]: https://lore.kernel.org/lkml/D4ELMFAUQYZ7.3LXGQZJSX68UF@bootlin.com/
> 
> Quoting here the original email for full context:

Thanks!

> 
> On Tue Sep 24, 2024 at 4:53 PM CEST, Théo Lebrun wrote:
> > On Wed Sep 18, 2024 at 7:28 AM CEST, Stephen Boyd wrote:
> > > Quoting Théo Lebrun (2024-07-30 09:04:46)
> > > > +       list_add_tail(&priv->list, &eqc_list);
> > >
> > > The list is also kind of unnecessary. Set a bool in the match_data and
> > > move on? We could have some sort of static_assert() check to make sure
> > > if there's a CLK_OF_DECLARE_DRIVER() then the bool is set in the
> > > match_data for the driver. Such a design is cheaper than taking a lock,
> > > adding to a list.
> >
> > This list's main goal is not to know what was early-inited. Its only
> > reason for existence is that we want to get, at eqc_probe(), the cells
> > pointer allocated at eqc_init().
> >
> > struct eqc_priv {
> >       /* this field is why we store priv inside a linked list: */
> >       struct clk_hw_onecell_data      *cells;
> >       /* the rest, we don't care much: */
> >       const struct eqc_early_match_data *early_data;

This is __initconst and won't be valid after init, which is possible if
the driver probe is delayed beyond the time that init memory is freed.

> >       const struct eqc_match_data     *data;
> >       void __iomem                    *base;
> >       struct device_node              *np;
> >       struct list_head                list;
> > };
> >
> > I do not see how to do that with a bool. We could put the pointer into
> > the match data, but that would mean we'd have to make them writable
> > (currently static const data). We are talking about a linked list with
> > two items in the worst case (EyeQ6H), accessed twice.

Ah I missed that you were trying to stash the onecell data away. You can
register a clk provider for the same node more than once. The first
"early" one can return an error pointer for the clk indices that will be
registered later. The second onecell data can be registered in the
regular driver probe and return the clks that are registered later. See
of_clk_get_hw_from_clkspec() and how it keeps trying to find a clk if
the provider that matches the node didn't return a valid pointer.

> >
> > The reason we store the whole of priv: simpler code and we avoid mapping
> > registers twice (once at eqc_init() and once at eqc_probe()).

The IO mapping code handles duplicate mappings internally by returning
the previous virtual address. It doesn't hurt to call it again.

> >
> > Can you confirm the current static linked list approach (without any
> > spinlock) will be good for next revision?
> 

Getting rid of the list will make the code simpler and avoid the driver
probe path from accessing the early data. Please do it!

Re: [PATCH v5 4/4] clk: eyeq: add driver

[Stephen Boyd]

Quoting Théo Lebrun (2024-10-07 06:49:19)
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 299bc678ed1b9fcd9110bb8c5937a1bd1ea60e23..ae7caa28985481ce7280421de7d3f2340f8f9ab3 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -226,6 +226,18 @@ config COMMON_CLK_EP93XX
>         help
>           This driver supports the SoC clocks on the Cirrus Logic ep93xx.
>  
> +config COMMON_CLK_EYEQ
> +       bool "Clock driver for the Mobileye EyeQ platform"
> +       depends on 64BIT # for readq()

This makes my build test script blow up. Please just pick one or the
other header as you don't really care. I will apply something like this
patch either way.

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index ae7caa289854..bb8f4fb5f996 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -228,7 +228,6 @@ config COMMON_CLK_EP93XX
 
 config COMMON_CLK_EYEQ
 	bool "Clock driver for the Mobileye EyeQ platform"
-	depends on 64BIT # for readq()
 	depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST
 	select AUXILIARY_BUS
 	default MACH_EYEQ5 || MACH_EYEQ6H
diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
index 762885333c03..2fd44e0d27f6 100644
--- a/drivers/clk/clk-eyeq.c
+++ b/drivers/clk/clk-eyeq.c
@@ -28,7 +28,9 @@
 #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/io-64-nonatomic-hi-lo.h>
 #include <linux/list.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_address.h>

> diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..762885333c0336e9ff1162a3677f5fb815fd461a
> --- /dev/null
> +++ b/drivers/clk/clk-eyeq.c
> @@ -0,0 +1,779 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PLL clock driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
> + *
> + * This controller handles read-only PLLs, all derived from the same main
> + * crystal clock. It also exposes divider clocks, those are children to PLLs.
> + * Parent clock is expected to be constant. This driver's registers live in
> + * a shared region called OLB. Some PLLs are initialised early by of_clk_init().
> + *
> + * We use eqc_ as prefix, as-in "EyeQ Clock", but way shorter.
> + *
> + * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
> + */
> +
> +/*
> + * Set pr_fmt() for printing from eqc_early_init().
> + * It is called at of_clk_init() stage (read: really early).
> + */
> +#define pr_fmt(fmt) "clk-eyeq: " fmt
> +
> +#include <linux/array_size.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>

You need mod_devicetable.h as well for of_device_id.

> +#include <linux/overflow.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
> +
> +#define EQC_MAX_DIV_COUNT              4
> +
> +/* In frac mode, it enables fractional noise canceling DAC. Else, no function. */
> +#define PCSR0_DAC_EN                   BIT(0)
> +/* Fractional or integer mode */
[...]
> +
> +/* Required early for GIC timer (pll-cpu) and UARTs (pll-per). */
> +static const struct eqc_pll eqc_eyeq5_early_plls[] = {
> +       { .index = EQ5C_PLL_CPU, .name = "pll-cpu",  .reg64 = 0x02C },
> +       { .index = EQ5C_PLL_PER, .name = "pll-per",  .reg64 = 0x05C },
> +};
> +
> +static const struct eqc_early_match_data eqc_eyeq5_early_match_data __initconst = {
> +       .early_pll_count        = ARRAY_SIZE(eqc_eyeq5_early_plls),
> +       .early_plls             = eqc_eyeq5_early_plls,
> +       .nb_late_clks = eqc_eyeq5_match_data.pll_count + eqc_eyeq5_match_data.div_count,
> +};
> +
> +/* Required early for GIC timer. */
> +static const struct eqc_pll eqc_eyeq6h_central_early_plls[] = {
> +       { .index = 0, .name = "pll-cpu", .reg64 = 0x02C },
> +};
> +
> +static const struct eqc_early_match_data eqc_eyeq6h_central_early_match_data __initconst = {
> +       .early_pll_count        = ARRAY_SIZE(eqc_eyeq6h_central_early_plls),
> +       .early_plls             = eqc_eyeq6h_central_early_plls,
> +       .nb_late_clks = 0,
> +};
> +
> +/* Required early for UART. */

I still don't get this. UART isn't an early device. It's only the
interrupt controller and the timer that matter. Does MIPS do something
special for UARTs?

> +static const struct eqc_pll eqc_eyeq6h_west_early_plls[] = {
> +       { .index = 0, .name = "pll-west", .reg64 = 0x074 },
> +};

Re: [PATCH v5 1/4] Revert "dt-bindings: clock: mobileye,eyeq5-clk: add bindings"

[Stephen Boyd]

Quoting Théo Lebrun (2024-10-07 06:49:16)
> Switch from one sub-node per functionality in the system-controller to a
> single node representing the entire OLB instance. This is the
> recommended approach for controllers handling many different
> functionalities; it is a single controller and should be represented by
> a single devicetree node.
> 
> The clock bindings is removed and all properties will be described by:
> soc/mobileye/mobileye,eyeq5-olb.yaml
> 
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---

Applied to clk-next

Re: [PATCH v5 2/4] dt-bindings: clock: add Mobileye EyeQ6L/EyeQ6H clock indexes

[Stephen Boyd]

Quoting Théo Lebrun (2024-10-07 06:49:17)
> Add #defines for Mobileye EyeQ6L and EyeQ6H SoC clocks.
> 
> Constant prefixes are:
>  - EQ6LC_PLL_: EyeQ6L clock PLLs
>  - EQ6HC_SOUTH_PLL_: EyeQ6H south OLB PLLs
>  - EQ6HC_SOUTH_DIV_: EyeQ6H south OLB divider clocks
>  - EQ6HC_ACC_PLL_: EyeQ6H accelerator OLB PLLs
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---

Applied to clk-next

Re: [PATCH v5 3/4] clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag

[Stephen Boyd]

Quoting Théo Lebrun (2024-10-07 06:49:18)
> Add CLK_DIVIDER_EVEN_INTEGERS flag to support divisor of 2, 4, 6, etc.
> The same divisor can be done using a table, which would be big and
> wasteful for a clock dividor of width 8 (256 entries).
> 
> Require increasing flags size from u8 to u16 because
> CLK_DIVIDER_EVEN_INTEGERS is the eighth flag. u16 is used inside struct
> clk_divider; `unsigned long` is used for function arguments.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---

Applied to clk-next

Re: [PATCH v5 4/4] clk: eyeq: add driver

[Théo Lebrun]

Hello Stephen,

On Thu Oct 17, 2024 at 8:48 PM CEST, Stephen Boyd wrote:
> Quoting Théo Lebrun (2024-10-07 06:49:19)
> > +/* Required early for GIC timer (pll-cpu) and UARTs (pll-per). */
> > +static const struct eqc_pll eqc_eyeq5_early_plls[] = {
> > +       { .index = EQ5C_PLL_CPU, .name = "pll-cpu",  .reg64 = 0x02C },
> > +       { .index = EQ5C_PLL_PER, .name = "pll-per",  .reg64 = 0x05C },
> > +};
> > +
> > +static const struct eqc_early_match_data eqc_eyeq5_early_match_data __initconst = {
> > +       .early_pll_count        = ARRAY_SIZE(eqc_eyeq5_early_plls),
> > +       .early_plls             = eqc_eyeq5_early_plls,
> > +       .nb_late_clks = eqc_eyeq5_match_data.pll_count + eqc_eyeq5_match_data.div_count,
> > +};
> > +
> > +/* Required early for GIC timer. */
> > +static const struct eqc_pll eqc_eyeq6h_central_early_plls[] = {
> > +       { .index = 0, .name = "pll-cpu", .reg64 = 0x02C },
> > +};
> > +
> > +static const struct eqc_early_match_data eqc_eyeq6h_central_early_match_data __initconst = {
> > +       .early_pll_count        = ARRAY_SIZE(eqc_eyeq6h_central_early_plls),
> > +       .early_plls             = eqc_eyeq6h_central_early_plls,
> > +       .nb_late_clks = 0,
> > +};
> > +
> > +/* Required early for UART. */
>
> I still don't get this. UART isn't an early device. It's only the
> interrupt controller and the timer that matter. Does MIPS do something
> special for UARTs?

Our hardware has a PL011. That is AMBA stuff; they get probed before
platform devices by of_platform_bus_create(). "pll-per" on EyeQ5 must
be available at that time.

In concrete terms, if we don't register pll-per on EyeQ5 at
of_clk_init(), we stare at void because the serial fails probing.
I haven't digged into why EPROBE_DEFER doesn't do its job. Anyway we
don't want our serial to stall for some time during our boot process.

Thanks for the extensive review! New revision is in your inbox.

Thanks,
Théo

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

Re: [PATCH v5 4/4] clk: eyeq: add driver

[Stephen Boyd]

Quoting Théo Lebrun (2024-10-23 04:08:31)
> On Thu Oct 17, 2024 at 8:48 PM CEST, Stephen Boyd wrote:
> > Quoting Théo Lebrun (2024-10-07 06:49:19)
> > > +/* Required early for UART. */
> >
> > I still don't get this. UART isn't an early device. It's only the
> > interrupt controller and the timer that matter. Does MIPS do something
> > special for UARTs?
> 
> Our hardware has a PL011. That is AMBA stuff; they get probed before
> platform devices by of_platform_bus_create(). "pll-per" on EyeQ5 must
> be available at that time.
> 
> In concrete terms, if we don't register pll-per on EyeQ5 at
> of_clk_init(), we stare at void because the serial fails probing.
> I haven't digged into why EPROBE_DEFER doesn't do its job. Anyway we
> don't want our serial to stall for some time during our boot process.
> 

Ok thanks for the details. It sounds like there's a bug in there
somewhere. Eventually this should be removed.

Can you dump_stack() in clk_get() when the "pll-per" clk is claimed?

I suspect of_clk_get_hw_from_clkspec() is seeing NULL if
of_clk_hw_onecell_get() is being used and the clk_hw pointer isn't set
yet. NULL is a valid clk and it will be returned to the consumer. You'll
want to write a custom 'get' function for of_clk_add_hw_provider() that
returns -EPROBE_DEFER for any clk that isn't registered early. Then the
AMBA stuff should defer probe until the "full" clk provider is
registered.

Re: [PATCH v5 4/4] clk: eyeq: add driver

[Théo Lebrun]

Hello Stephen, Conor, Krzysztof, Michael, Rob,

On Thu Oct 24, 2024 at 12:12 AM CEST, Stephen Boyd wrote:
> Quoting Théo Lebrun (2024-10-23 04:08:31)
> > On Thu Oct 17, 2024 at 8:48 PM CEST, Stephen Boyd wrote:
> > > Quoting Théo Lebrun (2024-10-07 06:49:19)
> > > > +/* Required early for UART. */
> > >
> > > I still don't get this. UART isn't an early device. It's only the
> > > interrupt controller and the timer that matter. Does MIPS do something
> > > special for UARTs?
> > 
> > Our hardware has a PL011. That is AMBA stuff; they get probed before
> > platform devices by of_platform_bus_create(). "pll-per" on EyeQ5 must
> > be available at that time.
> > 
> > In concrete terms, if we don't register pll-per on EyeQ5 at
> > of_clk_init(), we stare at void because the serial fails probing.
> > I haven't digged into why EPROBE_DEFER doesn't do its job. Anyway we
> > don't want our serial to stall for some time during our boot process.
> > 
>
> Ok thanks for the details. It sounds like there's a bug in there
> somewhere. Eventually this should be removed.
>
> Can you dump_stack() in clk_get() when the "pll-per" clk is claimed?
>
> I suspect of_clk_get_hw_from_clkspec() is seeing NULL if
> of_clk_hw_onecell_get() is being used and the clk_hw pointer isn't set
> yet. NULL is a valid clk and it will be returned to the consumer. You'll
> want to write a custom 'get' function for of_clk_add_hw_provider() that
> returns -EPROBE_DEFER for any clk that isn't registered early. Then the
> AMBA stuff should defer probe until the "full" clk provider is
> registered.

You encouraged me to keep digging.

The bug is elsewhere: we do get valid clocks from PL011. Both clk_get()
calls give proper pointers.

The issue is that we are using `compatible = "fixed-factor-clock"`
clocks in the middle, and those don't wait for their parents to be
active.

Simplified clock graph is: pll-per -> occ-periph.
pll-per is register by our driver. occ-periph looks like:

	occ_periph: occ-periph {
		compatible = "fixed-factor-clock";
		clocks = <&olb EQ5C_PLL_PER>;
		#clock-cells = <0>;
		clock-div = <16>;
		clock-mult = <1>;
	};

Sequence is:
 - eqc_early_init(): it registers a clock provider that will return
   EPROBE_DEFER for our pll-per.
 - _of_fixed_factor_clk_setup(): it registers occ-periph, even though
   its parent is EPROBE_DEFER. clk_core_populate_parent_map() runs all
   fine without complaining; logical as it doesn't query the clk_hw for
   its parent, it only stores indexes.
 - amba_get_enable_pclk(): it does a clk_get() which works because
   occ-periph exists.

Maybe __clk_register() should check the clk_hw for each parent: if any
is an EPROBE_DEFER then it should EPROBE_DEFER itself? That looks like
a rather big behavioral change.

The other solution is to keep as-is: provide all clocks consumed by
fixed-factor-clocks at of_clk_init() stage.

Hoping I provided enough info,
Thanks,

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

Re: [PATCH v5 4/4] clk: eyeq: add driver

[Stephen Boyd]

Quoting Théo Lebrun (2024-10-24 05:50:16)
> The bug is elsewhere: we do get valid clocks from PL011. Both clk_get()
> calls give proper pointers.
> 
> The issue is that we are using `compatible = "fixed-factor-clock"`
> clocks in the middle, and those don't wait for their parents to be
> active.
> 
> Simplified clock graph is: pll-per -> occ-periph.
> pll-per is register by our driver. occ-periph looks like:
> 
>         occ_periph: occ-periph {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&olb EQ5C_PLL_PER>;
>                 #clock-cells = <0>;
>                 clock-div = <16>;
>                 clock-mult = <1>;
>         };

Why is this fixed factor clk registered from DT vs. from the driver that
registers pll-per? Is it useful to describe it in DT because the factor
can change? Where does it physically exist? In the SoC?

> 
> Sequence is:
>  - eqc_early_init(): it registers a clock provider that will return
>    EPROBE_DEFER for our pll-per.
>  - _of_fixed_factor_clk_setup(): it registers occ-periph, even though
>    its parent is EPROBE_DEFER. clk_core_populate_parent_map() runs all
>    fine without complaining; logical as it doesn't query the clk_hw for
>    its parent, it only stores indexes.
>  - amba_get_enable_pclk(): it does a clk_get() which works because
>    occ-periph exists.
> 
> Maybe __clk_register() should check the clk_hw for each parent: if any
> is an EPROBE_DEFER then it should EPROBE_DEFER itself? That looks like
> a rather big behavioral change.
> 
> The other solution is to keep as-is: provide all clocks consumed by
> fixed-factor-clocks at of_clk_init() stage.

Another solution is to register the fixed factor clk from the pll-per
clk provider.

And yet another solution is to return EPROBE_DEFER for orphaned clks. We
have everything in place for that but we ran into trouble with consumers
wanting to get orphaned clks in their probe or during assigned-clocks
handling.

Re: [PATCH v5 4/4] clk: eyeq: add driver

[Théo Lebrun]

On Mon Oct 28, 2024 at 11:52 PM CET, Stephen Boyd wrote:
> Quoting Théo Lebrun (2024-10-24 05:50:16)
> > The bug is elsewhere: we do get valid clocks from PL011. Both clk_get()
> > calls give proper pointers.
> > 
> > The issue is that we are using `compatible = "fixed-factor-clock"`
> > clocks in the middle, and those don't wait for their parents to be
> > active.
> > 
> > Simplified clock graph is: pll-per -> occ-periph.
> > pll-per is register by our driver. occ-periph looks like:
> > 
> >         occ_periph: occ-periph {
> >                 compatible = "fixed-factor-clock";
> >                 clocks = <&olb EQ5C_PLL_PER>;
> >                 #clock-cells = <0>;
> >                 clock-div = <16>;
> >                 clock-mult = <1>;
> >         };
>
> Why is this fixed factor clk registered from DT vs. from the driver that
> registers pll-per? Is it useful to describe it in DT because the factor
> can change? Where does it physically exist? In the SoC?

Those are internal SoC clocks, yes. No reason for them to change from
one board to another. Adding them from the driver makes the most sense,
I'll patch that up.

> > Sequence is:
> >  - eqc_early_init(): it registers a clock provider that will return
> >    EPROBE_DEFER for our pll-per.
> >  - _of_fixed_factor_clk_setup(): it registers occ-periph, even though
> >    its parent is EPROBE_DEFER. clk_core_populate_parent_map() runs all
> >    fine without complaining; logical as it doesn't query the clk_hw for
> >    its parent, it only stores indexes.
> >  - amba_get_enable_pclk(): it does a clk_get() which works because
> >    occ-periph exists.
> > 
> > Maybe __clk_register() should check the clk_hw for each parent: if any
> > is an EPROBE_DEFER then it should EPROBE_DEFER itself? That looks like
> > a rather big behavioral change.
> > 
> > The other solution is to keep as-is: provide all clocks consumed by
> > fixed-factor-clocks at of_clk_init() stage.
>
> Another solution is to register the fixed factor clk from the pll-per
> clk provider.
>
> And yet another solution is to return EPROBE_DEFER for orphaned clks. We
> have everything in place for that but we ran into trouble with consumers
> wanting to get orphaned clks in their probe or during assigned-clocks
> handling.

No suprise this kind of edge-case behavior can cause trouble.

Thanks,

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