On Fri, 11 Nov 2022 15:39:19 +0100
Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote:
> The ad74412r/ad74413r has an internal 2.5V reference output, which (by
> tying the REFOUT pin to the REFIN pin) can be used in lieu of an
> external 2.5V input reference.
>
> Support that case by using devm_regulator_get_optional(), and simply
> hardcode the 2500000 uV in ad74413r_get_output_current_scale().
>
> I'm not sure this is completely correct, but it's certainly better
> than the current behaviour, where when refin-supply is not defined in
> device tree, the regulator framework helpfully does its
>
> supply refin not found, using dummy regulator
You could reasonably assume that's a bug in the firmware.. See suggestions
in reply to patch 2. Given external wiring is involved, I don't think
we can assume absence of a regulator means that loop back is in place.
We need to indicate that explicitly in the binding in some way.
Jonathan
>
> thing. When we then do the regulator_get_voltage(), that dummy
> regulator of course doesn't support that operation and thus returns
> -22 (-EINVAL) which is used without being checked.
>
> Signed-off-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/iio/addac/ad74413r.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> index 37485be88a63..9f77d2f514de 100644
> --- a/drivers/iio/addac/ad74413r.c
> +++ b/drivers/iio/addac/ad74413r.c
> @@ -608,7 +608,10 @@ static int ad74413r_get_output_voltage_scale(struct ad74413r_state *st,
> static int ad74413r_get_output_current_scale(struct ad74413r_state *st,
> int *val, int *val2)
> {
> - *val = regulator_get_voltage(st->refin_reg);
> + if (st->refin_reg)
> + *val = regulator_get_voltage(st->refin_reg);
> + else
> + *val = 2500000;
> *val2 = st->sense_resistor_ohms * AD74413R_DAC_CODE_MAX * 1000;
>
> return IIO_VAL_FRACTIONAL;
> @@ -1313,19 +1316,25 @@ static int ad74413r_probe(struct spi_device *spi)
> if (IS_ERR(st->regmap))
> return PTR_ERR(st->regmap);
>
> - st->refin_reg = devm_regulator_get(st->dev, "refin");
> - if (IS_ERR(st->refin_reg))
> - return dev_err_probe(st->dev, PTR_ERR(st->refin_reg),
> - "Failed to get refin regulator\n");
> + st->refin_reg = devm_regulator_get_optional(st->dev, "refin");
> + if (IS_ERR(st->refin_reg)) {
> + ret = PTR_ERR(st->refin_reg);
> + if (ret != -ENODEV)
> + return dev_err_probe(st->dev, ret,
> + "Failed to get refin regulator\n");
> + st->refin_reg = NULL;
> + }
>
> - ret = regulator_enable(st->refin_reg);
> - if (ret)
> - return ret;
> + if (st->refin_reg) {
> + ret = regulator_enable(st->refin_reg);
> + if (ret)
> + return ret;
>
> - ret = devm_add_action_or_reset(st->dev, ad74413r_regulator_disable,
> + ret = devm_add_action_or_reset(st->dev, ad74413r_regulator_disable,
> st->refin_reg);
> - if (ret)
> - return ret;
> + if (ret)
> + return ret;
> + }
>
> st->sense_resistor_ohms = 100000000;
> device_property_read_u32(st->dev, "shunt-resistor-micro-ohms",
