diff mbox series

[v7,07/14] clk: eyeq5: add platform driver, and init routine at of_clk_init()

Message ID 20240221-mbly-clk-v7-7-31d4ce3630c3@bootlin.com
State New
Headers show
Series Add support for Mobileye EyeQ5 system controller | expand

Commit Message

Théo Lebrun Feb. 21, 2024, 6:22 p.m. UTC
Add the Mobileye EyeQ5 clock controller driver. It might grow to add
support for other platforms from Mobileye.

It handles 10 read-only PLLs derived from the main crystal on board. It
exposes a table-based divider clock used for OSPI. Other platform
clocks are not configurable and therefore kept as fixed-factor
devicetree nodes.

Two PLLs are required early on and are therefore registered at
of_clk_init(). Those are pll-cpu for the GIC timer and pll-per for the
UARTs.

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

Comments

Andy Shevchenko Feb. 22, 2024, 5:06 a.m. UTC | #1
Wed, Feb 21, 2024 at 07:22:15PM +0100, Théo Lebrun kirjoitti:
> Add the Mobileye EyeQ5 clock controller driver. It might grow to add
> support for other platforms from Mobileye.
> 
> It handles 10 read-only PLLs derived from the main crystal on board. It
> exposes a table-based divider clock used for OSPI. Other platform
> clocks are not configurable and therefore kept as fixed-factor
> devicetree nodes.
> 
> Two PLLs are required early on and are therefore registered at
> of_clk_init(). Those are pll-cpu for the GIC timer and pll-per for the
> UARTs.

...

> +config COMMON_CLK_EYEQ5
> +	bool "Clock driver for the Mobileye EyeQ5 platform"

> +	depends on OF

Is this functional dependency? For compilation it seems you don't need it, also see below.

> +	depends on MACH_EYEQ5 || COMPILE_TEST
> +	default MACH_EYEQ5
> +	help
> +		This driver provides the 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
> +		and one divider clock based on one PLL.

Wrong indentation, have you run checkpatch?

...

> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk-provider.h>
> +#include <linux/mod_devicetable.h>

> +#include <linux/of_address.h>

Misused header. Also see below.

> +#include <linux/platform_device.h>

You have semi-random list of inclusions. Please, follow the IWUY principle.

Here I see _at least_ missing
array_size.h
err.h
io.h
slab.h
types.h

 
...

> +static int eq5c_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
> +				    unsigned long *div, unsigned long *acc)
> +{
> +	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);

One line?

