| Message ID | 20250104205437.184782-3-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] spi: atmel-quadspi: Fix struct atmel_qspi_pcal kerneldoc | expand |
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
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
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;
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(-)