public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] backlight: lp855x: use private data for regulator control
@ 2015-07-20  6:45 Milo Kim
  2015-07-20 13:30 ` Sean Paul
  2015-08-10 15:53 ` Lee Jones
  0 siblings, 2 replies; 4+ messages in thread
From: Milo Kim @ 2015-07-20  6:45 UTC (permalink / raw)
  To: jingoohan1, lee.jones
  Cc: Milo Kim, Sean Paul, Jingoo Han, Lee Jones, linux-kernel

LP855x backlight device can be enabled by external VDD input.
The 'supply' data is used for this purpose.
It's kind of private data which runs internally, so there is no reason to
expose to the platform data.

And devm_regulator_get() is moved from _parse_dt() to _probe().
Regulator consumer(lp855x) can control regulator not only from DT but also
from platform data configuration in a source file such like board-*.c.

v2 -> v3:
  The 'power' regulator is optional but the driver should check the error,
  '-EPROBE_DEFER' and return. It enables to let the driver retry to get
  probed.

v1 -> v2:
  Keeps optional property '<name>-supply' in LP855x DT binding.

Cc: Sean Paul <seanpaul@chromium.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Milo Kim <milo.kim@ti.com>
---
 drivers/video/backlight/lp855x_bl.c  | 23 ++++++++++++-----------
 include/linux/platform_data/lp855x.h |  2 --
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index 88116b4..f88df9e 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -73,6 +73,7 @@ struct lp855x {
 	struct device *dev;
 	struct lp855x_platform_data *pdata;
 	struct pwm_device *pwm;
+	struct regulator *supply;	/* regulator for VDD input */
 };
 
 static int lp855x_write_byte(struct lp855x *lp, u8 reg, u8 data)
@@ -378,13 +379,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
 		pdata->rom_data = &rom[0];
 	}
 
-	pdata->supply = devm_regulator_get(dev, "power");
-	if (IS_ERR(pdata->supply)) {
-		if (PTR_ERR(pdata->supply) == -EPROBE_DEFER)
-			return -EPROBE_DEFER;
-		pdata->supply = NULL;
-	}
-
 	lp->pdata = pdata;
 
 	return 0;
@@ -425,8 +419,15 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 	else
 		lp->mode = REGISTER_BASED;
 
