| Message ID | 1443196460-26156-10-git-send-email-dannenberg@ti.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi, On Fri, Sep 25, 2015 at 10:54:14AM -0500, Andreas Dannenberg wrote: > @@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq) > return PTR_ERR_OR_ZERO(bq->charger); > } > > -static int bq24257_pg_gpio_probe(struct bq24257_device *bq) > +static void bq24257_pg_gpio_probe(struct bq24257_device *bq) > { > - bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN); > + bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN); > + > if (IS_ERR(bq->pg)) { > - dev_err(bq->dev, "could not probe PG pin\n"); > - return PTR_ERR(bq->pg); > + dev_err(bq->dev, "error probing PG pin\n"); > + bq->pg = NULL; > + return; > } You should handle -EPROBE_DEFER here. -- Sebastian
On Sun, Sep 27, 2015 at 10:20:26PM +0200, Sebastian Reichel wrote: > Hi, > > On Fri, Sep 25, 2015 at 10:54:14AM -0500, Andreas Dannenberg wrote: > > @@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq) > > return PTR_ERR_OR_ZERO(bq->charger); > > } > > > > -static int bq24257_pg_gpio_probe(struct bq24257_device *bq) > > +static void bq24257_pg_gpio_probe(struct bq24257_device *bq) > > { > > - bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN); > > + bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN); > > + > > if (IS_ERR(bq->pg)) { > > - dev_err(bq->dev, "could not probe PG pin\n"); > > - return PTR_ERR(bq->pg); > > + dev_err(bq->dev, "error probing PG pin\n"); > > + bq->pg = NULL; > > + return; > > } > > You should handle -EPROBE_DEFER here. Hi Sebastian. From having a quick look at the Kernel tree it seems like this error shouldn't get exposed to the user (which it doesn't since it would get zeroe'd out) and the error itself suggests that I should "retry" which none of the things I looked at in the Kernel through "git grep" seemed to do. So before making any assumptions I wanted to see if I'm missing something or you could give me any pointers how to proceed. Thanks, -- Andreas Dannenberg Texas Instruments Inc -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 28, 2015 at 07:04:12AM -0500, Andreas Dannenberg wrote: > On Sun, Sep 27, 2015 at 10:20:26PM +0200, Sebastian Reichel wrote: > > Hi, > > > > On Fri, Sep 25, 2015 at 10:54:14AM -0500, Andreas Dannenberg wrote: > > > @@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq) > > > return PTR_ERR_OR_ZERO(bq->charger); > > > } > > > > > > -static int bq24257_pg_gpio_probe(struct bq24257_device *bq) > > > +static void bq24257_pg_gpio_probe(struct bq24257_device *bq) > > > { > > > - bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN); > > > + bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN); > > > + > > > if (IS_ERR(bq->pg)) { > > > - dev_err(bq->dev, "could not probe PG pin\n"); > > > - return PTR_ERR(bq->pg); > > > + dev_err(bq->dev, "error probing PG pin\n"); > > > + bq->pg = NULL; > > > + return; > > > } > > > > You should handle -EPROBE_DEFER here. > > Hi Sebastian. From having a quick look at the Kernel tree it seems like > this error shouldn't get exposed to the user (which it doesn't since it > would get zeroe'd out) and the error itself suggests that I should > "retry" which none of the things I looked at in the Kernel through "git > grep" seemed to do. So before making any assumptions I wanted to see if > I'm missing something or you could give me any pointers how to proceed. Ok after checking drivers/base/dd.c it seems like what I should be doing here is to simply exit the probe function and pass on -EPROBE_DEFER as the return value of the probe function, and the system would then automatically take care of re-trying the probe later, correct? Please let know, I want to make sure this patch series is converging into an acceptable state. Regards, -- Andreas Dannenberg Texas Instruments Inc -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Mon, Sep 28, 2015 at 07:04:12AM -0500, Andreas Dannenberg wrote: > On Sun, Sep 27, 2015 at 10:20:26PM +0200, Sebastian Reichel wrote: > > On Fri, Sep 25, 2015 at 10:54:14AM -0500, Andreas Dannenberg wrote: > > > @@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq) > > > return PTR_ERR_OR_ZERO(bq->charger); > > > } > > > > > > -static int bq24257_pg_gpio_probe(struct bq24257_device *bq) > > > +static void bq24257_pg_gpio_probe(struct bq24257_device *bq) > > > { > > > - bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN); > > > + bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN); > > > + > > > if (IS_ERR(bq->pg)) { > > > - dev_err(bq->dev, "could not probe PG pin\n"); > > > - return PTR_ERR(bq->pg); > > > + dev_err(bq->dev, "error probing PG pin\n"); > > > + bq->pg = NULL; > > > + return; > > > } > > > > You should handle -EPROBE_DEFER here. > > Hi Sebastian. From having a quick look at the Kernel tree it seems like > this error shouldn't get exposed to the user (which it doesn't since it > would get zeroe'd out) and the error itself suggests that I should > "retry" which none of the things I looked at in the Kernel through "git > grep" seemed to do. So before making any assumptions I wanted to see if > I'm missing something or you could give me any pointers how to proceed. If you return -EPROBE_DEFER from your driver's probe function, the driver probe will fail. In opposit to other error codes it will be marked for retry at a later point, though. You will get EPROBE_DEFER, if you try to access a GPIO from a not-yet initialized GPIO controller (e.g. if you load the charger driver before the gpio driver). Correct handling is simple: Just forward it, making sure, that all non-devm probed resources are correctly free'd. -- Sebastian
diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c index 7bbff80..5800697 100644 --- a/drivers/power/bq24257_charger.c +++ b/drivers/power/bq24257_charger.c @@ -359,7 +359,26 @@ static int bq24257_get_chip_state(struct bq24257_device *bq, state->fault = ret; - state->power_good = !gpiod_get_value_cansleep(bq->pg); + if (bq->pg) + state->power_good = !gpiod_get_value_cansleep(bq->pg); + else + /* + * If we have a chip without a dedicated power-good GPIO or + * some other explicit bit that would provide this information + * assume the power is good if there is no supply related + * fault - and not good otherwise. There is a possibility for + * other errors to mask that power in fact is not good but this + * is probably the best we can do here. + */ + switch (state->fault) { + case FAULT_INPUT_OVP: + case FAULT_INPUT_UVLO: + case FAULT_INPUT_LDO_LOW: + state->power_good = false; + break; + default: + state->power_good = true; + } return 0; } @@ -651,15 +670,18 @@ static int bq24257_power_supply_init(struct bq24257_device *bq) return PTR_ERR_OR_ZERO(bq->charger); } -static int bq24257_pg_gpio_probe(struct bq24257_device *bq) +static void bq24257_pg_gpio_probe(struct bq24257_device *bq) { - bq->pg = devm_gpiod_get_index(bq->dev, BQ24257_PG_GPIO, 0, GPIOD_IN); + bq->pg = devm_gpiod_get_optional(bq->dev, BQ24257_PG_GPIO, GPIOD_IN); + if (IS_ERR(bq->pg)) { - dev_err(bq->dev, "could not probe PG pin\n"); - return PTR_ERR(bq->pg); + dev_err(bq->dev, "error probing PG pin\n"); + bq->pg = NULL; + return; } - return 0; + if (bq->pg) + dev_dbg(bq->dev, "probed PG pin = %d\n", desc_to_gpio(bq->pg)); } static int bq24257_fw_probe(struct bq24257_device *bq) @@ -789,10 +811,17 @@ static int bq24257_probe(struct i2c_client *client, INIT_DELAYED_WORK(&bq->iilimit_setup_work, bq24257_iilimit_setup_work); - /* we can only check Power Good status by probing the PG pin */ - ret = bq24257_pg_gpio_probe(bq); - if (ret < 0) - return ret; + /* + * The BQ24250 doesn't have a dedicated Power Good (PG) pin so let's + * not probe for it and instead use a SW-based approach to determine + * the PG state. We also use a SW-based approach for all other devices + * if the PG pin is either not defined or can't be probed. + */ + if (bq->chip != BQ24250) + bq24257_pg_gpio_probe(bq); + + if (!bq->pg) + dev_info(bq->dev, "using SW-based power-good detection\n"); /* reset all registers to defaults */ ret = bq24257_field_write(bq, F_RESET, 1);
A software-based approach for determining the charger's input voltage "Power Good" state is introduced for devices like the bq24250 which don't have a dedicated hardware pin for that purpose. This SW-based approach is also used for other devices (with dedicated PG pin) as a fall back solution if that pin is not configured to be used through "pg-gpios". Signed-off-by: Andreas Dannenberg <dannenberg@ti.com> --- drivers/power/bq24257_charger.c | 49 ++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 10 deletions(-)