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

On 7/31/26 6:33 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.
> 
> 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);
> +}
> +
> +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);

Wrap the if (IS_ENABLED()) into 
cadence_qspi_apb_write_protect_enable()/disable() and simply do

if (!IS_ENABLED()) return 0;

in there.

Also, maybe fold the two functions into a single one 
cadence_qspi_apb_write_protect(..., bool enable);