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. 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); > > > > > > ... > > > > > > > >
