Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH] video / PM: Replace CONFIG_PM_RUNTIME with CONFIG_PM
From: Rafael J. Wysocki @ 2014-12-06  2:36 UTC (permalink / raw)
  To: Jingoo Han, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: linux-fbdev, Linux Kernel Mailing List, Linux PM list

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is
selected) PM_RUNTIME is always set if PM is set, so #ifdef blocks
depending on CONFIG_PM_RUNTIME may now be changed to depend on
CONFIG_PM.

The alternative of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME may be
replaced with CONFIG_PM too.

Make these changes in 2 files under drivers/video/.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Note: This depends on commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if
PM_SLEEP is selected) which is only in linux-next at the moment (via the
linux-pm tree).

Please let me know if it is OK to take this one into linux-pm.

---
 drivers/video/fbdev/s3c-fb.c          |    2 +-
 drivers/video/fbdev/sh_mobile_meram.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/video/fbdev/sh_mobile_meram.c
=================================--- linux-pm.orig/drivers/video/fbdev/sh_mobile_meram.c
+++ linux-pm/drivers/video/fbdev/sh_mobile_meram.c
@@ -569,7 +569,7 @@ EXPORT_SYMBOL_GPL(sh_mobile_meram_cache_
  * Power management
  */
 
-#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
+#ifdef CONFIG_PM
 static int sh_mobile_meram_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -612,7 +612,7 @@ static int sh_mobile_meram_resume(struct
 		meram_write_reg(priv->base, common_regs[i], priv->regs[i]);
 	return 0;
 }
-#endif /* CONFIG_PM_SLEEP || CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM */
 
 static UNIVERSAL_DEV_PM_OPS(sh_mobile_meram_dev_pm_ops,
 			    sh_mobile_meram_suspend,
Index: linux-pm/drivers/video/fbdev/s3c-fb.c
=================================--- linux-pm.orig/drivers/video/fbdev/s3c-fb.c
+++ linux-pm/drivers/video/fbdev/s3c-fb.c
@@ -1630,7 +1630,7 @@ static int s3c_fb_resume(struct device *
 }
 #endif
 
-#ifdef CONFIG_PM_RUNTIME
+#ifdef CONFIG_PM
 static int s3c_fb_runtime_suspend(struct device *dev)
 {
 	struct s3c_fb *sfb = dev_get_drvdata(dev);


^ permalink raw reply

* Re: [PATCH] video: ocfb: Fix data type warning
From: Stefan Kristiansson @ 2014-12-07  6:08 UTC (permalink / raw)
  To: Qiang Chen
  Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jingoo Han,
	Daniel Vetter, Laurent Pinchart, linux-fbdev, linux-kernel,
	Qiang Chen
In-Reply-To: <1417767510-31462-1-git-send-email-qiang2.chen@sonymobile.com>

On Fri, Dec 05, 2014 at 04:18:30PM +0800, Qiang Chen wrote:
> When allocate framebuffer memory using dma_alloc_coherent(),
> we'd better use dma_addr_t instead of phys_addr_t. Because the
> address we got in fact is DMA or bus address for the platform.
> 
> This patch also fixes below build warning:
> drivers/video/fbdev/ocfb.c:335:2:
> 	warning: passing argument 3 of ‘dma_alloc_attrs’
> 	from incompatible pointer type [enabled by default]
> 
> Signed-off-by: Qiang Chen <qiang2.chen@sonymobile.com>

Acked-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>

^ permalink raw reply

* Re: [PATCH 4/4] backlight/lp855x: Remove CONFIG_OF ifdef in favor of Kconfig depends
From: Jingoo Han @ 2014-12-08  2:17 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417805069-20441-4-git-send-email-seanpaul@chromium.org>

On Saturday, December 06, 2014 3:44 AM, Sean Paul wrote:
> 
> Now that we've removed all traces of pdata, remove the CONFIG_OF ifdef
> from lp855x and instead make the driver depend on OF.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/video/backlight/Kconfig     | 2 +-
>  drivers/video/backlight/lp855x_bl.c | 7 -------
>  2 files changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 8d03924..113c8b3 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -384,7 +384,7 @@ config BACKLIGHT_LM3639
> 
>  config BACKLIGHT_LP855X
>  	tristate "Backlight driver for TI LP855X"
> -	depends on BACKLIGHT_CLASS_DEVICE && I2C && PWM
> +	depends on BACKLIGHT_CLASS_DEVICE && I2C && PWM && OF
>  	help
>  	  This supports TI LP8550, LP8551, LP8552, LP8553, LP8555, LP8556 and
>  	  LP8557 backlight driver.
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 8b81d8e..3d3b8cf 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -379,7 +379,6 @@ static const struct attribute_group lp855x_attr_group = {
>  	.attrs = lp855x_attributes,
>  };
> 
> -#ifdef CONFIG_OF
>  static int lp855x_parse_dt(struct lp855x *lp)
>  {
>  	struct device *dev = lp->dev;
> @@ -425,12 +424,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
> 
>  	return 0;
>  }
> -#else
> -static int lp855x_parse_dt(struct lp855x *lp)
> -{
> -	return -EINVAL;
> -}
> -#endif
> 
>  static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>  {

One more thin, in this case, of_match_ptr() is redundant;
thus, of_match_ptr() can be also removed as follows.

--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -487,7 +487,7 @@ MODULE_DEVICE_TABLE(i2c, lp855x_ids);
 static struct i2c_driver lp855x_driver = {
	.driver = {
		.name = "lp855x",
-		 .of_match_table = of_match_ptr(lp855x_dt_ids),
+		.of_match_table = lp855x_dt_ids,
		},
	.probe = lp855x_probe,
	.remove = lp855x_remove,

Best regards,
Jingoo Han

> --
> 2.2.0.rc0.207.ga3a616c


^ permalink raw reply

* Re: [PATCH] video / PM: Replace CONFIG_PM_RUNTIME with CONFIG_PM
From: Tomi Valkeinen @ 2014-12-08 11:00 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jingoo Han, Jean-Christophe Plagniol-Villard, linux-fbdev,
	Linux Kernel Mailing List, Linux PM list
In-Reply-To: <19160360.7uJRUqlDQg@vostro.rjw.lan>

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

On 06/12/14 04:36, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is
> selected) PM_RUNTIME is always set if PM is set, so #ifdef blocks
> depending on CONFIG_PM_RUNTIME may now be changed to depend on
> CONFIG_PM.
> 
> The alternative of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME may be
> replaced with CONFIG_PM too.
> 
> Make these changes in 2 files under drivers/video/.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> Note: This depends on commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if
> PM_SLEEP is selected) which is only in linux-next at the moment (via the
> linux-pm tree).
> 
> Please let me know if it is OK to take this one into linux-pm.

Does not seem to conflict with fbdev changes, so:

Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH] video / PM: Replace CONFIG_PM_RUNTIME with CONFIG_PM
From: Jingoo Han @ 2014-12-08 11:06 UTC (permalink / raw)
  To: 'Tomi Valkeinen', 'Rafael J. Wysocki'
  Cc: 'Jean-Christophe Plagniol-Villard', linux-fbdev,
	'Linux Kernel Mailing List', 'Linux PM list',
	'Jingoo Han'
In-Reply-To: <548584CC.7010603@ti.com>

On Monday, December 08, 2014 8:00 PM, Tomi Valkeinen wrote
> 
> On 06/12/14 04:36, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > After commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if PM_SLEEP is
> > selected) PM_RUNTIME is always set if PM is set, so #ifdef blocks
> > depending on CONFIG_PM_RUNTIME may now be changed to depend on
> > CONFIG_PM.
> >
> > The alternative of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME may be
> > replaced with CONFIG_PM too.
> >
> > Make these changes in 2 files under drivers/video/.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > Note: This depends on commit b2b49ccbdd54 (PM: Kconfig: Set PM_RUNTIME if
> > PM_SLEEP is selected) which is only in linux-next at the moment (via the
> > linux-pm tree).
> >
> > Please let me know if it is OK to take this one into linux-pm.
> 
> Does not seem to conflict with fbdev changes, so:
> 
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

For drivers/video/fbdev/s3c-fb.c,

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> 
>  Tomi
> 



^ permalink raw reply

* Re: [PATCH 1/4] backlight/lp855x: Remove stale code from lp855x.h
From: Jingoo Han @ 2014-12-08 11:40 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417805069-20441-1-git-send-email-seanpaul@chromium.org>

On Saturday, December 06, 2014 3:44 AM, Sean Paul wrote:
> 
> These aren't used by anyone, remove them.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  include/linux/platform_data/lp855x.h | 100 -----------------------------------
>  1 file changed, 100 deletions(-)
> 
> diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
> index 9c7fd1e..9e3ac3c 100644
> --- a/include/linux/platform_data/lp855x.h
> +++ b/include/linux/platform_data/lp855x.h
> @@ -12,65 +12,6 @@
>  #ifndef _LP855X_H
>  #define _LP855X_H
> 
> -#define BL_CTL_SHFT	(0)
> -#define BRT_MODE_SHFT	(1)
> -#define BRT_MODE_MASK	(0x06)
> -
> -/* Enable backlight. Only valid when BRT_MODE\x10(I2C only) */
> -#define ENABLE_BL	(1)
> -#define DISABLE_BL	(0)
> -
> -#define I2C_CONFIG(id)	id ## _I2C_CONFIG
> -#define PWM_CONFIG(id)	id ## _PWM_CONFIG
> -
> -/* DEVICE CONTROL register - LP8550 */
> -#define LP8550_PWM_CONFIG	(LP8550_PWM_ONLY << BRT_MODE_SHFT)
> -#define LP8550_I2C_CONFIG	((ENABLE_BL << BL_CTL_SHFT) | \
> -				(LP8550_I2C_ONLY << BRT_MODE_SHFT))
> -
> -/* DEVICE CONTROL register - LP8551 */
> -#define LP8551_PWM_CONFIG	LP8550_PWM_CONFIG
> -#define LP8551_I2C_CONFIG	LP8550_I2C_CONFIG
> -
> -/* DEVICE CONTROL register - LP8552 */
> -#define LP8552_PWM_CONFIG	LP8550_PWM_CONFIG
> -#define LP8552_I2C_CONFIG	LP8550_I2C_CONFIG
> -
> -/* DEVICE CONTROL register - LP8553 */
> -#define LP8553_PWM_CONFIG	LP8550_PWM_CONFIG
> -#define LP8553_I2C_CONFIG	LP8550_I2C_CONFIG
> -
> -/* CONFIG register - LP8555 */
> -#define LP8555_PWM_STANDBY	BIT(7)
> -#define LP8555_PWM_FILTER	BIT(6)
> -#define LP8555_RELOAD_EPROM	BIT(3)	/* use it if EPROMs should be reset
> -					   when the backlight turns on */
> -#define LP8555_OFF_OPENLEDS	BIT(2)
> -#define LP8555_PWM_CONFIG	LP8555_PWM_ONLY
> -#define LP8555_I2C_CONFIG	LP8555_I2C_ONLY
> -#define LP8555_COMB1_CONFIG	LP8555_COMBINED1
> -#define LP8555_COMB2_CONFIG	LP8555_COMBINED2
> -
> -/* DEVICE CONTROL register - LP8556 */
> -#define LP8556_PWM_CONFIG	(LP8556_PWM_ONLY << BRT_MODE_SHFT)
> -#define LP8556_COMB1_CONFIG	(LP8556_COMBINED1 << BRT_MODE_SHFT)
> -#define LP8556_I2C_CONFIG	((ENABLE_BL << BL_CTL_SHFT) | \
> -				(LP8556_I2C_ONLY << BRT_MODE_SHFT))
> -#define LP8556_COMB2_CONFIG	(LP8556_COMBINED2 << BRT_MODE_SHFT)
> -#define LP8556_FAST_CONFIG	BIT(7) /* use it if EPROMs should be maintained
> -					  when exiting the low power mode */
> -
> -/* CONFIG register - LP8557 */
> -#define LP8557_PWM_STANDBY	BIT(7)
> -#define LP8557_PWM_FILTER	BIT(6)
> -#define LP8557_RELOAD_EPROM	BIT(3)	/* use it if EPROMs should be reset
> -					   when the backlight turns on */
> -#define LP8557_OFF_OPENLEDS	BIT(2)
> -#define LP8557_PWM_CONFIG	LP8557_PWM_ONLY
> -#define LP8557_I2C_CONFIG	LP8557_I2C_ONLY
> -#define LP8557_COMB1_CONFIG	LP8557_COMBINED1
> -#define LP8557_COMB2_CONFIG	LP8557_COMBINED2
> -
>  enum lp855x_chip_id {
>  	LP8550,
>  	LP8551,
> @@ -81,47 +22,6 @@ enum lp855x_chip_id {
>  	LP8557,
>  };
> 
> -enum lp8550_brighntess_source {
> -	LP8550_PWM_ONLY,
> -	LP8550_I2C_ONLY = 2,
> -};
> -
> -enum lp8551_brighntess_source {
> -	LP8551_PWM_ONLY = LP8550_PWM_ONLY,
> -	LP8551_I2C_ONLY = LP8550_I2C_ONLY,
> -};
> -
> -enum lp8552_brighntess_source {
> -	LP8552_PWM_ONLY = LP8550_PWM_ONLY,
> -	LP8552_I2C_ONLY = LP8550_I2C_ONLY,
> -};
> -
> -enum lp8553_brighntess_source {
> -	LP8553_PWM_ONLY = LP8550_PWM_ONLY,
> -	LP8553_I2C_ONLY = LP8550_I2C_ONLY,
> -};
> -
> -enum lp8555_brightness_source {
> -	LP8555_PWM_ONLY,
> -	LP8555_I2C_ONLY,
> -	LP8555_COMBINED1,	/* Brightness register with shaped PWM */
> -	LP8555_COMBINED2,	/* PWM with shaped brightness register */
> -};
> -
> -enum lp8556_brightness_source {
> -	LP8556_PWM_ONLY,
> -	LP8556_COMBINED1,	/* pwm + i2c before the shaper block */
> -	LP8556_I2C_ONLY,
> -	LP8556_COMBINED2,	/* pwm + i2c after the shaper block */
> -};
> -
> -enum lp8557_brightness_source {
> -	LP8557_PWM_ONLY,
> -	LP8557_I2C_ONLY,
> -	LP8557_COMBINED1,	/* pwm + i2c after the shaper block */
> -	LP8557_COMBINED2,	/* pwm + i2c before the shaper block */
> -};
> -
>  struct lp855x_rom_data {
>  	u8 addr;
>  	u8 val;
> --
> 2.2.0.rc0.207.ga3a616c


^ permalink raw reply

* Re: [PATCH 2/4] backlight/lp855x: Remove platform_data header
From: Jingoo Han @ 2014-12-08 11:41 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417805069-20441-2-git-send-email-seanpaul@chromium.org>

On Saturday, December 06, 2014 3:44 AM, Sean Paul wrote:
> 
> No one uses lp855x platform data any longer, remove the header
> and move its contents into the driver.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  MAINTAINERS                          |  1 -
>  drivers/video/backlight/lp855x_bl.c  | 37 +++++++++++++++++++++++++-
>  include/linux/platform_data/lp855x.h | 51 ------------------------------------
>  3 files changed, 36 insertions(+), 53 deletions(-)
>  delete mode 100644 include/linux/platform_data/lp855x.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3c64271..4896edb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9318,7 +9318,6 @@ M:	Milo Kim <milo.kim@ti.com>
>  S:	Maintained
>  F:	Documentation/backlight/lp855x-driver.txt
>  F:	drivers/video/backlight/lp855x_bl.c
> -F:	include/linux/platform_data/lp855x.h
> 
>  TI LP8727 CHARGER DRIVER
>  M:	Milo Kim <milo.kim@ti.com>
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index a26d3bb..d19b61c 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -15,7 +15,6 @@
>  #include <linux/backlight.h>
>  #include <linux/err.h>
>  #include <linux/of.h>
> -#include <linux/platform_data/lp855x.h>
>  #include <linux/pwm.h>
>  #include <linux/regulator/consumer.h>
> 
> @@ -63,6 +62,42 @@ struct lp855x_device_config {
>  	int (*post_init_device)(struct lp855x *);
>  };
> 
> +enum lp855x_chip_id {
> +	LP8550,
> +	LP8551,
> +	LP8552,
> +	LP8553,
> +	LP8555,
> +	LP8556,
> +	LP8557,
> +};
> +
> +struct lp855x_rom_data {
> +	u8 addr;
> +	u8 val;
> +};
> +
> +/**
> + * struct lp855x_platform_data
> + * @name : Backlight driver name. If it is not defined, default name is set.
> + * @device_control : value of DEVICE CONTROL register
> + * @initial_brightness : initial value of backlight brightness
> + * @period_ns : platform specific pwm period value. unit is nano.
> +		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;
> +	u8 device_control;
> +	u8 initial_brightness;
> +	unsigned int period_ns;
> +	int size_program;
> +	struct lp855x_rom_data *rom_data;
> +	struct regulator *supply;
> +};
> +
>  struct lp855x {
>  	const char *chipname;
>  	enum lp855x_chip_id chip_id;
> diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
> deleted file mode 100644
> index 9e3ac3c..0000000
> --- a/include/linux/platform_data/lp855x.h
> +++ /dev/null
> @@ -1,51 +0,0 @@
> -/*
> - * LP855x Backlight Driver
> - *
> - *			Copyright (C) 2011 Texas Instruments
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - *
> - */
> -
> -#ifndef _LP855X_H
> -#define _LP855X_H
> -
> -enum lp855x_chip_id {
> -	LP8550,
> -	LP8551,
> -	LP8552,
> -	LP8553,
> -	LP8555,
> -	LP8556,
> -	LP8557,
> -};
> -
> -struct lp855x_rom_data {
> -	u8 addr;
> -	u8 val;
> -};
> -
> -/**
> - * struct lp855x_platform_data
> - * @name : Backlight driver name. If it is not defined, default name is set.
> - * @device_control : value of DEVICE CONTROL register
> - * @initial_brightness : initial value of backlight brightness
> - * @period_ns : platform specific pwm period value. unit is nano.
> -		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;
> -	u8 device_control;
> -	u8 initial_brightness;
> -	unsigned int period_ns;
> -	int size_program;
> -	struct lp855x_rom_data *rom_data;
> -	struct regulator *supply;
> -};
> -
> -#endif
> --
> 2.2.0.rc0.207.ga3a616c


^ permalink raw reply

* Re: [PATCH 3/4] backlight/lp855x: Merge lp855x_platform_data with lp855x
From: Jingoo Han @ 2014-12-08 11:59 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417805069-20441-3-git-send-email-seanpaul@chromium.org>

On Saturday, December 06, 2014 3:44 AM, Sean Paul wrote:
> 
> Now that we have removed the platform_data header, merge lp855x_platform_data
> with lp855x and remove all traces of platform_data from the driver.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/video/backlight/lp855x_bl.c | 100 +++++++++++++++++-------------------
>  1 file changed, 46 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index d19b61c..8b81d8e 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -78,8 +78,16 @@ struct lp855x_rom_data {
>  };
> 
>  /**
> - * struct lp855x_platform_data
> - * @name : Backlight driver name. If it is not defined, default name is set.
> + * struct lp855x
> + * @chipname : Chip name, comes from the i2c_device_id
> + * @blname : Backlight driver name. If it is not defined, default name is set.
> + * @chip_id : The type of lp855x
> + * @mode : Whether brightness is controlled via pwm or register
> + * @cfg : Chip specific hooks & register offsets
> + * @client : The i2c client
> + * @bl : The backlight device
> + * @dev : Device pointer
> + * @pwm : The pwm device (if available)
>   * @device_control : value of DEVICE CONTROL register
>   * @initial_brightness : initial value of backlight brightness
>   * @period_ns : platform specific pwm period value. unit is nano.
> @@ -88,26 +96,23 @@ struct lp855x_rom_data {
>   * @rom_data : list of new eeprom/eprom registers
>   * @supply : regulator that supplies 3V input
>   */
> -struct lp855x_platform_data {
> -	const char *name;
> -	u8 device_control;
> -	u8 initial_brightness;
> -	unsigned int period_ns;
> -	int size_program;
> -	struct lp855x_rom_data *rom_data;
> -	struct regulator *supply;
> -};
> -
>  struct lp855x {
>  	const char *chipname;
> +	const char *blname;
>  	enum lp855x_chip_id chip_id;
>  	enum lp855x_brightness_ctrl_mode mode;
>  	struct lp855x_device_config *cfg;
>  	struct i2c_client *client;
>  	struct backlight_device *bl;
>  	struct device *dev;
> -	struct lp855x_platform_data *pdata;
>  	struct pwm_device *pwm;
> +
> +	u8 device_control;
> +	u8 initial_brightness;
> +	unsigned int period_ns;
> +	int size_program;
> +	struct lp855x_rom_data *rom_data;
> +	struct regulator *supply;
>  };

Now, 'struct lp855x_platform_data' is removed.
But, there is still remaining in 'Documentation/backlight/lp855x-driver.txt'.
Would you clean this lp855x-driver.txt file?

> 
>  static int lp855x_write_byte(struct lp855x *lp, u8 reg, u8 data)
> @@ -204,7 +209,6 @@ static int lp855x_configure(struct lp855x *lp)
>  {
>  	u8 val, addr;
>  	int i, ret;
> -	struct lp855x_platform_data *pd = lp->pdata;
> 
>  	switch (lp->chip_id) {
>  	case LP8550:
> @@ -230,20 +234,20 @@ static int lp855x_configure(struct lp855x *lp)
>  		}
>  	}
> 
> -	val = pd->initial_brightness;
> +	val = lp->initial_brightness;
>  	ret = lp855x_write_byte(lp, lp->cfg->reg_brightness, val);
>  	if (ret)
>  		goto err;
> 
> -	val = pd->device_control;
> +	val = lp->device_control;
>  	ret = lp855x_write_byte(lp, lp->cfg->reg_devicectrl, val);
>  	if (ret)
>  		goto err;
> 
> -	if (pd->size_program > 0) {
> -		for (i = 0; i < pd->size_program; i++) {
> -			addr = pd->rom_data[i].addr;
> -			val = pd->rom_data[i].val;
> +	if (lp->size_program > 0) {
> +		for (i = 0; i < lp->size_program; i++) {
> +			addr = lp->rom_data[i].addr;
> +			val = lp->rom_data[i].val;
>  			if (!lp855x_is_valid_rom_area(lp, addr))
>  				continue;
> 
> @@ -269,7 +273,7 @@ err:
> 
>  static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
>  {
> -	unsigned int period = lp->pdata->period_ns;
> +	unsigned int period = lp->period_ns;
>  	unsigned int duty = br * period / max_br;
>  	struct pwm_device *pwm;
> 
> @@ -320,16 +324,15 @@ static int lp855x_backlight_register(struct lp855x *lp)
>  {
>  	struct backlight_device *bl;
>  	struct backlight_properties props;
> -	struct lp855x_platform_data *pdata = lp->pdata;
> -	const char *name = pdata->name ? : DEFAULT_BL_NAME;
> +	const char *name = lp->blname ? : DEFAULT_BL_NAME;
> 
>  	props.type = BACKLIGHT_PLATFORM;
>  	props.max_brightness = MAX_BRIGHTNESS;
> 
> -	if (pdata->initial_brightness > props.max_brightness)
> -		pdata->initial_brightness = props.max_brightness;
> +	if (lp->initial_brightness > props.max_brightness)
> +		lp->initial_brightness = props.max_brightness;
> 
> -	props.brightness = pdata->initial_brightness;
> +	props.brightness = lp->initial_brightness;
> 
>  	bl = devm_backlight_device_register(lp->dev, name, lp->dev, lp,
>  				       &lp855x_bl_ops, &props);
> @@ -381,7 +384,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
>  {
>  	struct device *dev = lp->dev;
>  	struct device_node *node = dev->of_node;
> -	struct lp855x_platform_data *pdata;
>  	int rom_length;
> 
>  	if (!node) {
> @@ -389,14 +391,10 @@ static int lp855x_parse_dt(struct lp855x *lp)
>  		return -EINVAL;
>  	}
> 
> -	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> -	if (!pdata)
> -		return -ENOMEM;
> -
> -	of_property_read_string(node, "bl-name", &pdata->name);
> -	of_property_read_u8(node, "dev-ctrl", &pdata->device_control);
> -	of_property_read_u8(node, "init-brt", &pdata->initial_brightness);
> -	of_property_read_u32(node, "pwm-period", &pdata->period_ns);
> +	of_property_read_string(node, "bl-name", &lp->blname);
> +	of_property_read_u8(node, "dev-ctrl", &lp->device_control);
> +	of_property_read_u8(node, "init-brt", &lp->initial_brightness);
> +	of_property_read_u32(node, "pwm-period", &lp->period_ns);
> 
>  	/* Fill ROM platform data if defined */
>  	rom_length = of_get_child_count(node);
> @@ -415,19 +413,16 @@ static int lp855x_parse_dt(struct lp855x *lp)
>  			i++;
>  		}
> 
> -		pdata->size_program = rom_length;
> -		pdata->rom_data = &rom[0];
> +		lp->size_program = rom_length;
> +		lp->rom_data = &rom[0];
>  	}
> 
> -	pdata->supply = devm_regulator_get(dev, "power");
> -	if (IS_ERR(pdata->supply)) {
> -		if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
> +	lp->supply = devm_regulator_get(dev, "power");
> +	if (IS_ERR(lp->supply)) {
> +		if (PTR_ERR(lp->supply) = -EPROBE_DEFER)
>  			return -EPROBE_DEFER;
> -		pdata->supply = NULL;

Is 'lp->supply' not optional?
If 'lp->supply' is optional, please add the following.

+		lp->supply = NULL;

>  	}
> 
> -	lp->pdata = pdata;
> -
>  	return 0;
>  }
>  #else
> @@ -453,21 +448,18 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>  	lp->dev = &cl->dev;
>  	lp->chipname = id->name;
>  	lp->chip_id = id->driver_data;
> -	lp->pdata = dev_get_platdata(&cl->dev);
> 
> -	if (!lp->pdata) {
> -		ret = lp855x_parse_dt(lp);
> -		if (ret < 0)
> -			return ret;
> -	}
> +	ret = lp855x_parse_dt(lp);
> +	if (ret < 0)
> +		return ret;
> 
> -	if (lp->pdata->period_ns > 0)
> +	if (lp->period_ns > 0)
>  		lp->mode = PWM_BASED;
>  	else
>  		lp->mode = REGISTER_BASED;
> 
> -	if (lp->pdata->supply) {
> -		ret = regulator_enable(lp->pdata->supply);
> +	if (lp->supply) {
> +		ret = regulator_enable(lp->supply);
>  		if (ret < 0) {
>  			dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
>  			return ret;
> @@ -505,8 +497,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);

If 'lp->supply' is optional, this "if (lp->supply)" might
be unnecessary.

Best regards,
Jingoo Han

>  	sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
> 
>  	return 0;
> --
> 2.2.0.rc0.207.ga3a616c


^ permalink raw reply

* Re: [PATCH 3/4] backlight/lp855x: Merge lp855x_platform_data with lp855x
From: Jingoo Han @ 2014-12-08 12:03 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1417805069-20441-3-git-send-email-seanpaul@chromium.org>

On Monday, December 08, 2014 9:01 PM, Jingoo Han wrote:
> 
> On Saturday, December 06, 2014 3:44 AM, Sean Paul wrote:
> >
> > Now that we have removed the platform_data header, merge lp855x_platform_data
> > with lp855x and remove all traces of platform_data from the driver.
> >
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/video/backlight/lp855x_bl.c | 100 +++++++++++++++++-------------------
> >  1 file changed, 46 insertions(+), 54 deletions(-)
> >
> > diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> > index d19b61c..8b81d8e 100644
> > --- a/drivers/video/backlight/lp855x_bl.c
> > +++ b/drivers/video/backlight/lp855x_bl.c
> > @@ -78,8 +78,16 @@ struct lp855x_rom_data {
> >  };
> >
> >  /**
> > - * struct lp855x_platform_data
> > - * @name : Backlight driver name. If it is not defined, default name is set.
> > + * struct lp855x
> > + * @chipname : Chip name, comes from the i2c_device_id
> > + * @blname : Backlight driver name. If it is not defined, default name is set.
> > + * @chip_id : The type of lp855x
> > + * @mode : Whether brightness is controlled via pwm or register
> > + * @cfg : Chip specific hooks & register offsets
> > + * @client : The i2c client
> > + * @bl : The backlight device
> > + * @dev : Device pointer
> > + * @pwm : The pwm device (if available)
> >   * @device_control : value of DEVICE CONTROL register
> >   * @initial_brightness : initial value of backlight brightness
> >   * @period_ns : platform specific pwm period value. unit is nano.
> > @@ -88,26 +96,23 @@ struct lp855x_rom_data {
> >   * @rom_data : list of new eeprom/eprom registers
> >   * @supply : regulator that supplies 3V input
> >   */
> > -struct lp855x_platform_data {
> > -	const char *name;
> > -	u8 device_control;
> > -	u8 initial_brightness;
> > -	unsigned int period_ns;
> > -	int size_program;
> > -	struct lp855x_rom_data *rom_data;
> > -	struct regulator *supply;
> > -};
> > -
> >  struct lp855x {
> >  	const char *chipname;
> > +	const char *blname;
> >  	enum lp855x_chip_id chip_id;
> >  	enum lp855x_brightness_ctrl_mode mode;
> >  	struct lp855x_device_config *cfg;
> >  	struct i2c_client *client;
> >  	struct backlight_device *bl;
> >  	struct device *dev;
> > -	struct lp855x_platform_data *pdata;
> >  	struct pwm_device *pwm;
> > +
> > +	u8 device_control;
> > +	u8 initial_brightness;
> > +	unsigned int period_ns;
> > +	int size_program;
> > +	struct lp855x_rom_data *rom_data;
> > +	struct regulator *supply;
> >  };
> 
> Now, 'struct lp855x_platform_data' is removed.
> But, there is still remaining in 'Documentation/backlight/lp855x-driver.txt'.
> Would you clean this lp855x-driver.txt file?
> 
> >
> >  static int lp855x_write_byte(struct lp855x *lp, u8 reg, u8 data)
> > @@ -204,7 +209,6 @@ static int lp855x_configure(struct lp855x *lp)
> >  {
> >  	u8 val, addr;
> >  	int i, ret;
> > -	struct lp855x_platform_data *pd = lp->pdata;
> >
> >  	switch (lp->chip_id) {
> >  	case LP8550:
> > @@ -230,20 +234,20 @@ static int lp855x_configure(struct lp855x *lp)
> >  		}
> >  	}
> >
> > -	val = pd->initial_brightness;
> > +	val = lp->initial_brightness;
> >  	ret = lp855x_write_byte(lp, lp->cfg->reg_brightness, val);
> >  	if (ret)
> >  		goto err;
> >
> > -	val = pd->device_control;
> > +	val = lp->device_control;
> >  	ret = lp855x_write_byte(lp, lp->cfg->reg_devicectrl, val);
> >  	if (ret)
> >  		goto err;
> >
> > -	if (pd->size_program > 0) {
> > -		for (i = 0; i < pd->size_program; i++) {
> > -			addr = pd->rom_data[i].addr;
> > -			val = pd->rom_data[i].val;
> > +	if (lp->size_program > 0) {
> > +		for (i = 0; i < lp->size_program; i++) {
> > +			addr = lp->rom_data[i].addr;
> > +			val = lp->rom_data[i].val;
> >  			if (!lp855x_is_valid_rom_area(lp, addr))
> >  				continue;
> >
> > @@ -269,7 +273,7 @@ err:
> >
> >  static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
> >  {
> > -	unsigned int period = lp->pdata->period_ns;
> > +	unsigned int period = lp->period_ns;
> >  	unsigned int duty = br * period / max_br;
> >  	struct pwm_device *pwm;
> >
> > @@ -320,16 +324,15 @@ static int lp855x_backlight_register(struct lp855x *lp)
> >  {
> >  	struct backlight_device *bl;
> >  	struct backlight_properties props;
> > -	struct lp855x_platform_data *pdata = lp->pdata;
> > -	const char *name = pdata->name ? : DEFAULT_BL_NAME;
> > +	const char *name = lp->blname ? : DEFAULT_BL_NAME;
> >
> >  	props.type = BACKLIGHT_PLATFORM;
> >  	props.max_brightness = MAX_BRIGHTNESS;
> >
> > -	if (pdata->initial_brightness > props.max_brightness)
> > -		pdata->initial_brightness = props.max_brightness;
> > +	if (lp->initial_brightness > props.max_brightness)
> > +		lp->initial_brightness = props.max_brightness;
> >
> > -	props.brightness = pdata->initial_brightness;
> > +	props.brightness = lp->initial_brightness;
> >
> >  	bl = devm_backlight_device_register(lp->dev, name, lp->dev, lp,
> >  				       &lp855x_bl_ops, &props);
> > @@ -381,7 +384,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
> >  {
> >  	struct device *dev = lp->dev;
> >  	struct device_node *node = dev->of_node;
> > -	struct lp855x_platform_data *pdata;
> >  	int rom_length;
> >
> >  	if (!node) {
> > @@ -389,14 +391,10 @@ static int lp855x_parse_dt(struct lp855x *lp)
> >  		return -EINVAL;
> >  	}
> >
> > -	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> > -	if (!pdata)
> > -		return -ENOMEM;
> > -
> > -	of_property_read_string(node, "bl-name", &pdata->name);
> > -	of_property_read_u8(node, "dev-ctrl", &pdata->device_control);
> > -	of_property_read_u8(node, "init-brt", &pdata->initial_brightness);
> > -	of_property_read_u32(node, "pwm-period", &pdata->period_ns);
> > +	of_property_read_string(node, "bl-name", &lp->blname);
> > +	of_property_read_u8(node, "dev-ctrl", &lp->device_control);
> > +	of_property_read_u8(node, "init-brt", &lp->initial_brightness);
> > +	of_property_read_u32(node, "pwm-period", &lp->period_ns);
> >
> >  	/* Fill ROM platform data if defined */
> >  	rom_length = of_get_child_count(node);
> > @@ -415,19 +413,16 @@ static int lp855x_parse_dt(struct lp855x *lp)
> >  			i++;
> >  		}
> >
> > -		pdata->size_program = rom_length;
> > -		pdata->rom_data = &rom[0];
> > +		lp->size_program = rom_length;
> > +		lp->rom_data = &rom[0];
> >  	}
> >
> > -	pdata->supply = devm_regulator_get(dev, "power");
> > -	if (IS_ERR(pdata->supply)) {
> > -		if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
> > +	lp->supply = devm_regulator_get(dev, "power");
> > +	if (IS_ERR(lp->supply)) {
> > +		if (PTR_ERR(lp->supply) = -EPROBE_DEFER)
> >  			return -EPROBE_DEFER;
> > -		pdata->supply = NULL;
> 
> Is 'lp->supply' not optional?
> If 'lp->supply' is optional, please add the following.
> 
> +		lp->supply = NULL;
> 
> >  	}
> >
> > -	lp->pdata = pdata;
> > -
> >  	return 0;
> >  }
> >  #else
> > @@ -453,21 +448,18 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> >  	lp->dev = &cl->dev;
> >  	lp->chipname = id->name;
> >  	lp->chip_id = id->driver_data;
> > -	lp->pdata = dev_get_platdata(&cl->dev);
> >
> > -	if (!lp->pdata) {
> > -		ret = lp855x_parse_dt(lp);
> > -		if (ret < 0)
> > -			return ret;
> > -	}
> > +	ret = lp855x_parse_dt(lp);
> > +	if (ret < 0)
> > +		return ret;
> >
> > -	if (lp->pdata->period_ns > 0)
> > +	if (lp->period_ns > 0)
> >  		lp->mode = PWM_BASED;
> >  	else
> >  		lp->mode = REGISTER_BASED;
> >
> > -	if (lp->pdata->supply) {
> > -		ret = regulator_enable(lp->pdata->supply);
> > +	if (lp->supply) {
> > +		ret = regulator_enable(lp->supply);
> >  		if (ret < 0) {
> >  			dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
> >  			return ret;
> > @@ -505,8 +497,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);
> 
> If 'lp->supply' is optional, this "if (lp->supply)" might
> be unnecessary.

Sorry for my typo.
Please refer to the following.

If 'lp->supply' is _NOT_ optional, this "if (lp->supply)" might
be unnecessary.


> 
> Best regards,
> Jingoo Han
> 
> >  	sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
> >
> >  	return 0;
> > --
> > 2.2.0.rc0.207.ga3a616c


^ permalink raw reply

* [PATCH v2 1/4] backlight/lp855x: Remove stale code from lp855x.h
From: Sean Paul @ 2014-12-08 22:21 UTC (permalink / raw)
  To: linux-fbdev

These aren't used by anyone, remove them.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
Changes in v2:
	- None

 include/linux/platform_data/lp855x.h | 100 -----------------------------------
 1 file changed, 100 deletions(-)

diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
index 9c7fd1e..9e3ac3c 100644
--- a/include/linux/platform_data/lp855x.h
+++ b/include/linux/platform_data/lp855x.h
@@ -12,65 +12,6 @@
 #ifndef _LP855X_H
 #define _LP855X_H
 
-#define BL_CTL_SHFT	(0)
-#define BRT_MODE_SHFT	(1)
-#define BRT_MODE_MASK	(0x06)
-
-/* Enable backlight. Only valid when BRT_MODE\x10(I2C only) */
-#define ENABLE_BL	(1)
-#define DISABLE_BL	(0)
-
-#define I2C_CONFIG(id)	id ## _I2C_CONFIG
-#define PWM_CONFIG(id)	id ## _PWM_CONFIG
-
-/* DEVICE CONTROL register - LP8550 */
-#define LP8550_PWM_CONFIG	(LP8550_PWM_ONLY << BRT_MODE_SHFT)
-#define LP8550_I2C_CONFIG	((ENABLE_BL << BL_CTL_SHFT) | \
-				(LP8550_I2C_ONLY << BRT_MODE_SHFT))
-
-/* DEVICE CONTROL register - LP8551 */
-#define LP8551_PWM_CONFIG	LP8550_PWM_CONFIG
-#define LP8551_I2C_CONFIG	LP8550_I2C_CONFIG
-
-/* DEVICE CONTROL register - LP8552 */
-#define LP8552_PWM_CONFIG	LP8550_PWM_CONFIG
-#define LP8552_I2C_CONFIG	LP8550_I2C_CONFIG
-
-/* DEVICE CONTROL register - LP8553 */
-#define LP8553_PWM_CONFIG	LP8550_PWM_CONFIG
-#define LP8553_I2C_CONFIG	LP8550_I2C_CONFIG
-
-/* CONFIG register - LP8555 */
-#define LP8555_PWM_STANDBY	BIT(7)
-#define LP8555_PWM_FILTER	BIT(6)
-#define LP8555_RELOAD_EPROM	BIT(3)	/* use it if EPROMs should be reset
-					   when the backlight turns on */
-#define LP8555_OFF_OPENLEDS	BIT(2)
-#define LP8555_PWM_CONFIG	LP8555_PWM_ONLY
-#define LP8555_I2C_CONFIG	LP8555_I2C_ONLY
-#define LP8555_COMB1_CONFIG	LP8555_COMBINED1
-#define LP8555_COMB2_CONFIG	LP8555_COMBINED2
-
-/* DEVICE CONTROL register - LP8556 */
-#define LP8556_PWM_CONFIG	(LP8556_PWM_ONLY << BRT_MODE_SHFT)
-#define LP8556_COMB1_CONFIG	(LP8556_COMBINED1 << BRT_MODE_SHFT)
-#define LP8556_I2C_CONFIG	((ENABLE_BL << BL_CTL_SHFT) | \
-				(LP8556_I2C_ONLY << BRT_MODE_SHFT))
-#define LP8556_COMB2_CONFIG	(LP8556_COMBINED2 << BRT_MODE_SHFT)
-#define LP8556_FAST_CONFIG	BIT(7) /* use it if EPROMs should be maintained
-					  when exiting the low power mode */
-
-/* CONFIG register - LP8557 */
-#define LP8557_PWM_STANDBY	BIT(7)
-#define LP8557_PWM_FILTER	BIT(6)
-#define LP8557_RELOAD_EPROM	BIT(3)	/* use it if EPROMs should be reset
-					   when the backlight turns on */
-#define LP8557_OFF_OPENLEDS	BIT(2)
-#define LP8557_PWM_CONFIG	LP8557_PWM_ONLY
-#define LP8557_I2C_CONFIG	LP8557_I2C_ONLY
-#define LP8557_COMB1_CONFIG	LP8557_COMBINED1
-#define LP8557_COMB2_CONFIG	LP8557_COMBINED2
-
 enum lp855x_chip_id {
 	LP8550,
 	LP8551,
@@ -81,47 +22,6 @@ enum lp855x_chip_id {
 	LP8557,
 };
 
-enum lp8550_brighntess_source {
-	LP8550_PWM_ONLY,
-	LP8550_I2C_ONLY = 2,
-};
-
-enum lp8551_brighntess_source {
-	LP8551_PWM_ONLY = LP8550_PWM_ONLY,
-	LP8551_I2C_ONLY = LP8550_I2C_ONLY,
-};
-
-enum lp8552_brighntess_source {
-	LP8552_PWM_ONLY = LP8550_PWM_ONLY,
-	LP8552_I2C_ONLY = LP8550_I2C_ONLY,
-};
-
-enum lp8553_brighntess_source {
-	LP8553_PWM_ONLY = LP8550_PWM_ONLY,
-	LP8553_I2C_ONLY = LP8550_I2C_ONLY,
-};
-
-enum lp8555_brightness_source {
-	LP8555_PWM_ONLY,
-	LP8555_I2C_ONLY,
-	LP8555_COMBINED1,	/* Brightness register with shaped PWM */
-	LP8555_COMBINED2,	/* PWM with shaped brightness register */
-};
-
-enum lp8556_brightness_source {
-	LP8556_PWM_ONLY,
-	LP8556_COMBINED1,	/* pwm + i2c before the shaper block */
-	LP8556_I2C_ONLY,
-	LP8556_COMBINED2,	/* pwm + i2c after the shaper block */
-};
-
-enum lp8557_brightness_source {
-	LP8557_PWM_ONLY,
-	LP8557_I2C_ONLY,
-	LP8557_COMBINED1,	/* pwm + i2c after the shaper block */
-	LP8557_COMBINED2,	/* pwm + i2c before the shaper block */
-};
-
 struct lp855x_rom_data {
 	u8 addr;
 	u8 val;
-- 
2.1.1


^ permalink raw reply related

* [PATCH v2 2/4] backlight/lp855x: Remove platform_data header
From: Sean Paul @ 2014-12-08 22:21 UTC (permalink / raw)
  To: linux-fbdev

No one uses lp855x platform data any longer, remove the header
and move its contents into the driver.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
Changes in v2:
	- Removed platform data description from Documentation

 Documentation/backlight/lp855x-driver.txt | 38 -----------------------
 MAINTAINERS                               |  1 -
 drivers/video/backlight/lp855x_bl.c       | 37 +++++++++++++++++++++-
 include/linux/platform_data/lp855x.h      | 51 -------------------------------
 4 files changed, 36 insertions(+), 91 deletions(-)
 delete mode 100644 include/linux/platform_data/lp855x.h

diff --git a/Documentation/backlight/lp855x-driver.txt b/Documentation/backlight/lp855x-driver.txt
index 01bce24..33b83d9 100644
--- a/Documentation/backlight/lp855x-driver.txt
+++ b/Documentation/backlight/lp855x-driver.txt
@@ -26,41 +26,3 @@ Value : pwm based or register based
 2) chip_id
 The lp855x chip id.
 Value : lp8550/lp8551/lp8552/lp8553/lp8555/lp8556/lp8557
-
-Platform data for lp855x
-------------------------
-
-For supporting platform specific data, the lp855x platform data can be used.
-
-* name : Backlight driver name. If it is not defined, default name is set.
-* device_control : Value of DEVICE CONTROL register.
-* initial_brightness : Initial value of backlight brightness.
-* period_ns : Platform specific PWM period value. unit is nano.
-	     Only valid when brightness is pwm input mode.
-* size_program : Total size of lp855x_rom_data.
-* rom_data : List of new eeprom/eprom registers.
-
-example 1) lp8552 platform data : i2c register mode with new eeprom data
-
-#define EEPROM_A5_ADDR	0xA5
-#define EEPROM_A5_VAL	0x4f	/* EN_VSYNC=0 */
-
-static struct lp855x_rom_data lp8552_eeprom_arr[] = {
-	{EEPROM_A5_ADDR, EEPROM_A5_VAL},
-};
-
-static struct lp855x_platform_data lp8552_pdata = {
-	.name = "lcd-bl",
-	.device_control = I2C_CONFIG(LP8552),
-	.initial_brightness = INITIAL_BRT,
-	.size_program = ARRAY_SIZE(lp8552_eeprom_arr),
-	.rom_data = lp8552_eeprom_arr,
-};
-
-example 2) lp8556 platform data : pwm input mode with default rom data
-
-static struct lp855x_platform_data lp8556_pdata = {
-	.device_control = PWM_CONFIG(LP8556),
-	.initial_brightness = INITIAL_BRT,
-	.period_ns = 1000000,
-};
diff --git a/MAINTAINERS b/MAINTAINERS
index 3c64271..4896edb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9318,7 +9318,6 @@ M:	Milo Kim <milo.kim@ti.com>
 S:	Maintained
 F:	Documentation/backlight/lp855x-driver.txt
 F:	drivers/video/backlight/lp855x_bl.c
-F:	include/linux/platform_data/lp855x.h
 
 TI LP8727 CHARGER DRIVER
 M:	Milo Kim <milo.kim@ti.com>
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index a26d3bb..d19b61c 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -15,7 +15,6 @@
 #include <linux/backlight.h>
 #include <linux/err.h>
 #include <linux/of.h>
-#include <linux/platform_data/lp855x.h>
 #include <linux/pwm.h>
 #include <linux/regulator/consumer.h>
 
@@ -63,6 +62,42 @@ struct lp855x_device_config {
 	int (*post_init_device)(struct lp855x *);
 };
 
+enum lp855x_chip_id {
+	LP8550,
+	LP8551,
+	LP8552,
+	LP8553,
+	LP8555,
+	LP8556,
+	LP8557,
+};
+
+struct lp855x_rom_data {
+	u8 addr;
+	u8 val;
+};
+
+/**
+ * struct lp855x_platform_data
+ * @name : Backlight driver name. If it is not defined, default name is set.
+ * @device_control : value of DEVICE CONTROL register
+ * @initial_brightness : initial value of backlight brightness
+ * @period_ns : platform specific pwm period value. unit is nano.
+		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;
+	u8 device_control;
+	u8 initial_brightness;
+	unsigned int period_ns;
+	int size_program;
+	struct lp855x_rom_data *rom_data;
+	struct regulator *supply;
+};
+
 struct lp855x {
 	const char *chipname;
 	enum lp855x_chip_id chip_id;
diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
deleted file mode 100644
index 9e3ac3c..0000000
--- a/include/linux/platform_data/lp855x.h
+++ /dev/null
@@ -1,51 +0,0 @@
-/*
- * LP855x Backlight Driver
- *
- *			Copyright (C) 2011 Texas Instruments
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- */
-
-#ifndef _LP855X_H
-#define _LP855X_H
-
-enum lp855x_chip_id {
-	LP8550,
-	LP8551,
-	LP8552,
-	LP8553,
-	LP8555,
-	LP8556,
-	LP8557,
-};
-
-struct lp855x_rom_data {
-	u8 addr;
-	u8 val;
-};
-
-/**
- * struct lp855x_platform_data
- * @name : Backlight driver name. If it is not defined, default name is set.
- * @device_control : value of DEVICE CONTROL register
- * @initial_brightness : initial value of backlight brightness
- * @period_ns : platform specific pwm period value. unit is nano.
-		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;
-	u8 device_control;
-	u8 initial_brightness;
-	unsigned int period_ns;
-	int size_program;
-	struct lp855x_rom_data *rom_data;
-	struct regulator *supply;
-};
-
-#endif
-- 
2.1.1


^ permalink raw reply related

* [PATCH v2 3/4] backlight/lp855x: Merge lp855x_platform_data with lp855x
From: Sean Paul @ 2014-12-08 22:21 UTC (permalink / raw)
  To: linux-fbdev

Now that we have removed the platform_data header, merge lp855x_platform_data
with lp855x and remove all traces of platform_data from the driver.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
Changes in v2:
	- Assign NULL to lp->supply if devm_regulator_get fails

 drivers/video/backlight/lp855x_bl.c | 101 +++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 54 deletions(-)

diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index d19b61c..6530d60 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -78,8 +78,16 @@ struct lp855x_rom_data {
 };
 
 /**
- * struct lp855x_platform_data
- * @name : Backlight driver name. If it is not defined, default name is set.
+ * struct lp855x
+ * @chipname : Chip name, comes from the i2c_device_id
+ * @blname : Backlight driver name. If it is not defined, default name is set.
+ * @chip_id : The type of lp855x
+ * @mode : Whether brightness is controlled via pwm or register
+ * @cfg : Chip specific hooks & register offsets
+ * @client : The i2c client
+ * @bl : The backlight device
+ * @dev : Device pointer
+ * @pwm : The pwm device (if available)
  * @device_control : value of DEVICE CONTROL register
  * @initial_brightness : initial value of backlight brightness
  * @period_ns : platform specific pwm period value. unit is nano.
@@ -88,26 +96,23 @@ struct lp855x_rom_data {
  * @rom_data : list of new eeprom/eprom registers
  * @supply : regulator that supplies 3V input
  */
-struct lp855x_platform_data {
-	const char *name;
-	u8 device_control;
-	u8 initial_brightness;
-	unsigned int period_ns;
-	int size_program;
-	struct lp855x_rom_data *rom_data;
-	struct regulator *supply;
-};
-
 struct lp855x {
 	const char *chipname;
+	const char *blname;
 	enum lp855x_chip_id chip_id;
 	enum lp855x_brightness_ctrl_mode mode;
 	struct lp855x_device_config *cfg;
 	struct i2c_client *client;
 	struct backlight_device *bl;
 	struct device *dev;
-	struct lp855x_platform_data *pdata;
 	struct pwm_device *pwm;
+
+	u8 device_control;
+	u8 initial_brightness;
+	unsigned int period_ns;
+	int size_program;
+	struct lp855x_rom_data *rom_data;
+	struct regulator *supply;
 };
 
 static int lp855x_write_byte(struct lp855x *lp, u8 reg, u8 data)
@@ -204,7 +209,6 @@ static int lp855x_configure(struct lp855x *lp)
 {
 	u8 val, addr;
 	int i, ret;
-	struct lp855x_platform_data *pd = lp->pdata;
 
 	switch (lp->chip_id) {
 	case LP8550:
@@ -230,20 +234,20 @@ static int lp855x_configure(struct lp855x *lp)
 		}
 	}
 
-	val = pd->initial_brightness;
+	val = lp->initial_brightness;
 	ret = lp855x_write_byte(lp, lp->cfg->reg_brightness, val);
 	if (ret)
 		goto err;
 
-	val = pd->device_control;
+	val = lp->device_control;
 	ret = lp855x_write_byte(lp, lp->cfg->reg_devicectrl, val);
 	if (ret)
 		goto err;
 
-	if (pd->size_program > 0) {
-		for (i = 0; i < pd->size_program; i++) {
-			addr = pd->rom_data[i].addr;
-			val = pd->rom_data[i].val;
+	if (lp->size_program > 0) {
+		for (i = 0; i < lp->size_program; i++) {
+			addr = lp->rom_data[i].addr;
+			val = lp->rom_data[i].val;
 			if (!lp855x_is_valid_rom_area(lp, addr))
 				continue;
 
@@ -269,7 +273,7 @@ err:
 
 static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
 {
-	unsigned int period = lp->pdata->period_ns;
+	unsigned int period = lp->period_ns;
 	unsigned int duty = br * period / max_br;
 	struct pwm_device *pwm;
 
@@ -320,16 +324,15 @@ static int lp855x_backlight_register(struct lp855x *lp)
 {
 	struct backlight_device *bl;
 	struct backlight_properties props;
-	struct lp855x_platform_data *pdata = lp->pdata;
-	const char *name = pdata->name ? : DEFAULT_BL_NAME;
+	const char *name = lp->blname ? : DEFAULT_BL_NAME;
 
 	props.type = BACKLIGHT_PLATFORM;
 	props.max_brightness = MAX_BRIGHTNESS;
 
-	if (pdata->initial_brightness > props.max_brightness)
-		pdata->initial_brightness = props.max_brightness;
+	if (lp->initial_brightness > props.max_brightness)
+		lp->initial_brightness = props.max_brightness;
 
-	props.brightness = pdata->initial_brightness;
+	props.brightness = lp->initial_brightness;
 
 	bl = devm_backlight_device_register(lp->dev, name, lp->dev, lp,
 				       &lp855x_bl_ops, &props);
@@ -381,7 +384,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
 {
 	struct device *dev = lp->dev;
 	struct device_node *node = dev->of_node;
-	struct lp855x_platform_data *pdata;
 	int rom_length;
 
 	if (!node) {
@@ -389,14 +391,10 @@ static int lp855x_parse_dt(struct lp855x *lp)
 		return -EINVAL;
 	}
 
-	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return -ENOMEM;
-
-	of_property_read_string(node, "bl-name", &pdata->name);
-	of_property_read_u8(node, "dev-ctrl", &pdata->device_control);
-	of_property_read_u8(node, "init-brt", &pdata->initial_brightness);
-	of_property_read_u32(node, "pwm-period", &pdata->period_ns);
+	of_property_read_string(node, "bl-name", &lp->blname);
+	of_property_read_u8(node, "dev-ctrl", &lp->device_control);
+	of_property_read_u8(node, "init-brt", &lp->initial_brightness);
+	of_property_read_u32(node, "pwm-period", &lp->period_ns);
 
 	/* Fill ROM platform data if defined */
 	rom_length = of_get_child_count(node);
@@ -415,19 +413,17 @@ static int lp855x_parse_dt(struct lp855x *lp)
 			i++;
 		}
 
-		pdata->size_program = rom_length;
-		pdata->rom_data = &rom[0];
+		lp->size_program = rom_length;
+		lp->rom_data = &rom[0];
 	}
 
-	pdata->supply = devm_regulator_get(dev, "power");
-	if (IS_ERR(pdata->supply)) {
-		if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
+	lp->supply = devm_regulator_get(dev, "power");
+	if (IS_ERR(lp->supply)) {
+		if (PTR_ERR(lp->supply) = -EPROBE_DEFER)
 			return -EPROBE_DEFER;
-		pdata->supply = NULL;
+		lp->supply = NULL;
 	}
 
-	lp->pdata = pdata;
-
 	return 0;
 }
 #else
@@ -453,21 +449,18 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 	lp->dev = &cl->dev;
 	lp->chipname = id->name;
 	lp->chip_id = id->driver_data;
-	lp->pdata = dev_get_platdata(&cl->dev);
 
-	if (!lp->pdata) {
-		ret = lp855x_parse_dt(lp);
-		if (ret < 0)
-			return ret;
-	}
+	ret = lp855x_parse_dt(lp);
+	if (ret < 0)
+		return ret;
 
-	if (lp->pdata->period_ns > 0)
+	if (lp->period_ns > 0)
 		lp->mode = PWM_BASED;
 	else
 		lp->mode = REGISTER_BASED;
 
-	if (lp->pdata->supply) {
-		ret = regulator_enable(lp->pdata->supply);
+	if (lp->supply) {
+		ret = regulator_enable(lp->supply);
 		if (ret < 0) {
 			dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
 			return ret;
@@ -505,8 +498,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;
-- 
2.1.1


^ permalink raw reply related

* [PATCH v2 4/4] backlight/lp855x: Remove CONFIG_OF ifdef in favor of Kconfig depends
From: Sean Paul @ 2014-12-08 22:21 UTC (permalink / raw)
  To: linux-fbdev

Now that we've removed all traces of pdata, remove the CONFIG_OF ifdef
from lp855x and instead make the driver depend on OF.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
Changes in v2:
	- Remove of_match_ptr() wrapper

 drivers/video/backlight/Kconfig     | 2 +-
 drivers/video/backlight/lp855x_bl.c | 9 +--------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8d03924..113c8b3 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -384,7 +384,7 @@ config BACKLIGHT_LM3639
 
 config BACKLIGHT_LP855X
 	tristate "Backlight driver for TI LP855X"
-	depends on BACKLIGHT_CLASS_DEVICE && I2C && PWM
+	depends on BACKLIGHT_CLASS_DEVICE && I2C && PWM && OF
 	help
 	  This supports TI LP8550, LP8551, LP8552, LP8553, LP8555, LP8556 and
 	  LP8557 backlight driver.
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index 6530d60..f0a85f5 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -379,7 +379,6 @@ static const struct attribute_group lp855x_attr_group = {
 	.attrs = lp855x_attributes,
 };
 
-#ifdef CONFIG_OF
 static int lp855x_parse_dt(struct lp855x *lp)
 {
 	struct device *dev = lp->dev;
@@ -426,12 +425,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
 
 	return 0;
 }
-#else
-static int lp855x_parse_dt(struct lp855x *lp)
-{
-	return -EINVAL;
-}
-#endif
 
 static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 {
@@ -532,7 +525,7 @@ MODULE_DEVICE_TABLE(i2c, lp855x_ids);
 static struct i2c_driver lp855x_driver = {
 	.driver = {
 		   .name = "lp855x",
-		   .of_match_table = of_match_ptr(lp855x_dt_ids),
+		   .of_match_table = lp855x_dt_ids,
 		   },
 	.probe = lp855x_probe,
 	.remove = lp855x_remove,
-- 
2.1.1


^ permalink raw reply related

* Re: [PATCH v2 1/4] backlight/lp855x: Remove stale code from lp855x.h
From: Jingoo Han @ 2014-12-09  1:25 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1418077278-14144-1-git-send-email-seanpaul@chromium.org>

On Tuesday, December 09, 2014 7:21 AM, Sean Paul wrote:
> 
> These aren't used by anyone, remove them.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
> Changes in v2:
> 	- None
> 
>  include/linux/platform_data/lp855x.h | 100 -----------------------------------
>  1 file changed, 100 deletions(-)
> 
> diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
> index 9c7fd1e..9e3ac3c 100644
> --- a/include/linux/platform_data/lp855x.h
> +++ b/include/linux/platform_data/lp855x.h
> @@ -12,65 +12,6 @@
>  #ifndef _LP855X_H
>  #define _LP855X_H
> 
> -#define BL_CTL_SHFT	(0)
> -#define BRT_MODE_SHFT	(1)
> -#define BRT_MODE_MASK	(0x06)
> -
> -/* Enable backlight. Only valid when BRT_MODE\x10(I2C only) */
> -#define ENABLE_BL	(1)
> -#define DISABLE_BL	(0)
> -
> -#define I2C_CONFIG(id)	id ## _I2C_CONFIG
> -#define PWM_CONFIG(id)	id ## _PWM_CONFIG
> -
> -/* DEVICE CONTROL register - LP8550 */
> -#define LP8550_PWM_CONFIG	(LP8550_PWM_ONLY << BRT_MODE_SHFT)
> -#define LP8550_I2C_CONFIG	((ENABLE_BL << BL_CTL_SHFT) | \
> -				(LP8550_I2C_ONLY << BRT_MODE_SHFT))
> -
> -/* DEVICE CONTROL register - LP8551 */
> -#define LP8551_PWM_CONFIG	LP8550_PWM_CONFIG
> -#define LP8551_I2C_CONFIG	LP8550_I2C_CONFIG
> -
> -/* DEVICE CONTROL register - LP8552 */
> -#define LP8552_PWM_CONFIG	LP8550_PWM_CONFIG
> -#define LP8552_I2C_CONFIG	LP8550_I2C_CONFIG
> -
> -/* DEVICE CONTROL register - LP8553 */
> -#define LP8553_PWM_CONFIG	LP8550_PWM_CONFIG
> -#define LP8553_I2C_CONFIG	LP8550_I2C_CONFIG
> -
> -/* CONFIG register - LP8555 */
> -#define LP8555_PWM_STANDBY	BIT(7)
> -#define LP8555_PWM_FILTER	BIT(6)
> -#define LP8555_RELOAD_EPROM	BIT(3)	/* use it if EPROMs should be reset
> -					   when the backlight turns on */
> -#define LP8555_OFF_OPENLEDS	BIT(2)
> -#define LP8555_PWM_CONFIG	LP8555_PWM_ONLY
> -#define LP8555_I2C_CONFIG	LP8555_I2C_ONLY
> -#define LP8555_COMB1_CONFIG	LP8555_COMBINED1
> -#define LP8555_COMB2_CONFIG	LP8555_COMBINED2
> -
> -/* DEVICE CONTROL register - LP8556 */
> -#define LP8556_PWM_CONFIG	(LP8556_PWM_ONLY << BRT_MODE_SHFT)
> -#define LP8556_COMB1_CONFIG	(LP8556_COMBINED1 << BRT_MODE_SHFT)
> -#define LP8556_I2C_CONFIG	((ENABLE_BL << BL_CTL_SHFT) | \
> -				(LP8556_I2C_ONLY << BRT_MODE_SHFT))
> -#define LP8556_COMB2_CONFIG	(LP8556_COMBINED2 << BRT_MODE_SHFT)
> -#define LP8556_FAST_CONFIG	BIT(7) /* use it if EPROMs should be maintained
> -					  when exiting the low power mode */
> -
> -/* CONFIG register - LP8557 */
> -#define LP8557_PWM_STANDBY	BIT(7)
> -#define LP8557_PWM_FILTER	BIT(6)
> -#define LP8557_RELOAD_EPROM	BIT(3)	/* use it if EPROMs should be reset
> -					   when the backlight turns on */
> -#define LP8557_OFF_OPENLEDS	BIT(2)
> -#define LP8557_PWM_CONFIG	LP8557_PWM_ONLY
> -#define LP8557_I2C_CONFIG	LP8557_I2C_ONLY
> -#define LP8557_COMB1_CONFIG	LP8557_COMBINED1
> -#define LP8557_COMB2_CONFIG	LP8557_COMBINED2
> -
>  enum lp855x_chip_id {
>  	LP8550,
>  	LP8551,
> @@ -81,47 +22,6 @@ enum lp855x_chip_id {
>  	LP8557,
>  };
> 
> -enum lp8550_brighntess_source {
> -	LP8550_PWM_ONLY,
> -	LP8550_I2C_ONLY = 2,
> -};
> -
> -enum lp8551_brighntess_source {
> -	LP8551_PWM_ONLY = LP8550_PWM_ONLY,
> -	LP8551_I2C_ONLY = LP8550_I2C_ONLY,
> -};
> -
> -enum lp8552_brighntess_source {
> -	LP8552_PWM_ONLY = LP8550_PWM_ONLY,
> -	LP8552_I2C_ONLY = LP8550_I2C_ONLY,
> -};
> -
> -enum lp8553_brighntess_source {
> -	LP8553_PWM_ONLY = LP8550_PWM_ONLY,
> -	LP8553_I2C_ONLY = LP8550_I2C_ONLY,
> -};
> -
> -enum lp8555_brightness_source {
> -	LP8555_PWM_ONLY,
> -	LP8555_I2C_ONLY,
> -	LP8555_COMBINED1,	/* Brightness register with shaped PWM */
> -	LP8555_COMBINED2,	/* PWM with shaped brightness register */
> -};
> -
> -enum lp8556_brightness_source {
> -	LP8556_PWM_ONLY,
> -	LP8556_COMBINED1,	/* pwm + i2c before the shaper block */
> -	LP8556_I2C_ONLY,
> -	LP8556_COMBINED2,	/* pwm + i2c after the shaper block */
> -};
> -
> -enum lp8557_brightness_source {
> -	LP8557_PWM_ONLY,
> -	LP8557_I2C_ONLY,
> -	LP8557_COMBINED1,	/* pwm + i2c after the shaper block */
> -	LP8557_COMBINED2,	/* pwm + i2c before the shaper block */
> -};
> -
>  struct lp855x_rom_data {
>  	u8 addr;
>  	u8 val;
> --
> 2.1.1


^ permalink raw reply

* Re: [PATCH v2 2/4] backlight/lp855x: Remove platform_data header
From: Jingoo Han @ 2014-12-09  1:26 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1418077278-14144-2-git-send-email-seanpaul@chromium.org>

On Tuesday, December 09, 2014 7:21 AM, Sean Paul wrote:
> 
> No one uses lp855x platform data any longer, remove the header
> and move its contents into the driver.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
> Changes in v2:
> 	- Removed platform data description from Documentation
> 
>  Documentation/backlight/lp855x-driver.txt | 38 -----------------------
>  MAINTAINERS                               |  1 -
>  drivers/video/backlight/lp855x_bl.c       | 37 +++++++++++++++++++++-
>  include/linux/platform_data/lp855x.h      | 51 -------------------------------
>  4 files changed, 36 insertions(+), 91 deletions(-)
>  delete mode 100644 include/linux/platform_data/lp855x.h
> 
> diff --git a/Documentation/backlight/lp855x-driver.txt b/Documentation/backlight/lp855x-driver.txt
> index 01bce24..33b83d9 100644
> --- a/Documentation/backlight/lp855x-driver.txt
> +++ b/Documentation/backlight/lp855x-driver.txt
> @@ -26,41 +26,3 @@ Value : pwm based or register based
>  2) chip_id
>  The lp855x chip id.
>  Value : lp8550/lp8551/lp8552/lp8553/lp8555/lp8556/lp8557
> -
> -Platform data for lp855x
> -------------------------
> -
> -For supporting platform specific data, the lp855x platform data can be used.
> -
> -* name : Backlight driver name. If it is not defined, default name is set.
> -* device_control : Value of DEVICE CONTROL register.
> -* initial_brightness : Initial value of backlight brightness.
> -* period_ns : Platform specific PWM period value. unit is nano.
> -	     Only valid when brightness is pwm input mode.
> -* size_program : Total size of lp855x_rom_data.
> -* rom_data : List of new eeprom/eprom registers.
> -
> -example 1) lp8552 platform data : i2c register mode with new eeprom data
> -
> -#define EEPROM_A5_ADDR	0xA5
> -#define EEPROM_A5_VAL	0x4f	/* EN_VSYNC=0 */
> -
> -static struct lp855x_rom_data lp8552_eeprom_arr[] = {
> -	{EEPROM_A5_ADDR, EEPROM_A5_VAL},
> -};
> -
> -static struct lp855x_platform_data lp8552_pdata = {
> -	.name = "lcd-bl",
> -	.device_control = I2C_CONFIG(LP8552),
> -	.initial_brightness = INITIAL_BRT,
> -	.size_program = ARRAY_SIZE(lp8552_eeprom_arr),
> -	.rom_data = lp8552_eeprom_arr,
> -};
> -
> -example 2) lp8556 platform data : pwm input mode with default rom data
> -
> -static struct lp855x_platform_data lp8556_pdata = {
> -	.device_control = PWM_CONFIG(LP8556),
> -	.initial_brightness = INITIAL_BRT,
> -	.period_ns = 1000000,
> -};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3c64271..4896edb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9318,7 +9318,6 @@ M:	Milo Kim <milo.kim@ti.com>
>  S:	Maintained
>  F:	Documentation/backlight/lp855x-driver.txt
>  F:	drivers/video/backlight/lp855x_bl.c
> -F:	include/linux/platform_data/lp855x.h
> 
>  TI LP8727 CHARGER DRIVER
>  M:	Milo Kim <milo.kim@ti.com>
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index a26d3bb..d19b61c 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -15,7 +15,6 @@
>  #include <linux/backlight.h>
>  #include <linux/err.h>
>  #include <linux/of.h>
> -#include <linux/platform_data/lp855x.h>
>  #include <linux/pwm.h>
>  #include <linux/regulator/consumer.h>
> 
> @@ -63,6 +62,42 @@ struct lp855x_device_config {
>  	int (*post_init_device)(struct lp855x *);
>  };
> 
> +enum lp855x_chip_id {
> +	LP8550,
> +	LP8551,
> +	LP8552,
> +	LP8553,
> +	LP8555,
> +	LP8556,
> +	LP8557,
> +};
> +
> +struct lp855x_rom_data {
> +	u8 addr;
> +	u8 val;
> +};
> +
> +/**
> + * struct lp855x_platform_data
> + * @name : Backlight driver name. If it is not defined, default name is set.
> + * @device_control : value of DEVICE CONTROL register
> + * @initial_brightness : initial value of backlight brightness
> + * @period_ns : platform specific pwm period value. unit is nano.
> +		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;
> +	u8 device_control;
> +	u8 initial_brightness;
> +	unsigned int period_ns;
> +	int size_program;
> +	struct lp855x_rom_data *rom_data;
> +	struct regulator *supply;
> +};
> +
>  struct lp855x {
>  	const char *chipname;
>  	enum lp855x_chip_id chip_id;
> diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
> deleted file mode 100644
> index 9e3ac3c..0000000
> --- a/include/linux/platform_data/lp855x.h
> +++ /dev/null
> @@ -1,51 +0,0 @@
> -/*
> - * LP855x Backlight Driver
> - *
> - *			Copyright (C) 2011 Texas Instruments
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - *
> - */
> -
> -#ifndef _LP855X_H
> -#define _LP855X_H
> -
> -enum lp855x_chip_id {
> -	LP8550,
> -	LP8551,
> -	LP8552,
> -	LP8553,
> -	LP8555,
> -	LP8556,
> -	LP8557,
> -};
> -
> -struct lp855x_rom_data {
> -	u8 addr;
> -	u8 val;
> -};
> -
> -/**
> - * struct lp855x_platform_data
> - * @name : Backlight driver name. If it is not defined, default name is set.
> - * @device_control : value of DEVICE CONTROL register
> - * @initial_brightness : initial value of backlight brightness
> - * @period_ns : platform specific pwm period value. unit is nano.
> -		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;
> -	u8 device_control;
> -	u8 initial_brightness;
> -	unsigned int period_ns;
> -	int size_program;
> -	struct lp855x_rom_data *rom_data;
> -	struct regulator *supply;
> -};
> -
> -#endif
> --
> 2.1.1


^ permalink raw reply

* Re: [PATCH v2 3/4] backlight/lp855x: Merge lp855x_platform_data with lp855x
From: Jingoo Han @ 2014-12-09  1:35 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1418077278-14144-3-git-send-email-seanpaul@chromium.org>

On Tuesday, December 09, 2014 7:21 AM, Sean Paul wrote:
> 
> Now that we have removed the platform_data header, merge lp855x_platform_data
> with lp855x and remove all traces of platform_data from the driver.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
> Changes in v2:
> 	- Assign NULL to lp->supply if devm_regulator_get fails
> 
>  drivers/video/backlight/lp855x_bl.c | 101 +++++++++++++++++-------------------
>  1 file changed, 47 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index d19b61c..6530d60 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -78,8 +78,16 @@ struct lp855x_rom_data {
>  };
> 
>  /**
> - * struct lp855x_platform_data
> - * @name : Backlight driver name. If it is not defined, default name is set.
> + * struct lp855x
> + * @chipname : Chip name, comes from the i2c_device_id
> + * @blname : Backlight driver name. If it is not defined, default name is set.
> + * @chip_id : The type of lp855x
> + * @mode : Whether brightness is controlled via pwm or register
> + * @cfg : Chip specific hooks & register offsets
> + * @client : The i2c client
> + * @bl : The backlight device
> + * @dev : Device pointer
> + * @pwm : The pwm device (if available)
>   * @device_control : value of DEVICE CONTROL register
>   * @initial_brightness : initial value of backlight brightness
>   * @period_ns : platform specific pwm period value. unit is nano.
> @@ -88,26 +96,23 @@ struct lp855x_rom_data {
>   * @rom_data : list of new eeprom/eprom registers
>   * @supply : regulator that supplies 3V input
>   */
> -struct lp855x_platform_data {
> -	const char *name;
> -	u8 device_control;
> -	u8 initial_brightness;
> -	unsigned int period_ns;
> -	int size_program;
> -	struct lp855x_rom_data *rom_data;
> -	struct regulator *supply;
> -};
> -
>  struct lp855x {
>  	const char *chipname;
> +	const char *blname;
>  	enum lp855x_chip_id chip_id;
>  	enum lp855x_brightness_ctrl_mode mode;
>  	struct lp855x_device_config *cfg;
>  	struct i2c_client *client;
>  	struct backlight_device *bl;
>  	struct device *dev;
> -	struct lp855x_platform_data *pdata;
>  	struct pwm_device *pwm;
> +
> +	u8 device_control;
> +	u8 initial_brightness;
> +	unsigned int period_ns;
> +	int size_program;
> +	struct lp855x_rom_data *rom_data;
> +	struct regulator *supply;
>  };
> 
>  static int lp855x_write_byte(struct lp855x *lp, u8 reg, u8 data)
> @@ -204,7 +209,6 @@ static int lp855x_configure(struct lp855x *lp)
>  {
>  	u8 val, addr;
>  	int i, ret;
> -	struct lp855x_platform_data *pd = lp->pdata;
> 
>  	switch (lp->chip_id) {
>  	case LP8550:
> @@ -230,20 +234,20 @@ static int lp855x_configure(struct lp855x *lp)
>  		}
>  	}
> 
> -	val = pd->initial_brightness;
> +	val = lp->initial_brightness;
>  	ret = lp855x_write_byte(lp, lp->cfg->reg_brightness, val);
>  	if (ret)
>  		goto err;
> 
> -	val = pd->device_control;
> +	val = lp->device_control;
>  	ret = lp855x_write_byte(lp, lp->cfg->reg_devicectrl, val);
>  	if (ret)
>  		goto err;
> 
> -	if (pd->size_program > 0) {
> -		for (i = 0; i < pd->size_program; i++) {
> -			addr = pd->rom_data[i].addr;
> -			val = pd->rom_data[i].val;
> +	if (lp->size_program > 0) {
> +		for (i = 0; i < lp->size_program; i++) {
> +			addr = lp->rom_data[i].addr;
> +			val = lp->rom_data[i].val;
>  			if (!lp855x_is_valid_rom_area(lp, addr))
>  				continue;
> 
> @@ -269,7 +273,7 @@ err:
> 
>  static void lp855x_pwm_ctrl(struct lp855x *lp, int br, int max_br)
>  {
> -	unsigned int period = lp->pdata->period_ns;
> +	unsigned int period = lp->period_ns;
>  	unsigned int duty = br * period / max_br;
>  	struct pwm_device *pwm;
> 
> @@ -320,16 +324,15 @@ static int lp855x_backlight_register(struct lp855x *lp)
>  {
>  	struct backlight_device *bl;
>  	struct backlight_properties props;
> -	struct lp855x_platform_data *pdata = lp->pdata;
> -	const char *name = pdata->name ? : DEFAULT_BL_NAME;
> +	const char *name = lp->blname ? : DEFAULT_BL_NAME;
> 
>  	props.type = BACKLIGHT_PLATFORM;
>  	props.max_brightness = MAX_BRIGHTNESS;
> 
> -	if (pdata->initial_brightness > props.max_brightness)
> -		pdata->initial_brightness = props.max_brightness;
> +	if (lp->initial_brightness > props.max_brightness)
> +		lp->initial_brightness = props.max_brightness;
> 
> -	props.brightness = pdata->initial_brightness;
> +	props.brightness = lp->initial_brightness;
> 
>  	bl = devm_backlight_device_register(lp->dev, name, lp->dev, lp,
>  				       &lp855x_bl_ops, &props);
> @@ -381,7 +384,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
>  {
>  	struct device *dev = lp->dev;
>  	struct device_node *node = dev->of_node;
> -	struct lp855x_platform_data *pdata;
>  	int rom_length;
> 
>  	if (!node) {
> @@ -389,14 +391,10 @@ static int lp855x_parse_dt(struct lp855x *lp)
>  		return -EINVAL;
>  	}
> 
> -	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> -	if (!pdata)
> -		return -ENOMEM;
> -
> -	of_property_read_string(node, "bl-name", &pdata->name);
> -	of_property_read_u8(node, "dev-ctrl", &pdata->device_control);
> -	of_property_read_u8(node, "init-brt", &pdata->initial_brightness);
> -	of_property_read_u32(node, "pwm-period", &pdata->period_ns);
> +	of_property_read_string(node, "bl-name", &lp->blname);
> +	of_property_read_u8(node, "dev-ctrl", &lp->device_control);
> +	of_property_read_u8(node, "init-brt", &lp->initial_brightness);
> +	of_property_read_u32(node, "pwm-period", &lp->period_ns);
> 
>  	/* Fill ROM platform data if defined */
>  	rom_length = of_get_child_count(node);
> @@ -415,19 +413,17 @@ static int lp855x_parse_dt(struct lp855x *lp)
>  			i++;
>  		}
> 
> -		pdata->size_program = rom_length;
> -		pdata->rom_data = &rom[0];
> +		lp->size_program = rom_length;
> +		lp->rom_data = &rom[0];
>  	}
> 
> -	pdata->supply = devm_regulator_get(dev, "power");
> -	if (IS_ERR(pdata->supply)) {
> -		if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
> +	lp->supply = devm_regulator_get(dev, "power");
> +	if (IS_ERR(lp->supply)) {
> +		if (PTR_ERR(lp->supply) = -EPROBE_DEFER)
>  			return -EPROBE_DEFER;
> -		pdata->supply = NULL;
> +		lp->supply = NULL;
>  	}
> 
> -	lp->pdata = pdata;
> -
>  	return 0;
>  }
>  #else
> @@ -453,21 +449,18 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>  	lp->dev = &cl->dev;
>  	lp->chipname = id->name;
>  	lp->chip_id = id->driver_data;
> -	lp->pdata = dev_get_platdata(&cl->dev);
> 
> -	if (!lp->pdata) {
> -		ret = lp855x_parse_dt(lp);
> -		if (ret < 0)
> -			return ret;
> -	}
> +	ret = lp855x_parse_dt(lp);
> +	if (ret < 0)
> +		return ret;
> 
> -	if (lp->pdata->period_ns > 0)
> +	if (lp->period_ns > 0)
>  		lp->mode = PWM_BASED;
>  	else
>  		lp->mode = REGISTER_BASED;
> 
> -	if (lp->pdata->supply) {
> -		ret = regulator_enable(lp->pdata->supply);
> +	if (lp->supply) {
> +		ret = regulator_enable(lp->supply);
>  		if (ret < 0) {
>  			dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
>  			return ret;
> @@ -505,8 +498,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;
> --
> 2.1.1


^ permalink raw reply

* Re: [PATCH v2 4/4] backlight/lp855x: Remove CONFIG_OF ifdef in favor of Kconfig depends
From: Jingoo Han @ 2014-12-09  1:36 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1418077278-14144-4-git-send-email-seanpaul@chromium.org>

On Tuesday, December 09, 2014 7:21 AM, Sean Paul wrote:
> 
> Now that we've removed all traces of pdata, remove the CONFIG_OF ifdef
> from lp855x and instead make the driver depend on OF.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
> Changes in v2:
> 	- Remove of_match_ptr() wrapper
> 
>  drivers/video/backlight/Kconfig     | 2 +-
>  drivers/video/backlight/lp855x_bl.c | 9 +--------
>  2 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 8d03924..113c8b3 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -384,7 +384,7 @@ config BACKLIGHT_LM3639
> 
>  config BACKLIGHT_LP855X
>  	tristate "Backlight driver for TI LP855X"
> -	depends on BACKLIGHT_CLASS_DEVICE && I2C && PWM
> +	depends on BACKLIGHT_CLASS_DEVICE && I2C && PWM && OF
>  	help
>  	  This supports TI LP8550, LP8551, LP8552, LP8553, LP8555, LP8556 and
>  	  LP8557 backlight driver.
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 6530d60..f0a85f5 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -379,7 +379,6 @@ static const struct attribute_group lp855x_attr_group = {
>  	.attrs = lp855x_attributes,
>  };
> 
> -#ifdef CONFIG_OF
>  static int lp855x_parse_dt(struct lp855x *lp)
>  {
>  	struct device *dev = lp->dev;
> @@ -426,12 +425,6 @@ static int lp855x_parse_dt(struct lp855x *lp)
> 
>  	return 0;
>  }
> -#else
> -static int lp855x_parse_dt(struct lp855x *lp)
> -{
> -	return -EINVAL;
> -}
> -#endif
> 
>  static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
>  {
> @@ -532,7 +525,7 @@ MODULE_DEVICE_TABLE(i2c, lp855x_ids);
>  static struct i2c_driver lp855x_driver = {
>  	.driver = {
>  		   .name = "lp855x",
> -		   .of_match_table = of_match_ptr(lp855x_dt_ids),
> +		   .of_match_table = lp855x_dt_ids,
>  		   },
>  	.probe = lp855x_probe,
>  	.remove = lp855x_remove,
> --
> 2.1.1


^ permalink raw reply

* Re: [PATCH v2 2/2] backlight/lp855x: Add supply regulator to lp855x
From: Kim, Milo @ 2014-12-09  3:03 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Sean Paul, cooloney-Re5JQEeQqe8AvxtiuMwx3w,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, jg1.han-Sze3O3UU22JBDgjK7y7TUQ,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A, Stéphane Marchesin,
	Aaron Durbin
In-Reply-To: <5204192.bzM5jPIgHv@wuerfel>

Hi Arnd,

Good to talk to you again. It's been a long time since I met you in 
Linaro Connect HK few years ago :)

On 12/3/2014 11:11 PM, Arnd Bergmann wrote:
> While your two patches are both correct (and applied already), I took
> a look at the platform_data header and noticed that all users of this
> file have been converted to DT a while ago, so it would be nice to clean
> it up by integrating the linux/platform_data/lp855x.h header file into the
> drivers/video/backlight/lp855x_bl.c itself.

I like this cleanup but need to consider few things.

- Platform which does not support the DT
In this case, there is no way to configure options like backlight device 
control mode. Without the DT, only default options are set.
To change the settings, the driver modification is required.
I don't want to write project/platform dependency code inside the driver.

- Backward compatibility
Some customers have been using this platform data in their board-*.c 
(but this *.c has not been mainlined yet).
My concern is the backward compatibility issue may happen in their projects.

Best regards,
Milo

^ permalink raw reply

* Re: [PATCH v2 2/2] backlight/lp855x: Add supply regulator to lp855x
From: Jingoo Han @ 2014-12-09  3:46 UTC (permalink / raw)
  To: 'Kim, Milo', 'Arnd Bergmann'
  Cc: 'Sean Paul', cooloney-Re5JQEeQqe8AvxtiuMwx3w,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	'Stéphane Marchesin', 'Aaron Durbin',
	'Jingoo Han'
In-Reply-To: <54866695.3050307-l0cyMroinI0@public.gmane.org>

On Tuesday, December 09, 2014 12:04 PM, Kim, Milo wrote:
> 
> Hi Arnd,
> 
> Good to talk to you again. It's been a long time since I met you in
> Linaro Connect HK few years ago :)
> 
> On 12/3/2014 11:11 PM, Arnd Bergmann wrote:
> > While your two patches are both correct (and applied already), I took
> > a look at the platform_data header and noticed that all users of this
> > file have been converted to DT a while ago, so it would be nice to clean
> > it up by integrating the linux/platform_data/lp855x.h header file into the
> > drivers/video/backlight/lp855x_bl.c itself.
> 
> I like this cleanup but need to consider few things.
> 
> - Platform which does not support the DT
> In this case, there is no way to configure options like backlight device
> control mode. Without the DT, only default options are set.
> To change the settings, the driver modification is required.
> I don't want to write project/platform dependency code inside the driver.
> 
> - Backward compatibility
> Some customers have been using this platform data in their board-*.c
> (but this *.c has not been mainlined yet).
> My concern is the backward compatibility issue may happen in their projects.

I understand this situation. In the case of Exynos DisplayPort driver,
a similar issue happened. At that time, the conclusion was that
out-of-tree code (not mainlined yet) cannot be considered.

In this context, it is valuable to upstream the code to mainline kernel.
I think that one of the followings can be considered.

  1. Ask customers to upstream their machine code
  2. Ask customers to use DT

If they don't want to these things, someone (you or customers)
will need to make the patch for non-DT support, whenever new
kernel is released and is used by projects not supporting DT.

Best regards,
Jingoo Han

> 
> Best regards,
> Milo


^ permalink raw reply

* Re: [PATCH v2 2/2] backlight/lp855x: Add supply regulator to lp855x
From: Arnd Bergmann @ 2014-12-09  8:13 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Kim, Milo', 'Sean Paul',
	cooloney-Re5JQEeQqe8AvxtiuMwx3w,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	'Stéphane Marchesin', 'Aaron Durbin'
In-Reply-To: <000a01d01362$aee684d0$0cb38e70$%han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On Tuesday 09 December 2014 12:46:15 Jingoo Han wrote:
> On Tuesday, December 09, 2014 12:04 PM, Kim, Milo wrote:
> > 
> > Hi Arnd,
> > 
> > Good to talk to you again. It's been a long time since I met you in
> > Linaro Connect HK few years ago 
> > 
> > On 12/3/2014 11:11 PM, Arnd Bergmann wrote:
> > > While your two patches are both correct (and applied already), I took
> > > a look at the platform_data header and noticed that all users of this
> > > file have been converted to DT a while ago, so it would be nice to clean
> > > it up by integrating the linux/platform_data/lp855x.h header file into the
> > > drivers/video/backlight/lp855x_bl.c itself.
> > 
> > I like this cleanup but need to consider few things.
> > 
> > - Platform which does not support the DT
> > In this case, there is no way to configure options like backlight device
> > control mode. Without the DT, only default options are set.
> > To change the settings, the driver modification is required.
> > I don't want to write project/platform dependency code inside the driver.
> > 
> > - Backward compatibility
> > Some customers have been using this platform data in their board-*.c
> > (but this *.c has not been mainlined yet).
> > My concern is the backward compatibility issue may happen in their projects.
> 
> I understand this situation. In the case of Exynos DisplayPort driver,
> a similar issue happened. At that time, the conclusion was that
> out-of-tree code (not mainlined yet) cannot be considered.

Right. Basically when the only reason to have code in the kernel is
to support someone with an out-of-tree patch set, they can be expected
to add the extra bit to their patch set, it doesn't add any extra burden
to them, and it makes our lives easier.

	Arnd

^ permalink raw reply

* Re: [PATCH v2 1/4] backlight/lp855x: Remove stale code from lp855x.h
From: Arnd Bergmann @ 2014-12-09  8:39 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1418077278-14144-1-git-send-email-seanpaul@chromium.org>

On Monday 08 December 2014 17:21:15 Sean Paul wrote:
> These aren't used by anyone, remove them.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Very nicely done series!

All four patches

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* using fb1
From: Sudip Mukherjee @ 2014-12-09 14:39 UTC (permalink / raw)
  To: linux-fbdev

hi,
I am starting to work with SM712 driver which was in drivers/staging, but then removed. I have a demoboard of SM712, and testing the existing code on it.
After the module is inserted the board is initialized, and /dev/fb1 is successfully created. But after that how do i test? how do i confugure ubuntu to use that /dev/fb1 ?
I know it is not the proper place to post this, and I should have posted in some ubuntu specific forum or list, but i thought this will be the best place to get the best answer.
Looking forward for some pointer or some idea / hints in this regard.

thanks
sudip

^ permalink raw reply

* Re: FOSDEM15: Graphics DevRoom: call for speakers.
From: Luc Verhaegen @ 2014-12-09 14:39 UTC (permalink / raw)
  To: mesa-dev, xorg-devel, wayland-devel, mir-devel, directfb-dev,
	linux-fbdev, linux-media
  Cc: dri-devel, xorg, xorg-announce
In-Reply-To: <20141002174457.GA11725@skynet.be>

On Thu, Oct 02, 2014 at 07:44:57PM +0200, Luc Verhaegen wrote:
> Hi,
> 
> At FOSDEM on the 31st of january and the 1st of February 2015, there 
> will be another graphics DevRoom. URL: https://fosdem.org/2015/

> Slots will be handed out on a first come, first serve basis. The best 
> slots will go to those who apply the earliest. The amount of slots is 
> currently not known yet, but i expect there to be around 16 available (8 
> on each day), so act quickly.

> As for deadlines, i hope to have a pretty much complete schedule between 
> christmas and the new year. The rockhard printed schedule deadline is 
> probably January 9th, after that you will not be featured in the booklet 
> and you will have a lot less visitors. I will hopefully be able to lock 
> down entries and descriptions after that date.

It's been more than 2 months since the original email, it's less than 
two months away from the event, and one month away from what usually is 
the deadline for the booklet. File your talk now, while there are still 
some useful slots available.

Also, for those who have filed already but who have left their abstracts 
open, please get those filed in ASAP. Your talk will be only be ordered 
in when at least the basics are provided.

Thanks,

Luc Verhaegen.

^ permalink raw reply

* Compiled tridentfb hangs but distribution kernel works fine
From: Ondrej Zary @ 2014-12-09 21:33 UTC (permalink / raw)
  To: linux-fbdev; +Cc: Kernel development list

Hello,
I'm doing some work on tridentfb driver but encountered a weird problem.

When I compile custom kernel with tridentfb, it hangs with Blade3D card (ID 
0x9880) in blade_image_blit(). The screen is blank with some artifacts and 
machine does not respond to ping or keyboard. However, it can be rebooted by 
Alt+SysRq+B. It works fine with other cards (3DImage 9750 and CyberBlade XP) 
with no blit implementation. Commenting out contents of blade_image_blit() 
function makes the hang go away (nothing useful on the screen, of course).

The weird thing is that the Blade3D card works fine when I load Debian 
kernels! Debian 3.16-0.bpo.3-686-pae works fine but when I compile 3.16 
kernel, it hangs.

Any ideas?


-- 
Ondrej Zary

^ permalink raw reply

* Re: Compiled tridentfb hangs but distribution kernel works fine
From: Mike Galbraith @ 2014-12-10  7:15 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: linux-fbdev, Kernel development list
In-Reply-To: <201412092233.17030.linux@rainbow-software.org>

On Tue, 2014-12-09 at 22:33 +0100, Ondrej Zary wrote: 
> Hello,
> I'm doing some work on tridentfb driver but encountered a weird problem.
> 
> When I compile custom kernel with tridentfb, it hangs with Blade3D card (ID 
> 0x9880) in blade_image_blit(). The screen is blank with some artifacts and 
> machine does not respond to ping or keyboard. However, it can be rebooted by 
> Alt+SysRq+B. It works fine with other cards (3DImage 9750 and CyberBlade XP) 
> with no blit implementation. Commenting out contents of blade_image_blit() 
> function makes the hang go away (nothing useful on the screen, of course).
> 
> The weird thing is that the Blade3D card works fine when I load Debian 
> kernels! Debian 3.16-0.bpo.3-686-pae works fine but when I compile 3.16 
> kernel, it hangs.
> 
> Any ideas?

I'd start by looking for what Debian changed in source and/or config.

-Mike


^ permalink raw reply


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