* [bug report] iio: adc: ti-adc161s626: add regulator support
@ 2016-11-09 10:14 Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2016-11-09 10:14 UTC (permalink / raw)
To: mranostay
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
linux-iio
Hello Matt Ranostay,
The patch 92f0afb5b2be: "iio: adc: ti-adc161s626: add regulator
support" from Sep 18, 2016, leads to the following static checker
warning:
drivers/iio/adc/ti-adc161s626.c:237 ti_adc_probe()
error: 'data->ref' dereferencing possible ERR_PTR()
drivers/iio/adc/ti-adc161s626.c
214
215 data->ref = devm_regulator_get(&spi->dev, "vdda");
216 if (!IS_ERR(data->ref)) {
217 ret = regulator_enable(data->ref);
218 if (ret < 0)
219 return ret;
220 }
This code assumes that devm_regulator_get() can fail and we continue
with what we have, but none of the rest of the driver assumes that.
221
222 ret = iio_triggered_buffer_setup(indio_dev, NULL,
223 ti_adc_trigger_handler, NULL);
224 if (ret)
225 goto error_regulator_disable;
226
227 ret = iio_device_register(indio_dev);
228 if (ret)
229 goto error_unreg_buffer;
230
231 return 0;
232
233 error_unreg_buffer:
234 iio_triggered_buffer_cleanup(indio_dev);
235
236 error_regulator_disable:
237 regulator_disable(data->ref);
Every reference to "data->ref" needs to be updated to wrapped in
"if (!IS_ERR(data->ref)) " otherwise it will oops.
238
239 return ret;
240 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* [bug report] iio: adc: ti-adc161s626: add regulator support
@ 2016-10-25 19:06 Dan Carpenter
2016-10-28 7:58 ` Matt Ranostay
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2016-10-25 19:06 UTC (permalink / raw)
To: mranostay; +Cc: linux-iio
Hello Matt Ranostay,
The patch 92f0afb5b2be: "iio: adc: ti-adc161s626: add regulator
support" from Sep 18, 2016, leads to the following static checker
warning:
drivers/iio/adc/ti-adc161s626.c:237 ti_adc_probe()
error: 'data->ref' dereferencing possible ERR_PTR()
drivers/iio/adc/ti-adc161s626.c
214
215 data->ref = devm_regulator_get(&spi->dev, "vdda");
216 if (!IS_ERR(data->ref)) {
I don't understand what's going on here.
217 ret = regulator_enable(data->ref);
218 if (ret < 0)
219 return ret;
220 }
221
222 ret = iio_triggered_buffer_setup(indio_dev, NULL,
223 ti_adc_trigger_handler, NULL);
224 if (ret)
225 goto error_regulator_disable;
226
227 ret = iio_device_register(indio_dev);
228 if (ret)
229 goto error_unreg_buffer;
230
231 return 0;
232
233 error_unreg_buffer:
234 iio_triggered_buffer_cleanup(indio_dev);
235
236 error_regulator_disable:
237 regulator_disable(data->ref);
Static checker is correct.
238
239 return ret;
240 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] iio: adc: ti-adc161s626: add regulator support
2016-10-25 19:06 Dan Carpenter
@ 2016-10-28 7:58 ` Matt Ranostay
0 siblings, 0 replies; 3+ messages in thread
From: Matt Ranostay @ 2016-10-28 7:58 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Matt Ranostay, linux-iio
On Tue, Oct 25, 2016 at 12:06 PM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> Hello Matt Ranostay,
>
> The patch 92f0afb5b2be: "iio: adc: ti-adc161s626: add regulator
> support" from Sep 18, 2016, leads to the following static checker
> warning:
>
> drivers/iio/adc/ti-adc161s626.c:237 ti_adc_probe()
> error: 'data->ref' dereferencing possible ERR_PTR()
>
> drivers/iio/adc/ti-adc161s626.c
> 214
> 215 data->ref = devm_regulator_get(&spi->dev, "vdda");
> 216 if (!IS_ERR(data->ref)) {
>
> I don't understand what's going on here.
Ok seems weird but this is because of devm_regulator_get() returning a
dummy regulator (CONFIG_REGULATOR_DUMMY) in case "vdda" isn't defined.
So no reason to enable that...
>
> 217 ret = regulator_enable(data->ref);
> 218 if (ret < 0)
> 219 return ret;
> 220 }
> 221
> 222 ret = iio_triggered_buffer_setup(indio_dev, NULL,
> 223 ti_adc_trigger_handler, NULL);
> 224 if (ret)
> 225 goto error_regulator_disable;
> 226
> 227 ret = iio_device_register(indio_dev);
> 228 if (ret)
> 229 goto error_unreg_buffer;
> 230
> 231 return 0;
> 232
> 233 error_unreg_buffer:
> 234 iio_triggered_buffer_cleanup(indio_dev);
> 235
> 236 error_regulator_disable:
> 237 regulator_disable(data->ref);
>
> Static checker is correct.
Ok that is part is probably true and maybe needs a IS_ERR check
>
> 238
> 239 return ret;
> 240 }
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-09 10:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-09 10:14 [bug report] iio: adc: ti-adc161s626: add regulator support Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2016-10-25 19:06 Dan Carpenter
2016-10-28 7:58 ` Matt Ranostay
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).