Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc: remove PReP
From: Paul Bolle @ 2013-03-27 12:55 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-fbdev, Florian Tobias Schandinat, linux-doc, linux-kernel,
	Paul Mackerras, Rob Landley, Bjorn Helgaas, linuxppc-dev
In-Reply-To: <1364388152.1345.29.camel@x61.thuisdomein>

By the way, I get bounces from Adam's address:
> I'm sorry to have to inform you that your message could not
> be delivered to one or more recipients. It's attached below.
> 
> For further assistance, please send mail to <postmaster>
> 
> If you do so, please include this problem report. You can
> delete your own text from the attached returned message.
> 
> <abelay@mit.edu>: 550 5.1.1 <abelay@mit.edu>... User unknown

(It's now removed from the CC line.) Does anyone know what's going on
here?


Paul Bolle


^ permalink raw reply

* Re: [PATCH] powerpc: remove PReP
From: Benjamin Herrenschmidt @ 2013-03-27 15:27 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Paul Bolle, Florian Tobias Schandinat, linux-doc, linux-fbdev,
	linux-kernel, Paul Mackerras, Rob Landley, Bjorn Helgaas,
	linuxppc-dev, Adam Belay
In-Reply-To: <CAMuHMdVs-Xxu9e=8WZbTBXE1_+iYOBKODgj9_EKmVbZA3z=8JA@mail.gmail.com>

On Wed, 2013-03-27 at 13:05 +0100, Geert Uytterhoeven wrote:
> On Wed, Mar 27, 2013 at 11:47 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
> > 3) I removed two files in documentation that are almost entirely PReP
> > specific. The remaining lines looked uninteresting.
> 
> > --- a/Documentation/powerpc/sound.txt
> > +++ /dev/null
> 
> > -2. IBM CHRP
> > -
> > -  I have only tested this on the 43P-150.  Build the kernel with the cs4232
> > -  set as a module and load the module with irq=9 dma=1 dma2=2 io=0x550
> 
> This section is CHRP-specific. The cs4232 driver also worked on the
> CHRP LongTrail.
> But I don't remember if the parameter values above also applied to LongTrail.
> I do remember it didn't work without specifing the right parameters, so you
> probably want to keep this section.

Isn't LongTrail support LongDead ? Can we still boot these things at
all ? Anybody still has a functional one ?

Cheers,
Ben.

> 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

* [PATCH] drivers: video: matrox: changed kmalloc+memset with kzalloc
From: Andrei Epure @ 2013-03-27 20:12 UTC (permalink / raw)
  To: FlorianSchandinat, linux-fbdev; +Cc: linux-kernel, Andrei Epure

Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
---
 drivers/video/matrox/matroxfb_base.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/video/matrox/matroxfb_base.c b/drivers/video/matrox/matroxfb_base.c
index 401a56e..2456529 100644
--- a/drivers/video/matrox/matroxfb_base.c
+++ b/drivers/video/matrox/matroxfb_base.c
@@ -2029,10 +2029,9 @@ static int matroxfb_probe(struct pci_dev* pdev, const struct pci_device_id* dumm
 		return -1;
 	}
 
-	minfo = kmalloc(sizeof(*minfo), GFP_KERNEL);
+	minfo = kzalloc(sizeof(*minfo), GFP_KERNEL);
 	if (!minfo)
 		return -1;
-	memset(minfo, 0, sizeof(*minfo));
 
 	minfo->pcidev = pdev;
 	minfo->dead = 0;
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 00/26] DSS device model change
From: Tony Lindgren @ 2013-03-27 20:25 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
In-Reply-To: <51528E1F.9080608@ti.com>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [130326 23:18]:
> On 2013-03-26 19:10, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [130326 06:38]:
> >> Note about board files: I only convert a few board files here for example. I
> >> have a branch with board file changes for all the affected board files. I did
> >> not include them as they are more or less identical. If this series looks good,
> >> I will create an independent branch for the arch/arm stuff, so it can be pulled
> >> separately.
> > 
> > Great looks good to me thanks.
> 
> To make the DSS work properly after this device model change, we also
> need to solve the problem related to multiple display devices on the
> same video bus.
> 
> Did the approach I suggested in
> 
> http://permalink.gmane.org/gmane.linux.ports.arm.omap/96039
> 
> look ok? Overo is the most complex one, I think the rest of the boards
> that require changes have just LCD and DVI that conflict. But it still
> means we'll have that kind of choice Kconfig option for all those boards.

Yes I think that's the best way to go until we just drop the board-*.c
files in favor of device tree.

Regards,

Tony

^ permalink raw reply

* [PATCH] video: exynos: Convert to devm_ioremap_resource()
From: Andrei Epure @ 2013-03-27 20:28 UTC (permalink / raw)
  To: inki.dae, dh09.lee, kyungmin.park, FlorianSchandinat
  Cc: linux-fbdev, linux-kernel, Andrei Epure

The new devm_ioremap_resource() provides its own error messages.
Patch found using coccinelle.

Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
---
 drivers/video/exynos/exynos_mipi_dsi.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index fac7df6..dd5e5e9 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -384,10 +384,9 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
-	dsim->reg_base = devm_request_and_ioremap(&pdev->dev, res);
-	if (!dsim->reg_base) {
-		dev_err(&pdev->dev, "failed to remap io region\n");
-		ret = -ENOMEM;
+	dsim->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(dsim->reg_base)) {
+		ret = PTR_ERR(dsim->reg_base);
 		goto error;
 	}
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] video: omap2: dss: use PTR_RET function
From: Andrei Epure @ 2013-03-27 20:35 UTC (permalink / raw)
  To: tomi.valkeinen, FlorianSchandinat
  Cc: sumit.semwal, archit, cmahapatra, linux-omap, linux-fbdev,
	linux-kernel, Andrei Epure

Use PTR_RET function instead of IS_ERR + PTR_ERR.
Patch found using coccinelle.

Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
---
 drivers/video/omap2/dss/core.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index f8779d4..60cc6fe 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -181,10 +181,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
 	d = debugfs_create_file(name, S_IRUGO, dss_debugfs_dir,
 			write, &dss_debug_fops);
 
-	if (IS_ERR(d))
-		return PTR_ERR(d);
-
-	return 0;
+	return PTR_RET(d);
 }
 #else /* CONFIG_OMAP2_DSS_DEBUGFS */
 static inline int dss_initialize_debugfs(void)
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] video: mmp: removed reference preceded by free
From: Andrei Epure @ 2013-03-27 20:48 UTC (permalink / raw)
  To: FlorianSchandinat
  Cc: akpm, haojian.zhuang, cldu, zzhu3, linux-fbdev, linux-kernel,
	Andrei Epure

Patch found with coccinelle.

Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
---
 drivers/video/mmp/core.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/video/mmp/core.c b/drivers/video/mmp/core.c
index 9ed8341..84de263 100644
--- a/drivers/video/mmp/core.c
+++ b/drivers/video/mmp/core.c
@@ -252,7 +252,5 @@ void mmp_unregister_path(struct mmp_path *path)
 
 	kfree(path);
 	mutex_unlock(&disp_lock);
