Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH v2] efifb: prevent null-deref when iterating dmi_list
From: David Herrmann @ 2013-10-31 16:17 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: linux-fbdev@vger.kernel.org, James Bates, linux-kernel,
	Tomi Valkeinen, James Bates
In-Reply-To: <20131031104549.GZ18477@ns203013.ovh.net>

Hi

On Thu, Oct 31, 2013 at 11:45 AM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 18:40 Wed 02 Oct     , David Herrmann wrote:
>> The dmi_list array is initialized using gnu designated initializers, and
>> therefore may contain fewer explicitly defined entries as there are
>> elements in it. This is because the enum above with M_xyz constants
>> contains more items than the designated initializer. Those elements not
>> explicitly initialized are implicitly set to 0.
>>
>> Now efifb_setup() loops through all these array elements, and performs
>> a strcmp on each item. For non explicitly initialized elements this will
>> be a null pointer:
>>
>> This patch swaps the check order in the if statement, thus checks first
>> whether dmi_list[i].base is null.
>>
>> Signed-off-by: James Bates <james.h.bates@yahoo.com>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>
> with the simpleDRM arriving next merge I'm wondering if we need to keep it?

SimpleDRM is not coming next merge-window. It's basically finished,
but I'm still working on the user-space side as its KMS api is highly
reduced compared to fully-featured DRM/KMS drivers. Maybe 3.13 will
work out.

Anyhow, this patch is still needed as it fixes a serious bug for simplefb.

Thanks
David

^ permalink raw reply

* [PATCH v3 0/9] backlight: atmel-pwm-bl: fixes and clean ups
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1383064064-4983-1-git-send-email-jhovold@gmail.com>

These patches fix a few issues and clean up the atmel-pwm-bl driver
somewhat.

Acks from Jingoo Han retained on all patches but the slightly modified
first and final patch.

Johan

v2:
 - mask returned brightness rather than cast it to u16 (patch 1/9)
v3
 - use unsigned long for gpio flags (patch 9/9)

Johan Hovold (9):
  backlight: atmel-pwm-bl: fix reported brightness
  backlight: atmel-pwm-bl: fix gpio polarity in remove
  backlight: atmel-pwm-bl: fix module autoload
  backlight: atmel-pwm-bl: clean up probe error handling
  backlight: atmel-pwm-bl: clean up get_intensity
  backlight: atmel-pwm-bl: remove unused include
  backlight: atmel-pwm-bl: use gpio_is_valid
  backlight: atmel-pwm-bl: refactor gpio_on handling
  backlight: atmel-pwm-bl: use gpio_request_one

 drivers/video/backlight/atmel-pwm-bl.c | 86 ++++++++++++++++------------------
 1 file changed, 40 insertions(+), 46 deletions(-)

-- 
1.8.4.2


^ permalink raw reply

* [PATCH v3 1/9] backlight: atmel-pwm-bl: fix reported brightness
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold, stable
In-Reply-To: <1383242264-7652-1-git-send-email-jhovold@gmail.com>

The driver supports 16-bit brightness values, but the value returned
from get_brightness was truncated to eight bits.

Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 66885fb..0971a8e 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -70,7 +70,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
 {
 	struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
-	u8 intensity;
+	u32 intensity;
 
 	if (pwmbl->pdata->pwm_active_low) {
 		intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
@@ -80,7 +80,7 @@ static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
 			pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
 	}
 
-	return intensity;
+	return intensity & 0xffff;
 }
 
 static int atmel_pwm_bl_init_pwm(struct atmel_pwm_bl *pwmbl)
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH v3 2/9] backlight: atmel-pwm-bl: fix gpio polarity in remove
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold, stable
In-Reply-To: <1383242264-7652-1-git-send-email-jhovold@gmail.com>

Make sure to honour gpio polarity also at remove so that the backlight
is actually disabled on boards with active-low enable pin.

Cc: stable@vger.kernel.org
Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 0971a8e..e21beb6 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -205,8 +205,10 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
 {
 	struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
 
-	if (pwmbl->gpio_on != -1)
-		gpio_set_value(pwmbl->gpio_on, 0);
+	if (pwmbl->gpio_on != -1) {
+		gpio_set_value(pwmbl->gpio_on,
+					0 ^ pwmbl->pdata->on_active_low);
+	}
 	pwm_channel_disable(&pwmbl->pwmc);
 	pwm_channel_free(&pwmbl->pwmc);
 
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH v3 3/9] backlight: atmel-pwm-bl: fix module autoload
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1383242264-7652-1-git-send-email-jhovold@gmail.com>

Add missing module alias which is needed for module autoloading.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index e21beb6..4886028 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -229,3 +229,4 @@ module_platform_driver(atmel_pwm_bl_driver);
 MODULE_AUTHOR("Hans-Christian egtvedt <hans-christian.egtvedt@atmel.com>");
 MODULE_DESCRIPTION("Atmel PWM backlight driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:atmel-pwm-bl");
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH v3 4/9] backlight: atmel-pwm-bl: clean up probe error handling
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1383242264-7652-1-git-send-email-jhovold@gmail.com>

Clean up probe error handling by checking parameters before any
allocations and removing an obsolete error label. Also remove
unnecessary reset of private gpio number.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 4886028..01af5c2 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -126,40 +126,33 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	struct atmel_pwm_bl *pwmbl;
 	int retval;
 
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata)
+		return -ENODEV;
+
+	if (pdata->pwm_compare_max < pdata->pwm_duty_max ||
+			pdata->pwm_duty_min > pdata->pwm_duty_max ||
+			pdata->pwm_frequency = 0)
+		return -EINVAL;
+
 	pwmbl = devm_kzalloc(&pdev->dev, sizeof(struct atmel_pwm_bl),
 				GFP_KERNEL);
 	if (!pwmbl)
 		return -ENOMEM;
 
 	pwmbl->pdev = pdev;
-
-	pdata = dev_get_platdata(&pdev->dev);
-	if (!pdata) {
-		retval = -ENODEV;
-		goto err_free_mem;
-	}
-
-	if (pdata->pwm_compare_max < pdata->pwm_duty_max ||
-			pdata->pwm_duty_min > pdata->pwm_duty_max ||
-			pdata->pwm_frequency = 0) {
-		retval = -EINVAL;
-		goto err_free_mem;
-	}
-
 	pwmbl->pdata = pdata;
 	pwmbl->gpio_on = pdata->gpio_on;
 
 	retval = pwm_channel_alloc(pdata->pwm_channel, &pwmbl->pwmc);
 	if (retval)
-		goto err_free_mem;
+		return retval;
 
 	if (pwmbl->gpio_on != -1) {
 		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
 					"gpio_atmel_pwm_bl");
