Re: [PATCH 05/10] ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
On 09/23/2013 03:41 PM, Thierry Reding wrote:
The GPIO API defines 0 as being a valid GPIO number, so this field needs
to be initialized explicitly.
static void __init smdkv210_map_io(void)
@@ -70,6 +70,7 @@ static struct samsung_bl_drvdata samsung_dfl_bl_data
__initdata = {
.max_brightness = 255,
.dft_brightness = 255,
.pwm_period_ns = 78770,
+ .enable_gpio= -1,
.init = samsung_bl_init,
.exit = samsung_bl_exit,
},
@@ -121,6 +122,10 @@ void __init samsung_bl_set(struct samsung_bl_gpio_info
*gpio_info,
samsung_bl_data-lth_brightness = bl_data-lth_brightness;
if (bl_data-pwm_period_ns)
samsung_bl_data-pwm_period_ns = bl_data-pwm_period_ns;
+ if (bl_data-enable_gpio)
+ samsung_bl_data-enable_gpio = bl_data-enable_gpio;
+ if (bl_data-enable_gpio_flags)
+ samsung_bl_data-enable_gpio_flags = bl_data-enable_gpio_flags;
Won't this cause the core pwm_bl driver to request/manipulate the GPIO,
whereas this driver already does that inside the samsung_bl_init/exit
callbacks? I think you either need to adjust those callbacks, or not set
the new standard GPIO property in samsung_bl_data.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/10] ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
On Tue, Oct 01, 2013 at 12:31:04PM -0600, Stephen Warren wrote:
On 09/23/2013 03:41 PM, Thierry Reding wrote:
The GPIO API defines 0 as being a valid GPIO number, so this field needs
to be initialized explicitly.
static void __init smdkv210_map_io(void)
@@ -70,6 +70,7 @@ static struct samsung_bl_drvdata samsung_dfl_bl_data
__initdata = {
.max_brightness = 255,
.dft_brightness = 255,
.pwm_period_ns = 78770,
+ .enable_gpio= -1,
.init = samsung_bl_init,
.exit = samsung_bl_exit,
},
@@ -121,6 +122,10 @@ void __init samsung_bl_set(struct samsung_bl_gpio_info
*gpio_info,
samsung_bl_data-lth_brightness = bl_data-lth_brightness;
if (bl_data-pwm_period_ns)
samsung_bl_data-pwm_period_ns = bl_data-pwm_period_ns;
+ if (bl_data-enable_gpio)
+ samsung_bl_data-enable_gpio = bl_data-enable_gpio;
+ if (bl_data-enable_gpio_flags)
+ samsung_bl_data-enable_gpio_flags = bl_data-enable_gpio_flags;
Won't this cause the core pwm_bl driver to request/manipulate the GPIO,
whereas this driver already does that inside the samsung_bl_init/exit
callbacks? I think you either need to adjust those callbacks, or not set
the new standard GPIO property in samsung_bl_data.
I don't think so. The samsung_bl_data is a copy of samsung_dfl_bl_data
augmented by board-specific settings. So in fact copying these values
here is essential to allow boards to override the enable_gpio and flags
fields. Currently no board sets the enable_gpio to a valid GPIO so it's
all still handled by the callbacks only.
Thierry
pgp466urQi8Ar.pgp
Description: PGP signature
Re: [PATCH 05/10] ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
On 10/01/2013 02:43 PM, Thierry Reding wrote:
On Tue, Oct 01, 2013 at 12:31:04PM -0600, Stephen Warren wrote:
On 09/23/2013 03:41 PM, Thierry Reding wrote:
The GPIO API defines 0 as being a valid GPIO number, so this
field needs to be initialized explicitly.
static void __init smdkv210_map_io(void)
@@ -70,6 +70,7 @@ static struct samsung_bl_drvdata
samsung_dfl_bl_data __initdata = { .max_brightness = 255,
.dft_brightness = 255, .pwm_period_ns = 78770, + .enable_gpio
= -1, .init = samsung_bl_init, .exit =
samsung_bl_exit, }, @@ -121,6 +122,10 @@ void __init
samsung_bl_set(struct samsung_bl_gpio_info *gpio_info,
samsung_bl_data-lth_brightness = bl_data-lth_brightness; if
(bl_data-pwm_period_ns) samsung_bl_data-pwm_period_ns =
bl_data-pwm_period_ns; + if (bl_data-enable_gpio) +
samsung_bl_data-enable_gpio = bl_data-enable_gpio; + if
(bl_data-enable_gpio_flags) +
samsung_bl_data-enable_gpio_flags =
bl_data-enable_gpio_flags;
Won't this cause the core pwm_bl driver to request/manipulate the
GPIO, whereas this driver already does that inside the
samsung_bl_init/exit callbacks? I think you either need to adjust
those callbacks, or not set the new standard GPIO property in
samsung_bl_data.
I don't think so. The samsung_bl_data is a copy of
samsung_dfl_bl_data augmented by board-specific settings. So in
fact copying these values here is essential to allow boards to
override the enable_gpio and flags fields. Currently no board sets
the enable_gpio to a valid GPIO so it's all still handled by the
callbacks only.
Oh yes, you're right. I was confusing the new enable_gpio field in
pwm_bl's platform data with some other field in a custom data structure.
One minor point though:
+ if (bl_data-enable_gpio) + samsung_bl_data-enable_gpio =
bl_data-enable_gpio;
That assumes that enable_gpio==0 means none, whereas you've gone to
great pains in the rest of the series to allow 0 to be a valid GPIO
ID. right now, the default value of samsung_bl_data-enable_gpio is
-1, and if !bl_data-enable_gpio, that value won't be propagated across.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/10] ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
On Tue, Oct 01, 2013 at 02:58:22PM -0600, Stephen Warren wrote:
On 10/01/2013 02:43 PM, Thierry Reding wrote:
On Tue, Oct 01, 2013 at 12:31:04PM -0600, Stephen Warren wrote:
On 09/23/2013 03:41 PM, Thierry Reding wrote:
The GPIO API defines 0 as being a valid GPIO number, so this
field needs to be initialized explicitly.
static void __init smdkv210_map_io(void)
@@ -70,6 +70,7 @@ static struct samsung_bl_drvdata
samsung_dfl_bl_data __initdata = { .max_brightness = 255,
.dft_brightness = 255, .pwm_period_ns = 78770, + .enable_gpio
= -1, .init = samsung_bl_init, .exit =
samsung_bl_exit, }, @@ -121,6 +122,10 @@ void __init
samsung_bl_set(struct samsung_bl_gpio_info *gpio_info,
samsung_bl_data-lth_brightness = bl_data-lth_brightness; if
(bl_data-pwm_period_ns) samsung_bl_data-pwm_period_ns =
bl_data-pwm_period_ns; + if (bl_data-enable_gpio) +
samsung_bl_data-enable_gpio = bl_data-enable_gpio; +if
(bl_data-enable_gpio_flags) +
samsung_bl_data-enable_gpio_flags =
bl_data-enable_gpio_flags;
Won't this cause the core pwm_bl driver to request/manipulate the
GPIO, whereas this driver already does that inside the
samsung_bl_init/exit callbacks? I think you either need to adjust
those callbacks, or not set the new standard GPIO property in
samsung_bl_data.
I don't think so. The samsung_bl_data is a copy of
samsung_dfl_bl_data augmented by board-specific settings. So in
fact copying these values here is essential to allow boards to
override the enable_gpio and flags fields. Currently no board sets
the enable_gpio to a valid GPIO so it's all still handled by the
callbacks only.
Oh yes, you're right. I was confusing the new enable_gpio field in
pwm_bl's platform data with some other field in a custom data structure.
One minor point though:
+ if (bl_data-enable_gpio) + samsung_bl_data-enable_gpio =
bl_data-enable_gpio;
That assumes that enable_gpio==0 means none, whereas you've gone to
great pains in the rest of the series to allow 0 to be a valid GPIO
ID. right now, the default value of samsung_bl_data-enable_gpio is
-1, and if !bl_data-enable_gpio, that value won't be propagated across.
Right, that check should now be:
if (bl_data-enable_gpio = 0)
Well, it depends. It would be possible for the default to specify a
valid GPIO and for a board to override it with -1 (and provide a set of
corresponding callbacks). In that case the right thing to do here would
be not to check at all.
Then again, I don't think that will ever happen, because no fixed GPIO
will ever be a good default. So changing to = 0 instead of != 0 should
work fine.
Again, starting with 3.13 this should become a lot easier to handle
since the GPIO subsystem will gain functionality to use a per-board
lookup table, similarly to what the regulator and PWM subsystems do.
Once that's in place I plan to make another pass over all users of the
pwm-backlight driver and replace the enable_gpio field with a GPIO
lookup table, so that the driver can uniformly request them using a
simple gpiod_get().
Thierry
pgpT0EQDSLmhg.pgp
Description: PGP signature
[PATCH 05/10] ARM: SAMSUNG: Initialize PWM backlight enable_gpio field
The GPIO API defines 0 as being a valid GPIO number, so this field needs
to be initialized explicitly.
Signed-off-by: Thierry Reding tred...@nvidia.com
---
arch/arm/mach-s3c24xx/mach-h1940.c| 1 +
arch/arm/mach-s3c24xx/mach-rx1950.c | 1 +
arch/arm/mach-s3c64xx/mach-crag6410.c | 1 +
arch/arm/mach-s3c64xx/mach-hmt.c | 1 +
arch/arm/mach-s3c64xx/mach-smartq.c | 1 +
arch/arm/mach-s3c64xx/mach-smdk6410.c | 1 +
arch/arm/mach-s5p64x0/mach-smdk6440.c | 1 +
arch/arm/mach-s5p64x0/mach-smdk6450.c | 1 +
arch/arm/mach-s5pc100/mach-smdkc100.c | 1 +
arch/arm/mach-s5pv210/mach-smdkv210.c | 1 +
arch/arm/plat-samsung/dev-backlight.c | 5 +
11 files changed, 15 insertions(+)
diff --git a/arch/arm/mach-s3c24xx/mach-h1940.c
b/arch/arm/mach-s3c24xx/mach-h1940.c
index 74dd479..952b6a0 100644
--- a/arch/arm/mach-s3c24xx/mach-h1940.c
+++ b/arch/arm/mach-s3c24xx/mach-h1940.c
@@ -504,6 +504,7 @@ static struct platform_pwm_backlight_data backlight_data = {
.dft_brightness = 50,
/* tcnt = 0x31 */
.pwm_period_ns = 36296,
+ .enable_gpio= -1,
.init = h1940_backlight_init,
.notify = h1940_backlight_notify,
.exit = h1940_backlight_exit,
diff --git a/arch/arm/mach-s3c24xx/mach-rx1950.c
b/arch/arm/mach-s3c24xx/mach-rx1950.c
index 206b1f7..034b7fe 100644
--- a/arch/arm/mach-s3c24xx/mach-rx1950.c
+++ b/arch/arm/mach-s3c24xx/mach-rx1950.c
@@ -522,6 +522,7 @@ static struct platform_pwm_backlight_data
rx1950_backlight_data = {
.max_brightness = 24,
.dft_brightness = 4,
.pwm_period_ns = 48000,
+ .enable_gpio = -1,
.init = rx1950_backlight_init,
.notify = rx1950_backlight_notify,
.exit = rx1950_backlight_exit,
diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c
b/arch/arm/mach-s3c64xx/mach-crag6410.c
index 1a911df..b319bf9 100644
--- a/arch/arm/mach-s3c64xx/mach-crag6410.c
+++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
@@ -114,6 +114,7 @@ static struct platform_pwm_backlight_data
crag6410_backlight_data = {
.max_brightness = 1000,
.dft_brightness = 600,
.pwm_period_ns = 10, /* about 1kHz */
+ .enable_gpio= -1,
};
static struct platform_device crag6410_backlight_device = {
diff --git a/arch/arm/mach-s3c64xx/mach-hmt.c b/arch/arm/mach-s3c64xx/mach-hmt.c
index e806404..614a03a 100644
--- a/arch/arm/mach-s3c64xx/mach-hmt.c
+++ b/arch/arm/mach-s3c64xx/mach-hmt.c
@@ -114,6 +114,7 @@ static struct platform_pwm_backlight_data
hmt_backlight_data = {
.max_brightness = 100 * 256,
.dft_brightness = 40 * 256,
.pwm_period_ns = 10 / (100 * 256 * 20),
+ .enable_gpio= -1,
.init = hmt_bl_init,
.notify = hmt_bl_notify,
.exit = hmt_bl_exit,
diff --git a/arch/arm/mach-s3c64xx/mach-smartq.c
b/arch/arm/mach-s3c64xx/mach-smartq.c
index 0f47237..a6b338f 100644
--- a/arch/arm/mach-s3c64xx/mach-smartq.c
+++ b/arch/arm/mach-s3c64xx/mach-smartq.c
@@ -151,6 +151,7 @@ static struct platform_pwm_backlight_data
smartq_backlight_data = {
.max_brightness = 1000,
.dft_brightness = 600,
.pwm_period_ns = 10 / (1000 * 20),
+ .enable_gpio= -1,
.init = smartq_bl_init,
};
diff --git a/arch/arm/mach-s3c64xx/mach-smdk6410.c
b/arch/arm/mach-s3c64xx/mach-smdk6410.c
index 2a7b32c..d5ea938 100644
--- a/arch/arm/mach-s3c64xx/mach-smdk6410.c
+++ b/arch/arm/mach-s3c64xx/mach-smdk6410.c
@@ -625,6 +625,7 @@ static struct samsung_bl_gpio_info smdk6410_bl_gpio_info = {
static struct platform_pwm_backlight_data smdk6410_bl_data = {
.pwm_id = 1,
+ .enable_gpio = -1,
};
static struct s3c_hsotg_plat smdk6410_hsotg_pdata;
diff --git a/arch/arm/mach-s5p64x0/mach-smdk6440.c
b/arch/arm/mach-s5p64x0/mach-smdk6440.c
index 0b00304..9efdcc0 100644
--- a/arch/arm/mach-s5p64x0/mach-smdk6440.c
+++ b/arch/arm/mach-s5p64x0/mach-smdk6440.c
@@ -223,6 +223,7 @@ static struct samsung_bl_gpio_info smdk6440_bl_gpio_info = {
static struct platform_pwm_backlight_data smdk6440_bl_data = {
.pwm_id = 1,
+ .enable_gpio = -1,
};
static void __init smdk6440_map_io(void)
diff --git a/arch/arm/mach-s5p64x0/mach-smdk6450.c
b/arch/arm/mach-s5p64x0/mach-smdk6450.c
index 5949296..c3cacc0 100644
--- a/arch/arm/mach-s5p64x0/mach-smdk6450.c
+++ b/arch/arm/mach-s5p64x0/mach-smdk6450.c
@@ -242,6 +242,7 @@ static struct samsung_bl_gpio_info smdk6450_bl_gpio_info = {
static struct platform_pwm_backlight_data smdk6450_bl_data = {
.pwm_id = 1,
+ .enable_gpio = -1,
};
static void __init smdk6450_map_io(void)
diff --git a/arch/arm/mach-s5pc100/mach-smdkc100.c
b/arch/arm/mach-s5pc100/mach-smdkc100.c
index 7c57a22..9e256b9 100644
--- a/arch/arm/mach-s5pc100/mach-smdkc100.c
+++ b/arch/arm/mach-s5pc100/mach-smdkc100.c
@@ -216,6 +216,7 @@ static struct samsung_bl_gpio_info smdkc100_bl_gpio_info = {
static