-
-	dev_info(path->dev, "de-register %s\n", path->name);
 }
 EXPORT_SYMBOL_GPL(mmp_unregister_path);
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] video: exynos: remove useless safety check in list traversal
From: Andrei Epure @ 2013-03-27 21:19 UTC (permalink / raw)
  To: inki.dae, dh09.lee, kyungmin.park, FlorianSchandinat
  Cc: linux-fbdev, linux-kernel, Andrei Epure

list_for_each_entry_safe() does not require safety check.
Patch found using coccinelle.

Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
---
 drivers/video/exynos/exynos_mipi_dsi.c |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index dd5e5e9..fe84f08 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -214,8 +214,6 @@ static struct mipi_dsim_ddi *exynos_mipi_dsi_find_lcd_device(
 	mutex_lock(&mipi_dsim_lock);
 
 	list_for_each_entry_safe(dsim_ddi, next, &dsim_ddi_list, list) {
-		if (!dsim_ddi)
-			goto out;
 
 		lcd_dev = dsim_ddi->dsim_lcd_dev;
 		if (!lcd_dev)
@@ -473,17 +471,15 @@ static int exynos_mipi_dsi_remove(struct platform_device *pdev)
 	clk_disable(dsim->clock);
 
 	list_for_each_entry_safe(dsim_ddi, next, &dsim_ddi_list, list) {
-		if (dsim_ddi) {
-			if (dsim->id != dsim_ddi->bus_id)
-				continue;
+		if (dsim->id != dsim_ddi->bus_id)
+			continue;
 
-			dsim_lcd_drv = dsim_ddi->dsim_lcd_drv;
+		dsim_lcd_drv = dsim_ddi->dsim_lcd_drv;
 
-			if (dsim_lcd_drv->remove)
-				dsim_lcd_drv->remove(dsim_ddi->dsim_lcd_dev);
+		if (dsim_lcd_drv->remove)
+			dsim_lcd_drv->remove(dsim_ddi->dsim_lcd_dev);
 
-			kfree(dsim_ddi);
-		}
+		kfree(dsim_ddi);
 	}
 
 	return 0;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] video: fixed missing iounmap coccinelle errors
From: Andrei Epure @ 2013-03-27 23:43 UTC (permalink / raw)
  To: linux-arm-kernel

Modified or added the necessary goto statements so that the ioremapped
regions get unmapped before return.

Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
---
 drivers/video/vt8500lcdfb.c |    7 ++++---
 drivers/video/wm8505fb.c    |    7 ++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
index aa2579c..b4ccca2 100644
--- a/drivers/video/vt8500lcdfb.c
+++ b/drivers/video/vt8500lcdfb.c
@@ -350,7 +350,7 @@ static int vt8500lcd_probe(struct platform_device *pdev)
 	if (!np) {
 		pr_err("%s: No display description in Device Tree\n", __func__);
 		ret = -EINVAL;
-		goto failed_free_res;
+		goto failed_free_io;
 	}
 
 	/*
@@ -369,7 +369,7 @@ static int vt8500lcd_probe(struct platform_device *pdev)
 	ret |= of_property_read_u32(np, "bpp", &bpp);
 	if (ret) {
 		pr_err("%s: Unable to read display properties\n", __func__);
-		goto failed_free_res;
+		goto failed_free_io;
 	}
 	of_mode.vmode = FB_VMODE_NONINTERLACED;
 
@@ -379,7 +379,8 @@ static int vt8500lcd_probe(struct platform_device *pdev)
 				GFP_KERNEL);
 	if (!fb_mem_virt) {
 		pr_err("%s: Failed to allocate framebuffer\n", __func__);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto failed_free_io;
 	};
 
 	fbi->fb.fix.smem_start	= fb_mem_phys;
diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
index 4dd0580..2e8298e 100644
--- a/drivers/video/wm8505fb.c
+++ b/drivers/video/wm8505fb.c
@@ -332,7 +332,7 @@ static int wm8505fb_probe(struct platform_device *pdev)
 	if (!np) {
 		pr_err("%s: No display description in Device Tree\n", __func__);
 		ret = -EINVAL;
-		goto failed_free_res;
+		goto failed_free_io;
 	}
 
 	/*
@@ -351,7 +351,7 @@ static int wm8505fb_probe(struct platform_device *pdev)
 	ret |= of_property_read_u32(np, "bpp", &bpp);
 	if (ret) {
 		pr_err("%s: Unable to read display properties\n", __func__);
-		goto failed_free_res;
+		goto failed_free_io;
 	}
 
 	of_mode.vmode = FB_VMODE_NONINTERLACED;
@@ -369,7 +369,8 @@ static int wm8505fb_probe(struct platform_device *pdev)
 				GFP_KERNEL);
 	if (!fb_mem_virt) {
 		pr_err("%s: Failed to allocate framebuffer\n", __func__);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto failed_free_io;
 	};
 
 	fbi->fb.var.xres_virtual	= of_mode.xres;
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH] video: exynos: Convert to devm_ioremap_resource()
From: Donghwa Lee @ 2013-03-27 23:52 UTC (permalink / raw)
  To: Andrei Epure
  Cc: inki.dae, kyungmin.park, FlorianSchandinat, linux-fbdev,
	linux-kernel
In-Reply-To: <1364416081-24113-1-git-send-email-epure.andrei@gmail.com>


Hi,

Please refer to the following link.

http://marc.info/?l=linux-fbdev&m\x135963121130437&w=2

Best regard,
Donghwa Lee

On Thu, Mar 28, 2013 at 05:28, Andrei Epure wrote:
> The new devm_ioremap_resource() provides its own error messages.
> Patch found using coccinelle.
>
> Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
> ---
>   drivers/video/exynos/exynos_mipi_dsi.c |    7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
> index fac7df6..dd5e5e9 100644
> --- a/drivers/video/exynos/exynos_mipi_dsi.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
> @@ -384,10 +384,9 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   
> -	dsim->reg_base = devm_request_and_ioremap(&pdev->dev, res);
> -	if (!dsim->reg_base) {
> -		dev_err(&pdev->dev, "failed to remap io region\n");
> -		ret = -ENOMEM;
> +	dsim->reg_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(dsim->reg_base)) {
> +		ret = PTR_ERR(dsim->reg_base);
>   		goto error;
>   	}
>   


^ permalink raw reply

* [PATCH] video: fix invalid free of devm_ allocated data
From: Andrei Epure @ 2013-03-27 23:55 UTC (permalink / raw)
  To: linux-arm-kernel

The objects allocated by devm_* APIs are managed by devres and are freed when
the device is detached. Hence there is no need to use kfree() explicitly.
Patch found using coccinelle.

Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
---
 drivers/video/vt8500lcdfb.c |    3 ---
 drivers/video/wm8505fb.c    |    3 ---
 2 files changed, 6 deletions(-)

diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
index b4ccca2..ea3ee61 100644
--- a/drivers/video/vt8500lcdfb.c
+++ b/drivers/video/vt8500lcdfb.c
@@ -465,7 +465,6 @@ failed_free_res:
 	release_mem_region(res->start, resource_size(res));
 failed_fbi:
 	platform_set_drvdata(pdev, NULL);
-	kfree(fbi);
 failed:
 	return ret;
 }
@@ -494,8 +493,6 @@ static int vt8500lcd_remove(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(res->start, resource_size(res));
 
-	kfree(fbi);
-
 	return 0;
 }
 
diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
index 2e8298e..34a2de1 100644
--- a/drivers/video/wm8505fb.c
+++ b/drivers/video/wm8505fb.c
@@ -427,7 +427,6 @@ failed_free_res:
 	release_mem_region(res->start, resource_size(res));
 failed_fbi:
 	platform_set_drvdata(pdev, NULL);
-	kfree(fbi);
 failed:
 	return ret;
 }
@@ -451,8 +450,6 @@ static int wm8505fb_remove(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(res->start, resource_size(res));
 
-	kfree(fbi);
-
 	return 0;
 }
 
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH] video: exynos: remove useless safety check in list traversal
From: Donghwa Lee @ 2013-03-28  0:04 UTC (permalink / raw)
  To: Andrei Epure
  Cc: inki.dae, kyungmin.park, FlorianSchandinat, linux-fbdev,
	linux-kernel
In-Reply-To: <1364419145-30375-1-git-send-email-epure.andrei@gmail.com>


Hi,

It looks good to me.

Acked-by: Donghwa Lee <dh09.lee@samsung.com>

Best regard,
Donghwa Lee

On Thu, Mar 28, 2013 at 06:19, Andrei Epure wrote:
> list_for_each_entry_safe() does not require safety check.
> Patch found using coccinelle.
>
> Signed-off-by: Andrei Epure<epure.andrei@gmail.com>
> ---
>   drivers/video/exynos/exynos_mipi_dsi.c |   16 ++++++----------
>   1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
> index dd5e5e9..fe84f08 100644
> --- a/drivers/video/exynos/exynos_mipi_dsi.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
> @@ -214,8 +214,6 @@ static struct mipi_dsim_ddi *exynos_mipi_dsi_find_lcd_device(
>   	mutex_lock(&mipi_dsim_lock);
>   
>   	list_for_each_entry_safe(dsim_ddi, next, &dsim_ddi_list, list) {
> -		if (!dsim_ddi)
> -			goto out;
>   
>   		lcd_dev = dsim_ddi->dsim_lcd_dev;
>   		if (!lcd_dev)
> @@ -473,17 +471,15 @@ static int exynos_mipi_dsi_remove(struct platform_device *pdev)
>   	clk_disable(dsim->clock);
>   
>   	list_for_each_entry_safe(dsim_ddi, next, &dsim_ddi_list, list) {
> -		if (dsim_ddi) {
> -			if (dsim->id != dsim_ddi->bus_id)
> -				continue;
> +		if (dsim->id != dsim_ddi->bus_id)
> +			continue;
>   
> -			dsim_lcd_drv = dsim_ddi->dsim_lcd_drv;
> +		dsim_lcd_drv = dsim_ddi->dsim_lcd_drv;
>   
> -			if (dsim_lcd_drv->remove)
> -				dsim_lcd_drv->remove(dsim_ddi->dsim_lcd_dev);
> +		if (dsim_lcd_drv->remove)
> +			dsim_lcd_drv->remove(dsim_ddi->dsim_lcd_dev);
>   
> -			kfree(dsim_ddi);
> -		}
> +		kfree(dsim_ddi);
>   	}
>   
>   	return 0;


^ permalink raw reply

* Re: [PATCH] video: mmp: removed reference preceded by free
From: Zhou Zhu @ 2013-03-28  1:38 UTC (permalink / raw)
  To: Andrei Epure
  Cc: FlorianSchandinat@gmx.de, akpm@linux-foundation.org,
	haojian.zhuang@gmail.com, Lisa Du, linux-fbdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1364417318-26650-1-git-send-email-epure.andrei@gmail.com>

On 03/28/2013 04:48 AM, Andrei Epure wrote:
> Patch found with coccinelle.
>
> Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
> ---
>   drivers/video/mmp/core.c |    2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/drivers/video/mmp/core.c b/drivers/video/mmp/core.c
> index 9ed8341..84de263 100644
> --- a/drivers/video/mmp/core.c
> +++ b/drivers/video/mmp/core.c
> @@ -252,7 +252,5 @@ void mmp_unregister_path(struct mmp_path *path)
>
>   	kfree(path);
>   	mutex_unlock(&disp_lock);
> -
> -	dev_info(path->dev, "de-register %s\n", path->name);
>   }
>   EXPORT_SYMBOL_GPL(mmp_unregister_path);
>
Acked-by: Zhou Zhu <zzhu3@marvell.com>

^ permalink raw reply

* Re: [Bulk] [PATCH] video: fix invalid free of devm_ allocated data
From: Tony Prisk @ 2013-03-28  4:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1364428524-13821-1-git-send-email-epure.andrei@gmail.com>

On Thu, 2013-03-28 at 01:55 +0200, Andrei Epure wrote:
> The objects allocated by devm_* APIs are managed by devres and are freed when
> the device is detached. Hence there is no need to use kfree() explicitly.
> Patch found using coccinelle.
> 
> Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
> ---
>  drivers/video/vt8500lcdfb.c |    3 ---
>  drivers/video/wm8505fb.c    |    3 ---
>  2 files changed, 6 deletions(-)
> 
> diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
> index b4ccca2..ea3ee61 100644
> --- a/drivers/video/vt8500lcdfb.c
> +++ b/drivers/video/vt8500lcdfb.c
> @@ -465,7 +465,6 @@ failed_free_res:
>  	release_mem_region(res->start, resource_size(res));
>  failed_fbi:
>  	platform_set_drvdata(pdev, NULL);
> -	kfree(fbi);
>  failed:
>  	return ret;
>  }
> @@ -494,8 +493,6 @@ static int vt8500lcd_remove(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(res->start, resource_size(res));
>  
> -	kfree(fbi);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
> index 2e8298e..34a2de1 100644
> --- a/drivers/video/wm8505fb.c
> +++ b/drivers/video/wm8505fb.c
> @@ -427,7 +427,6 @@ failed_free_res:
>  	release_mem_region(res->start, resource_size(res));
>  failed_fbi:
>  	platform_set_drvdata(pdev, NULL);
> -	kfree(fbi);
>  failed:
>  	return ret;
>  }
> @@ -451,8 +450,6 @@ static int wm8505fb_remove(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(res->start, resource_size(res));
>  
> -	kfree(fbi);
> -
>  	return 0;
>  }
>  

NACK
Already have a patch queued up for this from Julia Lawall

Regards
Tony P


^ permalink raw reply

* Re: [PATCH] video: fixed missing iounmap coccinelle errors
From: Tony Prisk @ 2013-03-28  4:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1364427805-11972-1-git-send-email-epure.andrei@gmail.com>

On Thu, 2013-03-28 at 01:43 +0200, Andrei Epure wrote:
> Modified or added the necessary goto statements so that the ioremapped
> regions get unmapped before return.
> 
> Signed-off-by: Andrei Epure <epure.andrei@gmail.com>
> ---
>  drivers/video/vt8500lcdfb.c |    7 ++++---
>  drivers/video/wm8505fb.c    |    7 ++++---
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
> index aa2579c..b4ccca2 100644
> --- a/drivers/video/vt8500lcdfb.c
> +++ b/drivers/video/vt8500lcdfb.c
> @@ -350,7 +350,7 @@ static int vt8500lcd_probe(struct platform_device *pdev)
>  	if (!np) {
>  		pr_err("%s: No display description in Device Tree\n", __func__);
>  		ret = -EINVAL;
> -		goto failed_free_res;
> +		goto failed_free_io;
>  	}
>  
>  	/*
> @@ -369,7 +369,7 @@ static int vt8500lcd_probe(struct platform_device *pdev)
>  	ret |= of_property_read_u32(np, "bpp", &bpp);
>  	if (ret) {
>  		pr_err("%s: Unable to read display properties\n", __func__);
> -		goto failed_free_res;
> +		goto failed_free_io;
>  	}
>  	of_mode.vmode = FB_VMODE_NONINTERLACED;
>  
> @@ -379,7 +379,8 @@ static int vt8500lcd_probe(struct platform_device *pdev)
>  				GFP_KERNEL);
>  	if (!fb_mem_virt) {
>  		pr_err("%s: Failed to allocate framebuffer\n", __func__);
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto failed_free_io;
>  	};
>  
>  	fbi->fb.fix.smem_start	= fb_mem_phys;
> diff --git a/drivers/video/wm8505fb.c b/drivers/video/wm8505fb.c
> index 4dd0580..2e8298e 100644
> --- a/drivers/video/wm8505fb.c
> +++ b/drivers/video/wm8505fb.c
> @@ -332,7 +332,7 @@ static int wm8505fb_probe(struct platform_device *pdev)
>  	if (!np) {
>  		pr_err("%s: No display description in Device Tree\n", __func__);
>  		ret = -EINVAL;
> -		goto failed_free_res;
> +		goto failed_free_io;
>  	}
>  
>  	/*
> @@ -351,7 +351,7 @@ static int wm8505fb_probe(struct platform_device *pdev)
>  	ret |= of_property_read_u32(np, "bpp", &bpp);
>  	if (ret) {
>  		pr_err("%s: Unable to read display properties\n", __func__);
> -		goto failed_free_res;
> +		goto failed_free_io;
>  	}
>  
>  	of_mode.vmode = FB_VMODE_NONINTERLACED;
> @@ -369,7 +369,8 @@ static int wm8505fb_probe(struct platform_device *pdev)
>  				GFP_KERNEL);
>  	if (!fb_mem_virt) {
>  		pr_err("%s: Failed to allocate framebuffer\n", __func__);
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto failed_free_io;
>  	};
>  
>  	fbi->fb.var.xres_virtual	= of_mode.xres;

NACK
Already have a patch queued up for this from Julia Lawall

Regards
Tony P


^ permalink raw reply

* Re: [PATCH 6/6] video: fb: vt8500: Convert framebuffer drivers to standardized binding
From: Tony Prisk @ 2013-03-28  5:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5152D39F.1090104@ti.com>

On Wed, 2013-03-27 at 13:10 +0200, Tomi Valkeinen wrote:
> Hi,
> 
> On 2013-03-27 10:47, Tony Prisk wrote:
> > Now that a display timing binding is available, convert our almost identical
> > binding to use the standard binding.
> > 
> > This patch converts the vt8500 and wm8505 framebuffer drivers and
> > associated dts/dtsi files to use the standard binding as defined in
> > bindings/video/display-timing.txt.
> > 
> > There are two side-effects of making this conversion:
> > 
> > 1) The fb node should now be in the board file, rather than the soc file as
> > the display-timing node is a child of the fb node.
> > 
> > 2) We still require a bits per pixel property to initialize the framebuffer
> > for the different lcd panels. Rather than including this as part of the
> > display timing, it is moved into the framebuffer node.
> > 
> > I have also taken the opportunity to alphabetise the includes of each
> > driver to avoid double-ups.
> 
> I don't think this is correct. I don't have that much experience with
> DT, but I think you should have, for example:
> 
> wm8850.dtsi:
> 
> 	fb: fb@d8051700 {
> 		compatible = "wm,wm8505-fb";
> 		reg = <0xd8051700 0x200>;
> 	};
> 
> wm8850-w70v2.dts:
> 
> &fb {
> 	bits-per-pixel = <16>;
> 
> 	display-timings {
> 		native-mode = <&timing0>;
> 		timing0: 800x480 {
> 			clock-frequency = <0>;
> 			...
> 		};
> 	};
> };
> 
> So, the core fb part should be in the SoC's file, as it's part of the
> SoC. And the stuff that tells what kind of display is attached is in the
> board dts file.
> 
> Also, just a word of warning, I think the videomode series I've sent for
> review will cause some breakage with this series if the videomode series
> is accepted. Nothing difficult to fix, though, but we'll need some extra
> management to avoid compilation failures.
> 
>  Tomi
> 
> 

Thanks for the feedback and the heads-up.

I believe you are correct about the DT info - it looks right when
described the way you did, so I have changed it.

If there is no other feedback, I will post a version 2 after Easter.

Regards
Tony P


^ permalink raw reply

* [PATCH] compat/compat-drivers/linux-next: fb skip_vt_switch
From: Luis R. Rodriguez @ 2013-03-28 12:04 UTC (permalink / raw)
  To: FlorianSchandinat
  Cc: linux-fbdev, dri-devel, jbarnes, backports, cocci, linux-kernel,
	julia.lawall, rodrigo.vivi, daniel.vetter, rafael.j.wysocki,
	Luis R. Rodriguez

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

I maintain the the compat-drivers project [0] which aims at backporting the
Linux kernel drivers down to older kernels, automatically [1]. Thanks to
Ozan Caglayan as a GSoC project we now backport DRM drivers.

The initial framework we had set up to help with the automatic juice was
simply quilt refresh to help with hunk offsets, later it consisted of
writing cleaner diffs, then compartamentalizing shared differences into a
compat backport module [2].

Recently I looked into Coccinelle SmPL to help push for collateral evolutions
on the Linux kernel to be written when possible with SmPL [3] given that, as
discussed with Julia, the very inverse of the grammar may allow us to backport
that collateral evolution on older kernels. I'm still experimenting with that,
but another benefit of studying INRIA's Coccinelle work is that the concept
of collateral evolutions is now formalized and as I've studied their work
I'm realizing we have different categories for collateral evolutions. From
what I've experienced through the backport work, data structure changes are
the more difficult collateral evolutions to backport. Instead of updates on
our compat module these require manual diffs... One strategy to simplify
backporting these data structure changes however was to replace the uses of
the new elements on the data structure with static inlines and requiring
the heavy changes to be implementd on a helper. That is, we actually simplfied
the backport by changing the form of the collateral evolution.

The new commit by Jesse that extended the fb_info with a skip_vt_switch
element is the simplest example of a data structure expansion. We'd backport
this by adding a static inline to compat so that new kernels muck with the
new element and for older kernels this would be a no-op. This reduces the
size of the backport and unclutters the required patch with #idefs, and
insteads leaves only a replacement of the usage of the new elements with
a static inline, this however would still be required on our end:

	-      info->skip_vt_switch = true;
	+      fb_enable_skip_vt_switch(info);

So we'd then have to just add this static inline change for each new driver...
There may be a way to get SmPL to do this for us...

However... if the static inline makes it upstream it means 0 changes are
required for *any* driver!

I've been reluctant to request a change upstream because of these side
backport benefits but due to this case's simplcity I figured I'd shoot
this out for review now.

A more elaborate example would be the netdev_attach_ops() which is not
yet upstream. This exands to this simple inline for new kernels:

static inline void netdev_attach_ops(struct net_device *dev,                    
                       const struct net_device_ops *ops)                        
{                                                                               
        dev->netdev_ops = ops;                                                  
} 

For older kernels this expands to:

void netdev_attach_ops(struct net_device *dev,                                  
                       const struct net_device_ops *ops)                        
{                                                                               
        dev->open = ops->ndo_open;                                              
        dev->init = ops->ndo_init;                                              
        dev->stop = ops->ndo_stop;                                              
        dev->hard_start_xmit = ops->ndo_start_xmit;                             
        dev->change_rx_flags = ops->ndo_change_rx_flags;                        
        dev->set_multicast_list = ops->ndo_set_multicast_list;                  
        dev->validate_addr = ops->ndo_validate_addr;                            
        dev->do_ioctl = ops->ndo_do_ioctl;                                      
        dev->set_config = ops->ndo_set_config;                                  
        dev->change_mtu = ops->ndo_change_mtu;                                  
        dev->set_mac_address = ops->ndo_set_mac_address;                        
        dev->tx_timeout = ops->ndo_tx_timeout;                                  
        if (ops->ndo_get_stats)                                                 
                dev->get_stats = ops->ndo_get_stats;                            
        dev->vlan_rx_register = ops->ndo_vlan_rx_register;                      
        dev->vlan_rx_add_vid = ops->ndo_vlan_rx_add_vid;                        
        dev->vlan_rx_kill_vid = ops->ndo_vlan_rx_kill_vid;                      
#ifdef CONFIG_NET_POLL_CONTROLLER                                               
        dev->poll_controller = ops->ndo_poll_controller;                        
#endif                                                                          
                                                                                
#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,27))                              
        dev->select_queue = ops->ndo_select_queue;                              
#endif                                                                          
}

Even though we have the static inline in compat it still means
every driver must be changed to use it. For example:

--- a/drivers/net/usb/rndis_host.c                                              
+++ b/drivers/net/usb/rndis_host.c                                              
@@ -358,7 +358,7 @@ generic_rndis_bind(struct usbnet *dev, s                    
        dev->rx_urb_size &= ~(dev->maxpacket - 1);                              
        u.init->max_transfer_size = cpu_to_le32(dev->rx_urb_size);              
                                                                                
-       net->netdev_ops = &rndis_netdev_ops;                                    
+       netdev_attach_ops(net, &rndis_netdev_ops);                              
                                                                                
        retval = rndis_command(dev, u.header, CONTROL_BUFFER_SIZE);             
        if (unlikely(retval < 0)) {   

This is just one driver. The patch that deals with this collateral
evolution is now 290 lines. I've been working with Julia to express
these type of tranformations as SmPL grammar, seeing if its possible
to use the inverse of SmPL in the long run, and so on... but in the end,
if we add a static inline upstream for small changes like these -- we'd
end up requring zero changes for the backport of these type of collateral
evolutions.

This should mean less bugs and cleaner backport code and maintenance.

Curious if video guys would care to accept this as an example simple
test case to help with the project with this respective small
collateral evolution and test case.

What follows are patches that deal with the changes on all the projects.
I'll apply the compat and compat-driver patches now as these are required,
but if my fb patch (patch #4 in this series) is accepted it'd simpify
backporting this collateral evolution for all video drivers tremendously.

[0] https://backports.wiki.kernel.org
[1] http://www.do-not-panic.com/2012/08/automatically-backporting-linux-kernel.html
[2] https://git.kernel.org/cgit/linux/kernel/git/mcgrof/compat.git/
[3] http://www.do-not-panic.com/2012/08/optimizing-backporting-collateral.html

linux-next:

Luis R. Rodriguez (1):
  fb: add helpers to enable and test for the skip_vt_switch

 drivers/gpu/drm/i915/intel_fb.c |    2 +-
 drivers/video/fbmem.c           |    2 +-
 include/linux/fb.h              |   10 ++++++++++
 3 files changed, 12 insertions(+), 2 deletions(-)

compat:

Luis R. Rodriguez (1):                                                          
  compat: backport fb_info->skip_vt_switch using a static inline                
                                                                                
 include/linux/compat-3.10.h |   41 +++++++++++++++++++++++++++++++++++++++++   
 1 file changed, 41 insertions(+)  

compat-drivers:

Luis R. Rodriguez (2):                                                          
  compat-drivers: backport fb_info->skip_vt_switch using ifdefs                 
  compat-drivers: simplify backport fb_info->skip_vt_switch CE                  
                                                                                
 .../drm/0001-fb-info-vt_switch.patch               |   56 ++++++++++++++++++++ 
 1 file changed, 56 insertions(+)                                               
 create mode 100644 patches/collateral-evolutions/drm/0001-fb-info-vt_switch.patch

-- 
1.7.10.4


^ permalink raw reply

* [PATCH 1/4] compat-drivers: backport fb_info->skip_vt_switch using ifdefs
From: Luis R. Rodriguez @ 2013-03-28 12:04 UTC (permalink / raw)
  To: FlorianSchandinat
  Cc: linux-fbdev, dri-devel, jbarnes, backports, cocci, linux-kernel,
	julia.lawall, rodrigo.vivi, daniel.vetter, rafael.j.wysocki,
	Luis R. Rodriguez
In-Reply-To: <1364472270-9297-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

Commit 3cf2667 as of next-20130301 extended the struct fb_info
with a skip_vt_switch to allow drivers to skip the VT switch
at suspend/resume time. For older kernels we can skip this
as all this switch does is call pm_vt_switch_required() with true
or false depending on this new flag and later
pm_vt_switch_unregister() would not have been made.

This patch cannot be broken down further so I'm pegging
this as the first one with 4 digits under the DRM folder
for collateral evolutions. This reflects its as atomic as
is possible. As we'll see on the next commit, these type
of collateral evolutions can best be backported not by
keeping ifdef's as below but instead by using a wrapper
caller, to help reduce with the amount of lines of code
we need. If a static inline is added upstream for these
changes, then no code is required for backporting, at all,
we'd just implement the static inline later upstream as
a no-op.

The tradeoffs to consider for this is if we can live with
these practices upstream, we may be able to support full
subsystems only with a compat module, and no need for
patches. This also means less code and likely less bugs
on the distribution front when backporting is required.
At least IMHO this may be worthy to consider at least to
support kernels listed as supported on kernel.org. We could
just leave the ifdef hell to older unsupported kernels.

Relevant commits below, starting with the first one that
added this new collateral evolution.

commit 3cf2667b9f8b2c2fe298a427deb399e52321da6b
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Mon Feb 4 13:37:21 2013 +0000

    fb: add support for drivers not needing VT switch at suspend/resume time

    Use the new PM routines to indicate whether we need to VT switch at suspend
    and resume time.  When a new driver is bound, set its flag accordingly,
    and when unbound, remove it from the PM's console tracking list.

    Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
    Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

commit 24576d23976746cb52e7700c4cadbf4bc1bc3472
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Tue Mar 26 09:25:45 2013 -0700

    drm/i915: enable VT switchless resume v3

    With the other bits in place, we can do this safely.

    v2: disable backlight on suspend to prevent premature enablement on resume
    v3: disable CRTCs on suspend to allow RTD3 (Kristen)

    Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
    Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cc: cocci@systeme.lip6.fr
Cc: backports@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 .../drm/0001-fb-info-vt_switch.patch               |   73 ++++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 patches/collateral-evolutions/drm/0001-fb-info-vt_switch.patch

diff --git a/patches/collateral-evolutions/drm/0001-fb-info-vt_switch.patch b/patches/collateral-evolutions/drm/0001-fb-info-vt_switch.patch
new file mode 100644
index 0000000..cdced87
--- /dev/null
+++ b/patches/collateral-evolutions/drm/0001-fb-info-vt_switch.patch
@@ -0,0 +1,73 @@
+Commit 3cf2667 as of next-20130301 extended the struct fb_info
+with a skip_vt_switch to allow drivers to skip the VT switch
+at suspend/resume time. For older kernels we can skip this
+as all this switch does is call pm_vt_switch_required() with true
+or false depending on this new flag and later
+pm_vt_switch_unregister() would not have been made.
+
+This patch cannot be broken down further so I'm pegging
+this as the first one with 4 digits under the DRM folder
+for collateral evolutions. This reflects its as atomic as
+is possible. As we'll see on the next commit, these type
+of collateral evolutions can best be backported not by
+keeping ifdef's as below but instead by using a wrapper
+caller, to help reduce with the amount of lines of code
+we need. If a static inline is added upstream for these
+changes, then no code is required for backporting, at all,
+we'd just implement the static inline later upstream as
+a no-op.
+
+The tradeoffs to consider for this is if we can live with
+these practices upstream, we may be able to support full
+subsystems only with a compat module, and no need for
+patches. This also means less code and likely less bugs
+on the distribution front when backporting is required.
+At least IMHO this may be worthy to consider at least to
+support kernels listed as supported on kernel.org. We could
+just leave the ifdef hell to older unsupported kernels.
+
+Relevant commits below, starting with the first one that
+added this new collateral evolution.
+
+commit 3cf2667b9f8b2c2fe298a427deb399e52321da6b
+Author: Jesse Barnes <jbarnes@virtuousgeek.org>
+Date:   Mon Feb 4 13:37:21 2013 +0000
+
+    fb: add support for drivers not needing VT switch at suspend/resume time
+    
+    Use the new PM routines to indicate whether we need to VT switch at suspend
+    and resume time.  When a new driver is bound, set its flag accordingly,
+    and when unbound, remove it from the PM's console tracking list.
+    
+    Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
+    Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
+
+commit 24576d23976746cb52e7700c4cadbf4bc1bc3472
+Author: Jesse Barnes <jbarnes@virtuousgeek.org>
+Date:   Tue Mar 26 09:25:45 2013 -0700
+
+    drm/i915: enable VT switchless resume v3
+    
+    With the other bits in place, we can do this safely.
+    
+    v2: disable backlight on suspend to prevent premature enablement on resume
+    v3: disable CRTCs on suspend to allow RTD3 (Kristen)
+    
+    Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
+    Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
+    Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
+
+--- a/drivers/gpu/drm/i915/intel_fb.c
++++ b/drivers/gpu/drm/i915/intel_fb.c
+@@ -150,8 +150,10 @@ static int intelfb_create(struct drm_fb_
+ 	}
+ 	info->screen_size = size;
+ 
++#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,10,0))
+ 	/* This driver doesn't need a VT switch to restore the mode on resume */
+ 	info->skip_vt_switch = true;
++#endif
+ 
+ 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
+ 	drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 2/4] compat: backport fb_info->skip_vt_switch using a static inline
From: Luis R. Rodriguez @ 2013-03-28 12:04 UTC (permalink / raw)
  To: FlorianSchandinat
  Cc: linux-fbdev, dri-devel, jbarnes, backports, cocci, linux-kernel,
	julia.lawall, rodrigo.vivi, daniel.vetter, rafael.j.wysocki,
	Luis R. Rodriguez