-		if (retval) {
-			pwmbl->gpio_on = -1;
+		if (retval)
 			goto err_free_pwm;
-		}
 
 		/* Turn display off by default. */
 		retval = gpio_direction_output(pwmbl->gpio_on,
@@ -197,7 +190,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 
 err_free_pwm:
 	pwm_channel_free(&pwmbl->pwmc);
-err_free_mem:
+
 	return retval;
 }
 
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH v3 5/9] backlight: atmel-pwm-bl: clean up get_intensity
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1383242264-7652-1-git-send-email-jhovold@gmail.com>

Clean up get_intensity to increase readability.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 01af5c2..abfaada 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -70,15 +70,14 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 static int atmel_pwm_bl_get_intensity(struct backlight_device *bd)
 {
 	struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
+	u32 cdty;
 	u32 intensity;
 
-	if (pwmbl->pdata->pwm_active_low) {
-		intensity = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY) -
-			pwmbl->pdata->pwm_duty_min;
-	} else {
-		intensity = pwmbl->pdata->pwm_duty_max -
-			pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
-	}
+	cdty = pwm_channel_readl(&pwmbl->pwmc, PWM_CDTY);
+	if (pwmbl->pdata->pwm_active_low)
+		intensity = cdty - pwmbl->pdata->pwm_duty_min;
+	else
+		intensity = pwmbl->pdata->pwm_duty_max - cdty;
 
 	return intensity & 0xffff;
 }
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH v3 6/9] backlight: atmel-pwm-bl: remove unused include
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1383242264-7652-1-git-send-email-jhovold@gmail.com>

Remove unused include of clk.h.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index abfaada..bfd6a96 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -12,7 +12,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/fb.h>
-#include <linux/clk.h>
 #include <linux/gpio.h>
 #include <linux/backlight.h>
 #include <linux/atmel_pwm.h>
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH v3 7/9] backlight: atmel-pwm-bl: use gpio_is_valid
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1383242264-7652-1-git-send-email-jhovold@gmail.com>

