[PATCH 1/4] spi: atmel-quadspi: Fix struct atmel_qspi_pcal kerneldoc

[PATCH 1/4] spi: atmel-quadspi: Fix struct atmel_qspi_pcal kerneldoc

[Krzysztof Kozlowski]

Correct the typo in parameter name for 'struct atmel_qspi_pcal'
kerneldoc and W=1 warnings:

  drivers/spi/atmel-quadspi.c:244: warning: Function parameter or struct member 'pclk_div' not described in 'atmel_qspi_pcal'
  drivers/spi/atmel-quadspi.c:244: warning: Excess struct member 'pclkdiv' description in 'atmel_qspi_pcal'

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/spi/atmel-quadspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index f46da363574f..d135cca4e454 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -235,7 +235,7 @@
 /**
  * struct atmel_qspi_pcal - Pad Calibration Clock Division
  * @pclk_rate: peripheral clock rate.
- * @pclkdiv: calibration clock division. The clock applied to the calibration
+ * @pclk_div: calibration clock division. The clock applied to the calibration
  *           cell is divided by pclkdiv + 1.
  */
 struct atmel_qspi_pcal {
-- 
2.43.0

[PATCH 2/4] spi: atmel-quadspi: Fix printed error code during DMA setup

[Krzysztof Kozlowski]

On dma_request_chan() failure driver NULL-ifies the 'rx_chan' and
immediately uses it as PTR_ERR() so dev_err_probe() prints incorrect
error code.  Rework the code so proper error code will be printed and
NULL-ifying of 'rx_chan' will happen in common error handling block
(failure of DMA setup is not fatal for the driver and further code
depends on 'rx_chan' being non-NULL for DMA operations).

Reported by Smatch:
  drivers/spi/atmel-quadspi.c:1287 atmel_qspi_dma_init() warn: passing zero to 'PTR_ERR'

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/spi/atmel-quadspi.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index d135cca4e454..057bc20a74ce 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -1283,9 +1283,9 @@ static int atmel_qspi_dma_init(struct spi_controller *ctrl)
 
 	aq->rx_chan = dma_request_chan(&aq->pdev->dev, "rx");
 	if (IS_ERR(aq->rx_chan)) {
-		aq->rx_chan = NULL;
-		return dev_err_probe(&aq->pdev->dev, PTR_ERR(aq->rx_chan),
-				     "RX DMA channel is not available\n");
+		ret = dev_err_probe(&aq->pdev->dev, PTR_ERR(aq->rx_chan),
+				    "RX DMA channel is not available\n");
+		goto null_rx_chan;
 	}
 
 	aq->tx_chan = dma_request_chan(&aq->pdev->dev, "tx");
@@ -1306,8 +1306,9 @@ static int atmel_qspi_dma_init(struct spi_controller *ctrl)
 
 release_rx_chan:
 	dma_release_channel(aq->rx_chan);
-	aq->rx_chan = NULL;
 	aq->tx_chan = NULL;
+null_rx_chan:
+	aq->rx_chan = NULL;
 	return ret;
 }
 
-- 
2.43.0

[PATCH 3/4] spi: cs42l43: Make handling missing spk-id GPIOs explicit

[Krzysztof Kozlowski]

gpiod_get_array_optional() for spk-id GPIOs can return NULL, if they are
missing, so do not pass the value to PTR_ERR but instead explicitly
treat NULL as acceptable condition.  The old code was correct, but
misleading because PTR_ERR usually is used on errors.

Reported by Smatch:
  drivers/spi/spi-cs42l43.c:241 cs42l43_get_speaker_id_gpios() warn: passing zero to 'PTR_ERR'

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/spi/spi-cs42l43.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c
index ceefc253c549..90180662c4c2 100644
--- a/drivers/spi/spi-cs42l43.c
+++ b/drivers/spi/spi-cs42l43.c
@@ -237,7 +237,9 @@ static int cs42l43_get_speaker_id_gpios(struct cs42l43_spi *priv, int *result)
 	int i, ret;
 
 	descs = gpiod_get_array_optional(priv->dev, "spk-id", GPIOD_IN);
-	if (IS_ERR_OR_NULL(descs))
+	if (!descs)
+		return 0;
+	else if (IS_ERR_OR_NULL(descs))
 		return PTR_ERR(descs);
 
 	spkid = 0;
-- 
2.43.0

Re: [PATCH 3/4] spi: cs42l43: Make handling missing spk-id GPIOs explicit

[Charles Keepax]

