Re: [PATCH v4 1/2] power: supply: add sbs-charger driver

[Manish Badarkhe]

Hi Nicolas

On Wed, Dec 21, 2016 at 3:00 PM, Nicolas Saenz Julienne
<nicolas.sa...@prodys.net> wrote:
> Hi Manish, thanks for the interest.
>
> On 20/12/16 17:54, Manish Badarkhe wrote:
>> Hi Nicola
>>
> [...]
>>
>> Just some general comment, Can you add some more properties here to
>> know voltage and current?
>
> I assume you are mentioning reading current and voltage values. These
> properties are not supported by the sbs-charger standard. All you can do
> is set (write only) the charging current and charging voltage values.
> Yet in all the boards I've worked with it's the sbs-battery compliant
> chip that takes care of it.

thanks for clarifying me.


>> Also, can you add other properties present in charging status register
>> like POWER_FAIL, VOLTAGE_OR,
>> CURRENT_OR etc.
>
> CURRENT_OR and VOLTAGE_OR relate to wrong values in the charging
> voltage/current registers. Since the sbs-battery chip takes care of it I
> don't see any use for it.
> POWER_FAIL marks there is not enough voltage to charge the battery. I
> don't need that information so I did nothing to integrate it. Having a
> quick look at the power supply defines I don't see were it could easily fit.

thanks for clarifying me.

Regards
Manish Badarkhe

Re: [PATCH v4 1/2] power: supply: add sbs-charger driver

[Manish Badarkhe]

Hi Nicolas

On Wed, Dec 21, 2016 at 3:00 PM, Nicolas Saenz Julienne
 wrote:
> Hi Manish, thanks for the interest.
>
> On 20/12/16 17:54, Manish Badarkhe wrote:
>> Hi Nicola
>>
> [...]
>>
>> Just some general comment, Can you add some more properties here to
>> know voltage and current?
>
> I assume you are mentioning reading current and voltage values. These
> properties are not supported by the sbs-charger standard. All you can do
> is set (write only) the charging current and charging voltage values.
> Yet in all the boards I've worked with it's the sbs-battery compliant
> chip that takes care of it.

thanks for clarifying me.


>> Also, can you add other properties present in charging status register
>> like POWER_FAIL, VOLTAGE_OR,
>> CURRENT_OR etc.
>
> CURRENT_OR and VOLTAGE_OR relate to wrong values in the charging
> voltage/current registers. Since the sbs-battery chip takes care of it I
> don't see any use for it.
> POWER_FAIL marks there is not enough voltage to charge the battery. I
> don't need that information so I did nothing to integrate it. Having a
> quick look at the power supply defines I don't see were it could easily fit.

thanks for clarifying me.

Regards
Manish Badarkhe

Re: [PATCH v4 1/2] power: supply: add sbs-charger driver

[Manish Badarkhe]

Hi Nicola

On Tue, Dec 20, 2016 at 9:01 PM, Nicolas Saenz Julienne
<nicolas.sa...@prodys.net> wrote:
> This adds support for sbs-charger compilant chips as defined here:
> http://sbs-forum.org/specs/sbc110.pdf
>
> This was tested on a arm board connected to an LTC4100 battery charger
> chip.
>
> Signed-off-by: Nicolas Saenz Julienne <nicolas.sa...@prodys.net>
> ---
>  v3 -> v4
>  - drop "lltc,ltc4100" compatible string for now
>
>  v2 -> v3:
>  - add readable_reg() function to regmap config
>  - update compatible strings with part number
>
>  v1 -> v2:
>  - add spec link in header
>  - use proper gpio/interrupt interface
>  - update regmap configuration (max register & endianness)
>  - dropped oldschool .supplied_to assignments
>  - use devm_* APIs
>  drivers/power/supply/Kconfig   |   6 +
>  drivers/power/supply/Makefile  |   1 +
>  drivers/power/supply/sbs-charger.c | 274 
> +
>  3 files changed, 281 insertions(+)
>  create mode 100644 drivers/power/supply/sbs-charger.c


Just some general comment, Can you add some more properties here to
know voltage and current?
Also, can you add other properties present in charging status register
like POWER_FAIL, VOLTAGE_OR,
CURRENT_OR etc.

Don't know weather it is feasible to add or not and Also, let me know,
if it is already been covered in
some part of the code.

Thanks
Manish Badarkhe

Re: [PATCH v4 1/2] power: supply: add sbs-charger driver

[Manish Badarkhe]

Hi Nicola

On Tue, Dec 20, 2016 at 9:01 PM, Nicolas Saenz Julienne
 wrote:
> This adds support for sbs-charger compilant chips as defined here:
> http://sbs-forum.org/specs/sbc110.pdf
>
> This was tested on a arm board connected to an LTC4100 battery charger
> chip.
>
> Signed-off-by: Nicolas Saenz Julienne 
> ---
>  v3 -> v4
>  - drop "lltc,ltc4100" compatible string for now
>
>  v2 -> v3:
>  - add readable_reg() function to regmap config
>  - update compatible strings with part number
>
>  v1 -> v2:
>  - add spec link in header
>  - use proper gpio/interrupt interface
>  - update regmap configuration (max register & endianness)
>  - dropped oldschool .supplied_to assignments
>  - use devm_* APIs
>  drivers/power/supply/Kconfig   |   6 +
>  drivers/power/supply/Makefile  |   1 +
>  drivers/power/supply/sbs-charger.c | 274 
> +
>  3 files changed, 281 insertions(+)
>  create mode 100644 drivers/power/supply/sbs-charger.c


Just some general comment, Can you add some more properties here to
know voltage and current?
Also, can you add other properties present in charging status register
like POWER_FAIL, VOLTAGE_OR,
CURRENT_OR etc.

Don't know weather it is feasible to add or not and Also, let me know,
if it is already been covered in
some part of the code.

Thanks
Manish Badarkhe

Re: [PATCH v4 3/3] gpio: lp873x: Add support for General Purpose Outputs

[Manish Badarkhe]

Hi Keerthy,

sorry ignore my last two comments in previous mail.

On Wed, Jun 29, 2016 at 4:13 PM, Manish Badarkhe
<badarkhe.man...@gmail.com> wrote:
> Hi Keerthy
>
> Some minor comment
>
>> +static int lp873x_gpio_direction_output(struct gpio_chip *chip,
>> +   unsigned int offset, int value)
>> +{
>> +   struct lp873x_gpio *gpio = gpiochip_get_data(chip);
>> +
>> +   /* Set the initial value */
>> +   regmap_update_bits(gpio->lp873->regmap, LP873X_REG_GPO_CTRL,
>> +  BIT(offset * 4), value ? BIT(offset * 4) : 0);
>> +
>> +   return 0;
>> +}

 Error needs to be return, this function always return 0.

 Regards
 Manish Badarkhe

Re: [PATCH v4 3/3] gpio: lp873x: Add support for General Purpose Outputs

[Manish Badarkhe]

Hi Keerthy,

sorry ignore my last two comments in previous mail.

On Wed, Jun 29, 2016 at 4:13 PM, Manish Badarkhe
 wrote:
> Hi Keerthy
>
> Some minor comment
>
>> +static int lp873x_gpio_direction_output(struct gpio_chip *chip,
>> +   unsigned int offset, int value)
>> +{
>> +   struct lp873x_gpio *gpio = gpiochip_get_data(chip);
>> +
>> +   /* Set the initial value */
>> +   regmap_update_bits(gpio->lp873->regmap, LP873X_REG_GPO_CTRL,
>> +  BIT(offset * 4), value ? BIT(offset * 4) : 0);
>> +
>> +   return 0;
>> +}

 Error needs to be return, this function always return 0.

 Regards
 Manish Badarkhe

Re: [PATCH v4 3/3] gpio: lp873x: Add support for General Purpose Outputs

[Manish Badarkhe]

Hi Keerthy

Some minor comment

> +static int lp873x_gpio_direction_output(struct gpio_chip *chip,
> +   unsigned int offset, int value)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(chip);
> +
> +   /* Set the initial value */
> +   regmap_update_bits(gpio->lp873->regmap, LP873X_REG_GPO_CTRL,
> +  BIT(offset * 4), value ? BIT(offset * 4) : 0);
> +
> +   return 0;
> +}

Error needs to be return, this function always return 0.

> +
> +static int lp873x_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(chip);
> +   int ret, val;
> +
> +   ret = regmap_read(gpio->lp873->regmap, LP873X_REG_GPO_CTRL, );
> +   if (ret < 0)
> +   return ret;
> +
> +   return val & BIT(offset * 4);
> +}
> +
> +static void lp873x_gpio_set(struct gpio_chip *chip, unsigned int offset,
> +   int value)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(chip);
> +
> +   regmap_update_bits(gpio->lp873->regmap, LP873X_REG_GPO_CTRL,
> +  BIT(offset * 4), value ? BIT(offset * 4) : 0);
> +}
> +
> +static int lp873x_gpio_request(struct gpio_chip *gc, unsigned int offset)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(gc);
> +   int ret;
> +
> +   switch (offset) {
> +   case 0:
> +   /* No MUX Set up Needed for GPO */
> +   break;
> +   case 1:
> +   /* Setup the CLKIN_PIN_SEL MUX to GPO2 */
> +   ret = regmap_update_bits(gpio->lp873->regmap, 
> LP873X_REG_CONFIG,
> +LP873X_CONFIG_CLKIN_PIN_SEL, 0);
> +   if (ret)
> +   return ret;
> +
> +   break;
> +   default:
> +   return -EINVAL;
> +   }
> +
> +   return 0;
> +}

Error needs to be return, this function always return 0. Only default
returns error here which is unlikely condition.

> +
> +
> +static int lp873x_gpio_set_single_ended(struct gpio_chip *gc,
> +   unsigned int offset,
> +   enum single_ended_mode mode)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(gc);
> +
> +   switch (mode) {
> +   case LINE_MODE_OPEN_DRAIN:
> +   return regmap_update_bits(gpio->lp873->regmap,
> + LP873X_REG_GPO_CTRL,
> + BIT(offset * 4 + 2),
> + BIT(offset * 4 + 2));
> +   case LINE_MODE_PUSH_PULL:
> +   return regmap_update_bits(gpio->lp873->regmap,
> + LP873X_REG_GPO_CTRL,
> + BIT(offset * 4 + 2), 0);
> +   default:
> +   return -ENOTSUPP;
> +   }
> +}

Error needs to be return, this function always return 0. Only default
returns error here which is unlikely condition.

Regards
Manish Badarkhe

Re: [PATCH v4 3/3] gpio: lp873x: Add support for General Purpose Outputs

[Manish Badarkhe]

Hi Keerthy

Some minor comment

> +static int lp873x_gpio_direction_output(struct gpio_chip *chip,
> +   unsigned int offset, int value)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(chip);
> +
> +   /* Set the initial value */
> +   regmap_update_bits(gpio->lp873->regmap, LP873X_REG_GPO_CTRL,
> +  BIT(offset * 4), value ? BIT(offset * 4) : 0);
> +
> +   return 0;
> +}

Error needs to be return, this function always return 0.

> +
> +static int lp873x_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(chip);
> +   int ret, val;
> +
> +   ret = regmap_read(gpio->lp873->regmap, LP873X_REG_GPO_CTRL, );
> +   if (ret < 0)
> +   return ret;
> +
> +   return val & BIT(offset * 4);
> +}
> +
> +static void lp873x_gpio_set(struct gpio_chip *chip, unsigned int offset,
> +   int value)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(chip);
> +
> +   regmap_update_bits(gpio->lp873->regmap, LP873X_REG_GPO_CTRL,
> +  BIT(offset * 4), value ? BIT(offset * 4) : 0);
> +}
> +
> +static int lp873x_gpio_request(struct gpio_chip *gc, unsigned int offset)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(gc);
> +   int ret;
> +
> +   switch (offset) {
> +   case 0:
> +   /* No MUX Set up Needed for GPO */
> +   break;
> +   case 1:
> +   /* Setup the CLKIN_PIN_SEL MUX to GPO2 */
> +   ret = regmap_update_bits(gpio->lp873->regmap, 
> LP873X_REG_CONFIG,
> +LP873X_CONFIG_CLKIN_PIN_SEL, 0);
> +   if (ret)
> +   return ret;
> +
> +   break;
> +   default:
> +   return -EINVAL;
> +   }
> +
> +   return 0;
> +}

Error needs to be return, this function always return 0. Only default
returns error here which is unlikely condition.

> +
> +
> +static int lp873x_gpio_set_single_ended(struct gpio_chip *gc,
> +   unsigned int offset,
> +   enum single_ended_mode mode)
> +{
> +   struct lp873x_gpio *gpio = gpiochip_get_data(gc);
> +
> +   switch (mode) {
> +   case LINE_MODE_OPEN_DRAIN:
> +   return regmap_update_bits(gpio->lp873->regmap,
> + LP873X_REG_GPO_CTRL,
> + BIT(offset * 4 + 2),
> + BIT(offset * 4 + 2));
> +   case LINE_MODE_PUSH_PULL:
> +   return regmap_update_bits(gpio->lp873->regmap,
> + LP873X_REG_GPO_CTRL,
> + BIT(offset * 4 + 2), 0);
> +   default:
> +   return -ENOTSUPP;
> +   }
> +}

Error needs to be return, this function always return 0. Only default
returns error here which is unlikely condition.

Regards
Manish Badarkhe

Re: [PATCH v2] thermal: add Intel BXT WhiskeyCove PMIC thermal driver

[Manish Badarkhe]

Hi


> +static int pmic_thermal_remove(struct platform_device *pdev)
> +{
> +   return 0;
> +}
> +

Why this remove function still requires?

Regards
Manish Badarkhe

Re: [PATCH v2] thermal: add Intel BXT WhiskeyCove PMIC thermal driver

[Manish Badarkhe]

Hi


> +static int pmic_thermal_remove(struct platform_device *pdev)
> +{
> +   return 0;
> +}
> +

Why this remove function still requires?

Regards
Manish Badarkhe

Re: [RFC 1/1] drivers: i2c: omap: Add slave support

[Manish Badarkhe]

Hi Ravikumar

Some sanity comments, just good to have.

> +#ifdef CONFIG_I2C_SLAVE
> +static int omap_i2c_slave_irq(struct omap_i2c_dev *omap)
> +{
> +   u16 stat_raw;
> +   u16 stat;
> +   u16 bits;
> +   u8 value;
> +
> +   stat_raw = omap_i2c_read_reg(omap, OMAP_I2C_IP_V2_IRQSTATUS_RAW);
> +   bits = omap_i2c_read_reg(omap, OMAP_I2C_IE_REG);
> +   stat_raw &= bits;
> +
> +   if (stat_raw & OMAP_I2C_STAT_AAS) {
> +   omap_i2c_ack_stat(omap, OMAP_I2C_STAT_AAS);
> +   stat_raw &= ~OMAP_I2C_STAT_AAS;
> +   }
> +
> +out:
> +   return 0;
> +}
> +#endif

This function always return 0.

> +#ifdef CONFIG_I2C_SLAVE
> +static int omap_i2c_reg_slave(struct i2c_client *slave)
> +{
> +   struct omap_i2c_dev *omap = i2c_get_adapdata(slave->adapter);
> +   u16 reg;
> +   int ret = 0;
> +   /* Enable necessary interrupts */
> +   omap_i2c_write_reg(omap, OMAP_I2C_IE_REG, omap->iestate);
> +
> +   return 0;
> +
> +}

Better to return "ret" here as already been initialized to 0

> +
> +static int omap_i2c_unreg_slave(struct i2c_client *slave)
> +{
> +   struct omap_i2c_dev *omap = i2c_get_adapdata(slave->adapter);
> +   u16 reg;
> +
> +   pm_runtime_put(omap->dev);
> +   return 0;
> +}
> +#endif

This function always return 0


Regards
Manish Badarkhe

Re: [RFC 1/1] drivers: i2c: omap: Add slave support

[Manish Badarkhe]

Hi Ravikumar

Some sanity comments, just good to have.

> +#ifdef CONFIG_I2C_SLAVE
> +static int omap_i2c_slave_irq(struct omap_i2c_dev *omap)
> +{
> +   u16 stat_raw;
> +   u16 stat;
> +   u16 bits;
> +   u8 value;
> +
> +   stat_raw = omap_i2c_read_reg(omap, OMAP_I2C_IP_V2_IRQSTATUS_RAW);
> +   bits = omap_i2c_read_reg(omap, OMAP_I2C_IE_REG);
> +   stat_raw &= bits;
> +
> +   if (stat_raw & OMAP_I2C_STAT_AAS) {
> +   omap_i2c_ack_stat(omap, OMAP_I2C_STAT_AAS);
> +   stat_raw &= ~OMAP_I2C_STAT_AAS;
> +   }
> +
> +out:
> +   return 0;
> +}
> +#endif

This function always return 0.

> +#ifdef CONFIG_I2C_SLAVE
> +static int omap_i2c_reg_slave(struct i2c_client *slave)
> +{
> +   struct omap_i2c_dev *omap = i2c_get_adapdata(slave->adapter);
> +   u16 reg;
> +   int ret = 0;
> +   /* Enable necessary interrupts */
> +   omap_i2c_write_reg(omap, OMAP_I2C_IE_REG, omap->iestate);
> +
> +   return 0;
> +
> +}

Better to return "ret" here as already been initialized to 0

> +
> +static int omap_i2c_unreg_slave(struct i2c_client *slave)
> +{
> +   struct omap_i2c_dev *omap = i2c_get_adapdata(slave->adapter);
> +   u16 reg;
> +
> +   pm_runtime_put(omap->dev);
> +   return 0;
> +}
> +#endif

This function always return 0


Regards
Manish Badarkhe

Re: [PATCH] mfd: lp873x: Add lp873x PMIC support

[Manish Badarkhe]

Hi Keerthy,

> + * struct tps65086 - state holder for the tps65086 driver

need to change name over here in comment.

> + * Device data may be used to access the LP873X chip
> + */
> +struct lp873x {
> +   struct device *dev;
> +   unsigned long id;
> +   u8 rev;
> +   struct mutex lp873_lock;/* lock guarding the data structure */
> +   struct regmap *regmap;
> +};
> +#endif /*  __LINUX_MFD_LP873X_H */
> --

Regards
Manish Badarkhe

Re: [PATCH] mfd: lp873x: Add lp873x PMIC support

[Manish Badarkhe]

Hi Keerthy,

> + * struct tps65086 - state holder for the tps65086 driver

need to change name over here in comment.

> + * Device data may be used to access the LP873X chip
> + */
> +struct lp873x {
> +   struct device *dev;
> +   unsigned long id;
> +   u8 rev;
> +   struct mutex lp873_lock;/* lock guarding the data structure */
> +   struct regmap *regmap;
> +};
> +#endif /*  __LINUX_MFD_LP873X_H */
> --

Regards
Manish Badarkhe

Re: [PATCH v2 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Manish Badarkhe]

Hi Pramod

>>> @@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
>>> struct bam_device *bdev = bchan->bdev;
>>> unsigned long flag;
>>>
>>> +   pm_runtime_get_sync(bdev->dev);
>>> spin_lock_irqsave(>vc.lock, flag);
>>> writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
>>> bchan->paused = 0;
>>> spin_unlock_irqrestore(>vc.lock, flag);
>>> +   pm_runtime_mark_last_busy(bdev->dev);
>>> +   pm_runtime_put_autosuspend(bdev->dev);
>>>
>>> return 0;
>>>  }
>>
>> Why this function simply return 'success' without any error capture?
>>
> I should check return of pm_runtime_get_sync.

Ok, fine.

>>> @@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device 
>>> *pdev)
>>>
>>> tasklet_kill(>task);
>>>
>>> +   pm_runtime_get_sync(>dev);
>>> clk_disable_unprepare(bdev->bamclk);
>>> +   pm_runtime_disable(>dev);
>>> +   pm_runtime_put_noidle(>dev);
>>> +   pm_runtime_set_suspended(>dev);
>>> +
>>> +   return 0;
>>> +}
>>
>> Why this function simply return 'success' without any error capture?
> Need to handle it better.

Yes, need to handle __pm_runtime__ errors.

>>
>>> +static int bam_dma_runtime_suspend(struct device *dev)
>>> +{
>>> +   struct bam_device *bdev = dev_get_drvdata(dev);
>>> +
>>> +   clk_disable(bdev->bamclk);
>>> +
>>> +   return 0;
>>> +}
>>
>> Why this function simply return 'success' without any error capture?
> If probe has succeeded means bdev->bamclk is not NULL and error hence
> clk_disable wont return anything.

got it.

>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int bam_dma_suspend(struct device *dev)
>>> +{
>>> +   struct bam_device *bdev = dev_get_drvdata(dev);
>>> +
>>> +   pm_runtime_force_suspend(dev);
>>> +
>>> +   clk_unprepare(bdev->bamclk);
>>>
>>> return 0;
>>>  }
>>
>> Why this function simply return 'success' without any error capture?
> Same logic applies here as well, as pm_runtime_force_suspend and
> clk_unprepare may not return error in our case once probe is through.

got it.


Regards
Manish Badarkhe

Re: [PATCH v2 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Manish Badarkhe]

Hi Pramod

>>> @@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
>>> struct bam_device *bdev = bchan->bdev;
>>> unsigned long flag;
>>>
>>> +   pm_runtime_get_sync(bdev->dev);
>>> spin_lock_irqsave(>vc.lock, flag);
>>> writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
>>> bchan->paused = 0;
>>> spin_unlock_irqrestore(>vc.lock, flag);
>>> +   pm_runtime_mark_last_busy(bdev->dev);
>>> +   pm_runtime_put_autosuspend(bdev->dev);
>>>
>>> return 0;
>>>  }
>>
>> Why this function simply return 'success' without any error capture?
>>
> I should check return of pm_runtime_get_sync.

Ok, fine.

>>> @@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device 
>>> *pdev)
>>>
>>> tasklet_kill(>task);
>>>
>>> +   pm_runtime_get_sync(>dev);
>>> clk_disable_unprepare(bdev->bamclk);
>>> +   pm_runtime_disable(>dev);
>>> +   pm_runtime_put_noidle(>dev);
>>> +   pm_runtime_set_suspended(>dev);
>>> +
>>> +   return 0;
>>> +}
>>
>> Why this function simply return 'success' without any error capture?
> Need to handle it better.

Yes, need to handle __pm_runtime__ errors.