Use gpio_is_valid rather than open coding the more restrictive != -1
test.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index bfd6a96..c254209 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -48,7 +48,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 		pwm_duty = pwmbl->pdata->pwm_duty_min;
 
 	if (!intensity) {
-		if (pwmbl->gpio_on != -1) {
+		if (gpio_is_valid(pwmbl->gpio_on)) {
 			gpio_set_value(pwmbl->gpio_on,
 					0 ^ pwmbl->pdata->on_active_low);
 		}
@@ -57,7 +57,7 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 	} else {
 		pwm_channel_enable(&pwmbl->pwmc);
 		pwm_channel_writel(&pwmbl->pwmc, PWM_CUPD, pwm_duty);
-		if (pwmbl->gpio_on != -1) {
+		if (gpio_is_valid(pwmbl->gpio_on)) {
 			gpio_set_value(pwmbl->gpio_on,
 					1 ^ pwmbl->pdata->on_active_low);
 		}
@@ -146,7 +146,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	if (retval)
 		return retval;
 
-	if (pwmbl->gpio_on != -1) {
+	if (gpio_is_valid(pwmbl->gpio_on)) {
 		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
 					"gpio_atmel_pwm_bl");
 		if (retval)
@@ -196,7 +196,7 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
 {
 	struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
 
-	if (pwmbl->gpio_on != -1) {
+	if (gpio_is_valid(pwmbl->gpio_on)) {
 		gpio_set_value(pwmbl->gpio_on,
 					0 ^ pwmbl->pdata->on_active_low);
 	}
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH v3 8/9] backlight: atmel-pwm-bl: refactor gpio_on handling
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1383242264-7652-1-git-send-email-jhovold@gmail.com>

Add helper function to control the gpio_on signal.

Acked-by: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index c254209..bd1ed34 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -26,6 +26,14 @@ struct atmel_pwm_bl {
 	int					gpio_on;
 };
 
+static void atmel_pwm_bl_set_gpio_on(struct atmel_pwm_bl *pwmbl, int on)
+{
+	if (!gpio_is_valid(pwmbl->gpio_on))
+		return;
+
+	gpio_set_value(pwmbl->gpio_on, on ^ pwmbl->pdata->on_active_low);
+}
+
 static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 {
 	struct atmel_pwm_bl *pwmbl = bl_get_data(bd);
@@ -48,19 +56,13 @@ static int atmel_pwm_bl_set_intensity(struct backlight_device *bd)
 		pwm_duty = pwmbl->pdata->pwm_duty_min;
 
 	if (!intensity) {
-		if (gpio_is_valid(pwmbl->gpio_on)) {
-			gpio_set_value(pwmbl->gpio_on,
-					0 ^ pwmbl->pdata->on_active_low);
-		}
+		atmel_pwm_bl_set_gpio_on(pwmbl, 0);
 		pwm_channel_writel(&pwmbl->pwmc, PWM_CUPD, pwm_duty);
 		pwm_channel_disable(&pwmbl->pwmc);
 	} else {
 		pwm_channel_enable(&pwmbl->pwmc);
 		pwm_channel_writel(&pwmbl->pwmc, PWM_CUPD, pwm_duty);
-		if (gpio_is_valid(pwmbl->gpio_on)) {
-			gpio_set_value(pwmbl->gpio_on,
-					1 ^ pwmbl->pdata->on_active_low);
-		}
+		atmel_pwm_bl_set_gpio_on(pwmbl, 1);
 	}
 
 	return 0;
@@ -196,10 +198,7 @@ static int atmel_pwm_bl_remove(struct platform_device *pdev)
 {
 	struct atmel_pwm_bl *pwmbl = platform_get_drvdata(pdev);
 
-	if (gpio_is_valid(pwmbl->gpio_on)) {
-		gpio_set_value(pwmbl->gpio_on,
-					0 ^ pwmbl->pdata->on_active_low);
-	}
+	atmel_pwm_bl_set_gpio_on(pwmbl, 0);
 	pwm_channel_disable(&pwmbl->pwmc);
 	pwm_channel_free(&pwmbl->pwmc);
 
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH v3 9/9] backlight: atmel-pwm-bl: use gpio_request_one
From: Johan Hovold @ 2013-10-31 17:57 UTC (permalink / raw)
  To: Richard Purdie, Jingoo Han
  Cc: Nicolas Ferre, Tomi Valkeinen, Jean-Christophe Plagniol-Villard,
	Andrew Morton, linux-fbdev, linux-kernel, Johan Hovold
In-Reply-To: <1383242264-7652-1-git-send-email-jhovold@gmail.com>

Use devm_gpio_request_one rather than requesting and setting direction
in two calls.

Cc: Jingoo Han <jg1.han@samsung.com>
Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/video/backlight/atmel-pwm-bl.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index bd1ed34..261b1a4 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c
@@ -124,6 +124,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	const struct atmel_pwm_bl_platform_data *pdata;
 	struct backlight_device *bldev;
 	struct atmel_pwm_bl *pwmbl;
+	unsigned long flags;
 	int retval;
 
 	pdata = dev_get_platdata(&pdev->dev);
@@ -149,14 +150,14 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 		return retval;
 
 	if (gpio_is_valid(pwmbl->gpio_on)) {
-		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
-					"gpio_atmel_pwm_bl");
-		if (retval)
-			goto err_free_pwm;
-
 		/* Turn display off by default. */
-		retval = gpio_direction_output(pwmbl->gpio_on,
-				0 ^ pdata->on_active_low);
+		if (pdata->on_active_low)
+			flags = GPIOF_OUT_INIT_HIGH;
+		else
+			flags = GPIOF_OUT_INIT_LOW;
+
+		retval = devm_gpio_request_one(&pdev->dev, pwmbl->gpio_on,
+						flags, "gpio_atmel_pwm_bl");
 		if (retval)
 			goto err_free_pwm;
 	}
-- 
1.8.4.2


^ permalink raw reply related

* Re: [PATCH v2 19/19] fbdev: sh-mobile-lcdcfb: Enable the driver on all ARM platforms
From: Geert Uytterhoeven @ 2013-10-31 19:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1383086274-11049-20-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

	Hi Laurent,

On Tue, 29 Oct 2013, Laurent Pinchart wrote:
> Renesas ARM platforms are transitioning from single-platform to
> multi-platform kernels using the new ARCH_SHMOBILE_MULTI. Make the
> driver available on all ARM platforms to enable it on both ARCH_SHMOBILE
> and ARCH_SHMOBILE_MULTI, and increase build testing coverage with
> COMPILE_TEST.

> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 84b685f..32b5c86 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2482,7 +2482,7 @@ endif
>  
>  config FB_SH_MOBILE_MERAM
>  	tristate "SuperH Mobile MERAM read ahead support"
> -	depends on (SUPERH || ARCH_SHMOBILE)
> +	depends on (SUPERH || ARM || COMPILE_TEST)
>  	select GENERIC_ALLOCATOR
>  	---help---
>  	  Enable MERAM support for the SuperH controller.

While the below compiler warnings have been seen with sh-randconfig
before, they're more likely to happen with COMPILE_TEST=y.
I now see them with e.g. m68k-allmodconfig, so I created a patch.

From 15eb69172457c675cde177a6f742b6f1dabdeb18 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Thu, 31 Oct 2013 20:35:14 +0100
Subject: [PATCH] fbdev: sh_mobile_meram: Fix defined but not used compiler
 warnings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If CONFIG_PM_SLEEP resp. CONFIG_PM_RUNTIME are not set:

drivers/video/sh_mobile_meram.c:573: warning: ¡sh_mobile_meram_suspend¢ defined but not used
drivers/video/sh_mobile_meram.c:597: warning: ¡sh_mobile_meram_resume¢ defined but not used

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/video/sh_mobile_meram.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index e0f098562a74..56a8e47ffceb 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -569,6 +569,7 @@ EXPORT_SYMBOL_GPL(sh_mobile_meram_cache_update);
  * Power management
  */
 
+#ifdef CONFIG_PM_SLEEP
 static int sh_mobile_meram_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -592,7 +593,9 @@ static int sh_mobile_meram_suspend(struct device *dev)
 	}
 	return 0;
 }
+#endif
 
+#ifdef CONFIG_PM_RUNTIME
 static int sh_mobile_meram_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -611,6 +614,7 @@ static int sh_mobile_meram_resume(struct device *dev)
 		meram_write_reg(priv->base, common_regs[i], priv->regs[i]);
 	return 0;
 }
+#endif
 
 static UNIVERSAL_DEV_PM_OPS(sh_mobile_meram_dev_pm_ops,
 			    sh_mobile_meram_suspend,
-- 
1.7.9.5

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply related

* Re: [PATCH v3 21/32] drm/exynos: Move dp driver from video/ to drm/
From: Jingoo Han @ 2013-10-31 23:06 UTC (permalink / raw)
  To: 'Inki Dae', 'Sean Paul', dri-devel
  Cc: 'Tomi Valkeinen',
	'Jean-Christophe PLAGNIOL-VILLARD', linux-fbdev,
	linux-samsung-soc, airlied, tomasz.figa, marcheu,
	'Jingoo Han'
In-Reply-To: <031801ced626$7cca5820$765f0860$%dae@samsung.com>

On Thursday, October 31, 2013 7:47 PM, Inki Dae wrote:
> 
> CCing Jingoo,
> 
> Is that ok to remove eDP driver from video/exynos? Isn't this driver really
> used by Linux framebuffer driver, s3c-fb.c?

+cc Tomi Valkeinen, Jean-Christophe PLAGNIOL-VILLARD,
     linux-fbdev list, linux-samsung-soc list

Yes, it is used by s3c-fb.c.

> 
> Of course, now s3c-fb driver is dead code because this driver doesn't
> support device tree yet but we would need more reviews and discussions about
> moving this driver into drm side. Let's watch new rules for device tree
> bindings of DRM world. So I'd not like to merge this driver yet.

's3c-fb' driver is still used for other mass products projects.
Just, device tree support patch is not yet submitted.


Sean Paul,
When moving the driver, notify related Maintainers of that driver.
Please use 'scripts/get_maintainer.pl' next time.

Best regards,
Jingoo Han


^ permalink raw reply

* Re: [PATCH v3 21/32] drm/exynos: Move dp driver from video/ to drm/
From: Tomasz Figa @ 2013-10-31 23:11 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Inki Dae', 'Sean Paul', dri-devel,
	'Tomi Valkeinen',
	'Jean-Christophe PLAGNIOL-VILLARD', linux-fbdev,
	linux-samsung-soc, airlied, marcheu
In-Reply-To: <000701ced68d$c3cd7d80$4b687880$%han@samsung.com>

On Friday 01 of November 2013 08:06:00 Jingoo Han wrote:
> On Thursday, October 31, 2013 7:47 PM, Inki Dae wrote:
> > CCing Jingoo,
> > 
> > Is that ok to remove eDP driver from video/exynos? Isn't this driver
> > really used by Linux framebuffer driver, s3c-fb.c?
> 
> +cc Tomi Valkeinen, Jean-Christophe PLAGNIOL-VILLARD,
>      linux-fbdev list, linux-samsung-soc list
> 
> Yes, it is used by s3c-fb.c.
> 
> > Of course, now s3c-fb driver is dead code because this driver doesn't
> > support device tree yet but we would need more reviews and discussions
> > about moving this driver into drm side. Let's watch new rules for
> > device tree bindings of DRM world. So I'd not like to merge this
> > driver yet.
> 's3c-fb' driver is still used for other mass products projects.
> Just, device tree support patch is not yet submitted.

Current in-tree users of s3c-fb drivers are s3c2443, non-DT s3c64xx and 
all s5p* SoCs. It is not used on Exynos SoCs anymore.

As for Exynos DP driver, what SoCs does it support? If only Exynos (as the 
name suggests) then there is no point in keeping it at video/exynos and 
making it a part of Exynos DRM driver seems reasonable to me.

Best regards,
Tomasz


^ permalink raw reply

* Re: [PATCH v3 21/32] drm/exynos: Move dp driver from video/ to drm/
From: Jingoo Han @ 2013-10-31 23:23 UTC (permalink / raw)
  To: 'Tomasz Figa'
  Cc: 'Inki Dae', 'Sean Paul', dri-devel,
	'Tomi Valkeinen',
	'Jean-Christophe PLAGNIOL-VILLARD', linux-fbdev,
	linux-samsung-soc, airlied, marcheu, 'Jingoo Han'
In-Reply-To: <1598437.V5FBHdEq6k@flatron>

On Friday, November 01, 2013 8:12 AM, Tomasz Figa wrote:
> On Friday 01 of November 2013 08:06:00 Jingoo Han wrote:
> > On Thursday, October 31, 2013 7:47 PM, Inki Dae wrote:
> > > CCing Jingoo,
> > >
> > > Is that ok to remove eDP driver from video/exynos? Isn't this driver
> > > really used by Linux framebuffer driver, s3c-fb.c?
> >
> > +cc Tomi Valkeinen, Jean-Christophe PLAGNIOL-VILLARD,
> >      linux-fbdev list, linux-samsung-soc list
> >
> > Yes, it is used by s3c-fb.c.
> >
> > > Of course, now s3c-fb driver is dead code because this driver doesn't
> > > support device tree yet but we would need more reviews and discussions
> > > about moving this driver into drm side. Let's watch new rules for
> > > device tree bindings of DRM world. So I'd not like to merge this
> > > driver yet.
> > 's3c-fb' driver is still used for other mass products projects.
> > Just, device tree support patch is not yet submitted.
> 
> Current in-tree users of s3c-fb drivers are s3c2443, non-DT s3c64xx and
> all s5p* SoCs. It is not used on Exynos SoCs anymore.

Hi Tomasz Figa,

Some mass product projects using Exynos5250 and etc, use s3c-fb driver
and dp driver. Also, these projects are still using Framebuffer, not DRM.

> 
> As for Exynos DP driver, what SoCs does it support? If only Exynos (as the
> name suggests) then there is no point in keeping it at video/exynos and
> making it a part of Exynos DRM driver seems reasonable to me.

However, when considering only mainline kernel, I have no strong objection.
As you know, many Linux kernel based OS projects using Exynos, are using
DRM, not Framebuffer.

Also, if moving DP driver to DRM, MAINTAINERS entry for Exynos DP driver
should be updated, too.

Best regards,
Jingoo Han


^ permalink raw reply

* Re: [PATCH v3 21/32] drm/exynos: Move dp driver from video/ to drm/
From: Tomasz Figa @ 2013-10-31 23:27 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Inki Dae', 'Sean Paul', dri-devel,
	'Tomi Valkeinen',
	'Jean-Christophe PLAGNIOL-VILLARD', linux-fbdev,
	linux-samsung-soc, airlied, marcheu
In-Reply-To: <000801ced690$47480520$d5d80f60$%han@samsung.com>

On Friday 01 of November 2013 08:23:59 Jingoo Han wrote:
> On Friday, November 01, 2013 8:12 AM, Tomasz Figa wrote:
> > On Friday 01 of November 2013 08:06:00 Jingoo Han wrote:
> > > On Thursday, October 31, 2013 7:47 PM, Inki Dae wrote:
> > > > CCing Jingoo,
> > > > 
> > > > Is that ok to remove eDP driver from video/exynos? Isn't this
> > > > driver
> > > > really used by Linux framebuffer driver, s3c-fb.c?
> > > 
> > > +cc Tomi Valkeinen, Jean-Christophe PLAGNIOL-VILLARD,
> > > 
> > >      linux-fbdev list, linux-samsung-soc list
> > > 
> > > Yes, it is used by s3c-fb.c.
> > > 
> > > > Of course, now s3c-fb driver is dead code because this driver
> > > > doesn't
> > > > support device tree yet but we would need more reviews and
> > > > discussions
> > > > about moving this driver into drm side. Let's watch new rules for
> > > > device tree bindings of DRM world. So I'd not like to merge this
> > > > driver yet.
> > > 
> > > 's3c-fb' driver is still used for other mass products projects.
> > > Just, device tree support patch is not yet submitted.
> > 
> > Current in-tree users of s3c-fb drivers are s3c2443, non-DT s3c64xx
> > and
> > all s5p* SoCs. It is not used on Exynos SoCs anymore.
> 
> Hi Tomasz Figa,

Just Tomasz. ;)

> Some mass product projects using Exynos5250 and etc, use s3c-fb driver
> and dp driver. Also, these projects are still using Framebuffer, not
> DRM.

Well, those are based on vendor trees anyway, so do not really affect 
mainline kernel.

> > As for Exynos DP driver, what SoCs does it support? If only Exynos (as
> > the name suggests) then there is no point in keeping it at
> > video/exynos and making it a part of Exynos DRM driver seems
> > reasonable to me.
> 
> However, when considering only mainline kernel, I have no strong
> objection. As you know, many Linux kernel based OS projects using
> Exynos, are using DRM, not Framebuffer.

Generally, fbdev is strongly discouraged in any new systems and DRM is the 
way to go, so I don't think we should ever want to bring s3c-fb support 
back to Exynos platforms.

> 
> Also, if moving DP driver to DRM, MAINTAINERS entry for Exynos DP driver
> should be updated, too.

That's correct.

Best regards,
Tomasz


^ permalink raw reply

* Re: [PATCH v3 21/32] drm/exynos: Move dp driver from video/ to drm/
From: Jingoo Han @ 2013-10-31 23:55 UTC (permalink / raw)
  To: 'Tomasz Figa'
  Cc: 'Inki Dae', 'Sean Paul', dri-devel,
	'Tomi Valkeinen',
	'Jean-Christophe PLAGNIOL-VILLARD', linux-fbdev,
	linux-samsung-soc, airlied, marcheu, 'Jingoo Han'
In-Reply-To: <10329504.DMev7iL3eL@flatron>

On Friday, November 01, 2013 8:27 AM, Tomasz Figa wrote:
> On Friday 01 of November 2013 08:23:59 Jingoo Han wrote:
> > On Friday, November 01, 2013 8:12 AM, Tomasz Figa wrote:
> > > On Friday 01 of November 2013 08:06:00 Jingoo Han wrote:
> > > > On Thursday, October 31, 2013 7:47 PM, Inki Dae wrote:
> > > > > CCing Jingoo,
> > > > >
> > > > > Is that ok to remove eDP driver from video/exynos? Isn't this
> > > > > driver
> > > > > really used by Linux framebuffer driver, s3c-fb.c?
> > > >
> > > > +cc Tomi Valkeinen, Jean-Christophe PLAGNIOL-VILLARD,
> > > >
> > > >      linux-fbdev list, linux-samsung-soc list
> > > >
> > > > Yes, it is used by s3c-fb.c.
> > > >
> > > > > Of course, now s3c-fb driver is dead code because this driver
> > > > > doesn't
> > > > > support device tree yet but we would need more reviews and
> > > > > discussions
> > > > > about moving this driver into drm side. Let's watch new rules for
> > > > > device tree bindings of DRM world. So I'd not like to merge this
> > > > > driver yet.
> > > >
> > > > 's3c-fb' driver is still used for other mass products projects.
> > > > Just, device tree support patch is not yet submitted.
> > >
> > > Current in-tree users of s3c-fb drivers are s3c2443, non-DT s3c64xx
> > > and
> > > all s5p* SoCs. It is not used on Exynos SoCs anymore.
> >
> > Hi Tomasz Figa,
> 
> Just Tomasz. ;)

Hi Tomasz, :-)

