Hi Théo,
On Fr, 2024-06-28 at 18:11 +0200, Théo Lebrun wrote:
> Add Mobileye EyeQ reset controller driver, for EyeQ5, EyeQ6L and EyeQ6H
> SoCs. Instances belong to a shared register region called OLB and gets
> spawned as auxiliary device to the platform driver for clock.
>
> There is one OLB instance for EyeQ5 and EyeQ6L. There are seven OLB
> instances on EyeQ6H; three have a reset controller embedded:
> - West and east get handled by the same compatible.
> - Acc (accelerator) is another one.
>
> Each instance vary in the number and types of reset domains.
> Instances with single domain expect a single cell, others two.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
> ---
> drivers/reset/Kconfig | 13 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-eyeq.c | 562 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 576 insertions(+)
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 7112f5932609..14f3f4af0b10 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -66,6 +66,19 @@ config RESET_BRCMSTB_RESCAL
> This enables the RESCAL reset controller for SATA, PCIe0, or PCIe1 on
> BCM7216.
>
> +config RESET_EYEQ
> + bool "Mobileye EyeQ reset controller"
> + depends on AUXILIARY_BUS
This should
select AUXILIARY_BUS
instead.
> + depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST
> + default MACH_EYEQ5 || MACH_EYEQ6H
> + help
> + This enables the Mobileye EyeQ reset controller, used in EyeQ5, EyeQ6L
> + and EyeQ6H SoCs.
> +
> + It has one or more domains, with a varying number of resets in each.
> + Registers are located in a shared register region called OLB. EyeQ6H
> + has multiple reset instances.
> +
> config RESET_GPIO
> tristate "GPIO reset controller"
> help
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index fd8b49fa46fc..a4e6fea29800 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_BCM6345) += reset-bcm6345.o
> obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
> obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
> obj-$(CONFIG_RESET_BRCMSTB_RESCAL) += reset-brcmstb-rescal.o
> +obj-$(CONFIG_RESET_EYEQ) += reset-eyeq.o
> obj-$(CONFIG_RESET_GPIO) += reset-gpio.o
> obj-$(CONFIG_RESET_HSDK) += reset-hsdk.o
> obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
> diff --git a/drivers/reset/reset-eyeq.c b/drivers/reset/reset-eyeq.c
> new file mode 100644
> index 000000000000..9717572ac4b3
> --- /dev/null
> +++ b/drivers/reset/reset-eyeq.c
> @@ -0,0 +1,562 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Reset driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
> + *
> + * Controllers live in a shared register region called OLB. EyeQ5 and EyeQ6L
> + * have a single OLB instance for a single reset controller. EyeQ6H has seven
> + * OLB instances; three host reset controllers.
> + *
> + * Each reset controller has one or more domain. Domains are of a given type
> + * (see enum eqr_domain_type), with a valid offset mask (up to 32 resets per
> + * domain).
> + *
> + * Domain types define expected behavior: one-register-per-reset,
> + * one-bit-per-reset, status detection method, busywait duration, etc.
> + *
> + * We use eqr_ as prefix, as-in "EyeQ Reset", but way shorter.
> + *
> + * Known resets in EyeQ5 domain 0 (type EQR_EYEQ5_SARCR):
> + * 3. CAN0 4. CAN1 5. CAN2 6. SPI0
> + * 7. SPI1 8. SPI2 9. SPI3 10. UART0
> + * 11. UART1 12. UART2 13. I2C0 14. I2C1
> + * 15. I2C2 16. I2C3 17. I2C4 18. TIMER0
> + * 19. TIMER1 20. TIMER2 21. TIMER3 22. TIMER4
> + * 23. WD0 24. EXT0 25. EXT1 26. GPIO
> + * 27. WD1
> + *
> + * Known resets in EyeQ5 domain 1 (type EQR_EYEQ5_ACRP):
> + * 0. VMP0 1. VMP1 2. VMP2 3. VMP3
> + * 4. PMA0 5. PMA1 6. PMAC0 7. PMAC1
> + * 8. MPC0 9. MPC1 10. MPC2 11. MPC3
> + * 12. MPC4
> + *
> + * Known resets in EyeQ5 domain 2 (type EQR_EYEQ5_PCIE):
> + * 0. PCIE0_CORE 1. PCIE0_APB 2. PCIE0_LINK_AXI 3. PCIE0_LINK_MGMT
> + * 4. PCIE0_LINK_HOT 5. PCIE0_LINK_PIPE 6. PCIE1_CORE 7. PCIE1_APB
> + * 8. PCIE1_LINK_AXI 9. PCIE1_LINK_MGMT 10. PCIE1_LINK_HOT 11. PCIE1_LINK_PIPE
> + * 12. MULTIPHY 13. MULTIPHY_APB 15. PCIE0_LINK_MGMT 16. PCIE1_LINK_MGMT
> + * 17. PCIE0_LINK_PM 18. PCIE1_LINK_PM
> + *
> + * Known resets in EyeQ6L domain 0 (type EQR_EYEQ5_SARCR):
> + * 0. SPI0 1. SPI1 2. UART0 3. I2C0
> + * 4. I2C1 5. TIMER0 6. TIMER1 7. TIMER2
> + * 8. TIMER3 9. WD0 10. WD1 11. EXT0
> + * 12. EXT1 13. GPIO
> + *
> + * Known resets in EyeQ6L domain 1 (type EQR_EYEQ5_ACRP):
> + * 0. VMP0 1. VMP1 2. VMP2 3. VMP3
> + * 4. PMA0 5. PMA1 6. PMAC0 7. PMAC1
> + * 8. MPC0 9. MPC1 10. MPC2 11. MPC3
> + * 12. MPC4
> + *
> + * Known resets in EyeQ6H west/east (type EQR_EYEQ6H_SARCR):
> + * 0. CAN 1. SPI0 2. SPI1 3. UART0
> + * 4. UART1 5. I2C0 6. I2C1 7. -hole-
> + * 8. TIMER0 9. TIMER1 10. WD 11. EXT TIMER
> + * 12. GPIO
> + *
> + * Known resets in EyeQ6H acc (type EQR_EYEQ5_ACRP):
> + * 1. XNN0 2. XNN1 3. XNN2 4. XNN3
> + * 5. VMP0 6. VMP1 7. VMP2 8. VMP3
> + * 9. PMA0 10. PMA1 11. MPC0 12. MPC1
> + * 13. MPC2 14. MPC3 15. PERIPH
> + *
> + * Abbreviations:
> + * - PMA: Programmable Macro Array
> + * - MPC: Multi-threaded Processing Clusters
> + * - VMP: Vector Microcode Processors
> + *
> + * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/bug.h>
> +#include <linux/cleanup.h>
> +#include <linux/container_of.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/lockdep.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +/*
> + * A reset ID, as returned by eqr_of_xlate_*(), is a (domain, offset) pair.
> + * Low byte is domain, rest is offset.
> + */
> +#define ID_DOMAIN_MASK GENMASK(7, 0)
> +#define ID_OFFSET_MASK GENMASK(31, 8)
> +
> +enum eqr_domain_type {
> + EQR_EYEQ5_SARCR,
> + EQR_EYEQ5_ACRP,
> + EQR_EYEQ5_PCIE,
> + EQR_EYEQ6H_SARCR,
> +};
> +
> +/*
> + * Domain type EQR_EYEQ5_SARCR register offsets.
> + */
> +#define EQR_EYEQ5_SARCR_REQUEST (0x000)
> +#define EQR_EYEQ5_SARCR_STATUS (0x004)
> +
> +/*
> + * Domain type EQR_EYEQ5_ACRP register masks.
> + * Registers are: base + 4 * offset.
> + */
> +#define EQR_EYEQ5_ACRP_PD_REQ BIT(0)
> +#define EQR_EYEQ5_ACRP_ST_POWER_DOWN BIT(27)
> +#define EQR_EYEQ5_ACRP_ST_ACTIVE BIT(29)
> +
> +/*
> + * Domain type EQR_EYEQ6H_SARCR register offsets.
> + */
> +#define EQR_EYEQ6H_SARCR_RST_REQUEST (0x000)
> +#define EQR_EYEQ6H_SARCR_CLK_STATUS (0x004)
> +#define EQR_EYEQ6H_SARCR_RST_STATUS (0x008)
> +#define EQR_EYEQ6H_SARCR_CLK_REQUEST (0x00C)
> +
> +struct eqr_busy_wait_timings {
> + unsigned long sleep_us;
> + unsigned long timeout_us;
> +};
> +
> +static const struct eqr_busy_wait_timings eqr_timings[] = {
> + [EQR_EYEQ5_SARCR] = {1, 10},
> + [EQR_EYEQ5_ACRP] = {1, 40 * USEC_PER_MSEC}, /* LBIST implies long timeout. */
> + /* EQR_EYEQ5_PCIE does no busy waiting. */
> + [EQR_EYEQ6H_SARCR] = {1, 400},
> +};
> +
> +#define EQR_MAX_DOMAIN_COUNT 3
> +
> +struct eqr_domain_descriptor {
> + enum eqr_domain_type type;
> + u32 valid_mask;
> + unsigned int offset;
> +};
> +
> +struct eqr_match_data {
> + unsigned int domain_count;
> + const struct eqr_domain_descriptor *domains;
> +};
> +
> +struct eqr_private {
> + struct mutex mutexes[EQR_MAX_DOMAIN_COUNT];
Is there any benefit from per-domain mutexes over just a single mutex?
> + void __iomem *base;
> + const struct eqr_match_data *data;
> + struct reset_controller_dev rcdev;
> +};
> +
> +#define rcdev_to_priv(rcdev) container_of(rcdev, struct eqr_private, rcdev)
Please use checkpatch --strict, and ideally mention when you ignore a
warning on purpose. In this case, the macro parameter should named
something else, because the last parameter to container_of must be
"rcdev" verbatim. This only works by accident because the passed
parameter also happens to be called called "rcdev" at all call sites.
> +static u32 eqr_double_readl(void __iomem *addr_a, void __iomem *addr_b,
> + u32 *dest_a, u32 *dest_b)
> +{
> + *dest_a = readl(addr_a);
> + *dest_b = readl(addr_b);
> + return 0; /* read_poll_timeout() op argument must return something. */
> +}
> +
> +static int eqr_busy_wait_locked(struct eqr_private *priv, struct device *dev,
> + u32 domain, u32 offset, bool assert)
> +{
> + enum eqr_domain_type domain_type = priv->data->domains[domain].type;
> + unsigned long sleep_us, timeout_us;
> + u32 val, mask, val0, val1;
> + void __iomem *base, *reg;
> + int ret;
> +
> + lockdep_assert_held(&priv->mutexes[domain]);
> +
> + base = priv->base + priv->data->domains[domain].offset;
> + sleep_us = eqr_timings[domain_type].sleep_us;
> + timeout_us = eqr_timings[domain_type].timeout_us;
You can initialize these at the declaration.
> +
> + switch (domain_type) {
> + case EQR_EYEQ5_SARCR:
> + reg = base + EQR_EYEQ5_SARCR_STATUS;
> + mask = BIT(offset);
> +
> + ret = readl_poll_timeout(reg, val, !(val & mask) == assert,
> + sleep_us, timeout_us);
> + break;
> +
> + case EQR_EYEQ5_ACRP:
> + reg = base + 4 * offset;
> + if (assert)
> + mask = EQR_EYEQ5_ACRP_ST_POWER_DOWN;
> + else
> + mask = EQR_EYEQ5_ACRP_ST_ACTIVE;
> +
> + ret = readl_poll_timeout(reg, val, !!(val & mask),
> + sleep_us, timeout_us);
> + break;
> +
> + case EQR_EYEQ5_PCIE:
> + ret = 0; /* No busy waiting. */
> + break;
> +
> + case EQR_EYEQ6H_SARCR:
> + /*
> + * Wait until both bits change:
> + * readl(base + EQR_EYEQ6H_SARCR_RST_STATUS) & BIT(offset)
> + * readl(base + EQR_EYEQ6H_SARCR_CLK_STATUS) & BIT(offset)
> + */
> + mask = BIT(offset);
> + ret = read_poll_timeout(eqr_double_readl, val,
> + (!(val0 & mask) == assert) &&
> + (!(val1 & mask) == assert),
I'd remove a level of indentation here.
> + sleep_us, timeout_us, false,
> + base + EQR_EYEQ6H_SARCR_RST_STATUS,
> + base + EQR_EYEQ6H_SARCR_CLK_STATUS,
> + &val0, &val1);
Calling these variables something like rst_status and clk_status, would
make this a bit easier to parse.
> + break;
> +
> + default:
> + WARN_ON(1);
> + ret = -EINVAL;
> + break;
> + }
> +
> + if (ret == -ETIMEDOUT)
> + dev_dbg(dev, "%u-%u: timeout\n", domain, offset);
> + return ret;
> +}
> +
> +static void eqr_assert_locked(struct eqr_private *priv, u32 domain, u32 offset)
> +{
> + enum eqr_domain_type domain_type = priv->data->domains[domain].type;
> + void __iomem *base, *reg;
> + u32 val;
> +
> + lockdep_assert_held(&priv->mutexes[domain]);
> +
> + base = priv->base + priv->data->domains[domain].offset;
> +
> + switch (domain_type) {
> + case EQR_EYEQ5_SARCR:
> + reg = base + EQR_EYEQ5_SARCR_REQUEST;
> + writel(readl(reg) & ~BIT(offset), reg);
> + break;
> +
> + case EQR_EYEQ5_ACRP:
> + reg = base + 4 * offset;
> + writel(readl(reg) | EQR_EYEQ5_ACRP_PD_REQ, reg);
> + break;
> +
> + case EQR_EYEQ5_PCIE:
> + writel(readl(base) & ~BIT(offset), base);
> + break;
> +
> + case EQR_EYEQ6H_SARCR:
> + val = readl(base + EQR_EYEQ6H_SARCR_RST_REQUEST);
> + val &= ~BIT(offset);
> + writel(val, base + EQR_EYEQ6H_SARCR_RST_REQUEST);
> + writel(val, base + EQR_EYEQ6H_SARCR_CLK_REQUEST);
This looks peculiar. Why is it ok to write the value read from
RST_REQUEST into CLK_REQUEST?
> + break;
> +
> + default:
> + WARN_ON(1);
> + break;
> + }
> +}
> +
> +static int eqr_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct eqr_private *priv = rcdev_to_priv(rcdev);
> + u32 domain = FIELD_GET(ID_DOMAIN_MASK, id);
> + u32 offset = FIELD_GET(ID_OFFSET_MASK, id);
> +
> + dev_dbg(rcdev->dev, "%u-%u: assert request\n", domain, offset);
> +
> + guard(mutex)(&priv->mutexes[domain]);
> +
> + eqr_assert_locked(priv, domain, offset);
> + return eqr_busy_wait_locked(priv, rcdev->dev, domain, offset, true);
> +}
> +
> +static void eqr_deassert_locked(struct eqr_private *priv, u32 domain,
> + u32 offset)
> +{
> + enum eqr_domain_type domain_type = priv->data->domains[domain].type;
> + void __iomem *base, *reg;
> + u32 val;
> +
> + lockdep_assert_held(&priv->mutexes[domain]);
> +
> + base = priv->base + priv->data->domains[domain].offset;
> +
> + switch (domain_type) {
> + case EQR_EYEQ5_SARCR:
> + reg = base + EQR_EYEQ5_SARCR_REQUEST;
> + writel(readl(reg) | BIT(offset), reg);
> + break;
> +
> + case EQR_EYEQ5_ACRP:
> + reg = base + 4 * offset;
> + writel(readl(reg) & ~EQR_EYEQ5_ACRP_PD_REQ, reg);
> + break;
> +
> + case EQR_EYEQ5_PCIE:
> + writel(readl(base) | BIT(offset), base);
> + break;
> +
> + case EQR_EYEQ6H_SARCR:
> + val = readl(base + EQR_EYEQ6H_SARCR_RST_REQUEST);
> + val |= BIT(offset);
> + writel(val, base + EQR_EYEQ6H_SARCR_RST_REQUEST);
> + writel(val, base + EQR_EYEQ6H_SARCR_CLK_REQUEST);
> + break;
> +
> + default:
> + WARN_ON(1);
> + break;
> + }
> +}
> +
> +static int eqr_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct eqr_private *priv = rcdev_to_priv(rcdev);
> + u32 domain = FIELD_GET(ID_DOMAIN_MASK, id);
> + u32 offset = FIELD_GET(ID_OFFSET_MASK, id);
> +
> + dev_dbg(rcdev->dev, "%u-%u: deassert request\n", domain, offset);
> +
> + guard(mutex)(&priv->mutexes[domain]);
> +
> + eqr_deassert_locked(priv, domain, offset);
> + return eqr_busy_wait_locked(priv, rcdev->dev, domain, offset, false);
> +}
> +
> +static int eqr_status(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + u32 domain = FIELD_GET(ID_DOMAIN_MASK, id);
> + struct eqr_private *priv = rcdev_to_priv(rcdev);
> + enum eqr_domain_type domain_type = priv->data->domains[domain].type;
> + u32 offset = FIELD_GET(ID_OFFSET_MASK, id);
I'd put domain and offset declaration next to each other.
regards
Philipp