>>
>>> +static int bam_dma_runtime_suspend(struct device *dev)
>>> +{
>>> +   struct bam_device *bdev = dev_get_drvdata(dev);
>>> +
>>> +   clk_disable(bdev->bamclk);
>>> +
>>> +   return 0;
>>> +}
>>
>> Why this function simply return 'success' without any error capture?
> If probe has succeeded means bdev->bamclk is not NULL and error hence
> clk_disable wont return anything.

got it.

>>> +#ifdef CONFIG_PM_SLEEP
>>> +static int bam_dma_suspend(struct device *dev)
>>> +{
>>> +   struct bam_device *bdev = dev_get_drvdata(dev);
>>> +
>>> +   pm_runtime_force_suspend(dev);
>>> +
>>> +   clk_unprepare(bdev->bamclk);
>>>
>>> return 0;
>>>  }
>>
>> Why this function simply return 'success' without any error capture?
> Same logic applies here as well, as pm_runtime_force_suspend and
> clk_unprepare may not return error in our case once probe is through.

got it.


Regards
Manish Badarkhe

Re: [RESEND PATCH v10 4/4] power: wm831x_power: Support USB charger current limit management

[Manish Badarkhe]

Hi Mark

>> > +static const unsigned int wm831x_usb_limits[] = {
>> > +   0,
>> > +   2,
>> > +   100,
>> > +   500,
>> > +   900,
>> > +   1500,
>> > +   1800,
>> > +   550,
>> > +};
>
>> Just for curiosity, How these current limits are getting decided?
>> Can we have some proper defines over here so that it can be grasped easily?
>
> They're in the silicon, it's just a table of values that were put into
> the silicon at design time.  The defines would just be TABLE_ENTRY_1 or
> whatever.

Thanks for the clarification, In that case, comments/documentation
will work instead of making any defines.

Regards
Manish Badarkhe

Re: [RESEND PATCH v10 4/4] power: wm831x_power: Support USB charger current limit management

[Manish Badarkhe]

Hi Mark

>> > +static const unsigned int wm831x_usb_limits[] = {
>> > +   0,
>> > +   2,
>> > +   100,
>> > +   500,
>> > +   900,
>> > +   1500,
>> > +   1800,
>> > +   550,
>> > +};
>
>> Just for curiosity, How these current limits are getting decided?
>> Can we have some proper defines over here so that it can be grasped easily?
>
> They're in the silicon, it's just a table of values that were put into
> the silicon at design time.  The defines would just be TABLE_ENTRY_1 or
> whatever.

Thanks for the clarification, In that case, comments/documentation
will work instead of making any defines.

Regards
Manish Badarkhe

Re: [PATCH v2 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Manish Badarkhe]

Hi Pramod

> @@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
> struct bam_device *bdev = bchan->bdev;
> unsigned long flag;
>
> +   pm_runtime_get_sync(bdev->dev);
> spin_lock_irqsave(>vc.lock, flag);
> writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
> bchan->paused = 0;
> spin_unlock_irqrestore(>vc.lock, flag);
> +   pm_runtime_mark_last_busy(bdev->dev);
> +   pm_runtime_put_autosuspend(bdev->dev);
>
> return 0;
>  }

Why this function simply return 'success' without any error capture?

> @@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device 
> *pdev)
>
> tasklet_kill(>task);
>
> +   pm_runtime_get_sync(>dev);
> clk_disable_unprepare(bdev->bamclk);
> +   pm_runtime_disable(>dev);
> +   pm_runtime_put_noidle(>dev);
> +   pm_runtime_set_suspended(>dev);
> +
> +   return 0;
> +}

Why this function simply return 'success' without any error capture?

> +static int bam_dma_runtime_suspend(struct device *dev)
> +{
> +   struct bam_device *bdev = dev_get_drvdata(dev);
> +
> +   clk_disable(bdev->bamclk);
> +
> +   return 0;
> +}

Why this function simply return 'success' without any error capture?


> +#ifdef CONFIG_PM_SLEEP
> +static int bam_dma_suspend(struct device *dev)
> +{
> +   struct bam_device *bdev = dev_get_drvdata(dev);
> +
> +   pm_runtime_force_suspend(dev);
> +
> +   clk_unprepare(bdev->bamclk);
>
> return 0;
>  }

Why this function simply return 'success' without any error capture?

Regards
Manish Badarkhe

Re: [PATCH v2 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Manish Badarkhe]

Hi Pramod

> @@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
> struct bam_device *bdev = bchan->bdev;
> unsigned long flag;
>
> +   pm_runtime_get_sync(bdev->dev);
> spin_lock_irqsave(>vc.lock, flag);
> writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
> bchan->paused = 0;
> spin_unlock_irqrestore(>vc.lock, flag);
> +   pm_runtime_mark_last_busy(bdev->dev);
> +   pm_runtime_put_autosuspend(bdev->dev);
>
> return 0;
>  }

Why this function simply return 'success' without any error capture?

> @@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device 
> *pdev)
>
> tasklet_kill(>task);
>
> +   pm_runtime_get_sync(>dev);
> clk_disable_unprepare(bdev->bamclk);
> +   pm_runtime_disable(>dev);
> +   pm_runtime_put_noidle(>dev);
> +   pm_runtime_set_suspended(>dev);
> +
> +   return 0;
> +}

Why this function simply return 'success' without any error capture?

> +static int bam_dma_runtime_suspend(struct device *dev)
> +{
> +   struct bam_device *bdev = dev_get_drvdata(dev);
> +
> +   clk_disable(bdev->bamclk);
> +
> +   return 0;
> +}

Why this function simply return 'success' without any error capture?


> +#ifdef CONFIG_PM_SLEEP
> +static int bam_dma_suspend(struct device *dev)
> +{
> +   struct bam_device *bdev = dev_get_drvdata(dev);
> +
> +   pm_runtime_force_suspend(dev);
> +
> +   clk_unprepare(bdev->bamclk);
>
> return 0;
>  }

Why this function simply return 'success' without any error capture?

Regards
Manish Badarkhe

Re: [RESEND PATCH v10 4/4] power: wm831x_power: Support USB charger current limit management

[Manish Badarkhe]

On Tue, May 3, 2016 at 9:00 AM, Baolin Wang <baolin.w...@linaro.org> wrote:
> Integrate with the newly added USB charger interface to limit the current
> we draw from the USB input based on the input device configuration
> identified by the USB stack, allowing us to charge more quickly from high
> current inputs without drawing more current than specified from others.
>
> Signed-off-by: Mark Brown <broo...@kernel.org>
> Signed-off-by: Baolin Wang <baolin.w...@linaro.org>
> Acked-by: Lee Jones <lee.jo...@linaro.org>
> Acked-by: Charles Keepax <ckee...@opensource.wolfsonmicro.com>
> Acked-by: Peter Chen <peter.c...@freescale.com>
> Acked-by: Sebastian Reichel <s...@kernel.org>
> ---
>  drivers/power/wm831x_power.c |   69 
> ++
>  include/linux/mfd/wm831x/pdata.h |3 ++
>  2 files changed, 72 insertions(+)
>
> diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
> index 7082301..cef1812 100644
> --- a/drivers/power/wm831x_power.c
> +++ b/drivers/power/wm831x_power.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -31,6 +32,8 @@ struct wm831x_power {
> char usb_name[20];
> char battery_name[20];
> bool have_battery;
> +   struct usb_charger *usb_charger;
> +   struct notifier_block usb_notify;
>  };
>
>  static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
> @@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>  };
>
> +/* In milliamps */
> +static const unsigned int wm831x_usb_limits[] = {
> +   0,
> +   2,
> +   100,
> +   500,
> +   900,
> +   1500,
> +   1800,
> +   550,
> +};

Just for curiosity, How these current limits are getting decided?
Can we have some proper defines over here so that it can be grasped easily?

Thanks
Manish Badarkhe

Re: [RESEND PATCH v10 4/4] power: wm831x_power: Support USB charger current limit management

[Manish Badarkhe]

On Tue, May 3, 2016 at 9:00 AM, Baolin Wang  wrote:
> Integrate with the newly added USB charger interface to limit the current
> we draw from the USB input based on the input device configuration
> identified by the USB stack, allowing us to charge more quickly from high
> current inputs without drawing more current than specified from others.
>
> Signed-off-by: Mark Brown 
> Signed-off-by: Baolin Wang 
> Acked-by: Lee Jones 
> Acked-by: Charles Keepax 
> Acked-by: Peter Chen 
> Acked-by: Sebastian Reichel 
> ---
>  drivers/power/wm831x_power.c |   69 
> ++
>  include/linux/mfd/wm831x/pdata.h |3 ++
>  2 files changed, 72 insertions(+)
>
> diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
> index 7082301..cef1812 100644
> --- a/drivers/power/wm831x_power.c
> +++ b/drivers/power/wm831x_power.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -31,6 +32,8 @@ struct wm831x_power {
> char usb_name[20];
> char battery_name[20];
> bool have_battery;
> +   struct usb_charger *usb_charger;
> +   struct notifier_block usb_notify;
>  };
>
>  static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
> @@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
>  };
>
> +/* In milliamps */
> +static const unsigned int wm831x_usb_limits[] = {
> +   0,
> +   2,
> +   100,
> +   500,
> +   900,
> +   1500,
> +   1800,
> +   550,
> +};

Just for curiosity, How these current limits are getting decided?
Can we have some proper defines over here so that it can be grasped easily?

Thanks
Manish Badarkhe

Re: [PATCH 2/4] thermal: rcar: enable to use thermal-zone on DT

[Manish Badarkhe]

Hi


> +static int rcar_thermal_of_get_temp(void *data, int *temp)
> +{
> +   struct rcar_thermal_priv *priv = data;
> +
> +   *temp = rcar_thermal_get_current_temp(priv);
> +
> +   return 0;
> +}
> +
> +static int rcar_thermal_get_temp(struct thermal_zone_device *zone, int *temp)
> +{
> +   struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
> +
> +   *temp = rcar_thermal_get_current_temp(priv);
> +
> return 0;
>  }
>

Above two function, always returns 0. Can it possible to handle error
and log some messages.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/4] thermal: rcar: enable to use thermal-zone on DT

[Manish Badarkhe]

Hi


> +static int rcar_thermal_of_get_temp(void *data, int *temp)
> +{
> +   struct rcar_thermal_priv *priv = data;
> +
> +   *temp = rcar_thermal_get_current_temp(priv);
> +
> +   return 0;
> +}
> +
> +static int rcar_thermal_get_temp(struct thermal_zone_device *zone, int *temp)
> +{
> +   struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone);
> +
> +   *temp = rcar_thermal_get_current_temp(priv);
> +
> return 0;
>  }
>

Above two function, always returns 0. Can it possible to handle error
and log some messages.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 8/9] ARM: EXYNOS: rearrange static and non-static functions of PMU driver

[Manish Badarkhe]

On Tue, Nov 17, 2015 at 11:35 AM, Pankaj Dubey  wrote:
> This patch moves exynos_sys_powerdown_conf function above all
> static functions, to avoid confusion causing due to mixing of
> static-nonstatic-static functions and to improve readability of this
> driver.
>
> Signed-off-by: Pankaj Dubey 
> Suggested-by: Krzysztof Kozlowski 
> Reviewed-by: Krzysztof Kozlowski 
> ---
>  arch/arm/mach-exynos/pmu.c | 34 +-
>  1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
> index 01cb649..a7741d4 100644
> --- a/arch/arm/mach-exynos/pmu.c
> +++ b/arch/arm/mach-exynos/pmu.c
> @@ -39,23 +39,6 @@ u32 pmu_raw_readl(u32 offset)
> return readl_relaxed(pmu_base_addr + offset);
>  }
>
> -static void exynos_power_off(void)
> -{
> -   unsigned int tmp;
> -
> -   pr_info("Power down.\n");
> -   tmp = pmu_raw_readl(EXYNOS_PS_HOLD_CONTROL);
> -   tmp ^= (1 << 8);
> -   pmu_raw_writel(tmp, EXYNOS_PS_HOLD_CONTROL);
> -
> -   /* Wait a little so we don't give a false warning below */
> -   mdelay(100);
> -
> -   pr_err("Power down failed, please power off system manually.\n");
> -   while (1)
> -   ;
> -}
> -
>  void exynos_sys_powerdown_conf(enum sys_powerdown mode)
>  {
> unsigned int i;
> @@ -85,6 +68,23 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode)
> }
>  }
>
> +static void exynos_power_off(void)
> +{
> +   unsigned int tmp;
> +
> +   pr_info("Power down.\n");
> +   tmp = pmu_raw_readl(EXYNOS_PS_HOLD_CONTROL);
> +   tmp ^= (1 << 8);
Can we have some define over here? to operate this bit.

> +   pmu_raw_writel(tmp, EXYNOS_PS_HOLD_CONTROL);
> +
> +   /* Wait a little so we don't give a false warning below */
> +   mdelay(100);
> +
> +   pr_err("Power down failed, please power off system manually.\n");
> +   while (1)
> +   ;
> +}
> +
>  static int pmu_restart_notify(struct notifier_block *this,
> unsigned long code, void *unused)
>  {
> --
> 2.4.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v6 8/9] ARM: EXYNOS: rearrange static and non-static functions of PMU driver

[Manish Badarkhe]

On Tue, Nov 17, 2015 at 11:35 AM, Pankaj Dubey  wrote:
> This patch moves exynos_sys_powerdown_conf function above all
> static functions, to avoid confusion causing due to mixing of
> static-nonstatic-static functions and to improve readability of this
> driver.
>
> Signed-off-by: Pankaj Dubey 
> Suggested-by: Krzysztof Kozlowski 
> Reviewed-by: Krzysztof Kozlowski 
> ---
>  arch/arm/mach-exynos/pmu.c | 34 +-
>  1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
> index 01cb649..a7741d4 100644
> --- a/arch/arm/mach-exynos/pmu.c
> +++ b/arch/arm/mach-exynos/pmu.c
> @@ -39,23 +39,6 @@ u32 pmu_raw_readl(u32 offset)
> return readl_relaxed(pmu_base_addr + offset);
>  }
>
> -static void exynos_power_off(void)
> -{
> -   unsigned int tmp;
> -
> -   pr_info("Power down.\n");
> -   tmp = pmu_raw_readl(EXYNOS_PS_HOLD_CONTROL);
> -   tmp ^= (1 << 8);
> -   pmu_raw_writel(tmp, EXYNOS_PS_HOLD_CONTROL);
> -
> -   /* Wait a little so we don't give a false warning below */
> -   mdelay(100);
> -
> -   pr_err("Power down failed, please power off system manually.\n");
> -   while (1)
> -   ;
> -}
> -
>  void exynos_sys_powerdown_conf(enum sys_powerdown mode)
>  {
> unsigned int i;
> @@ -85,6 +68,23 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode)
> }
>  }
>
> +static void exynos_power_off(void)
> +{
> +   unsigned int tmp;
> +
> +   pr_info("Power down.\n");
> +   tmp = pmu_raw_readl(EXYNOS_PS_HOLD_CONTROL);
> +   tmp ^= (1 << 8);
Can we have some define over here? to operate this bit.

> +   pmu_raw_writel(tmp, EXYNOS_PS_HOLD_CONTROL);
> +
> +   /* Wait a little so we don't give a false warning below */
> +   mdelay(100);
> +
> +   pr_err("Power down failed, please power off system manually.\n");
> +   while (1)
> +   ;
> +}
> +
>  static int pmu_restart_notify(struct notifier_block *this,
> unsigned long code, void *unused)
>  {
> --
> 2.4.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] regulator: tps65218: Remove '.owner' field

[Manish Badarkhe]

Hi Mark,

On Sun, Nov 9, 2014 at 2:49 PM, Mark Brown  wrote:
> On Sat, Nov 08, 2014 at 05:42:52PM +0530, Manish Badarkhe wrote:
>> Remove '.owner' field of driver structure as it is
>> going to be filled via "module_platform_driver" call.
>
> Same question as ever with this stuff: if we've decided to do this why
> are we doing it for a single driver and not as a genric cleanup?

There are many drivers needs to be changed with such fix.
Can we fix drivers with this fix module by module or all at once?
Please let me know your opinion.

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] regulator: tps65218: Remove '.owner' field

[Manish Badarkhe]

Hi Mark,

On Sun, Nov 9, 2014 at 2:49 PM, Mark Brown broo...@kernel.org wrote:
 On Sat, Nov 08, 2014 at 05:42:52PM +0530, Manish Badarkhe wrote:
 Remove '.owner' field of driver structure as it is
 going to be filled via module_platform_driver call.

 Same question as ever with this stuff: if we've decided to do this why
 are we doing it for a single driver and not as a genric cleanup?

There are many drivers needs to be changed with such fix.
Can we fix drivers with this fix module by module or all at once?
Please let me know your opinion.

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] regulator: tps65218: Remove '.owner' field

[Manish Badarkhe]

Remove '.owner' field of driver structure as it is
going to be filled via "module_platform_driver" call.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 f0a4028... e2b97ac... M  drivers/regulator/tps65218-regulator.c
 drivers/regulator/tps65218-regulator.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/regulator/tps65218-regulator.c 
b/drivers/regulator/tps65218-regulator.c
index f0a4028..e2b97ac 100644
--- a/drivers/regulator/tps65218-regulator.c
+++ b/drivers/regulator/tps65218-regulator.c
@@ -255,7 +255,6 @@ static int tps65218_regulator_probe(struct platform_device 
*pdev)
 static struct platform_driver tps65218_regulator_driver = {
.driver = {
.name = "tps65218-pmic",
-   .owner = THIS_MODULE,
.of_match_table = tps65218_of_match,
},
.probe = tps65218_regulator_probe,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] regulator: tps65218: Remove '.owner' field

[Manish Badarkhe]

Remove '.owner' field of driver structure as it is
going to be filled via module_platform_driver call.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 f0a4028... e2b97ac... M  drivers/regulator/tps65218-regulator.c
 drivers/regulator/tps65218-regulator.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/regulator/tps65218-regulator.c 
b/drivers/regulator/tps65218-regulator.c
index f0a4028..e2b97ac 100644
--- a/drivers/regulator/tps65218-regulator.c
+++ b/drivers/regulator/tps65218-regulator.c
@@ -255,7 +255,6 @@ static int tps65218_regulator_probe(struct platform_device 
*pdev)
 static struct platform_driver tps65218_regulator_driver = {
.driver = {
.name = tps65218-pmic,
-   .owner = THIS_MODULE,
.of_match_table = tps65218_of_match,
},
.probe = tps65218_regulator_probe,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V4] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Hi Dmitry,

On Tue, Feb 11, 2014 at 10:15 PM, Manish Badarkhe
 wrote:
> Hi Dmitry,
>
> On Tue, Jan 28, 2014 at 10:24 PM, Manish Badarkhe
>  wrote:
>> Instead of "#ifdef CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
>> option for DT code to avoid if-deffery in code.
>>
>> Signed-off-by: Manish Badarkhe 
>> ---
>> Changes since V1:
>> 1.Updated code to use "IS_ENABLED" during retrieval
>>   of platform/DT data.
>>
>> Changes since V2:
>> 1.Updated code as per Dmitry's comment to give preferance to
>>   platform data.
>>
>> Changes since V3:
>> 1.Updated code to use "IS_ENABLED(CONFIG_OF)" to remove
>>   DT data retrieval function during compilation itself
>>   when CONFIG_OF option is not set.
>>
>> :100644 100644 b4513f2... a7482db... M  drivers/power/max8925_power.c
>>  drivers/power/max8925_power.c |   17 +
>>  1 file changed, 5 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
>> index b4513f2..a7482db 100644
>> --- a/drivers/power/max8925_power.c
>> +++ b/drivers/power/max8925_power.c
>> @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct 
>> max8925_power_info *info)
>> return 0;
>>  }
>>
>> -#ifdef CONFIG_OF
>>  static struct max8925_power_pdata *
>>  max8925_power_dt_init(struct platform_device *pdev)
>>  {
>> @@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
>> int no_insert_detect;
>> struct max8925_power_pdata *pdata;
>>
>> -   if (!nproot)
>> -   return pdev->dev.platform_data;
>> -
>> np = of_find_node_by_name(nproot, "charger");
>> if (!np) {
>> dev_err(>dev, "failed to find charger node\n");
>> @@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)
>>
>> return pdata;
>>  }
>> -#else
>> -static struct max8925_power_pdata *
>> -max8925_power_dt_init(struct platform_device *pdev)
>> -{
>> -   return pdev->dev.platform_data;
>> -}
>> -#endif
>>
>>  static int max8925_power_probe(struct platform_device *pdev)
>>  {
>> @@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device 
>> *pdev)
>> struct max8925_power_info *info;
>> int ret;
>>
>> -   pdata = max8925_power_dt_init(pdev);
>> +   pdata = dev_get_platdata(>dev);
>> +
>> +   if (IS_ENABLED(CONFIG_OF) && !pdata && chip->dev->of_node)
>> +   pdata = max8925_power_dt_init(pdev);
>> +
>> if (!pdata) {
>> dev_err(>dev, "platform data isn't assigned to "
>> "power supply\n");
>
> Gentle ping!!
> Please let me know, are there any review comments on this patch?

Gentle ping!!

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V4] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Hi Dmitry,

