On Fri, Sep 2, 2022 at 4:19 PM Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote: > > If VREF pin is attached, we should use external VREF source instead of > the internal. Otherwise we will get wrong measurements on some of channel the channel > types. Below are minor changes, not sure if you need a new version for that. ... > + priv->vref_reg = devm_regulator_get_optional(dev, "vref"); > + if (IS_ERR(priv->vref_reg) && PTR_ERR(priv->vref_reg) != -ENODEV) > + return PTR_ERR(priv->vref_reg); > + > + if (IS_ERR_OR_NULL(priv->vref_reg)) { > + priv->vref_reg = NULL; > + /* Use internal reference */ > + priv->vref_mv = TI_TSC2046_INT_VREF; > + return 0; > + } This can be refactored now if (IS_ERR(priv->vref_reg)) { /* If regulator exists but can't be get, return an error */ if (PTR_ERR(priv->vref_reg) != -ENODEV) return PTR_ERR(priv->vref_reg); priv->vref_reg = NULL; } if (!priv->vref_reg) { /* Use internal reference */ priv->vref_mv = TI_TSC2046_INT_VREF; return 0; } ... > + ret = devm_add_action_or_reset(dev, tsc2046_adc_regulator_disable, > + priv); I believe it's fine to be on one line. > + if (ret) > + return ret; ... > + priv->vref_mv = ret / 1000; MILLI? -- With Best Regards, Andy Shevchenko