[PATCH v4 1/3] drivers: misc: k3_bist: Add K3 BIST driver

Manorit Chawdhry m-chawdhry at ti.com
Mon Jun 9 14:26:00 CEST 2025


Hi Neha,

On 15:57-20250609, Neha Malcom Francis wrote:
> Add a driver for the BIST module that support triggering of both PBIST
> (Memory BIST) and LBIST (Logic BIST) tests. Also expose the relevant
> operations and functions that would be required for an end user to
> trigger the tests.
> 
> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
> ---
> Changes since v2 and v3:
> - code cleanup and minor optimizations
> - LBIST MISR calculation modified to pick up expected value for comparison in
>   POST LBIST check instead of hard coding the value
> - moved from using ti,sci-dev-id instead of ti,bist-under-test
> 
>  drivers/misc/Kconfig                      |   8 +
>  drivers/misc/Makefile                     |   1 +
>  drivers/misc/k3_bist.c                    | 807 ++++++++++++++++++++++
>  drivers/misc/k3_bist_static_data.h        | 673 ++++++++++++++++++
>  drivers/misc/k3_j784s4_bist_static_data.h | 370 ++++++++++
>  include/k3_bist.h                         |  44 ++
>  6 files changed, 1903 insertions(+)
>  create mode 100644 drivers/misc/k3_bist.c
>  create mode 100644 drivers/misc/k3_bist_static_data.h
>  create mode 100644 drivers/misc/k3_j784s4_bist_static_data.h
>  create mode 100644 include/k3_bist.h
> 
> diff --git a/drivers/misc/k3_bist.c b/drivers/misc/k3_bist.c
> new file mode 100644
> index 00000000000..3acb1a1ac1f
> --- /dev/null
> +++ b/drivers/misc/k3_bist.c
[..]
> +/**
> + * 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.
> + */
> +static int pbist_neg_self_test(struct pbist_config_neg *config)
> +{
[..]
> +	/* Set Registers*/
> +	writel(0x00000001, base + PBIST_RF0L);
> +	writel(0x00003123, base + PBIST_RF0U);

Do these have some meaning or are they supposed to be hardcoded?
Wondering if each bit corresponds to something then maybe we should make
some FLAGS like PBIST_RF0U_XYZ | PBIST_RF0L_ABC to make it more readable
but wondering if that'd be too much as well given soo many registers..
Don't have much idea about the IP but looking at this seems like way too
much magic to me..

> +	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);
> +
[..]
> +/**
> + * 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.
> + */
> +static int pbist_rom_self_test(struct pbist_config_rom *config)
> +{
[..]
> +
> +	/* Set Registers*/
> +	writel(0x00000001, base + PBIST_RF0L);
> +	writel(0x00003123, base + PBIST_RF0U);

Same..

> +	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);
> +
[..]
> +int prepare_pbist(struct ti_sci_handle *handle)
> +{
> +	struct ti_sci_proc_ops *proc_ops = &handle->ops.proc_ops;
> +	struct ti_sci_dev_ops *dev_ops = &handle->ops.dev_ops;
> +	struct pbist_inst_info *info_pbist = k3_bist_priv->pbist_info;
> +	struct core_under_test *cut = info_pbist->cut;
> +
> +	if (proc_ops->proc_request(handle, cut[0].proc_id)) {
> +		printf("%s: requesting primary core failed\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (proc_ops->proc_request(handle, cut[1].proc_id)) {

Do we get any return code from these? If yes, then ig better to do 
ret = proc_ops->proc_request(handle, cut[1].proc_id))
and maybe print that value in printf as well which might help in debugging

> +		printf("%s: requesting secondary core failed\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (dev_ops->set_device_resets(handle, cut[0].dev_id, 0x1)) {
> +		printf("%s: local reset primary core failed\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (dev_ops->set_device_resets(handle, cut[1].dev_id, 0x1)) {
> +		printf("%s: local reset secondary core failed\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (dev_ops->get_device(handle, cut[0].dev_id)) {
> +		printf("%s: power on primary core failed\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (dev_ops->get_device(handle, cut[1].dev_id)) {
> +		printf("%s: power on secondary core failed\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (dev_ops->get_device(handle, info_pbist->dev_id)) {
> +		printf("%s: power on PBIST failed\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
[..]
> diff --git a/drivers/misc/k3_j784s4_bist_static_data.h b/drivers/misc/k3_j784s4_bist_static_data.h
> new file mode 100644
> index 00000000000..7f9378e917f
> --- /dev/null
> +++ b/drivers/misc/k3_j784s4_bist_static_data.h
> @@ -0,0 +1,370 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Static Data for Texas Instruments' BIST logic for J784S4
> + *
> + * Copyright (C) 2025 Texas Instruments Incorporated - https://www.ti.com/
> + *
> + */
> +
> +/* Device IDs of IPs that can be tested under BIST */
> +#define TISCI_DEV_MCU_R5FSS2_CORE0		343
> +#define TISCI_DEV_MCU_R5FSS2_CORE1		344
> +#define TISCI_DEV_RTI32					365
> +#define TISCI_DEV_RTI33					366

Unused? 

[..]

> diff --git a/include/k3_bist.h b/include/k3_bist.h
> new file mode 100644
> index 00000000000..cc650f5a8c4
> --- /dev/null
> +++ b/include/k3_bist.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Texas Instruments' BIST (Built-In Self-Test) driver
> + *
> + * Copyright (C) 2025 Texas Instruments Incorporated - https://www.ti.com/
> + *      Neha Malcom Francis <n-francis at ti.com>
> + *
> + */
> +
> +#ifndef _INCLUDE_BIST_H_
> +#define _INCLUDE_BIST_H_
> +
> +#define PROC_BOOT_CTRL_FLAG_R5_CORE_HALT	0x00000001

Unused? 

> +#define PROC_ID_MCU_R5FSS2_CORE0		0x0A
> +#define PROC_ID_MCU_R5FSS2_CORE1		0x0B

> +#define PROC_BOOT_CTRL_FLAG_R5_LPSC		0x00000002

Unused? 

> +
> +#define TISCI_DEV_PBIST14 237
> +#define TISCI_DEV_R5FSS2_CORE0 343
> +#define TISCI_DEV_R5FSS2_CORE1 344

> +
> +#define TISCI_MSG_VALUE_DEVICE_SW_STATE_AUTO_OFF 0
> +#define TISCI_MSG_VALUE_DEVICE_SW_STATE_RETENTION 1
> +#define TISCI_MSG_VALUE_DEVICE_SW_STATE_ON 2
> +
> +#define TISCI_BIT(n)  ((1) << (n))

Unused? Also wondering that maybe these shouldn't even be here.. they
are tisci related..

> +
> +struct bist_ops {
> +	int (*run_lbist)(void);
> +	int (*run_lbist_post)(void);
> +	int (*run_pbist_post)(void);
> +	int (*run_pbist_neg)(void);
> +	int (*run_pbist_rom)(void);
> +	int (*run_pbist)(void);
> +};
> +
> +void lbist_enable_isolation(void);
> +void lbist_disable_isolation(void);
> +int prepare_pbist(struct ti_sci_handle *handle);
> +int deprepare_pbist(struct ti_sci_handle *handle);
> +int prepare_lbist(struct ti_sci_handle *handle);
> +int deprepare_lbist(struct ti_sci_handle *handle);
> +
> +#endif /* _INCLUDE_BIST_H_ */
> -- 
> 2.34.1
> 

Regards,
Manorit


More information about the U-Boot mailing list