On Tue, Feb 11, 2014 at 10:15 PM, Manish Badarkhe
badarkhe.man...@gmail.com wrote:
 Hi Dmitry,

 On Tue, Jan 28, 2014 at 10:24 PM, Manish Badarkhe
 badarkhe.man...@gmail.com wrote:
 Instead of #ifdef CONFIG_OF use IS_ENABLED(CONFIG_OF)
 option for DT code to avoid if-deffery in code.

 Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
 ---
 Changes since V1:
 1.Updated code to use IS_ENABLED during retrieval
   of platform/DT data.

 Changes since V2:
 1.Updated code as per Dmitry's comment to give preferance to
   platform data.

 Changes since V3:
 1.Updated code to use IS_ENABLED(CONFIG_OF) to remove
   DT data retrieval function during compilation itself
   when CONFIG_OF option is not set.

 :100644 100644 b4513f2... a7482db... M  drivers/power/max8925_power.c
  drivers/power/max8925_power.c |   17 +
  1 file changed, 5 insertions(+), 12 deletions(-)

 diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
 index b4513f2..a7482db 100644
 --- a/drivers/power/max8925_power.c
 +++ b/drivers/power/max8925_power.c
 @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct 
 max8925_power_info *info)
 return 0;
  }

 -#ifdef CONFIG_OF
  static struct max8925_power_pdata *
  max8925_power_dt_init(struct platform_device *pdev)
  {
 @@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 int no_insert_detect;
 struct max8925_power_pdata *pdata;

 -   if (!nproot)
 -   return pdev-dev.platform_data;
 -
 np = of_find_node_by_name(nproot, charger);
 if (!np) {
 dev_err(pdev-dev, failed to find charger node\n);
 @@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)

 return pdata;
  }
 -#else
 -static struct max8925_power_pdata *
 -max8925_power_dt_init(struct platform_device *pdev)
 -{
 -   return pdev-dev.platform_data;
 -}
 -#endif

  static int max8925_power_probe(struct platform_device *pdev)
  {
 @@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device 
 *pdev)
 struct max8925_power_info *info;
 int ret;

 -   pdata = max8925_power_dt_init(pdev);
 +   pdata = dev_get_platdata(pdev-dev);
 +
 +   if (IS_ENABLED(CONFIG_OF)  !pdata  chip-dev-of_node)
 +   pdata = max8925_power_dt_init(pdev);
 +
 if (!pdata) {
 dev_err(pdev-dev, platform data isn't assigned to 
 power supply\n);

 Gentle ping!!
 Please let me know, are there any review comments on this patch?

Gentle ping!!

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] video: da8xx-fb: Use "SIMPLE_DEV_PM_OPS" macro

[Manish Badarkhe]

Update driver to use SIMPLE_DEV_PM_OPS macro for power
management suspend and resume operations.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 a1d74dd... 0c0ba92... M  drivers/video/da8xx-fb.c
 drivers/video/da8xx-fb.c |   22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index a1d74dd..0c0ba92 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1546,7 +1546,7 @@ err_pm_runtime_disable:
return ret;
 }
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static struct lcdc_context {
u32 clk_enable;
u32 ctrl;
@@ -1610,9 +1610,9 @@ static void lcd_context_restore(void)
return;
 }
 
-static int fb_suspend(struct platform_device *dev, pm_message_t state)
+static int fb_suspend(struct device *dev)
 {
-   struct fb_info *info = platform_get_drvdata(dev);
+   struct fb_info *info = dev_get_drvdata(dev);
struct da8xx_fb_par *par = info->par;
 
console_lock();
@@ -1622,18 +1622,18 @@ static int fb_suspend(struct platform_device *dev, 
pm_message_t state)
fb_set_suspend(info, 1);
lcd_disable_raster(DA8XX_FRAME_WAIT);
lcd_context_save();
-   pm_runtime_put_sync(>dev);
+   pm_runtime_put_sync(dev);
console_unlock();
 
return 0;
 }
-static int fb_resume(struct platform_device *dev)
+static int fb_resume(struct device *dev)
 {
-   struct fb_info *info = platform_get_drvdata(dev);
+   struct fb_info *info = dev_get_drvdata(dev);
struct da8xx_fb_par *par = info->par;
 
console_lock();
-   pm_runtime_get_sync(>dev);
+   pm_runtime_get_sync(dev);
lcd_context_restore();
if (par->blank == FB_BLANK_UNBLANK) {
lcd_enable_raster();
@@ -1647,19 +1647,17 @@ static int fb_resume(struct platform_device *dev)
 
return 0;
 }
-#else
-#define fb_suspend NULL
-#define fb_resume NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(fb_pm_ops, fb_suspend, fb_resume);
+
 static struct platform_driver da8xx_fb_driver = {
.probe = fb_probe,
.remove = fb_remove,
-   .suspend = fb_suspend,
-   .resume = fb_resume,
.driver = {
   .name = DRIVER_NAME,
   .owner = THIS_MODULE,
+  .pm  = _pm_ops,
   },
 };
 module_platform_driver(da8xx_fb_driver);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] video: da8xx-fb: Use SIMPLE_DEV_PM_OPS macro

[Manish Badarkhe]

Update driver to use SIMPLE_DEV_PM_OPS macro for power
management suspend and resume operations.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 a1d74dd... 0c0ba92... M  drivers/video/da8xx-fb.c
 drivers/video/da8xx-fb.c |   22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index a1d74dd..0c0ba92 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1546,7 +1546,7 @@ err_pm_runtime_disable:
return ret;
 }
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static struct lcdc_context {
u32 clk_enable;
u32 ctrl;
@@ -1610,9 +1610,9 @@ static void lcd_context_restore(void)
return;
 }
 
-static int fb_suspend(struct platform_device *dev, pm_message_t state)
+static int fb_suspend(struct device *dev)
 {
-   struct fb_info *info = platform_get_drvdata(dev);
+   struct fb_info *info = dev_get_drvdata(dev);
struct da8xx_fb_par *par = info-par;
 
console_lock();
@@ -1622,18 +1622,18 @@ static int fb_suspend(struct platform_device *dev, 
pm_message_t state)
fb_set_suspend(info, 1);
lcd_disable_raster(DA8XX_FRAME_WAIT);
lcd_context_save();
-   pm_runtime_put_sync(dev-dev);
+   pm_runtime_put_sync(dev);
console_unlock();
 
return 0;
 }
-static int fb_resume(struct platform_device *dev)
+static int fb_resume(struct device *dev)
 {
-   struct fb_info *info = platform_get_drvdata(dev);
+   struct fb_info *info = dev_get_drvdata(dev);
struct da8xx_fb_par *par = info-par;
 
console_lock();
-   pm_runtime_get_sync(dev-dev);
+   pm_runtime_get_sync(dev);
lcd_context_restore();
if (par-blank == FB_BLANK_UNBLANK) {
lcd_enable_raster();
@@ -1647,19 +1647,17 @@ static int fb_resume(struct platform_device *dev)
 
return 0;
 }
-#else
-#define fb_suspend NULL
-#define fb_resume NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(fb_pm_ops, fb_suspend, fb_resume);
+
 static struct platform_driver da8xx_fb_driver = {
.probe = fb_probe,
.remove = fb_remove,
-   .suspend = fb_suspend,
-   .resume = fb_resume,
.driver = {
   .name = DRIVER_NAME,
   .owner = THIS_MODULE,
+  .pm  = fb_pm_ops,
   },
 };
 module_platform_driver(da8xx_fb_driver);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V4] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Hi Dmitry,

On Tue, Jan 28, 2014 at 10:24 PM, Manish Badarkhe
 wrote:
> Instead of "#ifdef CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
> option for DT code to avoid if-deffery in code.
>
> Signed-off-by: Manish Badarkhe 
> ---
> Changes since V1:
> 1.Updated code to use "IS_ENABLED" during retrieval
>   of platform/DT data.
>
> Changes since V2:
> 1.Updated code as per Dmitry's comment to give preferance to
>   platform data.
>
> Changes since V3:
> 1.Updated code to use "IS_ENABLED(CONFIG_OF)" to remove
>   DT data retrieval function during compilation itself
>   when CONFIG_OF option is not set.
>
> :100644 100644 b4513f2... a7482db... M  drivers/power/max8925_power.c
>  drivers/power/max8925_power.c |   17 +
>  1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
> index b4513f2..a7482db 100644
> --- a/drivers/power/max8925_power.c
> +++ b/drivers/power/max8925_power.c
> @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct 
> max8925_power_info *info)
> return 0;
>  }
>
> -#ifdef CONFIG_OF
>  static struct max8925_power_pdata *
>  max8925_power_dt_init(struct platform_device *pdev)
>  {
> @@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
> int no_insert_detect;
> struct max8925_power_pdata *pdata;
>
> -   if (!nproot)
> -   return pdev->dev.platform_data;
> -
> np = of_find_node_by_name(nproot, "charger");
> if (!np) {
> dev_err(>dev, "failed to find charger node\n");
> @@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)
>
> return pdata;
>  }
> -#else
> -static struct max8925_power_pdata *
> -max8925_power_dt_init(struct platform_device *pdev)
> -{
> -   return pdev->dev.platform_data;
> -}
> -#endif
>
>  static int max8925_power_probe(struct platform_device *pdev)
>  {
> @@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device 
> *pdev)
> struct max8925_power_info *info;
> int ret;
>
> -   pdata = max8925_power_dt_init(pdev);
> +   pdata = dev_get_platdata(>dev);
> +
> +   if (IS_ENABLED(CONFIG_OF) && !pdata && chip->dev->of_node)
> +   pdata = max8925_power_dt_init(pdev);
> +
> if (!pdata) {
> dev_err(>dev, "platform data isn't assigned to "
> "power supply\n");

Gentle ping!!
Please let me know, are there any review comments on this patch?

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V4] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Hi Dmitry,

On Tue, Jan 28, 2014 at 10:24 PM, Manish Badarkhe
badarkhe.man...@gmail.com wrote:
 Instead of #ifdef CONFIG_OF use IS_ENABLED(CONFIG_OF)
 option for DT code to avoid if-deffery in code.

 Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
 ---
 Changes since V1:
 1.Updated code to use IS_ENABLED during retrieval
   of platform/DT data.

 Changes since V2:
 1.Updated code as per Dmitry's comment to give preferance to
   platform data.

 Changes since V3:
 1.Updated code to use IS_ENABLED(CONFIG_OF) to remove
   DT data retrieval function during compilation itself
   when CONFIG_OF option is not set.

 :100644 100644 b4513f2... a7482db... M  drivers/power/max8925_power.c
  drivers/power/max8925_power.c |   17 +
  1 file changed, 5 insertions(+), 12 deletions(-)

 diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
 index b4513f2..a7482db 100644
 --- a/drivers/power/max8925_power.c
 +++ b/drivers/power/max8925_power.c
 @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct 
 max8925_power_info *info)
 return 0;
  }

 -#ifdef CONFIG_OF
  static struct max8925_power_pdata *
  max8925_power_dt_init(struct platform_device *pdev)
  {
 @@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 int no_insert_detect;
 struct max8925_power_pdata *pdata;

 -   if (!nproot)
 -   return pdev-dev.platform_data;
 -
 np = of_find_node_by_name(nproot, charger);
 if (!np) {
 dev_err(pdev-dev, failed to find charger node\n);
 @@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)

 return pdata;
  }
 -#else
 -static struct max8925_power_pdata *
 -max8925_power_dt_init(struct platform_device *pdev)
 -{
 -   return pdev-dev.platform_data;
 -}
 -#endif

  static int max8925_power_probe(struct platform_device *pdev)
  {
 @@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device 
 *pdev)
 struct max8925_power_info *info;
 int ret;

 -   pdata = max8925_power_dt_init(pdev);
 +   pdata = dev_get_platdata(pdev-dev);
 +
 +   if (IS_ENABLED(CONFIG_OF)  !pdata  chip-dev-of_node)
 +   pdata = max8925_power_dt_init(pdev);
 +
 if (!pdata) {
 dev_err(pdev-dev, platform data isn't assigned to 
 power supply\n);

Gentle ping!!
Please let me know, are there any review comments on this patch?

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2] regulator: tps6507x: Use "IS_ENABLED" for DT code.

[Manish Badarkhe]

Instead of "#ifdef CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
option for DT code to avoid if-deffery in code.
Also, modify code as per coding style.

Signed-off-by: Manish Badarkhe 
---
Changes since V1:
1. fix build break when CONFIG_OF is not set.

:100644 100644 162a0fa... 862cc81... M  drivers/regulator/tps6507x-regulator.c
 drivers/regulator/tps6507x-regulator.c |   18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/regulator/tps6507x-regulator.c 
b/drivers/regulator/tps6507x-regulator.c
index 162a0fa..862cc81 100644
--- a/drivers/regulator/tps6507x-regulator.c
+++ b/drivers/regulator/tps6507x-regulator.c
@@ -359,7 +359,6 @@ static struct regulator_ops tps6507x_pmic_ops = {
.map_voltage = regulator_map_voltage_ascend,
 };
 
-#ifdef CONFIG_OF
 static struct of_regulator_match tps6507x_matches[] = {
{ .name = "VDCDC1"},
{ .name = "VDCDC2"},
@@ -424,15 +423,7 @@ static struct tps6507x_board *tps6507x_parse_dt_reg_data(
 
return tps_board;
 }
-#else
-static inline struct tps6507x_board *tps6507x_parse_dt_reg_data(
-   struct platform_device *pdev,
-   struct of_regulator_match **tps6507x_reg_matches)
-{
-   *tps6507x_reg_matches = NULL;
-   return NULL;
-}
-#endif
+
 static int tps6507x_pmic_probe(struct platform_device *pdev)
 {
struct tps6507x_dev *tps6507x_dev = dev_get_drvdata(pdev->dev.parent);
@@ -453,9 +444,10 @@ static int tps6507x_pmic_probe(struct platform_device 
*pdev)
 */
 
tps_board = dev_get_platdata(tps6507x_dev->dev);
-   if (!tps_board && tps6507x_dev->dev->of_node)
+   if (IS_ENABLED(CONFIG_OF) && !tps_board &&
+   tps6507x_dev->dev->of_node)
tps_board = tps6507x_parse_dt_reg_data(pdev,
-   _reg_matches);
+   _reg_matches);
if (!tps_board)
return -EINVAL;
 
@@ -481,7 +473,7 @@ static int tps6507x_pmic_probe(struct platform_device *pdev)
tps->info[i] = info;
if (init_data->driver_data) {
struct tps6507x_reg_platform_data *data =
-   init_data->driver_data;
+   init_data->driver_data;
tps->info[i]->defdcdc_default = data->defdcdc_default;
}
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] regulator: tps6507x: Use "IS_ENABLED" for DT code.

[Manish Badarkhe]

Instead of "#ifdef CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
option for DT code to avoid if-deffery in code.
Also, modify code as per coding style.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 162a0fa... 97868b7... M  drivers/regulator/tps6507x-regulator.c
 drivers/regulator/tps6507x-regulator.c |   18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/regulator/tps6507x-regulator.c 
b/drivers/regulator/tps6507x-regulator.c
index 162a0fa..97868b7 100644
--- a/drivers/regulator/tps6507x-regulator.c
+++ b/drivers/regulator/tps6507x-regulator.c
@@ -367,6 +367,7 @@ static struct of_regulator_match tps6507x_matches[] = {
{ .name = "LDO1"},
{ .name = "LDO2"},
 };
+#endif
 
 static struct tps6507x_board *tps6507x_parse_dt_reg_data(
struct platform_device *pdev,
@@ -424,15 +425,7 @@ static struct tps6507x_board *tps6507x_parse_dt_reg_data(
 
return tps_board;
 }
-#else
-static inline struct tps6507x_board *tps6507x_parse_dt_reg_data(
-   struct platform_device *pdev,
-   struct of_regulator_match **tps6507x_reg_matches)
-{
-   *tps6507x_reg_matches = NULL;
-   return NULL;
-}
-#endif
+
 static int tps6507x_pmic_probe(struct platform_device *pdev)
 {
struct tps6507x_dev *tps6507x_dev = dev_get_drvdata(pdev->dev.parent);
@@ -453,9 +446,10 @@ static int tps6507x_pmic_probe(struct platform_device 
*pdev)
 */
 
tps_board = dev_get_platdata(tps6507x_dev->dev);
-   if (!tps_board && tps6507x_dev->dev->of_node)
+   if (IS_ENABLED(CONFIG_OF) && !tps_board &&
+   tps6507x_dev->dev->of_node)
tps_board = tps6507x_parse_dt_reg_data(pdev,
-   _reg_matches);
+   _reg_matches);
if (!tps_board)
return -EINVAL;
 
@@ -481,7 +475,7 @@ static int tps6507x_pmic_probe(struct platform_device *pdev)
tps->info[i] = info;
if (init_data->driver_data) {
struct tps6507x_reg_platform_data *data =
-   init_data->driver_data;
+   init_data->driver_data;
tps->info[i]->defdcdc_default = data->defdcdc_default;
}
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] regulator: tps6507x: Use IS_ENABLED for DT code.

[Manish Badarkhe]

Instead of #ifdef CONFIG_OF use IS_ENABLED(CONFIG_OF)
option for DT code to avoid if-deffery in code.
Also, modify code as per coding style.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 162a0fa... 97868b7... M  drivers/regulator/tps6507x-regulator.c
 drivers/regulator/tps6507x-regulator.c |   18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/regulator/tps6507x-regulator.c 
b/drivers/regulator/tps6507x-regulator.c
index 162a0fa..97868b7 100644
--- a/drivers/regulator/tps6507x-regulator.c
+++ b/drivers/regulator/tps6507x-regulator.c
@@ -367,6 +367,7 @@ static struct of_regulator_match tps6507x_matches[] = {
{ .name = LDO1},
{ .name = LDO2},
 };
+#endif
 
 static struct tps6507x_board *tps6507x_parse_dt_reg_data(
struct platform_device *pdev,
@@ -424,15 +425,7 @@ static struct tps6507x_board *tps6507x_parse_dt_reg_data(
 
return tps_board;
 }
-#else
-static inline struct tps6507x_board *tps6507x_parse_dt_reg_data(
-   struct platform_device *pdev,
-   struct of_regulator_match **tps6507x_reg_matches)
-{
-   *tps6507x_reg_matches = NULL;
-   return NULL;
-}
-#endif
+
 static int tps6507x_pmic_probe(struct platform_device *pdev)
 {
struct tps6507x_dev *tps6507x_dev = dev_get_drvdata(pdev-dev.parent);
@@ -453,9 +446,10 @@ static int tps6507x_pmic_probe(struct platform_device 
*pdev)
 */
 
tps_board = dev_get_platdata(tps6507x_dev-dev);
-   if (!tps_board  tps6507x_dev-dev-of_node)
+   if (IS_ENABLED(CONFIG_OF)  !tps_board 
+   tps6507x_dev-dev-of_node)
tps_board = tps6507x_parse_dt_reg_data(pdev,
-   tps6507x_reg_matches);
+   tps6507x_reg_matches);
if (!tps_board)
return -EINVAL;
 
@@ -481,7 +475,7 @@ static int tps6507x_pmic_probe(struct platform_device *pdev)
tps-info[i] = info;
if (init_data-driver_data) {
struct tps6507x_reg_platform_data *data =
-   init_data-driver_data;
+   init_data-driver_data;
tps-info[i]-defdcdc_default = data-defdcdc_default;
}
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2] regulator: tps6507x: Use IS_ENABLED for DT code.

[Manish Badarkhe]

Instead of #ifdef CONFIG_OF use IS_ENABLED(CONFIG_OF)
option for DT code to avoid if-deffery in code.
Also, modify code as per coding style.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
Changes since V1:
1. fix build break when CONFIG_OF is not set.

:100644 100644 162a0fa... 862cc81... M  drivers/regulator/tps6507x-regulator.c
 drivers/regulator/tps6507x-regulator.c |   18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/regulator/tps6507x-regulator.c 
b/drivers/regulator/tps6507x-regulator.c
index 162a0fa..862cc81 100644
--- a/drivers/regulator/tps6507x-regulator.c
+++ b/drivers/regulator/tps6507x-regulator.c
@@ -359,7 +359,6 @@ static struct regulator_ops tps6507x_pmic_ops = {
.map_voltage = regulator_map_voltage_ascend,
 };
 
-#ifdef CONFIG_OF
 static struct of_regulator_match tps6507x_matches[] = {
{ .name = VDCDC1},
{ .name = VDCDC2},
@@ -424,15 +423,7 @@ static struct tps6507x_board *tps6507x_parse_dt_reg_data(
 
return tps_board;
 }
-#else
-static inline struct tps6507x_board *tps6507x_parse_dt_reg_data(
-   struct platform_device *pdev,
-   struct of_regulator_match **tps6507x_reg_matches)
-{
-   *tps6507x_reg_matches = NULL;
-   return NULL;
-}
-#endif
+
 static int tps6507x_pmic_probe(struct platform_device *pdev)
 {
struct tps6507x_dev *tps6507x_dev = dev_get_drvdata(pdev-dev.parent);
@@ -453,9 +444,10 @@ static int tps6507x_pmic_probe(struct platform_device 
*pdev)
 */
 
tps_board = dev_get_platdata(tps6507x_dev-dev);
-   if (!tps_board  tps6507x_dev-dev-of_node)
+   if (IS_ENABLED(CONFIG_OF)  !tps_board 
+   tps6507x_dev-dev-of_node)
tps_board = tps6507x_parse_dt_reg_data(pdev,
-   tps6507x_reg_matches);
+   tps6507x_reg_matches);
if (!tps_board)
return -EINVAL;
 
@@ -481,7 +473,7 @@ static int tps6507x_pmic_probe(struct platform_device *pdev)
tps-info[i] = info;
if (init_data-driver_data) {
struct tps6507x_reg_platform_data *data =
-   init_data-driver_data;
+   init_data-driver_data;
tps-info[i]-defdcdc_default = data-defdcdc_default;
}
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V4 0/2] devm_* API operation for fixed regulator

[Manish Badarkhe]

Hi Mark,



On Wed, Jan 29, 2014 at 8:27 PM, Manish Badarkhe
 wrote:
> Use devm_* API operations for fixed regulator driver so that
> driver core will manage resources.
>
> Also, introduce a new API "devm_kstrdup" and used it in fixed
> regulator driver to manage resources.
>
> Changes since V3:
>  1. Update "devm_kstrdup" function to remove "size" argument.
> Also,used "devm_kmalloc" instead of "devm_kzalloc".
>
> Changes since V2:
>  1. Update driver to use new API "devm_kstrdup" instead of
> "kstrdup".
>  2. Added a seperate patch to introduce new API "devm_kstrdup"
>
> Changes since V1:
>  1. Updated driver to use "devm_kzalloc" instead of "kstrdup".
>  2. Updated commit message.
>
> Manish Badarkhe (2):
>   devres: introduce API "devm_kstrdup"
>   regulator: fixed: update to devm_* API
>
>  drivers/base/devres.c |   26 ++
>  drivers/regulator/fixed.c |   42 ++++--
>  include/linux/device.h|1 +
>  3 files changed, 39 insertions(+), 30 deletions(-)

Are there any review comments on this series?

Regards,
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V4 0/2] devm_* API operation for fixed regulator

[Manish Badarkhe]

Hi Mark,



