Re: [PATCH 2/2] spi: ma35d1: Add Nuvoton MA35D1 SPI controller support
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-23 04:30:17
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] spi: ma35d1: arbitrary num-cs limit rejects valid GPIO chip selects - [Medium] spi: ma35d1: missing PM suspend/resume hooks breaks hardware after deep sleep -- commit 5e29b920c6ee893a0356b0c18698305417951afd Author: Chi-Wen Weng [off-list ref] spi: ma35d1: Add Nuvoton MA35D1 SPI controller support This commit adds support for the SPI controller found in the Nuvoton MA35D1 SoC, handling SPI host mode using PIO transfers and configuring clock dividers, chip selects, and basic transfer settings.
quoted hunk ↗ jump to hunk
diff --git a/drivers/spi/spi-ma35d1.c b/drivers/spi/spi-ma35d1.c new file mode 100644 index 0000000000000..14aebf5019203 --- /dev/null +++ b/drivers/spi/spi-ma35d1.c
[ ... ]
quoted hunk ↗ jump to hunk
+static void nuvoton_spi_set_cs_level(struct nuvoton_spi *nspi, + unsigned int cs, bool assert) +{ + u32 mask; + + switch (cs) { + case 0: + mask = NUVOTON_SPI_SSCTL_SS0_MASK; + break; + case 1: + mask = NUVOTON_SPI_SSCTL_SS1_MASK; + break; + default: + dev_warn(nspi->dev, "invalid chip select %u\n", cs); + return; + } + + nuvoton_spi_update_ssctl_bits(nspi, mask, assert ? mask : 0); +}
[ ... ]
+static int nuvoton_spi_hw_init(struct nuvoton_spi *nspi)
+{[ ... ]
quoted hunk ↗ jump to hunk
+ nuvoton_spi_update_bits(nspi, NUVOTON_SPI_PDMACTL_OFFSET, + NUVOTON_SPI_PDMACTL_TXPDMAEN_MASK | + NUVOTON_SPI_PDMACTL_RXPDMAEN_MASK, 0); + + fifo_mask = NUVOTON_SPI_FIFOCTL_SLVBERX_MASK | + NUVOTON_SPI_FIFOCTL_TXUFIEN_MASK | + NUVOTON_SPI_FIFOCTL_TXUFPOL_MASK | + NUVOTON_SPI_FIFOCTL_RXOVIEN_MASK | + NUVOTON_SPI_FIFOCTL_RXTOIEN_MASK | + NUVOTON_SPI_FIFOCTL_TXTHIEN_MASK | + NUVOTON_SPI_FIFOCTL_RXTHIEN_MASK; + + nuvoton_spi_update_bits(nspi, NUVOTON_SPI_FIFOCTL_OFFSET, fifo_mask, 0);
[Severity: Medium] Will these critical non-transfer-specific register initializations (like PDMA and FIFO control configurations) be lost if the hardware loses state during a deep sleep? [ ... ]
quoted hunk ↗ jump to hunk
+static void nuvoton_spi_set_cs(struct spi_device *spi, bool level) +{ + struct nuvoton_spi *nspi = spi_controller_get_devdata(spi->controller); + + /* + * The SPI core passes the physical CS level to ->set_cs(). This + * initial driver only supports active-low native chip selects. + */ + nuvoton_spi_set_cs_level(nspi, spi_get_chipselect(spi, 0), !level); +}
[Severity: Medium] Will this unconditionally pass the CS index to nuvoton_spi_set_cs_level() without checking if it is a GPIO chip select? The SPI core calls this function to ensure native CS lines are inactive even when using GPIO chip selects. This could result in nuvoton_spi_set_cs_level() spamming the kernel log with "invalid chip select" warnings on every transfer targeting a GPIO CS with an index >= 2. [ ... ]
+static int nuvoton_spi_probe(struct platform_device *pdev)
+{[ ... ]
quoted hunk ↗ jump to hunk
+ ret = device_property_read_u32(dev, "num-cs", &num_cs); + if (ret && ret != -EINVAL) + return dev_err_probe(dev, ret, "failed to read num-cs\n"); + + if (!num_cs || num_cs > NUVOTON_SPI_MAX_NATIVE_CS) + return dev_err_probe(dev, -EINVAL, "invalid num-cs %u\n", + num_cs);
[Severity: Medium] Does this check improperly reject valid device tree configurations? A user might legitimately specify more than two total chip selects by utilizing GPIOs alongside the native ones. [ ... ]
quoted hunk ↗ jump to hunk
+static struct platform_driver nuvoton_spi_driver = { + .driver = { + .name = "ma35d1-spi", + .of_match_table = nuvoton_spi_of_match, + }, + .probe = nuvoton_spi_probe, +}; +module_platform_driver(nuvoton_spi_driver);
[Severity: Medium] Does this platform driver lack a .pm assignment, causing it to completely miss system suspend and resume events? Without calling spi_controller_suspend() and spi_controller_resume(), the SPI core might not be notified to pause its queues, which could lead to hangs, timeouts, or data corruption on subsequent transfers when resuming from sleep. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260923041926.425551-1-cwweng.linux@gmail.com?part=2