-	if (lp->pdata->supply) {
-		ret = regulator_enable(lp->pdata->supply);
+	lp->supply = devm_regulator_get(lp->dev, "power");
+	if (IS_ERR(lp->supply)) {
+		if (PTR_ERR(lp->supply) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		lp->supply = NULL;
+	}
+
+	if (lp->supply) {
+		ret = regulator_enable(lp->supply);
 		if (ret < 0) {
 			dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
 			return ret;
@@ -464,8 +465,8 @@ static int lp855x_remove(struct i2c_client *cl)
 
 	lp->bl->props.brightness = 0;
 	backlight_update_status(lp->bl);
-	if (lp->pdata->supply)
-		regulator_disable(lp->pdata->supply);
+	if (lp->supply)
+		regulator_disable(lp->supply);
 	sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
 
 	return 0;
diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
index 9c7fd1e..1b2ba24 100644
--- a/include/linux/platform_data/lp855x.h
+++ b/include/linux/platform_data/lp855x.h
@@ -136,7 +136,6 @@ struct lp855x_rom_data {
 		Only valid when mode is PWM_BASED.
  * @size_program : total size of lp855x_rom_data
  * @rom_data : list of new eeprom/eprom registers
- * @supply : regulator that supplies 3V input
  */
 struct lp855x_platform_data {
 	const char *name;
@@ -145,7 +144,6 @@ struct lp855x_platform_data {
 	unsigned int period_ns;
 	int size_program;
 	struct lp855x_rom_data *rom_data;
-	struct regulator *supply;
 };
 
 #endif
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] backlight: lp855x: use private data for regulator control
  2015-07-20  6:45 [PATCH v3] backlight: lp855x: use private data for regulator control Milo Kim
@ 2015-07-20 13:30 ` Sean Paul
  2015-08-06 15:42   ` Jingoo Han
  2015-08-10 15:53 ` Lee Jones
  1 sibling, 1 reply; 4+ messages in thread
From: Sean Paul @ 2015-07-20 13:30 UTC (permalink / raw)
  To: Milo Kim; +Cc: Jingoo Han, Lee Jones, Linux Kernel Mailing List

On Mon, Jul 20, 2015 at 2:45 AM, Milo Kim <milo.kim@ti.com> wrote:
> LP855x backlight device can be enabled by external VDD input.
> The 'supply' data is used for this purpose.
> It's kind of private data which runs internally, so there is no reason to
> expose to the platform data.
>
> And devm_regulator_get() is moved from _parse_dt() to _probe().
> Regulator consumer(lp855x) can control regulator not only from DT but also
> from platform data configuration in a source file such like board-*.c.
>
> v2 -> v3:
>   The 'power' regulator is optional but the driver should check the error,
>   '-EPROBE_DEFER' and return. It enables to let the driver retry to get
>   probed.
>
> v1 -> v2:
>   Keeps optional property '<name>-supply' in LP855x DT binding.
>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Milo Kim <milo.kim@ti.com>

Acked-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/video/backlight/lp855x_bl.c  | 23 ++++++++++++-----------
>  include/linux/platform_data/lp855x.h |  2 --
>  2 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 88116b4..f88df9e 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -73,6 +73,7 @@ struct lp855x {
>         struct device *dev;
>         struct lp855x_platform_data *pdata;
>         struct pwm_device *pwm;
> +       struct regulator *supply;       /* regulator for VDD input */
>  };
>
>  static int lp855x_write_byte(struct lp855x *lp, u8 reg, u8 data)
> @@ -378,13 +379,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
>                 pdata->rom_data = &rom[0];
>         }
>
> -       pdata->supply = devm_regulator_get(dev, "power");
> -       if (IS_ERR(pdata->supply)) {
> -               if (PTR_ERR(pdata->supply) == -EPROBE_DEFER)
> -                       return -EPROBE_DEFER;
> -               pdata->supply = NULL;
> -       }
> -
>         lp->pdata = pdata;
>
>         return 0;
> @@ -425,8 +419,15 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>         else
>                 lp->mode = REGISTER_BASED;
>
> -       if (lp->pdata->supply) {
> -               ret = regulator_enable(lp->pdata->supply);
> +       lp->supply = devm_regulator_get(lp->dev, "power");
> +       if (IS_ERR(lp->supply)) {
> +               if (PTR_ERR(lp->supply) == -EPROBE_DEFER)
> +                       return -EPROBE_DEFER;
> +               lp->supply = NULL;
> +       }
> +
> +       if (lp->supply) {
> +               ret = regulator_enable(lp->supply);
>                 if (ret < 0) {
>                         dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
>                         return ret;
> @@ -464,8 +465,8 @@ static int lp855x_remove(struct i2c_client *cl)
>
>         lp->bl->props.brightness = 0;
>         backlight_update_status(lp->bl);
> -       if (lp->pdata->supply)
> -               regulator_disable(lp->pdata->supply);
> +       if (lp->supply)
> +               regulator_disable(lp->supply);
>         sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
>
>         return 0;
> diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
> index 9c7fd1e..1b2ba24 100644
> --- a/include/linux/platform_data/lp855x.h
> +++ b/include/linux/platform_data/lp855x.h
> @@ -136,7 +136,6 @@ struct lp855x_rom_data {
>                 Only valid when mode is PWM_BASED.
>   * @size_program : total size of lp855x_rom_data
>   * @rom_data : list of new eeprom/eprom registers
> - * @supply : regulator that supplies 3V input
>   */
>  struct lp855x_platform_data {
>         const char *name;
> @@ -145,7 +144,6 @@ struct lp855x_platform_data {
>         unsigned int period_ns;
>         int size_program;
>         struct lp855x_rom_data *rom_data;
> -       struct regulator *supply;
>  };
>
>  #endif
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] backlight: lp855x: use private data for regulator control
  2015-07-20 13:30 ` Sean Paul
@ 2015-08-06 15:42   ` Jingoo Han
  0 siblings, 0 replies; 4+ messages in thread
From: Jingoo Han @ 2015-08-06 15:42 UTC (permalink / raw)
  To: 'Milo Kim'
  Cc: 'Sean Paul', 'Lee Jones',
	'Linux Kernel Mailing List', 'Jingoo Han'

On Monday, July 20, 2015 10:31 PM, Sean Paul wrote:
> On Mon, Jul 20, 2015 at 2:45 AM, Milo Kim <milo.kim@ti.com> wrote:
> > LP855x backlight device can be enabled by external VDD input.
> > The 'supply' data is used for this purpose.
> > It's kind of private data which runs internally, so there is no reason to
> > expose to the platform data.
> >
> > And devm_regulator_get() is moved from _parse_dt() to _probe().
> > Regulator consumer(lp855x) can control regulator not only from DT but also
> > from platform data configuration in a source file such like board-*.c.
> >
> > v2 -> v3:
> >   The 'power' regulator is optional but the driver should check the error,
> >   '-EPROBE_DEFER' and return. It enables to let the driver retry to get
> >   probed.
> >
> > v1 -> v2:
> >   Keeps optional property '<name>-supply' in LP855x DT binding.
> >
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: Jingoo Han <jingoohan1@gmail.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Milo Kim <milo.kim@ti.com>
> 
> Acked-by: Sean Paul <seanpaul@chromium.org>

Acked-by: Jingoo Han <jingoohan1@gmail.com>

Best regards,
Jingoo Han

> 
> > ---
> >  drivers/video/backlight/lp855x_bl.c  | 23 ++++++++++++-----------
> >  include/linux/platform_data/lp855x.h |  2 --
> >  2 files changed, 12 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> > index 88116b4..f88df9e 100644
> > --- a/drivers/video/backlight/lp855x_bl.c
> > +++ b/drivers/video/backlight/lp855x_bl.c
> > @@ -73,6 +73,7 @@ struct lp855x {
> >         struct device *dev;
> >         struct lp855x_platform_data *pdata;
> >         struct pwm_device *pwm;
> > +       struct regulator *supply;       /* regulator for VDD input */
> >  };
> >
> >  static int lp855x_write_byte(struct lp855x *lp, u8 reg, u8 data)
> > @@ -378,13 +379,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
> >                 pdata->rom_data = &rom[0];
> >         }
> >
> > -       pdata->supply = devm_regulator_get(dev, "power");
> > -       if (IS_ERR(pdata->supply)) {
> > -               if (PTR_ERR(pdata->supply) == -EPROBE_DEFER)
> > -                       return -EPROBE_DEFER;
> > -               pdata->supply = NULL;
> > -       }
> > -
> >         lp->pdata = pdata;
> >
> >         return 0;
> > @@ -425,8 +419,15 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> >         else
> >                 lp->mode = REGISTER_BASED;
> >
> > -       if (lp->pdata->supply) {
> > -               ret = regulator_enable(lp->pdata->supply);
> > +       lp->supply = devm_regulator_get(lp->dev, "power");
> > +       if (IS_ERR(lp->supply)) {
> > +               if (PTR_ERR(lp->supply) == -EPROBE_DEFER)
> > +                       return -EPROBE_DEFER;
> > +               lp->supply = NULL;
> > +       }
> > +
> > +       if (lp->supply) {
> > +               ret = regulator_enable(lp->supply);
> >                 if (ret < 0) {
> >                         dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
> >                         return ret;
> > @@ -464,8 +465,8 @@ static int lp855x_remove(struct i2c_client *cl)
> >
> >         lp->bl->props.brightness = 0;
> >         backlight_update_status(lp->bl);
> > -       if (lp->pdata->supply)
> > -               regulator_disable(lp->pdata->supply);
> > +       if (lp->supply)
> > +               regulator_disable(lp->supply);
> >         sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
> >
> >         return 0;
> > diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
> > index 9c7fd1e..1b2ba24 100644
> > --- a/include/linux/platform_data/lp855x.h
> > +++ b/include/linux/platform_data/lp855x.h
> > @@ -136,7 +136,6 @@ struct lp855x_rom_data {
> >                 Only valid when mode is PWM_BASED.
> >   * @size_program : total size of lp855x_rom_data
> >   * @rom_data : list of new eeprom/eprom registers
> > - * @supply : regulator that supplies 3V input
> >   */
> >  struct lp855x_platform_data {
> >         const char *name;
> > @@ -145,7 +144,6 @@ struct lp855x_platform_data {
> >         unsigned int period_ns;
> >         int size_program;
> >         struct lp855x_rom_data *rom_data;
> > -       struct regulator *supply;
> >  };
> >
> >  #endif
> > --
> > 1.9.1
> >


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] backlight: lp855x: use private data for regulator control
  2015-07-20  6:45 [PATCH v3] backlight: lp855x: use private data for regulator control Milo Kim
  2015-07-20 13:30 ` Sean Paul
@ 2015-08-10 15:53 ` Lee Jones
  1 sibling, 0 replies; 4+ messages in thread
From: Lee Jones @ 2015-08-10 15:53 UTC (permalink / raw)
  To: Milo Kim; +Cc: jingoohan1, Sean Paul, linux-kernel

On Mon, 20 Jul 2015, Milo Kim wrote:

> LP855x backlight device can be enabled by external VDD input.
> The 'supply' data is used for this purpose.
> It's kind of private data which runs internally, so there is no reason to
> expose to the platform data.
> 
> And devm_regulator_get() is moved from _parse_dt() to _probe().
> Regulator consumer(lp855x) can control regulator not only from DT but also
> from platform data configuration in a source file such like board-*.c.
> 
> v2 -> v3:
>   The 'power' regulator is optional but the driver should check the error,
>   '-EPROBE_DEFER' and return. It enables to let the driver retry to get
>   probed.
> 
> v1 -> v2:
>   Keeps optional property '<name>-supply' in LP855x DT binding.
> 
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Milo Kim <milo.kim@ti.com>
> ---
>  drivers/video/backlight/lp855x_bl.c  | 23 ++++++++++++-----------
>  include/linux/platform_data/lp855x.h |  2 --
>  2 files changed, 12 insertions(+), 13 deletions(-)

Applied with Jingoo's Ack, thanks.

> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 88116b4..f88df9e 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -73,6 +73,7 @@ struct lp855x {
>  	struct device *dev;
>  	struct lp855x_platform_data *pdata;
>  	struct pwm_device *pwm;
> +	struct regulator *supply;	/* regulator for VDD input */
>  };
>  
>  static int lp855x_write_byte(struct lp855x *lp, u8 reg, u8 data)
> @@ -378,13 +379,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
>  		pdata->rom_data = &rom[0];
>  	}
>  
> -	pdata->supply = devm_regulator_get(dev, "power");
> -	if (IS_ERR(pdata->supply)) {
> -		if (PTR_ERR(pdata->supply) == -EPROBE_DEFER)
> -			return -EPROBE_DEFER;
> -		pdata->supply = NULL;
> -	}
> -
>  	lp->pdata = pdata;
>  
>  	return 0;
> @@ -425,8 +419,15 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>  	else
>  		lp->mode = REGISTER_BASED;
>  
> -	if (lp->pdata->supply) {
> -		ret = regulator_enable(lp->pdata->supply);
> +	lp->supply = devm_regulator_get(lp->dev, "power");
> +	if (IS_ERR(lp->supply)) {
> +		if (PTR_ERR(lp->supply) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		lp->supply = NULL;
> +	}
> +
> +	if (lp->supply) {
> +		ret = regulator_enable(lp->supply);
>  		if (ret < 0) {
>  			dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
>  			return ret;
> @@ -464,8 +465,8 @@ static int lp855x_remove(struct i2c_client *cl)
>  
>  	lp->bl->props.brightness = 0;
>  	backlight_update_status(lp->bl);
> -	if (lp->pdata->supply)
> -		regulator_disable(lp->pdata->supply);
> +	if (lp->supply)
> +		regulator_disable(lp->supply);
>  	sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
>  
>  	return 0;
> diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
> index 9c7fd1e..1b2ba24 100644
> --- a/include/linux/platform_data/lp855x.h
> +++ b/include/linux/platform_data/lp855x.h
> @@ -136,7 +136,6 @@ struct lp855x_rom_data {
>  		Only valid when mode is PWM_BASED.
>   * @size_program : total size of lp855x_rom_data
>   * @rom_data : list of new eeprom/eprom registers
> - * @supply : regulator that supplies 3V input
>   */
>  struct lp855x_platform_data {
>  	const char *name;
> @@ -145,7 +144,6 @@ struct lp855x_platform_data {
>  	unsigned int period_ns;
>  	int size_program;
>  	struct lp855x_rom_data *rom_data;
> -	struct regulator *supply;
>  };
>  
>  #endif

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-08-10 15:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-20  6:45 [PATCH v3] backlight: lp855x: use private data for regulator control Milo Kim
2015-07-20 13:30 ` Sean Paul
2015-08-06 15:42   ` Jingoo Han
2015-08-10 15:53 ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox