diff mbox series

[RFC,4/6] drivers: misc: k3_bist: Add K3 BIST driver

Message ID 20240903114402.2155740-5-n-francis@ti.com
State RFC
Delegated to: Tom Rini
Headers show
Series Add support for K3 BIST | expand

Commit Message

Neha Malcom Francis Sept. 3, 2024, 11:44 a.m. UTC
Add a driver for the BIST module which currently includes support for
BIST IPs that trigger PBIST (Memory BIST).

Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
---
 drivers/misc/Kconfig               |   8 +
 drivers/misc/Makefile              |   1 +
 drivers/misc/k3_bist.c             | 507 ++++++++++++++++++++++++++
 drivers/misc/k3_bist_static_data.h | 551 +++++++++++++++++++++++++++++
 4 files changed, 1067 insertions(+)
 create mode 100644 drivers/misc/k3_bist.c
 create mode 100644 drivers/misc/k3_bist_static_data.h

Comments

Kumar, Udit Sept. 4, 2024, 5:05 a.m. UTC | #1
On 9/3/2024 5:14 PM, Neha Malcom Francis wrote:
> Add a driver for the BIST module which currently includes support for
> BIST IPs that trigger PBIST (Memory BIST).
>
> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
> ---
>   drivers/misc/Kconfig               |   8 +
>   drivers/misc/Makefile              |   1 +
>   drivers/misc/k3_bist.c             | 507 ++++++++++++++++++++++++++
>   drivers/misc/k3_bist_static_data.h | 551 +++++++++++++++++++++++++++++
>   4 files changed, 1067 insertions(+)
>   create mode 100644 drivers/misc/k3_bist.c
>   create mode 100644 drivers/misc/k3_bist_static_data.h
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 6009d55f400..8e28a93d74c 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -664,6 +664,14 @@ config ESM_K3
>   	help
>   	  Support ESM (Error Signaling Module) on TI K3 SoCs.
>   
> +config K3_BIST
> +	bool "Enable K3 BIST driver"
> +	depends on ARCH_K3
> +	help
> +	  Support BIST (Built-In Self Test) module on TI K3 SoCs. This driver
> +	  supports running both PBIST (Memory BIST) and LBIST (Logic BIST) on
> +	  a region or IP in the SoC.
> +
>   config MICROCHIP_FLEXCOM
>   	bool "Enable Microchip Flexcom driver"
>   	depends on MISC
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index e53d52c47b3..15c5c4810dd 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -89,6 +89,7 @@ obj-$(CONFIG_JZ4780_EFUSE) += jz4780_efuse.o
>   obj-$(CONFIG_MICROCHIP_FLEXCOM) += microchip_flexcom.o
>   obj-$(CONFIG_K3_AVS0) += k3_avs.o
>   obj-$(CONFIG_ESM_K3) += k3_esm.o
> +obj-$(CONFIG_K3_BIST) += k3_bist.o
>   obj-$(CONFIG_ESM_PMIC) += esm_pmic.o
>   obj-$(CONFIG_SL28CPLD) += sl28cpld.o
>   obj-$(CONFIG_SPL_SOCFPGA_DT_REG) += socfpga_dtreg.o
> diff --git a/drivers/misc/k3_bist.c b/drivers/misc/k3_bist.c
> new file mode 100644
> index 00000000000..a4728376b73
> --- /dev/null
> +++ b/drivers/misc/k3_bist.c
> @@ -0,0 +1,507 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Texas Instruments' BIST (Built-In Self-Test) driver
> + *
> + * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
> + *      Neha Malcom Francis <n-francis@ti.com>
> + *
> + */
> +
> +#include <dm.h>
> +#include <errno.h>
> +#include <clk.h>
> +#include <asm/io.h>
> +#include <dm/device_compat.h>
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <asm/arch/hardware.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +#include <remoteproc.h>
> +#include <power-domain.h>
> +


In general, few macro's name are too long

many places hard-coded values are used, please consider to move to macro

driver looks to be j784s4 specific including header files  ,

please see, if we can make this generic driver.


> +#include "k3_bist_static_data.h"
> +
> +/* PBIST Timeout Value */
> +#define PBIST_MAX_TIMEOUT_VALUE		100000000
> +
> +/**
> + * struct k3_bist_privdata - K3 BIST structure
> + * @dev: device pointer
> + * @base: base of register set
> + * @instance: PBIST instance number
> + * @intr_num: corresponding interrupt ID of the PBIST instance
> + */
> +struct k3_bist_privdata {
> +	struct udevice *dev;
> +	void *base;
> +	u32 instance;
> +	u32 intr_num;
> +};
> +
> +static struct k3_bist_privdata *k3_bist_priv;
> +
> +/**
> + * pbist_run_post_pbist_check() - Check POST results
> + *
> + * Function to check whether HW Power-On Self Test, i.e. POST has run
> + * successfully on the MCU domain.
> + *
> + * Return: 0 if all went fine, else corresponding error.
> + */
> +int pbist_run_post_pbist_check(void)

Please consider to rename function name as per description above 
something like

check_pbist_results_of_mcu_domain,, if you agree

Also give more context, I believe, ROM runs BIST on MCU domain, Please 
consider to mention, if you want