On Wed, Jan 29, 2014 at 8:27 PM, Manish Badarkhe
badarkhe.man...@gmail.com wrote:
 Use devm_* API operations for fixed regulator driver so that
 driver core will manage resources.

 Also, introduce a new API devm_kstrdup and used it in fixed
 regulator driver to manage resources.

 Changes since V3:
  1. Update devm_kstrdup function to remove size argument.
 Also,used devm_kmalloc instead of devm_kzalloc.

 Changes since V2:
  1. Update driver to use new API devm_kstrdup instead of
 kstrdup.
  2. Added a seperate patch to introduce new API devm_kstrdup

 Changes since V1:
  1. Updated driver to use devm_kzalloc instead of kstrdup.
  2. Updated commit message.

 Manish Badarkhe (2):
   devres: introduce API devm_kstrdup
   regulator: fixed: update to devm_* API

  drivers/base/devres.c |   26 ++
  drivers/regulator/fixed.c |   42 --
  include/linux/device.h|1 +
  3 files changed, 39 insertions(+), 30 deletions(-)

Are there any review comments on this series?

Regards,
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4 0/2] devm_* API operation for fixed regulator

[Manish Badarkhe]

Use devm_* API operations for fixed regulator driver so that
driver core will manage resources.

Also, introduce a new API "devm_kstrdup" and used it in fixed
regulator driver to manage resources.

Changes since V3:
 1. Update "devm_kstrdup" function to remove "size" argument.
Also,used "devm_kmalloc" instead of "devm_kzalloc".

Changes since V2:
 1. Update driver to use new API "devm_kstrdup" instead of
"kstrdup".
 2. Added a seperate patch to introduce new API "devm_kstrdup"

Changes since V1:
 1. Updated driver to use "devm_kzalloc" instead of "kstrdup".
 2. Updated commit message.

Manish Badarkhe (2):
  devres: introduce API "devm_kstrdup"
  regulator: fixed: update to devm_* API

 drivers/base/devres.c |   26 ++
 drivers/regulator/fixed.c |   42 --
 include/linux/device.h|1 +
 3 files changed, 39 insertions(+), 30 deletions(-)

-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4 2/2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Update the code to use devm_* API so that driver core will manage
resources.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 5ea64b9... 3c307d6... M  drivers/regulator/fixed.c
 drivers/regulator/fixed.c |   42 --
 1 file changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..3c307d6 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
   GFP_KERNEL);
if (drvdata == NULL) {
dev_err(>dev, "Failed to allocate device data\n");
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
 
-   drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
+   drvdata->desc.name = devm_kstrdup(>dev,
+ config->supply_name,
+ GFP_KERNEL);
if (drvdata->desc.name == NULL) {
dev_err(>dev, "Failed to allocate supply name\n");
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
drvdata->desc.type = REGULATOR_VOLTAGE;
drvdata->desc.owner = THIS_MODULE;
@@ -149,13 +149,13 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata->desc.enable_time = config->startup_delay;
 
if (config->input_supply) {
-   drvdata->desc.supply_name = kstrdup(config->input_supply,
-   GFP_KERNEL);
+   drvdata->desc.supply_name = devm_kstrdup(>dev,
+   config->input_supply,
+   GFP_KERNEL);
if (!drvdata->desc.supply_name) {
dev_err(>dev,
"Failed to allocate input supply\n");
-   ret = -ENOMEM;
-   goto err_name;
+   return -ENOMEM;
}
}
 
@@ -186,11 +186,12 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev->dev.of_node;
 
-   drvdata->dev = regulator_register(>desc, );
+   drvdata->dev = devm_regulator_register(>dev, >desc,
+  );
if (IS_ERR(drvdata->dev)) {
ret = PTR_ERR(drvdata->dev);
dev_err(>dev, "Failed to register regulator: %d\n", ret);
-   goto err_input;
+   return ret;
}
 
platform_set_drvdata(pdev, drvdata);
@@ -199,24 +200,6 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata->desc.fixed_uV);
 
return 0;
-
-err_input:
-   kfree(drvdata->desc.supply_name);
-err_name:
-   kfree(drvdata->desc.name);
-err:
-   return ret;
-}
-
-static int reg_fixed_voltage_remove(struct platform_device *pdev)
-{
-   struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
-
-   regulator_unregister(drvdata->dev);
-   kfree(drvdata->desc.supply_name);
-   kfree(drvdata->desc.name);
-
-   return 0;
 }
 
 #if defined(CONFIG_OF)
