On 12.01.2016 23:28, zhangqing wrote: > > > On 01/11/2016 09:03 PM, Krzysztof Kozlowski wrote: >> 2016-01-12 20:05 GMT+09:00 zhangqing <zhangqing at rock-chips.com>: >>> Setting the set_suspend_enable/disable callback to support >>> enable and disable the dcdc when system is suspend. >>> >>> Signed-off-by: zhangqing <zhangqing at rock-chips.com> >>> --- >>> drivers/regulator/fan53555.c | 18 ++++++++++++++++++ >>> 1 file changed, 18 insertions(+) >>> >>> diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c >>> index 4940e82..2cb5cc3 100644 >>> --- a/drivers/regulator/fan53555.c >>> +++ b/drivers/regulator/fan53555.c >>> @@ -114,6 +114,22 @@ static int fan53555_set_suspend_voltage(struct >>> regulator_dev *rdev, int uV) >>> return 0; >>> } >>> >>> +static int fan53555_set_suspend_enable(struct regulator_dev *rdev) >>> +{ >>> + struct fan53555_device_info *di = rdev_get_drvdata(rdev); >>> + >>> + return regmap_update_bits(di->regmap, di->sleep_reg, >>> + VSEL_BUCK_EN, VSEL_BUCK_EN); >> >> You are just writing the enable_mask (BTW, just use the enable_mask, >> not the value itself in such case) instead of enabling the suspend >> mode. In the disable callback you are just disabling the regulator. >> >> What do you want to achieve with these callbacks? > return regmap_update_bits(di->regmap, di->sleep_reg, > VSEL_BUCK_EN, VSEL_BUCK_EN); > This callback is setting sleep_reg, setting this dcdc output is enable > or disable when system enter sleep. > In our system this dcdc need disabled when sleep. But the current > software not support. > I missed that the register is different - sleep_reg instead of enable_reg. It makes sense and as this is separate register then usage of desc->enable_mask is up to you, I think. Reviewed-by: Krzysztof Kozlowski <k.kozlowski at samsung.com> Best regards, Krzysztof