Messages in this thread | | | From | Ricardo Ribalda Delgado <> | Date | Mon, 11 Jan 2016 17:23:27 +0100 | Subject | Re: [PATCH v5] iio: add ad5761 DAC driver |
| |
Hello Jonathan
Thanks for your review
On Sat, Jan 9, 2016 at 5:15 PM, Jonathan Cameron <jic23@kernel.org> wrote: > I'm a little unsure that we shouldn't just fail the probe if the > range is supplied. Even the default (the best option available) > could in theory do damage to a circuit with a maximum of 3V though > it's probably unlikely...
I personally hate when a driver needs a mandatory pdata. I expect a default behaviour on the absence of it.
>> + st->vref_reg = devm_regulator_get_optional(&st->spi->dev, "vref"); >> + if (!st->vref_reg || PTR_ERR(st->vref_reg) == -ENODEV) { > This is a little unusual... Only one of those errors is returned by > devm_regulator_get_optional if the regulator is not specified. > I believe from a quick look at the regulator code that it returns the > -ENODEV part. > > So how can it be null? >
After a fast look:
devm_regulator_get_optional-> _devm_regulator_get -> regulator_get (regulator/consumer.h)
Probably consumer.h cannot be reached at the same time than _devm_regulator_get. But for safety I rather leave the check.
I send v6 right away.
Thanks!
-- Ricardo Ribalda
| |