On 23/01/2024 19:46, Théo Lebrun wrote:
> Add the Mobileye EyeQ5 reset controller driver. It belongs to a syscon
> region called OLB. It might grow to add later support of other
> platforms from Mobileye.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
> ---
> MAINTAINERS | 1 +
> drivers/reset/Kconfig | 12 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-eyeq5.c | 383 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 397 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3ea96ab7d2b8..dd3b5834386f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14794,6 +14794,7 @@ 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: drivers/reset/reset-eyeq5.c
> F: include/dt-bindings/clock/mobileye,eyeq5-clk.h
> F: include/dt-bindings/soc/mobileye,eyeq5.h
>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index ccd59ddd7610..80bfde54c076 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -66,6 +66,18 @@ config RESET_BRCMSTB_RESCAL
> This enables the RESCAL reset controller for SATA, PCIe0, or PCIe1 on
> BCM7216.
>
> +config RESET_EYEQ5
> + bool "Mobileye EyeQ5 reset controller"
> + depends on MFD_SYSCON
> + depends on MACH_EYEQ5 || COMPILE_TEST
> + default MACH_EYEQ5
> + help
> + This enables the Mobileye EyeQ5 reset controller.
> +
> + It has three domains, with a varying number of resets in each of them.
> + Registers are located in a shared register region called OLB accessed
> + through a syscon & regmap.
> +
> config RESET_HSDK
> bool "Synopsys HSDK Reset Driver"
> depends on HAS_IOMEM
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 8270da8a4baa..4fabe0070390 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_EYEQ5) += reset-eyeq5.o
> obj-$(CONFIG_RESET_HSDK) += reset-hsdk.o
> obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
> obj-$(CONFIG_RESET_INTEL_GW) += reset-intel-gw.o
> diff --git a/drivers/reset/reset-eyeq5.c b/drivers/reset/reset-eyeq5.c
> new file mode 100644
> index 000000000000..2217e42e140b
> --- /dev/null
> +++ b/drivers/reset/reset-eyeq5.c
> @@ -0,0 +1,383 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Reset driver for the Mobileye EyeQ5 platform.
> + *
> + * The registers are located in a syscon region called OLB. We handle three
> + * reset domains. Domains 0 and 2 look similar in that they both use one bit
> + * per reset line. Domain 1 has a register per reset.
> + *
> + * We busy-wait after updating a reset in domains 0 or 1. The reason is hardware
> + * logic built-in self-test (LBIST) that might be enabled.
> + *
> + * We use eq5r_ as prefix, as-in "EyeQ5 Reset", but way shorter.
> + *
> + * Known resets in domain 0:
> + * 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 domain 1:
> + * 0. VMP0 (Vector Microcode Processors)
> + * 1. VMP1
> + * 2. VMP2
> + * 3. VMP3
> + * 4. PMA0 (Programmable Macro Array)
> + * 5. PMA1
> + * 6. PMAC0
> + * 7. PMAC1
> + * 8. MPC0 (Multi-threaded Processing Clusters)
> + * 9. MPC1
> + *
> + * Known resets in domain 2:
> + * 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
> + *
> + * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
> + */
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +
> +/* Offsets into the OLB region as well as masks for domain 1 registers. */
> +#define EQ5R_OLB_SARCR0 (0x004)
> +#define EQ5R_OLB_SARCR1 (0x008)
> +#define EQ5R_OLB_PCIE_GP (0x120)
> +#define EQ5R_OLB_ACRP_REG(n) (0x200 + 4 * (n)) // n=0..12
> +#define EQ5R_OLB_ACRP_PD_REQ BIT(0)
> +#define EQ5R_OLB_ACRP_ST_POWER_DOWN BIT(27)
> +#define EQ5R_OLB_ACRP_ST_ACTIVE BIT(29)
> +
> +/* Vendor-provided values. D1 has a long timeout because of LBIST. */
> +#define D0_TIMEOUT_POLL 10
> +#define D1_TIMEOUT_POLL 40000
> +
> +/*
> + * Masks for valid reset lines in each domain. This array is also used to get
> + * the domain and reset counts.
> + */
> +static const u32 eq5r_valid_masks[] = { 0x0FFFFFF8, 0x00001FFF, 0x0007BFFF };
> +
> +#define EQ5R_DOMAIN_COUNT ARRAY_SIZE(eq5r_valid_masks)
> +
> +struct eq5r_private {
> + struct mutex mutexes[EQ5R_DOMAIN_COUNT]; /* We serialize all reset operations. */
> + struct regmap *olb; /* Writes go to a syscon regmap. */
> + struct reset_controller_dev rcdev;
> +};
> +
> +static int _eq5r_busy_wait(struct eq5r_private *priv, struct device *dev,
> + u32 domain, u32 offset, bool assert)
> +{
> + unsigned int val, mask;
> + int i;
> +
> + lockdep_assert_held(&priv->mutexes[domain]);
> +
> + switch (domain) {
> + case 0:
> + for (i = 0; i < D0_TIMEOUT_POLL; i++) {
> + regmap_read(priv->olb, EQ5R_OLB_SARCR1, &val);
> + val = !(val & BIT(offset));
> + if (val == assert)
> + return 0;
> + __udelay(1);
What is even "__udelay"? It is the first use in drivers. Please use
common methods, like fsleep or udelay... but actually you should rather
use regmap_read_poll_timeout() or some variants instead of open-coding it.
> + }
> + break;
> + case 1:
> + mask = assert ? EQ5R_OLB_ACRP_ST_POWER_DOWN : EQ5R_OLB_ACRP_ST_ACTIVE;
> + for (i = 0; i < D1_TIMEOUT_POLL; i++) {
> + regmap_read(priv->olb, EQ5R_OLB_ACRP_REG(offset), &val);
> + if (val & mask)
> + return 0;
> + __udelay(1);
> + }
> + break;
> + case 2:
> + return 0; /* No busy waiting for domain 2. */
> + default:
> + WARN_ON(1);
> + return -EINVAL;
> + }
> +
> + dev_dbg(dev, "%u-%u: timeout\n", domain, offset);
> + return -ETIMEDOUT;
> +}
> +
> +static void _eq5r_assert(struct eq5r_private *priv, u32 domain, u32 offset)
Drop leading _ and name the function in some informative way.
> +{
> + lockdep_assert_held(&priv->mutexes[domain]);
> +
> + switch (domain) {
> + case 0:
> + regmap_clear_bits(priv->olb, EQ5R_OLB_SARCR0, BIT(offset));
> + break;
> + case 1:
> + regmap_set_bits(priv->olb, EQ5R_OLB_ACRP_REG(offset),
> + EQ5R_OLB_ACRP_PD_REQ);
> + break;
> + case 2:
> + regmap_clear_bits(priv->olb, EQ5R_OLB_PCIE_GP, BIT(offset));
> + break;
> + default:
> + WARN_ON(1);
> + }
> +}
> +
> +static int eq5r_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct eq5r_private *priv = dev_get_drvdata(rcdev->dev);
> + u32 offset = id & GENMASK(7, 0);
> + u32 domain = id >> 8;
> + int ret;
> +
> + if (WARN_ON(domain >= EQ5R_DOMAIN_COUNT))
> + return -EINVAL;
> +
> + dev_dbg(rcdev->dev, "%u-%u: assert request\n", domain, offset);
> +
> + mutex_lock(&priv->mutexes[domain]);
> + _eq5r_assert(priv, domain, offset);
> + ret = _eq5r_busy_wait(priv, rcdev->dev, domain, offset, true);
> + mutex_unlock(&priv->mutexes[domain]);
> +
> + return ret;
> +}
> +
> +static void _eq5r_deassert(struct eq5r_private *priv, u32 domain, u32 offset)
> +{
> + lockdep_assert_held(&priv->mutexes[domain]);
> +
> + switch (domain) {
> + case 0:
> + regmap_set_bits(priv->olb, EQ5R_OLB_SARCR0, BIT(offset));
> + break;
> + case 1:
> + regmap_clear_bits(priv->olb, EQ5R_OLB_ACRP_REG(offset),
> + EQ5R_OLB_ACRP_PD_REQ);
> + break;
> + case 2:
> + regmap_set_bits(priv->olb, EQ5R_OLB_PCIE_GP, BIT(offset));
> + break;
> + default:
> + WARN_ON(1);
> + }
> +}
> +
> +static int eq5r_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct eq5r_private *priv = dev_get_drvdata(rcdev->dev);
> + u32 offset = id & GENMASK(7, 0);
> + u32 domain = id >> 8;
> + int ret;
> +
> + if (WARN_ON(domain >= EQ5R_DOMAIN_COUNT))
> + return -EINVAL;
> +
> + dev_dbg(rcdev->dev, "%u-%u: deassert request\n", domain, offset);
> +
> + mutex_lock(&priv->mutexes[domain]);
> + _eq5r_deassert(priv, domain, offset);
> + ret = _eq5r_busy_wait(priv, rcdev->dev, domain, offset, false);
> + mutex_unlock(&priv->mutexes[domain]);
> +
> + return ret;
> +}
> +
> +static int eq5r_reset(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct device *dev = rcdev->dev;
> + struct eq5r_private *priv = dev_get_drvdata(dev);
> + u32 offset = id & GENMASK(7, 0);
> + u32 domain = id >> 8;
> + int ret;
> +
> + if (WARN_ON(domain >= EQ5R_DOMAIN_COUNT))
> + return -EINVAL;
> +
> + dev_dbg(dev, "%u-%u: reset request\n", domain, offset);
> +
> + mutex_lock(&priv->mutexes[domain]);
> +
> + _eq5r_assert(priv, domain, offset);
> + ret = _eq5r_busy_wait(priv, dev, domain, offset, true);
> + if (ret) /* don't let an error disappear silently */
> + dev_warn(dev, "%u-%u: reset assert failed: %d\n",
> + domain, offset, ret);
> +
> + _eq5r_deassert(priv, domain, offset);
> + ret = _eq5r_busy_wait(priv, dev, domain, offset, false);
> +
> + mutex_unlock(&priv->mutexes[domain]);
> +
> + return ret;
> +}
> +
> +static int eq5r_status(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct eq5r_private *priv = dev_get_drvdata(rcdev->dev);
> + u32 offset = id & GENMASK(7, 0);
> + u32 domain = id >> 8;
> + unsigned int val;
> + int ret;
> +
> + if (WARN_ON(domain >= EQ5R_DOMAIN_COUNT))
> + return -EINVAL;
> +
> + dev_dbg(rcdev->dev, "%u-%u: status request\n", domain, offset);
> +
> + mutex_lock(&priv->mutexes[domain]);
> +
> + switch (domain) {
> + case 0:
> + regmap_read(priv->olb, EQ5R_OLB_SARCR1, &val);
> + ret = !(val & BIT(offset));
> + break;
> + case 1:
> + regmap_read(priv->olb, EQ5R_OLB_ACRP_REG(offset), &val);
> + ret = !(val & EQ5R_OLB_ACRP_ST_ACTIVE);
> + break;
> + case 2:
> + regmap_read(priv->olb, EQ5R_OLB_PCIE_GP, &val);
> + ret = !(val & BIT(offset));
> + break;
> + }
> +
> + mutex_unlock(&priv->mutexes[domain]);
> +
> + return ret;
> +}
> +
> +static const struct reset_control_ops eq5r_ops = {
> + .reset = eq5r_reset,
> + .assert = eq5r_assert,
> + .deassert = eq5r_deassert,
> + .status = eq5r_status,
> +};
> +
> +static int eq5r_of_xlate(struct reset_controller_dev *rcdev,
> + const struct of_phandle_args *reset_spec)
> +{
> + u32 domain, offset;
> +
> + if (WARN_ON(reset_spec->args_count != 2))
> + return -EINVAL;
> +
> + domain = reset_spec->args[0];
> + offset = reset_spec->args[1];
> +
> + if (domain >= EQ5R_DOMAIN_COUNT || offset > 31 ||
> + !(eq5r_valid_masks[domain] & BIT(offset))) {
> + dev_err(rcdev->dev, "%u-%u: invalid reset\n", domain, offset);
> + return -EINVAL;
> + }
> +
> + return (domain << 8) | offset;
> +}
> +
> +static int eq5r_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct device_node *parent_np = of_get_parent(np);
> + struct eq5r_private *priv;
> + int ret, i;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
You leak parent.
> +
> + dev_set_drvdata(dev, priv);
> +
> + priv->olb = ERR_PTR(-ENODEV);
> + if (parent_np) {
> + priv->olb = syscon_node_to_regmap(parent_np);
> + of_node_put(parent_np);
> + }
> + if (IS_ERR(priv->olb))
Also here
> + return PTR_ERR(priv->olb);
This looks over-complicated. First, you cannot just
dev_get_regmap(pdev->dev.parent)?
> +
> + for (i = 0; i < EQ5R_DOMAIN_COUNT; i++)
> + mutex_init(&priv->mutexes[i]);
> +
> + priv->rcdev.ops = &eq5r_ops;
> + priv->rcdev.owner = THIS_MODULE;
> + priv->rcdev.dev = dev;
> + priv->rcdev.of_node = np;
> + priv->rcdev.of_reset_n_cells = 2;
> + priv->rcdev.of_xlate = eq5r_of_xlate;
> +
> + priv->rcdev.nr_resets = 0;
> + for (i = 0; i < EQ5R_DOMAIN_COUNT; i++)
> + priv->rcdev.nr_resets += __builtin_popcount(eq5r_valid_masks[i]);
> +
> + ret = reset_controller_register(&priv->rcdev);
> + if (ret) {
> + dev_err(dev, "Failed registering reset controller: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id eq5r_match_table[] = {
> + { .compatible = "mobileye,eyeq5-reset" },
> + {}
> +};
> +
> +static struct platform_driver eq5r_driver = {
> + .probe = eq5r_probe,
> + .driver = {
> + .name = "eyeq5-reset",
> + .of_match_table = eq5r_match_table,
> + },
> +};
> +
> +static int __init eq5r_init(void)
> +{
> + return platform_driver_register(&eq5r_driver);
> +}
> +
> +arch_initcall(eq5r_init);
This is does not look like arch code, but driver or subsys. Use regular
module_driver. I see there is such pattern in reset but I doubt this is
something good.
Best regards,
Krzysztof