@@ -229,7 +212,6 @@ MODULE_DEVICE_TABLE(of, fixed_of_match);
 
 static struct platform_driver regulator_fixed_voltage_driver = {
.probe  = reg_fixed_voltage_probe,
-   .remove = reg_fixed_voltage_remove,
.driver = {
.name   = "reg-fixed-voltage",
.owner  = THIS_MODULE,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4 1/2] devres: introduce API "devm_kstrdup"

[Manish Badarkhe]

This patch introduces "devm_kstrdup" API so that the
device's driver can allocate memory and copy string.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 545c4de... db4e264... M  drivers/base/devres.c
:100644 100644 952b010... ec1b6e2... M  include/linux/device.h
 drivers/base/devres.c  |   26 ++
 include/linux/device.h |1 +
 2 files changed, 27 insertions(+)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 545c4de..db4e264 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -791,6 +791,32 @@ void * devm_kmalloc(struct device *dev, size_t size, gfp_t 
gfp)
 EXPORT_SYMBOL_GPL(devm_kmalloc);
 
 /**
+ * devm_kstrdup - Allocate resource managed space and
+ *copy an existing string into that.
+ * @dev: Device to allocate memory for
+ * @s: the string to duplicate
+ * @gfp: the GFP mask used in the devm_kmalloc() call when
+ *   allocating memory
+ * RETURNS:
+ * Pointer to allocated string on success, NULL on failure.
+ */
+char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
+{
+   size_t size;
+   char *buf;
+
+   if (!s)
+   return NULL;
+
+   size = strlen(s) + 1;
+   buf = devm_kmalloc(dev, size, gfp);
+   if (buf)
+   memcpy(buf, s, size);
+   return buf;
+}
+EXPORT_SYMBOL_GPL(devm_kstrdup);
+
+/**
  * devm_kfree - Resource-managed kfree
  * @dev: Device this memory belongs to
  * @p: Memory to free
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..ec1b6e2 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -626,6 +626,7 @@ static inline void *devm_kcalloc(struct device *dev,
return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
 }
 extern void devm_kfree(struct device *dev, void *p);
+extern char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp);
 
 void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
 void __iomem *devm_request_and_ioremap(struct device *dev,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 1/2] devres: introduce API "devm_kstrdup"

[Manish Badarkhe]

Hi Mark,

On Wed, Jan 29, 2014 at 5:10 PM, Mark Brown  wrote:
> On Wed, Jan 29, 2014 at 04:38:20PM +0530, Manish Badarkhe wrote:
>> On Wed, Jan 29, 2014 at 9:48 AM, Joe Perches  wrote:
>
>> >> + buf = devm_kzalloc(dev, size, gfp);
>
>> > If this is really necessary, please use devm_kmalloc
>
>> devm_kzalloc is always better giving zeroed memory locations.
>> Is there any reason not to go for it?
>
> If the allocated memory is going to be immediately overwritten with the
> string then it shouldn't make any difference if it was zeroed when
> allocated.

Thank you for clarification. I will go ahead to make it devm_kmalloc
and post a new version of patch.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 1/2] devres: introduce API "devm_kstrdup"

[Manish Badarkhe]

Hi Joe,

Thank you for your review.

On Wed, Jan 29, 2014 at 9:48 AM, Joe Perches  wrote:
> On Wed, 2014-01-29 at 09:33 +0530, Manish Badarkhe wrote:
>> This patch introduces "devm_kstrdup" API so that the
>> device's driver can allocate memory and copy string.
> []
>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> []
>> @@ -791,6 +791,32 @@ void * devm_kmalloc(struct device *dev, size_t size, 
>> gfp_t gfp)
>>  EXPORT_SYMBOL_GPL(devm_kmalloc);
>>
>>  /**
>> + * devm_kstrdup - Allocate resource managed space and
>> + *and copy an existing string
>> + * @dev: Device to allocate memory for
>> + * @s: the string to duplicate
>> + * @size: Allocation size
>
> Why is size necessary at all?
> I think it should be calculated by strlen

I thought of avoiding string length calculation in function.
But,yes its better to do it in function to avoid extra parsing
of argument to function.Will update code and
post a patch.

>> +char *devm_kstrdup(struct device *dev,
>> +const char *s, size_t size, gfp_t gfp)
>> +{
>> + char *buf;
>> +
>> + if (!s)
>> + return NULL;
>> +
>> + buf = devm_kzalloc(dev, size, gfp);
>
> If this is really necessary, please use devm_kmalloc

devm_kzalloc is always better giving zeroed memory locations.
Is there any reason not to go for it?

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 1/2] devres: introduce API devm_kstrdup

[Manish Badarkhe]

Hi Joe,

Thank you for your review.

On Wed, Jan 29, 2014 at 9:48 AM, Joe Perches j...@perches.com wrote:
 On Wed, 2014-01-29 at 09:33 +0530, Manish Badarkhe wrote:
 This patch introduces devm_kstrdup API so that the
 device's driver can allocate memory and copy string.
 []
 diff --git a/drivers/base/devres.c b/drivers/base/devres.c
 []
 @@ -791,6 +791,32 @@ void * devm_kmalloc(struct device *dev, size_t size, 
 gfp_t gfp)
  EXPORT_SYMBOL_GPL(devm_kmalloc);

  /**
 + * devm_kstrdup - Allocate resource managed space and
 + *and copy an existing string
 + * @dev: Device to allocate memory for
 + * @s: the string to duplicate
 + * @size: Allocation size

 Why is size necessary at all?
 I think it should be calculated by strlen

I thought of avoiding string length calculation in function.
But,yes its better to do it in function to avoid extra parsing
of argument to function.Will update code and
post a patch.

 +char *devm_kstrdup(struct device *dev,
 +const char *s, size_t size, gfp_t gfp)
 +{
 + char *buf;
 +
 + if (!s)
 + return NULL;
 +
 + buf = devm_kzalloc(dev, size, gfp);

 If this is really necessary, please use devm_kmalloc

devm_kzalloc is always better giving zeroed memory locations.
Is there any reason not to go for it?

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3 1/2] devres: introduce API devm_kstrdup

[Manish Badarkhe]

Hi Mark,

On Wed, Jan 29, 2014 at 5:10 PM, Mark Brown broo...@kernel.org wrote:
 On Wed, Jan 29, 2014 at 04:38:20PM +0530, Manish Badarkhe wrote:
 On Wed, Jan 29, 2014 at 9:48 AM, Joe Perches j...@perches.com wrote:

  + buf = devm_kzalloc(dev, size, gfp);

  If this is really necessary, please use devm_kmalloc

 devm_kzalloc is always better giving zeroed memory locations.
 Is there any reason not to go for it?

 If the allocated memory is going to be immediately overwritten with the
 string then it shouldn't make any difference if it was zeroed when
 allocated.

Thank you for clarification. I will go ahead to make it devm_kmalloc
and post a new version of patch.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4 1/2] devres: introduce API devm_kstrdup

[Manish Badarkhe]

This patch introduces devm_kstrdup API so that the
device's driver can allocate memory and copy string.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 545c4de... db4e264... M  drivers/base/devres.c
:100644 100644 952b010... ec1b6e2... M  include/linux/device.h
 drivers/base/devres.c  |   26 ++
 include/linux/device.h |1 +
 2 files changed, 27 insertions(+)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 545c4de..db4e264 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -791,6 +791,32 @@ void * devm_kmalloc(struct device *dev, size_t size, gfp_t 
gfp)
 EXPORT_SYMBOL_GPL(devm_kmalloc);
 
 /**
+ * devm_kstrdup - Allocate resource managed space and
+ *copy an existing string into that.
+ * @dev: Device to allocate memory for
+ * @s: the string to duplicate
+ * @gfp: the GFP mask used in the devm_kmalloc() call when
+ *   allocating memory
+ * RETURNS:
+ * Pointer to allocated string on success, NULL on failure.
+ */
+char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp)
+{
+   size_t size;
+   char *buf;
+
+   if (!s)
+   return NULL;
+
+   size = strlen(s) + 1;
+   buf = devm_kmalloc(dev, size, gfp);
+   if (buf)
+   memcpy(buf, s, size);
+   return buf;
+}
+EXPORT_SYMBOL_GPL(devm_kstrdup);
+
+/**
  * devm_kfree - Resource-managed kfree
  * @dev: Device this memory belongs to
  * @p: Memory to free
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..ec1b6e2 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -626,6 +626,7 @@ static inline void *devm_kcalloc(struct device *dev,
return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
 }
 extern void devm_kfree(struct device *dev, void *p);
+extern char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp);
 
 void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
 void __iomem *devm_request_and_ioremap(struct device *dev,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4 0/2] devm_* API operation for fixed regulator

[Manish Badarkhe]

Use devm_* API operations for fixed regulator driver so that
driver core will manage resources.

Also, introduce a new API devm_kstrdup and used it in fixed
regulator driver to manage resources.

Changes since V3:
 1. Update devm_kstrdup function to remove size argument.
Also,used devm_kmalloc instead of devm_kzalloc.

Changes since V2:
 1. Update driver to use new API devm_kstrdup instead of
kstrdup.
 2. Added a seperate patch to introduce new API devm_kstrdup

Changes since V1:
 1. Updated driver to use devm_kzalloc instead of kstrdup.
 2. Updated commit message.

Manish Badarkhe (2):
  devres: introduce API devm_kstrdup
  regulator: fixed: update to devm_* API

 drivers/base/devres.c |   26 ++
 drivers/regulator/fixed.c |   42 --
 include/linux/device.h|1 +
 3 files changed, 39 insertions(+), 30 deletions(-)

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4 2/2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Update the code to use devm_* API so that driver core will manage
resources.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 5ea64b9... 3c307d6... M  drivers/regulator/fixed.c
 drivers/regulator/fixed.c |   42 --
 1 file changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..3c307d6 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
   GFP_KERNEL);
if (drvdata == NULL) {
dev_err(pdev-dev, Failed to allocate device data\n);
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
 
-   drvdata-desc.name = kstrdup(config-supply_name, GFP_KERNEL);
+   drvdata-desc.name = devm_kstrdup(pdev-dev,
+ config-supply_name,
+ GFP_KERNEL);
if (drvdata-desc.name == NULL) {
dev_err(pdev-dev, Failed to allocate supply name\n);
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
drvdata-desc.type = REGULATOR_VOLTAGE;
drvdata-desc.owner = THIS_MODULE;
@@ -149,13 +149,13 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata-desc.enable_time = config-startup_delay;
 
if (config-input_supply) {
-   drvdata-desc.supply_name = kstrdup(config-input_supply,
-   GFP_KERNEL);
+   drvdata-desc.supply_name = devm_kstrdup(pdev-dev,
+   config-input_supply,
+   GFP_KERNEL);
if (!drvdata-desc.supply_name) {
dev_err(pdev-dev,
Failed to allocate input supply\n);
-   ret = -ENOMEM;
-   goto err_name;
+   return -ENOMEM;
}
}
 
@@ -186,11 +186,12 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev-dev.of_node;
 
-   drvdata-dev = regulator_register(drvdata-desc, cfg);
+   drvdata-dev = devm_regulator_register(pdev-dev, drvdata-desc,
+  cfg);
if (IS_ERR(drvdata-dev)) {
ret = PTR_ERR(drvdata-dev);
dev_err(pdev-dev, Failed to register regulator: %d\n, ret);
-   goto err_input;
+   return ret;
}
 
platform_set_drvdata(pdev, drvdata);
@@ -199,24 +200,6 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata-desc.fixed_uV);
 
return 0;
-
-err_input:
-   kfree(drvdata-desc.supply_name);
-err_name:
-   kfree(drvdata-desc.name);
-err:
-   return ret;
-}
-
-static int reg_fixed_voltage_remove(struct platform_device *pdev)
-{
-   struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
-
-   regulator_unregister(drvdata-dev);
-   kfree(drvdata-desc.supply_name);
-   kfree(drvdata-desc.name);
-
-   return 0;
 }
 
 #if defined(CONFIG_OF)
@@ -229,7 +212,6 @@ MODULE_DEVICE_TABLE(of, fixed_of_match);
 
 static struct platform_driver regulator_fixed_voltage_driver = {
.probe  = reg_fixed_voltage_probe,
-   .remove = reg_fixed_voltage_remove,
.driver = {
.name   = reg-fixed-voltage,
.owner  = THIS_MODULE,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V3 0/2] devm_* API operation for fixed regulator

[Manish Badarkhe]

Use devm_* API operations for fixed regulator driver so that
driver core will manage resources.

Also, introduce a new API "devm_kstrdup" and used it in fixed
regulator driver to manage resources.

Changes since V2:
 1. Update driver to use new API "devm_kstrdup" instead of
"kstrdup".
 2. Added a seperate patch to introduce new API "devm_kstrdup"

Changes since V1:
 1. Updated driver to use "devm_kzalloc" instead of "kstrdup".
 2. Updated commit message.

Manish Badarkhe (2):
  devres: introduce API "devm_kstrdup"
  regulator: fixed: update to devm_* API

 drivers/base/devres.c |   26 ++
 drivers/regulator/fixed.c |   44 ++--
 include/linux/device.h|2 ++
 3 files changed, 42 insertions(+), 30 deletions(-)

-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V3 1/2] devres: introduce API "devm_kstrdup"

[Manish Badarkhe]

This patch introduces "devm_kstrdup" API so that the
device's driver can allocate memory and copy string.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 545c4de... 6e88a3d... M  drivers/base/devres.c
:100644 100644 952b010... 8fee649... M  include/linux/device.h
 drivers/base/devres.c  |   26 ++
 include/linux/device.h |2 ++
 2 files changed, 28 insertions(+)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 545c4de..6e88a3d 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -791,6 +791,32 @@ void * devm_kmalloc(struct device *dev, size_t size, gfp_t 
gfp)
 EXPORT_SYMBOL_GPL(devm_kmalloc);
 
 /**
+ * devm_kstrdup - Allocate resource managed space and
+ *and copy an existing string
+ * @dev: Device to allocate memory for
+ * @s: the string to duplicate
+ * @size: Allocation size
+ * @gfp: the GFP mask used in the devm_kzalloc() call when
+ *   allocating memory
+ * RETURNS:
+ * Pointer to allocated string on success, NULL on failure.
+ */
+char *devm_kstrdup(struct device *dev,
+  const char *s, size_t size, gfp_t gfp)
+{
+   char *buf;
+
+   if (!s)
+   return NULL;
+
+   buf = devm_kzalloc(dev, size, gfp);
+   if (buf)
+   memcpy(buf, s, size);
+   return buf;
+}
+EXPORT_SYMBOL_GPL(devm_kstrdup);
+
+/**
  * devm_kfree - Resource-managed kfree
  * @dev: Device this memory belongs to
  * @p: Memory to free
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..8fee649 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -626,6 +626,8 @@ static inline void *devm_kcalloc(struct device *dev,
return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
 }
 extern void devm_kfree(struct device *dev, void *p);
+extern char *devm_kstrdup(struct device *dev, const char *s, size_t size,
+   gfp_t gfp);
 
 void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
 void __iomem *devm_request_and_ioremap(struct device *dev,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V3 2/2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Update the code to use devm_* API so that driver core will manage
resources.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 5ea64b9... 1d42613... M  drivers/regulator/fixed.c
 drivers/regulator/fixed.c |   44 ++--
 1 file changed, 14 insertions(+), 30 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..1d42613 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -132,15 +132,16 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
   GFP_KERNEL);
if (drvdata == NULL) {
dev_err(>dev, "Failed to allocate device data\n");
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
 
-   drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
+   drvdata->desc.name = devm_kstrdup(>dev,
+ config->supply_name,
+ strlen(config->supply_name) + 1,
+ GFP_KERNEL);
if (drvdata->desc.name == NULL) {
dev_err(>dev, "Failed to allocate supply name\n");
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
drvdata->desc.type = REGULATOR_VOLTAGE;
drvdata->desc.owner = THIS_MODULE;
@@ -149,13 +150,14 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata->desc.enable_time = config->startup_delay;
 
if (config->input_supply) {
-   drvdata->desc.supply_name = kstrdup(config->input_supply,
-   GFP_KERNEL);
+   drvdata->desc.supply_name = devm_kstrdup(>dev,
+   config->input_supply,
+   strlen(config->input_supply) + 1,
+   GFP_KERNEL);
if (!drvdata->desc.supply_name) {
dev_err(>dev,
"Failed to allocate input supply\n");
-   ret = -ENOMEM;
-   goto err_name;
+   return -ENOMEM;
}
}
 
@@ -186,11 +188,12 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev->dev.of_node;
 
-   drvdata->dev = regulator_register(>desc, );
+   drvdata->dev = devm_regulator_register(>dev, >desc,
+  );
if (IS_ERR(drvdata->dev)) {
ret = PTR_ERR(drvdata->dev);
dev_err(>dev, "Failed to register regulator: %d\n", ret);
-   goto err_input;
+   return ret;
}
 
platform_set_drvdata(pdev, drvdata);
@@ -199,24 +202,6 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata->desc.fixed_uV);
 
return 0;
-
-err_input:
-   kfree(drvdata->desc.supply_name);
-err_name:
-   kfree(drvdata->desc.name);
-err:
-   return ret;
-}
-
-static int reg_fixed_voltage_remove(struct platform_device *pdev)
-{
-   struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
-
-   regulator_unregister(drvdata->dev);
-   kfree(drvdata->desc.supply_name);
-   kfree(drvdata->desc.name);
-
-   return 0;
 }
 
 #if defined(CONFIG_OF)
@@ -229,7 +214,6 @@ MODULE_DEVICE_TABLE(of, fixed_of_match);
 
 static struct platform_driver regulator_fixed_voltage_driver = {
.probe  = reg_fixed_voltage_probe,
-   .remove = reg_fixed_voltage_remove,
.driver = {
.name   = "reg-fixed-voltage",
.owner  = THIS_MODULE,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Hi Mark,

On Tue, Jan 28, 2014 at 4:15 PM, Mark Brown  wrote:
> On Tue, Jan 28, 2014 at 02:31:32PM +0530, Manish Badarkhe wrote:
>> On Tue, Jan 28, 2014 at 2:25 PM, Heiko Stübner  wrote:
>
>> > I don't have a strong opinion on this and others are most likely more 
>> > qualified
>> > to have a definitive answer, I just found it strange to exchange one 
>> > open-coded
>> > pattern against another open-coded one.
>> >
>> > So don't let me keep you from it ;-)
>
>> Okay.
>> @Mark, Dmitry: Please let me know your opinion on this.
>
> How about implementing devm_kstrdup() and then using that?

Okay, I will update kstrdup()->devm_kstrdup() and post next version of patch.

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Hi Stephan,

On Tue, Jan 28, 2014 at 10:11 PM, Stephen Warren  wrote:
> On 01/27/2014 08:12 PM, Manish Badarkhe wrote:
>> Update the code to use devm_* API so that driver core will manage
>> resources.
>
> I'm not sure why this patch is sent to linux-te...@vger.kernel.org; it
> seems nothing to do with Tegra (or Samsung or OMAP for that matter).

Sorry, for the noise. I sent it to have review comments from community.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Instead of "#ifdef CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe 
---
Changes since V1:
1.Updated code to use "IS_ENABLED" during retrieval 
  of platform/DT data.

Changes since V2:
1.Updated code as per Dmitry's comment to give preferance to
  platform data. 

Changes since V3:
1.Updated code to use "IS_ENABLED(CONFIG_OF)" to remove 
  DT data retrieval function during compilation itself
  when CONFIG_OF option is not set.

:100644 100644 b4513f2... a7482db... M  drivers/power/max8925_power.c
 drivers/power/max8925_power.c |   17 +
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..a7482db 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct max8925_power_info 
*info)
return 0;
 }
 
-#ifdef CONFIG_OF
 static struct max8925_power_pdata *
 max8925_power_dt_init(struct platform_device *pdev)
 {
@@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
int no_insert_detect;
struct max8925_power_pdata *pdata;
 
-   if (!nproot)
-   return pdev->dev.platform_data;
-
np = of_find_node_by_name(nproot, "charger");
if (!np) {
dev_err(>dev, "failed to find charger node\n");
@@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 
return pdata;
 }
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
-   return pdev->dev.platform_data;
-}
-#endif
 
 static int max8925_power_probe(struct platform_device *pdev)
 {
@@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device 
*pdev)
struct max8925_power_info *info;
int ret;
 
-   pdata = max8925_power_dt_init(pdev);
+   pdata = dev_get_platdata(>dev);
+
+   if (IS_ENABLED(CONFIG_OF) && !pdata && chip->dev->of_node)
+   pdata = max8925_power_dt_init(pdev);
+
if (!pdata) {
dev_err(>dev, "platform data isn't assigned to "
"power supply\n");
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Hi Tomasz,

On Tue, Jan 28, 2014 at 5:21 PM, Tomasz Figa  wrote:
> Hi,
>
>
> On 27.01.2014 19:37, Manish Badarkhe wrote:
>>
>> Instead of "#ifdef CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
>> option for DT code to avoid if-deffery in code.
>>
>> Signed-off-by: Manish Badarkhe 
>> ---
>> :100644 100644 b4513f2... 3e54476... M  drivers/power/max8925_power.c
>>   drivers/power/max8925_power.c |   17 +
>>   1 file changed, 5 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
>> index b4513f2..3e54476 100644
>> --- a/drivers/power/max8925_power.c
>> +++ b/drivers/power/max8925_power.c
>> @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct
>> max8925_power_info *info)
>> return 0;
>>   }
>>
>> -#ifdef CONFIG_OF
>>   static struct max8925_power_pdata *
>>   max8925_power_dt_init(struct platform_device *pdev)
>>   {
>> @@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
>> int no_insert_detect;
>> struct max8925_power_pdata *pdata;
>>
>> -   if (!nproot)
>> -   return pdev->dev.platform_data;
>> -
>> np = of_find_node_by_name(nproot, "charger");
>> if (!np) {
>> dev_err(>dev, "failed to find charger node\n");
>> @@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)
>>
>> return pdata;
>>   }
>> -#else
>> -static struct max8925_power_pdata *
>> -max8925_power_dt_init(struct platform_device *pdev)
>> -{
>> -   return pdev->dev.platform_data;
>> -}
>> -#endif
>>
>>   static int max8925_power_probe(struct platform_device *pdev)
>>   {
>> @@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device
>> *pdev)
>> struct max8925_power_info *info;
>> int ret;
>>
>> -   pdata = max8925_power_dt_init(pdev);
>> +   pdata = dev_get_platdata(>dev);
>> +
>> +   if (!pdata && chip->dev->of_node)
>
>
> Shouldn't IS_ENABLED(CONFIG_OF) also be checked here to let the compiler
> throw max8925_power_dt_init() away if the condition always evaluates to
> false?

oops, I missed that. I will add it and repost patch.

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

On Tue, Jan 28, 2014 at 2:25 PM, Heiko Stübner  wrote:
> On Tuesday, 28. January 2014 14:16:39 Manish Badarkhe wrote:
>> Hi Heiko
>>
>> Thank you for your reply.
>>
>> On Tue, Jan 28, 2014 at 1:08 PM, Heiko Stübner  wrote:
>> > On Tuesday, 28. January 2014 12:46:01 Manish Badarkhe wrote:
>> >> Hi Dmitry,
>> >>
>> >> Thank you for your review.
>> >>
>> >> On Tue, Jan 28, 2014 at 12:03 PM, Dmitry Torokhov
>> >>
>> >>  wrote:
>> >> > Hi Manish,
>> >> >
>> >> > On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
>> >> >> Update the code to use devm_* API so that driver core will manage
>> >> >> resources.
>> >> >>
>> >> >> Signed-off-by: Manish Badarkhe 
>> >> >> ---
>> >> >> Changes since V1:
>> >> >> 1. Updated driver to use "devm_kzalloc" to "kstrdup".
>> >> >> 2. Updated commit message.
>> >> >>
>> >> >> Not tested on any board.
>> >> >>
>> >> >> :100644 100644 5ea64b9... e9763a4... M
>> >> >> :drivers/regulator/fixed.c
>> >> >> :
>> >> >>  drivers/regulator/fixed.c |   42
>> >> >>  --
>> >> >>  1 file changed, 12 insertions(+), 30 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
>> >> >> index 5ea64b9..e9763a4 100644
>> >> >> --- a/drivers/regulator/fixed.c
>> >> >> +++ b/drivers/regulator/fixed.c
>> >> >> @@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct
>> >> >> platform_device *pdev)>>
>> >> >>
>> >> >>  GFP_KERNEL);
>> >> >>
>> >> >>   if (drvdata == NULL) {
>> >> >>
>> >> >>   dev_err(>dev, "Failed to allocate device data\n");
>> >> >>
>> >> >> - ret = -ENOMEM;
>> >> >> - goto err;
>> >> >> + return -ENOMEM;
>> >> >>
>> >> >>   }
>> >> >>
>> >> >> - drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
>> >> >> + drvdata->desc.name = devm_kzalloc(>dev,
>> >> >> +   strlen(config->supply_name) +
>> >> >> 1,
>> >> >> +   GFP_KERNEL);
>> >> >>
>> >> >>   if (drvdata->desc.name == NULL) {
>> >> >>
>> >> >>   dev_err(>dev, "Failed to allocate supply name\n");
>> >> >>
>> >> >> - ret = -ENOMEM;
>> >> >> - goto err;
>> >> >> + return -ENOMEM;
>> >> >>
>> >> >>   }
>> >> >
>> >> > Umm, I am fairly certain that devm_kzalloc() can't be used as a
>> >> > substitute for kstrdup, at least not without accompanying memcpy.
>> >>
>> >> Yes, I have provided allocation but it should be followed with
>> >> assignment.
>> >> Can I modify like this,
>> >>
>> >> + drvdata->desc.name = devm_kzalloc(>dev,
>> >> +   strlen(config->supply_name) + 1,
>> >> +   GFP_KERNEL);
>> >> + if (drvdata->desc.name)
>> >> + sprintf(drvdata->desc.name, "%s", config->supply_name);
>> >
>> > hmm, so you replaced a general helper function by open coding the string-
>> > duplication. Doesn't this defeat the target of simplifying the code?
>>
>> Intention here, is to use devm_ API and to adopt this I have to do these
>> modifications for "kstrdup" functions. I have seen in regulator folder
>> almost all drivers adopted to "devm_" API. Hence same thing I am following
>> to update this driver. Please let me know whether to go ahead with this
>> patch or retain driver as it is.
>
> I don't have a strong opinion on this and others are most likely more 
> qualified
> to have a definitive answer, I just found it strange to exchange one 
> open-coded
> pattern against another open-coded one.
>
> So don't let me keep you from it ;-)

Okay.
@Mark, Dmitry: Please let me know your opinion on this.

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Hi Heiko

Thank you for your reply.

On Tue, Jan 28, 2014 at 1:08 PM, Heiko Stübner  wrote:
> On Tuesday, 28. January 2014 12:46:01 Manish Badarkhe wrote:
>> Hi Dmitry,
>>
>> Thank you for your review.
>>
>> On Tue, Jan 28, 2014 at 12:03 PM, Dmitry Torokhov
>>
>>  wrote:
>> > Hi Manish,
>> >
>> > On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
>> >> Update the code to use devm_* API so that driver core will manage
>> >> resources.
>> >>
>> >> Signed-off-by: Manish Badarkhe 
>> >> ---
>> >> Changes since V1:
>> >> 1. Updated driver to use "devm_kzalloc" to "kstrdup".
>> >> 2. Updated commit message.
>> >>
>> >> Not tested on any board.
>> >>
>> >> :100644 100644 5ea64b9... e9763a4... Mdrivers/regulator/fixed.c
>> >> :
>> >>  drivers/regulator/fixed.c |   42
>> >>  --
>> >>  1 file changed, 12 insertions(+), 30 deletions(-)
>> >>
>> >> diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
>> >> index 5ea64b9..e9763a4 100644
>> >> --- a/drivers/regulator/fixed.c
>> >> +++ b/drivers/regulator/fixed.c
>> >> @@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct
>> >> platform_device *pdev)>>
>> >>  GFP_KERNEL);
>> >>
>> >>   if (drvdata == NULL) {
>> >>
>> >>   dev_err(>dev, "Failed to allocate device data\n");
>> >>
>> >> - ret = -ENOMEM;
>> >> - goto err;
>> >> + return -ENOMEM;
>> >>
>> >>   }
>> >>
>> >> - drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
>> >> + drvdata->desc.name = devm_kzalloc(>dev,
>> >> +   strlen(config->supply_name) + 1,
>> >> +   GFP_KERNEL);
>> >>
>> >>   if (drvdata->desc.name == NULL) {
>> >>
>> >>   dev_err(>dev, "Failed to allocate supply name\n");
>> >>
>> >> - ret = -ENOMEM;
>> >> - goto err;
>> >> + return -ENOMEM;
>> >>
>> >>   }
>> >
>> > Umm, I am fairly certain that devm_kzalloc() can't be used as a
>> > substitute for kstrdup, at least not without accompanying memcpy.
>>
>> Yes, I have provided allocation but it should be followed with assignment.
>> Can I modify like this,
>>
>> + drvdata->desc.name = devm_kzalloc(>dev,
>> +   strlen(config->supply_name) + 1,
>> +   GFP_KERNEL);
>> + if (drvdata->desc.name)
>> + sprintf(drvdata->desc.name, "%s", config->supply_name);
>
> hmm, so you replaced a general helper function by open coding the string-
> duplication. Doesn't this defeat the target of simplifying the code?

Intention here, is to use devm_ API and to adopt this I have to do these
modifications for "kstrdup" functions. I have seen in regulator folder almost
all drivers adopted to "devm_" API. Hence same thing I am following to
update this driver. Please let me know whether to go ahead with this patch
or retain driver as it is.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Hi Heiko

Thank you for your reply.

On Tue, Jan 28, 2014 at 1:08 PM, Heiko Stübner he...@sntech.de wrote:
 On Tuesday, 28. January 2014 12:46:01 Manish Badarkhe wrote:
 Hi Dmitry,

 Thank you for your review.

 On Tue, Jan 28, 2014 at 12:03 PM, Dmitry Torokhov

 dmitry.torok...@gmail.com wrote:
  Hi Manish,
 
  On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
  Update the code to use devm_* API so that driver core will manage
  resources.
 
  Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
  ---
  Changes since V1:
  1. Updated driver to use devm_kzalloc to kstrdup.
  2. Updated commit message.
 
  Not tested on any board.
 
  :100644 100644 5ea64b9... e9763a4... Mdrivers/regulator/fixed.c
  :
   drivers/regulator/fixed.c |   42
   --
   1 file changed, 12 insertions(+), 30 deletions(-)
 
  diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
  index 5ea64b9..e9763a4 100644
  --- a/drivers/regulator/fixed.c
  +++ b/drivers/regulator/fixed.c
  @@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct
  platform_device *pdev)
   GFP_KERNEL);
 
if (drvdata == NULL) {
 
dev_err(pdev-dev, Failed to allocate device data\n);
 
  - ret = -ENOMEM;
  - goto err;
  + return -ENOMEM;
 
}
 
  - drvdata-desc.name = kstrdup(config-supply_name, GFP_KERNEL);
  + drvdata-desc.name = devm_kzalloc(pdev-dev,
  +   strlen(config-supply_name) + 1,
  +   GFP_KERNEL);
 
if (drvdata-desc.name == NULL) {
 
dev_err(pdev-dev, Failed to allocate supply name\n);
 
  - ret = -ENOMEM;
  - goto err;
  + return -ENOMEM;
 
}
 
  Umm, I am fairly certain that devm_kzalloc() can't be used as a
  substitute for kstrdup, at least not without accompanying memcpy.

 Yes, I have provided allocation but it should be followed with assignment.
 Can I modify like this,

 + drvdata-desc.name = devm_kzalloc(pdev-dev,
 +   strlen(config-supply_name) + 1,
 +   GFP_KERNEL);
 + if (drvdata-desc.name)
 + sprintf(drvdata-desc.name, %s, config-supply_name);

 hmm, so you replaced a general helper function by open coding the string-
 duplication. Doesn't this defeat the target of simplifying the code?

Intention here, is to use devm_ API and to adopt this I have to do these
modifications for kstrdup functions. I have seen in regulator folder almost
all drivers adopted to devm_ API. Hence same thing I am following to
update this driver. Please let me know whether to go ahead with this patch
or retain driver as it is.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

On Tue, Jan 28, 2014 at 2:25 PM, Heiko Stübner he...@sntech.de wrote:
 On Tuesday, 28. January 2014 14:16:39 Manish Badarkhe wrote:
 Hi Heiko

 Thank you for your reply.

 On Tue, Jan 28, 2014 at 1:08 PM, Heiko Stübner he...@sntech.de wrote:
  On Tuesday, 28. January 2014 12:46:01 Manish Badarkhe wrote:
  Hi Dmitry,
 
  Thank you for your review.
 
  On Tue, Jan 28, 2014 at 12:03 PM, Dmitry Torokhov
 
  dmitry.torok...@gmail.com wrote:
   Hi Manish,
  
   On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
   Update the code to use devm_* API so that driver core will manage
   resources.
  
   Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
   ---
   Changes since V1:
   1. Updated driver to use devm_kzalloc to kstrdup.
   2. Updated commit message.
  
   Not tested on any board.
  
   :100644 100644 5ea64b9... e9763a4... M
   :drivers/regulator/fixed.c
   :
drivers/regulator/fixed.c |   42
--
1 file changed, 12 insertions(+), 30 deletions(-)
  
   diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
   index 5ea64b9..e9763a4 100644
   --- a/drivers/regulator/fixed.c
   +++ b/drivers/regulator/fixed.c
   @@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct
   platform_device *pdev)
  
GFP_KERNEL);
  
 if (drvdata == NULL) {
  
 dev_err(pdev-dev, Failed to allocate device data\n);
  
   - ret = -ENOMEM;
   - goto err;
   + return -ENOMEM;
  
 }
  
   - drvdata-desc.name = kstrdup(config-supply_name, GFP_KERNEL);
   + drvdata-desc.name = devm_kzalloc(pdev-dev,
   +   strlen(config-supply_name) +
   1,
   +   GFP_KERNEL);
  
 if (drvdata-desc.name == NULL) {
  
 dev_err(pdev-dev, Failed to allocate supply name\n);
  
   - ret = -ENOMEM;
   - goto err;
   + return -ENOMEM;
  
 }
  
   Umm, I am fairly certain that devm_kzalloc() can't be used as a
   substitute for kstrdup, at least not without accompanying memcpy.
 
  Yes, I have provided allocation but it should be followed with
  assignment.
  Can I modify like this,
 
  + drvdata-desc.name = devm_kzalloc(pdev-dev,
  +   strlen(config-supply_name) + 1,
  +   GFP_KERNEL);
  + if (drvdata-desc.name)
  + sprintf(drvdata-desc.name, %s, config-supply_name);
 
  hmm, so you replaced a general helper function by open coding the string-
  duplication. Doesn't this defeat the target of simplifying the code?

 Intention here, is to use devm_ API and to adopt this I have to do these
 modifications for kstrdup functions. I have seen in regulator folder
 almost all drivers adopted to devm_ API. Hence same thing I am following
 to update this driver. Please let me know whether to go ahead with this
 patch or retain driver as it is.

 I don't have a strong opinion on this and others are most likely more 
 qualified
 to have a definitive answer, I just found it strange to exchange one 
 open-coded
 pattern against another open-coded one.

 So don't let me keep you from it ;-)

Okay.
@Mark, Dmitry: Please let me know your opinion on this.

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V3] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Hi Tomasz,

On Tue, Jan 28, 2014 at 5:21 PM, Tomasz Figa t.f...@samsung.com wrote:
 Hi,


 On 27.01.2014 19:37, Manish Badarkhe wrote:

 Instead of #ifdef CONFIG_OF use IS_ENABLED(CONFIG_OF)
 option for DT code to avoid if-deffery in code.

 Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
 ---
 :100644 100644 b4513f2... 3e54476... M  drivers/power/max8925_power.c
   drivers/power/max8925_power.c |   17 +
   1 file changed, 5 insertions(+), 12 deletions(-)

 diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
 index b4513f2..3e54476 100644
 --- a/drivers/power/max8925_power.c
 +++ b/drivers/power/max8925_power.c
 @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct
 max8925_power_info *info)
 return 0;
   }

 -#ifdef CONFIG_OF
   static struct max8925_power_pdata *
   max8925_power_dt_init(struct platform_device *pdev)
   {
 @@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 int no_insert_detect;
 struct max8925_power_pdata *pdata;

 -   if (!nproot)
 -   return pdev-dev.platform_data;
 -
 np = of_find_node_by_name(nproot, charger);
 if (!np) {
 dev_err(pdev-dev, failed to find charger node\n);
 @@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)

 return pdata;
   }
 -#else
 -static struct max8925_power_pdata *
 -max8925_power_dt_init(struct platform_device *pdev)
 -{
 -   return pdev-dev.platform_data;
 -}
 -#endif

   static int max8925_power_probe(struct platform_device *pdev)
   {
 @@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device
 *pdev)
 struct max8925_power_info *info;
 int ret;

 -   pdata = max8925_power_dt_init(pdev);
 +   pdata = dev_get_platdata(pdev-dev);
 +
 +   if (!pdata  chip-dev-of_node)


 Shouldn't IS_ENABLED(CONFIG_OF) also be checked here to let the compiler
 throw max8925_power_dt_init() away if the condition always evaluates to
 false?

oops, I missed that. I will add it and repost patch.

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Instead of #ifdef CONFIG_OF use IS_ENABLED(CONFIG_OF)
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
Changes since V1:
1.Updated code to use IS_ENABLED during retrieval 
  of platform/DT data.

Changes since V2:
1.Updated code as per Dmitry's comment to give preferance to
  platform data. 

Changes since V3:
1.Updated code to use IS_ENABLED(CONFIG_OF) to remove 
  DT data retrieval function during compilation itself
  when CONFIG_OF option is not set.

:100644 100644 b4513f2... a7482db... M  drivers/power/max8925_power.c
 drivers/power/max8925_power.c |   17 +
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..a7482db 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct max8925_power_info 
*info)
return 0;
 }
 
-#ifdef CONFIG_OF
 static struct max8925_power_pdata *
 max8925_power_dt_init(struct platform_device *pdev)
 {
@@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
int no_insert_detect;
struct max8925_power_pdata *pdata;
 
-   if (!nproot)
-   return pdev-dev.platform_data;
-
np = of_find_node_by_name(nproot, charger);
if (!np) {
dev_err(pdev-dev, failed to find charger node\n);
@@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 
return pdata;
 }
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
-   return pdev-dev.platform_data;
-}
-#endif
 
 static int max8925_power_probe(struct platform_device *pdev)
 {
@@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device 
*pdev)
struct max8925_power_info *info;
int ret;
 
-   pdata = max8925_power_dt_init(pdev);
+   pdata = dev_get_platdata(pdev-dev);
+
+   if (IS_ENABLED(CONFIG_OF)  !pdata  chip-dev-of_node)
+   pdata = max8925_power_dt_init(pdev);
+
if (!pdata) {
dev_err(pdev-dev, platform data isn't assigned to 
power supply\n);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Hi Stephan,

On Tue, Jan 28, 2014 at 10:11 PM, Stephen Warren swar...@wwwdotorg.org wrote:
 On 01/27/2014 08:12 PM, Manish Badarkhe wrote:
 Update the code to use devm_* API so that driver core will manage
 resources.

 I'm not sure why this patch is sent to linux-te...@vger.kernel.org; it
 seems nothing to do with Tegra (or Samsung or OMAP for that matter).

Sorry, for the noise. I sent it to have review comments from community.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Hi Mark,

On Tue, Jan 28, 2014 at 4:15 PM, Mark Brown broo...@kernel.org wrote:
 On Tue, Jan 28, 2014 at 02:31:32PM +0530, Manish Badarkhe wrote:
 On Tue, Jan 28, 2014 at 2:25 PM, Heiko Stübner he...@sntech.de wrote:

  I don't have a strong opinion on this and others are most likely more 
  qualified
  to have a definitive answer, I just found it strange to exchange one 
  open-coded
  pattern against another open-coded one.
 
  So don't let me keep you from it ;-)

 Okay.
 @Mark, Dmitry: Please let me know your opinion on this.

 How about implementing devm_kstrdup() and then using that?

Okay, I will update kstrdup()-devm_kstrdup() and post next version of patch.

Thanks
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V3 2/2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Update the code to use devm_* API so that driver core will manage
resources.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 5ea64b9... 1d42613... M  drivers/regulator/fixed.c
 drivers/regulator/fixed.c |   44 ++--
 1 file changed, 14 insertions(+), 30 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..1d42613 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -132,15 +132,16 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
   GFP_KERNEL);
if (drvdata == NULL) {
dev_err(pdev-dev, Failed to allocate device data\n);
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
 
-   drvdata-desc.name = kstrdup(config-supply_name, GFP_KERNEL);
+   drvdata-desc.name = devm_kstrdup(pdev-dev,
+ config-supply_name,
+ strlen(config-supply_name) + 1,
+ GFP_KERNEL);
if (drvdata-desc.name == NULL) {
dev_err(pdev-dev, Failed to allocate supply name\n);
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
drvdata-desc.type = REGULATOR_VOLTAGE;
drvdata-desc.owner = THIS_MODULE;
@@ -149,13 +150,14 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata-desc.enable_time = config-startup_delay;
 
if (config-input_supply) {
-   drvdata-desc.supply_name = kstrdup(config-input_supply,
-   GFP_KERNEL);
+   drvdata-desc.supply_name = devm_kstrdup(pdev-dev,
+   config-input_supply,
+   strlen(config-input_supply) + 1,
+   GFP_KERNEL);
if (!drvdata-desc.supply_name) {
dev_err(pdev-dev,
Failed to allocate input supply\n);
-   ret = -ENOMEM;
-   goto err_name;
+   return -ENOMEM;
}
}
 
@@ -186,11 +188,12 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev-dev.of_node;
 
-   drvdata-dev = regulator_register(drvdata-desc, cfg);
+   drvdata-dev = devm_regulator_register(pdev-dev, drvdata-desc,
+  cfg);
if (IS_ERR(drvdata-dev)) {
ret = PTR_ERR(drvdata-dev);
dev_err(pdev-dev, Failed to register regulator: %d\n, ret);
-   goto err_input;
+   return ret;
}
 
platform_set_drvdata(pdev, drvdata);
@@ -199,24 +202,6 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata-desc.fixed_uV);
 
return 0;
-
-err_input:
-   kfree(drvdata-desc.supply_name);
-err_name:
-   kfree(drvdata-desc.name);
-err:
-   return ret;
-}
-
-static int reg_fixed_voltage_remove(struct platform_device *pdev)
-{
-   struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
-
-   regulator_unregister(drvdata-dev);
-   kfree(drvdata-desc.supply_name);
-   kfree(drvdata-desc.name);
-
-   return 0;
 }
 
 #if defined(CONFIG_OF)
@@ -229,7 +214,6 @@ MODULE_DEVICE_TABLE(of, fixed_of_match);
 
 static struct platform_driver regulator_fixed_voltage_driver = {
.probe  = reg_fixed_voltage_probe,
-   .remove = reg_fixed_voltage_remove,
.driver = {
.name   = reg-fixed-voltage,
.owner  = THIS_MODULE,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V3 1/2] devres: introduce API devm_kstrdup

[Manish Badarkhe]

This patch introduces devm_kstrdup API so that the
device's driver can allocate memory and copy string.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 545c4de... 6e88a3d... M  drivers/base/devres.c
:100644 100644 952b010... 8fee649... M  include/linux/device.h
 drivers/base/devres.c  |   26 ++
 include/linux/device.h |2 ++
 2 files changed, 28 insertions(+)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 545c4de..6e88a3d 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -791,6 +791,32 @@ void * devm_kmalloc(struct device *dev, size_t size, gfp_t 
gfp)
 EXPORT_SYMBOL_GPL(devm_kmalloc);
 
 /**
+ * devm_kstrdup - Allocate resource managed space and
+ *and copy an existing string
+ * @dev: Device to allocate memory for
+ * @s: the string to duplicate
+ * @size: Allocation size
+ * @gfp: the GFP mask used in the devm_kzalloc() call when
+ *   allocating memory
+ * RETURNS:
+ * Pointer to allocated string on success, NULL on failure.
+ */
+char *devm_kstrdup(struct device *dev,
+  const char *s, size_t size, gfp_t gfp)
+{
+   char *buf;
+
+   if (!s)
+   return NULL;
+
+   buf = devm_kzalloc(dev, size, gfp);
+   if (buf)
+   memcpy(buf, s, size);
+   return buf;
+}
+EXPORT_SYMBOL_GPL(devm_kstrdup);
+
+/**
  * devm_kfree - Resource-managed kfree
  * @dev: Device this memory belongs to
  * @p: Memory to free
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b010..8fee649 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -626,6 +626,8 @@ static inline void *devm_kcalloc(struct device *dev,
return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
 }
 extern void devm_kfree(struct device *dev, void *p);
+extern char *devm_kstrdup(struct device *dev, const char *s, size_t size,
+   gfp_t gfp);
 
 void __iomem *devm_ioremap_resource(struct device *dev, struct resource *res);
 void __iomem *devm_request_and_ioremap(struct device *dev,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V3 0/2] devm_* API operation for fixed regulator

[Manish Badarkhe]

Use devm_* API operations for fixed regulator driver so that
driver core will manage resources.

Also, introduce a new API devm_kstrdup and used it in fixed
regulator driver to manage resources.

Changes since V2:
 1. Update driver to use new API devm_kstrdup instead of
kstrdup.
 2. Added a seperate patch to introduce new API devm_kstrdup

Changes since V1:
 1. Updated driver to use devm_kzalloc instead of kstrdup.
 2. Updated commit message.

Manish Badarkhe (2):
  devres: introduce API devm_kstrdup
  regulator: fixed: update to devm_* API

 drivers/base/devres.c |   26 ++
 drivers/regulator/fixed.c |   44 ++--
 include/linux/device.h|2 ++
 3 files changed, 42 insertions(+), 30 deletions(-)

-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Hi Dmitry,

Thank you for your review.

On Tue, Jan 28, 2014 at 12:03 PM, Dmitry Torokhov
 wrote:
> Hi Manish,
>
> On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
>> Update the code to use devm_* API so that driver core will manage
>> resources.
>>
>> Signed-off-by: Manish Badarkhe 
>> ---
>> Changes since V1:
>> 1. Updated driver to use "devm_kzalloc" to "kstrdup".
>> 2. Updated commit message.
>>
>> Not tested on any board.
>>
>> :100644 100644 5ea64b9... e9763a4... Mdrivers/regulator/fixed.c
>>  drivers/regulator/fixed.c |   42 --
>>  1 file changed, 12 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
>> index 5ea64b9..e9763a4 100644
>> --- a/drivers/regulator/fixed.c
>> +++ b/drivers/regulator/fixed.c
>> @@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct 
>> platform_device *pdev)
>>  GFP_KERNEL);
>>   if (drvdata == NULL) {
>>   dev_err(>dev, "Failed to allocate device data\n");
>> - ret = -ENOMEM;
>> - goto err;
>> + return -ENOMEM;
>>   }
>>
>> - drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
>> + drvdata->desc.name = devm_kzalloc(>dev,
>> +   strlen(config->supply_name) + 1,
>> +   GFP_KERNEL);
>>   if (drvdata->desc.name == NULL) {
>>   dev_err(>dev, "Failed to allocate supply name\n");
>> - ret = -ENOMEM;
>> - goto err;
>> + return -ENOMEM;
>>   }
>
> Umm, I am fairly certain that devm_kzalloc() can't be used as a
> substitute for kstrdup, at least not without accompanying memcpy.

Yes, I have provided allocation but it should be followed with assignment.
Can I modify like this,

+ drvdata->desc.name = devm_kzalloc(>dev,
+   strlen(config->supply_name) + 1,
+   GFP_KERNEL);
+ if (drvdata->desc.name)
+ sprintf(drvdata->desc.name, "%s", config->supply_name);

Thanks
Manish Badakhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Update the code to use devm_* API so that driver core will manage
resources.

Signed-off-by: Manish Badarkhe 
---
Changes since V1:
1. Updated driver to use "devm_kzalloc" to "kstrdup".
2. Updated commit message.

Not tested on any board.

:100644 100644 5ea64b9... e9763a4... M  drivers/regulator/fixed.c
 drivers/regulator/fixed.c |   42 --
 1 file changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..e9763a4 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
   GFP_KERNEL);
if (drvdata == NULL) {
dev_err(>dev, "Failed to allocate device data\n");
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
 
-   drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
+   drvdata->desc.name = devm_kzalloc(>dev,
+ strlen(config->supply_name) + 1,
+ GFP_KERNEL);
if (drvdata->desc.name == NULL) {
dev_err(>dev, "Failed to allocate supply name\n");
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
drvdata->desc.type = REGULATOR_VOLTAGE;
drvdata->desc.owner = THIS_MODULE;
@@ -149,13 +149,13 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata->desc.enable_time = config->startup_delay;
 
if (config->input_supply) {
-   drvdata->desc.supply_name = kstrdup(config->input_supply,
-   GFP_KERNEL);
+   drvdata->desc.supply_name = devm_kzalloc(>dev,
+   strlen(config->input_supply) + 1,
+   GFP_KERNEL);
if (!drvdata->desc.supply_name) {
dev_err(>dev,
"Failed to allocate input supply\n");
-   ret = -ENOMEM;
-   goto err_name;
+   return -ENOMEM;
}
}
 
@@ -186,11 +186,12 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev->dev.of_node;
 
-   drvdata->dev = regulator_register(>desc, );
+   drvdata->dev = devm_regulator_register(>dev, >desc,
+  );
if (IS_ERR(drvdata->dev)) {
ret = PTR_ERR(drvdata->dev);
dev_err(>dev, "Failed to register regulator: %d\n", ret);
-   goto err_input;
+   return ret;
}
 
platform_set_drvdata(pdev, drvdata);
@@ -199,24 +200,6 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata->desc.fixed_uV);
 
return 0;
-
-err_input:
-   kfree(drvdata->desc.supply_name);
-err_name:
-   kfree(drvdata->desc.name);
-err:
-   return ret;
-}
-
-static int reg_fixed_voltage_remove(struct platform_device *pdev)
-{
-   struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
-
-   regulator_unregister(drvdata->dev);
-   kfree(drvdata->desc.supply_name);
-   kfree(drvdata->desc.name);
-
-   return 0;
 }
 
 #if defined(CONFIG_OF)
@@ -229,7 +212,6 @@ MODULE_DEVICE_TABLE(of, fixed_of_match);
 
 static struct platform_driver regulator_fixed_voltage_driver = {
.probe  = reg_fixed_voltage_probe,
-   .remove = reg_fixed_voltage_remove,
.driver = {
.name   = "reg-fixed-voltage",
.owner  = THIS_MODULE,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V3] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Instead of "#ifdef CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 b4513f2... 3e54476... M  drivers/power/max8925_power.c
 drivers/power/max8925_power.c |   17 +
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..3e54476 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct max8925_power_info 
*info)
return 0;
 }
 
-#ifdef CONFIG_OF
 static struct max8925_power_pdata *
 max8925_power_dt_init(struct platform_device *pdev)
 {
@@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
int no_insert_detect;
struct max8925_power_pdata *pdata;
 
-   if (!nproot)
-   return pdev->dev.platform_data;
-
np = of_find_node_by_name(nproot, "charger");
if (!np) {
dev_err(>dev, "failed to find charger node\n");
@@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 
return pdata;
 }
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
-   return pdev->dev.platform_data;
-}
-#endif
 
 static int max8925_power_probe(struct platform_device *pdev)
 {
@@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device 
*pdev)
struct max8925_power_info *info;
int ret;
 
-   pdata = max8925_power_dt_init(pdev);
+   pdata = dev_get_platdata(>dev);
+
+   if (!pdata && chip->dev->of_node)
+   pdata = max8925_power_dt_init(pdev);
+
if (!pdata) {
dev_err(>dev, "platform data isn't assigned to "
"power supply\n");
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V3] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Instead of #ifdef CONFIG_OF use IS_ENABLED(CONFIG_OF)
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 b4513f2... 3e54476... M  drivers/power/max8925_power.c
 drivers/power/max8925_power.c |   17 +
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..3e54476 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct max8925_power_info 
*info)
return 0;
 }
 
-#ifdef CONFIG_OF
 static struct max8925_power_pdata *
 max8925_power_dt_init(struct platform_device *pdev)
 {
@@ -440,9 +439,6 @@ max8925_power_dt_init(struct platform_device *pdev)
int no_insert_detect;
struct max8925_power_pdata *pdata;
 
-   if (!nproot)
-   return pdev-dev.platform_data;
-
np = of_find_node_by_name(nproot, charger);
if (!np) {
dev_err(pdev-dev, failed to find charger node\n);
@@ -468,13 +464,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 
return pdata;
 }
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
-   return pdev-dev.platform_data;
-}
-#endif
 
 static int max8925_power_probe(struct platform_device *pdev)
 {
@@ -483,7 +472,11 @@ static int max8925_power_probe(struct platform_device 
*pdev)
struct max8925_power_info *info;
int ret;
 
-   pdata = max8925_power_dt_init(pdev);
+   pdata = dev_get_platdata(pdev-dev);
+
+   if (!pdata  chip-dev-of_node)
+   pdata = max8925_power_dt_init(pdev);
+
if (!pdata) {
dev_err(pdev-dev, platform data isn't assigned to 
power supply\n);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Update the code to use devm_* API so that driver core will manage
resources.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
Changes since V1:
1. Updated driver to use devm_kzalloc to kstrdup.
2. Updated commit message.

Not tested on any board.

:100644 100644 5ea64b9... e9763a4... M  drivers/regulator/fixed.c
 drivers/regulator/fixed.c |   42 --
 1 file changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..e9763a4 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
   GFP_KERNEL);
if (drvdata == NULL) {
dev_err(pdev-dev, Failed to allocate device data\n);
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
 
-   drvdata-desc.name = kstrdup(config-supply_name, GFP_KERNEL);
+   drvdata-desc.name = devm_kzalloc(pdev-dev,
+ strlen(config-supply_name) + 1,
+ GFP_KERNEL);
if (drvdata-desc.name == NULL) {
dev_err(pdev-dev, Failed to allocate supply name\n);
-   ret = -ENOMEM;
-   goto err;
+   return -ENOMEM;
}
drvdata-desc.type = REGULATOR_VOLTAGE;
drvdata-desc.owner = THIS_MODULE;
@@ -149,13 +149,13 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata-desc.enable_time = config-startup_delay;
 
if (config-input_supply) {
-   drvdata-desc.supply_name = kstrdup(config-input_supply,
-   GFP_KERNEL);
+   drvdata-desc.supply_name = devm_kzalloc(pdev-dev,
+   strlen(config-input_supply) + 1,
+   GFP_KERNEL);
if (!drvdata-desc.supply_name) {
dev_err(pdev-dev,
Failed to allocate input supply\n);
-   ret = -ENOMEM;
-   goto err_name;
+   return -ENOMEM;
}
}
 
@@ -186,11 +186,12 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev-dev.of_node;
 
-   drvdata-dev = regulator_register(drvdata-desc, cfg);
+   drvdata-dev = devm_regulator_register(pdev-dev, drvdata-desc,
+  cfg);
if (IS_ERR(drvdata-dev)) {
ret = PTR_ERR(drvdata-dev);
dev_err(pdev-dev, Failed to register regulator: %d\n, ret);
-   goto err_input;
+   return ret;
}
 
platform_set_drvdata(pdev, drvdata);
@@ -199,24 +200,6 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
drvdata-desc.fixed_uV);
 
return 0;
-
-err_input:
-   kfree(drvdata-desc.supply_name);
-err_name:
-   kfree(drvdata-desc.name);
-err:
-   return ret;
-}
-
-static int reg_fixed_voltage_remove(struct platform_device *pdev)
-{
-   struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
-
-   regulator_unregister(drvdata-dev);
-   kfree(drvdata-desc.supply_name);
-   kfree(drvdata-desc.name);
-
-   return 0;
 }
 
 #if defined(CONFIG_OF)
@@ -229,7 +212,6 @@ MODULE_DEVICE_TABLE(of, fixed_of_match);
 
 static struct platform_driver regulator_fixed_voltage_driver = {
.probe  = reg_fixed_voltage_probe,
-   .remove = reg_fixed_voltage_remove,
.driver = {
.name   = reg-fixed-voltage,
.owner  = THIS_MODULE,
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V2] regulator: fixed: update to devm_* API

[Manish Badarkhe]

Hi Dmitry,

Thank you for your review.

On Tue, Jan 28, 2014 at 12:03 PM, Dmitry Torokhov
dmitry.torok...@gmail.com wrote:
 Hi Manish,

 On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
 Update the code to use devm_* API so that driver core will manage
 resources.

 Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
 ---
 Changes since V1:
 1. Updated driver to use devm_kzalloc to kstrdup.
 2. Updated commit message.

 Not tested on any board.

 :100644 100644 5ea64b9... e9763a4... Mdrivers/regulator/fixed.c
  drivers/regulator/fixed.c |   42 --
  1 file changed, 12 insertions(+), 30 deletions(-)

 diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
 index 5ea64b9..e9763a4 100644
 --- a/drivers/regulator/fixed.c
 +++ b/drivers/regulator/fixed.c
 @@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct 
 platform_device *pdev)
  GFP_KERNEL);
   if (drvdata == NULL) {
   dev_err(pdev-dev, Failed to allocate device data\n);
 - ret = -ENOMEM;
 - goto err;
 + return -ENOMEM;
   }

 - drvdata-desc.name = kstrdup(config-supply_name, GFP_KERNEL);
 + drvdata-desc.name = devm_kzalloc(pdev-dev,
 +   strlen(config-supply_name) + 1,
 +   GFP_KERNEL);
   if (drvdata-desc.name == NULL) {
   dev_err(pdev-dev, Failed to allocate supply name\n);
 - ret = -ENOMEM;
 - goto err;
 + return -ENOMEM;
   }

 Umm, I am fairly certain that devm_kzalloc() can't be used as a
 substitute for kstrdup, at least not without accompanying memcpy.

Yes, I have provided allocation but it should be followed with assignment.
Can I modify like this,

+ drvdata-desc.name = devm_kzalloc(pdev-dev,
+   strlen(config-supply_name) + 1,
+   GFP_KERNEL);
+ if (drvdata-desc.name)
+ sprintf(drvdata-desc.name, %s, config-supply_name);

Thanks
Manish Badakhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Hi

Thank you for review.

On Mon, Jan 27, 2014 at 5:16 AM, Dmitry Eremin-Solenikov
 wrote:
> On Mon, Jan 27, 2014 at 3:14 AM, Dmitry Torokhov
>  wrote:
>> On Mon, Jan 27, 2014 at 02:31:59AM +0400, Dmitry Eremin-Solenikov wrote:
>>> On Mon, Jan 27, 2014 at 1:49 AM, Tomasz Figa  wrote:
>>> > On 26.01.2014 22:45, Dmitry Torokhov wrote:
>>> >>
>>> >> On Sun, Jan 26, 2014 at 07:31:50PM +0530, Manish Badarkhe wrote:
>>> >>>
>>> >>> Hi Tomasz,
>>> >>>
>>> >>> Thank you for your review comments.
>>> >>>
>>> >>> On Sun, Jan 26, 2014 at 6:52 PM, Tomasz Figa 
>>> >>> wrote:
>>> >>>>
>>> >>>>
>>> >>>> Hi Manish,
>>> >>>>
>>> >>>>
>>> >>>> On 26.01.2014 08:15, Manish Badarkhe wrote:
>>> >>>>>
>>> >>>>>
>>> >>>>> Instead of "#if define CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
>>> >>>>> option for DT code to avoid if-deffery in code.
>>> >>>>>
>>> >>>>> Signed-off-by: Manish Badarkhe 
>>> >>>>> ---
>>> >>>>> :100644 100644 b4513f2... d353fbc... M  drivers/power/max8925_power.c
>>> >>>>>drivers/power/max8925_power.c |   14 +-
>>> >>>>>1 file changed, 5 insertions(+), 9 deletions(-)
>>> >>>>>
>>> >>>>> diff --git a/drivers/power/max8925_power.c
>>> >>>>> b/drivers/power/max8925_power.c
>>> >>>>> index b4513f2..d353fbc 100644
>>> >>>>> --- a/drivers/power/max8925_power.c
>>> >>>>> +++ b/drivers/power/max8925_power.c
>>> >>>>> @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct
>>> >>>>> max8925_power_info *info)
>>> >>>>>  return 0;
>>> >>>>>}
>>> >>>>>
>>> >>>>> -#ifdef CONFIG_OF
>>> >>>>>static struct max8925_power_pdata *
>>> >>>>>max8925_power_dt_init(struct platform_device *pdev)
>>> >>>>>{
>>> >>>>> @@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device
>>> >>>>> *pdev)
>>> >>>>>
>>> >>>>>  return pdata;
>>> >>>>>}
>>> >>>>> -#else
>>> >>>>> -static struct max8925_power_pdata *
>>> >>>>> -max8925_power_dt_init(struct platform_device *pdev)
>>> >>>>> -{
>>> >>>>> -   return pdev->dev.platform_data;
>>> >>>>> -}
>>> >>>>> -#endif
>>> >>>>>
>>> >>>>>static int max8925_power_probe(struct platform_device *pdev)
>>> >>>>>{
>>> >>>>> @@ -483,7 +475,11 @@ static int max8925_power_probe(struct
>>> >>>>> platform_device *pdev)
>>> >>>>>  struct max8925_power_info *info;
>>> >>>>>  int ret;
>>> >>>>>
>>> >>>>> -   pdata = max8925_power_dt_init(pdev);
>>> >>>>> +   if (IS_ENABLED(CONFIG_OF))
>>> >>>>> +   pdata = max8925_power_dt_init(pdev);
>>> >>>>> +   else
>>> >>>>> +   pdata = pdev->dev.platform_data;
>>> >>>>> +
>>> >>>>
>>> >>>>
>>> >>>>
>>> >>>> This does not look much better than before this patch. Instead of
>>> >>>> "if-deffery" outside function bodies you are adding "iffery" (if there 
>>> >>>> is
>>> >>>> such a word) inside a function.
>>> >>>> What about adding
>>> >>>>
>>> >>>>  if (!IS_ENABLED(CONFIG_OF))
>>> >>>>  return pdev->dev.platform_data;
>>> >>>>
>>> >>>> on top of max8925_power_dt_init() instead or maybe also renaming this
>>> >>>> function to max8925_get_pdata()?
>>> >>>
>>> >>>
>>> >>> Okay, I will rename function "max8925_power_dt_init()" to
>>> >>> "max8925_get_pdata()".
>>> >>> As you suggested, in the body of this function  will add a logic to
>>> >>> retrieve  data in case
>>> >>> of DT and non-DT platforms.
>>> >>
>>> >>
>>> >> Should we not always favor platform-supplied data regardless of
>>> >> CONFIG_OF state and fall back to DT (firmware) supplied data if platform
>>> >> data is absent? This way one can tweak kernel behavior without needing
>>> >> to change firmware.
>>> >
>>> >
>>> > I guess we should, but apparently this was not the original behavior 
>>> > before
>>> > this patch, so I'm not sure if we should be squashing such semantic change
>>> > with this simple refactor.
>>>
>>> Hmm. Judging from the code, max8925_power_dt_init() function follows exactly
>>> opposite strategy - it uses platform_data if of_node is not 
>>> populated/available.
>>> So (if dt_init will compile with CONFIG_OF disabled) one can always
>>> use _dt_init()
>>> function to retrieve pdata.
>>
>> Right, and I question whether this is good behavior or if it should be
>> corrected.
>
> I'd say, correct it.

Okay, I will update code as per Dmitry's feedback.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] regulator: fixed: Use devm_regulator_register

[Manish Badarkhe]

Hi

On Mon, Jan 27, 2014 at 5:33 AM, Mark Brown  wrote:
> On Sun, Jan 26, 2014 at 01:36:53PM -0800, Dmitry Torokhov wrote:
>> On Sat, Jan 25, 2014 at 11:35:54PM +0530, Manish Badarkhe wrote:
>
>> > Use "devm_regulator_register" instead of "regulator_register"
>> > which simplifies the code.
>
>> ... and also breaks the driver: now you are freeing desc->name and
>> desc->supply_name while regulator structures are still alive and can be
>> referenced from its sysfs attributes, etc.
>
> Yup, they need to be converted to managed allocations too if the
> regulator_register() is going to be converted (see previous discussions
> on similar patches).  It would be good to do so.

Thank you for pointing this out.
I missed to convert these two " desc->name" and "desc->supply_name"
to manage allocation. I will modify code and repost patch.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Instead of "#ifdef CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe 
---
Changes since V1:
1. update code to retrieve platform/dt data in same function

:100644 100644 b4513f2... 20a7100... M  drivers/power/max8925_power.c
 drivers/power/max8925_power.c |   15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..20a7100 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,9 +427,8 @@ static int max8925_deinit_charger(struct max8925_power_info 
*info)
return 0;
 }
 
-#ifdef CONFIG_OF
 static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
+max8925_get_data(struct platform_device *pdev)
 {
struct device_node *nproot = pdev->dev.parent->of_node;
struct device_node *np;
@@ -440,7 +439,7 @@ max8925_power_dt_init(struct platform_device *pdev)
int no_insert_detect;
struct max8925_power_pdata *pdata;
 
-   if (!nproot)
+   if (!IS_ENABLED(CONFIG_OF) || !nproot)
return pdev->dev.platform_data;
 
np = of_find_node_by_name(nproot, "charger");
@@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 
return pdata;
 }
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
-   return pdev->dev.platform_data;
-}
-#endif
 
 static int max8925_power_probe(struct platform_device *pdev)
 {
@@ -483,7 +475,8 @@ static int max8925_power_probe(struct platform_device *pdev)
struct max8925_power_info *info;
int ret;
 
-   pdata = max8925_power_dt_init(pdev);
+   pdata = max8925_get_data(pdev);
+
if (!pdata) {
dev_err(>dev, "platform data isn't assigned to "
"power supply\n");
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Hi Tomasz,

Thank you for your review comments.

On Sun, Jan 26, 2014 at 6:52 PM, Tomasz Figa  wrote:
>
> Hi Manish,
>
>
> On 26.01.2014 08:15, Manish Badarkhe wrote:
>>
>> Instead of "#if define CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
>> option for DT code to avoid if-deffery in code.
>>
>> Signed-off-by: Manish Badarkhe 
>> ---
>> :100644 100644 b4513f2... d353fbc... M  drivers/power/max8925_power.c
>>   drivers/power/max8925_power.c |   14 +-
>>   1 file changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
>> index b4513f2..d353fbc 100644
>> --- a/drivers/power/max8925_power.c
>> +++ b/drivers/power/max8925_power.c
>> @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct 
>> max8925_power_info *info)
>> return 0;
>>   }
>>
>> -#ifdef CONFIG_OF
>>   static struct max8925_power_pdata *
>>   max8925_power_dt_init(struct platform_device *pdev)
>>   {
>> @@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device *pdev)
>>
>> return pdata;
>>   }
>> -#else
>> -static struct max8925_power_pdata *
>> -max8925_power_dt_init(struct platform_device *pdev)
>> -{
>> -   return pdev->dev.platform_data;
>> -}
>> -#endif
>>
>>   static int max8925_power_probe(struct platform_device *pdev)
>>   {
>> @@ -483,7 +475,11 @@ static int max8925_power_probe(struct platform_device 
>> *pdev)
>> struct max8925_power_info *info;
>> int ret;
>>
>> -   pdata = max8925_power_dt_init(pdev);
>> +   if (IS_ENABLED(CONFIG_OF))
>> +   pdata = max8925_power_dt_init(pdev);
>> +   else
>> +   pdata = pdev->dev.platform_data;
>> +
>
>
> This does not look much better than before this patch. Instead of 
> "if-deffery" outside function bodies you are adding "iffery" (if there is 
> such a word) inside a function.
> What about adding
>
> if (!IS_ENABLED(CONFIG_OF))
> return pdev->dev.platform_data;
>
> on top of max8925_power_dt_init() instead or maybe also renaming this 
> function to max8925_get_pdata()?

Okay, I will rename function "max8925_power_dt_init()" to "max8925_get_pdata()".
As you suggested, in the body of this function  will add a logic to
retrieve  data in case
of DT and non-DT platforms.


Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Hi Tomasz,

Thank you for your review comments.

On Sun, Jan 26, 2014 at 6:52 PM, Tomasz Figa tomasz.f...@gmail.com wrote:

 Hi Manish,


 On 26.01.2014 08:15, Manish Badarkhe wrote:

 Instead of #if define CONFIG_OF use IS_ENABLED(CONFIG_OF)
 option for DT code to avoid if-deffery in code.

 Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
 ---
 :100644 100644 b4513f2... d353fbc... M  drivers/power/max8925_power.c
   drivers/power/max8925_power.c |   14 +-
   1 file changed, 5 insertions(+), 9 deletions(-)

 diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
 index b4513f2..d353fbc 100644
 --- a/drivers/power/max8925_power.c
 +++ b/drivers/power/max8925_power.c
 @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct 
 max8925_power_info *info)
 return 0;
   }

 -#ifdef CONFIG_OF
   static struct max8925_power_pdata *
   max8925_power_dt_init(struct platform_device *pdev)
   {
 @@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device *pdev)

 return pdata;
   }
 -#else
 -static struct max8925_power_pdata *
 -max8925_power_dt_init(struct platform_device *pdev)
 -{
 -   return pdev-dev.platform_data;
 -}
 -#endif

   static int max8925_power_probe(struct platform_device *pdev)
   {
 @@ -483,7 +475,11 @@ static int max8925_power_probe(struct platform_device 
 *pdev)
 struct max8925_power_info *info;
 int ret;

 -   pdata = max8925_power_dt_init(pdev);
 +   if (IS_ENABLED(CONFIG_OF))
 +   pdata = max8925_power_dt_init(pdev);
 +   else
 +   pdata = pdev-dev.platform_data;
 +


 This does not look much better than before this patch. Instead of 
 if-deffery outside function bodies you are adding iffery (if there is 
 such a word) inside a function.
 What about adding

 if (!IS_ENABLED(CONFIG_OF))
 return pdev-dev.platform_data;

 on top of max8925_power_dt_init() instead or maybe also renaming this 
 function to max8925_get_pdata()?

Okay, I will rename function max8925_power_dt_init() to max8925_get_pdata().
As you suggested, in the body of this function  will add a logic to
retrieve  data in case
of DT and non-DT platforms.


Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V2] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Instead of #ifdef CONFIG_OF use IS_ENABLED(CONFIG_OF)
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
Changes since V1:
1. update code to retrieve platform/dt data in same function

:100644 100644 b4513f2... 20a7100... M  drivers/power/max8925_power.c
 drivers/power/max8925_power.c |   15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..20a7100 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,9 +427,8 @@ static int max8925_deinit_charger(struct max8925_power_info 
*info)
return 0;
 }
 
-#ifdef CONFIG_OF
 static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
+max8925_get_data(struct platform_device *pdev)
 {
struct device_node *nproot = pdev-dev.parent-of_node;
struct device_node *np;
@@ -440,7 +439,7 @@ max8925_power_dt_init(struct platform_device *pdev)
int no_insert_detect;
struct max8925_power_pdata *pdata;
 
-   if (!nproot)
+   if (!IS_ENABLED(CONFIG_OF) || !nproot)
return pdev-dev.platform_data;
 
np = of_find_node_by_name(nproot, charger);
@@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 
return pdata;
 }
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
-   return pdev-dev.platform_data;
-}
-#endif
 
 static int max8925_power_probe(struct platform_device *pdev)
 {
@@ -483,7 +475,8 @@ static int max8925_power_probe(struct platform_device *pdev)
struct max8925_power_info *info;
int ret;
 
-   pdata = max8925_power_dt_init(pdev);
+   pdata = max8925_get_data(pdev);
+
if (!pdata) {
dev_err(pdev-dev, platform data isn't assigned to 
power supply\n);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] regulator: fixed: Use devm_regulator_register

[Manish Badarkhe]

Hi

On Mon, Jan 27, 2014 at 5:33 AM, Mark Brown broo...@kernel.org wrote:
 On Sun, Jan 26, 2014 at 01:36:53PM -0800, Dmitry Torokhov wrote:
 On Sat, Jan 25, 2014 at 11:35:54PM +0530, Manish Badarkhe wrote:

  Use devm_regulator_register instead of regulator_register
  which simplifies the code.

 ... and also breaks the driver: now you are freeing desc-name and
 desc-supply_name while regulator structures are still alive and can be
 referenced from its sysfs attributes, etc.

 Yup, they need to be converted to managed allocations too if the
 regulator_register() is going to be converted (see previous discussions
 on similar patches).  It would be good to do so.

Thank you for pointing this out.
I missed to convert these two  desc-name and desc-supply_name
to manage allocation. I will modify code and repost patch.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Hi

Thank you for review.

On Mon, Jan 27, 2014 at 5:16 AM, Dmitry Eremin-Solenikov
dbarysh...@gmail.com wrote:
 On Mon, Jan 27, 2014 at 3:14 AM, Dmitry Torokhov
 dmitry.torok...@gmail.com wrote:
 On Mon, Jan 27, 2014 at 02:31:59AM +0400, Dmitry Eremin-Solenikov wrote:
 On Mon, Jan 27, 2014 at 1:49 AM, Tomasz Figa tomasz.f...@gmail.com wrote:
  On 26.01.2014 22:45, Dmitry Torokhov wrote:
 
  On Sun, Jan 26, 2014 at 07:31:50PM +0530, Manish Badarkhe wrote:
 
  Hi Tomasz,
 
  Thank you for your review comments.
 
  On Sun, Jan 26, 2014 at 6:52 PM, Tomasz Figa tomasz.f...@gmail.com
  wrote:
 
 
  Hi Manish,
 
 
  On 26.01.2014 08:15, Manish Badarkhe wrote:
 
 
  Instead of #if define CONFIG_OF use IS_ENABLED(CONFIG_OF)
  option for DT code to avoid if-deffery in code.
 
  Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
  ---
  :100644 100644 b4513f2... d353fbc... M  drivers/power/max8925_power.c
 drivers/power/max8925_power.c |   14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)
 
  diff --git a/drivers/power/max8925_power.c
  b/drivers/power/max8925_power.c
  index b4513f2..d353fbc 100644
  --- a/drivers/power/max8925_power.c
  +++ b/drivers/power/max8925_power.c
  @@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct
  max8925_power_info *info)
   return 0;
 }
 
  -#ifdef CONFIG_OF
 static struct max8925_power_pdata *
 max8925_power_dt_init(struct platform_device *pdev)
 {
  @@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device
  *pdev)
 
   return pdata;
 }
  -#else
  -static struct max8925_power_pdata *
  -max8925_power_dt_init(struct platform_device *pdev)
  -{
  -   return pdev-dev.platform_data;
  -}
  -#endif
 
 static int max8925_power_probe(struct platform_device *pdev)
 {
  @@ -483,7 +475,11 @@ static int max8925_power_probe(struct
  platform_device *pdev)
   struct max8925_power_info *info;
   int ret;
 
  -   pdata = max8925_power_dt_init(pdev);
  +   if (IS_ENABLED(CONFIG_OF))
  +   pdata = max8925_power_dt_init(pdev);
  +   else
  +   pdata = pdev-dev.platform_data;
  +
 
 
 
  This does not look much better than before this patch. Instead of
  if-deffery outside function bodies you are adding iffery (if there 
  is
  such a word) inside a function.
  What about adding
 
   if (!IS_ENABLED(CONFIG_OF))
   return pdev-dev.platform_data;
 
  on top of max8925_power_dt_init() instead or maybe also renaming this
  function to max8925_get_pdata()?
 
 
  Okay, I will rename function max8925_power_dt_init() to
  max8925_get_pdata().
  As you suggested, in the body of this function  will add a logic to
  retrieve  data in case
  of DT and non-DT platforms.
 
 
  Should we not always favor platform-supplied data regardless of
  CONFIG_OF state and fall back to DT (firmware) supplied data if platform
  data is absent? This way one can tweak kernel behavior without needing
  to change firmware.
 
 
  I guess we should, but apparently this was not the original behavior 
  before
  this patch, so I'm not sure if we should be squashing such semantic change
  with this simple refactor.

 Hmm. Judging from the code, max8925_power_dt_init() function follows exactly
 opposite strategy - it uses platform_data if of_node is not 
 populated/available.
 So (if dt_init will compile with CONFIG_OF disabled) one can always
 use _dt_init()
 function to retrieve pdata.

 Right, and I question whether this is good behavior or if it should be
 corrected.

 I'd say, correct it.

Okay, I will update code as per Dmitry's feedback.

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] max8925_power: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Instead of "#if define CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 b4513f2... d353fbc... M  drivers/power/max8925_power.c
 drivers/power/max8925_power.c |   14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..d353fbc 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct max8925_power_info 
*info)
return 0;
 }
 
-#ifdef CONFIG_OF
 static struct max8925_power_pdata *
 max8925_power_dt_init(struct platform_device *pdev)
 {
@@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 
return pdata;
 }
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
-   return pdev->dev.platform_data;
-}
-#endif
 
 static int max8925_power_probe(struct platform_device *pdev)
 {
@@ -483,7 +475,11 @@ static int max8925_power_probe(struct platform_device 
*pdev)
struct max8925_power_info *info;
int ret;
 
-   pdata = max8925_power_dt_init(pdev);
+   if (IS_ENABLED(CONFIG_OF))
+   pdata = max8925_power_dt_init(pdev);
+   else
+   pdata = pdev->dev.platform_data;
+
if (!pdata) {
dev_err(>dev, "platform data isn't assigned to "
"power supply\n");
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] regulator: fixed: Use devm_regulator_register

[Manish Badarkhe]

Use "devm_regulator_register" instead of "regulator_register"
which simplifies the code.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 5ea64b9... 6d32341... M  drivers/regulator/fixed.c
 drivers/regulator/fixed.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..6d32341 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -186,7 +186,8 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev->dev.of_node;
 
-   drvdata->dev = regulator_register(>desc, );
+   drvdata->dev = devm_regulator_register(>dev, >desc,
+  );
if (IS_ERR(drvdata->dev)) {
ret = PTR_ERR(drvdata->dev);
dev_err(>dev, "Failed to register regulator: %d\n", ret);
@@ -212,7 +213,6 @@ static int reg_fixed_voltage_remove(struct platform_device 
*pdev)
 {
struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
 
-   regulator_unregister(drvdata->dev);
kfree(drvdata->desc.supply_name);
kfree(drvdata->desc.name);
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] regulator: fixed: Use devm_regulator_register

[Manish Badarkhe]

Use devm_regulator_register instead of regulator_register
which simplifies the code.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 5ea64b9... 6d32341... M  drivers/regulator/fixed.c
 drivers/regulator/fixed.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..6d32341 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -186,7 +186,8 @@ static int reg_fixed_voltage_probe(struct platform_device 
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev-dev.of_node;
 
-   drvdata-dev = regulator_register(drvdata-desc, cfg);
+   drvdata-dev = devm_regulator_register(pdev-dev, drvdata-desc,
+  cfg);
if (IS_ERR(drvdata-dev)) {
ret = PTR_ERR(drvdata-dev);
dev_err(pdev-dev, Failed to register regulator: %d\n, ret);
@@ -212,7 +213,6 @@ static int reg_fixed_voltage_remove(struct platform_device 
*pdev)
 {
struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
 
-   regulator_unregister(drvdata-dev);
kfree(drvdata-desc.supply_name);
kfree(drvdata-desc.name);
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Instead of #if define CONFIG_OF use IS_ENABLED(CONFIG_OF)
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 b4513f2... d353fbc... M  drivers/power/max8925_power.c
 drivers/power/max8925_power.c |   14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..d353fbc 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct max8925_power_info 
*info)
return 0;
 }
 
-#ifdef CONFIG_OF
 static struct max8925_power_pdata *
 max8925_power_dt_init(struct platform_device *pdev)
 {
@@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device *pdev)
 
return pdata;
 }
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
-   return pdev-dev.platform_data;
-}
-#endif
 
 static int max8925_power_probe(struct platform_device *pdev)
 {
@@ -483,7 +475,11 @@ static int max8925_power_probe(struct platform_device 
*pdev)
struct max8925_power_info *info;
int ret;
 
-   pdata = max8925_power_dt_init(pdev);
+   if (IS_ENABLED(CONFIG_OF))
+   pdata = max8925_power_dt_init(pdev);
+   else
+   pdata = pdev-dev.platform_data;
+
if (!pdata) {
dev_err(pdev-dev, platform data isn't assigned to 
power supply\n);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] mfd: max8997: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Hi Samuel,

On Sun, Dec 22, 2013 at 11:18 PM, Manish Badarkhe
 wrote:
>
> Instead of "#if define CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
> option for DT code to avoid if-deffery in code.
>
> Signed-off-by: Manish Badarkhe 
> ---
> :100644 100644 791aea3... be9a8b0... M  drivers/mfd/max8997.c
>  drivers/mfd/max8997.c |   14 ++
>  1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
> index 791aea3..be9a8b0 100644
> --- a/drivers/mfd/max8997.c
> +++ b/drivers/mfd/max8997.c
> @@ -133,7 +133,6 @@ int max8997_update_reg(struct i2c_client *i2c, u8 reg, u8 
> val, u8 mask)
>  }
>  EXPORT_SYMBOL_GPL(max8997_update_reg);
>
> -#ifdef CONFIG_OF
>  /*
>   * Only the common platform data elements for max8997 are parsed here from 
> the
>   * device tree. Other sub-modules of max8997 such as pmic, rtc and others 
> have
> @@ -164,24 +163,15 @@ static struct max8997_platform_data 
> *max8997_i2c_parse_dt_pdata(
>
> return pd;
>  }
> -#else
> -static struct max8997_platform_data *max8997_i2c_parse_dt_pdata(
> -   struct device *dev)
> -{
> -   return 0;
> -}
> -#endif
>
>  static inline int max8997_i2c_get_driver_data(struct i2c_client *i2c,
> const struct i2c_device_id 
> *id)
>  {
> -#ifdef CONFIG_OF
> -   if (i2c->dev.of_node) {
> +   if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node) {
> const struct of_device_id *match;
> match = of_match_node(max8997_pmic_dt_match, 
> i2c->dev.of_node);
> return (int)match->data;
> }
> -#endif
> return (int)id->driver_data;
>  }
>
> @@ -203,7 +193,7 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
> max8997->type = max8997_i2c_get_driver_data(i2c, id);
> max8997->irq = i2c->irq;
>
> -   if (max8997->dev->of_node) {
> +   if (IS_ENABLED(CONFIG_OF) && max8997->dev->of_node) {
> pdata = max8997_i2c_parse_dt_pdata(max8997->dev);
> if (IS_ERR(pdata))
> return PTR_ERR(pdata);

Gentle ping.
Please let me know, Are there any review comments on this patch?

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] mfd: max8997: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Hi Samuel,

On Sun, Dec 22, 2013 at 11:18 PM, Manish Badarkhe
badarkhe.man...@gmail.com wrote:

 Instead of #if define CONFIG_OF use IS_ENABLED(CONFIG_OF)
 option for DT code to avoid if-deffery in code.

 Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
 ---
 :100644 100644 791aea3... be9a8b0... M  drivers/mfd/max8997.c
  drivers/mfd/max8997.c |   14 ++
  1 file changed, 2 insertions(+), 12 deletions(-)

 diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
 index 791aea3..be9a8b0 100644
 --- a/drivers/mfd/max8997.c
 +++ b/drivers/mfd/max8997.c
 @@ -133,7 +133,6 @@ int max8997_update_reg(struct i2c_client *i2c, u8 reg, u8 
 val, u8 mask)
  }
  EXPORT_SYMBOL_GPL(max8997_update_reg);

 -#ifdef CONFIG_OF
  /*
   * Only the common platform data elements for max8997 are parsed here from 
 the
   * device tree. Other sub-modules of max8997 such as pmic, rtc and others 
 have
 @@ -164,24 +163,15 @@ static struct max8997_platform_data 
 *max8997_i2c_parse_dt_pdata(

 return pd;
  }
 -#else
 -static struct max8997_platform_data *max8997_i2c_parse_dt_pdata(
 -   struct device *dev)
 -{
 -   return 0;
 -}
 -#endif

  static inline int max8997_i2c_get_driver_data(struct i2c_client *i2c,
 const struct i2c_device_id 
 *id)
  {
 -#ifdef CONFIG_OF
 -   if (i2c-dev.of_node) {
 +   if (IS_ENABLED(CONFIG_OF)  i2c-dev.of_node) {
 const struct of_device_id *match;
 match = of_match_node(max8997_pmic_dt_match, 
 i2c-dev.of_node);
 return (int)match-data;
 }
 -#endif
 return (int)id-driver_data;
  }

 @@ -203,7 +193,7 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
 max8997-type = max8997_i2c_get_driver_data(i2c, id);
 max8997-irq = i2c-irq;

 -   if (max8997-dev-of_node) {
 +   if (IS_ENABLED(CONFIG_OF)  max8997-dev-of_node) {
 pdata = max8997_i2c_parse_dt_pdata(max8997-dev);
 if (IS_ERR(pdata))
 return PTR_ERR(pdata);

Gentle ping.
Please let me know, Are there any review comments on this patch?

Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] mfd: max8997: Use "IS_ENABLED(CONFIG_OF)" for DT code.

[Manish Badarkhe]

Instead of "#if define CONFIG_OF" use "IS_ENABLED(CONFIG_OF)"
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe 
---
:100644 100644 791aea3... be9a8b0... M  drivers/mfd/max8997.c
 drivers/mfd/max8997.c |   14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index 791aea3..be9a8b0 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -133,7 +133,6 @@ int max8997_update_reg(struct i2c_client *i2c, u8 reg, u8 
val, u8 mask)
 }
 EXPORT_SYMBOL_GPL(max8997_update_reg);
 
-#ifdef CONFIG_OF
 /*
  * Only the common platform data elements for max8997 are parsed here from the
  * device tree. Other sub-modules of max8997 such as pmic, rtc and others have
@@ -164,24 +163,15 @@ static struct max8997_platform_data 
*max8997_i2c_parse_dt_pdata(
 
return pd;
 }
-#else
-static struct max8997_platform_data *max8997_i2c_parse_dt_pdata(
-   struct device *dev)
-{
-   return 0;
-}
-#endif
 
 static inline int max8997_i2c_get_driver_data(struct i2c_client *i2c,
const struct i2c_device_id *id)
 {
-#ifdef CONFIG_OF
-   if (i2c->dev.of_node) {
+   if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node) {
const struct of_device_id *match;
match = of_match_node(max8997_pmic_dt_match, i2c->dev.of_node);
return (int)match->data;
}
-#endif
return (int)id->driver_data;
 }
 
@@ -203,7 +193,7 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
max8997->type = max8997_i2c_get_driver_data(i2c, id);
max8997->irq = i2c->irq;
 
-   if (max8997->dev->of_node) {
+   if (IS_ENABLED(CONFIG_OF) && max8997->dev->of_node) {
pdata = max8997_i2c_parse_dt_pdata(max8997->dev);
if (IS_ERR(pdata))
return PTR_ERR(pdata);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] mfd: max8997: Use IS_ENABLED(CONFIG_OF) for DT code.

[Manish Badarkhe]

Instead of #if define CONFIG_OF use IS_ENABLED(CONFIG_OF)
option for DT code to avoid if-deffery in code.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 791aea3... be9a8b0... M  drivers/mfd/max8997.c
 drivers/mfd/max8997.c |   14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/mfd/max8997.c b/drivers/mfd/max8997.c
index 791aea3..be9a8b0 100644
--- a/drivers/mfd/max8997.c
+++ b/drivers/mfd/max8997.c
@@ -133,7 +133,6 @@ int max8997_update_reg(struct i2c_client *i2c, u8 reg, u8 
val, u8 mask)
 }
 EXPORT_SYMBOL_GPL(max8997_update_reg);
 
-#ifdef CONFIG_OF
 /*
  * Only the common platform data elements for max8997 are parsed here from the
  * device tree. Other sub-modules of max8997 such as pmic, rtc and others have
@@ -164,24 +163,15 @@ static struct max8997_platform_data 
*max8997_i2c_parse_dt_pdata(
 
return pd;
 }
-#else
-static struct max8997_platform_data *max8997_i2c_parse_dt_pdata(
-   struct device *dev)
-{
-   return 0;
-}
-#endif
 
 static inline int max8997_i2c_get_driver_data(struct i2c_client *i2c,
const struct i2c_device_id *id)
 {
-#ifdef CONFIG_OF
-   if (i2c-dev.of_node) {
+   if (IS_ENABLED(CONFIG_OF)  i2c-dev.of_node) {
const struct of_device_id *match;
match = of_match_node(max8997_pmic_dt_match, i2c-dev.of_node);
return (int)match-data;
}
-#endif
return (int)id-driver_data;
 }
 
@@ -203,7 +193,7 @@ static int max8997_i2c_probe(struct i2c_client *i2c,
max8997-type = max8997_i2c_get_driver_data(i2c, id);
max8997-irq = i2c-irq;
 
-   if (max8997-dev-of_node) {
+   if (IS_ENABLED(CONFIG_OF)  max8997-dev-of_node) {
pdata = max8997_i2c_parse_dt_pdata(max8997-dev);
if (IS_ERR(pdata))
return PTR_ERR(pdata);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC

[Manish Badarkhe]

Hi Keerthy,

> +   rdev = regulator_register([id], );

Can you make use of "devm_regulator_register" instead?

> +   if (IS_ERR(rdev)) {
> +   dev_err(tps->dev, "failed to register %s regulator\n",
> +   pdev->name);
> +   return PTR_ERR(rdev);
> +   }
> +
> +   /* Save regulator */
> +   tps->rdev[id] = rdev;
> +
> +   return 0;
> +}