> 
> > Some mass product projects using Exynos5250 and etc, use s3c-fb driver
> > and dp driver. Also, these projects are still using Framebuffer, not
> > DRM.
> 
> Well, those are based on vendor trees anyway, so do not really affect
> mainline kernel.

OK, I see.

> 
> > > As for Exynos DP driver, what SoCs does it support? If only Exynos (as
> > > the name suggests) then there is no point in keeping it at
> > > video/exynos and making it a part of Exynos DRM driver seems
> > > reasonable to me.
> >
> > However, when considering only mainline kernel, I have no strong
> > objection. As you know, many Linux kernel based OS projects using
> > Exynos, are using DRM, not Framebuffer.
> 
> Generally, fbdev is strongly discouraged in any new systems and DRM is the
> way to go, so I don't think we should ever want to bring s3c-fb support
> back to Exynos platforms.

Yes, you're right.
Personally, I think that all Exynos platforms should go into DRM, not FB.

One more thing, then how about moving Exynos MIPI to DRM side?
Is there any plan on Exynos MIPI?

Best regards,
Jingoo Han

> 
> >
> > Also, if moving DP driver to DRM, MAINTAINERS entry for Exynos DP driver
> > should be updated, too.
> 
> That's correct.


^ permalink raw reply

* Re: [PATCH v3 21/32] drm/exynos: Move dp driver from video/ to drm/
From: Tomasz Figa @ 2013-11-01  0:01 UTC (permalink / raw)
  To: Jingoo Han
  Cc: linux-fbdev, linux-samsung-soc, dri-devel,
	'Tomi Valkeinen', marcheu,
	'Jean-Christophe PLAGNIOL-VILLARD'
