[PATCH 1/2] Driver for Xilinx ZynqMP AXI Timeout Block (ATB)
Krzysztof Kozlowski
krzk at kernel.org
Sun May 25 20:59:22 PDT 2025
On 25/05/2025 21:59, Maciej Andrzejewski wrote:
> This module implements a driver for the Xilinx AXI Timeout Block (ATB).
> The ATB is used to detect and handle timeouts on AXI transactions.
> It supports configuration and handling of timeout interrupts for both
> the Full Power Domain (FPD) and Low Power Domain (LPD) in Xilinx SoCs.
> The driver reads configuration from the device tree, sets up the necessary
> registers, and handles interrupts to report timeout events.
>
> AXI timeouts can be harmful as they stall the bus and can potentially stall
> the entire Linux system. Due to hardware limitations, this driver cannot
> completely prevent bus stalls; only a limited number of AXI timeouts can be
> registered before the bus will eventually stall.
>
> It is important to note that this driver should produce dmesg error output
> (grep for "atb: Timeout detected") to notify the user that the system
> configuration is not properly handled. Once an AXI timeout is detected,
> additional measures should be taken to address the issue.
>
> To make this driver work, the device tree must contain the following
> properties:
>
> atb {
> compatible = "xlnx,zynqmp-atb";
> status = "okay";
>
> fpd_afifs1_enable;
> fpd_afifs0_enable;
> fpd_fpds_enable;
> lpd_afifs2_enable;
> lpd_lpdm_enable;
> };
>
> The bool properties enable the corresponding interrupts for the
> FPD and LPD.
> The driver will read these properties and configure the ATB accordingly.
> If the bool property is not present, the functionality for this ATB will be
> disabled.
>
> Enable this driver by selecting ZYNQMP_ATB in the kernel config.
>
> Signed-off-by: Maciej Andrzejewski <maciej.andrzejewski at m-works.net>
> Signed-off-by: Maciej Andrzejewski <maciej.andrzejewski at iceye.com>
> ---
> MAINTAINERS | 6 +
> drivers/soc/xilinx/Kconfig | 53 ++--
> drivers/soc/xilinx/Makefile | 1 +
> drivers/soc/xilinx/zynqmp_atb.c | 417 ++++++++++++++++++++++++++++++++
> 4 files changed, 457 insertions(+), 20 deletions(-)
> create mode 100644 drivers/soc/xilinx/zynqmp_atb.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3c6f9e70ace0..37cd15a59d83 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26734,6 +26734,12 @@ S: Maintained
> F: Documentation/devicetree/bindings/nvmem/xlnx,zynqmp-nvmem.yaml
> F: drivers/nvmem/zynqmp_nvmem.c
>
> +XILINX ZYNQMP ATB DRIVER
> +M: Maciej Andrzejewski <maciej.andrzejewski at m-works.net>
> +S: Maintained
> +F: Documentation/devicetree/bindings/soc/xilinx/xlnx,zynqmp-atb.txt
> +F: drivers/soc/xilinx/zynqmp_atb.c
> +
> XILLYBUS DRIVER
> M: Eli Billauer <eli.billauer at gmail.com>
> L: linux-kernel at vger.kernel.org
> diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
> index 49d69d6e18fe..b2b1187c70ec 100644
> --- a/drivers/soc/xilinx/Kconfig
> +++ b/drivers/soc/xilinx/Kconfig
> @@ -2,28 +2,41 @@
> menu "Xilinx SoC drivers"
>
> config ZYNQMP_POWER
> - bool "Enable Xilinx Zynq MPSoC Power Management driver"
> - depends on PM && ZYNQMP_FIRMWARE
> - default y
> - select MAILBOX
> - select ZYNQMP_IPI_MBOX
> - help
> - Say yes to enable power management support for ZyqnMP SoC.
> - This driver uses firmware driver as an interface for power
> - management request to firmware. It registers isr to handle
> - power management callbacks from firmware. It registers mailbox client
> - to handle power management callbacks from firmware.
> + bool "Enable Xilinx Zynq MPSoC Power Management driver"
> + depends on PM && ZYNQMP_FIRMWARE
> + default y
> + select MAILBOX
> + select ZYNQMP_IPI_MBOX
Why doing this change? Code was correct before.
> + help
> + Say yes to enable power management support for ZyqnMP SoC.
> + This driver uses firmware driver as an interface for power
> + management request to firmware. It registers isr to handle
> + power management callbacks from firmware. It registers mailbox client
> + to handle power management callbacks from firmware.
>
> - If in doubt, say N.
> + If in doubt, say N.
>
> config XLNX_EVENT_MANAGER
> - bool "Enable Xilinx Event Management Driver"
> - depends on ZYNQMP_FIRMWARE
> - default ZYNQMP_FIRMWARE
> - help
> - Say yes to enable event management support for Xilinx.
> - This driver uses firmware driver as an interface for event/power
> - management request to firmware.
> + bool "Enable Xilinx Event Management Driver"
> + depends on ZYNQMP_FIRMWARE
> + default ZYNQMP_FIRMWARE
> + help
> + Say yes to enable event management support for Xilinx.
> + This driver uses firmware driver as an interface for event/power
> + management request to firmware.
> +
> + If in doubt, say N.
> +
> +config ZYNQMP_ATB
> + bool "Enable ZynqMP AXI Timeout Block driver"
> + depends on ARCH_ZYNQMP
> + help
> + Say yes to enable ZynqMP AXI Timeout Block driver.
> + The ATB is used to detect and handle timeouts on AXI transactions.
> + It supports configuration and handling of timeout interrupts for both
> + the Full Power Domain (FPD) and Low Power Domain (LPD) in
> + Xilinx SoCs. The driver reads configuration from the device tree,
> + sets up the necessary registers, and handles interrupts to report
> + timeout events.
>
> - If in doubt, say N.
> endmenu
> diff --git a/drivers/soc/xilinx/Makefile b/drivers/soc/xilinx/Makefile
> index 33d94395fd87..30768593701e 100644
> --- a/drivers/soc/xilinx/Makefile
> +++ b/drivers/soc/xilinx/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_ZYNQMP_POWER) += zynqmp_power.o
> obj-$(CONFIG_XLNX_EVENT_MANAGER) += xlnx_event_manager.o
> +obj-$(CONFIG_ZYNQMP_ATB) += zynqmp_atb.o
> \ No newline at end of file
All youro patches have patch warnings.
> diff --git a/drivers/soc/xilinx/zynqmp_atb.c b/drivers/soc/xilinx/zynqmp_atb.c
> new file mode 100644
> index 000000000000..41e942352b38
> --- /dev/null
> +++ b/drivers/soc/xilinx/zynqmp_atb.c
> @@ -0,0 +1,417 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/**
> + * Implementation of Xilinx ZynqMP AXI Timeout Block peripheral driver.
> + *
> + * Copyright (C) 2025 ICEYE,
> + * Maciej Andrzejewski <maciej.andrzejewski at iceye.com>
> + */
> +
...
> +
> + return ret;
> +}
> +
> +/**
> + * read_dts - Read DTS configuration for the driver
> + * @config: Pointer to driver configuration
> + * @return: 0 on success, otherwise error code
> + */
> +static int read_dts(struct device *dev, struct atb_config *config)
> +{
> + struct device_node *np;
> +
> + np = of_find_node_by_name(NULL, MOD_NAME);
No, you cannot do such stuff.
Match properly your driver.
> + if (!np) {
> + dev_err(dev, "unable to find device tree node '%s'", MOD_NAME);
So you will print such error on every machine, x86, arm (other vendors,
like Qualcomm), riscv64?
No, drop entirely.
> + return -ENODEV;
> + }
> +
> + // FPD domain
> + config->fpd.opt_val = 0;
> + if (of_property_read_bool(np, "fpd_afifs1_enable"))
> + config->fpd.opt_val |= FPD_ATB_AFIFS1_MASK;
> + if (of_property_read_bool(np, "fpd_afifs0_enable"))
> + config->fpd.opt_val |= FPD_ATB_AFIFS0_MASK;
> + if (of_property_read_bool(np, "fpd_fpds_enable"))
> + config->fpd.opt_val |= FPD_ATB_FPDS_MASK;
> +
...
> +
> +/**
> + * atb_domain_setup - Setup ATB in a domain
> + * @cfg: Pointer to ATB domain configuration
> + * @pdev: Pointer to platform device
> + * @base: Base address of the ATB domain
> + * @irq: IRQ number
> + * @domain_name: Domain name
> + * @irq_name: IRQ name
> + * @return: 0 on success, otherwise error code
> + */
> +static int atb_domain_setup(struct atb_domain_config *cfg,
> + struct platform_device *pdev, uintptr_t base,
> + int irq, const char *domain_name,
> + const char *irq_name)
> +{
> + struct device *dev = &pdev->dev;
> +
> + // get physical address
> + cfg->base = devm_ioremap(dev, base, SZ_64);
No, you do not have MMIO. Just look at the binding.
> + if (!cfg->base) {
> + dev_err(dev, "failed to map %s base address", domain_name);
> + return -1;
> + }
> +
> + // map IRQ
> + irq = map_irq(dev, irq);
Look at the binding: you do not have interrupts.
> + if (irq <= 0) {
> + dev_err(dev, "failed to map ATB %s IRQ", domain_name);
> + return -2;
> + }
> + cfg->irq = irq;
> +
> + // request IRQ
> + if (devm_request_irq(dev, irq, irq_handler, IRQF_SHARED, irq_name,
> + pdev)) {
> + dev_err(dev, "failed to request ATB %s IRQ.", domain_name);
> + return -3;
> + }
> +
> + // disable ATB for configuration
> + atb_write(cfg, 0x0, ATB_RESP_EN_REG);
> +
> + // response type: ATB returns 'SLVERR' on timeout
> + // which results in app 'bus fault'
> + atb_write(cfg, cfg->opt_val, ATB_RESP_TYPE_REG);
> + atb_write(cfg, cfg->opt_val, ATB_CMD_STORE_EN_REG);
> +
> + // timeout prescaler: 16b - enable, 15:0b - prescaler value
> + // based on 100 MHz clock
> + atb_write(cfg, (ATB_PRESCALER_EN_MASK + ATB_PRESCALER_TIMEOUT_VAL),
> + ATB_PRESCALE_REG);
> +
> + // clear interrupt flags
> + atb_write(cfg, cfg->opt_val, ERR_ATB_ISR_REG);
> +
> + // enable selected interrupts
> + atb_write(cfg, cfg->opt_val, ERR_ATB_IER_REG);
> +
> + // enable ATB
> + atb_write(cfg, cfg->opt_val, ATB_RESP_EN_REG);
> +
> + cfg->init = true;
> +
> + return 0;
> +}
> +
> +/**
> + * atb_probe - Probe function for the driver
> + * @pdev: Pointer to platform device
> + * @return: 0 on success, otherwise error code
> + */
> +static int atb_probe(struct platform_device *pdev)
> +{
> + int ret;
> + struct atb_config *config;
> + struct device *dev = &pdev->dev;
> +
> + config = devm_kzalloc(dev, sizeof(*config), GFP_KERNEL);
> + if (!config)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, config);
> +
> + // read DTS config
> + ret = read_dts(dev, config);
> + if (ret) {
> + dev_err(dev, "failed to read DTS: %d", ret);
> + goto err;
> + }
> +
> + // configure FPD
> + ret = atb_domain_setup(&config->fpd, pdev, FPD_SLCR_ATB_BASE_ADDR,
> + IRQ_ZYNQMP_GIC_ATB_FPD, "FPD", "atb_fpd_irq");
> + if (ret) {
> + dev_err(dev, "failed to setup FPD domain: %d", ret);
> + goto err;
> + } else {
> + dev_info(dev, "ATB FPD configured");
> + }
> +
> + // configure LPD
> + ret = atb_domain_setup(&config->lpd, pdev, LPD_SLCR_ATB_BASE_ADDR,
> + IRQ_ZYNQMP_GIC_ATB_LPD, "LPD", "atb_lpd_irq");
> + if (ret) {
> + dev_err(dev, "failed to setup LPD domain: %d", ret);
> + goto err_fpd;
> + } else {
> + dev_info(dev, "ATB LPD configured");
Drop all of such, drivers should be silent on success. See Linux coding
style.
> + }
> +
> + return 0;
> +
> +err_fpd:
> + if (config->fpd.init)
> + atb_write(&config->fpd, 0x0, ATB_RESP_EN_REG);
> +err:
> + return -EFAULT;
> +}
> +
> +/**
> + * atb_remove - Remove function for the driver
> + * @pdev: Pointer to platform device
> + * @return: 0 on success, otherwise error code
> + */
> +static int atb_remove(struct platform_device *pdev)
> +{
> + int val, reg;
> + struct device *dev = &pdev->dev;
> + struct atb_config *config = platform_get_drvdata(pdev);
> +
> + if (!config) {
> + dev_err(dev, "config is NULL in remove");
> + return -EFAULT;
> + }
> +
> + // disable ATB
> + if (config->fpd.init) {
> + val = config->fpd.opt_val;
> + reg = atb_read(&config->fpd, ATB_RESP_EN_REG);
> + val &= ~reg;
> + atb_write(&config->fpd, val, ATB_RESP_EN_REG);
> + }
> + if (config->lpd.init) {
> + val = config->lpd.opt_val;
> + reg = atb_read(&config->lpd, ATB_RESP_EN_REG);
> + val &= ~reg;
> + atb_write(&config->lpd, val, ATB_RESP_EN_REG);
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id atb_of_match[] = {
> + {
> + .compatible = "xlnx,zynqmp-atb",
> + },
> + {},
> +};
> +
> +static struct platform_driver atb_driver = {
> + .probe = atb_probe,
> + .remove = atb_remove,
> + .driver = {
> + .name = MOD_NAME,
> + .of_match_table = atb_of_match,
> + },
> +};
> +
> +static int __init atb_early_init(void)
> +{
> + return platform_driver_register(&atb_driver);
> +}
> +
> +arch_initcall(atb_early_init);
Don't order initcalls. That's not arch. This is supposed to be driver.
Best regards,
Krzysztof
More information about the linux-arm-kernel
mailing list