Best Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC

[Manish Badarkhe]

Hi Keerthy,

 +   rdev = regulator_register(regulators[id], config);

Can you make use of devm_regulator_register instead?

 +   if (IS_ERR(rdev)) {
 +   dev_err(tps-dev, failed to register %s regulator\n,
 +   pdev-name);
 +   return PTR_ERR(rdev);
 +   }
 +
 +   /* Save regulator */
 +   tps-rdev[id] = rdev;
 +
 +   return 0;
 +}


Best Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH V4 1/2] tps6507x-ts: Add DT support

[Manish Badarkhe]

Hi Mark,

Apologize for my late reply.


On Thu, Oct 24, 2013 at 10:57 PM, Mark Rutland  wrote:
>
> On Thu, Oct 24, 2013 at 06:05:53PM +0100, Manish Badarkhe wrote:
> > Hi Mark,
> >
> > Thank you for your reply.
> >
> > On Wed, Oct 23, 2013 at 10:15 PM, Mark Rutland  wrote:
> >
> > On Wed, Oct 23, 2013 at 05:28:52PM +0100, Manish Badarkhe wrote:
> > > Add device tree based support for TI's tps6507x touchscreen.
> > >
> > > Signed-off-by: Manish Badarkhe 
> > > ---
> > > Changes since V3:
> > >  - Rebased on top of Dmitry's changes
> > >  - Removed error handling for optional DT properties
> > >
> > > Changes since V2:
> > >  - Removed unnecessary code.
> > >  - Updated Documentation to provide proper names of
> > >devicetree properties.
> > >
> > > Changes since V1:
> > >  - Updated documentation to specify tps6507x as multifunctional
> > >device.
> > >  - return proper error value in absence of platform and DT
> > >data for touchscreen.
> > >  - Updated commit message.
> > >
> > > :100755 100755 8fffa3c... e1b9cfd... M
> > Documentation/devicetree/
> > bindings/mfd/tps6507x.txt
> > > :100644 100644 db604e0... 0cf0eb1... M
> > drivers/input/touchscreen/
> > tps6507x-ts.c
> > >  Documentation/devicetree/bindings/mfd/tps6507x.txt |   24 ++-
> > >  drivers/input/touchscreen/tps6507x-ts.c|   72
> > 
> > >  2 files changed, 67 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/
> > Documentation/devicetree/bindings/mfd/tps6507x.txt
> > > index 8fffa3c..e1b9cfd 100755
> > > --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> > > +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> > > @@ -1,4 +1,8 @@
> > > -TPS6507x Power Management Integrated Circuit
> > > +TPS6507x Multifunctional Device.
> > > +
> > > +Features provided by TPS6507x:
> > > +1.Power Management Integrated Circuit.
> > > +2.Touch-Screen.
> > >
> > >  Required properties:
> > >  - compatible: "ti,tps6507x"
> > > @@ -23,6 +27,9 @@ Required properties:
> > > vindcdc1_2-supply: VDCDC1 and VDCDC2 input.
> > > vindcdc3-supply  : VDCDC3 input.
> > > vldo1_2-supply   : VLDO1 and VLDO2 input.
> > > +- tsc: This node specifies touch screen data.
> >
> > This is a node, but is listed un "Required properties". That seems odd.
> >
> > When is it required?
> >
> > Why is the data under the tsc subnode? Why not directly under the main
> > node?
> >
> >
> > Yes, It seems odd to add a subnode properties here. I gone through other
> > documentation
> > for MFD devices and observed that separate documentation file is present for
> > sub node modules.
>
> I was trying to say that nodes != properties rather than that this should be
> split into separate files.
>