In-Reply-To: <003801ced694$a3342310$e99c6930$%han@samsung.com>

On Friday 01 of November 2013 08:55:12 Jingoo Han wrote:
> On Friday, November 01, 2013 8:27 AM, Tomasz Figa wrote:
> > On Friday 01 of November 2013 08:23:59 Jingoo Han wrote:
> > > On Friday, November 01, 2013 8:12 AM, Tomasz Figa wrote:
> > > > On Friday 01 of November 2013 08:06:00 Jingoo Han wrote:
> > > > > On Thursday, October 31, 2013 7:47 PM, Inki Dae wrote:
> > > > > > CCing Jingoo,
> > > > > > 
> > > > > > Is that ok to remove eDP driver from video/exynos? Isn't this
> > > > > > driver
> > > > > > really used by Linux framebuffer driver, s3c-fb.c?
> > > > > 
> > > > > +cc Tomi Valkeinen, Jean-Christophe PLAGNIOL-VILLARD,
> > > > > 
> > > > >      linux-fbdev list, linux-samsung-soc list
> > > > > 
> > > > > Yes, it is used by s3c-fb.c.
> > > > > 
> > > > > > Of course, now s3c-fb driver is dead code because this driver
> > > > > > doesn't
> > > > > > support device tree yet but we would need more reviews and
> > > > > > discussions
> > > > > > about moving this driver into drm side. Let's watch new rules
> > > > > > for
> > > > > > device tree bindings of DRM world. So I'd not like to merge
> > > > > > this
> > > > > > driver yet.
> > > > > 
> > > > > 's3c-fb' driver is still used for other mass products projects.
> > > > > Just, device tree support patch is not yet submitted.
> > > > 
> > > > Current in-tree users of s3c-fb drivers are s3c2443, non-DT
> > > > s3c64xx
> > > > and
> > > > all s5p* SoCs. It is not used on Exynos SoCs anymore.
> > > 
> > > Hi Tomasz Figa,
> > 
> > Just Tomasz. ;)
> 
> Hi Tomasz, :-)
> 
> > > Some mass product projects using Exynos5250 and etc, use s3c-fb
> > > driver
> > > and dp driver. Also, these projects are still using Framebuffer, not
> > > DRM.
> > 
> > Well, those are based on vendor trees anyway, so do not really affect
> > mainline kernel.
> 
> OK, I see.
> 
> > > > As for Exynos DP driver, what SoCs does it support? If only Exynos
> > > > (as
> > > > the name suggests) then there is no point in keeping it at
> > > > video/exynos and making it a part of Exynos DRM driver seems
> > > > reasonable to me.
> > > 
> > > However, when considering only mainline kernel, I have no strong
> > > objection. As you know, many Linux kernel based OS projects using
> > > Exynos, are using DRM, not Framebuffer.
> > 
> > Generally, fbdev is strongly discouraged in any new systems and DRM is
> > the way to go, so I don't think we should ever want to bring s3c-fb
> > support back to Exynos platforms.
> 
> Yes, you're right.
> Personally, I think that all Exynos platforms should go into DRM, not
> FB.
> 
> One more thing, then how about moving Exynos MIPI to DRM side?
> Is there any plan on Exynos MIPI?