> +	/* Fractional mode, in 2^20 (0x100000) parts. */
> +	if (r0 & PCSR0_DSM_EN) {
> +		*div *= 0x100000;
> +		*mult = *mult * 0x100000 + FIELD_GET(PCSR1_FRAC_IN, r1);
> +	}
> +
> +	if (!*mult || !*div)
> +		return -EINVAL;
> +
> +	/* Spread spectrum. */
> +	if (!(r1 & (PCSR1_RESET | 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(PCSR1_SPREAD, r1);

Missing blank line.

> +		*acc = spread * 500000;
> +		if (r1 & PCSR1_DOWN_SPREAD) {
> +			/*
> +			 * Downspreading: the central frequency is half a
> +			 * spread lower.
> +			 */
> +			*mult *= 2000 - spread;
> +			*div *= 2000;
> +		}
> +	} else {
> +		*acc = 0;
> +	}
> +
> +	return 0;
> +}

Looking at this function what I would do is to replace mul/div pair by
respective struct uXX_fract, add something like

#define mult_fract(fract, ...)		\
	...

and replace those

	*mult/*div *= ...

with

	mult_fract(fract, 2000);

etc.

...

> +static int eq5c_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	void __iomem *base_plls, *base_ospi;
> +	struct clk_hw *hw;
> +	int i;

> +	if (IS_ERR(eq5c_clk_data))
> +		return PTR_ERR(eq5c_clk_data);
> +	else if (!eq5c_clk_data)
> +		return -EINVAL;

Besides unneeded 'else', why so complicated? Can't you choose one: either NULL
or error pointer for the invalid state?

> +	base_plls = devm_platform_ioremap_resource_byname(pdev, "plls");
> +	base_ospi = devm_platform_ioremap_resource_byname(pdev, "ospi");

> +	if (!base_plls || !base_ospi)
> +		return -ENODEV;

Huh?! Are they not an error pointers and never be NULL?

> +	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;
> +
> +		r0 = readl(base_plls + pll->reg);
> +		r1 = readl(base_plls + pll->reg + sizeof(r0));
> +
> +		ret = eq5c_pll_parse_registers(r0, r1, &mult, &div, &acc);
> +		if (ret) {
> +			dev_warn(dev, "failed parsing state of %s\n", pll->name);
> +			continue;
> +		}
> +
> +		hw = clk_hw_register_fixed_factor_with_accuracy_fwname(dev, np,
> +				pll->name, "ref", 0, mult, div, acc);
> +		eq5c_clk_data->hws[pll->index] = hw;

Why do you feel the data with errorneous one (in some cases)? It's quite
unusual pattern.

> +		if (IS_ERR(hw)) {
> +			dev_err(dev, "failed registering %s: %ld\n",
> +				pll->name, PTR_ERR(hw));
> +		}

Besides unnecessity of {} can't you unify the output format by using
dev_err_probe() in all error messages in ->probe()?

> +	}
> +
> +	hw = clk_hw_register_divider_table_parent_hw(dev, EQ5C_OSPI_DIV_CLK_NAME,
> +			eq5c_clk_data->hws[EQ5C_PLL_PER], 0,
> +			base_ospi, 0, EQ5C_OSPI_DIV_WIDTH, 0,
> +			eq5c_ospi_div_table, NULL);

> +	eq5c_clk_data->hws[EQ5C_DIV_OSPI] = hw;

Same as above.

> +	if (IS_ERR(hw)) {
> +		dev_err(dev, "failed registering %s: %ld\n",
> +			EQ5C_OSPI_DIV_CLK_NAME, PTR_ERR(hw));
> +	}

Same as above.

> +	return 0;
> +}

...

> +static struct platform_driver eq5c_driver = {
> +	.probe = eq5c_probe,
> +	.driver = {
> +		.name = "clk-eyeq5",
> +		.of_match_table = eq5c_match_table,
> +	},
> +};

> +

Redundant blank line.

> +builtin_platform_driver(eq5c_driver);

...

> +	index_plls = of_property_match_string(np, "reg-names", "plls");
> +	index_ospi = of_property_match_string(np, "reg-names", "ospi");
> +	if (index_plls < 0 || index_ospi < 0) {
> +		ret = -ENODEV;

Why error codes are shadowed?

> +		goto err;
> +	}
Théo Lebrun Feb. 22, 2024, 2:56 p.m. UTC | #2
Hello,

On Thu Feb 22, 2024 at 6:06 AM CET,  wrote:
> Wed, Feb 21, 2024 at 07:22:15PM +0100, Théo Lebrun kirjoitti:
> > Add the Mobileye EyeQ5 clock controller driver. It might grow to add
> > support for other platforms from Mobileye.
> > 
> > It handles 10 read-only PLLs derived from the main crystal on board. It
> > exposes a table-based divider clock used for OSPI. Other platform
> > clocks are not configurable and therefore kept as fixed-factor
> > devicetree nodes.
> > 
> > Two PLLs are required early on and are therefore registered at
> > of_clk_init(). Those are pll-cpu for the GIC timer and pll-per for the
> > UARTs.
>
> ...
>
> > +config COMMON_CLK_EYEQ5
> > +	bool "Clock driver for the Mobileye EyeQ5 platform"
>
> > +	depends on OF
>
> Is this functional dependency? For compilation it seems you don't need
> it, also see below.

Indeed it is a functional dependency. See of_iomap() or
of_property_match_string() usage for example. If CONFIG_OF=n both build
fine but have no behavior. In the case of such a driver having a
polyfill that does nothing is not helpful, it'd be more useful to have
the build fail.

> > +	depends on MACH_EYEQ5 || COMPILE_TEST
> > +	default MACH_EYEQ5
> > +	help
> > +		This driver provides the 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
> > +		and one divider clock based on one PLL.
>
> Wrong indentation, have you run checkpatch?

`./scripts/checkpatch.pl --strict` on this commit does not complain
about this help block indentation. I'll fix it anyway.

>
> ...
>
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/mod_devicetable.h>
>
> > +#include <linux/of_address.h>
>
> Misused header. Also see below.

It provides of_iomap() and isn't indirectly included by anything else.
Removing this include leads to a build error.

>
> > +#include <linux/platform_device.h>
>
> You have semi-random list of inclusions. Please, follow the IWUY principle.
>
> Here I see _at least_ missing
> array_size.h
> err.h
> io.h
> slab.h
> types.h

Here is the list I land on. I've read the file from top to bottom
checking out each symbol.

#include <linux/array_size.h>
#include <linux/bitfield.h>
#include <linux/bits.h>
#include <linux/clk-provider.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>
#include <linux/printk.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <dt-bindings/clock/mobileye,eyeq5-clk.h>

>
>  
> ...
>
> > +static int eq5c_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
> > +				    unsigned long *div, unsigned long *acc)
> > +{
> > +	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);
>
> One line?
>
> > +	/* Fractional mode, in 2^20 (0x100000) parts. */
> > +	if (r0 & PCSR0_DSM_EN) {
> > +		*div *= 0x100000;
> > +		*mult = *mult * 0x100000 + FIELD_GET(PCSR1_FRAC_IN, r1);
> > +	}
> > +
> > +	if (!*mult || !*div)
> > +		return -EINVAL;
> > +
> > +	/* Spread spectrum. */
> > +	if (!(r1 & (PCSR1_RESET | 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(PCSR1_SPREAD, r1);
>
> Missing blank line.
>
> > +		*acc = spread * 500000;
> > +		if (r1 & PCSR1_DOWN_SPREAD) {
> > +			/*
> > +			 * Downspreading: the central frequency is half a
> > +			 * spread lower.
> > +			 */
> > +			*mult *= 2000 - spread;
> > +			*div *= 2000;
> > +		}
> > +	} else {
> > +		*acc = 0;
> > +	}
> > +
> > +	return 0;
> > +}
>
> Looking at this function what I would do is to replace mul/div pair by
> respective struct uXX_fract, add something like
>
> #define mult_fract(fract, ...)		\
> 	...
>
> and replace those
>
> 	*mult/*div *= ...
>
> with
>
> 	mult_fract(fract, 2000);
>
> etc.

I'm not sure I see the logic (?). We multiply div and mult by the same
constant once, in the fractional mode if-statement. Would it clarify
the code to add a new type?

Let's try it out, the code would become:

struct eq5c_fract { unsigned long mult, div; };

static void mult_fract(struct eq5c_fract *fract, unsigned long c)
{
	fract->mul *= c;
	fract->div *= c;
}

static int eq5c_pll_parse_registers(u32 r0, u32 r1,
				    struct eq5c_fract *fract,
				    unsigned long *acc)
{
	if (r0 & PCSR0_BYPASS) {
		fract->mult = 1;
		fract->div = 1;
		*acc = 0;
		return 0;
	}

	if (!(r0 & PCSR0_PLL_LOCKED))
		return -EINVAL;

	fract->mult = FIELD_GET(PCSR0_INTIN, r0);
	fract->div = FIELD_GET(PCSR0_REF_DIV, r0);
	if (r0 & PCSR0_FOUTPOSTDIV_EN)
		fract->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) {
		mult_fract(fract, 0x100000);
		fract->mult += FIELD_GET(PCSR1_FRAC_IN, r1);
	}

	if (!fract->mult || !fract->div)
		return -EINVAL;

	/* Spread spectrum. */
	if (!(r1 & (PCSR1_RESET | 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(PCSR1_SPREAD, r1);
		*acc = spread * 500000;
		if (r1 & PCSR1_DOWN_SPREAD) {
			/*
			 * Downspreading: the central frequency is half a
			 * spread lower.
			 */
			fract->mult *= 2000 - spread;
			fract->div *= 2000;
		}
	} else {
		*acc = 0;
	}

	return 0;
}

As-is, I'm not convinced. Maybe some other helpers would help? Still
unsure: it would add indirection. If we did a lot of this fract
manipulation (or if helpers existed globally) I'd understand but here
we are talking about a 50 lines function.

>
> ...
>
> > +static int eq5c_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *np = dev->of_node;
> > +	void __iomem *base_plls, *base_ospi;
> > +	struct clk_hw *hw;
> > +	int i;
>
> > +	if (IS_ERR(eq5c_clk_data))
> > +		return PTR_ERR(eq5c_clk_data);
> > +	else if (!eq5c_clk_data)
> > +		return -EINVAL;
>
> Besides unneeded 'else', why so complicated? Can't you choose one: either NULL
> or error pointer for the invalid state?

IS_ERR(eq5c_clk_data) is in the case of an error in eq5c_init()
execution. It allows eq5c_init() to pick the error int to return from
probe. eq5c_clk_data == NULL is in the case of eq5c_init() not being
called, ie if arch doesn't call of_clk_init().

>
> > +	base_plls = devm_platform_ioremap_resource_byname(pdev, "plls");
> > +	base_ospi = devm_platform_ioremap_resource_byname(pdev, "ospi");
>
> > +	if (!base_plls || !base_ospi)
> > +		return -ENODEV;
>
> Huh?! Are they not an error pointers and never be NULL?

They are indeed error pointers; I'll be fixing that.

>
> > +	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;
> > +
> > +		r0 = readl(base_plls + pll->reg);
> > +		r1 = readl(base_plls + pll->reg + sizeof(r0));
> > +
> > +		ret = eq5c_pll_parse_registers(r0, r1, &mult, &div, &acc);
> > +		if (ret) {
> > +			dev_warn(dev, "failed parsing state of %s\n", pll->name);
> > +			continue;
> > +		}
> > +
> > +		hw = clk_hw_register_fixed_factor_with_accuracy_fwname(dev, np,
> > +				pll->name, "ref", 0, mult, div, acc);
> > +		eq5c_clk_data->hws[pll->index] = hw;
>
> Why do you feel the data with errorneous one (in some cases)? It's quite
> unusual pattern.

Actually many clk drivers put ERR_PTR(...) when a clock is not
present/available/whatever. See:

	$ git grep 'hws\[.*ERR_PTR' drivers/clk/

Options from my POV are:

 - Put the error as-is.
 - Shadow the error with ENOENT or ENODEV.
 - Put NULL.

I picked option 1. Would option 3 be better?

I start eq5c_init() by marking all clocks as EPROBE_DEFER. So we must
overwrite a value to all clks once we tried creating them. I thought
putting the clk_hw_register_*() error would make sense.

That makes me notice that if eq5c_pll_parse_registers() fails I don't
put a value in the clk hw and leave the EPROBE_DEFER. I'll fix that.

>
> > +		if (IS_ERR(hw)) {
> > +			dev_err(dev, "failed registering %s: %ld\n",
> > +				pll->name, PTR_ERR(hw));
> > +		}
>
> Besides unnecessity of {} can't you unify the output format by using
> dev_err_probe() in all error messages in ->probe()?

Sure, will remove {} and use dev_err_probe().

>
> > +	}
> > +
> > +	hw = clk_hw_register_divider_table_parent_hw(dev, EQ5C_OSPI_DIV_CLK_NAME,
> > +			eq5c_clk_data->hws[EQ5C_PLL_PER], 0,
> > +			base_ospi, 0, EQ5C_OSPI_DIV_WIDTH, 0,
> > +			eq5c_ospi_div_table, NULL);
>
> > +	eq5c_clk_data->hws[EQ5C_DIV_OSPI] = hw;
>
> Same as above.
>
> > +	if (IS_ERR(hw)) {
> > +		dev_err(dev, "failed registering %s: %ld\n",
> > +			EQ5C_OSPI_DIV_CLK_NAME, PTR_ERR(hw));
> > +	}
>
> Same as above.
>
> > +	return 0;
> > +}
>
> ...
>
> > +static struct platform_driver eq5c_driver = {
> > +	.probe = eq5c_probe,
> > +	.driver = {
> > +		.name = "clk-eyeq5",
> > +		.of_match_table = eq5c_match_table,
> > +	},
> > +};
>
> > +
>
> Redundant blank line.
>
> > +builtin_platform_driver(eq5c_driver);
>
> ...
>
> > +	index_plls = of_property_match_string(np, "reg-names", "plls");
> > +	index_ospi = of_property_match_string(np, "reg-names", "ospi");
> > +	if (index_plls < 0 || index_ospi < 0) {
> > +		ret = -ENODEV;
>
> Why error codes are shadowed?

Good question, I'll fix that.

Thanks for your review! I've seen all remarks but not answered them all.

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

Patch

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 50af5fc7f570..d5043ce2a75c 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 MACH_EYEQ5 || COMPILE_TEST
+	default MACH_EYEQ5
+	help
+		This driver provides the 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
+		and one divider clock based on one PLL.
+
 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 14fa8d4ecc1f..81c4d11ca437 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..9598139e0383
--- /dev/null
+++ b/drivers/clk/clk-eyeq5.c
@@ -0,0 +1,289 @@ 
+// 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. It also exposes one divider clock, a child of one of the
+ * PLLs. The parent clock is expected to be constant. This driver's registers
+ * live in a shared region called OLB. Two PLLs must be initialized by
+ * of_clk_init().
+ *
+ * We use eq5c_ as prefix, as-in "EyeQ5 Clock", but way shorter.
+ *
+ * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
+ */
+
+#define pr_fmt(fmt) "clk-eyeq5: " fmt
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk-provider.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
+
+/* 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)
+
+static struct clk_hw_onecell_data *eq5c_clk_data;
+
+struct eq5c_pll {
+	int		index;
+	const char	*name;
+	u32		reg;	/* next 8 bytes are r0 and r1 */
+};
+
+/* Required early for the GIC timer (pll-cpu) and UARTs (pll-per). */
+static const struct eq5c_pll eq5c_early_plls[] = {
+	{ .index = EQ5C_PLL_CPU, .name = "pll-cpu",  .reg = 0x00, },
+	{ .index = EQ5C_PLL_PER, .name = "pll-per",  .reg = 0x30, },
+};
+
+static const struct eq5c_pll eq5c_plls[] = {
+	{ .index = EQ5C_PLL_VMP,  .name = "pll-vmp",  .reg = 0x08, },
+	{ .index = EQ5C_PLL_PMA,  .name = "pll-pma",  .reg = 0x10, },
+	{ .index = EQ5C_PLL_VDI,  .name = "pll-vdi",  .reg = 0x18, },
+	{ .index = EQ5C_PLL_DDR0, .name = "pll-ddr0", .reg = 0x20, },
+	{ .index = EQ5C_PLL_PCI,  .name = "pll-pci",  .reg = 0x28, },
+	{ .index = EQ5C_PLL_PMAC, .name = "pll-pmac", .reg = 0x38, },
+	{ .index = EQ5C_PLL_MPC,  .name = "pll-mpc",  .reg = 0x40, },
+	{ .index = EQ5C_PLL_DDR1, .name = "pll-ddr1", .reg = 0x48, },
+};
+
+#define EQ5C_OSPI_DIV_CLK_NAME	"div-ospi"
+#define EQ5C_OSPI_DIV_WIDTH	4
+
+#define EQ5C_NB_CLKS	(ARRAY_SIZE(eq5c_early_plls) + ARRAY_SIZE(eq5c_plls) + 1)
+
+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 */
+};
+
+static int eq5c_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
+				    unsigned long *div, unsigned long *acc)
+{
+	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 *= 0x100000;
+		*mult = *mult * 0x100000 + FIELD_GET(PCSR1_FRAC_IN, r1);
+	}
+
+	if (!*mult || !*div)
+		return -EINVAL;
+
+	/* Spread spectrum. */
+	if (!(r1 & (PCSR1_RESET | 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(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;
+		}
+	} else {
+		*acc = 0;
+	}
+
+	return 0;
+}
+
+static int eq5c_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	void __iomem *base_plls, *base_ospi;
+	struct clk_hw *hw;
+	int i;
+
+	if (IS_ERR(eq5c_clk_data))
+		return PTR_ERR(eq5c_clk_data);
+	else if (!eq5c_clk_data)
+		return -EINVAL;
+
+	base_plls = devm_platform_ioremap_resource_byname(pdev, "plls");
+	base_ospi = devm_platform_ioremap_resource_byname(pdev, "ospi");
+	if (!base_plls || !base_ospi)
+		return -ENODEV;
+
+	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;
+
+		r0 = readl(base_plls + pll->reg);
+		r1 = readl(base_plls + pll->reg + sizeof(r0));
+
+		ret = eq5c_pll_parse_registers(r0, r1, &mult, &div, &acc);
+		if (ret) {
+			dev_warn(dev, "failed parsing state of %s\n", pll->name);
+			continue;
+		}
+
+		hw = clk_hw_register_fixed_factor_with_accuracy_fwname(dev, np,
+				pll->name, "ref", 0, mult, div, acc);
+		eq5c_clk_data->hws[pll->index] = hw;
+		if (IS_ERR(hw)) {
+			dev_err(dev, "failed registering %s: %ld\n",
+				pll->name, PTR_ERR(hw));
+		}
+	}
+
+	hw = clk_hw_register_divider_table_parent_hw(dev, EQ5C_OSPI_DIV_CLK_NAME,
+			eq5c_clk_data->hws[EQ5C_PLL_PER], 0,
+			base_ospi, 0, EQ5C_OSPI_DIV_WIDTH, 0,
+			eq5c_ospi_div_table, NULL);
+	eq5c_clk_data->hws[EQ5C_DIV_OSPI] = hw;
+	if (IS_ERR(hw)) {
+		dev_err(dev, "failed registering %s: %ld\n",
+			EQ5C_OSPI_DIV_CLK_NAME, PTR_ERR(hw));
+	}
+
+	return 0;
+}
+
+static const struct of_device_id eq5c_match_table[] = {
+	{ .compatible = "mobileye,eyeq5-clk" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, eq5c_match_table);
+
+static struct platform_driver eq5c_driver = {
+	.probe = eq5c_probe,
+	.driver = {
+		.name = "clk-eyeq5",
+		.of_match_table = eq5c_match_table,
+	},
+};
+
+builtin_platform_driver(eq5c_driver);
+
+static void __init eq5c_init(struct device_node *np)
+{
+	void __iomem *base_plls, *base_ospi;
+	int index_plls, index_ospi;
+	int i, ret;
+
+	eq5c_clk_data = kzalloc(struct_size(eq5c_clk_data, hws, EQ5C_NB_CLKS),
+				GFP_KERNEL);
+	if (!eq5c_clk_data) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	eq5c_clk_data->num = EQ5C_NB_CLKS;
+
+	/*
+	 * Mark all clocks as deferred. We register some now and others at
+	 * platform device probe.
+	 */
+	for (i = 0; i < EQ5C_NB_CLKS; i++)
+		eq5c_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
+
+	index_plls = of_property_match_string(np, "reg-names", "plls");
+	index_ospi = of_property_match_string(np, "reg-names", "ospi");
+	if (index_plls < 0 || index_ospi < 0) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	base_plls = of_iomap(np, index_plls);
+	base_ospi = of_iomap(np, index_ospi);
+	if (!base_plls || !base_ospi) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(eq5c_early_plls); i++) {
+		const struct eq5c_pll *pll = &eq5c_early_plls[i];
+		unsigned long mult, div, acc;
+		struct clk_hw *hw;
+		u32 r0, r1;
+
+		r0 = readl(base_plls + pll->reg);
+		r1 = readl(base_plls + pll->reg + sizeof(r0));
+
+		ret = eq5c_pll_parse_registers(r0, r1, &mult, &div, &acc);
+		if (ret) {
+			pr_warn("failed parsing state of %s\n", pll->name);
+			continue;
+		}
+
+		hw = clk_hw_register_fixed_factor_with_accuracy_fwname(NULL,
+				np, pll->name, "ref", 0, mult, div, acc);
+		eq5c_clk_data->hws[pll->index] = hw;
+		if (IS_ERR(hw)) {
+			pr_err("failed registering %s: %ld\n",
+			       pll->name, PTR_ERR(hw));
+		}
+	}
+
+	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, eq5c_clk_data);
+	if (ret) {
+		pr_err("failed registering clk provider: %d\n", ret);
+		goto err;
+	}
+
+	return;
+
+err:
+	kfree(eq5c_clk_data);
+	/* Signal to platform driver probe that we failed init. */
+	eq5c_clk_data = ERR_PTR(ret);
+}
+
+CLK_OF_DECLARE_DRIVER(eq5c, "mobileye,eyeq5-clk", eq5c_init);