Re: [PATCH v2 3/3] spi: ma35d1-qspi: Add GPIO chip select support

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

sashiko-bot@kernel.org 於 2026/10/1 下午 09:46 寫道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] spi: incorrect validation of total chip selects limits GPIO expansion
> --
> 
> commit bf74299215867df971e01913ae533377b76db610
> Author: Chi-Wen Weng <cwweng@nuvoton.com>
> 
>      spi: ma35d1-qspi: Add GPIO chip select support
>      
>      The generic SPI controller binding allows GPIO-based chip selects, but
>      the MA35D1 QSPI driver does not currently enable the SPI core's GPIO
>      descriptor handling. This patch enables GPIO chip-select support and
>      specifies that the controller provides two native chip selects.
> 
>> diff --git a/drivers/spi/spi-ma35d1-qspi.c b/drivers/spi/spi-ma35d1-qspi.c
>> index 30e3239f777d6..0644bb2990b8c 100644
>> --- a/drivers/spi/spi-ma35d1-qspi.c
>> +++ b/drivers/spi/spi-ma35d1-qspi.c
> [ ... ]
>> @@ -638,18 +664,21 @@ static int nuvoton_qspi_probe(struct platform_device *pdev)
>>   		goto err_assert;
>>   	}
>>   
>> -	if (!num_cs || num_cs > NUVOTON_QSPI_MAX_NUM_CS) {
>> +	if (!num_cs || num_cs > NUVOTON_QSPI_MAX_NATIVE_CS) {
> 
> [Severity: Medium]
> Does this validation check incorrectly apply the native chip select limit to the
> total number of chip selects?
> 
> If the Devicetree specifies a num-cs greater than 2 to declare additional
> GPIO-backed chip selects, it appears this check will cause nuvoton_qspi_probe()
> to fail unconditionally, preventing the use of the new GPIO expansion feature.
> 
>>   		ret = dev_err_probe(dev, -EINVAL, "invalid num-cs %u\n",
>>   				     num_cs);
>>   		goto err_assert;
>>   	}
>>   
>>   	ctlr->num_chipselect = num_cs;
>> +	ctlr->max_native_cs = NUVOTON_QSPI_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_QSPI_MAX_NATIVE_CS;
     ctlr->use_gpio_descriptors = true;

the SPI core handles the GPIO chip-select expansion when the controller
is registered. The number of usable chip-select slots is expanded
according to the number of `cs-gpios` entries.

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 native chip-select indices. GPIO-backed chip
selects beyond that native limit are still valid.

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