On Wednesday, April 17, 2019 12:25 AM +0900, Mark Brown wrote: > On Tue, Apr 16, 2019 at 01:37:27PM +0900, Eric Jeong wrote: > > > +static int slg51000_regulator_is_enabled(struct regulator_dev *rdev) > > +{ > > + struct slg51000 *chip = rdev_get_drvdata(rdev); > > + int ret, id = rdev_get_id(rdev); > > + unsigned int state; > > + > > + ret = regmap_read(chip->regmap, es_reg[id].sreg, &state); > > + if (ret < 0) { > > + dev_err(chip->dev, "Failed to read status register(%d)\n", > > + ret); > > + return ret; > > + } > > + > > + if (!(state & SLG51000_STA_ILIM_FLAG_MASK) && > > + (state & SLG51000_STA_VOUT_OK_FLAG_MASK)) > > + return 1; > > + else > > + return 0; > > This looks like it should be a get_status() operation as it's reading > status bits rather than the command we sent to the device - for that > just use regulator_is_enabled_regmap(). I thought that it needs to return current status of a regulator when the function is called. I am wondering that the *is_enabled()* function is just to check If a regulator has been turned on or not rather than getting current status of the regulator. Thanks Eric