In-Reply-To: <1364472270-9297-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

Commit 3cf2667 as of next-20130301 extended the struct fb_info
with a skip_vt_switch to allow drivers to skip the VT switch
at suspend/resume time. For older kernels we can skip this
as all this switch does is call pm_vt_switch_required() with true
or false depending on this new flag and later
pm_vt_switch_unregister() would not have been made.

compat-drivers was backporting this using #ifdef's but by
integrating a static inline we'd reduce the number of lines
to backport to just 1 line replacement. This would be something
like:

  -       info->skip_vt_switch = true;
  +       fb_enable_skip_vt_switch(info);

For kernels >= 3.10 we'd set the attribute as we do upstream,
for older kernels this would be a no-op.

If this static inline would have been added upstream it would
have meant this collateral evolution would require just adding
a no-op static inline to backport, and no changes as the above
example hunk for every driver that requires the change.

If this static inline ends up upstream *now* it means we do *not*
require the type of hunk above for every driver that requires
the change.

All the code would be left intact !

This is a linux-next 'data structure element collateral evolution'.

Cc: cocci@systeme.lip6.fr
Cc: backports@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 include/linux/compat-3.10.h |   41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/include/linux/compat-3.10.h b/include/linux/compat-3.10.h
index 69ddc11..9abfc4b 100644
--- a/include/linux/compat-3.10.h
+++ b/include/linux/compat-3.10.h
@@ -7,6 +7,7 @@
 
 #include <linux/scatterlist.h>
 #include <linux/mm.h>
