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

[Andreas Dannenberg <dannenberg@ti.com>]

On Thu, Sep 10, 2015 at 05:40:53PM +0300, Laurentiu Palcu wrote:
> 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...

Good catch! Fixed.

Thanks,

--
Andreas Dannenberg
Texas Instruments Inc

> 
> The rest seems fine.
> 
> laurentiu