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

[Stephen Boyd <sboyd@kernel.org>]

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 },
> +};