+#include <linux/fb.h>
 
 #define sg_page_iter_page LINUX_BACKPORT(sg_page_iter_page)
 /**
@@ -29,6 +30,46 @@ static inline dma_addr_t sg_page_iter_dma_address(struct sg_page_iter *piter)
 	return sg_dma_address(piter->sg) + (piter->sg_pgoffset << PAGE_SHIFT);
 }
 
+/*
+ * This is a linux-next data structure element collateral evolution,
+ * we use a wrapper to avoid #ifdef hell to backport it. This allows
+ * us to use a simple fb_info_skip_vt_switch() replacement for when
+ * the new data structure element is used. If coccinelle SmPL grammar
+ * could be used to express the transformation for us on compat-drivers
+ * it means we'd need to express it only once. If the structure element
+ * collateral evolution were to be used *at development* time and we'd
+ * have a way to express the inverse through SmPL we'd be able to
+ * backport this collateral evolution automatically for any new driver
+ * that used it. We'd use coccinelle to look for it and do the
+ * transformations for us based on the original commit (maybe SmPL
+ * would be listed on the commit log.
+ *
+ * We may need the LINUX_BACKPORT() call that adds the backport_
+ * prefix for older kernels than 3.10 if distros decide to
+ * add this same static inline themselves (although unlikely).
+ */
+#define fb_enable_skip_vt_switch LINUX_BACKPORT(fb_enable_skip_vt_switch)
+static inline void fb_enable_skip_vt_switch(struct fb_info *info)
+{
+}
+
+#else /* kernel is >= 3.10 */
+/*
+ * We'd delete this upstream ever got this, we use our
+ * backport_ prefix with LINUX_BACKPORT() so that if this
+ * does get upstream we would not have to add another ifdef
+ * here for the kernels in between v3.10.. up to the point
+ * the routine would have gotten added, we'd just delete this
+ * #else condition completely. If we didn't have this and
+ * say 3.12 added the static inline upstream, we'd have a
+ * clash on the backport for 3.12 as the routine would
+ * already be defined *but* we'd need it for 3.11.
+ */
+#define fb_enable_skip_vt_switch LINUX_BACKPORT(fb_enable_skip_vt_switch)
+static inline void fb_enable_skip_vt_switch(struct fb_info *info)
+{
+	info->skip_vt_switch = true;
+}
 #endif /* (LINUX_VERSION_CODE < KERNEL_VERSION(3,10,0)) */
 
 #endif /* LINUX_3_10_COMPAT_H */
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 3/4] compat-drivers: simplify backport fb_info->skip_vt_switch CE
From: Luis R. Rodriguez @ 2013-03-28 12:04 UTC (permalink / raw)
  To: FlorianSchandinat
  Cc: linux-fbdev, dri-devel, jbarnes, backports, cocci, linux-kernel,
	julia.lawall, rodrigo.vivi, daniel.vetter, rafael.j.wysocki,
	Luis R. Rodriguez