> +{
> +	bool is_done, timed_out;
> +	u32 mask;
> +	u32 post_reg_val, shift;
> +
> +	/* Read HW POST status register */
> +	post_reg_val = readl(WKUP_CTRL_MMR0_BASE + WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT);
> +
> +	/* Check if HW POST PBIST was performed */
> +	shift = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_DONE_SHIFT;
> +	is_done = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? (bool)true : (bool)false;
> +
> +	if (!is_done) {
> +		/* HW POST: PBIST not completed, check if it timed out */
> +		shift = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_TIMEOUT_SHIFT;
Too long macro name
> +		timed_out = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? (bool)true : (bool)false;
> +
> +		if (!timed_out) {
> +			debug("%s: PBIST was not performed at all on this device for this core\n",
> +			      __func__);
> +			return -EINVAL;

This is error no ? , move to dev_err instead of debug


> +		} else {
> +			debug("%s: PBIST was attempted but timed out for this section\n", __func__);
> +			return -ETIMEDOUT;

This is error no ? , move to dev_err instead of debug .

What next, reboot SOC or just continue booting in case of error


> +		}
> +	} else {
> +		/* HW POST: PBIST was completed on this device, check the result */
> +		mask = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_FAIL_MASK;
> +
> +		if ((post_reg_val & mask) != 0) {
> +			debug("%s: PBIST was completed, but the test failed\n", __func__);
> +			return -EINVAL;
> +		} else {
> +			debug("%s: HW POST PBIST completed, test passed\n", __func__);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * core_get_by_index() - Configure processor to correct state


Two operation here. please rename if possible


> + *
> + * Function to configure processor under test to correct state for SW-initiated
> + * PBIST
> + * @dev: BIST device
> + * @index: corresponding index of the core in the cores-under-test list
> + * @turnoff: true if core is needed to be turned off
> + *
> + * Return: 0 if all went fine, else corresponding error.
> + */
> +int core_get_by_index(struct udevice *dev, int index, bool turnoff)
> +{
> +	struct ofnode_phandle_args args;
> +	int ret;
> +	struct udevice *dev_core;
> +
> +	ret = dev_read_phandle_with_args(dev, "cores-under-test", NULL, 0, index, &args);
> +	if (ret) {
> +		debug("%s: dev_read_phandle_with_args failed: %d\n", __func__,
> +		      ret);
> +		return ret;
> +	}
> +	ret =  uclass_get_device_by_ofnode(UCLASS_REMOTEPROC, args.node, &dev_core);
> +	if (ret) {
> +		debug("%s: uclass_get_device_by_of_offset failed: %d\n",
> +		      __func__, ret);
> +		return ret;
> +	}
> +
> +	if (turnoff) {
> +		struct power_domain pwrdmn;
> +		struct clk fclk;
> +

isn't some tisci call are ok to turn off the CPUs.


> +		ret = power_domain_get_by_index(dev_core, &pwrdmn, 0);
> +		if (ret) {
> +			dev_err(dev, "failed to get power domain for the core %d\n", ret);
> +			return ret;
> +		}
> +
> +		ret = clk_get_by_index(dev_core, 0, &fclk);
> +		if (ret) {
> +			dev_err(dev, "failed to get clock for the core %d\n", ret);
> +			return ret;
> +		}
> +
> +		ret = power_domain_off(&pwrdmn);
> +		if (ret) {
> +			dev_err(dev, "failed to power OFF the core %d\n", ret);
> +			return ret;
> +		}
> +
> +		ret = power_domain_free(&pwrdmn);
> +		if (ret) {
> +			dev_err(dev, "failed to free the core %d\n", ret);
> +			return ret;
> +		}
> +		ret = clk_disable(&fclk);
> +		if (ret) {
> +			dev_err(dev, "failed to disable clock of the core %d\n", ret);
> +			return ret;
> +		}
> +	}
> +	return 0;
> +}
> +
> +/**
> + * pbist_self_test() - Run PBIST_TEST on specified cores
> + * @config: pbist_config structure for PBIST test
> + *
> + * Function to run PBIST_TEST
> + *
> + * Return: 0 if all went fine, else corresponding error.
> + */
> +int pbist_self_test(struct pbist_config *config)
> +{
> +	struct udevice *dev = k3_bist_priv->dev;
> +	void *base = k3_bist_priv->base;
> +	u32 intr_num = k3_bist_priv->intr_num;
> +	bool test_result = true;
> +
> +	/* Turns on PBIST clock in PBIST ACTivate register */
> +	writel(PBIST_PACT_PACT_MASK, base + PBIST_PACT);
> +
> +	/* Set Margin mode register for Test mode */
> +	writel(PBIST_TEST_MODE, base + PBIST_MARGIN_MODE);
> +
> +	/* Zero out Loop counter 0 */
> +	writel(0x0, base + PBIST_L0);
> +
> +	/* Set algorithm bitmap */
> +	writel(config->algorithms_bit_map, base + PBIST_ALGO);
> +
> +	/* Set Memory group bitmap */
> +	writel(config->memory_groups_bit_map, base + PBIST_RINFO);
> +
> +	/* Zero out override register */
> +	writel(config->override, base + PBIST_OVER);
> +
> +	/* Set Scramble value - 64 bit*/
> +	writel(config->scramble_value_lo, base + PBIST_SCR_LO);
> +	writel(config->scramble_value_hi, base + PBIST_SCR_HI);
> +
> +	/* Set DLR register for ROM based testing and Config Access */
> +	writel(PBIST_DLR_DLR0_ROM_MASK
> +	| PBIST_DLR_DLR0_CAM_MASK, base + PBIST_DLR);
> +
> +	udelay(1000);
> +
> +	u32 timeout_count = 0;
Please move timeout_count at start of function
> +
> +	while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
> +	       (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
> +		;

Do you want to add some delay instead of just reading in a loop


> +
> +	if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
> +		test_result = false;
> +		debug("%s(dev=%p): test failed\n", __func__, dev);

Fail is error , no ?


> +	} else {
> +		debug("%s(dev=%p): test passed\n", __func__, dev);
> +	}
> +
> +	writel(0xffffffff, VIM_STS(intr_num));
> +
> +	return 0;


Caller always will see success


> +}
> +
> +/**
> + * pbist_neg_self_test() - Run PBIST_negTEST on specified cores
> + * @config: pbist_config_neg structure for PBIST negative test
> + *
> + * Function to run PBIST failure insertion test
> + *
> + * Return: 0 if all went fine, else corresponding error.
> + */
> +int pbist_neg_self_test(struct pbist_config_neg *config)
> +{
> +	struct udevice *dev = k3_bist_priv->dev;
> +	void *base = k3_bist_priv->base;
> +	u32 intr_num = k3_bist_priv->intr_num;
> +	bool test_result = true;
> +
> +	/* Turns on PBIST clock in PBIST ACTivate register */
> +	writel(PBIST_PACT_PACT_MASK, base + PBIST_PACT);
> +
> +	/* Set Margin mode register for Test mode */
> +	writel(PBIST_FAILURE_INSERTION_TEST_MODE, base + PBIST_MARGIN_MODE);
> +
> +	/* Zero out Loop counter 0 */
> +	writel(0x0, base + PBIST_L0);
> +
> +	/* Set DLR register */
> +	writel(0x10, base + PBIST_DLR);
> +
> +	/* Set Registers*/
> +	writel(0x00000001, base + PBIST_RF0L);
> +	writel(0x00003123, base + PBIST_RF0U);
> +	writel(0x0513FC02, base + PBIST_RF1L);
> +	writel(0x00000002, base + PBIST_RF1U);
> +	writel(0x00000003, base + PBIST_RF2L);
> +	writel(0x00000000, base + PBIST_RF2U);
> +	writel(0x00000004, base + PBIST_RF3L);
> +	writel(0x00000028, base + PBIST_RF3U);
> +	writel(0x64000044, base + PBIST_RF4L);
> +	writel(0x00000000, base + PBIST_RF4U);
> +	writel(0x0006A006, base + PBIST_RF5L);
> +	writel(0x00000000, base + PBIST_RF5U);
> +	writel(0x00000007, base + PBIST_RF6L);
> +	writel(0x0000A0A0, base + PBIST_RF6U);
> +	writel(0x00000008, base + PBIST_RF7L);
> +	writel(0x00000064, base + PBIST_RF7U);
> +	writel(0x00000009, base + PBIST_RF8L);
> +	writel(0x0000A5A5, base + PBIST_RF8U);
> +	writel(0x0000000A, base + PBIST_RF9L);
> +	writel(0x00000079, base + PBIST_RF9U);
> +	writel(0x00000000, base + PBIST_RF10L);
> +	writel(0x00000001, base + PBIST_RF10U);
> +	writel(0xAAAAAAAA, base + PBIST_D);
> +	writel(0xAAAAAAAA, base + PBIST_E);

too much direct values

> +
> +	writel(config->CA2, base + PBIST_CA2);
> +	writel(config->CL0, base + PBIST_CL0);
> +	writel(config->CA3, base + PBIST_CA3);
> +	writel(config->I0, base + PBIST_I0);
> +	writel(config->CL1, base + PBIST_CL1);
> +	writel(config->I3, base + PBIST_I3);
> +	writel(config->I2, base + PBIST_I2);
> +	writel(config->CL2, base + PBIST_CL2);
> +	writel(config->CA1, base + PBIST_CA1);
> +	writel(config->CA0, base + PBIST_CA0);
> +	writel(config->CL3, base + PBIST_CL3);
> +	writel(config->I1, base + PBIST_I1);
> +	writel(config->RAMT, base + PBIST_RAMT);
> +	writel(config->CSR, base + PBIST_CSR);
> +	writel(config->CMS, base + PBIST_CMS);
> +
> +	writel(0x00000009, base + PBIST_STR);
> +
> +	/* Start PBIST */
> +	writel(0x00000001, base + PBIST_STR);
> +
> +	udelay(1000);
> +
> +	u32 timeout_count = 0;
> +
> +	while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
> +	       (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
> +		;
> +
> +	if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
> +		test_result = false;
> +		debug("%s(dev=%p): test failed\n", __func__, dev);
> +	} else {
> +		debug("%s(dev=%p): test passed\n", __func__, dev);
> +	}
> +
> +	writel(0xffffffff, VIM_STS(intr_num));
> +
> +	return 0;

Same as in above function for error

> +}
> +
> +/**
> + * pbist_rom_self_test() - Run PBIST_ROM_TEST on specified cores
> + * @config: pbist_config_rom structure for PBIST negative test
> + *
> + * Function to run PBIST test of ROM
> + *
> + * Return: 0 if all went fine, else corresponding error.
> + */
> +int pbist_rom_self_test(struct pbist_config_rom *config)
> +{
> +	struct udevice *dev = k3_bist_priv->dev;
> +	void *base = k3_bist_priv->base;
> +	u32 intr_num = k3_bist_priv->intr_num;
> +	bool test_result = true;
> +
> +	/* Turns on PBIST clock in PBIST ACTivate register */
> +	writel(0x1, base + PBIST_PACT);
> +
> +	/* Set Margin mode register for Test mode */
> +	writel(0xf, base + PBIST_MARGIN_MODE);
> +
> +	/* Zero out Loop counter 0 */
> +	writel(0x0, base + PBIST_L0);
> +
> +	/* Set DLR register */
> +	writel(0x310, base + PBIST_DLR);
> +
> +	/* Set Registers*/
> +	writel(0x00000001, base + PBIST_RF0L);
> +	writel(0x00003123, base + PBIST_RF0U);
> +	writel(0x7A400183, base + PBIST_RF1L);
> +	writel(0x00000060, base + PBIST_RF1U);
> +	writel(0x00000184, base + PBIST_RF2L);
> +	writel(0x00000000, base + PBIST_RF2U);
> +	writel(0x7B600181, base + PBIST_RF3L);
> +	writel(0x00000061, base + PBIST_RF3U);
> +	writel(0x00000000, base + PBIST_RF4L);
> +	writel(0x00000000, base + PBIST_RF4U);
> +
> +	writel(config->D, base + PBIST_D);
> +	writel(config->E, base + PBIST_E);
> +	writel(config->CA2, base + PBIST_CA2);
> +	writel(config->CL0, base + PBIST_CL0);
> +	writel(config->CA3, base + PBIST_CA3);
> +	writel(config->I0, base + PBIST_I0);
> +	writel(config->CL1, base + PBIST_CL1);
> +	writel(config->I3, base + PBIST_I3);
> +	writel(config->I2, base + PBIST_I2);
> +	writel(config->CL2, base + PBIST_CL2);
> +	writel(config->CA1, base + PBIST_CA1);
> +	writel(config->CA0, base + PBIST_CA0);
> +	writel(config->CL3, base + PBIST_CL3);
> +	writel(config->I1, base + PBIST_I1);
> +	writel(config->RAMT, base + PBIST_RAMT);
> +	writel(config->CSR, base + PBIST_CSR);
> +	writel(config->CMS, base + PBIST_CMS);
> +
> +	writel(0x00000009, base + PBIST_STR);
> +
> +	/* Start PBIST */
> +	writel(0x00000001, base + PBIST_STR);
> +
> +	udelay(1000);
> +
Why delay is needed. please add comment for that
> +	u32 timeout_count = 0;
same as above
> +
> +	while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
> +	       (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
> +		;
> +
> +	if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
> +		test_result = false;
> +		debug("%s(dev=%p): test failed\n", __func__, dev);
> +	} else {
> +		debug("%s(dev=%p): test passed\n", __func__, dev);
> +	}
> +
> +	writel(0xffffffff, VIM_STS(intr_num));
> +
> +	return 0;
> +}
> +
> +/**
> + * k3_bist_probe() - Basic probe
> + * @dev: corresponding BIST device
> + *
> + * Parses BIST info from device tree, and configures the module accordingly.
> + * Return: 0 if all goes good, else appropriate error message.
> + */
> +static int k3_bist_probe(struct udevice *dev)
> +{
> +	int ret = 0, num_runs, i, j;
> +	struct k3_bist_privdata *priv = dev_get_priv(dev);
> +	struct pbist_inst_info *info;
> +
> +	debug("%s(dev=%p)\n", __func__, dev);
> +
> +	priv = dev_get_priv(dev);

NULL error check for priv


> +	priv->dev = dev;
> +
> +	k3_bist_priv = priv;
> +
> +	priv->base = dev_remap_addr_index(dev, 0);
> +	if (!priv->base)
> +		return -ENODEV;
> +
> +	ret = dev_read_u32(dev, "ti,bist-instance", &priv->instance);
> +	if (!priv->instance)
> +		return -ENODEV;
> +
> +	switch (priv->instance) {
> +	case PBIST14_INSTANCE:
> +		info = &pbist14_inst_info;
> +		priv->intr_num = info->intr_num;
> +		break;
> +	default:
> +		dev_err(dev, "%s: PBIST instance %d not supported\n", __func__, priv->instance);
> +		return -ENODEV;
> +	};
> +
> +	/* Probe the cores under test */
> +	for (i = 0; ; i++) {
> +		ret = core_get_by_index(dev, i, false);
> +		if (ret)
> +			break;
> +	}
> +
> +	if (!i) {
> +		dev_err(dev, "%s: Acquiring the core failed. ret = %d\n", __func__, ret);
> +		return ret;
> +	}

Please add a check, what you expect in device tree at start of probe .

I assume, you can hit this only case of incorrect DT


> +
> +	/* Check whether HW POST successfully completely PBIST on the MCU domain */
> +	ret = pbist_run_post_pbist_check();
> +	if (ret) {
> +		dev_err(dev, "HW POST failed to run successfully %d\n", ret);
> +		return ret;
> +	}
> +

you might need to do this check first. before probing other cores


> +	/* Run PBIST test */
> +	num_runs = info->num_pbist_runs;
if you want num_runs configurable, prefer to use DT
> +
> +	for (j = 0; j < num_runs; j++) {
> +		ret = pbist_self_test(&info->pbist_config_run[j]);
> +		if (ret) {
> +			dev_err(dev, "failed to run PBIST test %d\n", ret);
> +			return ret;
> +		}
> +	}
Dummy question, will above run the BIST on all selected cores ?
> +
> +	/* Run PBIST failure insertion test */
> +	ret = pbist_neg_self_test(&info->pbist_neg_config_run);
> +	if (ret) {
> +		dev_err(dev, "failed to run PBIST negative test %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Run PBIST test on ROM */
> +	num_runs = info->num_pbist_rom_test_runs;
> +
> +	for (j = 0; j < num_runs; j++) {
> +		ret = pbist_rom_self_test(&info->pbist_rom_test_config_run[j]);
> +		if (ret) {
> +			dev_err(dev, "failed to run ROM PBIST test %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	/* Power off cores under test */
> +	while (i--) {
> +		ret = core_get_by_index(dev, i, true);
> +		if (ret)
> +			break;
> +	}

can we get rid from this 'i' , May you can do all core ON and OFF in one 
function,

Largely its from device tree

> +
> +	if (i) {
> +		dev_err(dev, "%s: Stopping the core failed. ret = %d\n", __func__, ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct udevice_id k3_bist_ids[] = {
> +	{ .compatible = "ti,j784s4-bist" },
> +	{}
> +};
> +
> +U_BOOT_DRIVER(k3_bist) = {
> +	.name = "k3_bist",
> +	.of_match = k3_bist_ids,
> +	.id = UCLASS_MISC,
> +	.probe = k3_bist_probe,
> +	.priv_auto = sizeof(struct k3_bist_privdata),
> +};
> diff --git a/drivers/misc/k3_bist_static_data.h b/drivers/misc/k3_bist_static_data.h
> new file mode 100644
> index 00000000000..f30fb7935b6
> --- /dev/null
> +++ b/drivers/misc/k3_bist_static_data.h
> @@ -0,0 +1,551 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Static Data for Texas Instruments' BIST (Built-In Self-Test) driver
> + *
> + * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
> + *
> + */
> +
> +#ifndef __K3_BIST_STATIC_DATA_H
> +#define __K3_BIST_STATIC_DATA_H
> +
> +#define PBIST_MAX_NUM_RUNS	2
> +#define NUM_MAX_PBIST_TEST_ROM_RUNS 13
> +#define PBIST14_DFT_PBIST_CPU_0_INTR_NUM 311
> +
> +/* VIM Registers */
> +#define VIM_STS_BASE						  0x40f80404
> +#define VIM_RAW_BASE						  0x40f80400
> +
> +#define VIM_STS(i)			(VIM_STS_BASE + (i) / 32 * 0x20)
> +#define VIM_RAW(i)	      (VIM_RAW_BASE + (i) / 32 * 0x20)
> +#define VIM_RAW_MASK(i)	 (BIT((i) % 32))

Above this SOC specific data , Please have some SOC specific data in 
another header file and include here.

So that adding next SOC will be easy

> [..]
> +#if IS_ENABLED(CONFIG_SOC_K3_J784S4)

Please put this data in SOC specific header file

> [..]
Neha Malcom Francis Sept. 5, 2024, 9:41 a.m. UTC | #2
On 04/09/24 10:35, Kumar, Udit wrote:
> 
> On 9/3/2024 5:14 PM, Neha Malcom Francis wrote:
>> Add a driver for the BIST module which currently includes support for
>> BIST IPs that trigger PBIST (Memory BIST).
>>
>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>> ---
>>   drivers/misc/Kconfig               |   8 +
>>   drivers/misc/Makefile              |   1 +
>>   drivers/misc/k3_bist.c             | 507 ++++++++++++++++++++++++++
>>   drivers/misc/k3_bist_static_data.h | 551 +++++++++++++++++++++++++++++
>>   4 files changed, 1067 insertions(+)
>>   create mode 100644 drivers/misc/k3_bist.c
>>   create mode 100644 drivers/misc/k3_bist_static_data.h
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 6009d55f400..8e28a93d74c 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -664,6 +664,14 @@ config ESM_K3
>>       help
>>         Support ESM (Error Signaling Module) on TI K3 SoCs.
>> +config K3_BIST
>> +    bool "Enable K3 BIST driver"
>> +    depends on ARCH_K3
>> +    help
>> +      Support BIST (Built-In Self Test) module on TI K3 SoCs. This driver
>> +      supports running both PBIST (Memory BIST) and LBIST (Logic BIST) on
>> +      a region or IP in the SoC.
>> +
>>   config MICROCHIP_FLEXCOM
>>       bool "Enable Microchip Flexcom driver"
>>       depends on MISC
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index e53d52c47b3..15c5c4810dd 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -89,6 +89,7 @@ obj-$(CONFIG_JZ4780_EFUSE) += jz4780_efuse.o
>>   obj-$(CONFIG_MICROCHIP_FLEXCOM) += microchip_flexcom.o
>>   obj-$(CONFIG_K3_AVS0) += k3_avs.o
>>   obj-$(CONFIG_ESM_K3) += k3_esm.o
>> +obj-$(CONFIG_K3_BIST) += k3_bist.o
>>   obj-$(CONFIG_ESM_PMIC) += esm_pmic.o
>>   obj-$(CONFIG_SL28CPLD) += sl28cpld.o
>>   obj-$(CONFIG_SPL_SOCFPGA_DT_REG) += socfpga_dtreg.o
>> diff --git a/drivers/misc/k3_bist.c b/drivers/misc/k3_bist.c
>> new file mode 100644
>> index 00000000000..a4728376b73
>> --- /dev/null
>> +++ b/drivers/misc/k3_bist.c
>> @@ -0,0 +1,507 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Texas Instruments' BIST (Built-In Self-Test) driver
>> + *
>> + * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
>> + *      Neha Malcom Francis <n-francis@ti.com>
>> + *
>> + */
>> +
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <clk.h>
>> +#include <asm/io.h>
>> +#include <dm/device_compat.h>
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
>> +#include <asm/arch/hardware.h>
>> +#include <linux/soc/ti/ti_sci_protocol.h>
>> +#include <remoteproc.h>
>> +#include <power-domain.h>
>> +
> 
> 
> In general, few macro's name are too long
> 
> many places hard-coded values are used, please consider to move to macro
> 
> driver looks to be j784s4 specific including header files  ,
> 
> please see, if we can make this generic driver.

I've put SoC specific (J784S4 right now) data protected with SoC specific 
configs in k3_bist_static_data.h; the hardcoded values are a sequence for 
triggering a specific test, whatever is generic and known I have put as a macro, 
however I'll try to better understand the sequence if I can put them as macros.

> 
> 
>> +#include "k3_bist_static_data.h"
>> +
>> +/* PBIST Timeout Value */
>> +#define PBIST_MAX_TIMEOUT_VALUE        100000000
>> +
>> +/**
>> + * struct k3_bist_privdata - K3 BIST structure
>> + * @dev: device pointer
>> + * @base: base of register set
>> + * @instance: PBIST instance number
>> + * @intr_num: corresponding interrupt ID of the PBIST instance
>> + */
>> +struct k3_bist_privdata {
>> +    struct udevice *dev;
>> +    void *base;
>> +    u32 instance;
>> +    u32 intr_num;
>> +};
>> +
>> +static struct k3_bist_privdata *k3_bist_priv;
>> +
>> +/**
>> + * pbist_run_post_pbist_check() - Check POST results
>> + *
>> + * Function to check whether HW Power-On Self Test, i.e. POST has run
>> + * successfully on the MCU domain.
>> + *
>> + * Return: 0 if all went fine, else corresponding error.
>> + */
>> +int pbist_run_post_pbist_check(void)
> 
> Please consider to rename function name as per description above something like
> 
> check_pbist_results_of_mcu_domain,, if you agree
> 
> Also give more context, I believe, ROM runs BIST on MCU domain, Please consider 
> to mention, if you want

Yes will do!

> 
>> +{
>> +    bool is_done, timed_out;
>> +    u32 mask;
>> +    u32 post_reg_val, shift;
>> +
>> +    /* Read HW POST status register */
>> +    post_reg_val = readl(WKUP_CTRL_MMR0_BASE + 
>> WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT);
>> +
>> +    /* Check if HW POST PBIST was performed */
>> +    shift = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_DONE_SHIFT;
>> +    is_done = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? (bool)true : 
>> (bool)false;
>> +
>> +    if (!is_done) {
>> +        /* HW POST: PBIST not completed, check if it timed out */
>> +        shift = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_TIMEOUT_SHIFT;
> Too long macro name
>> +        timed_out = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? (bool)true : 
>> (bool)false;
>> +
>> +        if (!timed_out) {
>> +            debug("%s: PBIST was not performed at all on this device for this 
>> core\n",
>> +                  __func__);
>> +            return -EINVAL;
> 
> This is error no ? , move to dev_err instead of debug
> 

The return in k3_bist_probe throws a dev_err saying HW POST failed to run 
successfully. So these were added as debugs in case the end user wants to know 
exact cause of failure, I can move it as a dev_err as well.

> 
>> +        } else {
>> +            debug("%s: PBIST was attempted but timed out for this section\n", 
>> __func__);
>> +            return -ETIMEDOUT;
> 
> This is error no ? , move to dev_err instead of debug .
> 
> What next, reboot SOC or just continue booting in case of error

This is also something I wanted this RFC to address, I prefer rebooting SoC if 
HW POST fails. HW POST is performed by ROM based on certain switch settings, 
which implies that an end-user wants this check done if selected. And if it 
fails on the MCU domain itself, I do not think we should continue.
> 
> 
>> +        }
>> +    } else {
>> +        /* HW POST: PBIST was completed on this device, check the result */
>> +        mask = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_FAIL_MASK;
>> +
>> +        if ((post_reg_val & mask) != 0) {
>> +            debug("%s: PBIST was completed, but the test failed\n", __func__);
>> +            return -EINVAL;
>> +        } else {
>> +            debug("%s: HW POST PBIST completed, test passed\n", __func__);
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +/**
>> + * core_get_by_index() - Configure processor to correct state
> 
> 
> Two operation here. please rename if possible
> 

Will do, thanks!

> 
>> + *
>> + * Function to configure processor under test to correct state for SW-initiated
>> + * PBIST
>> + * @dev: BIST device
>> + * @index: corresponding index of the core in the cores-under-test list
>> + * @turnoff: true if core is needed to be turned off
>> + *
>> + * Return: 0 if all went fine, else corresponding error.
>> + */
>> +int core_get_by_index(struct udevice *dev, int index, bool turnoff)
>> +{
>> +    struct ofnode_phandle_args args;
>> +    int ret;
>> +    struct udevice *dev_core;
>> +
>> +    ret = dev_read_phandle_with_args(dev, "cores-under-test", NULL, 0, index, 
>> &args);
>> +    if (ret) {
>> +        debug("%s: dev_read_phandle_with_args failed: %d\n", __func__,
>> +              ret);
>> +        return ret;
>> +    }
>> +    ret =  uclass_get_device_by_ofnode(UCLASS_REMOTEPROC, args.node, &dev_core);
>> +    if (ret) {
>> +        debug("%s: uclass_get_device_by_of_offset failed: %d\n",
>> +              __func__, ret);
>> +        return ret;
>> +    }
>> +
>> +    if (turnoff) {
>> +        struct power_domain pwrdmn;
>> +        struct clk fclk;
>> +
> 
> isn't some tisci call are ok to turn off the CPUs.
> 

DM (Device Manager) firmware, responsible for power management is not up at this 
point in the boot flow (R5 SPL). Thus TISCI calls that turn on/turn off clocks 
and power domains are not available and we rely on the primitive clk-k3.c and 
ti-power-domain.c drivers to do this for us. As seen below, using the uclass 
functions which internally calls these primitive drivers would be the way to go.

I could have used the remoteproc framework to do this but currently the rproc 
driver uses TISCI firmware calls from DM and as mentioned above that's not possible.

We should probably target modifying our remoteproc driver to use these generic 
uclass APIs instead of direct TISCI calls.

> 
>> +        ret = power_domain_get_by_index(dev_core, &pwrdmn, 0);
>> +        if (ret) {
>> +            dev_err(dev, "failed to get power domain for the core %d\n", ret);
>> +            return ret;
>> +        }
>> +
>> +        ret = clk_get_by_index(dev_core, 0, &fclk);
>> +        if (ret) {
>> +            dev_err(dev, "failed to get clock for the core %d\n", ret);
>> +            return ret;
>> +        }
>> +
>> +        ret = power_domain_off(&pwrdmn);
>> +        if (ret) {
>> +            dev_err(dev, "failed to power OFF the core %d\n", ret);
>> +            return ret;
>> +        }
>> +
>> +        ret = power_domain_free(&pwrdmn);
>> +        if (ret) {
>> +            dev_err(dev, "failed to free the core %d\n", ret);
>> +            return ret;
>> +        }
>> +        ret = clk_disable(&fclk);
>> +        if (ret) {
>> +            dev_err(dev, "failed to disable clock of the core %d\n", ret);
>> +            return ret;
>> +        }
>> +    }
>> +    return 0;
>> +}
>> +
>> +/**
>> + * pbist_self_test() - Run PBIST_TEST on specified cores
>> + * @config: pbist_config structure for PBIST test
>> + *
>> + * Function to run PBIST_TEST
>> + *
>> + * Return: 0 if all went fine, else corresponding error.
>> + */
>> +int pbist_self_test(struct pbist_config *config)
>> +{
>> +    struct udevice *dev = k3_bist_priv->dev;
>> +    void *base = k3_bist_priv->base;
>> +    u32 intr_num = k3_bist_priv->intr_num;
>> +    bool test_result = true;
>> +
>> +    /* Turns on PBIST clock in PBIST ACTivate register */
>> +    writel(PBIST_PACT_PACT_MASK, base + PBIST_PACT);
>> +
>> +    /* Set Margin mode register for Test mode */
>> +    writel(PBIST_TEST_MODE, base + PBIST_MARGIN_MODE);
>> +
>> +    /* Zero out Loop counter 0 */
>> +    writel(0x0, base + PBIST_L0);
>> +
>> +    /* Set algorithm bitmap */
>> +    writel(config->algorithms_bit_map, base + PBIST_ALGO);
>> +
>> +    /* Set Memory group bitmap */
>> +    writel(config->memory_groups_bit_map, base + PBIST_RINFO);
>> +
>> +    /* Zero out override register */
>> +    writel(config->override, base + PBIST_OVER);
>> +
>> +    /* Set Scramble value - 64 bit*/
>> +    writel(config->scramble_value_lo, base + PBIST_SCR_LO);
>> +    writel(config->scramble_value_hi, base + PBIST_SCR_HI);
>> +
>> +    /* Set DLR register for ROM based testing and Config Access */
>> +    writel(PBIST_DLR_DLR0_ROM_MASK
>> +    | PBIST_DLR_DLR0_CAM_MASK, base + PBIST_DLR);
>> +
>> +    udelay(1000);
>> +
>> +    u32 timeout_count = 0;
> Please move timeout_count at start of function
>> +
>> +    while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
>> +           (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
>> +        ;
> 
> Do you want to add some delay instead of just reading in a loop
> 

That is possible... is there any benefit of delay over polling? Eithercase the 
possible time would be at max be PBIST_MAX_TIMEOUT_VALUE.

> 
>> +
>> +    if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
>> +        test_result = false;
>> +        debug("%s(dev=%p): test failed\n", __func__, dev);
> 
> Fail is error , no ?
> 
> 
>> +    } else {
>> +        debug("%s(dev=%p): test passed\n", __func__, dev);
>> +    }
>> +
>> +    writel(0xffffffff, VIM_STS(intr_num));
>> +
>> +    return 0;
> 
> 
> Caller always will see success
> 

Right, I wasn't sure about what action to take based on the result.

> 
>> +}
>> +
>> +/**
>> + * pbist_neg_self_test() - Run PBIST_negTEST on specified cores
>> + * @config: pbist_config_neg structure for PBIST negative test
>> + *
>> + * Function to run PBIST failure insertion test
>> + *
>> + * Return: 0 if all went fine, else corresponding error.
>> + */
>> +int pbist_neg_self_test(struct pbist_config_neg *config)
>> +{
>> +    struct udevice *dev = k3_bist_priv->dev;
>> +    void *base = k3_bist_priv->base;
>> +    u32 intr_num = k3_bist_priv->intr_num;
>> +    bool test_result = true;
>> +
>> +    /* Turns on PBIST clock in PBIST ACTivate register */
>> +    writel(PBIST_PACT_PACT_MASK, base + PBIST_PACT);
>> +
>> +    /* Set Margin mode register for Test mode */
>> +    writel(PBIST_FAILURE_INSERTION_TEST_MODE, base + PBIST_MARGIN_MODE);
>> +
>> +    /* Zero out Loop counter 0 */
>> +    writel(0x0, base + PBIST_L0);
>> +
>> +    /* Set DLR register */
>> +    writel(0x10, base + PBIST_DLR);
>> +
>> +    /* Set Registers*/
>> +    writel(0x00000001, base + PBIST_RF0L);
>> +    writel(0x00003123, base + PBIST_RF0U);
>> +    writel(0x0513FC02, base + PBIST_RF1L);
>> +    writel(0x00000002, base + PBIST_RF1U);
>> +    writel(0x00000003, base + PBIST_RF2L);
>> +    writel(0x00000000, base + PBIST_RF2U);
>> +    writel(0x00000004, base + PBIST_RF3L);
>> +    writel(0x00000028, base + PBIST_RF3U);
>> +    writel(0x64000044, base + PBIST_RF4L);
>> +    writel(0x00000000, base + PBIST_RF4U);
>> +    writel(0x0006A006, base + PBIST_RF5L);
>> +    writel(0x00000000, base + PBIST_RF5U);
>> +    writel(0x00000007, base + PBIST_RF6L);
>> +    writel(0x0000A0A0, base + PBIST_RF6U);
>> +    writel(0x00000008, base + PBIST_RF7L);
>> +    writel(0x00000064, base + PBIST_RF7U);
>> +    writel(0x00000009, base + PBIST_RF8L);
>> +    writel(0x0000A5A5, base + PBIST_RF8U);
>> +    writel(0x0000000A, base + PBIST_RF9L);
>> +    writel(0x00000079, base + PBIST_RF9U);
>> +    writel(0x00000000, base + PBIST_RF10L);
>> +    writel(0x00000001, base + PBIST_RF10U);
>> +    writel(0xAAAAAAAA, base + PBIST_D);
>> +    writel(0xAAAAAAAA, base + PBIST_E);
> 
> too much direct values
> 

Will try seeing if this sequence has a meaning I can move to MACROS for.

>> +
>> +    writel(config->CA2, base + PBIST_CA2);
>> +    writel(config->CL0, base + PBIST_CL0);
>> +    writel(config->CA3, base + PBIST_CA3);
>> +    writel(config->I0, base + PBIST_I0);
>> +    writel(config->CL1, base + PBIST_CL1);
>> +    writel(config->I3, base + PBIST_I3);
>> +    writel(config->I2, base + PBIST_I2);
>> +    writel(config->CL2, base + PBIST_CL2);
>> +    writel(config->CA1, base + PBIST_CA1);
>> +    writel(config->CA0, base + PBIST_CA0);
>> +    writel(config->CL3, base + PBIST_CL3);
>> +    writel(config->I1, base + PBIST_I1);
>> +    writel(config->RAMT, base + PBIST_RAMT);
>> +    writel(config->CSR, base + PBIST_CSR);
>> +    writel(config->CMS, base + PBIST_CMS);
>> +
>> +    writel(0x00000009, base + PBIST_STR);
>> +
>> +    /* Start PBIST */
>> +    writel(0x00000001, base + PBIST_STR);
>> +
>> +    udelay(1000);
>> +
>> +    u32 timeout_count = 0;
>> +
>> +    while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
>> +           (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
>> +        ;
>> +
>> +    if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
>> +        test_result = false;
>> +        debug("%s(dev=%p): test failed\n", __func__, dev);
>> +    } else {
>> +        debug("%s(dev=%p): test passed\n", __func__, dev);
>> +    }
>> +
>> +    writel(0xffffffff, VIM_STS(intr_num));
>> +
>> +    return 0;
> 
> Same as in above function for error
> 
>> +}
>> +
>> +/**
>> + * pbist_rom_self_test() - Run PBIST_ROM_TEST on specified cores
>> + * @config: pbist_config_rom structure for PBIST negative test
>> + *
>> + * Function to run PBIST test of ROM
>> + *
>> + * Return: 0 if all went fine, else corresponding error.
>> + */
>> +int pbist_rom_self_test(struct pbist_config_rom *config)
>> +{
>> +    struct udevice *dev = k3_bist_priv->dev;
>> +    void *base = k3_bist_priv->base;
>> +    u32 intr_num = k3_bist_priv->intr_num;
>> +    bool test_result = true;
>> +
>> +    /* Turns on PBIST clock in PBIST ACTivate register */
>> +    writel(0x1, base + PBIST_PACT);
>> +
>> +    /* Set Margin mode register for Test mode */
>> +    writel(0xf, base + PBIST_MARGIN_MODE);
>> +
>> +    /* Zero out Loop counter 0 */
>> +    writel(0x0, base + PBIST_L0);
>> +
>> +    /* Set DLR register */
>> +    writel(0x310, base + PBIST_DLR);
>> +
>> +    /* Set Registers*/
>> +    writel(0x00000001, base + PBIST_RF0L);
>> +    writel(0x00003123, base + PBIST_RF0U);
>> +    writel(0x7A400183, base + PBIST_RF1L);
>> +    writel(0x00000060, base + PBIST_RF1U);
>> +    writel(0x00000184, base + PBIST_RF2L);
>> +    writel(0x00000000, base + PBIST_RF2U);
>> +    writel(0x7B600181, base + PBIST_RF3L);
>> +    writel(0x00000061, base + PBIST_RF3U);
>> +    writel(0x00000000, base + PBIST_RF4L);
>> +    writel(0x00000000, base + PBIST_RF4U);
>> +
>> +    writel(config->D, base + PBIST_D);
>> +    writel(config->E, base + PBIST_E);
>> +    writel(config->CA2, base + PBIST_CA2);
>> +    writel(config->CL0, base + PBIST_CL0);
>> +    writel(config->CA3, base + PBIST_CA3);
>> +    writel(config->I0, base + PBIST_I0);
>> +    writel(config->CL1, base + PBIST_CL1);
>> +    writel(config->I3, base + PBIST_I3);
>> +    writel(config->I2, base + PBIST_I2);
>> +    writel(config->CL2, base + PBIST_CL2);
>> +    writel(config->CA1, base + PBIST_CA1);
>> +    writel(config->CA0, base + PBIST_CA0);
>> +    writel(config->CL3, base + PBIST_CL3);
>> +    writel(config->I1, base + PBIST_I1);
>> +    writel(config->RAMT, base + PBIST_RAMT);
>> +    writel(config->CSR, base + PBIST_CSR);
>> +    writel(config->CMS, base + PBIST_CMS);
>> +
>> +    writel(0x00000009, base + PBIST_STR);
>> +
>> +    /* Start PBIST */
>> +    writel(0x00000001, base + PBIST_STR);
>> +
>> +    udelay(1000);
>> +
> Why delay is needed. please add comment for that
>> +    u32 timeout_count = 0;
> same as above
>> +
>> +    while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
>> +           (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
>> +        ;
>> +
>> +    if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
>> +        test_result = false;
>> +        debug("%s(dev=%p): test failed\n", __func__, dev);
>> +    } else {
>> +        debug("%s(dev=%p): test passed\n", __func__, dev);
>> +    }
>> +
>> +    writel(0xffffffff, VIM_STS(intr_num));
>> +
>> +    return 0;
>> +}
>> +
>> +/**
>> + * k3_bist_probe() - Basic probe
>> + * @dev: corresponding BIST device
>> + *
>> + * Parses BIST info from device tree, and configures the module accordingly.
>> + * Return: 0 if all goes good, else appropriate error message.
>> + */
>> +static int k3_bist_probe(struct udevice *dev)
>> +{
>> +    int ret = 0, num_runs, i, j;
>> +    struct k3_bist_privdata *priv = dev_get_priv(dev);
>> +    struct pbist_inst_info *info;
>> +
>> +    debug("%s(dev=%p)\n", __func__, dev);
>> +
>> +    priv = dev_get_priv(dev);
> 
> NULL error check for priv
> 

Got it, thanks!

> 
>> +    priv->dev = dev;
>> +
>> +    k3_bist_priv = priv;
>> +
>> +    priv->base = dev_remap_addr_index(dev, 0);
>> +    if (!priv->base)
>> +        return -ENODEV;
>> +
>> +    ret = dev_read_u32(dev, "ti,bist-instance", &priv->instance);
>> +    if (!priv->instance)
>> +        return -ENODEV;
>> +
>> +    switch (priv->instance) {
>> +    case PBIST14_INSTANCE:
>> +        info = &pbist14_inst_info;
>> +        priv->intr_num = info->intr_num;
>> +        break;
>> +    default:
>> +        dev_err(dev, "%s: PBIST instance %d not supported\n", __func__, 
>> priv->instance);
>> +        return -ENODEV;
>> +    };
>> +
>> +    /* Probe the cores under test */
>> +    for (i = 0; ; i++) {
>> +        ret = core_get_by_index(dev, i, false);
>> +        if (ret)
>> +            break;
>> +    }
>> +
>> +    if (!i) {
>> +        dev_err(dev, "%s: Acquiring the core failed. ret = %d\n", __func__, 
>> ret);
>> +        return ret;
>> +    }
> 
> Please add a check, what you expect in device tree at start of probe .
> 
> I assume, you can hit this only case of incorrect DT
> 

Right, will add a check at the start.

> 
>> +
>> +    /* Check whether HW POST successfully completely PBIST on the MCU domain */
>> +    ret = pbist_run_post_pbist_check();
>> +    if (ret) {
>> +        dev_err(dev, "HW POST failed to run successfully %d\n", ret);
>> +        return ret;
>> +    }
>> +
> 
> you might need to do this check first. before probing other cores
> 

Hm yes okay, can move that in front.

> 
>> +    /* Run PBIST test */
>> +    num_runs = info->num_pbist_runs;
> if you want num_runs configurable, prefer to use DT
>> +
>> +    for (j = 0; j < num_runs; j++) {
>> +        ret = pbist_self_test(&info->pbist_config_run[j]);
>> +        if (ret) {
>> +            dev_err(dev, "failed to run PBIST test %d\n", ret);
>> +            return ret;
>> +        }
>> +    }
> Dummy question, will above run the BIST on all selected cores ?

A probe of a single BIST instance will run the BIST test on all it's 
modules/cores. So yes, each test is triggering the BIST run on all cores.

>> +
>> +    /* Run PBIST failure insertion test */
>> +    ret = pbist_neg_self_test(&info->pbist_neg_config_run);
>> +    if (ret) {
>> +        dev_err(dev, "failed to run PBIST negative test %d\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    /* Run PBIST test on ROM */
>> +    num_runs = info->num_pbist_rom_test_runs;
>> +
>> +    for (j = 0; j < num_runs; j++) {
>> +        ret = pbist_rom_self_test(&info->pbist_rom_test_config_run[j]);
>> +        if (ret) {
>> +            dev_err(dev, "failed to run ROM PBIST test %d\n", ret);
>> +            return ret;
>> +        }
>> +    }
>> +
>> +    /* Power off cores under test */
>> +    while (i--) {
>> +        ret = core_get_by_index(dev, i, true);
>> +        if (ret)
>> +            break;
>> +    }
> 
> can we get rid from this 'i' , May you can do all core ON and OFF in one function,
> 
> Largely its from device tree

Okay I will try changing this.

> 
>> +
>> +    if (i) {
>> +        dev_err(dev, "%s: Stopping the core failed. ret = %d\n", __func__, ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct udevice_id k3_bist_ids[] = {
>> +    { .compatible = "ti,j784s4-bist" },
>> +    {}
>> +};
>> +
>> +U_BOOT_DRIVER(k3_bist) = {
>> +    .name = "k3_bist",
>> +    .of_match = k3_bist_ids,
>> +    .id = UCLASS_MISC,
>> +    .probe = k3_bist_probe,
>> +    .priv_auto = sizeof(struct k3_bist_privdata),
>> +};
>> diff --git a/drivers/misc/k3_bist_static_data.h 
>> b/drivers/misc/k3_bist_static_data.h
>> new file mode 100644
>> index 00000000000..f30fb7935b6
>> --- /dev/null
>> +++ b/drivers/misc/k3_bist_static_data.h
>> @@ -0,0 +1,551 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Static Data for Texas Instruments' BIST (Built-In Self-Test) driver
>> + *
>> + * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
>> + *
>> + */
>> +
>> +#ifndef __K3_BIST_STATIC_DATA_H
>> +#define __K3_BIST_STATIC_DATA_H
>> +
>> +#define PBIST_MAX_NUM_RUNS    2
>> +#define NUM_MAX_PBIST_TEST_ROM_RUNS 13
>> +#define PBIST14_DFT_PBIST_CPU_0_INTR_NUM 311
>> +
>> +/* VIM Registers */
>> +#define VIM_STS_BASE                          0x40f80404
>> +#define VIM_RAW_BASE                          0x40f80400
>> +
>> +#define VIM_STS(i)            (VIM_STS_BASE + (i) / 32 * 0x20)
>> +#define VIM_RAW(i)          (VIM_RAW_BASE + (i) / 32 * 0x20)
>> +#define VIM_RAW_MASK(i)     (BIT((i) % 32))
> 
> Above this SOC specific data , Please have some SOC specific data in another 
> header file and include here.
> 
> So that adding next SOC will be easy
> 
>> [..]
>> +#if IS_ENABLED(CONFIG_SOC_K3_J784S4)
> 
> Please put this data in SOC specific header file

Okay I will create an SoC specific header file.

> 
>> [..]

Thanks for reviewing!
Kumar, Udit Sept. 5, 2024, 11:50 a.m. UTC | #3
On 9/5/2024 3:11 PM, Neha Malcom Francis wrote:
> On 04/09/24 10:35, Kumar, Udit wrote:
>>
>> On 9/3/2024 5:14 PM, Neha Malcom Francis wrote:
>>> Add a driver for the BIST module which currently includes support for
>>> BIST IPs that trigger PBIST (Memory BIST).
>>>
>>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>>> ---
>>>   drivers/misc/Kconfig               |   8 +
>>>   drivers/misc/Makefile              |   1 +
>>>   drivers/misc/k3_bist.c             | 507 ++++++++++++++++++++++++++
>>>   drivers/misc/k3_bist_static_data.h | 551 
>>> +++++++++++++++++++++++++++++
>>>   4 files changed, 1067 insertions(+)
>>>   create mode 100644 drivers/misc/k3_bist.c
>>>   create mode 100644 drivers/misc/k3_bist_static_data.h
>>>
>>> [...]
>>> +
>>
>>
>> In general, few macro's name are too long
>>
>> many places hard-coded values are used, please consider to move to macro
>>
>> driver looks to be j784s4 specific including header files  ,
>>
>> please see, if we can make this generic driver.
>
> I've put SoC specific (J784S4 right now) data protected with SoC 
> specific configs in k3_bist_static_data.h; the hardcoded values are a 
> sequence for triggering a specific test, whatever is generic and known 
> I have put as a macro, however I'll try to better understand the 
> sequence if I can put them as macros.
>
>>
>>
>>> +#include "k3_bist_static_data.h"
>>> +
>>> [...]
>>
>> Please consider to rename function name as per description above 
>> something like
>>
>> check_pbist_results_of_mcu_domain,, if you agree
>>
>> Also give more context, I believe, ROM runs BIST on MCU domain, 
>> Please consider to mention, if you want
>
> Yes will do!
>
>>
>>> +{
>>> +    bool is_done, timed_out;
>>> +    u32 mask;
>>> +    u32 post_reg_val, shift;
>>> +
>>> +    /* Read HW POST status register */
>>> +    post_reg_val = readl(WKUP_CTRL_MMR0_BASE + 
>>> WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT);
>>> +
>>> +    /* Check if HW POST PBIST was performed */
>>> +    shift = 
>>> WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_DONE_SHIFT;
>>> +    is_done = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? 
>>> (bool)true : (bool)false;
>>> +
>>> +    if (!is_done) {
>>> +        /* HW POST: PBIST not completed, check if it timed out */
>>> +        shift = 
>>> WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_TIMEOUT_SHIFT;
>> Too long macro name
>>> +        timed_out = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? 
>>> (bool)true : (bool)false;
>>> +
>>> +        if (!timed_out) {
>>> +            debug("%s: PBIST was not performed at all on this 
>>> device for this core\n",
>>> +                  __func__);
>>> +            return -EINVAL;
>>
>> This is error no ? , move to dev_err instead of debug
>>
>
> The return in k3_bist_probe throws a dev_err saying HW POST failed to 
> run successfully. So these were added as debugs in case the end user 
> wants to know exact cause of failure, I can move it as a dev_err as well.

ok, thanks


>
>>
>>> +        } else {
>>> +            debug("%s: PBIST was attempted but timed out for this 
>>> section\n", __func__);
>>> +            return -ETIMEDOUT;
>>
>> This is error no ? , move to dev_err instead of debug .
>>
>> What next, reboot SOC or just continue booting in case of error
>
> This is also something I wanted this RFC to address, I prefer 
> rebooting SoC if HW POST fails. HW POST is performed by ROM based on 
> certain switch settings, which implies that an end-user wants this 
> check done if selected. And if it fails on the MCU domain itself, I do 
> not think we should continue.

Ok, if you are saying BIST in MCU domain in done based upon some switch 
settings, then please put that switch logic here .

Reading code above looks, BIST run always

if (!is_done) {
+		/* HW POST: PBIST not completed, check if it timed out */
+	} else {
+		/* HW POST: PBIST was completed on this device, check the result */

So now have three conditions,
1) BIST not attempted
2) BIST ran and passed
3) BIST ran and failed

For 1) condition, please see if you can read some register or switch 
settings or so.

For 3) , as default we should hang.. In case users wants to continue on 
failure they can modify u-boot source to do so :)


>>
>>
>>> [..]
>>> isn't some tisci call are ok to turn off the CPUs.
>>>
>
> DM (Device Manager) firmware, responsible for power management is not 
> up at this point in the boot flow (R5 SPL). Thus TISCI calls that turn 
> on/turn off clocks and power domains are not available and we rely on 
> the primitive clk-k3.c and ti-power-domain.c drivers to do this for 
> us. As seen below, using the uclass functions which internally calls 
> these primitive drivers would be the way to go.
>
But DM is kind implemented by dev-data and dev-clk logic ?

For few calls DM just talk to TIFS, which are available at this point.

I request to check once, if possible


> I could have used the remoteproc framework to do this but currently 
> the rproc driver uses TISCI firmware calls from DM and as mentioned 
> above that's not possible.
>
> We should probably target modifying our remoteproc driver to use these 
> generic uclass APIs instead of direct TISCI calls.

Ok,

>
>>
>>> [...]
>>> +    udelay(1000);
>>> +
>>> +    u32 timeout_count = 0;
>> Please move timeout_count at start of function
>>> +
>>> +    while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
>>> +           (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
>>> +        ;
>>
>> Do you want to add some delay instead of just reading in a loop
>>
>
> That is possible... is there any benefit of delay over polling? 
> Eithercase the possible time would be at max be PBIST_MAX_TIMEOUT_VALUE.


You could be definitive in waiting for this register,

Say if hardware specs says, VIM register will be set in max time of 100 
ms. then

keep PBIST_MAX_TIMEOUT_VALUE as 100 and read wait for 1 ms and then read.


>
>>
>>> +
>>> +    if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
>>> +        test_result = false;
>>> +        debug("%s(dev=%p): test failed\n", __func__, dev);
>>
>> Fail is error , no ?
>>
>>
>>> +    } else {
>>> +        debug("%s(dev=%p): test passed\n", __func__, dev);
>>> +    }
>>> +
>>> +    writel(0xffffffff, VIM_STS(intr_num));
>>> +
>>> +    return 0;
>>
>>
>> Caller always will see success
>>
>
> Right, I wasn't sure about what action to take based on the result.


Failure is failure, as default please print and call hang

>
>>
>>> +}
>>> +
>>> +/**
>>> [..]
Neha Malcom Francis Sept. 10, 2024, 9:11 a.m. UTC | #4
Hi Udit,

On 05/09/24 17:20, Kumar, Udit wrote:
> 
> On 9/5/2024 3:11 PM, Neha Malcom Francis wrote:
>> On 04/09/24 10:35, Kumar, Udit wrote:
>>>
>>> On 9/3/2024 5:14 PM, Neha Malcom Francis wrote:
>>>> Add a driver for the BIST module which currently includes support for
>>>> BIST IPs that trigger PBIST (Memory BIST).
>>>>
>>>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>>>> ---
>>>>   drivers/misc/Kconfig               |   8 +
>>>>   drivers/misc/Makefile              |   1 +
>>>>   drivers/misc/k3_bist.c             | 507 ++++++++++++++++++++++++++
>>>>   drivers/misc/k3_bist_static_data.h | 551 +++++++++++++++++++++++++++++
>>>>   4 files changed, 1067 insertions(+)
>>>>   create mode 100644 drivers/misc/k3_bist.c
>>>>   create mode 100644 drivers/misc/k3_bist_static_data.h
>>>>
>>>> [...]
>>>> +
>>>
>>>
>>> In general, few macro's name are too long
>>>
>>> many places hard-coded values are used, please consider to move to macro
>>>
>>> driver looks to be j784s4 specific including header files  ,
>>>
>>> please see, if we can make this generic driver.
>>
>> I've put SoC specific (J784S4 right now) data protected with SoC specific 
>> configs in k3_bist_static_data.h; the hardcoded values are a sequence for 
>> triggering a specific test, whatever is generic and known I have put as a 
>> macro, however I'll try to better understand the sequence if I can put them as 
>> macros.
>>
>>>
>>>
>>>> +#include "k3_bist_static_data.h"
>>>> +
>>>> [...]
>>>
>>> Please consider to rename function name as per description above something like
>>>
>>> check_pbist_results_of_mcu_domain,, if you agree
>>>
>>> Also give more context, I believe, ROM runs BIST on MCU domain, Please 
>>> consider to mention, if you want
>>
>> Yes will do!
>>
>>>
>>>> +{
>>>> +    bool is_done, timed_out;
>>>> +    u32 mask;
>>>> +    u32 post_reg_val, shift;
>>>> +
>>>> +    /* Read HW POST status register */
>>>> +    post_reg_val = readl(WKUP_CTRL_MMR0_BASE + 
>>>> WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT);
>>>> +
>>>> +    /* Check if HW POST PBIST was performed */
>>>> +    shift = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_DONE_SHIFT;
>>>> +    is_done = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? (bool)true : 
>>>> (bool)false;
>>>> +
>>>> +    if (!is_done) {
>>>> +        /* HW POST: PBIST not completed, check if it timed out */
>>>> +        shift = 
>>>> WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_TIMEOUT_SHIFT;
>>> Too long macro name
>>>> +        timed_out = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? (bool)true 
>>>> : (bool)false;
>>>> +
>>>> +        if (!timed_out) {
>>>> +            debug("%s: PBIST was not performed at all on this device for 
>>>> this core\n",
>>>> +                  __func__);
>>>> +            return -EINVAL;
>>>
>>> This is error no ? , move to dev_err instead of debug
>>>
>>
>> The return in k3_bist_probe throws a dev_err saying HW POST failed to run 
>> successfully. So these were added as debugs in case the end user wants to know 
>> exact cause of failure, I can move it as a dev_err as well.
> 
> ok, thanks
> 
> 
>>
>>>
>>>> +        } else {
>>>> +            debug("%s: PBIST was attempted but timed out for this 
>>>> section\n", __func__);
>>>> +            return -ETIMEDOUT;
>>>
>>> This is error no ? , move to dev_err instead of debug .
>>>
>>> What next, reboot SOC or just continue booting in case of error
>>
>> This is also something I wanted this RFC to address, I prefer rebooting SoC if 
>> HW POST fails. HW POST is performed by ROM based on certain switch settings, 
>> which implies that an end-user wants this check done if selected. And if it 
>> fails on the MCU domain itself, I do not think we should continue.
> 
> Ok, if you are saying BIST in MCU domain in done based upon some switch 
> settings, then please put that switch logic here .
> 
> Reading code above looks, BIST run always
> 
> if (!is_done) {
> +        /* HW POST: PBIST not completed, check if it timed out */
> +    } else {
> +        /* HW POST: PBIST was completed on this device, check the result */
> 
> So now have three conditions,
> 1) BIST not attempted
> 2) BIST ran and passed
> 3) BIST ran and failed
> 
> For 1) condition, please see if you can read some register or switch settings or 
> so.
> 
> For 3) , as default we should hang.. In case users wants to continue on failure 
> they can modify u-boot source to do so :)
> 

Okay that makes sense, I will make the change in v1.
> 
>>>
>>>
>>>> [..]
>>>> isn't some tisci call are ok to turn off the CPUs.
>>>>
>>
>> DM (Device Manager) firmware, responsible for power management is not up at 
>> this point in the boot flow (R5 SPL). Thus TISCI calls that turn on/turn off 
>> clocks and power domains are not available and we rely on the primitive 
>> clk-k3.c and ti-power-domain.c drivers to do this for us. As seen below, using 
>> the uclass functions which internally calls these primitive drivers would be 
>> the way to go.
>>
> But DM is kind implemented by dev-data and dev-clk logic ?
> 

Yes, it is. I am using the uclass API for both power and clock which internally 
uses the drivers with this logic.

> For few calls DM just talk to TIFS, which are available at this point.

Yes, but from my POV this driver does not need to know what the clock and power 
APIs internally do, it should just call the uclass APIs like this. And in case 
of power off, whatever DM would have done is already covered in ti-power-domain.c
> 
> I request to check once, if possible
> 
> 
>> I could have used the remoteproc framework to do this but currently the rproc 
>> driver uses TISCI firmware calls from DM and as mentioned above that's not 
>> possible.
>>
>> We should probably target modifying our remoteproc driver to use these generic 
>> uclass APIs instead of direct TISCI calls.
> 
> Ok,
> 
>>
>>>
>>>> [...]
>>>> +    udelay(1000);
>>>> +
>>>> +    u32 timeout_count = 0;
>>> Please move timeout_count at start of function
>>>> +
>>>> +    while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
>>>> +           (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
>>>> +        ;
>>>
>>> Do you want to add some delay instead of just reading in a loop
>>>
>>
>> That is possible... is there any benefit of delay over polling? Eithercase the 
>> possible time would be at max be PBIST_MAX_TIMEOUT_VALUE.
> 
> 
> You could be definitive in waiting for this register,
> 
> Say if hardware specs says, VIM register will be set in max time of 100 ms. then
> 
> keep PBIST_MAX_TIMEOUT_VALUE as 100 and read wait for 1 ms and then read.
> 
> 

I'm not sure I understand, the timeout condition if it happens first it will 
break from the loop, so the polling will not continue indefinitely.

>>
>>>
>>>> +
>>>> +    if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
>>>> +        test_result = false;
>>>> +        debug("%s(dev=%p): test failed\n", __func__, dev);
>>>
>>> Fail is error , no ?
>>>
>>>
>>>> +    } else {
>>>> +        debug("%s(dev=%p): test passed\n", __func__, dev);
>>>> +    }
>>>> +
>>>> +    writel(0xffffffff, VIM_STS(intr_num));
>>>> +
>>>> +    return 0;
>>>
>>>
>>> Caller always will see success
>>>
>>
>> Right, I wasn't sure about what action to take based on the result.
> 
> 
> Failure is failure, as default please print and call hang
> 

Got it!

>>
>>>
>>>> +}
>>>> +
>>>> +/**
>>>> [..]
diff mbox series

Patch

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 6009d55f400..8e28a93d74c 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -664,6 +664,14 @@  config ESM_K3
 	help
 	  Support ESM (Error Signaling Module) on TI K3 SoCs.
 
+config K3_BIST
+	bool "Enable K3 BIST driver"
+	depends on ARCH_K3
+	help
+	  Support BIST (Built-In Self Test) module on TI K3 SoCs. This driver
+	  supports running both PBIST (Memory BIST) and LBIST (Logic BIST) on
+	  a region or IP in the SoC.
+
 config MICROCHIP_FLEXCOM
 	bool "Enable Microchip Flexcom driver"
 	depends on MISC
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index e53d52c47b3..15c5c4810dd 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -89,6 +89,7 @@  obj-$(CONFIG_JZ4780_EFUSE) += jz4780_efuse.o
 obj-$(CONFIG_MICROCHIP_FLEXCOM) += microchip_flexcom.o
 obj-$(CONFIG_K3_AVS0) += k3_avs.o
 obj-$(CONFIG_ESM_K3) += k3_esm.o
+obj-$(CONFIG_K3_BIST) += k3_bist.o
 obj-$(CONFIG_ESM_PMIC) += esm_pmic.o
 obj-$(CONFIG_SL28CPLD) += sl28cpld.o
 obj-$(CONFIG_SPL_SOCFPGA_DT_REG) += socfpga_dtreg.o
diff --git a/drivers/misc/k3_bist.c b/drivers/misc/k3_bist.c
new file mode 100644
index 00000000000..a4728376b73
--- /dev/null
+++ b/drivers/misc/k3_bist.c
@@ -0,0 +1,507 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Texas Instruments' BIST (Built-In Self-Test) driver
+ *
+ * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
+ *      Neha Malcom Francis <n-francis@ti.com>
+ *
+ */
+
+#include <dm.h>
+#include <errno.h>
+#include <clk.h>
+#include <asm/io.h>
+#include <dm/device_compat.h>
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <asm/arch/hardware.h>
+#include <linux/soc/ti/ti_sci_protocol.h>
+#include <remoteproc.h>
+#include <power-domain.h>
+
+#include "k3_bist_static_data.h"
+
+/* PBIST Timeout Value */
+#define PBIST_MAX_TIMEOUT_VALUE		100000000
+
+/**
+ * struct k3_bist_privdata - K3 BIST structure
+ * @dev: device pointer
+ * @base: base of register set
+ * @instance: PBIST instance number
+ * @intr_num: corresponding interrupt ID of the PBIST instance
+ */
+struct k3_bist_privdata {
+	struct udevice *dev;
+	void *base;
+	u32 instance;
+	u32 intr_num;
+};
+
+static struct k3_bist_privdata *k3_bist_priv;
+
+/**
+ * pbist_run_post_pbist_check() - Check POST results
+ *
+ * Function to check whether HW Power-On Self Test, i.e. POST has run
+ * successfully on the MCU domain.
+ *
+ * Return: 0 if all went fine, else corresponding error.
+ */
+int pbist_run_post_pbist_check(void)
+{
+	bool is_done, timed_out;
+	u32 mask;
+	u32 post_reg_val, shift;
+
+	/* Read HW POST status register */
+	post_reg_val = readl(WKUP_CTRL_MMR0_BASE + WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT);
+
+	/* Check if HW POST PBIST was performed */
+	shift = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_DONE_SHIFT;
+	is_done = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? (bool)true : (bool)false;
+
+	if (!is_done) {
+		/* HW POST: PBIST not completed, check if it timed out */
+		shift = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_TIMEOUT_SHIFT;
+		timed_out = (((post_reg_val >> shift) & 0x1u) == 0x1u) ? (bool)true : (bool)false;
+
+		if (!timed_out) {
+			debug("%s: PBIST was not performed at all on this device for this core\n",
+			      __func__);
+			return -EINVAL;
+		} else {
+			debug("%s: PBIST was attempted but timed out for this section\n", __func__);
+			return -ETIMEDOUT;
+		}
+	} else {
+		/* HW POST: PBIST was completed on this device, check the result */
+		mask = WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_FAIL_MASK;
+
+		if ((post_reg_val & mask) != 0) {
+			debug("%s: PBIST was completed, but the test failed\n", __func__);
+			return -EINVAL;
+		} else {
+			debug("%s: HW POST PBIST completed, test passed\n", __func__);
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * core_get_by_index() - Configure processor to correct state
+ *
+ * Function to configure processor under test to correct state for SW-initiated
+ * PBIST
+ * @dev: BIST device
+ * @index: corresponding index of the core in the cores-under-test list
+ * @turnoff: true if core is needed to be turned off
+ *
+ * Return: 0 if all went fine, else corresponding error.
+ */
+int core_get_by_index(struct udevice *dev, int index, bool turnoff)
+{
+	struct ofnode_phandle_args args;
+	int ret;
+	struct udevice *dev_core;
+
+	ret = dev_read_phandle_with_args(dev, "cores-under-test", NULL, 0, index, &args);
+	if (ret) {
+		debug("%s: dev_read_phandle_with_args failed: %d\n", __func__,
+		      ret);
+		return ret;
+	}
+	ret =  uclass_get_device_by_ofnode(UCLASS_REMOTEPROC, args.node, &dev_core);
+	if (ret) {
+		debug("%s: uclass_get_device_by_of_offset failed: %d\n",
+		      __func__, ret);
+		return ret;
+	}
+
+	if (turnoff) {
+		struct power_domain pwrdmn;
+		struct clk fclk;
+
+		ret = power_domain_get_by_index(dev_core, &pwrdmn, 0);
+		if (ret) {
+			dev_err(dev, "failed to get power domain for the core %d\n", ret);
+			return ret;
+		}
+
+		ret = clk_get_by_index(dev_core, 0, &fclk);
+		if (ret) {
+			dev_err(dev, "failed to get clock for the core %d\n", ret);
+			return ret;
+		}
+
+		ret = power_domain_off(&pwrdmn);
+		if (ret) {
+			dev_err(dev, "failed to power OFF the core %d\n", ret);
+			return ret;
+		}
+
+		ret = power_domain_free(&pwrdmn);
+		if (ret) {
+			dev_err(dev, "failed to free the core %d\n", ret);
+			return ret;
+		}
+		ret = clk_disable(&fclk);
+		if (ret) {
+			dev_err(dev, "failed to disable clock of the core %d\n", ret);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+/**
+ * pbist_self_test() - Run PBIST_TEST on specified cores
+ * @config: pbist_config structure for PBIST test
+ *
+ * Function to run PBIST_TEST
+ *
+ * Return: 0 if all went fine, else corresponding error.
+ */
+int pbist_self_test(struct pbist_config *config)
+{
+	struct udevice *dev = k3_bist_priv->dev;
+	void *base = k3_bist_priv->base;
+	u32 intr_num = k3_bist_priv->intr_num;
+	bool test_result = true;
+
+	/* Turns on PBIST clock in PBIST ACTivate register */
+	writel(PBIST_PACT_PACT_MASK, base + PBIST_PACT);
+
+	/* Set Margin mode register for Test mode */
+	writel(PBIST_TEST_MODE, base + PBIST_MARGIN_MODE);
+
+	/* Zero out Loop counter 0 */
+	writel(0x0, base + PBIST_L0);
+
+	/* Set algorithm bitmap */
+	writel(config->algorithms_bit_map, base + PBIST_ALGO);
+
+	/* Set Memory group bitmap */
+	writel(config->memory_groups_bit_map, base + PBIST_RINFO);
+
+	/* Zero out override register */
+	writel(config->override, base + PBIST_OVER);
+
+	/* Set Scramble value - 64 bit*/
+	writel(config->scramble_value_lo, base + PBIST_SCR_LO);
+	writel(config->scramble_value_hi, base + PBIST_SCR_HI);
+
+	/* Set DLR register for ROM based testing and Config Access */
+	writel(PBIST_DLR_DLR0_ROM_MASK
+	| PBIST_DLR_DLR0_CAM_MASK, base + PBIST_DLR);
+
+	udelay(1000);
+
+	u32 timeout_count = 0;
+
+	while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
+	       (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
+		;
+
+	if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
+		test_result = false;
+		debug("%s(dev=%p): test failed\n", __func__, dev);
+	} else {
+		debug("%s(dev=%p): test passed\n", __func__, dev);
+	}
+
+	writel(0xffffffff, VIM_STS(intr_num));
+
+	return 0;
+}
+
+/**
+ * pbist_neg_self_test() - Run PBIST_negTEST on specified cores
+ * @config: pbist_config_neg structure for PBIST negative test
+ *
+ * Function to run PBIST failure insertion test
+ *
+ * Return: 0 if all went fine, else corresponding error.
+ */
+int pbist_neg_self_test(struct pbist_config_neg *config)
+{
+	struct udevice *dev = k3_bist_priv->dev;
+	void *base = k3_bist_priv->base;
+	u32 intr_num = k3_bist_priv->intr_num;
+	bool test_result = true;
+
+	/* Turns on PBIST clock in PBIST ACTivate register */
+	writel(PBIST_PACT_PACT_MASK, base + PBIST_PACT);
+
+	/* Set Margin mode register for Test mode */
+	writel(PBIST_FAILURE_INSERTION_TEST_MODE, base + PBIST_MARGIN_MODE);
+
+	/* Zero out Loop counter 0 */
+	writel(0x0, base + PBIST_L0);
+
+	/* Set DLR register */
+	writel(0x10, base + PBIST_DLR);
+
+	/* Set Registers*/
+	writel(0x00000001, base + PBIST_RF0L);
+	writel(0x00003123, base + PBIST_RF0U);
+	writel(0x0513FC02, base + PBIST_RF1L);
+	writel(0x00000002, base + PBIST_RF1U);
+	writel(0x00000003, base + PBIST_RF2L);
+	writel(0x00000000, base + PBIST_RF2U);
+	writel(0x00000004, base + PBIST_RF3L);
+	writel(0x00000028, base + PBIST_RF3U);
+	writel(0x64000044, base + PBIST_RF4L);
+	writel(0x00000000, base + PBIST_RF4U);
+	writel(0x0006A006, base + PBIST_RF5L);
+	writel(0x00000000, base + PBIST_RF5U);
+	writel(0x00000007, base + PBIST_RF6L);
+	writel(0x0000A0A0, base + PBIST_RF6U);
+	writel(0x00000008, base + PBIST_RF7L);
+	writel(0x00000064, base + PBIST_RF7U);
+	writel(0x00000009, base + PBIST_RF8L);
+	writel(0x0000A5A5, base + PBIST_RF8U);
+	writel(0x0000000A, base + PBIST_RF9L);
+	writel(0x00000079, base + PBIST_RF9U);
+	writel(0x00000000, base + PBIST_RF10L);
+	writel(0x00000001, base + PBIST_RF10U);
+	writel(0xAAAAAAAA, base + PBIST_D);
+	writel(0xAAAAAAAA, base + PBIST_E);
+
+	writel(config->CA2, base + PBIST_CA2);
+	writel(config->CL0, base + PBIST_CL0);
+	writel(config->CA3, base + PBIST_CA3);
+	writel(config->I0, base + PBIST_I0);
+	writel(config->CL1, base + PBIST_CL1);
+	writel(config->I3, base + PBIST_I3);
+	writel(config->I2, base + PBIST_I2);
+	writel(config->CL2, base + PBIST_CL2);
+	writel(config->CA1, base + PBIST_CA1);
+	writel(config->CA0, base + PBIST_CA0);
+	writel(config->CL3, base + PBIST_CL3);
+	writel(config->I1, base + PBIST_I1);
+	writel(config->RAMT, base + PBIST_RAMT);
+	writel(config->CSR, base + PBIST_CSR);
+	writel(config->CMS, base + PBIST_CMS);
+
+	writel(0x00000009, base + PBIST_STR);
+
+	/* Start PBIST */
+	writel(0x00000001, base + PBIST_STR);
+
+	udelay(1000);
+
+	u32 timeout_count = 0;
+
+	while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
+	       (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
+		;
+
+	if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
+		test_result = false;
+		debug("%s(dev=%p): test failed\n", __func__, dev);
+	} else {
+		debug("%s(dev=%p): test passed\n", __func__, dev);
+	}
+
+	writel(0xffffffff, VIM_STS(intr_num));
+
+	return 0;
+}
+
+/**
+ * pbist_rom_self_test() - Run PBIST_ROM_TEST on specified cores
+ * @config: pbist_config_rom structure for PBIST negative test
+ *
+ * Function to run PBIST test of ROM
+ *
+ * Return: 0 if all went fine, else corresponding error.
+ */
+int pbist_rom_self_test(struct pbist_config_rom *config)
+{
+	struct udevice *dev = k3_bist_priv->dev;
+	void *base = k3_bist_priv->base;
+	u32 intr_num = k3_bist_priv->intr_num;
+	bool test_result = true;
+
+	/* Turns on PBIST clock in PBIST ACTivate register */
+	writel(0x1, base + PBIST_PACT);
+
+	/* Set Margin mode register for Test mode */
+	writel(0xf, base + PBIST_MARGIN_MODE);
+
+	/* Zero out Loop counter 0 */
+	writel(0x0, base + PBIST_L0);
+
+	/* Set DLR register */
+	writel(0x310, base + PBIST_DLR);
+
+	/* Set Registers*/
+	writel(0x00000001, base + PBIST_RF0L);
+	writel(0x00003123, base + PBIST_RF0U);
+	writel(0x7A400183, base + PBIST_RF1L);
+	writel(0x00000060, base + PBIST_RF1U);
+	writel(0x00000184, base + PBIST_RF2L);
+	writel(0x00000000, base + PBIST_RF2U);
+	writel(0x7B600181, base + PBIST_RF3L);
+	writel(0x00000061, base + PBIST_RF3U);
+	writel(0x00000000, base + PBIST_RF4L);
+	writel(0x00000000, base + PBIST_RF4U);
+
+	writel(config->D, base + PBIST_D);
+	writel(config->E, base + PBIST_E);
+	writel(config->CA2, base + PBIST_CA2);
+	writel(config->CL0, base + PBIST_CL0);
+	writel(config->CA3, base + PBIST_CA3);
+	writel(config->I0, base + PBIST_I0);
+	writel(config->CL1, base + PBIST_CL1);
+	writel(config->I3, base + PBIST_I3);
+	writel(config->I2, base + PBIST_I2);
+	writel(config->CL2, base + PBIST_CL2);
+	writel(config->CA1, base + PBIST_CA1);
+	writel(config->CA0, base + PBIST_CA0);
+	writel(config->CL3, base + PBIST_CL3);
+	writel(config->I1, base + PBIST_I1);
+	writel(config->RAMT, base + PBIST_RAMT);
+	writel(config->CSR, base + PBIST_CSR);
+	writel(config->CMS, base + PBIST_CMS);
+
+	writel(0x00000009, base + PBIST_STR);
+
+	/* Start PBIST */
+	writel(0x00000001, base + PBIST_STR);
+
+	udelay(1000);
+
+	u32 timeout_count = 0;
+
+	while ((!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) &&
+	       (timeout_count++ < PBIST_MAX_TIMEOUT_VALUE))
+		;
+
+	if (!(readl(VIM_RAW(intr_num)) & VIM_RAW_MASK(intr_num))) {
+		test_result = false;
+		debug("%s(dev=%p): test failed\n", __func__, dev);
+	} else {
+		debug("%s(dev=%p): test passed\n", __func__, dev);
+	}
+
+	writel(0xffffffff, VIM_STS(intr_num));
+
+	return 0;
+}
+
+/**
+ * k3_bist_probe() - Basic probe
+ * @dev: corresponding BIST device
+ *
+ * Parses BIST info from device tree, and configures the module accordingly.
+ * Return: 0 if all goes good, else appropriate error message.
+ */
+static int k3_bist_probe(struct udevice *dev)
+{
+	int ret = 0, num_runs, i, j;
+	struct k3_bist_privdata *priv = dev_get_priv(dev);
+	struct pbist_inst_info *info;
+
+	debug("%s(dev=%p)\n", __func__, dev);
+
+	priv = dev_get_priv(dev);
+	priv->dev = dev;
+
+	k3_bist_priv = priv;
+
+	priv->base = dev_remap_addr_index(dev, 0);
+	if (!priv->base)
+		return -ENODEV;
+
+	ret = dev_read_u32(dev, "ti,bist-instance", &priv->instance);
+	if (!priv->instance)
+		return -ENODEV;
+
+	switch (priv->instance) {
+	case PBIST14_INSTANCE:
+		info = &pbist14_inst_info;
+		priv->intr_num = info->intr_num;
+		break;
+	default:
+		dev_err(dev, "%s: PBIST instance %d not supported\n", __func__, priv->instance);
+		return -ENODEV;
+	};
+
+	/* Probe the cores under test */
+	for (i = 0; ; i++) {
+		ret = core_get_by_index(dev, i, false);
+		if (ret)
+			break;
+	}
+
+	if (!i) {
+		dev_err(dev, "%s: Acquiring the core failed. ret = %d\n", __func__, ret);
+		return ret;
+	}
+
+	/* Check whether HW POST successfully completely PBIST on the MCU domain */
+	ret = pbist_run_post_pbist_check();
+	if (ret) {
+		dev_err(dev, "HW POST failed to run successfully %d\n", ret);
+		return ret;
+	}
+
+	/* Run PBIST test */
+	num_runs = info->num_pbist_runs;
+
+	for (j = 0; j < num_runs; j++) {
+		ret = pbist_self_test(&info->pbist_config_run[j]);
+		if (ret) {
+			dev_err(dev, "failed to run PBIST test %d\n", ret);
+			return ret;
+		}
+	}
+
+	/* Run PBIST failure insertion test */
+	ret = pbist_neg_self_test(&info->pbist_neg_config_run);
+	if (ret) {
+		dev_err(dev, "failed to run PBIST negative test %d\n", ret);
+		return ret;
+	}
+
+	/* Run PBIST test on ROM */
+	num_runs = info->num_pbist_rom_test_runs;
+
+	for (j = 0; j < num_runs; j++) {
+		ret = pbist_rom_self_test(&info->pbist_rom_test_config_run[j]);
+		if (ret) {
+			dev_err(dev, "failed to run ROM PBIST test %d\n", ret);
+			return ret;
+		}
+	}
+
+	/* Power off cores under test */
+	while (i--) {
+		ret = core_get_by_index(dev, i, true);
+		if (ret)
+			break;
+	}
+
+	if (i) {
+		dev_err(dev, "%s: Stopping the core failed. ret = %d\n", __func__, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct udevice_id k3_bist_ids[] = {
+	{ .compatible = "ti,j784s4-bist" },
+	{}
+};
+
+U_BOOT_DRIVER(k3_bist) = {
+	.name = "k3_bist",
+	.of_match = k3_bist_ids,
+	.id = UCLASS_MISC,
+	.probe = k3_bist_probe,
+	.priv_auto = sizeof(struct k3_bist_privdata),
+};
diff --git a/drivers/misc/k3_bist_static_data.h b/drivers/misc/k3_bist_static_data.h
new file mode 100644
index 00000000000..f30fb7935b6
--- /dev/null
+++ b/drivers/misc/k3_bist_static_data.h
@@ -0,0 +1,551 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Static Data for Texas Instruments' BIST (Built-In Self-Test) driver
+ *
+ * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
+ *
+ */
+
+#ifndef __K3_BIST_STATIC_DATA_H
+#define __K3_BIST_STATIC_DATA_H
+
+#define PBIST_MAX_NUM_RUNS	2
+#define NUM_MAX_PBIST_TEST_ROM_RUNS 13
+#define PBIST14_DFT_PBIST_CPU_0_INTR_NUM 311
+
+/* VIM Registers */
+#define VIM_STS_BASE						  0x40f80404
+#define VIM_RAW_BASE						  0x40f80400
+
+#define VIM_STS(i)			(VIM_STS_BASE + (i) / 32 * 0x20)
+#define VIM_RAW(i)	      (VIM_RAW_BASE + (i) / 32 * 0x20)
+#define VIM_RAW_MASK(i)	 (BIT((i) % 32))
+
+/* PBIST Registers and Flags*/
+#define PBIST_RF0L						    0x00000000
+#define PBIST_RF1L						    0x00000004
+#define PBIST_RF2L						    0x00000008
+#define PBIST_RF3L						    0x0000000C
+#define PBIST_RF4L						    0x0000010
+#define PBIST_RF5L						    0x0000014
+#define PBIST_RF6L						    0x0000018
+#define PBIST_RF7L						    0x000001C
+#define PBIST_RF8L						    0x0000020
+#define PBIST_RF9L						    0x0000024
+#define PBIST_RF10L						   0x0000028
+#define PBIST_RF11L						   0x000002C
+#define PBIST_RF12L						   0x0000030
+#define PBIST_RF13L						   0x0000034
+#define PBIST_RF14L						   0x0000038
+#define PBIST_RF15L						   0x000003C
+#define PBIST_RF0U						    0x0000040
+#define PBIST_RF1U						    0x0000044
+#define PBIST_RF2U						    0x0000048
+#define PBIST_RF3U						    0x000004C
+#define PBIST_RF4U						    0x0000050
+#define PBIST_RF5U						    0x0000054
+#define PBIST_RF6U						    0x0000058
+#define PBIST_RF7U						    0x000005C
+#define PBIST_RF8U						    0x0000060
+#define PBIST_RF9U						    0x0000064
+#define PBIST_RF10U						   0x0000068
+#define PBIST_RF11U						   0x000006C
+#define PBIST_RF12U						   0x0000070
+#define PBIST_RF13U						   0x0000074
+#define PBIST_RF14U						   0x0000078
+#define PBIST_RF15U						   0x000007C
+#define PBIST_A0							    0x0000100
+#define PBIST_A1							    0x0000104
+#define PBIST_A2							    0x0000108
+#define PBIST_A3							    0x000010C
+#define PBIST_L0							    0x0000110
+#define PBIST_L1							    0x0000114
+#define PBIST_L2							    0x0000118
+#define PBIST_L3							    0x000011C
+#define PBIST_D							     0x0000120
+#define PBIST_E							     0x0000124
+#define PBIST_CA0							   0x0000130
+#define PBIST_CA1							   0x0000134
+#define PBIST_CA2							   0x0000138
+#define PBIST_CA3							   0x000013C
+#define PBIST_CL0							   0x0000140
+#define PBIST_CL1							   0x0000144
+#define PBIST_CL2							   0x0000148
+#define PBIST_CL3							   0x000014C
+#define PBIST_I0							    0x0000150
+#define PBIST_I1							    0x0000154
+#define PBIST_I2							    0x0000158
+#define PBIST_I3							    0x000015C
+#define PBIST_RAMT							  0x0000160
+#define PBIST_DLR							   0x0000164
+#define PBIST_CMS							   0x0000168
+#define PBIST_STR							   0x000016C
+#define PBIST_SCR							   0x0000170
+#define PBIST_SCR_LO								0x0000170
+#define PBIST_SCR_HI								0x0000174
+#define PBIST_CSR							   0x0000178
+#define PBIST_FDLY							  0x000017C
+#define PBIST_PACT							  0x0000180
+#define PBIST_PID							   0x0000184
+#define PBIST_OVER							  0x0000188
+#define PBIST_FSPBIST_RF						    0x0000190
+#define PBIST_FSRC							  0x0000198
+#define PBIST_FSRA							  0x00001A0
+#define PBIST_FSRDL0							0x00001A8
+#define PBIST_FSRDL1							0x00001B0
+#define PBIST_MARGIN_MODE						   0x00001B4
+#define PBIST_WRENZ							 0x00001B8
+#define PBIST_PAGE_PGS						      0x00001BC
+#define PBIST_ROM							   0x00001C0
+#define PBIST_ALGO							  0x00001C4
+#define PBIST_RINFO							 0x00001C8
+
+#define PBIST_MARGIN_MODE_PBIST_DFT_WRITE_MASK				0x00000003
+#define PBIST_MARGIN_MODE_PBIST_DFT_READ_SHIFT				0x00000002
+#define PBIST_MARGIN_MODE_PBIST_DFT_READ_MASK				0x0000000C
+#define PBIST_PACT_PACT_MASK						0x00000001
+#define PBIST_DLR_DLR0_ROM_MASK						0x00000004
+#define PBIST_DLR_DLR0_CAM_MASK						0x00000010
+#define PBIST_NOT_DONE							0
+#define PBIST_DONE							1
+
+/* PBIST test mode */
+#define PBIST_TEST_MODE (PBIST_MARGIN_MODE_PBIST_DFT_WRITE_MASK \
+			 | (1 << PBIST_MARGIN_MODE_PBIST_DFT_READ_SHIFT))
+
+/* PBIST Failure Insertion test mode */
+#define PBIST_FAILURE_INSERTION_TEST_MODE (PBIST_MARGIN_MODE_PBIST_DFT_WRITE_MASK \
+					   | PBIST_MARGIN_MODE_PBIST_DFT_READ_MASK)
+
+/*
+ * struct pbist_config - Structure for different configuration used for PBIST
+ * @override: Override value for memory configuration
+ * @algorithms_bit_map: Bitmap to select algorithms to use for test
+ * @memory_groups_bit_map: Bitmap to select memory groups to run test on
+ * @scramble_value_lo: Lower scramble value to be used for test
+ * @scramble_value_hi: Higher scramble value to be used for test
+ */
+struct pbist_config {
+	u32 override;
+	u32 algorithms_bit_map;
+	u64 memory_groups_bit_map;
+	u32 scramble_value_lo;
+	u32 scramble_value_hi;
+};
+
+/*
+ * struct pbist_config_neg - Structure for different configuration used for PBIST
+ * for the failure insertion test to generate negative result
+ * @CA0: Failure insertion value for CA0
+ * @CA1: Failure insertion value for CA1
+ * @CA2: Failure insertion value for CA2
+ * @CA3: Failure insertion value for CA3
+ * @CL0: Failure insertion value for CL0
+ * @CL1: Failure insertion value for CL1
+ * @CL2: Failure insertion value for CL2
+ * @CL3: Failure insertion value for CL3
+ * @CMS: Failure insertion value for CMS
+ * @CSR: Failure insertion value for CSR
+ * @I0: Failure insertion value for I0
+ * @I1: Failure insertion value for I1
+ * @I2: Failure insertion value for I2
+ * @I3: Failure insertion value for I3
+ * @RAMT: Failure insertion value for RAMT
+ */
+struct pbist_config_neg {
+	u32 CA0;
+	u32 CA1;
+	u32 CA2;
+	u32 CA3;
+	u32 CL0;
+	u32 CL1;
+	u32 CL2;
+	u32 CL3;
+	u32 CMS;
+	u32 CSR;
+	u32 I0;
+	u32 I1;
+	u32 I2;
+	u32 I3;
+	u32 RAMT;
+};
+
+/*
+ * struct pbist_config_neg - Structure for different configuration used for PBIST
+ * test of ROM
+ * @D: ROM test value for D
+ * @E: ROM test value for E
+ * @CA2: ROM test value for CA2
+ * @CL0: ROM test value for CL0
+ * @CA3: ROM test value for CA3
+ * @I0: ROM test value for I0
+ * @CL1: ROM test value for CL1
+ * @I3: ROM test value for I3
+ * @I2: ROM test value for I2
+ * @CL2: ROM test value for CL2
+ * @CA1: ROM test value for CA1
+ * @CA0: ROM test value for CA0
+ * @CL3: ROM test value for CL3
+ * @I1: ROM test value for I1
+ * @RAMT: ROM test value for RAMT
+ * @CSR: ROM test value for CSR
+ * @CMS: ROM test value for CMS
+ */
+struct pbist_config_rom {
+	u32 D;
+	u32 E;
+	u32 CA2;
+	u32 CL0;
+	u32 CA3;
+	u32 I0;
+	u32 CL1;
+	u32 I3;
+	u32 I2;
+	u32 CL2;
+	u32 CA1;
+	u32 CA0;
+	u32 CL3;
+	u32 I1;
+	u32 RAMT;
+	u32 CSR;
+	u32 CMS;
+};
+
+/*
+ * struct pbist_inst_info - Structure for different configuration used for PBIST
+ * @num_pbist_runs: Number of runs of PBIST test
+ * @intr_num: Interrupt number triggered by this PBIST instance to MCU R5 VIM
+ * @pbist_config_run: Configuration for PBIST test
+ * @pbist_neg_config_run: Configuration for PBIST negative test
+ * @num_pbist_rom_test_runs: Number of runs of PBIST test on ROM
+ * @pbist_rom_test_config_run: Configuration for PBIST test on ROM
+ */
+struct pbist_inst_info {
+	u32 num_pbist_runs;
+	u32 intr_num;
+	struct pbist_config pbist_config_run[PBIST_MAX_NUM_RUNS];
+	struct pbist_config_neg pbist_neg_config_run;
+	u32 num_pbist_rom_test_runs;
+	struct pbist_config_rom pbist_rom_test_config_run[NUM_MAX_PBIST_TEST_ROM_RUNS];
+};
+
+#if IS_ENABLED(CONFIG_SOC_K3_J784S4)
+
+/* WKUP CTRL MMR Registers */
+#define WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT				0x0000C2C0
+#define WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_DONE_SHIFT	0x00000008
+#define WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_TIMEOUT_SHIFT	0x00000009
+#define WKUP_CTRL_MMR_CFG0_WKUP_POST_STAT_POST_MCU_PBIST_FAIL_MASK	0x00008000
+
+/* Properties of PBIST instances in: PBIST14 */
+#define PBIST14_INSTANCE					      14
+#define PBIST14_NUM_TEST_VECTORS				      0x1
+#define PBIST14_ALGO_BITMAP_0					      0x00000003
+#define PBIST14_MEM_BITMAP_0					      0x000CCCCC
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CA0			      0x00000000
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CA1			      0x000001FF
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CA2			      0x000001FF
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CA3			      0x00000000
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CL0			      0x0000007F
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CL1			      0x00000003
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CL2			      0x00000008
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CL3			      0x000001FF
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CMS			      0x00000000
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_CSR			      0x20000000
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_I0			      0x00000001
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_I1			      0x00000004
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_I2			      0x00000008
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_I3			      0x00000000
+#define PBIST14_FAIL_INSERTION_TEST_VECTOR_RAMT			      0x011D2528
+
+static struct pbist_inst_info pbist14_inst_info = {
+	/* Main Pulsar 2 Instance 1 or MAIN_R52_x */
+	.num_pbist_runs = 1,
+	.intr_num = PBIST14_DFT_PBIST_CPU_0_INTR_NUM,
+	.pbist_config_run = {
+		{
+			.override = 0,
+			.algorithms_bit_map = PBIST14_ALGO_BITMAP_0,
+			.memory_groups_bit_map = PBIST14_MEM_BITMAP_0,
+			.scramble_value_lo = 0x76543210,
+			.scramble_value_hi = 0xFEDCBA98,
+		},
+		{
+			.override = 0,
+			.algorithms_bit_map = 0,
+			.memory_groups_bit_map = 0,
+			.scramble_value_lo = 0,
+			.scramble_value_hi = 0,
+		},
+	},
+	.pbist_neg_config_run = {
+		.CA0   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CA0,
+		.CA1   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CA1,
+		.CA2   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CA2,
+		.CA3   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CA3,
+		.CL0   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CL0,
+		.CL1   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CL1,
+		.CL2   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CL2,
+		.CL3   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CL3,
+		.CMS   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CMS,
+		.CSR   = PBIST14_FAIL_INSERTION_TEST_VECTOR_CSR,
+		.I0    = PBIST14_FAIL_INSERTION_TEST_VECTOR_I0,
+		.I1    = PBIST14_FAIL_INSERTION_TEST_VECTOR_I1,
+		.I2    = PBIST14_FAIL_INSERTION_TEST_VECTOR_I2,
+		.I3    = PBIST14_FAIL_INSERTION_TEST_VECTOR_I3,
+		.RAMT  = PBIST14_FAIL_INSERTION_TEST_VECTOR_RAMT
+	},
+	.num_pbist_rom_test_runs = 1,
+	.pbist_rom_test_config_run = {
+		{
+			.D = 0xF412605Eu,
+			.E = 0xF412605Eu,
+			.CA2 = 0x7FFFu,
+			.CL0 = 0x3FFu,
+			.CA3 = 0x0u,
+			.I0 = 0x1u,
+			.CL1 = 0x1Fu,
+			.I3 = 0x0u,
+			.I2 = 0xEu,
+			.CL2 = 0xEu,
+			.CA1 = 0x7FFFu,
+			.CA0 = 0x0u,
+			.CL3 = 0x7FFFu,
+			.I1 = 0x20u,
+			.RAMT = 0x08002020u,
+			.CSR = 0x00000001u,
+			.CMS = 0x01u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+		{
+			.D = 0x0u,
+			.E = 0x0u,
+			.CA2 = 0x0u,
+			.CL0 = 0x0u,
+			.CA3 = 0x0u,
+			.I0 = 0x0u,
+			.CL1 = 0x0u,
+			.I3 = 0x0u,
+			.I2 = 0x0u,
+			.CL2 = 0x0u,
+			.CA1 = 0x0u,
+			.CA0 = 0x0u,
+			.CL3 = 0x0u,
+			.I1 = 0x0u,
+			.RAMT = 0x0u,
+			.CSR = 0x0u,
+			.CMS = 0x0u
+		},
+	},
+};
+
+#endif /* CONFIG_SOC_K3_J784S4 */
+#endif /* __TI_SCI_STATIC_DATA_H */