Well, it will eventually have to be moved somewhere else than it is, but I 
believe this will have to wait for Common Display Framework.

This is because the case of MIPI DSI is slightly different from 
DisplayPort, since it is not an enumerable/auto-configurable interface and 
requires dedicated panel drivers and static data provided by board 
designers (in DT for example). Handling of such things in a generic way 
will be done by CDF.

Best regards,
Tomasz


^ permalink raw reply

* Re: outcome of DRM/KMS DT bindings session
From: Dave Airlie @ 2013-11-01  0:10 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: ksummit-2013-discuss, Linux Fbdev development list, dri-devel
In-Reply-To: <20131030120229.GF24559@pengutronix.de>

>> After looking at some of the ordering issues we've had with x86 GPUs
>> (which are really just a tightly coupled SoC) I don't want separate
>> drivers all having their own init, suspend/resume paths in them as I
>> know we'll have to start making special vtable entry points etc to
>> solve some random ordering issues that crop up.
>
> The DRM device has to be initialized/suspended/resumed as a whole, no
> doubt about that. If that's not the case you indeed open up the door for
> all kinds of ordering issues.
>
> Still the different components can be multiple devices, just initialize
> the drm device once all components are probed. Remove it again once a
> component is removed. Handle suspend in the DRM device, not in
> the individual component drivers. The suspend in the component drivers
> would only be called after the DRM device is completely quiesced.
> Similarly the resume in the component drivers would not reenable the
> components, this instead would be done in the DRM device when all
> components are there again.

But why? why should we have separate drivers for each component of a
tightly coupled SoC?

it makes no sense, having a driver node per every block in the chip
isn't an advantage, it complicates
things for no advantage at all. If we don't have hotplug hw removing
one device shouldn't be possible
this idea that removing a sub-driver should teardown the drm is crazy as well.

>
> This way all components could be proper (driver model)devices with
> proper drivers without DRM even noticing that multiple components are
> involved.
>
> Side note: We have no choice anyway. All SoCs can (sometimes must)
> be extended with external I2C devices. On every SoC the I2C bus master
> is a separate device, so we have a multicomponent device (in the sense
> of driver model) already in many cases.
>

Having off-chip i2c devices being part of the driver model is fine,
stuff works like that everywhere,
having each SoC block part of the device model isn't fine unless you
can really prove re-use and
why having separate driver templating for each block is helpful.

I'm not willing to have overly generic sub drivers that provide no
advantage and only add lots
of disadvantage like init and suspend/resume ordering. I know there is
going to be SoC ordering
issues at init time that will end up circular between two separate
drivers each deferring because
they want another driver up. Don't dig us into that hole, i2c has a
well defined ordering of init,
I don't think internal SoC devices are so well defined.

Dave.

^ permalink raw reply

* Re: outcome of DRM/KMS DT bindings session
From: Mark Brown @ 2013-11-01  0:32 UTC (permalink / raw)
  To: Dave Airlie; +Cc: dri-devel, ksummit-2013-discuss, Linux Fbdev development list
In-Reply-To: <CAPM=9ty7+A=YjSbRLxmo9mXSQSkZsZngBvTsryZ8+tHppEMPyw@mail.gmail.com>

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

On Fri, Nov 01, 2013 at 10:10:41AM +1000, Dave Airlie wrote:

> > Still the different components can be multiple devices, just initialize
> > the drm device once all components are probed. Remove it again once a

> But why? why should we have separate drivers for each component of a
> tightly coupled SoC?

> it makes no sense, having a driver node per every block in the chip
> isn't an advantage, it complicates
> things for no advantage at all. If we don't have hotplug hw removing
> one device shouldn't be possible
> this idea that removing a sub-driver should teardown the drm is crazy as well.

One case where this may be required is for integration with SoC power
domains where the DRM components are split between multiple domains (and
it may be more idiomatic even if they aren't).  If the SoC is using
power domains then it will expect to see at least one device within each
domain that gets used to reference count the activity for the domain.

This could just be a composite device per domain though.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv9][ 1/6] video: imxfb: Introduce regulator support.
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-11-01  6:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1383210928-18906-1-git-send-email-denis@eukrea.com>

On 10:15 Thu 31 Oct     , Denis Carikli wrote:
> This commit is based on the following commit by Fabio Estevam:
>   4344429 video: mxsfb: Introduce regulator support
> 
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---

next time you need to add a patch 0/3 with the description and ALL the history
of the different change between version so we can follow

