Re: [PATCH 03/17] spi: cadence_qspi: Use APB write protection

"Kumar, Udit" <[email protected]> Sat, 1 Aug 2026 12:53:38 +0530
Newsgroups gmane.comp.boot-loaders.u-boot
Message-ID <[email protected]>

On 7/31/2026 10:03 PM, Ralph Siemsen wrote:
> Prevent random memory writes from corrupting flash memory.
> 
> When using the Direct Access Controller (DAC), the QSPI flash appears as
> a memory-mapped device. A stray write within the DAC region can corrupt
> flash contents.
>

my suggestion will be check first, why stray write are happening

> To prevent this, enable the hardware write protection feature. Due to
> known errata [1] of the QSPI controller, the entire address range is
> protected, and only gets unlocked during intentional QSPI writes.
> 



> [1] https://www.renesas.com/en/document/tcu/qspi-contoller-issue
> 
> Signed-off-by: Ralph Siemsen <[email protected]>
> ---
>  drivers/spi/Kconfig            | 11 +++++++++++
>  drivers/spi/cadence_qspi.h     |  6 ++++++
>  drivers/spi/cadence_qspi_apb.c | 27 +++++++++++++++++++++++++++
>  3 files changed, 44 insertions(+)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 007ad5e7733..19f8d48507b 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -168,6 +168,17 @@ config CADENCE_QSPI
>  	  used to access the SPI NOR flash on platforms embedding this
>  	  Cadence IP core.
>  
> +config CADENCE_QSPI_WRITE_PROTECT
> +	bool "Cadence QSPI write protection"
> +	depends on CADENCE_QSPI
> +	help
> +	  Enable hardware write protection in the QSPI controller. Helps avoid
> +	  flash corruption due to stray writes. The direct access region is kept
> +	  locked by default, and only gets unlocked during "sf write".
> +
> +	  This is similar to SYS_FLASH_PROTECTION however blocking is done by
> +	  the controller, rather than by the flash device.
> +
>  config HAS_CQSPI_REF_CLK
>  	bool "Cadence QSPI static reference clock"
>  	depends on CADENCE_QSPI
> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
> index 1e9081c2d17..6b3ed46977a 100644
> --- a/drivers/spi/cadence_qspi.h
> +++ b/drivers/spi/cadence_qspi.h
> @@ -119,6 +119,12 @@
>  #define CQSPI_REG_IRQSTATUS                     0x40
>  #define CQSPI_REG_IRQMASK                       0x44
>  
> +#define CQSPI_REG_LOWER_WRITE_PROTECT           0x50
> +#define CQSPI_REG_UPPER_WRITE_PROTECT           0x54
> +
> +#define CQSPI_REG_WRITE_PROTECT_CTRL            0x58
> +#define CQSPI_REG_WRPROT_ENABLE                 BIT(1)
> +
>  #define CQSPI_REG_INDIRECTRD                    0x60
>  #define CQSPI_REG_INDIRECTRD_START              BIT(0)
>  #define CQSPI_REG_INDIRECTRD_CANCEL             BIT(1)
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index 0d4bc685f5d..353b57a46af 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -325,6 +325,17 @@ void cadence_qspi_apb_delay(void *reg_base,
>  	cadence_qspi_apb_controller_enable(reg_base);
>  }
>  
> +static void cadence_qspi_apb_write_protect_enable(void *reg_base)
> +{
> +	writel(CQSPI_REG_WRPROT_ENABLE,
> +	       reg_base + CQSPI_REG_WRITE_PROTECT_CTRL);
> +}
> +

I expect compilation warning on platform, which does not define
CONFIG_CADENCE_QSPI_WRITE_PROTECT

> +static void cadence_qspi_apb_write_protect_disable(void *reg_base)
> +{
> +	writel(0, reg_base + CQSPI_REG_WRITE_PROTECT_CTRL);
> +}
> +
>  void cadence_qspi_apb_controller_init(struct cadence_spi_priv *priv)
>  {
>  	unsigned reg;
> @@ -346,6 +357,18 @@ void cadence_qspi_apb_controller_init(struct cadence_spi_priv *priv)
>  	/* Indirect mode configurations */
>  	writel(priv->fifo_depth / 2, priv->regbase + CQSPI_REG_SRAMPARTITION);
>  
> +	if (IS_ENABLED(CONFIG_CADENCE_QSPI_WRITE_PROTECT)) {
> +		/*
> +		 * Enable AHB write protection, to reduce the chance of corrupting
> +		 * flash memory due to a stray write within the DAC region.
> +		 * Protect the entire address range, regardless of the flash size.
> +		 * During "sf write" the protection will be temporarily disabled.
> +		 */
> +		writel(0, priv->regbase + CQSPI_REG_LOWER_WRITE_PROTECT);
> +		writel(~0, priv->regbase + CQSPI_REG_UPPER_WRITE_PROTECT);
> +		cadence_qspi_apb_write_protect_enable(priv->regbase);
> +	}
> +
>  	/* Disable all interrupts */
>  	writel(0, priv->regbase + CQSPI_REG_IRQMASK);
>  
> @@ -955,7 +978,11 @@ int cadence_qspi_apb_write_execute(struct cadence_spi_priv *priv,
>  	 */
>  	cadence_qspi_apb_enable_linear_mode(true);
>  	if (!priv->dtr && priv->use_dac_mode && (to + len < priv->ahbsize)) {
> +		if (IS_ENABLED(CONFIG_CADENCE_QSPI_WRITE_PROTECT))
> +			cadence_qspi_apb_write_protect_disable(priv->regbase);
>  		memcpy_toio(priv->ahbbase + to, buf, len);
> +		if (IS_ENABLED(CONFIG_CADENCE_QSPI_WRITE_PROTECT))
> +			cadence_qspi_apb_write_protect_enable(priv->regbase);
>  		if (!cadence_qspi_wait_idle(priv->regbase))
>  			return -EIO;
>  		return 0;
>