On 12/23/24 6:00 AM, Jonathan Cameron wrote: > On Fri, 20 Dec 2024 14:01:34 +0200 > Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> wrote: > ... >> + ret = devm_regulator_get_enable_optional(dev, "vddh"); >> + if (ret < 0 && ret != -ENODEV) >> + return dev_err_probe(dev, ret, "failed to enable vddh voltage\n"); >> + >> + ret = devm_regulator_get_enable_optional(dev, "vddl"); >> + if (ret < 0 && ret != -ENODEV) >> + return dev_err_probe(dev, ret, "failed to enable vddl voltage\n"); >> + >> + ret = devm_regulator_get_enable_optional(dev, "vrefbuf"); >> + if (ret < 0 && ret != -ENODEV) >> + return dev_err_probe(dev, ret, "failed to enable vrefbuf voltage\n"); >> + >> + if (ret > 0) > > I'm fairly sure that call never returns a positive. Will return 0 for success so I think this > should be if (ret == 0) Even better, make it 1 line instead of 4 and use the specific error code we are checking for: st->vrefbuf_en = ret != -ENODEV; > >> + st->vrefbuf_en = true; >> + else >> + st->vrefbuf_en = false; >> + >> + ret = devm_regulator_get_enable_optional(dev, "vrefio"); >> + if (ret < 0 && ret != -ENODEV) >> + return dev_err_probe(dev, ret, "failed to enable vrefio voltage\n"); >> + >> + if (ret > 0) > > Same here. ^ > >> + st->vrefio_en = true; >> + else >> + st->vrefio_en = false; >> + > ... > >> +} >