Best Regards,
J.
> ChangeLog v8->v9:
> - return an error if regulator_{enable,disable} fails in
>   imxfb_{enable,disable}_controller, and use it.
> ---
>  drivers/video/imxfb.c |   53 ++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
> index 44ee678..322b358 100644
> --- a/drivers/video/imxfb.c
> +++ b/drivers/video/imxfb.c
> @@ -28,6 +28,7 @@
>  #include <linux/cpufreq.h>
>  #include <linux/clk.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/io.h>
>  #include <linux/math64.h>
> @@ -145,6 +146,7 @@ struct imxfb_info {
>  	struct clk		*clk_ipg;
>  	struct clk		*clk_ahb;
>  	struct clk		*clk_per;
> +	struct regulator	*reg_lcd;
>  	enum imxfb_type		devtype;
>  	bool			enabled;
>  
> @@ -561,14 +563,25 @@ static void imxfb_exit_backlight(struct imxfb_info *fbi)
>  }
>  #endif
>  
> -static void imxfb_enable_controller(struct imxfb_info *fbi)
> +static int imxfb_enable_controller(struct imxfb_info *fbi)
>  {
> +	int ret;
>  
>  	if (fbi->enabled)
> -		return;
> +		return 0;
>  
>  	pr_debug("Enabling LCD controller\n");
>  
> +	if (fbi->reg_lcd) {
> +		ret = regulator_enable(fbi->reg_lcd);
> +		if (ret) {
> +			dev_err(&fbi->pdev->dev,
> +				"lcd regulator enable failed with error: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
>  	writel(fbi->screen_dma, fbi->regs + LCDC_SSA);
>  
>  	/* panning offset 0 (0 pixel offset)        */
> @@ -593,12 +606,16 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
>  		fbi->backlight_power(1);
>  	if (fbi->lcd_power)
>  		fbi->lcd_power(1);
> +
> +	return 0;
>  }
>  
> -static void imxfb_disable_controller(struct imxfb_info *fbi)
> +static int imxfb_disable_controller(struct imxfb_info *fbi)
>  {
> +	int ret;
> +
>  	if (!fbi->enabled)
> -		return;
> +		return 0;
>  
>  	pr_debug("Disabling LCD controller\n");
>  
> @@ -613,6 +630,15 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
>  	fbi->enabled = false;
>  
>  	writel(0, fbi->regs + LCDC_RMCR);
> +
> +	if (fbi->reg_lcd) {
> +		ret = regulator_disable(fbi->reg_lcd);
> +		if (ret)
> +			dev_err(&fbi->pdev->dev,
> +				"lcd regulator disable failed with error: %d\n",
> +				ret);
> +			return ret;
> +	}
>  }
>  
>  static int imxfb_blank(int blank, struct fb_info *info)
> @@ -626,13 +652,12 @@ static int imxfb_blank(int blank, struct fb_info *info)
>  	case FB_BLANK_VSYNC_SUSPEND:
>  	case FB_BLANK_HSYNC_SUSPEND:
>  	case FB_BLANK_NORMAL:
> -		imxfb_disable_controller(fbi);
> -		break;
> +		return imxfb_disable_controller(fbi);
>  
>  	case FB_BLANK_UNBLANK:
> -		imxfb_enable_controller(fbi);
> -		break;
> +		return imxfb_enable_controller(fbi);
>  	}
> +
>  	return 0;
>  }
>  
> @@ -734,8 +759,7 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
>  
>  	pr_debug("%s\n", __func__);
>  
> -	imxfb_disable_controller(fbi);
> -	return 0;
> +	return imxfb_disable_controller(fbi);
>  }
>  
>  static int imxfb_resume(struct platform_device *dev)
> @@ -745,8 +769,7 @@ static int imxfb_resume(struct platform_device *dev)
>  
>  	pr_debug("%s\n", __func__);
>  
> -	imxfb_enable_controller(fbi);
> -	return 0;
> +	return imxfb_enable_controller(fbi);
>  }
>  #else
>  #define imxfb_suspend	NULL
> @@ -1020,6 +1043,12 @@ static int imxfb_probe(struct platform_device *pdev)
>  		goto failed_register;
>  	}
>  
> +	fbi->reg_lcd = devm_regulator_get(&pdev->dev, "lcd");
> +	if (IS_ERR(fbi->reg_lcd)) {
> +		dev_info(&pdev->dev, "No lcd regulator used.\n");
> +		fbi->reg_lcd = NULL;
> +	}
> +
>  	imxfb_enable_controller(fbi);
>  	fbi->pdev = pdev;
>  #ifdef PWMR_BACKLIGHT_AVAILABLE
> -- 
> 1.7.9.5
> 

^ permalink raw reply

* Re: outcome of DRM/KMS DT bindings session
From: Thierry Reding @ 2013-11-01  9:12 UTC (permalink / raw)
  To: Dave Airlie; +Cc: dri-devel, ksummit-2013-discuss, Linux Fbdev development list
In-Reply-To: <CAPM=9ty7+A=YjSbRLxmo9mXSQSkZsZngBvTsryZ8+tHppEMPyw@mail.gmail.com>

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

On Fri, Nov 01, 2013 at 10:10:41AM +1000, Dave Airlie wrote:
> >> After looking at some of the ordering issues we've had with x86 GPUs
> >> (which are really just a tightly coupled SoC) I don't want separate
> >> drivers all having their own init, suspend/resume paths in them as I
> >> know we'll have to start making special vtable entry points etc to
> >> solve some random ordering issues that crop up.
> >
> > The DRM device has to be initialized/suspended/resumed as a whole, no
> > doubt about that. If that's not the case you indeed open up the door for
> > all kinds of ordering issues.
> >
> > Still the different components can be multiple devices, just initialize
> > the drm device once all components are probed. Remove it again once a
> > component is removed. Handle suspend in the DRM device, not in
> > the individual component drivers. The suspend in the component drivers
> > would only be called after the DRM device is completely quiesced.
> > Similarly the resume in the component drivers would not reenable the
> > components, this instead would be done in the DRM device when all
> > components are there again.
> 
> But why? why should we have separate drivers for each component of a
> tightly coupled SoC?
> 
> it makes no sense, having a driver node per every block in the chip
> isn't an advantage, it complicates
> things for no advantage at all. If we don't have hotplug hw removing
> one device shouldn't be possible
> this idea that removing a sub-driver should teardown the drm is crazy as well.

In my opinion separating things out into separate drivers makes it less
complicated. For instance it makes it very easy to manage the various
resources used by each driver (registers, interrupts, ...).

The only added complexity lies in the fact that we need some code to
synchronize the DRM device setup and teardown (and suspend and resume
for that matter). It's been discussed elsewhere that most SoCs are very
similar in their requirements, so I think we should be able to come up
with a piece of code that can be shared between drivers. Perhaps it
would even be possible to share that code between subsystems, since ALSA
and V4L2 may have similar requirements.

That's effectively not very different from what you're proposing. As far
as I can tell the only difference would be that this works in sort of a
"bottom-up" fashion, whereas your proposal would be "top-down".

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v2] efifb: prevent null-deref when iterating dmi_list
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-11-01 11:10 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-fbdev@vger.kernel.org, James Bates, linux-kernel,
	Tomi Valkeinen, James Bates
In-Reply-To: <CANq1E4QAwOG8Vdw2nCb8+LirZ278w58j+wpieeKvnNepga+baw@mail.gmail.com>

On 17:17 Thu 31 Oct     , David Herrmann wrote:
> Hi
> 
> On Thu, Oct 31, 2013 at 11:45 AM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 18:40 Wed 02 Oct     , David Herrmann wrote:
> >> The dmi_list array is initialized using gnu designated initializers, and
> >> therefore may contain fewer explicitly defined entries as there are
> >> elements in it. This is because the enum above with M_xyz constants
> >> contains more items than the designated initializer. Those elements not
> >> explicitly initialized are implicitly set to 0.
> >>
> >> Now efifb_setup() loops through all these array elements, and performs
> >> a strcmp on each item. For non explicitly initialized elements this will
> >> be a null pointer:
> >>
> >> This patch swaps the check order in the if statement, thus checks first
> >> whether dmi_list[i].base is null.
> >>
> >> Signed-off-by: James Bates <james.h.bates@yahoo.com>
> >> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> >
> > with the simpleDRM arriving next merge I'm wondering if we need to keep it?
> 
> SimpleDRM is not coming next merge-window. It's basically finished,
> but I'm still working on the user-space side as its KMS api is highly
> reduced compared to fully-featured DRM/KMS drivers. Maybe 3.13 will
> work out.

do you have a git tree for the simpleDRM that I can pull?
> 
> Anyhow, this patch is still needed as it fixes a serious bug for simplefb.

