| Message ID | 20240410-mbly-olb-v1-5-335e496d7be3@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add Mobileye EyeQ system controller support (clk, reset, pinctrl) | expand |
Quoting Théo Lebrun (2024-04-10 10:12:34) > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 50af5fc7f570..1eb6e70977a3 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_EYEQ > + bool "Clock driver for the Mobileye EyeQ platform" > + depends on OF || COMPILE_TEST The OF build dependency looks useless as we have the MACH_ dependency below. > + depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST > + 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/clk-eyeq.c b/drivers/clk/clk-eyeq.c > new file mode 100644 > index 000000000000..bb2535010ae6 > --- /dev/null > +++ b/drivers/clk/clk-eyeq.c > @@ -0,0 +1,644 @@ > +// 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(). Is OLB a different DT node? It sounds like maybe this is trying to jam a driver into DT when the OLB node should be a #clock-cells node. > + * > + * We use eqc_ as prefix, as-in "EyeQ Clock", but way shorter. > + * > + * Copyright (C) 2024 Mobileye Vision Technologies Ltd. > + */ > + > +#define pr_fmt(fmt) "clk-eyeq: " fmt > + > +#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/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_init() if PLLs are required > + * early (before platform bus is ready). Store struct eqc_priv inside linked > + * list to pass clock provider from eqc_init() to eqc_probe() and register > + * remaining clocks from platform device probe. > + * > + * Clock provider is NOT created by eqc_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 DEFINE_SPINLOCK(eqc_list_slock); > +static LIST_HEAD(eqc_list); > + > +struct eqc_pll { > + unsigned int index; > + const char *name; > + u32 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; > + const char *resource_name; > + u8 shift; > + u8 width; > +}; > + > +struct eqc_match_data { > + unsigned int early_pll_count; > + const struct eqc_pll *early_plls; > + > + unsigned int pll_count; > + const struct eqc_pll *plls; > + > + unsigned int div_count; > + const struct eqc_div *divs; > +}; > + > +struct eqc_priv { > + struct clk_hw_onecell_data *cells; > + const struct eqc_match_data *data; > + void __iomem *base_plls; > + struct device_node *np; > + struct list_head list; > +}; > + > +static int eqc_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 unsigned int eqc_compute_clock_count(const struct eqc_match_data *data) > +{ > + unsigned int i, nb_clks = 0; > + > + for (i = 0; i < data->early_pll_count; i++) > + if (data->early_plls[i].index >= nb_clks) > + nb_clks = data->early_plls[i].index + 1; > + 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 != data->early_pll_count + data->pll_count + data->div_count); > + > + return nb_clks; > +} > + > +static int eqc_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + void __iomem *div_resources[EQC_MAX_DIV_COUNT]; > + struct device_node *np = dev->of_node; > + const struct eqc_match_data *data; > + struct eqc_priv *priv = NULL; > + struct clk_hw *hw; > + unsigned int i; > + > + data = device_get_match_data(dev); > + if (!data) > + return -ENODEV; > + > + if (data->early_pll_count) { > + /* Device got inited early. Retrieve clock provider from list. */ > + struct eqc_priv *entry; > + > + spin_lock(&eqc_list_slock); > + list_for_each_entry(entry, &eqc_list, list) { > + if (entry->np == np) { > + priv = entry; > + break; > + } > + } > + spin_unlock(&eqc_list_slock); > + > + if (!priv) > + return -ENODEV; This can be a sub-function. > + } else { > + /* Device did NOT get init early. Do it now. */ > + unsigned int nb_clks; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->np = np; > + priv->data = data; > + > + nb_clks = eqc_compute_clock_count(data); > + priv->cells = devm_kzalloc(dev, struct_size(priv->cells, hws, nb_clks), > + GFP_KERNEL); > + if (!priv->cells) > + return -ENOMEM; > + > + priv->cells->num = nb_clks; > + > + /* > + * We expect named resources if divider clocks are present. > + * Else, we only expect one resource. > + */ > + if (data->div_count) > + priv->base_plls = devm_platform_ioremap_resource_byname(pdev, "plls"); > + else > + priv->base_plls = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(priv->base_plls)) > + return PTR_ERR(priv->base_plls); > + } > + > + for (i = 0; i < data->pll_count; i++) { > + const struct eqc_pll *pll = &data->plls[i]; > + unsigned long mult, div, acc; > + u32 r0, r1; > + u64 val; > + int ret; All variables should be declared at the start of the function. Once it becomes "too heavy" you can split it up into smaller functions, that again have all variables declared at the start of the function. > + > + val = readq(priv->base_plls + 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); > + } > + > + BUG_ON(ARRAY_SIZE(div_resources) < data->div_count); Can this be a static assert instead on the arrays these are based on? Put some static_assert() near the match data macros. > + > + for (i = 0; i < data->div_count; i++) { > + const struct eqc_div *div = &data->divs[i]; > + void __iomem *base = NULL; > + struct clk_hw *parent; > + unsigned int j; > + > + /* > + * Multiple divider clocks can request the same resource. Store > + * resource pointers during probe(). For each divider clock, > + * check if previous clocks referenced the same resource name. > + * > + * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS. > + */ > + for (j = 0; j < i; j++) { > + if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) { > + base = div_resources[j]; > + break; > + } > + } > + > + /* Resource is first encountered. */ > + if (!base) { > + base = devm_platform_ioremap_resource_byname(pdev, div->resource_name); > + if (IS_ERR(base)) { > + dev_warn(dev, "failed to iomap resource for %s\n", div->name); > + priv->cells->hws[div->index] = base; > + continue; > + } > + } I don't get this code at all. The driver should simply map the resources because it knows that there's an io resource. I'll look at the binding which is probably wrong and causing the driver to be written this way. > + > + div_resources[i] = base; > + > + parent = priv->cells->hws[div->parent]; > + hw = clk_hw_register_divider_table_parent_hw(dev, div->name, > + parent, 0, base, 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); > + } > + > + /* Clock provider has not been registered by eqc_init(). Do it now. */ > + if (data->early_pll_count == 0) { > + /* When providing a single clock, require no cell. */ > + if (priv->cells->num == 1) > + return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, > + priv->cells->hws); > + else > + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, > + priv->cells); > + } > + > + return 0; > +} > + > +/* 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 = 0x00, }, > + { .index = EQ5C_PLL_PER, .name = "pll-per", .reg64 = 0x30, }, > +}; > + > +static const struct eqc_pll eqc_eyeq5_plls[] = { > + { .index = EQ5C_PLL_VMP, .name = "pll-vmp", .reg64 = 0x08, }, > + { .index = EQ5C_PLL_PMA, .name = "pll-pma", .reg64 = 0x10, }, > + { .index = EQ5C_PLL_VDI, .name = "pll-vdi", .reg64 = 0x18, }, > + { .index = EQ5C_PLL_DDR0, .name = "pll-ddr0", .reg64 = 0x20, }, > + { .index = EQ5C_PLL_PCI, .name = "pll-pci", .reg64 = 0x28, }, > + { .index = EQ5C_PLL_PMAC, .name = "pll-pmac", .reg64 = 0x38, }, > + { .index = EQ5C_PLL_MPC, .name = "pll-mpc", .reg64 = 0x40, }, > + { .index = EQ5C_PLL_DDR1, .name = "pll-ddr1", .reg64 = 0x48, }, > +}; > + > +static const struct eqc_div eqc_eyeq5_divs[] = { > + { > + .index = EQ5C_DIV_OSPI, > + .name = "div-ospi", > + .parent = EQ5C_PLL_PER, > + .resource_name = "ospi", > + .shift = 0, > + .width = 4, > + }, > +}; > + > +static const struct eqc_match_data eqc_eyeq5_match_data = { > + .early_pll_count = ARRAY_SIZE(eqc_eyeq5_early_plls), > + .early_plls = eqc_eyeq5_early_plls, > + > + .pll_count = ARRAY_SIZE(eqc_eyeq5_plls), > + .plls = eqc_eyeq5_plls, > + > + .div_count = ARRAY_SIZE(eqc_eyeq5_divs), > + .divs = eqc_eyeq5_divs, > +}; > + > +static const struct eqc_pll eqc_eyeq6l_plls[] = { > + { .index = EQ6LC_PLL_DDR, .name = "pll-ddr", .reg64 = 0x2C }, > + { .index = EQ6LC_PLL_CPU, .name = "pll-cpu", .reg64 = 0x34 }, /* also acc */ > + { .index = EQ6LC_PLL_PER, .name = "pll-per", .reg64 = 0x3C }, > + { .index = EQ6LC_PLL_VDI, .name = "pll-vdi", .reg64 = 0x44 }, > +}; > + > +static const struct eqc_match_data eqc_eyeq6l_match_data = { > + .pll_count = ARRAY_SIZE(eqc_eyeq6l_plls), > + .plls = eqc_eyeq6l_plls, > +}; > + > +/* Required early for GIC timer. */ > +static const struct eqc_pll eqc_eyeq6h_central_early_plls[] = { > + { .index = 0, .name = "pll-cpu", .reg64 = 0x00 }, > +}; > + > +static const struct eqc_match_data eqc_eyeq6h_central_match_data = { > + .early_pll_count = ARRAY_SIZE(eqc_eyeq6h_central_early_plls), > + .early_plls = eqc_eyeq6h_central_early_plls, > +}; > + > +/* Required early for UART. */ > +static const struct eqc_pll eqc_eyeq6h_west_early_plls[] = { > + { .index = 0, .name = "pll-west", .reg64 = 0x00 }, > +}; > + > +static const struct eqc_match_data eqc_eyeq6h_west_match_data = { > + .early_pll_count = ARRAY_SIZE(eqc_eyeq6h_west_early_plls), > + .early_plls = eqc_eyeq6h_west_early_plls, > +}; > + > +static const struct eqc_pll eqc_eyeq6h_east_plls[] = { > + { .index = 0, .name = "pll-east", .reg64 = 0x00 }, > +}; > + > +static const struct eqc_match_data eqc_eyeq6h_east_match_data = { > + .pll_count = ARRAY_SIZE(eqc_eyeq6h_east_plls), > + .plls = eqc_eyeq6h_east_plls, > +}; > + > +static const struct eqc_pll eqc_eyeq6h_south_plls[] = { > + { .index = EQ6HC_SOUTH_PLL_VDI, .name = "pll-vdi", .reg64 = 0x00 }, > + { .index = EQ6HC_SOUTH_PLL_PCIE, .name = "pll-pcie", .reg64 = 0x08 }, > + { .index = EQ6HC_SOUTH_PLL_PER, .name = "pll-per", .reg64 = 0x10 }, > + { .index = EQ6HC_SOUTH_PLL_ISP, .name = "pll-isp", .reg64 = 0x18 }, > +}; > + > +static const struct eqc_div eqc_eyeq6h_south_divs[] = { > + { > + .index = EQ6HC_SOUTH_DIV_EMMC, > + .name = "div-emmc", > + .parent = EQ6HC_SOUTH_PLL_PER, > + .resource_name = "emmc", > + .shift = 4, > + .width = 4, > + }, > + { > + .index = EQ6HC_SOUTH_DIV_OSPI_REF, > + .name = "div-ospi-ref", > + .parent = EQ6HC_SOUTH_PLL_PER, > + .resource_name = "ospi", > + .shift = 4, > + .width = 4, > + }, > + { > + .index = EQ6HC_SOUTH_DIV_OSPI_SYS, > + .name = "div-ospi-sys", > + .parent = EQ6HC_SOUTH_PLL_PER, > + .resource_name = "ospi", > + .shift = 8, > + .width = 1, > + }, > + { > + .index = EQ6HC_SOUTH_DIV_TSU, > + .name = "div-tsu", > + .parent = EQ6HC_SOUTH_PLL_PCIE, > + .resource_name = "tsu", > + .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 = 0x00 }, > +}; > + > +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 = 0x00 }, > +}; > + > +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 = 0x00 }, > + { .index = EQ6HC_ACC_PLL_VMP, .name = "pll-vmp", .reg64 = 0x10 }, > + { .index = EQ6HC_ACC_PLL_PMA, .name = "pll-pma", .reg64 = 0x1C }, > + { .index = EQ6HC_ACC_PLL_MPC, .name = "pll-mpc", .reg64 = 0x28 }, > + { .index = EQ6HC_ACC_PLL_NOC, .name = "pll-noc", .reg64 = 0x30 }, > +}; > + > +static const struct eqc_match_data eqc_eyeq6h_acc_match_data = { > + .pll_count = ARRAY_SIZE(eqc_eyeq6h_acc_plls), > + .plls = eqc_eyeq6h_acc_plls, > +}; > + > +static const struct of_device_id eqc_match_table[] = { > + { .compatible = "mobileye,eyeq5-clk", .data = &eqc_eyeq5_match_data }, > + { .compatible = "mobileye,eyeq6l-clk", .data = &eqc_eyeq6l_match_data }, > + { .compatible = "mobileye,eyeq6h-central-clk", .data = &eqc_eyeq6h_central_match_data }, > + { .compatible = "mobileye,eyeq6h-west-clk", .data = &eqc_eyeq6h_west_match_data }, > + { .compatible = "mobileye,eyeq6h-east-clk", .data = &eqc_eyeq6h_east_match_data }, > + { .compatible = "mobileye,eyeq6h-south-clk", .data = &eqc_eyeq6h_south_match_data }, > + { .compatible = "mobileye,eyeq6h-ddr0-clk", .data = &eqc_eyeq6h_ddr0_match_data }, > + { .compatible = "mobileye,eyeq6h-ddr1-clk", .data = &eqc_eyeq6h_ddr1_match_data }, > + { .compatible = "mobileye,eyeq6h-acc-clk", .data = &eqc_eyeq6h_acc_match_data }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, eqc_match_table); > + > +static struct platform_driver eqc_driver = { > + .probe = eqc_probe, > + .driver = { > + .name = "clk-eyeq", > + .of_match_table = eqc_match_table, > + }, > +}; > +builtin_platform_driver(eqc_driver); > + > +static void __init eqc_init(struct device_node *np) > +{ > + const struct eqc_match_data *data; > + unsigned int nb_clks = 0; > + struct eqc_priv *priv; > + unsigned int i; > + int ret; > + > + data = of_match_node(eqc_match_table, np)->data; > + > + /* No reason to early init this clock provider. Do it at probe. */ > + if (data->early_pll_count == 0) You can have a different match table for this function then. > + return; > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) { > + ret = -ENOMEM; > + goto err; > + } > + > + priv->np = np; > + priv->data = data; > + > + nb_clks = eqc_compute_clock_count(data); > + 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 non-early clocks as deferred; they'll be registered at platform > + * device probe. > + */ > + for (i = 0; i < data->pll_count; i++) > + priv->cells->hws[data->plls[i].index] = ERR_PTR(-EPROBE_DEFER); > + for (i = 0; i < data->div_count; i++) > + priv->cells->hws[data->divs[i].index] = ERR_PTR(-EPROBE_DEFER); > + > + /* > + * We expect named resources if divider clocks are present. > + * Else, we only expect one resource. > + */ Please avoid named resources. They give the false sense of hope that the binding can re-order the reg property when that can't be done. Instead, just index and know which index to use in the driver.
Hello, On Thu Apr 11, 2024 at 5:22 AM CEST, Stephen Boyd wrote: > Quoting Théo Lebrun (2024-04-10 10:12:34) > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > > index 50af5fc7f570..1eb6e70977a3 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_EYEQ > > + bool "Clock driver for the Mobileye EyeQ platform" > > + depends on OF || COMPILE_TEST > > The OF build dependency looks useless as we have the MACH_ dependency > below. Indeed. I thought explicit dependency could be useful. Will remove. > > + depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST > > + 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/clk-eyeq.c b/drivers/clk/clk-eyeq.c > > new file mode 100644 > > index 000000000000..bb2535010ae6 > > --- /dev/null > > +++ b/drivers/clk/clk-eyeq.c > > @@ -0,0 +1,644 @@ > > +// 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(). > > Is OLB a different DT node? It sounds like maybe this is trying to jam a > driver into DT when the OLB node should be a #clock-cells node. Yes OLB is a different DT node. It looks like on EyeQ5: olb: system-controller@e00000 { compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd"; reg = <0 0xe00000 0x0 0x400>; ranges = <0x0 0x0 0xe00000 0x400>; #address-cells = <1>; #size-cells = <1>; reset: reset-controller@e00000 { compatible = "mobileye,eyeq5-reset"; reg = <0x000 0x0c>, <0x200 0x34>, <0x120 0x04>; reg-names = "d0", "d1", "d2"; #reset-cells = <2>; }; clocks: clock-controller@e0002c { compatible = "mobileye,eyeq5-clk"; reg = <0x02c 0x50>, <0x11c 0x04>; reg-names = "plls", "ospi"; #clock-cells = <1>; clocks = <&xtal>; clock-names = "ref"; }; pinctrl: pinctrl@e000b0 { compatible = "mobileye,eyeq5-pinctrl"; reg = <0x0b0 0x30>; }; }; Keep in mind OLB is a complex beast. On EyeQ5, it hosts something like 150 registers, describing 20ish various hardware features. We have to expose registers to drivers for one-off reads/writes. One example found upstream: I2C speed mode register. Others will be Ethernet, eMMC DMA config, etc. A syscon makes sense. I2C looks like like this for example, look at mobileye,olb. i2c@300000 { compatible = "mobileye,eyeq5-i2c", "arm,primecell"; reg = <0x300000 0x1000>; interrupt-parent = <&gic>; interrupts = <GIC_SHARED 1 IRQ_TYPE_LEVEL_HIGH>; clock-frequency = <400000>; #address-cells = <1>; #size-cells = <0>; clocks = <&i2c_ser_clk>, <&i2c_clk>; clock-names = "i2cclk", "apb_pclk"; mobileye,olb = <&olb 0>; }; See commits 7d4c57abb928 and 1b9a8e8af0d9: i2c: nomadik: support Mobileye EyeQ5 I2C controller dt-bindings: i2c: nomadik: add mobileye,eyeq5-i2c bindings and example > > + > > +static int eqc_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + void __iomem *div_resources[EQC_MAX_DIV_COUNT]; > > + struct device_node *np = dev->of_node; > > + const struct eqc_match_data *data; > > + struct eqc_priv *priv = NULL; > > + struct clk_hw *hw; > > + unsigned int i; > > + > > + data = device_get_match_data(dev); > > + if (!data) > > + return -ENODEV; > > + > > + if (data->early_pll_count) { > > + /* Device got inited early. Retrieve clock provider from list. */ > > + struct eqc_priv *entry; > > + > > + spin_lock(&eqc_list_slock); > > + list_for_each_entry(entry, &eqc_list, list) { > > + if (entry->np == np) { > > + priv = entry; > > + break; > > + } > > + } > > + spin_unlock(&eqc_list_slock); > > + > > + if (!priv) > > + return -ENODEV; > > This can be a sub-function. Will do. [...] > > + for (i = 0; i < data->pll_count; i++) { > > + const struct eqc_pll *pll = &data->plls[i]; > > + unsigned long mult, div, acc; > > + u32 r0, r1; > > + u64 val; > > + int ret; > > All variables should be declared at the start of the function. Once it > becomes "too heavy" you can split it up into smaller functions, that > again have all variables declared at the start of the function. Will avoid variables declarations at start of loops. > > + > > + val = readq(priv->base_plls + 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); > > + } > > + > > + BUG_ON(ARRAY_SIZE(div_resources) < data->div_count); > > Can this be a static assert instead on the arrays these are based on? > Put some static_assert() near the match data macros. I hesitated before sending. Will update. > > + > > + for (i = 0; i < data->div_count; i++) { > > + const struct eqc_div *div = &data->divs[i]; > > + void __iomem *base = NULL; > > + struct clk_hw *parent; > > + unsigned int j; > > + > > + /* > > + * Multiple divider clocks can request the same resource. Store > > + * resource pointers during probe(). For each divider clock, > > + * check if previous clocks referenced the same resource name. > > + * > > + * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS. > > + */ > > + for (j = 0; j < i; j++) { > > + if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) { > > + base = div_resources[j]; > > + break; > > + } > > + } > > + > > + /* Resource is first encountered. */ > > + if (!base) { > > + base = devm_platform_ioremap_resource_byname(pdev, div->resource_name); > > + if (IS_ERR(base)) { > > + dev_warn(dev, "failed to iomap resource for %s\n", div->name); > > + priv->cells->hws[div->index] = base; > > + continue; > > + } > > + } > > I don't get this code at all. The driver should simply map the > resources because it knows that there's an io resource. I'll look at the > binding which is probably wrong and causing the driver to be written > this way. This is here for a single reason: EyeQ6H south OLB has two clocks that live in the same register: - div-ospi-ref, reg offset 0x90, mask GENMASK(9, 8) == 0x300. - div-ospi-sys, reg offset 0x90, mask GENMASK(12, 4) == 0x1FF0. Calling twice devm_platform_ioremap_resource_byname() with the same resource name gives an error. So we need to buffer resources already requested. If there is a simpler & better solution I'd be happy to take it. [...] > > +static void __init eqc_init(struct device_node *np) > > +{ > > + const struct eqc_match_data *data; > > + unsigned int nb_clks = 0; > > + struct eqc_priv *priv; > > + unsigned int i; > > + int ret; > > + > > + data = of_match_node(eqc_match_table, np)->data; > > + > > + /* No reason to early init this clock provider. Do it at probe. */ > > + if (data->early_pll_count == 0) > > You can have a different match table for this function then. Ah, clever. Will do. [...] > > + /* > > + * We expect named resources if divider clocks are present. > > + * Else, we only expect one resource. > > + */ > > Please avoid named resources. They give the false sense of hope that the > binding can re-order the reg property when that can't be done. Instead, > just index and know which index to use in the driver. It is unclear what you mean by not being able to re-order reg property? Are you talking about reg-names being most often defined as items const list and therefore cannot be reordered? Here binding declare things using minItems/maxItems/enum so it can be reordered, looking like: properties: reg: minItems: 2 maxItems: 2 reg-names: minItems: 2 maxItems: 2 items: enum: [ plls, ospi ] If this is not what you are talking about then I rambled about garbage and I'll use indexed resources. Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Quoting Théo Lebrun (2024-04-11 03:46:04) > Hello, > > On Thu Apr 11, 2024 at 5:22 AM CEST, Stephen Boyd wrote: > > Quoting Théo Lebrun (2024-04-10 10:12:34) > > > diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c > > > new file mode 100644 > > > index 000000000000..bb2535010ae6 > > > --- /dev/null > > > +++ b/drivers/clk/clk-eyeq.c > > > @@ -0,0 +1,644 @@ > > > +// 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(). > > > > Is OLB a different DT node? It sounds like maybe this is trying to jam a > > driver into DT when the OLB node should be a #clock-cells node. > > Yes OLB is a different DT node. It looks like on EyeQ5: > > olb: system-controller@e00000 { > compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd"; > reg = <0 0xe00000 0x0 0x400>; > ranges = <0x0 0x0 0xe00000 0x400>; > #address-cells = <1>; > #size-cells = <1>; > > reset: reset-controller@e00000 { > compatible = "mobileye,eyeq5-reset"; > reg = <0x000 0x0c>, <0x200 0x34>, <0x120 0x04>; > reg-names = "d0", "d1", "d2"; > #reset-cells = <2>; > }; > > clocks: clock-controller@e0002c { > compatible = "mobileye,eyeq5-clk"; > reg = <0x02c 0x50>, <0x11c 0x04>; Is this reg property always the same value '0x2c'? > reg-names = "plls", "ospi"; > #clock-cells = <1>; > clocks = <&xtal>; > clock-names = "ref"; > }; > > pinctrl: pinctrl@e000b0 { > compatible = "mobileye,eyeq5-pinctrl"; > reg = <0x0b0 0x30>; > }; > }; > > Keep in mind OLB is a complex beast. On EyeQ5, it hosts something like > 150 registers, describing 20ish various hardware features. We have to > expose registers to drivers for one-off reads/writes. One example found > upstream: I2C speed mode register. Others will be Ethernet, eMMC DMA > config, etc. A syscon makes sense. Syscons are a slippery slope. It makes it easy to give up abstracting the 20ish hardware features and makes the resulting drivers which use the syscon highly platform specific. Regardless of having a syscon or not, the binding should collapse the sub-nodes into the olb node. If that requires making a different compatible for different olb nodes, then that's actually better because there may be some quirk for one of the olbs and not the other and we won't be able to fix that without a compatible string update. It would also make the reg-names property go away, because the sub-functionality drivers would have the register offsets hard-coded as some offset from the base of olb, instead of encoding that in DT. > > I2C looks like like this for example, look at mobileye,olb. > > i2c@300000 { > compatible = "mobileye,eyeq5-i2c", "arm,primecell"; > reg = <0x300000 0x1000>; > interrupt-parent = <&gic>; > interrupts = <GIC_SHARED 1 IRQ_TYPE_LEVEL_HIGH>; > clock-frequency = <400000>; > #address-cells = <1>; > #size-cells = <0>; > clocks = <&i2c_ser_clk>, <&i2c_clk>; > clock-names = "i2cclk", "apb_pclk"; > mobileye,olb = <&olb 0>; > }; > > See commits 7d4c57abb928 and 1b9a8e8af0d9: > i2c: nomadik: support Mobileye EyeQ5 I2C controller > dt-bindings: i2c: nomadik: add mobileye,eyeq5-i2c bindings and example Why isn't i2c speed mode another clk exposed by OLB that rounds to the different rates? > > > > + > > > + for (i = 0; i < data->div_count; i++) { > > > + const struct eqc_div *div = &data->divs[i]; > > > + void __iomem *base = NULL; > > > + struct clk_hw *parent; > > > + unsigned int j; > > > + > > > + /* > > > + * Multiple divider clocks can request the same resource. Store > > > + * resource pointers during probe(). For each divider clock, > > > + * check if previous clocks referenced the same resource name. > > > + * > > > + * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS. > > > + */ > > > + for (j = 0; j < i; j++) { > > > + if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) { > > > + base = div_resources[j]; > > > + break; > > > + } > > > + } > > > + > > > + /* Resource is first encountered. */ > > > + if (!base) { > > > + base = devm_platform_ioremap_resource_byname(pdev, div->resource_name); > > > + if (IS_ERR(base)) { > > > + dev_warn(dev, "failed to iomap resource for %s\n", div->name); > > > + priv->cells->hws[div->index] = base; > > > + continue; > > > + } > > > + } > > > > I don't get this code at all. The driver should simply map the > > resources because it knows that there's an io resource. I'll look at the > > binding which is probably wrong and causing the driver to be written > > this way. > > This is here for a single reason: EyeQ6H south OLB has two clocks that > live in the same register: > > - div-ospi-ref, reg offset 0x90, mask GENMASK(9, 8) == 0x300. > - div-ospi-sys, reg offset 0x90, mask GENMASK(12, 4) == 0x1FF0. > > Calling twice devm_platform_ioremap_resource_byname() with the same > resource name gives an error. So we need to buffer resources already > requested. > > If there is a simpler & better solution I'd be happy to take it. Sure, don't call platform_ioremap_resource() and friends more than once per index. But why is the code written in a way that that is happening? Maybe the driver can ioremap resources, and then register clks for those resources. I suspect the only way of getting here is that the driver is focused on registering clks, and ioremapping resources while registering clks. Don't do that, because then you have to write code to track resources. > > > [...] > > > > + /* > > > + * We expect named resources if divider clocks are present. > > > + * Else, we only expect one resource. > > > + */ > > > > Please avoid named resources. They give the false sense of hope that the > > binding can re-order the reg property when that can't be done. Instead, > > just index and know which index to use in the driver. > > It is unclear what you mean by not being able to re-order reg property? > Are you talking about reg-names being most often defined as items const > list and therefore cannot be reordered? Here binding declare things > using minItems/maxItems/enum so it can be reordered, looking like: Yes, that's wrong. > > properties: > reg: > minItems: 2 > maxItems: 2 > reg-names: > minItems: 2 > maxItems: 2 > items: > enum: [ plls, ospi ] > > If this is not what you are talking about then I rambled about garbage > and I'll use indexed resources. > You cannot reorder strings in a DT binding property after the fact. While the code will keep working if the reg-names elements are re-ordered, the binding will be backwards incompatible, because the reg-names property must have the same order. It can be convenient to use reg-names if you have a long list of reg properties to map, but having two or one elements isn't a very strong argument.
Hello, On Fri Apr 12, 2024 at 7:46 AM CEST, Stephen Boyd wrote: > Quoting Théo Lebrun (2024-04-11 03:46:04) > > On Thu Apr 11, 2024 at 5:22 AM CEST, Stephen Boyd wrote: > > > Quoting Théo Lebrun (2024-04-10 10:12:34) > > > > diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c > > > > new file mode 100644 > > > > index 000000000000..bb2535010ae6 > > > > --- /dev/null > > > > +++ b/drivers/clk/clk-eyeq.c > > > > @@ -0,0 +1,644 @@ > > > > +// 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(). > > > > > > Is OLB a different DT node? It sounds like maybe this is trying to jam a > > > driver into DT when the OLB node should be a #clock-cells node. > > > > Yes OLB is a different DT node. It looks like on EyeQ5: > > > > olb: system-controller@e00000 { > > compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd"; > > reg = <0 0xe00000 0x0 0x400>; > > ranges = <0x0 0x0 0xe00000 0x400>; > > #address-cells = <1>; > > #size-cells = <1>; > > > > reset: reset-controller@e00000 { > > compatible = "mobileye,eyeq5-reset"; > > reg = <0x000 0x0c>, <0x200 0x34>, <0x120 0x04>; > > reg-names = "d0", "d1", "d2"; > > #reset-cells = <2>; > > }; > > > > clocks: clock-controller@e0002c { > > compatible = "mobileye,eyeq5-clk"; > > reg = <0x02c 0x50>, <0x11c 0x04>; > > Is this reg property always the same value '0x2c'? On EyeQ5 yes. On EyeQ6L and EyeQ6H (next revisions, different compatible), no. > > reg-names = "plls", "ospi"; > > #clock-cells = <1>; > > clocks = <&xtal>; > > clock-names = "ref"; > > }; > > > > pinctrl: pinctrl@e000b0 { > > compatible = "mobileye,eyeq5-pinctrl"; > > reg = <0x0b0 0x30>; > > }; > > }; > > > > > > Keep in mind OLB is a complex beast. On EyeQ5, it hosts something like > > 150 registers, describing 20ish various hardware features. We have to > > expose registers to drivers for one-off reads/writes. One example found > > upstream: I2C speed mode register. Others will be Ethernet, eMMC DMA > > config, etc. A syscon makes sense. > > Syscons are a slippery slope. It makes it easy to give up abstracting > the 20ish hardware features and makes the resulting drivers which use > the syscon highly platform specific. I see where you are coming from. In my mind syscons make sense here because most features in the block are only small additions/tweaks to existing blocks. A few examples: - eMMC DMA integration config - eMMC high impedance register - OSPI high impedance register Those cannot be abstracted. The main node is elsewhere and it needs access to those registers. Some other registers however will be able to be abstracted away, as custom clocks for example (eg I'm seeing SGMII PLL control register). That is not the case for all however. > Regardless of having a syscon or not, the binding should collapse the > sub-nodes into the olb node. If that requires making a different > compatible for different olb nodes, then that's actually better because > there may be some quirk for one of the olbs and not the other and we > won't be able to fix that without a compatible string update. It would > also make the reg-names property go away, because the sub-functionality > drivers would have the register offsets hard-coded as some offset from > the base of olb, instead of encoding that in DT. Do you have examples of existing nodes that both are syscons and expose multiple resources (clocks, resets, etc)? I did not envision this as possible which is why I didn't go this route. Also, this would require big changes to dt-bindings currently in v6.9-rc. To be honest, I do not comprehend what aggregating sub-nodes into a single one brings. Here we have one node per feature, each exposing their small feature set, with a single driver handling each. Devicetree is longer but more straight forward, with small resource provider nodes versus a big behemoth handling all of it. Simple and easy: my brain has an easy time grasping it all. What you are proposing sounds more complex, for no clear benefit to my eyes. Can you expand on what that brings? I guess answer has been given elsewhere in depth, so answer might just be a link to the right resource. This is an open question, not reserved to Stephen. Thanks! > > I2C looks like like this for example, look at mobileye,olb. > > > > i2c@300000 { > > compatible = "mobileye,eyeq5-i2c", "arm,primecell"; > > reg = <0x300000 0x1000>; > > interrupt-parent = <&gic>; > > interrupts = <GIC_SHARED 1 IRQ_TYPE_LEVEL_HIGH>; > > clock-frequency = <400000>; > > #address-cells = <1>; > > #size-cells = <0>; > > clocks = <&i2c_ser_clk>, <&i2c_clk>; > > clock-names = "i2cclk", "apb_pclk"; > > mobileye,olb = <&olb 0>; > > }; > > > > See commits 7d4c57abb928 and 1b9a8e8af0d9: > > i2c: nomadik: support Mobileye EyeQ5 I2C controller > > dt-bindings: i2c: nomadik: add mobileye,eyeq5-i2c bindings and example > > Why isn't i2c speed mode another clk exposed by OLB that rounds to the > different rates? It would be a new clock referenced from DT on which we would only do clk_set_rate(priv->clk_freq). How would it fit in the clock tree? It wouldn't have any parent, even though in practice is it parented to an internal Nomadik I2C clock that is not being exposed. I thought it would be an issue to have a clock modeled as having no parent (because its true parent is not exposed) which is a blatant lie. > > > > + for (i = 0; i < data->div_count; i++) { > > > > + const struct eqc_div *div = &data->divs[i]; > > > > + void __iomem *base = NULL; > > > > + struct clk_hw *parent; > > > > + unsigned int j; > > > > + > > > > + /* > > > > + * Multiple divider clocks can request the same resource. Store > > > > + * resource pointers during probe(). For each divider clock, > > > > + * check if previous clocks referenced the same resource name. > > > > + * > > > > + * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS. > > > > + */ > > > > + for (j = 0; j < i; j++) { > > > > + if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) { > > > > + base = div_resources[j]; > > > > + break; > > > > + } > > > > + } > > > > + > > > > + /* Resource is first encountered. */ > > > > + if (!base) { > > > > + base = devm_platform_ioremap_resource_byname(pdev, div->resource_name); > > > > + if (IS_ERR(base)) { > > > > + dev_warn(dev, "failed to iomap resource for %s\n", div->name); > > > > + priv->cells->hws[div->index] = base; > > > > + continue; > > > > + } > > > > + } > > > > > > I don't get this code at all. The driver should simply map the > > > resources because it knows that there's an io resource. I'll look at the > > > binding which is probably wrong and causing the driver to be written > > > this way. > > > > This is here for a single reason: EyeQ6H south OLB has two clocks that > > live in the same register: > > > > - div-ospi-ref, reg offset 0x90, mask GENMASK(9, 8) == 0x300. > > - div-ospi-sys, reg offset 0x90, mask GENMASK(12, 4) == 0x1FF0. > > > > Calling twice devm_platform_ioremap_resource_byname() with the same > > resource name gives an error. So we need to buffer resources already > > requested. > > > > If there is a simpler & better solution I'd be happy to take it. > > Sure, don't call platform_ioremap_resource() and friends more than once > per index. But why is the code written in a way that that is happening? > Maybe the driver can ioremap resources, and then register clks for those > resources. I suspect the only way of getting here is that the driver is > focused on registering clks, and ioremapping resources while registering > clks. Don't do that, because then you have to write code to track > resources. So code would look like (removing reg-names as it brings nothing, and error checking for brevity): void __iomem *div_resources[EQC_MAX_DIV_COUNT]; unsigned int max_div_resource_index = 0; const struct eqc_div *div; struct clk_hw *parent; void __iomem *base; struct clk_hw *hw; // Learn what resources should be acquired for (i = 0; i < data->div_count; i++) { div = &data->divs[i]; if (div->resource_index > max_div_resource_index) max_div_resource_index = div->resource_index; } // Grab resources (starting at 1 because 0 is for PLLs) for (i = 1; i < max_div_resource_index; i++) { div_resources[i] = devm_platform_ioremap_resource(pdev, i); // TODO: error checking } // Register clocks for (i = 0; i < data->div_count; i++) { div = &data->divs[i]; base = div_resources[div->resource_index]; parent = priv->cells->hws[div->parent]; hw = clk_hw_register_divider_table_parent_hw(dev, div->name, parent, 0, base, div->shift, div->width, CLK_DIVIDER_EVEN_INTEGERS, NULL, NULL); priv->cells->hws[div->index] = hw; // TODO: error checking } > > > > > > > [...] > > > > > > + /* > > > > + * We expect named resources if divider clocks are present. > > > > + * Else, we only expect one resource. > > > > + */ > > > > > > Please avoid named resources. They give the false sense of hope that the > > > binding can re-order the reg property when that can't be done. Instead, > > > just index and know which index to use in the driver. > > > > It is unclear what you mean by not being able to re-order reg property? > > Are you talking about reg-names being most often defined as items const > > list and therefore cannot be reordered? Here binding declare things > > using minItems/maxItems/enum so it can be reordered, looking like: > > Yes, that's wrong. > > > > > properties: > > reg: > > minItems: 2 > > maxItems: 2 > > reg-names: > > minItems: 2 > > maxItems: 2 > > items: > > enum: [ plls, ospi ] > > > > If this is not what you are talking about then I rambled about garbage > > and I'll use indexed resources. > > > > You cannot reorder strings in a DT binding property after the fact. > While the code will keep working if the reg-names elements are > re-ordered, the binding will be backwards incompatible, because the > reg-names property must have the same order. It can be convenient to use > reg-names if you have a long list of reg properties to map, but having > two or one elements isn't a very strong argument. I didn't know that beforehands. I completely get what you mean now. Why it is that way is unclear to me but whatever. It is even documented: https://elixir.bootlin.com/linux/v6.8.6/source/Documentation/devicetree/bindings/writing-bindings.rst#L64 Thanks Stephen, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
diff --git a/MAINTAINERS b/MAINTAINERS index 42553da10be9..33168ebf3cc5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14932,6 +14932,7 @@ F: Documentation/devicetree/bindings/soc/mobileye/ F: arch/mips/boot/dts/mobileye/ F: arch/mips/configs/eyeq5_defconfig F: arch/mips/mobileye/board-epm5.its.S +F: drivers/clk/clk-eyeq5.c F: include/dt-bindings/clock/mobileye,eyeq5-clk.h MODULE SUPPORT diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 50af5fc7f570..1eb6e70977a3 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_EYEQ + bool "Clock driver for the Mobileye EyeQ platform" + depends on OF || COMPILE_TEST + depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST + 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 14fa8d4ecc1f..52de92309aa8 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_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 000000000000..bb2535010ae6 --- /dev/null +++ b/drivers/clk/clk-eyeq.c @@ -0,0 +1,644 @@ +// 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. + */ + +#define pr_fmt(fmt) "clk-eyeq: " fmt + +#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/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_init() if PLLs are required + * early (before platform bus is ready). Store struct eqc_priv inside linked + * list to pass clock provider from eqc_init() to eqc_probe() and register + * remaining clocks from platform device probe. + * + * Clock provider is NOT created by eqc_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 DEFINE_SPINLOCK(eqc_list_slock); +static LIST_HEAD(eqc_list); + +struct eqc_pll { + unsigned int index; + const char *name; + u32 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; + const char *resource_name; + u8 shift; + u8 width; +}; + +struct eqc_match_data { + unsigned int early_pll_count; + const struct eqc_pll *early_plls; + + unsigned int pll_count; + const struct eqc_pll *plls; + + unsigned int div_count; + const struct eqc_div *divs; +}; + +struct eqc_priv { + struct clk_hw_onecell_data *cells; + const struct eqc_match_data *data; + void __iomem *base_plls; + struct device_node *np; + struct list_head list; +}; + +static int eqc_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 unsigned int eqc_compute_clock_count(const struct eqc_match_data *data) +{ + unsigned int i, nb_clks = 0; + + for (i = 0; i < data->early_pll_count; i++) + if (data->early_plls[i].index >= nb_clks) + nb_clks = data->early_plls[i].index + 1; + 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 != data->early_pll_count + data->pll_count + data->div_count); + + return nb_clks; +} + +static int eqc_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + void __iomem *div_resources[EQC_MAX_DIV_COUNT]; + struct device_node *np = dev->of_node; + const struct eqc_match_data *data; + struct eqc_priv *priv = NULL; + struct clk_hw *hw; + unsigned int i; + + data = device_get_match_data(dev); + if (!data) + return -ENODEV; + + if (data->early_pll_count) { + /* Device got inited early. Retrieve clock provider from list. */ + struct eqc_priv *entry; + + spin_lock(&eqc_list_slock); + list_for_each_entry(entry, &eqc_list, list) { + if (entry->np == np) { + priv = entry; + break; + } + } + spin_unlock(&eqc_list_slock); + + if (!priv) + return -ENODEV; + } else { + /* Device did NOT get init early. Do it now. */ + unsigned int nb_clks; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->np = np; + priv->data = data; + + nb_clks = eqc_compute_clock_count(data); + priv->cells = devm_kzalloc(dev, struct_size(priv->cells, hws, nb_clks), + GFP_KERNEL); + if (!priv->cells) + return -ENOMEM; + + priv->cells->num = nb_clks; + + /* + * We expect named resources if divider clocks are present. + * Else, we only expect one resource. + */ + if (data->div_count) + priv->base_plls = devm_platform_ioremap_resource_byname(pdev, "plls"); + else + priv->base_plls = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(priv->base_plls)) + return PTR_ERR(priv->base_plls); + } + + for (i = 0; i < data->pll_count; i++) { + const struct eqc_pll *pll = &data->plls[i]; + unsigned long mult, div, acc; + u32 r0, r1; + u64 val; + int ret; + + val = readq(priv->base_plls + 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); + } + + BUG_ON(ARRAY_SIZE(div_resources) < data->div_count); + + for (i = 0; i < data->div_count; i++) { + const struct eqc_div *div = &data->divs[i]; + void __iomem *base = NULL; + struct clk_hw *parent; + unsigned int j; + + /* + * Multiple divider clocks can request the same resource. Store + * resource pointers during probe(). For each divider clock, + * check if previous clocks referenced the same resource name. + * + * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS. + */ + for (j = 0; j < i; j++) { + if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) { + base = div_resources[j]; + break; + } + } + + /* Resource is first encountered. */ + if (!base) { + base = devm_platform_ioremap_resource_byname(pdev, div->resource_name); + if (IS_ERR(base)) { + dev_warn(dev, "failed to iomap resource for %s\n", div->name); + priv->cells->hws[div->index] = base; + continue; + } + } + + div_resources[i] = base; + + parent = priv->cells->hws[div->parent]; + hw = clk_hw_register_divider_table_parent_hw(dev, div->name, + parent, 0, base, 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); + } + + /* Clock provider has not been registered by eqc_init(). Do it now. */ + if (data->early_pll_count == 0) { + /* When providing a single clock, require no cell. */ + if (priv->cells->num == 1) + return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, + priv->cells->hws); + else + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, + priv->cells); + } + + return 0; +} + +/* 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 = 0x00, }, + { .index = EQ5C_PLL_PER, .name = "pll-per", .reg64 = 0x30, }, +}; + +static const struct eqc_pll eqc_eyeq5_plls[] = { + { .index = EQ5C_PLL_VMP, .name = "pll-vmp", .reg64 = 0x08, }, + { .index = EQ5C_PLL_PMA, .name = "pll-pma", .reg64 = 0x10, }, + { .index = EQ5C_PLL_VDI, .name = "pll-vdi", .reg64 = 0x18, }, + { .index = EQ5C_PLL_DDR0, .name = "pll-ddr0", .reg64 = 0x20, }, + { .index = EQ5C_PLL_PCI, .name = "pll-pci", .reg64 = 0x28, }, + { .index = EQ5C_PLL_PMAC, .name = "pll-pmac", .reg64 = 0x38, }, + { .index = EQ5C_PLL_MPC, .name = "pll-mpc", .reg64 = 0x40, }, + { .index = EQ5C_PLL_DDR1, .name = "pll-ddr1", .reg64 = 0x48, }, +}; + +static const struct eqc_div eqc_eyeq5_divs[] = { + { + .index = EQ5C_DIV_OSPI, + .name = "div-ospi", + .parent = EQ5C_PLL_PER, + .resource_name = "ospi", + .shift = 0, + .width = 4, + }, +}; + +static const struct eqc_match_data eqc_eyeq5_match_data = { + .early_pll_count = ARRAY_SIZE(eqc_eyeq5_early_plls), + .early_plls = eqc_eyeq5_early_plls, + + .pll_count = ARRAY_SIZE(eqc_eyeq5_plls), + .plls = eqc_eyeq5_plls, + + .div_count = ARRAY_SIZE(eqc_eyeq5_divs), + .divs = eqc_eyeq5_divs, +}; + +static const struct eqc_pll eqc_eyeq6l_plls[] = { + { .index = EQ6LC_PLL_DDR, .name = "pll-ddr", .reg64 = 0x2C }, + { .index = EQ6LC_PLL_CPU, .name = "pll-cpu", .reg64 = 0x34 }, /* also acc */ + { .index = EQ6LC_PLL_PER, .name = "pll-per", .reg64 = 0x3C }, + { .index = EQ6LC_PLL_VDI, .name = "pll-vdi", .reg64 = 0x44 }, +}; + +static const struct eqc_match_data eqc_eyeq6l_match_data = { + .pll_count = ARRAY_SIZE(eqc_eyeq6l_plls), + .plls = eqc_eyeq6l_plls, +}; + +/* Required early for GIC timer. */ +static const struct eqc_pll eqc_eyeq6h_central_early_plls[] = { + { .index = 0, .name = "pll-cpu", .reg64 = 0x00 }, +}; + +static const struct eqc_match_data eqc_eyeq6h_central_match_data = { + .early_pll_count = ARRAY_SIZE(eqc_eyeq6h_central_early_plls), + .early_plls = eqc_eyeq6h_central_early_plls, +}; + +/* Required early for UART. */ +static const struct eqc_pll eqc_eyeq6h_west_early_plls[] = { + { .index = 0, .name = "pll-west", .reg64 = 0x00 }, +}; + +static const struct eqc_match_data eqc_eyeq6h_west_match_data = { + .early_pll_count = ARRAY_SIZE(eqc_eyeq6h_west_early_plls), + .early_plls = eqc_eyeq6h_west_early_plls, +}; + +static const struct eqc_pll eqc_eyeq6h_east_plls[] = { + { .index = 0, .name = "pll-east", .reg64 = 0x00 }, +}; + +static const struct eqc_match_data eqc_eyeq6h_east_match_data = { + .pll_count = ARRAY_SIZE(eqc_eyeq6h_east_plls), + .plls = eqc_eyeq6h_east_plls, +}; + +static const struct eqc_pll eqc_eyeq6h_south_plls[] = { + { .index = EQ6HC_SOUTH_PLL_VDI, .name = "pll-vdi", .reg64 = 0x00 }, + { .index = EQ6HC_SOUTH_PLL_PCIE, .name = "pll-pcie", .reg64 = 0x08 }, + { .index = EQ6HC_SOUTH_PLL_PER, .name = "pll-per", .reg64 = 0x10 }, + { .index = EQ6HC_SOUTH_PLL_ISP, .name = "pll-isp", .reg64 = 0x18 }, +}; + +static const struct eqc_div eqc_eyeq6h_south_divs[] = { + { + .index = EQ6HC_SOUTH_DIV_EMMC, + .name = "div-emmc", + .parent = EQ6HC_SOUTH_PLL_PER, + .resource_name = "emmc", + .shift = 4, + .width = 4, + }, + { + .index = EQ6HC_SOUTH_DIV_OSPI_REF, + .name = "div-ospi-ref", + .parent = EQ6HC_SOUTH_PLL_PER, + .resource_name = "ospi", + .shift = 4, + .width = 4, + }, + { + .index = EQ6HC_SOUTH_DIV_OSPI_SYS, + .name = "div-ospi-sys", + .parent = EQ6HC_SOUTH_PLL_PER, + .resource_name = "ospi", + .shift = 8, + .width = 1, + }, + { + .index = EQ6HC_SOUTH_DIV_TSU, + .name = "div-tsu", + .parent = EQ6HC_SOUTH_PLL_PCIE, + .resource_name = "tsu", + .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 = 0x00 }, +}; + +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 = 0x00 }, +}; + +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 = 0x00 }, + { .index = EQ6HC_ACC_PLL_VMP, .name = "pll-vmp", .reg64 = 0x10 }, + { .index = EQ6HC_ACC_PLL_PMA, .name = "pll-pma", .reg64 = 0x1C }, + { .index = EQ6HC_ACC_PLL_MPC, .name = "pll-mpc", .reg64 = 0x28 }, + { .index = EQ6HC_ACC_PLL_NOC, .name = "pll-noc", .reg64 = 0x30 }, +}; + +static const struct eqc_match_data eqc_eyeq6h_acc_match_data = { + .pll_count = ARRAY_SIZE(eqc_eyeq6h_acc_plls), + .plls = eqc_eyeq6h_acc_plls, +}; + +static const struct of_device_id eqc_match_table[] = { + { .compatible = "mobileye,eyeq5-clk", .data = &eqc_eyeq5_match_data }, + { .compatible = "mobileye,eyeq6l-clk", .data = &eqc_eyeq6l_match_data }, + { .compatible = "mobileye,eyeq6h-central-clk", .data = &eqc_eyeq6h_central_match_data }, + { .compatible = "mobileye,eyeq6h-west-clk", .data = &eqc_eyeq6h_west_match_data }, + { .compatible = "mobileye,eyeq6h-east-clk", .data = &eqc_eyeq6h_east_match_data }, + { .compatible = "mobileye,eyeq6h-south-clk", .data = &eqc_eyeq6h_south_match_data }, + { .compatible = "mobileye,eyeq6h-ddr0-clk", .data = &eqc_eyeq6h_ddr0_match_data }, + { .compatible = "mobileye,eyeq6h-ddr1-clk", .data = &eqc_eyeq6h_ddr1_match_data }, + { .compatible = "mobileye,eyeq6h-acc-clk", .data = &eqc_eyeq6h_acc_match_data }, + {} +}; +MODULE_DEVICE_TABLE(of, eqc_match_table); + +static struct platform_driver eqc_driver = { + .probe = eqc_probe, + .driver = { + .name = "clk-eyeq", + .of_match_table = eqc_match_table, + }, +}; +builtin_platform_driver(eqc_driver); + +static void __init eqc_init(struct device_node *np) +{ + const struct eqc_match_data *data; + unsigned int nb_clks = 0; + struct eqc_priv *priv; + unsigned int i; + int ret; + + data = of_match_node(eqc_match_table, np)->data; + + /* No reason to early init this clock provider. Do it at probe. */ + if (data->early_pll_count == 0) + return; + + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) { + ret = -ENOMEM; + goto err; + } + + priv->np = np; + priv->data = data; + + nb_clks = eqc_compute_clock_count(data); + 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 non-early clocks as deferred; they'll be registered at platform + * device probe. + */ + for (i = 0; i < data->pll_count; i++) + priv->cells->hws[data->plls[i].index] = ERR_PTR(-EPROBE_DEFER); + for (i = 0; i < data->div_count; i++) + priv->cells->hws[data->divs[i].index] = ERR_PTR(-EPROBE_DEFER); + + /* + * We expect named resources if divider clocks are present. + * Else, we only expect one resource. + */ + if (data->div_count) + ret = of_property_match_string(np, "reg-names", "plls"); + else + ret = 0; + if (ret < 0) + goto err; + + priv->base_plls = of_iomap(np, ret); + if (!priv->base_plls) { + ret = -ENODEV; + goto err; + } + + for (i = 0; i < data->early_pll_count; i++) { + const struct eqc_pll *pll = &data->early_plls[i]; + unsigned long mult, div, acc; + struct clk_hw *hw; + u32 r0, r1; + u64 val; + + val = readq(priv->base_plls + 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); + 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; + } + + spin_lock(&eqc_list_slock); + list_add_tail(&priv->list, &eqc_list); + spin_unlock(&eqc_list_slock); + + 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 < data->early_pll_count; i++) { + const struct eqc_pll *pll = &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); +} + +CLK_OF_DECLARE_DRIVER(eqc_eyeq5, "mobileye,eyeq5-clk", eqc_init); +CLK_OF_DECLARE_DRIVER(eqc_eyeq6h_central, "mobileye,eyeq6h-central-clk", eqc_init); +CLK_OF_DECLARE_DRIVER(eqc_eyeq6h_west, "mobileye,eyeq6h-west-clk", eqc_init);
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_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. Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> --- MAINTAINERS | 1 + drivers/clk/Kconfig | 11 + drivers/clk/Makefile | 1 + drivers/clk/clk-eyeq.c | 644 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 657 insertions(+)