spi: ma35d1-qspi: Add Nuvoton MA35D1 QSPI controller
Subsystems (3)
v3 cover spi: ma35d1-qspi: Add Nuvoton MA35D1 QSPI controller
From: Chi-Wen Weng <[email protected]>
Add devicetree binding and SPI controller driver support for the Nuvoton MA35D1 Quad SPI controller.
The MA35D1 QSPI controller supports SPI memory devices such as SPI NOR and SPI NAND flashes in single, dual and quad I/O modes. This initial driver implements a conservative PIO-based transfer path and spi-mem operation support.
Changes in v3:
- Fixed DCO mismatch by using a consistent author and Signed-off-by address.
- Added spi_controller_mem_caps and set per_op_freq for spi-mem operations.
- Added SPI_CS_HIGH to controller mode_bits.
- Added a short delay after requesting FIFO reset before polling reset completion.
- Documented that the MA35D1 QSPI controller pushes one RX FIFO entry for each TX word in single, dual-output and quad-output modes, so TX-only transfers intentionally drain and discard RX data.
- Added missing linux/delay.h include for udelay().
- Cleaned up unused register bit definitions.
Changes in v2:
- Updated patch subject lines to match SPI subsystem style.
- Added commit message to the dt-bindings patch.
- Added ARCH_MA35 || COMPILE_TEST dependency to Kconfig.
- Expanded Kconfig help text.
- Converted the driver file header to // comments.
- Added reset control handling to the driver.
- Added resets property to the binding.
- Added num-cs constraint to the binding.
- Dropped the flash child node from the binding example.
- Used op->max_freq for spi-mem operations.
- Split low-level CS register handling from the SPI core .set_cs() callback.
- Handled SPI_CS_HIGH explicitly for the spi-mem direct CS path.
- Fixed spi-mem opcode transfer to use a u8 buffer.
- Limited spi-mem command opcode length to one byte.
- Forced spi-mem operations to 8-bit word size.
- Avoided driving bidirectional data pins during dummy cycles.
- Drained RX FIFO during TX-only transfers.
- Rejected invalid chip-select numbers instead of mapping them to SS1.
- Rejected unsupported dual/quad full-duplex generic SPI transfers.
- Fixed checkpatch style issues.
Chi-Wen Weng (2):
dt-bindings: spi: nuvoton,ma35d1-qspi: Add Nuvoton MA35D1 QSPI
spi: ma35d1-qspi: Add Nuvoton MA35D1 QSPI controller support
.../bindings/spi/nuvoton,ma35d1-qspi.yaml | 62 ++ drivers/spi/Kconfig | 10 + drivers/spi/Makefile | 1 + drivers/spi/spi-ma35d1-qspi.c | 633 ++++++++++++++++++ 4 files changed, 706 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml create mode 100644 drivers/spi/spi-ma35d1-qspi.c
Signature · 1 line
2.25.1
PATCH v3 1/2 dt-bindings: spi: nuvoton,ma35d1-qspi: Add Nuvoton MA35D1 QSPI
From: Chi-Wen Weng <[email protected]>
Add a devicetree binding for the Quad SPI controller found in Nuvoton MA35D1 SoCs.
The controller supports SPI memory devices such as SPI NOR and SPI NAND flashes. It has one register range, one clock input and one reset line, and supports up to two chip selects.
--- .../bindings/spi/nuvoton,ma35d1-qspi.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +62 -0
@@ -0,0 +1,62 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/nuvoton,ma35d1-qspi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton MA35D1 Quad SPI Controller + +maintainers: + - Chi-Wen Weng <[email protected]> + +allOf: + - $ref: /schemas/spi/spi-controller.yaml# + +properties: + compatible: + const: nuvoton,ma35d1-qspi + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + num-cs: + maximum: 2 + +required: + - compatible + - reg + - clocks + - resets + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + spi@40680000 { + compatible = "nuvoton,ma35d1-qspi"; + reg = <0 0x40680000 0 0x100>; + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk QSPI0_GATE>; + resets = <&sys MA35D1_RESET_QSPI0>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; +
Signature · 1 line
2.25.1
Re: [PATCH v3 1/2] dt-bindings: spi: nuvoton,ma35d1-qspi: Add Nuvoton MA35D1 QSPI
Quoted 43 lines · Chi-Wen Weng
From: Chi-Wen Weng <[email protected]>
Add a devicetree binding for the Quad SPI controller found in Nuvoton MA35D1 SoCs.
The controller supports SPI memory devices such as SPI NOR and SPI NAND flashes. It has one register range, one clock input and one reset line, and supports up to two chip selects.
Signed-off-by Chi-Wen Weng <[email protected]>--- .../bindings/spi/nuvoton,ma35d1-qspi.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yamlDocumentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +22
-0@@ -0,0 +1,62 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/nuvoton,ma35d1-qspi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton MA35D1 Quad SPI Controller + +maintainers: + - Chi-Wen Weng <[email protected]> + +allOf: + - $ref: /schemas/spi/spi-controller.yaml# + +properties: + compatible: + const: nuvoton,ma35d1-qspi + + reg: + maxItems: 1 + + interrupts:
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml line 32
Quoted 10 lines
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +10
-0+ maxItems: 1 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + num-cs: + maximum: 2
Missing a default of 2, unless you make the property required. FWIW, your driver doesn't appear to read this value.
pw-bot: changes-requested
Cheers,
Conor.
Quoted 33 lines
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +30
-0+ +required: + - compatible + - reg + - clocks + - resets + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + spi@40680000 { + compatible = "nuvoton,ma35d1-qspi"; + reg = <0 0x40680000 0 0x100>; + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk QSPI0_GATE>; + resets = <&sys MA35D1_RESET_QSPI0>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; +Signature · 1 line
2.25.1
Re: [PATCH v3 1/2] dt-bindings: spi: nuvoton,ma35d1-qspi: Add Nuvoton MA35D1 QSPI
Hi Conor,
Thanks for the review.
I will add a default value for num-cs in v4:
num-cs:
maximum: 2
default: 2
The controller has two native chip selects and the driver currently uses that hardware default.
Best regards,
Chi-Wen
Conor Dooley 於 2026/6/12 上午 01:34 寫道:
Quoted 95 lines · Chi-Wen Weng
Quoted 43 lines
From: Chi-Wen Weng <[email protected]>
Add a devicetree binding for the Quad SPI controller found in Nuvoton MA35D1 SoCs.
The controller supports SPI memory devices such as SPI NOR and SPI NAND flashes. It has one register range, one clock input and one reset line, and supports up to two chip selects.
Signed-off-by Chi-Wen Weng <[email protected]>--- .../bindings/spi/nuvoton,ma35d1-qspi.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+)create mode 100644
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yamlDocumentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +22
-0@@ -0,0 +1,62 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/nuvoton,ma35d1-qspi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton MA35D1 Quad SPI Controller + +maintainers: + - Chi-Wen Weng <[email protected]> + +allOf: + - $ref: /schemas/spi/spi-controller.yaml# + +properties: + compatible: + const: nuvoton,ma35d1-qspi + + reg: + maxItems: 1 + + interrupts:Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml line 32
Quoted 10 lines
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +10
-0+ maxItems: 1 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + num-cs: + maximum: 2Missing a default of 2, unless you make the property required. FWIW, your driver doesn't appear to read this value.
pw-bot: changes-requested
Cheers,
Conor.Quoted 33 lines
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +30
-0+ +required: + - compatible + - reg + - clocks + - resets + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + spi@40680000 { + compatible = "nuvoton,ma35d1-qspi"; + reg = <0 0x40680000 0 0x100>; + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk QSPI0_GATE>; + resets = <&sys MA35D1_RESET_QSPI0>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; +Signature · 1 line
2.25.1
Re: [PATCH v3 1/2] dt-bindings: spi: nuvoton,ma35d1-qspi: Add Nuvoton MA35D1 QSPI
Quoted 4 lines · Chi-Wen Weng
Hi Conor,
Thanks for the review.
Quoted 8 lines
I will add a default value for num-cs in v4:
num-cs:
maximum: 2
default: 2The controller has two native chip selects and the driver currently uses that hardware default.
The driver should handle the property and fall back to the default. It's not complex to support, so surely there's no reason not to?
Cheers,
Conor.
Quoted 100 lines
Best regards,
Chi-WenConor Dooley 於 2026/6/12 上午 01:34 寫道:
Quoted 95 lines · Chi-Wen Weng
Quoted 43 lines
From: Chi-Wen Weng <[email protected]>
Add a devicetree binding for the Quad SPI controller found in Nuvoton MA35D1 SoCs.
The controller supports SPI memory devices such as SPI NOR and SPI NAND flashes. It has one register range, one clock input and one reset line, and supports up to two chip selects.
Signed-off-by Chi-Wen Weng <[email protected]>--- .../bindings/spi/nuvoton,ma35d1-qspi.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+)create mode 100644
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yamlDocumentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +22
-0@@ -0,0 +1,62 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/nuvoton,ma35d1-qspi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton MA35D1 Quad SPI Controller + +maintainers: + - Chi-Wen Weng <[email protected]> + +allOf: + - $ref: /schemas/spi/spi-controller.yaml# + +properties: + compatible: + const: nuvoton,ma35d1-qspi + + reg: + maxItems: 1 + + interrupts:Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml line 32
Quoted 10 lines
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +10
-0+ maxItems: 1 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + num-cs: + maximum: 2Missing a default of 2, unless you make the property required. FWIW, your driver doesn't appear to read this value.
pw-bot: changes-requested
Cheers,
Conor.Quoted 33 lines
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +30
-0+ +required: + - compatible + - reg + - clocks + - resets + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + spi@40680000 { + compatible = "nuvoton,ma35d1-qspi"; + reg = <0 0x40680000 0 0x100>; + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk QSPI0_GATE>; + resets = <&sys MA35D1_RESET_QSPI0>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; +Signature · 1 line
2.25.1
Re: [PATCH v3 1/2] dt-bindings: spi: nuvoton,ma35d1-qspi: Add Nuvoton MA35D1 QSPI
Hi Conor,
Thanks for the clarification.
I will make the driver read num-cs in v4 and fall back to the hardware default of 2 when the property is not present. I will also keep the binding default in sync with that behavior.
Best regards,
Chi-Wen
Conor Dooley 於 2026/6/12 下午 11:48 寫道:
Quoted 118 lines · Chi-Wen Weng
Quoted 4 lines
Hi Conor,
Thanks for the review.
Quoted 8 lines
I will add a default value for num-cs in v4:
num-cs:
maximum: 2
default: 2The controller has two native chip selects and the driver currently uses that hardware default.
The driver should handle the property and fall back to the default. It's not complex to support, so surely there's no reason not to?
Cheers,
Conor.Quoted 99 lines
Best regards,
Chi-WenConor Dooley 於 2026/6/12 上午 01:34 寫道:
Quoted 95 lines · Chi-Wen Weng
Quoted 43 lines
From: Chi-Wen Weng <[email protected]>
Add a devicetree binding for the Quad SPI controller found in Nuvoton MA35D1 SoCs.
The controller supports SPI memory devices such as SPI NOR and SPI NAND flashes. It has one register range, one clock input and one reset line, and supports up to two chip selects.
Signed-off-by Chi-Wen Weng <[email protected]>--- .../bindings/spi/nuvoton,ma35d1-qspi.yaml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+)create mode 100644
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yamlDocumentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +22
-0@@ -0,0 +1,62 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/nuvoton,ma35d1-qspi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton MA35D1 Quad SPI Controller + +maintainers: + - Chi-Wen Weng <[email protected]> + +allOf: + - $ref: /schemas/spi/spi-controller.yaml# + +properties: + compatible: + const: nuvoton,ma35d1-qspi + + reg: + maxItems: 1 + + interrupts:Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml line 32
Quoted 10 lines
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +10
-0+ maxItems: 1 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + num-cs: + maximum: 2Missing a default of 2, unless you make the property required. FWIW, your driver doesn't appear to read this value.
pw-bot: changes-requested
Cheers,
Conor.Quoted 33 lines
Documentation/devicetree/bindings/spi/nuvoton,ma35d1-qspi.yaml new file +30
-0+ +required: + - compatible + - reg + - clocks + - resets + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h> + #include <dt-bindings/reset/nuvoton,ma35d1-reset.h> + + soc { + #address-cells = <2>; + #size-cells = <2>; + + spi@40680000 { + compatible = "nuvoton,ma35d1-qspi"; + reg = <0 0x40680000 0 0x100>; + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk QSPI0_GATE>; + resets = <&sys MA35D1_RESET_QSPI0>; + #address-cells = <1>; + #size-cells = <0>; + }; + }; +Signature · 1 line
2.25.1
PATCH v3 2/2 spi: ma35d1-qspi: Add Nuvoton MA35D1 QSPI controller support
In reply to Chi-Wen WengFrom: Chi-Wen Weng <[email protected]>
Add SPI controller driver support for the Nuvoton MA35D1 Quad SPI controller.
The controller supports standard SPI transfers and spi-mem operations for SPI memory devices such as SPI NOR and SPI NAND flashes. The driver supports single, dual and quad I/O modes and uses a conservative word-by-word PIO transfer path for the initial upstream version.
The driver also handles controller reset, clock divider programming, chip-select control, transfer mode setup and spi-mem command/address/ dummy/data phases.
--- drivers/spi/Kconfig | 10 + drivers/spi/Makefile | 1 + drivers/spi/spi-ma35d1-qspi.c | 633 ++++++++++++++++++++++++++++++++++ 3 files changed, 644 insertions(+) create mode 100644 drivers/spi/spi-ma35d1-qspi.c
drivers/spi/Kconfig +10 -0
@@ -694,6 +694,16 @@ config SPI_LJCA This driver can also be built as a module. If so, the module will be called spi-ljca. +config SPI_MA35D1_QSPI + tristate "Nuvoton MA35D1 QSPI controller" + depends on ARCH_MA35 || COMPILE_TEST + help + This enables support for the Quad SPI controller found in + Nuvoton MA35D1 SoCs. + + The controller supports SPI memory devices such as SPI NOR and + SPI NAND flashes in single, dual and quad I/O modes. + config SPI_MESON_SPICC tristate "Amlogic Meson SPICC controller" depends on COMMON_CLK
drivers/spi/Makefile +1 -0
@@ -85,6 +85,7 @@ obj-$(CONFIG_SPI_LOONGSON_CORE) += spi-loongson-core.o obj-$(CONFIG_SPI_LOONGSON_PCI) += spi-loongson-pci.o obj-$(CONFIG_SPI_LOONGSON_PLATFORM) += spi-loongson-plat.o obj-$(CONFIG_SPI_LP8841_RTC) += spi-lp8841-rtc.o +obj-$(CONFIG_SPI_MA35D1_QSPI) += spi-ma35d1-qspi.o obj-$(CONFIG_SPI_MESON_SPICC) += spi-meson-spicc.o obj-$(CONFIG_SPI_MESON_SPIFC) += spi-meson-spifc.o obj-$(CONFIG_SPI_MICROCHIP_CORE_QSPI) += spi-microchip-core-qspi.o
drivers/spi/spi-ma35d1-qspi.c new file +633 -0
633 lines not on this page · raw message
Signature · 1 line
2.25.1
[email protected]
AI reviewRe: [PATCH v3 2/2] spi: ma35d1-qspi: Add Nuvoton MA35D1 QSPI controller support
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Inconsistent chip-select polarity logic between SPI core and spi-mem operations leads to inverted…
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Inconsistent chip-select polarity logic between SPI core and spi-mem operations leads to inverted CS levels.
- [High] Data race on the Slave Select Control (SSCTL) register due to concurrent spi_setup() and transfers.
- [High] The PIO transfer loop uses atomic polling for bulk data transfers without yielding the CPU, risking soft lockups on large memory operations. --
commit 870f4cc6d9d6ab3e7f64bb28977c07e0fcfd4027
Author: Chi-Wen Weng <[email protected]>
spi: ma35d1-qspi: Add Nuvoton MA35D1 QSPI controller support
Add SPI controller driver support for the Nuvoton MA35D1 Quad SPI controller. The controller supports standard SPI transfers and spi-mem operations in single, dual, and quad modes.
drivers/spi/spi-ma35d1-qspi.c new file +0 -0
[ ... ]
Quoted 33 lines
(quoted diff) +33
-0+static int nuvoton_qspi_txrx(struct nuvoton_qspi *qspi, const void *txbuf, + void *rxbuf, unsigned int len) +{ + unsigned int bytes_per_word = DIV_ROUND_UP(qspi->bits_per_word, 8); + unsigned int words; + u32 val; + int ret; + int i; + + if (!len) + return 0; + + if (len % bytes_per_word) + return -EINVAL; + + words = len / bytes_per_word; + + ret = nuvoton_qspi_reset_fifo(qspi); + if (ret) { + dev_err(qspi->dev, "FIFO reset timed out\n"); + return ret; + } + + /* + * Use conservative word-by-word PIO access. This keeps the initial driver + * simple and avoids relying on FIFO threshold interrupts or DMA support. + * + * The MA35D1 QSPI controller pushes one RX FIFO entry for each TX word in + * single, dual-output and quad-output modes. Drain RX after every TX word + * and discard the value for TX-only transfers to avoid RX FIFO overflow. + */ + for (i = 0; i < words; i++) { + ret = nuvoton_qspi_wait_tx_not_full(qspi);
Review
Quoted 10 lines
(quoted diff) +10
-0+ if (ret) { + dev_err(qspi->dev, "TX FIFO full timeout\n"); + return ret; + } + + nuvoton_qspi_write(qspi, nuvoton_qspi_tx_word(txbuf, i, + qspi->bits_per_word), + NUVOTON_QSPI_TX_OFFSET); + + ret = nuvoton_qspi_wait_rx_not_empty(qspi);
[Severity: High]
Will this loop trigger kernel soft lockups on large memory operations?
The wait helpers nuvoton_qspi_wait_tx_not_full() and nuvoton_qspi_wait_rx_not_empty() use readl_poll_timeout_atomic(), which busy-waits without yielding the CPU.
Since the driver does not set max_transfer_size limits, the SPI framework can pass down arbitrarily large buffers (e.g., several megabytes for firmware flashes or filesystems).
Does this loop need a cond_resched(), or should the controller set appropriate transfer size limits to prevent CPU stalls?
Quoted 9 lines
(quoted diff) +9
-0+ if (ret) { + dev_err(qspi->dev, "RX FIFO empty timeout\n"); + return ret; + } + + val = nuvoton_qspi_read(qspi, NUVOTON_QSPI_RX_OFFSET); + if (rxbuf) + nuvoton_qspi_rx_word(rxbuf, i, val, qspi->bits_per_word); + }
[ ... ]
Quoted 16 lines
(quoted diff) +16
-0+static void nuvoton_qspi_set_cs_level(struct nuvoton_qspi *qspi, + unsigned int cs, bool assert) +{ + u32 mask; + u32 val; + + switch (cs) { + case 0: + mask = NUVOTON_QSPI_SSCTL_SS0_MASK; + break; + case 1: + mask = NUVOTON_QSPI_SSCTL_SS1_MASK; + break; + default: + dev_warn(qspi->dev, "invalid chip select %u\n", cs); + return;
Review
Quoted 10 lines
(quoted diff) +10
-0+ } + + val = nuvoton_qspi_read(qspi, NUVOTON_QSPI_SSCTL_OFFSET); + + if (assert) + val |= mask; + else + val &= ~mask; + + nuvoton_qspi_write(qspi, val, NUVOTON_QSPI_SSCTL_OFFSET);
[Severity: High]
Can this unprotected read-modify-write corrupt the active chip-select state?
The SPI core invokes spi_set_cs() directly during spi_setup() without holding controller-level locks.
If a transfer is active for CS0, and a concurrent setup call occurs for CS1 (such as an ioctl(SPI_IOC_WR_MODE) from spidev), could this read-modify-write on the shared SSCTL register race and cause silent data corruption or aborted transfers?
Quoted 8 lines
(quoted diff) +8
-0+} + +static void nuvoton_qspi_set_cs(struct spi_device *spi, bool enable) +{ + struct nuvoton_qspi *qspi = spi_controller_get_devdata(spi->controller); + + nuvoton_qspi_set_cs_level(qspi, spi_get_chipselect(spi, 0), enable); +}
Review
Quoted 10 lines
(quoted diff) +10
-0+ +static void nuvoton_qspi_mem_set_cs(struct spi_device *spi, bool enable) +{ + struct nuvoton_qspi *qspi = spi_controller_get_devdata(spi->controller); + bool assert = enable; + + if (spi->mode & SPI_CS_HIGH) + assert = !assert; + + nuvoton_qspi_set_cs_level(qspi, spi_get_chipselect(spi, 0), assert);
[Severity: High]
Does this cause contradictory chip-select polarity between standard SPI transfers and SPI memory operations?
The SPI framework passes the physical level to drive to ctlr->set_cs(), where false means drive low and true means drive high. nuvoton_qspi_set_cs() passes this directly to nuvoton_qspi_set_cs_level().
However, nuvoton_qspi_mem_set_cs() treats its enable parameter as a logical assertion flag and performs a manual inversion.
For an active-low device, will one of these paths end up driving the chip-select line with the wrong polarity?
Quoted 1 line
+}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2