ok
> 
> Thanks
> David

^ permalink raw reply

* Re: [PATCH v2] efifb: prevent null-deref when iterating dmi_list
From: David Herrmann @ 2013-11-01 12:09 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: linux-fbdev@vger.kernel.org, James Bates, linux-kernel,
	Tomi Valkeinen, James Bates
In-Reply-To: <20131101111020.GD18477@ns203013.ovh.net>

Hi

On Fri, Nov 1, 2013 at 12:10 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 17:17 Thu 31 Oct     , David Herrmann wrote:
>> Hi
>>
>> On Thu, Oct 31, 2013 at 11:45 AM, Jean-Christophe PLAGNIOL-VILLARD
>> <plagnioj@jcrosoft.com> wrote:
>> > On 18:40 Wed 02 Oct     , David Herrmann wrote:
>> >> The dmi_list array is initialized using gnu designated initializers, and
>> >> therefore may contain fewer explicitly defined entries as there are
>> >> elements in it. This is because the enum above with M_xyz constants
>> >> contains more items than the designated initializer. Those elements not
>> >> explicitly initialized are implicitly set to 0.
>> >>
>> >> Now efifb_setup() loops through all these array elements, and performs
>> >> a strcmp on each item. For non explicitly initialized elements this will
>> >> be a null pointer:
>> >>
>> >> This patch swaps the check order in the if statement, thus checks first
>> >> whether dmi_list[i].base is null.
>> >>
>> >> Signed-off-by: James Bates <james.h.bates@yahoo.com>
>> >> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> >
>> > with the simpleDRM arriving next merge I'm wondering if we need to keep it?
>>
>> SimpleDRM is not coming next merge-window. It's basically finished,
>> but I'm still working on the user-space side as its KMS api is highly
>> reduced compared to fully-featured DRM/KMS drivers. Maybe 3.13 will
>> work out.
>
> do you have a git tree for the simpleDRM that I can pull?

Sure, I usually push it to my fdo tree:
  http://cgit.freedesktop.org/~dvdhrm/linux/log/?h=simpledrm
But note that you shouldn't be using it right now. User-space fails on
it as SimpleDRM only provides a single KMS-FB. It also needs to be
adjusted to the new x86-sysfb changes and I am reworking the handover
to real drivers.

Cheers
David

^ permalink raw reply

* Re: outcome of DRM/KMS DT bindings session
From: Inki Dae @ 2013-11-01 15:28 UTC (permalink / raw)
  To: Dave Airlie; +Cc: dri-devel, ksummit-2013-discuss, Linux Fbdev development list
In-Reply-To: <CAPM=9ty7+A=YjSbRLxmo9mXSQSkZsZngBvTsryZ8+tHppEMPyw@mail.gmail.com>

2013/11/1 Dave Airlie <airlied@gmail.com>:
>>> After looking at some of the ordering issues we've had with x86 GPUs
>>> (which are really just a tightly coupled SoC) I don't want separate
>>> drivers all having their own init, suspend/resume paths in them as I
>>> know we'll have to start making special vtable entry points etc to
>>> solve some random ordering issues that crop up.
>>
>> The DRM device has to be initialized/suspended/resumed as a whole, no
>> doubt about that. If that's not the case you indeed open up the door for
>> all kinds of ordering issues.
>>
>> Still the different components can be multiple devices, just initialize
>> the drm device once all components are probed. Remove it again once a
>> component is removed. Handle suspend in the DRM device, not in
>> the individual component drivers. The suspend in the component drivers
>> would only be called after the DRM device is completely quiesced.
>> Similarly the resume in the component drivers would not reenable the
>> components, this instead would be done in the DRM device when all
>> components are there again.
>
> But why? why should we have separate drivers for each component of a
> tightly coupled SoC?
>
> it makes no sense, having a driver node per every block in the chip
> isn't an advantage, it complicates
> things for no advantage at all. If we don't have hotplug hw removing
> one device shouldn't be possible
> this idea that removing a sub-driver should teardown the drm is crazy as well.
>
>>
>> This way all components could be proper (driver model)devices with
>> proper drivers without DRM even noticing that multiple components are
>> involved.
>>
>> Side note: We have no choice anyway. All SoCs can (sometimes must)
>> be extended with external I2C devices. On every SoC the I2C bus master
>> is a separate device, so we have a multicomponent device (in the sense
>> of driver model) already in many cases.
>>
>
> Having off-chip i2c devices being part of the driver model is fine,
> stuff works like that everywhere,
> having each SoC block part of the device model isn't fine unless you
> can really prove re-use and
> why having separate driver templating for each block is helpful.
>
> I'm not willing to have overly generic sub drivers that provide no
> advantage and only add lots
> of disadvantage like init and suspend/resume ordering. I know there is
> going to be SoC ordering
> issues at init time that will end up circular between two separate
> drivers each deferring because
> they want another driver up. Don't dig us into that hole, i2c has a
> well defined ordering of init,
> I don't think internal SoC devices are so well defined.
>

It seems that the main reason we should go to a single drm driver is
the probe ordering issue of sub drivers and the power ordering issue
of them.

First, I'd like to ask qustions to myself and other people. Do we
really need to define the display pipeline node? Is there really any
good way to can use only existing device nodes?

Please suppose the below things,
1. crtc and encoder/connector can be created when KMS driver and
display driver are probed regardless of the ordering
2. A crtc and a connector are connected when last one is created. This
means that a framebuffer will be created and the framebuffer's image
will be transferred to display via KMS driver.


And let see how hardware pipe lines can be linked each other,
1. Top level
CRTC -------- Encoder ---------- Connector

2. CRTC
Display controller or HDMI
Display controller or HDMI ----------  Image Enhancement chips or other

3. Encoder/Connector
LCD Panel
Display bus(mipi, dp)  ------- LCD panel or TV
Display bus(mipi, dp)  ------- bridge device(lvds) ------- LCD panel or TV


As you can see the above, if a crtc and a connector could be connected
each other regardless of the probe order - actually possible, and we
are already using this way in internal project - then I think it's
enough to consider display pipeline node to CRTC, and to
Encoder/Connector individually. DT binding of CRTC including Image
Enhancement chips can be done at top level of drm driver, and DT
binding of Encoder/Connector including bridge device and panel can be
done at probe of separated encoder/connector driver. Of course, for
avoiding power ordering issue, each encoder/connector drivers
shouldn't have their resume/suspend interfaces, and their pm should be
handled by dpms callback at top level of drm driver.

This way I think we could simplify to compose the display pipeline
node as device tree,and also we could have a separated device driver
as driver model.

Thanks,
Inki Dae


> Dave.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ 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