On Sat, Jan 04, 2025 at 09:54:36PM +0100, Krzysztof Kozlowski wrote:
> gpiod_get_array_optional() for spk-id GPIOs can return NULL, if they are
> missing, so do not pass the value to PTR_ERR but instead explicitly
> treat NULL as acceptable condition.  The old code was correct, but
> misleading because PTR_ERR usually is used on errors.
> 
> Reported by Smatch:
>   drivers/spi/spi-cs42l43.c:241 cs42l43_get_speaker_id_gpios() warn: passing zero to 'PTR_ERR'
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/spi/spi-cs42l43.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c
> index ceefc253c549..90180662c4c2 100644
> --- a/drivers/spi/spi-cs42l43.c
> +++ b/drivers/spi/spi-cs42l43.c
> @@ -237,7 +237,9 @@ static int cs42l43_get_speaker_id_gpios(struct cs42l43_spi *priv, int *result)
>  	int i, ret;
>  
>  	descs = gpiod_get_array_optional(priv->dev, "spk-id", GPIOD_IN);
> -	if (IS_ERR_OR_NULL(descs))
> +	if (!descs)
> +		return 0;
> +	else if (IS_ERR_OR_NULL(descs))

Should switch to using just IS_ERR() if adding an explicit case
for the NULL. Otherwise looks good to me.

Thanks,
Charles

Re: [PATCH 3/4] spi: cs42l43: Make handling missing spk-id GPIOs explicit

[Krzysztof Kozlowski]

On 06/01/2025 12:32, Charles Keepax wrote:
> On Sat, Jan 04, 2025 at 09:54:36PM +0100, Krzysztof Kozlowski wrote:
>> gpiod_get_array_optional() for spk-id GPIOs can return NULL, if they are
>> missing, so do not pass the value to PTR_ERR but instead explicitly
>> treat NULL as acceptable condition.  The old code was correct, but
>> misleading because PTR_ERR usually is used on errors.
>>
>> Reported by Smatch:
>>   drivers/spi/spi-cs42l43.c:241 cs42l43_get_speaker_id_gpios() warn: passing zero to 'PTR_ERR'
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  drivers/spi/spi-cs42l43.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c
>> index ceefc253c549..90180662c4c2 100644
>> --- a/drivers/spi/spi-cs42l43.c
>> +++ b/drivers/spi/spi-cs42l43.c
>> @@ -237,7 +237,9 @@ static int cs42l43_get_speaker_id_gpios(struct cs42l43_spi *priv, int *result)
>>  	int i, ret;
>>  
>>  	descs = gpiod_get_array_optional(priv->dev, "spk-id", GPIOD_IN);
>> -	if (IS_ERR_OR_NULL(descs))
>> +	if (!descs)
>> +		return 0;
>> +	else if (IS_ERR_OR_NULL(descs))
> 
> Should switch to using just IS_ERR() if adding an explicit case
> for the NULL. Otherwise looks good to me.
Yes, of course. I'll fix it in v2.

Best regards,
Krzysztof

[PATCH 4/4] spi: cadence-quadspi: Assume device could match via platform

[Krzysztof Kozlowski]

Driver has only of_device_id table, however it also has MODULE_ALIAS()
for platform name, thus assume there is a configuration where this can
be loaded outside of OF system.  In such case of_device_get_match_data()
will return NULL, which is already checked in one place of probe()
function but not in the other, leading to Smatch warning:

  drivers/spi/spi-cadence-quadspi.c:1942 cqspi_probe() error: we previously assumed 'ddata' could be null (see line 1885)

Driver should be consistent, so assume device can be matched via
platform bus and of_device_get_match_data() can indeed return NULL.
This is also possible with malformed DTS on OF-platform: no unit address
and device node name matching driver name.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/spi/spi-cadence-quadspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 47477f2d9a25..e9197bf9d739 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1939,7 +1939,7 @@ static int cqspi_probe(struct platform_device *pdev)
 
 	host->num_chipselect = cqspi->num_chipselect;
 
-	if (ddata->quirks & CQSPI_SUPPORT_DEVICE_RESET)
+	if (ddata && (ddata->quirks & CQSPI_SUPPORT_DEVICE_RESET))
 		cqspi_device_reset(cqspi);
 
 	if (cqspi->use_direct_mode) {
-- 
2.43.0

Re: [PATCH 1/4] spi: atmel-quadspi: Fix struct atmel_qspi_pcal kerneldoc

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 426 bytes --]

On Sat, Jan 04, 2025 at 09:54:34PM +0100, Krzysztof Kozlowski wrote:
> Correct the typo in parameter name for 'struct atmel_qspi_pcal'
> kerneldoc and W=1 warnings:

As mentioned in submitting-patches.rst when submitting a patch series
you should supply a cover letter for that patch series which describes
the overall content of the series.  This helps people understand what
they are looking at and how things fit together.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]