Ok, got it.I will modify documentation so that only properties comes
under "required
properties" section.

>
> >
> > TPS6507x is multifunctional device and having touch screen submodule. As it 
> > is
> > child node for
> > TPS6507x multifunctional device, I have created child node as "tsc".
> >
> >
> >
> > > + ti,poll-period : Time at which touch input is getting sampled in
> > ms.
> >
> > Is this for debounce? Other bindings have similar but differently named
> > properties.
> >
> >
> > This is not debounce time. This time is required for input poll devices.
> >
> >
> > Why is this necessary? Can the driver not choose a sane value?
> >
> >
> > poll-period is necessary to sample touch inputs. Driver will chose value
> > default poll
> > period if this value is not present. I was bit confused here, whether to add
> > this property
> > as optional or required. As with default period touch not achieve 
> > performance.
> > Can I make this property optional?
>
> Please elaborate. Why will the default period be sub-optimal? What's the
> tradeoff here?

Currently, default period and period passed from Device tree b

Re: [PATCH V4 1/2] tps6507x-ts: Add DT support

[Manish Badarkhe]

Hi Mark,

Apologize for my late reply.


On Thu, Oct 24, 2013 at 10:57 PM, Mark Rutland mark.rutl...@arm.com wrote:

 On Thu, Oct 24, 2013 at 06:05:53PM +0100, Manish Badarkhe wrote:
  Hi Mark,
 
  Thank you for your reply.
 
  On Wed, Oct 23, 2013 at 10:15 PM, Mark Rutland mark.rutl...@arm.com wrote:
 
  On Wed, Oct 23, 2013 at 05:28:52PM +0100, Manish Badarkhe wrote:
   Add device tree based support for TI's tps6507x touchscreen.
  
   Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
   ---
   Changes since V3:
