Re: [PATCH v2 2/3] spi: ma35d1: Add Nuvoton MA35D1 SPI controller support

[Chi-Wen Weng <cwweng.linux@gmail.com>]

sashiko-bot@kernel.org 於 2026/10/1 下午 09:37 寫道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] spi: ma35d1: probe fails on valid DT with num-cs > 2 when using GPIO CS
> --
> 
> commit 297501e7117bfad68b786ba4098b2489480c1405
> Author: Chi-Wen Weng <cwweng@nuvoton.com>
> 
> spi: ma35d1: Add Nuvoton MA35D1 SPI controller support
> 
> This commit adds a new driver for the SPI controller found in the Nuvoton
> MA35D1 SoC. The driver supports SPI host mode using polling-based PIO
> transfers, standard SPI modes, and natively controls up to two chip selects.
> 
>> diff --git a/drivers/spi/spi-ma35d1.c b/drivers/spi/spi-ma35d1.c
>> new file mode 100644
>> index 0000000000000..7560cb7e0b099
>> --- /dev/null
>> +++ b/drivers/spi/spi-ma35d1.c
> [ ... ]
>> +static int nuvoton_spi_probe(struct platform_device *pdev)
>> +{
> [ ... ]
>> +	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]
> Will this check inadvertently reject valid device tree configurations that use
> GPIO chip selects for additional devices?
> 
> The SPI core correctly supports expanding the total number of chip selects
> beyond the native hardware limit when using GPIO descriptors. Since this
> driver sets ctlr->use_gpio_descriptors to true, a device tree could specify a
> num-cs value greater than NUVOTON_SPI_MAX_NATIVE_CS (which is 2) in order to
> attach a 3rd or 4th SPI target using GPIOs.
> 
> Because nuvoton_spi_probe() checks the total num-cs read from the device
> tree directly against NUVOTON_SPI_MAX_NATIVE_CS and returns -EINVAL, could
> this artificially limit the bus to 2 devices overall and prevent GPIO chip
> select expansion?
> 
>> +
>> +	ctlr->num_chipselect = num_cs;
>> +	ctlr->max_native_cs = NUVOTON_SPI_MAX_NATIVE_CS;
>> +	ctlr->use_gpio_descriptors = true;
> [ ... ]
> 

Hi,

Thanks for the review.

I don't think this check prevents additional GPIO chip selects.

For the MA35D1 binding, `num-cs` is limited to the two native hardware
chip selects:

     num-cs:
       minimum: 1
       maximum: 2
       default: 2

Additional chip selects are described by entries in `cs-gpios`, rather
than by increasing `num-cs` beyond two.

Since the driver sets:

     ctlr->max_native_cs = NUVOTON_SPI_MAX_NATIVE_CS;
     ctlr->use_gpio_descriptors = true;

the SPI core handles the GPIO chip-select expansion in
`spi_get_gpio_descs()`. In particular, it updates `num_chipselect` using
the number of `cs-gpios` entries:

     ctlr->num_chipselect = max_t(int, nb, ctlr->num_chipselect);

For example, with two native chip selects and two additional GPIO chip
selects, `num-cs` remains 2 while `cs-gpios` contains four entries. The
SPI core then expands `ctlr->num_chipselect` to 4.

`max_native_cs` only limits chip-select indices which do not have a GPIO
descriptor, so the additional GPIO-backed chip selects are not rejected.

Therefore a DT with `num-cs > 2` is not valid for this controller
binding, while valid configurations using additional GPIO chip selects
continue to work through the SPI core.

Thanks,
Chi-Wen