In-Reply-To: <1364472270-9297-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

The collateral evolution (CE) on the fb_info data structure
that added the skip_vt_switch element can be simplified
further by replacing the #ifdef hell with a static inline.

Furthermore, if the static inline is added upstream it'd mean
we can get rid of all these static inline replacements for
this data structure element CE.

Cc: cocci@systeme.lip6.fr
Cc: backports@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 .../drm/0001-fb-info-vt_switch.patch               |   25 ++++----------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/patches/collateral-evolutions/drm/0001-fb-info-vt_switch.patch b/patches/collateral-evolutions/drm/0001-fb-info-vt_switch.patch
index cdced87..39b13d1 100644
--- a/patches/collateral-evolutions/drm/0001-fb-info-vt_switch.patch
+++ b/patches/collateral-evolutions/drm/0001-fb-info-vt_switch.patch
@@ -8,23 +8,7 @@ pm_vt_switch_unregister() would not have been made.
 This patch cannot be broken down further so I'm pegging
 this as the first one with 4 digits under the DRM folder
 for collateral evolutions. This reflects its as atomic as
-is possible. As we'll see on the next commit, these type
-of collateral evolutions can best be backported not by
-keeping ifdef's as below but instead by using a wrapper
-caller, to help reduce with the amount of lines of code
-we need. If a static inline is added upstream for these
-changes, then no code is required for backporting, at all,
-we'd just implement the static inline later upstream as
-a no-op.
-
-The tradeoffs to consider for this is if we can live with
-these practices upstream, we may be able to support full
-subsystems only with a compat module, and no need for
-patches. This also means less code and likely less bugs
-on the distribution front when backporting is required.
-At least IMHO this may be worthy to consider at least to
-support kernels listed as supported on kernel.org. We could
-just leave the ifdef hell to older unsupported kernels.
+is possible.
 
 Relevant commits below, starting with the first one that
 added this new collateral evolution.