- Rebased on top of Dmitry's changes
- Removed error handling for optional DT properties
  
   Changes since V2:
- Removed unnecessary code.
- Updated Documentation to provide proper names of
  devicetree properties.
  
   Changes since V1:
- Updated documentation to specify tps6507x as multifunctional
  device.
- return proper error value in absence of platform and DT
  data for touchscreen.
- Updated commit message.
  
   :100755 100755 8fffa3c... e1b9cfd... M
  Documentation/devicetree/
  bindings/mfd/tps6507x.txt
   :100644 100644 db604e0... 0cf0eb1... M
  drivers/input/touchscreen/
  tps6507x-ts.c
Documentation/devicetree/bindings/mfd/tps6507x.txt |   24 ++-
drivers/input/touchscreen/tps6507x-ts.c|   72
  
2 files changed, 67 insertions(+), 29 deletions(-)
  
   diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/
  Documentation/devicetree/bindings/mfd/tps6507x.txt
   index 8fffa3c..e1b9cfd 100755
   --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
   +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
   @@ -1,4 +1,8 @@
   -TPS6507x Power Management Integrated Circuit
   +TPS6507x Multifunctional Device.
   +
   +Features provided by TPS6507x:
   +1.Power Management Integrated Circuit.
   +2.Touch-Screen.
  
Required properties:
- compatible: ti,tps6507x
   @@ -23,6 +27,9 @@ Required properties:
   vindcdc1_2-supply: VDCDC1 and VDCDC2 input.
   vindcdc3-supply  : VDCDC3 input.
   vldo1_2-supply   : VLDO1 and VLDO2 input.
   +- tsc: This node specifies touch screen data.
 
  This is a node, but is listed un Required properties. That seems odd.
 
  When is it required?
 
  Why is the data under the tsc subnode? Why not directly under the main
  node?
 
 
  Yes, It seems odd to add a subnode properties here. I gone through other
  documentation
  for MFD devices and observed that separate documentation file is present for
  sub node modules.

 I was trying to say that nodes != properties rather than that this should be
 split into separate files.


Ok, got it.I will modify documentation so that only properties comes
under required
properties section.


 
  TPS6507x is multifunctional device and having touch screen submodule. As it 
  is
  child node for
  TPS6507x multifunctional device, I have created child node as tsc.
 
 
 
   + ti,poll-period : Time at which touch input is getting sampled in
  ms.
 
  Is this for debounce? Other bindings have similar but differently named
  properties.
 
 
  This is not debounce time. This time is required for input poll devices.
 
 
  Why is this necessary? Can the driver not choose a sane value?
 
 
  poll-period is necessary to sample touch inputs. Driver will chose value
  default poll
  period if this value is not present. I was bit confused here, whether to add
  this property
  as optional or required. As with default period touch not achieve 
  performance.
  Can I make this property optional?

 Please elaborate. Why will the default period be sub-optimal? What's the
 tradeoff here?

Currently, default period and period passed from Device tree both
are same.I made an assumption here, if default period is less then
touch get sampled faster and report it to upper layer.As I am not I
will make this confirm with Texas instrument's engineers.


 
 
   + ti,min-pressure: Minimum pressure value to trigger touch.
 
  What type are these? Single (u32) cells?
 
  What units is ti,min-pressure in?
 
 
  No, its a u16 cell. It is measured in ohm. Again default value for 
  min-pressure
  is available
  in driver code. Can I make this property optional?

 Why is it a u16, it's very uncommon to have u16 properties.

 What _physical_ units is this in, and what order of magnitude? e.g. Pascals,
 millipascals.


I gone through data sheet but not able to find any units for this
pressure field.
As per data sheet, we read pressure values from two ADC result
registers and these two registers are 8 bit long and hence provide
totally 16 bit pressure value.


 
 
  
Regulator Optional properties:
- defdcdc_default

[PATCH V4 1/2] tps6507x-ts: Add DT support

[Manish Badarkhe]

Add device tree based support for TI's tps6507x touchscreen.

Signed-off-by: Manish Badarkhe 
---
Changes since V3:
 - Rebased on top of Dmitry's changes
 - Removed error handling for optional DT properties

Changes since V2:
 - Removed unnecessary code.
 - Updated Documentation to provide proper names of
   devicetree properties.

Changes since V1:
 - Updated documentation to specify tps6507x as multifunctional
   device.
 - return proper error value in absence of platform and DT
   data for touchscreen.
 - Updated commit message.

:100755 100755 8fffa3c... e1b9cfd... M  
Documentation/devicetree/bindings/mfd/tps6507x.txt
:100644 100644 db604e0... 0cf0eb1... M  drivers/input/touchscreen/tps6507x-ts.c
 Documentation/devicetree/bindings/mfd/tps6507x.txt |   24 ++-
 drivers/input/touchscreen/tps6507x-ts.c|   72 
 2 files changed, 67 insertions(+), 29 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt 
b/Documentation/devicetree/bindings/mfd/tps6507x.txt
index 8fffa3c..e1b9cfd 100755
--- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
+++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
@@ -1,4 +1,8 @@
-TPS6507x Power Management Integrated Circuit
+TPS6507x Multifunctional Device.
+
+Features provided by TPS6507x:
+1.Power Management Integrated Circuit.
+2.Touch-Screen.
 
 Required properties:
 - compatible: "ti,tps6507x"
@@ -23,6 +27,9 @@ Required properties:
vindcdc1_2-supply: VDCDC1 and VDCDC2 input.
vindcdc3-supply  : VDCDC3 input.
vldo1_2-supply   : VLDO1 and VLDO2 input.
+- tsc: This node specifies touch screen data.
+   ti,poll-period : Time at which touch input is getting sampled in ms.
+   ti,min-pressure: Minimum pressure value to trigger touch.
 
 Regulator Optional properties:
 - defdcdc_default: It's property of DCDC2 and DCDC3 regulators.
@@ -30,6 +37,14 @@ Regulator Optional properties:
1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
   If this property is not defined, it defaults to 0 (not enabled).
 
+Touchscreen Optional properties:
+- ti,vendor : Touchscreen vendor id to populate
+ in sysclass interface.
+- ti,product: Touchscreen product id to populate
+ in sysclass interface.
+- ti,version: Touchscreen version id to populate
+ in sysclass interface.
+
 Example:
 
pmu: tps6507x@48 {
@@ -88,4 +103,11 @@ Example:
};
};
 
+   tsc {
+   ti,poll-period = <30>;
+   ti,min-pressure = <0x30>;
+   ti,vendor = <0>;
+   ti,product = <65070>;
+   ti,version = <0x100>;
+   };
};
diff --git a/drivers/input/touchscreen/tps6507x-ts.c 
b/drivers/input/touchscreen/tps6507x-ts.c
index db604e0..0cf0eb1 100644
--- a/drivers/input/touchscreen/tps6507x-ts.c
+++ b/drivers/input/touchscreen/tps6507x-ts.c
@@ -22,6 +22,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #define TSC_DEFAULT_POLL_PERIOD 30 /* ms */
 #define TPS_DEFAULT_MIN_PRESSURE 0x30
@@ -206,33 +208,54 @@ done:
tps6507x_adc_standby(tsc);
 }
 
+static void tsc_init_data(struct tps6507x_ts *tsc,
+   struct input_dev *input_dev)
+{
+   struct device_node *node = tsc->dev->of_node;
+   const struct tps6507x_board *tps_board =
+   dev_get_platdata(tsc->dev);
+   const struct touchscreen_init_data *init_data = NULL;
+
+   if (node)
+   node = of_find_node_by_name(node, "tsc");
+   if (tps_board)
+   /*
+* init_data points to array of touchscreen_init structure
+* coming from the board-evm file.
+*/
+   init_data = tps_board->tps6507x_ts_init_data;
+
+   if (node == NULL || init_data == NULL) {
+   tsc->poll_dev->poll_interval = TSC_DEFAULT_POLL_PERIOD;
+   tsc->min_pressure = TPS_DEFAULT_MIN_PRESSURE;
+   } else if (init_data) {
+   tsc->poll_dev->poll_interval = init_data->poll_period;
+   tsc->min_pressure = init_data->min_pressure;
+   input_dev->id.vendor = init_data->vendor;
+   input_dev->id.product = init_data->product;
+   input_dev->id.version = init_data->version;
+   } else if (node) {
+   of_property_read_u32(node, "ti,poll-period",
+   >poll_dev->poll_interval);
+   of_property_read_u16(node, "ti,min-pressure",
+   >min_pressure);
+   of_property_read_u16(node, "ti,vendor",
+   _dev->id.vendor);
+   of_property_read_u16(node, "ti,product",

[PATCH V4 0/2] Add DT support for tps6507x touchscreen

[Manish Badarkhe]

Patch set adds DT support for tps6507x based touchscreen.
Also, add DT data for tps6507x touchscreen in da850-evm by
providing touchscreen node inside tps6507x mfd device.

This patch series applies on top of linux-next tree and depends on [1].

[1]tps6507x-ts: update to devm_* API
   https://patchwork.kernel.org/patch/2324441/

Changes since V3:
 - Rebased on top of Dmitry's changes
 - Removed error handling for optional DT properties

Changes since V2:
 - Updated tps6507x documentation.
 - Removed unnecessary code.

Changes since V1:
 - Updated tps6507x documentation.
 - Updated commit message.
 - return proper error value in absence platform and DT data
   for touchscreen.

Manish Badarkhe (2):
  tps6507x-ts: Add DT support
  ARM: davinci: da850: add tps6507x touchscreen DT data

 Documentation/devicetree/bindings/mfd/tps6507x.txt |   24 +-
 arch/arm/boot/dts/da850-evm.dts|8 ++
 drivers/input/touchscreen/tps6507x-ts.c|   79 +---
 3 files changed, 82 insertions(+), 29 deletions(-)

-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4 2/2] ARM: davinci: da850: add tps6507x touchscreen DT data

[Manish Badarkhe]

Add tps6507x touchscreen DT node to da850-evm.
Touchscreen DT data is added as per da850 board file.

Signed-off-by: Manish Badarkhe 
---
Changes since V3:
 - Removed vref property.

Changes since V2:
 - Updated tps6507x documentation.
 - Removed unnecessary code.

Changes since V1:
 - Updated tps6507x documentation.
 - Updated commit message.
 - return proper error value in absence platform and DT data
   for touchscreen.

:100644 100644 588ce58... 5a35264... M  arch/arm/boot/dts/da850-evm.dts
 arch/arm/boot/dts/da850-evm.dts |8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 588ce58..5a35264 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -166,4 +166,12 @@
regulator-boot-on;
};
};
+
+   tsc {
+   ti,poll-period = <30>;
+   ti,min-pressure = <0x30>;
+   ti,vendor = <0>;
+   ti,product = <65070>;
+   ti,version = <0x100>;
+   };
 };
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4 2/2] ARM: davinci: da850: add tps6507x touchscreen DT data

[Manish Badarkhe]

Add tps6507x touchscreen DT node to da850-evm.
Touchscreen DT data is added as per da850 board file.

Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
Changes since V3:
 - Removed vref property.

Changes since V2:
 - Updated tps6507x documentation.
 - Removed unnecessary code.

Changes since V1:
 - Updated tps6507x documentation.
 - Updated commit message.
 - return proper error value in absence platform and DT data
   for touchscreen.

:100644 100644 588ce58... 5a35264... M  arch/arm/boot/dts/da850-evm.dts
 arch/arm/boot/dts/da850-evm.dts |8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 588ce58..5a35264 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -166,4 +166,12 @@
regulator-boot-on;
};
};
+
+   tsc {
+   ti,poll-period = 30;
+   ti,min-pressure = 0x30;
+   ti,vendor = 0;
+   ti,product = 65070;
+   ti,version = 0x100;
+   };
 };
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/