On Fri, 8 Nov 2024 14:12:29 -0500 Frank Li <Frank.li@xxxxxxx> wrote: > On Fri, Nov 08, 2024 at 07:13:20AM +0100, Christophe JAILLET wrote: > > Le 07/11/2024 à 20:49, Frank Li a écrit : > > > On Thu, Nov 07, 2024 at 08:38:20PM +0100, Christophe JAILLET wrote: > > > > Le 07/11/2024 à 20:18, Frank Li a écrit : > > > > > Use devm_* and dev_err_probe() simplify probe function and remove > > > > > vf610_adc_remove(). Change type of 'vref_uv' to int because > > > > > regulator_get_voltage() return type is int. > > > > > > > > > > Reviewed-by: Haibo Chen <haibo.chen-3arQi8VN3Tc-XMD5yJDbdMReXY1tMh2IBg@xxxxxxxxxxxxxxxx> > > > > > Signed-off-by: Frank Li <Frank.Li-3arQi8VN3Tc-XMD5yJDbdMReXY1tMh2IBg@xxxxxxxxxxxxxxxx> > > > > > --- > > > > ... > > > > > > > > > > > info->vref = devm_regulator_get(&pdev->dev, "vref"); > > > > > > > > With the change to devm_regulator_get_enable_read_voltage(), is it still > > > > needed? > > > > > > Suspend function need vref to disable regulator. > > > > Ok. > > > > But why switch to devm_regulator_get_enable_read_voltage() then? > > Shouldn't keeping regulator_get_voltage() be enough and simpler? > > Avoid goto err after devm_regulator_get_enable_read_voltage(), if use > regulator_enable(), it needs regulator_disable() in err handle branch after > it. Don't get the same regulator twice - that works but is likely to be error prone in the long run and is a bit too subtle. This isn't an appropriate use of devm_regulator_get_enable_read_voltage() Instead just use a local callback and devm_add_action_or_reset() to disable the regulator in the devm tear down path. Jonathan > > Frank > > > > > CJ > > > > > > > > > > > > > CJ > > > > > > > > > if (IS_ERR(info->vref)) > > > > > return PTR_ERR(info->vref); > > > > > - ret = regulator_enable(info->vref); > > > > > - if (ret) > > > > > - return ret; > > > > > - > > > > > - info->vref_uv = regulator_get_voltage(info->vref); > > > > > + info->vref_uv = devm_regulator_get_enable_read_voltage(&pdev->dev, "vref"); > > > > > + if (info->vref_uv < 0) > > > > > + return info->vref_uv; > > > > > device_property_read_u32_array(dev, "fsl,adck-max-frequency", info->max_adck_rate, 3); > > > > > > > > ... > > > > > > > > > > > >