@@ -60,14 +44,13 @@ Date:   Tue Mar 26 09:25:45 2013 -0700
 
 --- a/drivers/gpu/drm/i915/intel_fb.c
 +++ b/drivers/gpu/drm/i915/intel_fb.c
-@@ -150,8 +150,10 @@ static int intelfb_create(struct drm_fb_
+@@ -150,8 +150,8 @@ static int intelfb_create(struct drm_fb_
  	}
  	info->screen_size = size;
  
-+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,10,0))
  	/* This driver doesn't need a VT switch to restore the mode on resume */
- 	info->skip_vt_switch = true;
-+#endif
+- 	info->skip_vt_switch = true;
++ 	fb_enable_skip_vt_switch(info);
  
  	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
  	drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 4/4] fb: add helpers to enable and test for the skip_vt_switch
From: Luis R. Rodriguez @ 2013-03-28 12:04 UTC (permalink / raw)
  To: FlorianSchandinat
  Cc: linux-fbdev, dri-devel, jbarnes, backports, cocci, linux-kernel,
	julia.lawall, rodrigo.vivi, daniel.vetter, rafael.j.wysocki,
	Luis R. Rodriguez
In-Reply-To: <1364472270-9297-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

This adds helpers to enable and test for the skip_vt_switch.
This gets us two things:

