Re: [PATCH v2 11/13] power: bq24257: Add platform data based initialization

Laurentiu Palcu
2015-09-10T14:40:53Z

On Tue, Sep 08, 2015 at 07:12:35PM -0500, Andreas Dannenberg wrote:
> The patch adds a way to setup and initialize the device through the use
> of platform data with configuration options equivalent to when using
> device firmware (DT or ACPI) for systems where this is not available.
>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> ---
> drivers/power/bq24257_charger.c | 95 +++++++++++++++++++++++++++++++++--
> include/linux/power/bq24257_charger.h | 30 +++++++++++
> 2 files changed, 120 insertions(+), 5 deletions(-)
> create mode 100644 include/linux/power/bq24257_charger.h
>
> diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c
> index 9a4a7a0..b5e82ed 100644
> --- a/drivers/power/bq24257_charger.c
> +++ b/drivers/power/bq24257_charger.c
> @@ -27,10 +27,13 @@
> #include <linux/gpio/consumer.h>
> #include <linux/interrupt.h>
> #include <linux/delay.h>
> +#include <linux/gpio.h>
>
> #include <linux/acpi.h>
> #include <linux/of.h>
>
> +#include <linux/power/bq24257_charger.h>
> +
> #define BQ24257_REG_1 0x00
> #define BQ24257_REG_2 0x01
> #define BQ24257_REG_3 0x02
> @@ -958,28 +961,102 @@ static int bq24257_power_supply_init(struct bq24257_device *bq)
>
> static int bq24257_irq_probe(struct bq24257_device *bq)
> {
> + struct bq24257_platform_data *pdata = bq->client->dev.platform_data;
> struct gpio_desc *stat_irq;
> + int ret;
> +
> + if (!pdata)
> + stat_irq = devm_gpiod_get_index(bq->dev, BQ24257_STAT_IRQ, 0,
> + GPIOD_IN);
> + else {
> + if (!gpio_is_valid(pdata->stat_gpio)) {
> + dev_err(bq->dev, "invalid stat_irq pin\n");
> + return -EINVAL;
> + }
> +
> + ret = devm_gpio_request_one(bq->dev, pdata->stat_gpio,
> + GPIOD_IN, BQ24257_STAT_IRQ);
> + if (ret) {
> + dev_err(bq->dev, "stat_irq pin request failed\n");
> + return ret;
> + }
> +
> + stat_irq = gpio_to_desc(pdata->stat_gpio);
> + }
>
> - stat_irq = devm_gpiod_get_index(bq->dev, BQ24257_STAT_IRQ, 0, GPIOD_IN);
> if (IS_ERR(stat_irq)) {
gpio_to_desc() can return NULL. In this case, IS_ERR() will be false. I
think this is not what you want...

The rest seems fine.

laurentiu