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

[Chi-Wen Weng]

Hi Mark,

Thanks for the review.

I will address these in v2:
- add "depends on ARCH_MA35 || COMPILE_TEST" to the Kconfig entry;
- convert the file header to // comments;
- split the low-level CS register update from the SPI core .set_cs()
   callback, and make the spi-mem direct CS path handle SPI_CS_HIGH
   explicitly;
- use op->max_freq for spi-mem operations instead of spi->max_speed_hz.

I will also fix the subject lines in the next version.

Best regards,
Chi-Wen Weng

Mark Brown 於 2026/6/3 下午 05:30 寫道:
> 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 at nuvoton.com>
>> + */
> 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.