1) It allows us to require less collateral evolutions
   should we need changes on the fb_info data structure
   later (perhaps a bitmap flag).

2) Allows this feature to be backported with 0 delta
   required on the upstream drivers, we'd just require
   the static inline to do a no-op.

1) may be worth while considering, 2) is a new consideration
I'm trying to get others to evaluate to help with automatically
backporting the Linux kernel. This is the first time I am
aware someone argues for it.

Cc: cocci@systeme.lip6.fr
Cc: backports@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Cc: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---
 drivers/gpu/drm/i915/intel_fb.c |    2 +-
 drivers/video/fbmem.c           |    2 +-
 include/linux/fb.h              |   10 ++++++++++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 8d81c929..c0f306c 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -151,7 +151,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 	info->screen_size = size;
 
 	/* This driver doesn't need a VT switch to restore the mode on resume */
-	info->skip_vt_switch = true;
+	fb_enable_skip_vt_switch(info);
 
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
 	drm_fb_helper_fill_var(info, &ifbdev->helper, sizes->fb_width, sizes->fb_height);
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index ccd44b0..daffadc 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1645,7 +1645,7 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	if (!fb_info->modelist.prev || !fb_info->modelist.next)
 		INIT_LIST_HEAD(&fb_info->modelist);
 
-	if (fb_info->skip_vt_switch)
+	if (fb_skip_vt_switch_isset(fb_info))
 		pm_vt_switch_required(fb_info->dev, false);
 	else
 		pm_vt_switch_required(fb_info->dev, true);
diff --git a/include/linux/fb.h b/include/linux/fb.h
index d49c60f..d534966 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -505,6 +505,16 @@ struct fb_info {
 	bool skip_vt_switch; /* no VT switch on suspend/resume required */
 };
 
