Re: [PATCH 2/2] spi: Add Nuvoton MA35D1 QSPI controller support

[Mark Brown]

On Wed, Jun 03, 2026 at 12:35:51PM +0800, Chi-Wen Weng wrote:

> +config SPI_MA35D1_QSPI
> +	tristate "Nuvoton MA35D1 QSPI Controller"
> +	help
> +	  This driver provides support for Nuvoton MA35D1
> +	  QSPI controller in master mode.
> +

Other drivers for this SoC seem to have ARCH_MA35 || COMPILE_TEST?

> @@ -0,0 +1,579 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Nuvoton MA35D1 QSPI controller driver
> + *
> + * Copyright (c) 2026 Nuvoton Technology Corp.
> + * Author: Chi-Wen Weng <cwweng@xxxxxxxxxxx>
> + */

Please make the entire comment a C++ one so things look more
intentional.

> +static void nuvoton_qspi_set_cs(struct spi_device *spi, bool enable)
> +{
> +	struct nuvoton_qspi *qspi = spi_controller_get_devdata(spi->controller);
> +	unsigned int cs = spi_get_chipselect(spi, 0);
> +	u32 mask;
> +	u32 val;
> +
> +	if (cs == 0)
> +		mask = NUVOTON_QSPI_SSCTL_SS0_MASK;
> +	else
> +		mask = NUVOTON_QSPI_SSCTL_SS1_MASK;
> +
> +	val = nuvoton_qspi_read(qspi, NUVOTON_QSPI_SSCTL_OFFSET);
> +
> +	/* SPI core passes enable=true when CS is asserted (typically active-low) */
> +	if (enable)
> +		val |= mask;
> +	else
> +		val &= ~mask;
> +
> +	nuvoton_qspi_write(qspi, val, NUVOTON_QSPI_SSCTL_OFFSET);
> +}

Note that the core deals with SPI_CS_HIGH, the driver doesn't need to...

> +static int nuvoton_qspi_mem_exec_op(struct spi_mem *mem,
> +				    const struct spi_mem_op *op)
> +{
> +	struct spi_device *spi = mem->spi;
> +	struct nuvoton_qspi *qspi = spi_controller_get_devdata(spi->controller);
> +	u8 addr[4];
> +	int ret;
> +	int i;
> +
> +	ret = nuvoton_qspi_setup_transfer(spi, NULL);
> +	if (ret)
> +		return ret;

This uses spi->max_speed_hz but spi_mem configures p->max_freq which you
should use (it might be lower).

> +
> +	nuvoton_qspi_set_cs(spi, true);

...except where you're calling in directly at which point the driver
needs to figure this out.

Attachment: signature.asc
Description: PGP signature