+static inline void fb_enable_skip_vt_switch(struct fb_info *info)
+{
+	info->skip_vt_switch = true;
+}
+
+static inline bool fb_skip_vt_switch_isset(struct fb_info *info)
+{
+	return info->skip_vt_switch;
+}
+
 static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
 	struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
 			+ max_num * sizeof(struct aperture), GFP_KERNEL);
-- 
1.7.10.4


^ permalink raw reply related

* [GIT PULL] fbdev fixes for 3.9
From: Tomi Valkeinen @ 2013-03-28 12:26 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Florian Tobias Schandinat, linux-fbdev, linux-omap, linux-kernel

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

Hi Linus,

Since Florian is still away/inactive, I volunteered to collect fbdev fixes for
3.9 and changes for 3.10. I didn't receive any other fbdev fixes than OMAP yet,
but I didn't want to delay this further as there's a compilation fix for OMAP1.
So there could be still some fbdev fixes on the way a bit later.

 Tomi

The following changes since commit a937536b868b8369b98967929045f1df54234323:

  Linux 3.9-rc3 (2013-03-17 15:59:32 -0700)

are available in the git repository at:

  git://gitorious.org/linux-omap-dss2/linux.git tags/fbdev-fixes-3.9-rc4

for you to fetch changes up to ff588d83bf12fe05521a64e85dd4e2b848c0b20d:

  omapdss: features: fix supported outputs for OMAP4 (2013-03-22 10:14:32 +0200)

----------------------------------------------------------------
* Fix OMAP1 compilation
* OMAP display fixes

----------------------------------------------------------------
Aaro Koskinen (1):
      omapfb: fix broken build on OMAP1

Archit Taneja (1):
      omapdss: features: fix supported outputs for OMAP4

Grazvydas Ignotas (1):
      OMAPDSS: tpo-td043 panel: fix data passing between SPI/DSS parts

 drivers/video/omap/omapfb_main.c                    |    2 ++
 drivers/video/omap2/displays/panel-tpo-td043mtea1.c |   13 +++++++++++--
 drivers/video/omap2/dss/dss_features.c              |    6 ++----
 3 files changed, 15 insertions(+), 6 deletions(-)


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

^ permalink raw reply

* Re: [PATCH] compat/compat-drivers/linux-next: fb skip_vt_switch
From: Jesse Barnes @ 2013-03-28 15:39 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: linux-fbdev, backports, FlorianSchandinat, daniel.vetter,
	rafael.j.wysocki, linux-kernel, dri-devel, rodrigo.vivi, cocci
In-Reply-To: <1364472270-9297-1-git-send-email-mcgrof@do-not-panic.com>

On Thu, 28 Mar 2013 05:04:26 -0700
"Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:

> The new commit by Jesse that extended the fb_info with a skip_vt_switch
> element is the simplest example of a data structure expansion. We'd backport
> this by adding a static inline to compat so that new kernels muck with the
> new element and for older kernels this would be a no-op. This reduces the
> size of the backport and unclutters the required patch with #idefs, and
> insteads leaves only a replacement of the usage of the new elements with
> a static inline, this however would still be required on our end:
> 
> 	-      info->skip_vt_switch = true;
> 	+      fb_enable_skip_vt_switch(info);
> 
> So we'd then have to just add this static inline change for each new driver...
> There may be a way to get SmPL to do this for us...

Yeah I'm not attached to the direct structure reference; a couple of
inlines are just as easy to read.  So no argument from me.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply

* Re: [PATCH] compat/compat-drivers/linux-next: fb skip_vt_switch
From: Julia Lawall @ 2013-03-28 16:19 UTC (permalink / raw)
  To: Jesse Barnes
  Cc: Luis R. Rodriguez, FlorianSchandinat, linux-fbdev, dri-devel,
	backports, cocci, linux-kernel, julia.lawall, rodrigo.vivi,
	daniel.vetter, rafael.j.wysocki
In-Reply-To: <20130328083943.01e61b4b@jbarnes-desktop>

On Thu, 28 Mar 2013, Jesse Barnes wrote:

> On Thu, 28 Mar 2013 05:04:26 -0700
> "Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:
>
> > The new commit by Jesse that extended the fb_info with a skip_vt_switch
> > element is the simplest example of a data structure expansion. We'd backport
> > this by adding a static inline to compat so that new kernels muck with the
> > new element and for older kernels this would be a no-op. This reduces the
> > size of the backport and unclutters the required patch with #idefs, and
> > insteads leaves only a replacement of the usage of the new elements with
> > a static inline, this however would still be required on our end:
> >
> > 	-      info->skip_vt_switch = true;
> > 	+      fb_enable_skip_vt_switch(info);
> >
> > So we'd then have to just add this static inline change for each new driver...
> > There may be a way to get SmPL to do this for us...

@@
type of info  *info;
@@

-      info->skip_vt_switch = true;
+      fb_enable_skip_vt_switch(info);

for whatever the type of info is.

Then I guess there would be a similar rule for the false case?

julia


>
> Yeah I'm not attached to the direct structure reference; a couple of
> inlines are just as easy to read.  So no argument from me.
>
> Thanks,
> --
> Jesse Barnes, Intel Open Source Technology Center
>

^ permalink raw reply

* Re: [PATCH] compat/compat-drivers/linux-next: fb skip_vt_switch
From: Luis R. Rodriguez @ 2013-03-28 18:05 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Jesse Barnes, FlorianSchandinat, linux-fbdev, dri-devel,
	backports@vger.kernel.org, cocci, linux-kernel@vger.kernel.org,
	rodrigo.vivi, Daniel Vetter, rafael.j.wysocki
In-Reply-To: <alpine.DEB.2.02.1303281718490.1928@hadrien>

On Thu, Mar 28, 2013 at 9:19 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Thu, 28 Mar 2013, Jesse Barnes wrote:
>> >     -      info->skip_vt_switch = true;
>> >     +      fb_enable_skip_vt_switch(info);
>> >
>> > So we'd then have to just add this static inline change for each new driver...
>> > There may be a way to get SmPL to do this for us...
>
> @@
> type of info  *info;
> @@
>
> -      info->skip_vt_switch = true;
> +      fb_enable_skip_vt_switch(info);
>
> for whatever the type of info is.

Thanks Julia! I'll be sure to try to add this to compat-drivers if the
upstream fb patch is not accepted. If it is accepted we would not need
this at all!

> Then I guess there would be a similar rule for the false case?

Nope, see that's the proactive strategy taken by the static inline and
hence the patch. compat would have a static inline for both cases, and
for the false case it'd be a no-op. If accepted upstream though then
we would not need any changes for this collateral evolution. However
*spotting* these collateral evolutions and giving you SmPL for them as
a proactive strategy might be good given that if these type of patches
are indeed welcomed upstream we'd then be able to address these as
secondary steps. If they are not accepted then indeed we'd use them to
backport that collateral evolution through both compat (adds the
static inlines) and compat-drivers (the SmPL).

  Luis

^ 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