Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH v5 11/12] video: da8xx-fb: setup struct lcd_ctrl_config for dt
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

strcut lcd_ctrl_config information required for driver is currently
obtained via platform data. To handle DT probing, create
lcd_ctrl_config and populate it with default values, these values are
sufficient for the panels so far used with this controller to work.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 1c1a616..5455682 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1254,6 +1254,35 @@ static struct fb_ops da8xx_fb_ops = {
 	.fb_blank = cfb_blank,
 };
 
+static struct lcd_ctrl_config *da8xx_fb_create_cfg(struct platform_device *dev)
+{
+	struct lcd_ctrl_config *cfg;
+
+	cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode), GFP_KERNEL);
+	if (!cfg) {
+		dev_err(&dev->dev, "memory allocation failed\n");
+		return NULL;
+	}
+
+	/* default values */
+
+	if (lcd_revision = LCD_VERSION_1)
+		cfg->bpp = 16;
+	else
+		cfg->bpp = 32;
+
+	/*
+	 * For panels so far used with this LCDC, below statement is sufficient.
+	 * For new panels, if required, struct lcd_ctrl_cfg fields to be updated
+	 * with additional/modified values. Those values would have to be then
+	 * obtained from dt(requiring new dt bindings).
+	 */
+
+	cfg->panel_shade = COLOR_ACTIVE;
+
+	return cfg;
+}
+
 static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
 {
 	struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
@@ -1345,7 +1374,10 @@ static int fb_probe(struct platform_device *device)
 		break;
 	}
 
-	lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
+	if (device->dev.of_node)
+		lcd_cfg = da8xx_fb_create_cfg(device);
+	else
+		lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
 
 	if (!lcd_cfg) {
 		ret = -EINVAL;
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 10/12] video: da8xx-fb: ensure pdata only for non-dt
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

This driver is DT probe-able, hence ensure presence of platform data
only for non-DT boot.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0c68712..1c1a616 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1303,7 +1303,7 @@ static int fb_probe(struct platform_device *device)
 	int ret;
 	unsigned long ulcm;
 
-	if (fb_pdata = NULL) {
+	if (fb_pdata = NULL && !device->dev.of_node) {
 		dev_err(&device->dev, "Can not get platform data\n");
 		return -ENOENT;
 	}
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 09/12] video: da8xx-fb: obtain fb_videomode info from dt
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Obtain fb_videomode details for the connected lcd panel using the
display timing details present in DT.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 .../devicetree/bindings/video/fb-da8xx.txt          | 21 +++++++++++++++++++++
 drivers/video/da8xx-fb.c                            | 17 +++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt
index 581e014..0741f78 100644
--- a/Documentation/devicetree/bindings/video/fb-da8xx.txt
+++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt
@@ -6,6 +6,12 @@ Required properties:
 	AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc"
 - reg: Address range of lcdc register set
 - interrupts: lcdc interrupt
+- display-timings: typical videomode of lcd panel, represented as child.
+  Refer Documentation/devicetree/bindings/video/display-timing.txt for
+  display timing binding details. If multiple videomodes are mentioned
+  in display timings node, typical videomode has to be mentioned as the
+  native mode or it has to be first child (driver cares only for native
+  videomode).
 
 Example:
 
@@ -13,4 +19,19 @@ lcdc@4830e000 {
 	compatible = "ti,am3352-lcdc", "ti,da830-lcdc";
 	reg =  <0x4830e000 0x1000>;
 	interrupts = <36>;
+	display-timings {
+		800x480p62 {
+			clock-frequency = <30000000>;
+			hactive = <800>;
+			vactive = <480>;
+			hfront-porch = <39>;
+			hback-porch = <39>;
+			hsync-len = <47>;
+			vback-porch = <29>;
+			vfront-porch = <13>;
+			vsync-len = <2>;
+			hsync-active = <1>;
+			vsync-active = <1>;
+		};
+	};
 };
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 0beed20..0c68712 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -36,6 +36,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/lcm.h>
+#include <video/of_display_timing.h>
 #include <video/da8xx-fb.h>
 #include <asm/div64.h>
 
@@ -1257,8 +1258,24 @@ static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
 {
 	struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
 	struct fb_videomode *lcdc_info;
+	struct device_node *np = dev->dev.of_node;
 	int i;
 
+	if (np) {
+		lcdc_info = devm_kzalloc(&dev->dev,
+					 sizeof(struct fb_videomode),
+					 GFP_KERNEL);
+		if (!lcdc_info) {
+			dev_err(&dev->dev, "memory allocation failed\n");
+			return NULL;
+		}
+		if (of_get_fb_videomode(np, lcdc_info, OF_USE_NATIVE_MODE)) {
+			dev_err(&dev->dev, "timings not available in DT\n");
+			return NULL;
+		}
+		return lcdc_info;
+	}
+
 	for (i = 0, lcdc_info = known_lcd_panels;
 		i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
 		if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 08/12] video: da8xx-fb: invoke platform callback safely
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Ensure that platform data is present before checking whether platform
callback is present (the one used to control backlight). So far this
was not an issue as driver was purely non-DT triggered, but now DT
support has been added.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 08ee8eb..0beed20 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1347,7 +1347,7 @@ static int fb_probe(struct platform_device *device)
 	par->dev = &device->dev;
 	par->lcdc_clk = fb_clk;
 	par->lcd_fck_rate = clk_get_rate(fb_clk);
-	if (fb_pdata->panel_power_ctrl) {
+	if (fb_pdata && fb_pdata->panel_power_ctrl) {
 		par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
 		par->panel_power_ctrl(1);
 	}
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 07/12] video: da8xx-fb: minimal dt support
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Driver is provided a means to have the probe triggered by DT.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 Documentation/devicetree/bindings/video/fb-da8xx.txt | 16 ++++++++++++++++
 drivers/video/da8xx-fb.c                             |  7 +++++++
 2 files changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt

diff --git a/Documentation/devicetree/bindings/video/fb-da8xx.txt b/Documentation/devicetree/bindings/video/fb-da8xx.txt
new file mode 100644
index 0000000..581e014
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/fb-da8xx.txt
@@ -0,0 +1,16 @@
+TI LCD Controller on DA830/DA850/AM335x SoC's
+
+Required properties:
+- compatible:
+	DA830 - "ti,da830-lcdc"
+	AM335x SoC's - "ti,am3352-lcdc", "ti,da830-lcdc"
+- reg: Address range of lcdc register set
+- interrupts: lcdc interrupt
+
+Example:
+
+lcdc@4830e000 {
+	compatible = "ti,am3352-lcdc", "ti,da830-lcdc";
+	reg =  <0x4830e000 0x1000>;
+	interrupts = <36>;
+};
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index b6ea5e9..08ee8eb 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1595,6 +1595,12 @@ static int fb_resume(struct platform_device *dev)
 #define fb_resume NULL
 #endif
 
+static const struct of_device_id da8xx_fb_of_match[] = {
+	{.compatible = "ti,da830-lcdc", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
+
 static struct platform_driver da8xx_fb_driver = {
 	.probe = fb_probe,
 	.remove = fb_remove,
@@ -1603,6 +1609,7 @@ static struct platform_driver da8xx_fb_driver = {
 	.driver = {
 		   .name = DRIVER_NAME,
 		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(da8xx_fb_of_match),
 		   },
 };
 
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 06/12] video: da8xx-fb: reorganize panel detection
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Mike Turquette, Florian Tobias Schandinat, Rob Herring,
	Tomi Valkeinen
In-Reply-To: <cover.1359356015.git.afzal-l0cyMroinI0@public.gmane.org>

Move panel detection to a separate function, this helps in readability
as well as makes DT support cleaner.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 3b146bc..b6ea5e9 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1253,6 +1253,27 @@ static struct fb_ops da8xx_fb_ops = {
 	.fb_blank = cfb_blank,
 };
 
+static struct fb_videomode *da8xx_fb_get_videomode(struct platform_device *dev)
+{
+	struct da8xx_lcdc_platform_data *fb_pdata = dev->dev.platform_data;
+	struct fb_videomode *lcdc_info;
+	int i;
+
+	for (i = 0, lcdc_info = known_lcd_panels;
+		i < ARRAY_SIZE(known_lcd_panels); i++, lcdc_info++) {
+		if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
+			break;
+	}
+
+	if (i = ARRAY_SIZE(known_lcd_panels)) {
+		dev_err(&dev->dev, "no panel found\n");
+		return NULL;
+	}
+	dev_info(&dev->dev, "found %s panel\n", lcdc_info->name);
+
+	return lcdc_info;
+}
+
 static int fb_probe(struct platform_device *device)
 {
 	struct da8xx_lcdc_platform_data *fb_pdata @@ -1262,7 +1283,7 @@ static int fb_probe(struct platform_device *device)
 	struct fb_info *da8xx_fb_info;
 	struct clk *fb_clk = NULL;
 	struct da8xx_fb_par *par;
-	int ret, i;
+	int ret;
 	unsigned long ulcm;
 
 	if (fb_pdata = NULL) {
@@ -1270,6 +1291,10 @@ static int fb_probe(struct platform_device *device)
 		return -ENOENT;
 	}
 
+	lcdc_info = da8xx_fb_get_videomode(device);
+	if (lcdc_info = NULL)
+		return -ENODEV;
+
 	lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
 	da8xx_fb_reg_base = devm_request_and_ioremap(&device->dev, lcdc_regs);
 	if (!da8xx_fb_reg_base) {
@@ -1303,21 +1328,6 @@ static int fb_probe(struct platform_device *device)
 		break;
 	}
 
-	for (i = 0, lcdc_info = known_lcd_panels;
-		i < ARRAY_SIZE(known_lcd_panels);
-		i++, lcdc_info++) {
-		if (strcmp(fb_pdata->type, lcdc_info->name) = 0)
-			break;
-	}
-
-	if (i = ARRAY_SIZE(known_lcd_panels)) {
-		dev_err(&device->dev, "GLCD: No valid panel found\n");
-		ret = -ENODEV;
-		goto err_pm_runtime_disable;
-	} else
-		dev_info(&device->dev, "GLCD: Found %s panel\n",
-					fb_pdata->type);
-
 	lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
 
 	if (!lcd_cfg) {
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 05/12] video: da8xx-fb: ensure non-null cfg in pdata
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Mike Turquette, Florian Tobias Schandinat, Rob Herring,
	Tomi Valkeinen
In-Reply-To: <cover.1359356015.git.afzal-l0cyMroinI0@public.gmane.org>

Ensure that platform data contains pointer for lcd_ctrl_config.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 7a32e83..3b146bc 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1320,6 +1320,11 @@ static int fb_probe(struct platform_device *device)
 
 	lcd_cfg = (struct lcd_ctrl_config *)fb_pdata->controller_data;
 
+	if (!lcd_cfg) {
+		ret = -EINVAL;
+		goto err_pm_runtime_disable;
+	}
+
 	da8xx_fb_info = framebuffer_alloc(sizeof(struct da8xx_fb_par),
 					&device->dev);
 	if (!da8xx_fb_info) {
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 04/12] video: da8xx-fb: use devres
From: Afzal Mohammed @ 2013-01-28  9:33 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Replace existing resource handling in the driver with managed device
resource.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index ca69e01..7a32e83 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -1036,12 +1036,9 @@ static int fb_remove(struct platform_device *dev)
 				  par->p_palette_base);
 		dma_free_coherent(NULL, par->vram_size, par->vram_virt,
 				  par->vram_phys);
-		free_irq(par->irq, par);
 		pm_runtime_put_sync(&dev->dev);
 		pm_runtime_disable(&dev->dev);
 		framebuffer_release(info);
-		iounmap(da8xx_fb_reg_base);
-		release_mem_region(lcdc_regs->start, resource_size(lcdc_regs));
 
 	}
 	return 0;
@@ -1265,7 +1262,6 @@ static int fb_probe(struct platform_device *device)
 	struct fb_info *da8xx_fb_info;
 	struct clk *fb_clk = NULL;
 	struct da8xx_fb_par *par;
-	resource_size_t len;
 	int ret, i;
 	unsigned long ulcm;
 
@@ -1275,29 +1271,16 @@ static int fb_probe(struct platform_device *device)
 	}
 
 	lcdc_regs = platform_get_resource(device, IORESOURCE_MEM, 0);
-	if (!lcdc_regs) {
-		dev_err(&device->dev,
-			"Can not get memory resource for LCD controller\n");
-		return -ENOENT;
-	}
-
-	len = resource_size(lcdc_regs);
-
-	lcdc_regs = request_mem_region(lcdc_regs->start, len, lcdc_regs->name);
-	if (!lcdc_regs)
-		return -EBUSY;
-
-	da8xx_fb_reg_base = ioremap(lcdc_regs->start, len);
+	da8xx_fb_reg_base = devm_request_and_ioremap(&device->dev, lcdc_regs);
 	if (!da8xx_fb_reg_base) {
-		ret = -EBUSY;
-		goto err_request_mem;
+		dev_err(&device->dev, "memory resource setup failed\n");
+		return -EADDRNOTAVAIL;
 	}
 
-	fb_clk = clk_get(&device->dev, "fck");
+	fb_clk = devm_clk_get(&device->dev, "fck");
 	if (IS_ERR(fb_clk)) {
 		dev_err(&device->dev, "Can not get device clock\n");
-		ret = -ENODEV;
-		goto err_ioremap;
+		return -ENODEV;
 	}
 
 	pm_runtime_enable(&device->dev);
@@ -1458,7 +1441,7 @@ static int fb_probe(struct platform_device *device)
 		lcdc_irq_handler = lcdc_irq_handler_rev02;
 	}
 
-	ret = request_irq(par->irq, lcdc_irq_handler, 0,
+	ret = devm_request_irq(&device->dev, par->irq, lcdc_irq_handler, 0,
 			DRIVER_NAME, par);
 	if (ret)
 		goto irq_freq;
@@ -1488,12 +1471,6 @@ err_pm_runtime_disable:
 	pm_runtime_put_sync(&device->dev);
 	pm_runtime_disable(&device->dev);
 
-err_ioremap:
-	iounmap(da8xx_fb_reg_base);
-
-err_request_mem:
-	release_mem_region(lcdc_regs->start, len);
-
 	return ret;
 }
 
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 03/12] video: da8xx-fb: enable sync lost intr for v2 ip
From: Afzal Mohammed @ 2013-01-28  9:32 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

interrupt handler is checking for sync lost interrupt, but it was not
enabled, enable it.

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 7f92f37..ca69e01 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -318,7 +318,7 @@ static void lcd_blit(int load_mode, struct da8xx_fb_par *par)
 			reg_int = lcdc_read(LCD_INT_ENABLE_SET_REG) |
 				LCD_V2_END_OF_FRAME0_INT_ENA |
 				LCD_V2_END_OF_FRAME1_INT_ENA |
-				LCD_FRAME_DONE;
+				LCD_FRAME_DONE | LCD_SYNC_LOST;
 			lcdc_write(reg_int, LCD_INT_ENABLE_SET_REG);
 		}
 		reg_dma |= LCD_DUAL_FRAME_BUFFER_ENABLE;
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 02/12] video: da8xx-fb: fix 24bpp raster configuration
From: Afzal Mohammed @ 2013-01-28  9:32 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette, Manjunathappa, Prakash
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

From: "Manjunathappa, Prakash" <prakash.pm@ti.com>

Set only LCD_V2_TFT_24BPP_MODE bit for 24bpp and LCD_V2_TFT_24BPP_UNPACK
bit along with LCD_V2_TFT_24BPP_MODE for 32bpp configuration.

Patch is tested on am335x-evm for 24bpp and da850-evm for 16bpp
configurations.

Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
Signed-off-by: Afzal Mohammed <afzal@ti.com>
---
 drivers/video/da8xx-fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 35a33ca..7f92f37 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -550,10 +550,10 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par, u32 width, u32 height,
 	case 4:
 	case 16:
 		break;
-	case 24:
-		reg |= LCD_V2_TFT_24BPP_MODE;
 	case 32:
 		reg |= LCD_V2_TFT_24BPP_UNPACK;
+	case 24:
+		reg |= LCD_V2_TFT_24BPP_MODE;
 		break;
 
 	case 8:
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 01/12] video: da8xx-fb: make io operations safe
From: Afzal Mohammed @ 2013-01-28  9:32 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette
In-Reply-To: <cover.1359356015.git.afzal@ti.com>

Replace __raw_readl/__raw_writel with readl/writel; this driver is
reused on ARMv7 (AM335x SoC).

Signed-off-by: Afzal Mohammed <afzal@ti.com>
---

v2: new patch

 drivers/video/da8xx-fb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 720604c..35a33ca 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -141,12 +141,12 @@ static int frame_done_flag;
 
 static inline unsigned int lcdc_read(unsigned int addr)
 {
-	return (unsigned int)__raw_readl(da8xx_fb_reg_base + (addr));
+	return (unsigned int)readl(da8xx_fb_reg_base + (addr));
 }
 
 static inline void lcdc_write(unsigned int val, unsigned int addr)
 {
-	__raw_writel(val, da8xx_fb_reg_base + (addr));
+	writel(val, da8xx_fb_reg_base + (addr));
 }
 
 struct da8xx_fb_par {
-- 
1.7.12


^ permalink raw reply related

* [PATCH v5 00/12] video: da8xx-fb: am335x DT support
From: Afzal Mohammed @ 2013-01-28  9:32 UTC (permalink / raw)
  To: linux-fbdev, linux-omap, devicetree-discuss, linux-doc,
	linux-kernel
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, Mike Turquette

Hi,

This series adds DT support to da8xx-fb driver (device found on
DaVinci and AM335x SoC's). It does certain cleanup's in the process.

This series as compared to previous version goes back to v2 way of
configuring pixel clock rate. i.e. set divider if rate is within
the range that is configurable with existing input clock rate, else
change input clock rate as required instead of modeling CCF clock
nodes in the driver (more details in 12/12)

This makes use of Steffen Trumtrar's v17 of display timing DT support.

Testing has been done on AM335x SoC based boards like AM335x EVM. It
has also been verified that display on DA850 EVM (non-DT boot) works
as earlier.

This series is based on v3.8-rc3,
 and is dependent on,
1. Series v17 "of: add display helper" by,
        Steffen Trumtrar <s.trumtrar@pengutronix.de>
2. Patch "da8xx: Allow use by am33xx based devices" by,
        Pantelis Antoniou <panto@antoniou-consulting.com>
3. Series v3 "video: da8xx-fb: runtime timing configuration" by,
        me (Afzal Mohammed <afzal@ti.com>)

To test this series on AM335x based boards,
1. Series "HWMOD fixes for AM33xx PWM submodules and device tree nodes" by,
        Philip, Avinash <avinashphilip@ti.com>
as well as following,
2. Series v2 "ARM: dts: AM33XX: lcdc support",
3. Patch v2 "ARM: OMAP2+: dpll: am335x - avoid freqsel",
4. Patch v2 "ARM: OMAP2+: clock: DEFINE_STRUCT_CLK_FLAGS helper",
5. Patch v2 "ARM: AM33XX: clock: SET_RATE_PARENT in lcd path" by,
	me (Afzal Mohammed <afzal@ti.com>)
would be needed.

All above dependencies along with those required for testing is available
@ git://gitorious.org/x0148406-public/linux-kernel.git tags/da8xx-fb-dt-v5

Regards
Afzal

v5: use v2 method of configuring pixel clock rate instead of modeling
    CCF clock nodes in driver, i.e. set divider if rate is within
    the range that is configurable with existing input clock rate,
    else change input clock rate as required.
v4: use new registration for clock divider having minimum divider
    requirement and have ifdef'ery in a better way
v3: model CCF clock divider with parent propogation if CCF selected
v2: 2 new patches - one to configure clock rate properly (12/12)and
    other to make io operations safe (1/12)



Afzal Mohammed (11):
  video: da8xx-fb: make io operations safe
  video: da8xx-fb: enable sync lost intr for v2 ip
  video: da8xx-fb: use devres
  video: da8xx-fb: ensure non-null cfg in pdata
  video: da8xx-fb: reorganize panel detection
  video: da8xx-fb: minimal dt support
  video: da8xx-fb: invoke platform callback safely
  video: da8xx-fb: obtain fb_videomode info from dt
  video: da8xx-fb: ensure pdata only for non-dt
  video: da8xx-fb: setup struct lcd_ctrl_config for dt
  video: da8xx-fb: set upstream clock rate (if reqd)

Manjunathappa, Prakash (1):
  video: da8xx-fb: fix 24bpp raster configuration

 .../devicetree/bindings/video/fb-da8xx.txt         |  37 ++++
 drivers/video/da8xx-fb.c                           | 226 ++++++++++++++-------
 2 files changed, 194 insertions(+), 69 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/fb-da8xx.txt

-- 
1.7.12


^ permalink raw reply

* RE: RE: RE: [PATCH v4 12/12] video: da8xx-fb: CCF clock divider handling
From: Mohammed, Afzal @ 2013-01-28  9:17 UTC (permalink / raw)
  To: Mike Turquette,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: Florian Tobias Schandinat, Rob Herring, Valkeinen, Tomi
In-Reply-To: <20130125224453.10623.74434@quantum>

SGkgTWlrZSwNCg0KT24gU2F0LCBKYW4gMjYsIDIwMTMgYXQgMDQ6MTQ6NTMsIE1pa2UgVHVycXVl
dHRlIHdyb3RlOg0KDQo+IEkgdGhpbmsgUGF1bCBXLiBvciBzb21lb25lIG9uIHRoZSBUSSBzaWRl
IHNob3VsZCB3ZWlnaCBpbiBvbiB5b3VyIGNsa2Rldg0KPiBlbnRyaWVzLiAgTXkgbWFpbiBwb2lu
dCBpcyB0aGF0IHRoZSBhY3R1YWwgdHJlZSBzaG91bGQgYmUgbW9kZWxlZCBhbmQNCj4gY2xvY2tz
IHNob3VsZG4ndCBiZSBnbG9iYmVkIHRvZ2V0aGVyIHVubmVjZXNzYXJpbHkuICBBcyBtZW50aW9u
ZWQgaW4gdGhlDQo+IG90aGVyIG1haWwgdGhyZWFkIHlvdSBtaWdodCBiZSBiZXR0ZXIgb2ZmIG1h
a2luZyBhIGRpdmlkZXIgZm9yIHlvdXIgTENEQw0KPiBJUCBibG9jayBhbmQgbW9kZWxpbmcgZWFj
aCBub2RlIGluZGl2aWR1YWxseS4NCg0KSXQgc2VlbXMgY29tcGxleGl0eSBvZiBkcml2ZXIgd291
bGQgaW5jcmVhc2UgYnkgY3JlYXRpbmcgYSBuZXcgaW5oZXJpdGVkDQpkaXZpZGVyIGNsb2NrIGFu
ZCBoYXZpbmcgYSB0b3RhbCAzLTQgY2xvY2sgbm9kZXMuIFRoZSBhZHZhbnRhZ2UgZ29pbmcNCndp
dGggaXQgd291bGQgYmUgaGlnaGVyIGNvbmZpZ3VyYWJsZSByZXNvbHV0aW9uIGZvciBwaXhlbCBj
bG9jay4NCkN1cnJlbnQgdXNlIGNhc2VzIHdvcmsgd2l0aG91dCBoaWdoZXIgcGl4ZWwgY2xvY2sg
cmVzb2x1dGlvbi4NCg0KQW5kIGRybSBkcml2ZXIgcG9zdGVkIGZvciB0aGUgc2FtZSBJUCBpcyB3
aXRob3V0IENDRiBtb2RlbGluZy4NCg0KU28gSSB3aWxsIHByZXNlbnRseSBub3QgbW9kZWwgY2xv
Y2sgbm9kZXMgaW4gTENEQyBJUCwgbGF0ZXIgaWYgdXNlIGNhc2VzDQpiYWRseSByZXF1aXJlLCB0
aGlzIGNhbiBiZSBkb25lIChhbmQgaWYgaXQgaGFwcGVucywgaG9wZWZ1bGx5IGJ5IHRoYXQNCkRh
VmluY2kgd291bGQgYmUgQ0NGJ2VkIGFuZCBpdCB3b3VsZCBiZSBtb3JlIGNsZWFuIHRvIGltcGxl
bWVudCBpdCkuDQoNClRoYW5rcyBmb3Igc2hhcmluZyB5b3VyIGlkZWFzLg0KDQpSZWdhcmRzDQpB
ZnphbA0K

^ permalink raw reply

* Re: [PATCH] backlight: add an AS3711 PMIC backlight driver
From: Jingoo Han @ 2013-01-28  0:53 UTC (permalink / raw)
  To: 'Guennadi Liakhovetski'
  Cc: 'Andrew Morton', linux-kernel, linux-fbdev, linux-sh,
	'Magnus Damm', 'Richard Purdie'
In-Reply-To: <Pine.LNX.4.64.1301250836280.17518@axis700.grange>

On Friday, January 25, 2013 4:49 PM, Guennadi Liakhovetski wrote
> 
> Hi Jingoo Han
> 
> On Fri, 25 Jan 2013, Jingoo Han wrote:
> 
> > On Wednesday, January 09, 2013 2:55 AM, Guennadi Liakhovetski wrote
> > >
> > > This is an initial commit of a backlight driver, using step-up DCDC power
> > > supplies on AS3711 PMIC. Only one mode has actually been tested, several
> > > further modes have been implemented "dry," but disabled to avoid accidental
> > > hardware damage. Anyone wishing to use any of those modes will have to
> > > modify the driver.
> > >
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> >
> > CC'ed Andrew Morton.
> >
> > Hi Guennadi Liakhovetski,
> >
> > I have reviewed this patch with AS3711 datasheet.
> > I cannot find any problems. It looks good.
> 
> Thanks for the review.
> 
> > However, some hardcoded numbers need to be changed
> > to the bit definitions.
> 
> Which specific hardcoded values do you mean? A while ago in a discussion
> on one of kernel mailing lists a conclusion has been made, that macros do
> not always improve code readability. E.g. in a statement like this
> 
> +	case AS3711_SU2_CURR_AUTO:
> +		if (pdata->su2_auto_curr1)
> +			ctl = 2;
> +		if (pdata->su2_auto_curr2)
> +			ctl |= 8;
> +		if (pdata->su2_auto_curr3)
> +			ctl |= 0x20;
> 
> making it
> 
> +	case AS3711_SU2_CURR_AUTO:
> +		if (pdata->su2_auto_curr1)
> +			ctl = SU2_AUTO_CURR1;
> 
> would hardly make it more readable. IMHO it is already pretty clear, that
> bit 1 enables the current-1 sink. As long as these fields are only used at
> one location in the driver (and they are), using a macro and defining it
> elsewhere only makes it harder to see actual values. Speaking of this, a
> comment like

I don't think so. Some people feel that it is not clear a bit.
Of course, I know what you mean.
Also, your comment is reasonable.
However, personally, I prefer the latter.
Because, I think that the meaning of bits is more important than
actual bits. In the latter case, we can notice the meaning of bits
more easily.


Best regards,
Jingoo Han

> 
> 		/*
> 		 * Select, which current sinks shall be used for automatic
> 		 * feedback selection
> 		 */
> 
> would help much more than any macro names. But as it stands, I think the
> current version is also possible to understand :-) If desired, however,
> comments can be added in an incremental patch

> 
> Thanks
> Guennadi
> 
> > Acked-by: Jingoo Han <jg1.han@samsung.com>
> >
> >
> > Best regards,
> > Jingoo Han
> >
> > > ---
> > >
> > > Tested on sh73a0-based kzm9g board. As the commit message says, only one
> > > mode has been tested and is enabled. That mode copies the sample code from
> > > the manufacturer. Deviations from that code proved to be fatal for the
> > > hardware...
> > >
> > >  drivers/video/backlight/Kconfig     |    7 +
> > >  drivers/video/backlight/Makefile    |    1 +
> > >  drivers/video/backlight/as3711_bl.c |  379 +++++++++++++++++++++++++++++++++++
> > >  3 files changed, 387 insertions(+), 0 deletions(-)
> > >  create mode 100644 drivers/video/backlight/as3711_bl.c
> > >
> > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> > > index 765a945..6ef0ede 100644
> > > --- a/drivers/video/backlight/Kconfig
> > > +++ b/drivers/video/backlight/Kconfig
> > > @@ -390,6 +390,13 @@ config BACKLIGHT_TPS65217
> > >  	  If you have a Texas Instruments TPS65217 say Y to enable the
> > >  	  backlight driver.
> > >
> > > +config BACKLIGHT_AS3711
> > > +	tristate "AS3711 Backlight"
> > > +	depends on BACKLIGHT_CLASS_DEVICE && MFD_AS3711
> > > +	help
> > > +	  If you have an Austrian Microsystems AS3711 say Y to enable the
> > > +	  backlight driver.
> > > +
> > >  endif # BACKLIGHT_CLASS_DEVICE
> > >
> > >  endif # BACKLIGHT_LCD_SUPPORT
> > > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> > > index e7ce729..d3e188f 100644
> > > --- a/drivers/video/backlight/Makefile
> > > +++ b/drivers/video/backlight/Makefile
> > > @@ -45,3 +45,4 @@ obj-$(CONFIG_BACKLIGHT_PCF50633)	+= pcf50633-backlight.o
> > >  obj-$(CONFIG_BACKLIGHT_AAT2870) += aat2870_bl.o
> > >  obj-$(CONFIG_BACKLIGHT_OT200) += ot200_bl.o
> > >  obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
> > > +obj-$(CONFIG_BACKLIGHT_AS3711) += as3711_bl.o
> > > diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
> > > new file mode 100644
> > > index 0000000..c6bc65d
> > > --- /dev/null
> > > +++ b/drivers/video/backlight/as3711_bl.c
> > > @@ -0,0 +1,379 @@
> > > +/*
> > > + * AS3711 PMIC backlight driver, using DCDC Step Up Converters
> > > + *
> > > + * Copyright (C) 2012 Renesas Electronics Corporation
> > > + * Author: Guennadi Liakhovetski, <g.liakhovetski@gmx.de>
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the version 2 of the GNU General Public License as
> > > + * published by the Free Software Foundation
> > > + */
> > > +
> > > +#include <linux/backlight.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/err.h>
> > > +#include <linux/fb.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/mfd/as3711.h>
> > > +#include <linux/module.h>
> > > +#include <linux/regmap.h>
> > > +#include <linux/slab.h>
> > > +
> > > +enum as3711_bl_type {
> > > +	AS3711_BL_SU1,
> > > +	AS3711_BL_SU2,
> > > +};
> > > +
> > > +struct as3711_bl_data {
> > > +	bool powered;
> > > +	const char *fb_name;
> > > +	struct device *fb_dev;
> > > +	enum as3711_bl_type type;
> > > +	int brightness;
> > > +	struct backlight_device *bl;
> > > +};
> > > +
> > > +struct as3711_bl_supply {
> > > +	struct as3711_bl_data su1;
> > > +	struct as3711_bl_data su2;
> > > +	const struct as3711_bl_pdata *pdata;
> > > +	struct as3711 *as3711;
> > > +};
> > > +
> > > +static struct as3711_bl_supply *to_supply(struct as3711_bl_data *su)
> > > +{
> > > +	switch (su->type) {
> > > +	case AS3711_BL_SU1:
> > > +		return container_of(su, struct as3711_bl_supply, su1);
> > > +	case AS3711_BL_SU2:
> > > +		return container_of(su, struct as3711_bl_supply, su2);
> > > +	}
> > > +	return NULL;
> > > +}
> > > +
> > > +static int as3711_set_brightness_auto_i(struct as3711_bl_data *data,
> > > +					unsigned int brightness)
> > > +{
> > > +	struct as3711_bl_supply *supply = to_supply(data);
> > > +	struct as3711 *as3711 = supply->as3711;
> > > +	const struct as3711_bl_pdata *pdata = supply->pdata;
> > > +	int ret = 0;
> > > +
> > > +	/* Only all equal current values are supported */
> > > +	if (pdata->su2_auto_curr1)
> > > +		ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> > > +				   brightness);
> > > +	if (!ret && pdata->su2_auto_curr2)
> > > +		ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> > > +				   brightness);
> > > +	if (!ret && pdata->su2_auto_curr3)
> > > +		ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> > > +				   brightness);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static int as3711_set_brightness_v(struct as3711 *as3711,
> > > +				   unsigned int brightness,
> > > +				   unsigned int reg)
> > > +{
> > > +	if (brightness > 31)
> > > +		return -EINVAL;
> > > +
> > > +	return regmap_update_bits(as3711->regmap, reg, 0xf0,
> > > +				  brightness << 4);
> > > +}
> > > +
> > > +static int as3711_bl_su2_reset(struct as3711_bl_supply *supply)
> > > +{
> > > +	struct as3711 *as3711 = supply->as3711;
> > > +	int ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_5,
> > > +				     3, supply->pdata->su2_fbprot);
> > > +	if (!ret)
> > > +		ret = regmap_update_bits(as3711->regmap,
> > > +					 AS3711_STEPUP_CONTROL_2, 1, 0);
> > > +	if (!ret)
> > > +		ret = regmap_update_bits(as3711->regmap,
> > > +					 AS3711_STEPUP_CONTROL_2, 1, 1);
> > > +	return ret;
> > > +}
> > > +
> > > +/*
> > > + * Someone with less fragile or less expensive hardware could try to simplify
> > > + * the brightness adjustment procedure.
> > > + */
> > > +static int as3711_bl_update_status(struct backlight_device *bl)
> > > +{
> > > +	struct as3711_bl_data *data = bl_get_data(bl);
> > > +	struct as3711_bl_supply *supply = to_supply(data);
> > > +	struct as3711 *as3711 = supply->as3711;
> > > +	int brightness = bl->props.brightness;
> > > +	int ret = 0;
> > > +
> > > +	dev_dbg(&bl->dev, "%s(): brightness %u, pwr %x, blank %x, state %x\n",
> > > +		__func__, bl->props.brightness, bl->props.power,
> > > +		bl->props.fb_blank, bl->props.state);
> > > +
> > > +	if (bl->props.power != FB_BLANK_UNBLANK ||
> > > +	    bl->props.fb_blank != FB_BLANK_UNBLANK ||
> > > +	    bl->props.state & (BL_CORE_SUSPENDED | BL_CORE_FBBLANK))
> > > +		brightness = 0;
> > > +
> > > +	if (data->type = AS3711_BL_SU1) {
> > > +		ret = as3711_set_brightness_v(as3711, brightness,
> > > +					      AS3711_STEPUP_CONTROL_1);
> > > +	} else {
> > > +		const struct as3711_bl_pdata *pdata = supply->pdata;
> > > +
> > > +		switch (pdata->su2_feedback) {
> > > +		case AS3711_SU2_VOLTAGE:
> > > +			ret = as3711_set_brightness_v(as3711, brightness,
> > > +						      AS3711_STEPUP_CONTROL_2);
> > > +			break;
> > > +		case AS3711_SU2_CURR_AUTO:
> > > +			ret = as3711_set_brightness_auto_i(data, brightness / 4);
> > > +			if (ret < 0)
> > > +				return ret;
> > > +			if (brightness) {
> > > +				ret = as3711_bl_su2_reset(supply);
> > > +				if (ret < 0)
> > > +					return ret;
> > > +				udelay(500);
> > > +				ret = as3711_set_brightness_auto_i(data, brightness);
> > > +			} else {
> > > +				ret = regmap_update_bits(as3711->regmap,
> > > +						AS3711_STEPUP_CONTROL_2, 1, 0);
> > > +			}
> > > +			break;
> > > +		/* Manual one current feedback pin below */
> > > +		case AS3711_SU2_CURR1:
> > > +			ret = regmap_write(as3711->regmap, AS3711_CURR1_VALUE,
> > > +					   brightness);
> > > +			break;
> > > +		case AS3711_SU2_CURR2:
> > > +			ret = regmap_write(as3711->regmap, AS3711_CURR2_VALUE,
> > > +					   brightness);
> > > +			break;
> > > +		case AS3711_SU2_CURR3:
> > > +			ret = regmap_write(as3711->regmap, AS3711_CURR3_VALUE,
> > > +					   brightness);
> > > +			break;
> > > +		default:
> > > +			ret = -EINVAL;
> > > +		}
> > > +	}
> > > +	if (!ret)
> > > +		data->brightness = brightness;
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static int as3711_bl_get_brightness(struct backlight_device *bl)
> > > +{
> > > +	struct as3711_bl_data *data = bl_get_data(bl);
> > > +
> > > +	return data->brightness;
> > > +}
> > > +
> > > +static const struct backlight_ops as3711_bl_ops = {
> > > +	.update_status	= as3711_bl_update_status,
> > > +	.get_brightness	= as3711_bl_get_brightness,
> > > +};
> > > +
> > > +static int as3711_bl_init_su2(struct as3711_bl_supply *supply)
> > > +{
> > > +	struct as3711 *as3711 = supply->as3711;
> > > +	const struct as3711_bl_pdata *pdata = supply->pdata;
> > > +	u8 ctl = 0;
> > > +	int ret;
> > > +
> > > +	dev_dbg(as3711->dev, "%s(): use %u\n", __func__, pdata->su2_feedback);
> > > +
> > > +	/* Turn SU2 off */
> > > +	ret = regmap_write(as3711->regmap, AS3711_STEPUP_CONTROL_2, 0);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	switch (pdata->su2_feedback) {
> > > +	case AS3711_SU2_VOLTAGE:
> > > +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 0);
> > > +		break;
> > > +	case AS3711_SU2_CURR1:
> > > +		ctl = 1;
> > > +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 1);
> > > +		break;
> > > +	case AS3711_SU2_CURR2:
> > > +		ctl = 4;
> > > +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 2);
> > > +		break;
> > > +	case AS3711_SU2_CURR3:
> > > +		ctl = 0x10;
> > > +		ret = regmap_update_bits(as3711->regmap, AS3711_STEPUP_CONTROL_4, 3, 3);
> > > +		break;
> > > +	case AS3711_SU2_CURR_AUTO:
> > > +		if (pdata->su2_auto_curr1)
> > > +			ctl = 2;
> > > +		if (pdata->su2_auto_curr2)
> > > +			ctl |= 8;
> > > +		if (pdata->su2_auto_curr3)
> > > +			ctl |= 0x20;
> > > +		ret = 0;
> > > +		break;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (!ret)
> > > +		ret = regmap_write(as3711->regmap, AS3711_CURR_CONTROL, ctl);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static int as3711_bl_register(struct platform_device *pdev,
> > > +			      unsigned int max_brightness, struct as3711_bl_data *su)
> > > +{
> > > +	struct backlight_properties props = {.type = BACKLIGHT_RAW,};
> > > +	struct backlight_device *bl;
> > > +
> > > +	/* max tuning I = 31uA for voltage- and 38250uA for current-feedback */
> > > +	props.max_brightness = max_brightness;
> > > +
> > > +	bl = backlight_device_register(su->type = AS3711_BL_SU1 ?
> > > +				       "as3711-su1" : "as3711-su2",
> > > +				       &pdev->dev, su,
> > > +				       &as3711_bl_ops, &props);
> > > +	if (IS_ERR(bl)) {
> > > +		dev_err(&pdev->dev, "failed to register backlight\n");
> > > +		return PTR_ERR(bl);
> > > +	}
> > > +
> > > +	bl->props.brightness = props.max_brightness;
> > > +
> > > +	backlight_update_status(bl);
> > > +
> > > +	su->bl = bl;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int as3711_backlight_probe(struct platform_device *pdev)
> > > +{
> > > +	struct as3711_bl_pdata *pdata = dev_get_platdata(&pdev->dev);
> > > +	struct as3711 *as3711 = dev_get_drvdata(pdev->dev.parent);
> > > +	struct as3711_bl_supply *supply;
> > > +	struct as3711_bl_data *su;
> > > +	unsigned int max_brightness;
> > > +	int ret;
> > > +
> > > +	if (!pdata || (!pdata->su1_fb && !pdata->su2_fb)) {
> > > +		dev_err(&pdev->dev, "No platform data, exiting...\n");
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	/*
> > > +	 * Due to possible hardware damage I chose to block all modes,
> > > +	 * unsupported on my hardware. Anyone, wishing to use any of those modes
> > > +	 * will have to first review the code, then activate and test it.
> > > +	 */
> > > +	if (pdata->su1_fb ||
> > > +	    pdata->su2_fbprot != AS3711_SU2_GPIO4 ||
> > > +	    pdata->su2_feedback != AS3711_SU2_CURR_AUTO) {
> > > +		dev_warn(&pdev->dev,
> > > +			 "Attention! An untested mode has been chosen!\n"
> > > +			 "Please, review the code, enable, test, and report success:-)\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	supply = devm_kzalloc(&pdev->dev, sizeof(*supply), GFP_KERNEL);
> > > +	if (!supply)
> > > +		return -ENOMEM;
> > > +
> > > +	supply->as3711 = as3711;
> > > +	supply->pdata = pdata;
> > > +
> > > +	if (pdata->su1_fb) {
> > > +		su = &supply->su1;
> > > +		su->fb_name = pdata->su1_fb;
> > > +		su->type = AS3711_BL_SU1;
> > > +
> > > +		max_brightness = min(pdata->su1_max_uA, 31);
> > > +		ret = as3711_bl_register(pdev, max_brightness, su);
> > > +		if (ret < 0)
> > > +			return ret;
> > > +	}
> > > +
> > > +	if (pdata->su2_fb) {
> > > +		su = &supply->su2;
> > > +		su->fb_name = pdata->su2_fb;
> > > +		su->type = AS3711_BL_SU2;
> > > +
> > > +		switch (pdata->su2_fbprot) {
> > > +		case AS3711_SU2_GPIO2:
> > > +		case AS3711_SU2_GPIO3:
> > > +		case AS3711_SU2_GPIO4:
> > > +		case AS3711_SU2_LX_SD4:
> > > +			break;
> > > +		default:
> > > +			ret = -EINVAL;
> > > +			goto esu2;
> > > +		}
> > > +
> > > +		switch (pdata->su2_feedback) {
> > > +		case AS3711_SU2_VOLTAGE:
> > > +			max_brightness = min(pdata->su2_max_uA, 31);
> > > +			break;
> > > +		case AS3711_SU2_CURR1:
> > > +		case AS3711_SU2_CURR2:
> > > +		case AS3711_SU2_CURR3:
> > > +		case AS3711_SU2_CURR_AUTO:
> > > +			max_brightness = min(pdata->su2_max_uA / 150, 255);
> > > +			break;
> > > +		default:
> > > +			ret = -EINVAL;
> > > +			goto esu2;
> > > +		}
> > > +
> > > +		ret = as3711_bl_init_su2(supply);
> > > +		if (ret < 0)
> > > +			return ret;
> > > +
> > > +		ret = as3711_bl_register(pdev, max_brightness, su);
> > > +		if (ret < 0)
> > > +			goto esu2;
> > > +	}
> > > +
> > > +	platform_set_drvdata(pdev, supply);
> > > +
> > > +	return 0;
> > > +
> > > +esu2:
> > > +	backlight_device_unregister(supply->su1.bl);
> > > +	return ret;
> > > +}
> > > +
> > > +static int as3711_backlight_remove(struct platform_device *pdev)
> > > +{
> > > +	struct as3711_bl_supply *supply = platform_get_drvdata(pdev);
> > > +
> > > +	backlight_device_unregister(supply->su1.bl);
> > > +	backlight_device_unregister(supply->su2.bl);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static struct platform_driver as3711_backlight_driver = {
> > > +	.driver		= {
> > > +		.name	= "as3711-backlight",
> > > +		.owner	= THIS_MODULE,
> > > +	},
> > > +	.probe		= as3711_backlight_probe,
> > > +	.remove		= as3711_backlight_remove,
> > > +};
> > > +
> > > +module_platform_driver(as3711_backlight_driver);
> > > +
> > > +MODULE_DESCRIPTION("Backlight Driver for AS3711 PMICs");
> > > +MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de");
> > > +MODULE_LICENSE("GPL");
> > > +MODULE_ALIAS("platform:as3711-backlight");
> > > --
> > > 1.7.2.5
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/


^ permalink raw reply

* Re: [git pull] fbcon locking fixes.
From: Daniel Vetter @ 2013-01-27 22:53 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Maarten Lankhorst, linux-fbdev, linux-kernel, DRI mailing list,
	torvalds
In-Reply-To: <20130124165055.0bceb033.akpm@linux-foundation.org>

On Fri, Jan 25, 2013 at 1:50 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Fri, 25 Jan 2013 00:42:37 +0000 (GMT)
> Dave Airlie <airlied@linux.ie> wrote:
>
>> These patches have been sailing around long enough, waiting for a maintainer
>> to reappear, so I've decided enough is enough, lockdep is kinda useful to have.
>>
>> Thanks to Daniel for annoying me enough :-)
>
> Me too, but the patches broke Maarten's EFI driver setup:
> https://lkml.org/lkml/2013/1/15/203.

Oops, didn't know that there are open issues, otherwise I'd have
worked on fixes instead of annoying you guys. To make due I've crawled
through fbcon code a bit to hunt for ways we could solve this mess
better than just flinging duct-tape. All the patches extend the
console_lock'ed area quite a bit, and I don't really like big kernel
looks with creeping coverage. More so if they come with funny
semantics like console_lock attached.

Looking at the fb notifier there are three use-cases for it:
- Special blank/unblank handling, used by the backlight and lcd
drivers in the fbdev subsystem. Could easily be extracted to another
notifier chain, and seems to have no need for the console lock. Also,
for kms drivers I don't care about this one bit.
- suspend/resume handling for the same fbdev lcd drivers. Same applies
(besides that it probably should get converted to proper device model
suspend/resume stuff).
- Runtime dependency injection between fbdev (and vga_switcheroor) for
fbcon. This allows you to load fbcon.ko and fbdev drives in any order,
and end up with an fbcon on each fbdev. Or not load fbcon.ko at all,
and only use the underlying fbdev. On a quick look it also racy as
hell.

The last one is the tricky one. If we could just add a hard
(compile-time) dependency between fbdevs and fbcon, we could rip out
all the fb notifier madness which also involves the console_lock in
some way and should be able to straighten out the locking. Users could
still opt out of using the fbcon at runtime, but ofc this might break
a really obscure setup somewhere. Quick survey of distros shows though
that all use the sane option of built-in fbcon support.

My proposal is to change the fbcon tristate to a bool and see what
happens. If it works out for a few kernel releases, garbage-collect
the fb_notifier for fbcon and use direct function calls instead
(stubbed it out properly if fbcon is disabeld ofc). Or is that too
much bending of the "thou shalt not break stuff" rule?

Plan B would be to simply mash-up a console on top of kms drivers
directly (since that's what I care about). Which has the downside that
we'd still need to keep the drm fbdev emulation helper code around,
since quite a few people use that to run older userspace (and even
report some bugs, e.g. mplayer on the linux console played some tricks
we needed to catch). And so end up with two pieces of code who's main
job is to paint console output onto the screen.

Ideas?

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* [PATCH]video:uvesafb: Fix dereference NULL pointer code path
From: Wang YanQing @ 2013-01-27  6:13 UTC (permalink / raw)
  To: FlorianSchandinat; +Cc: spock, linux-fbdev, linux-kernel

platform_device_alloc could failed and return NULL,
we should check this before call platform_device_put.

Signed-off-by: Wang YanQing <udknight@gmail.com>
---
 drivers/video/uvesafb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index 2f8f82d..230bd45 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -1975,7 +1975,8 @@ static int __devinit uvesafb_init(void)
 			err = -ENOMEM;
 
 		if (err) {
-			platform_device_put(uvesafb_device);
+			if (uvesafb_device)
+				platform_device_put(uvesafb_device);
 			platform_driver_unregister(&uvesafb_driver);
 			cn_del_callback(&uvesafb_cn_id);
 			return err;
-- 
1.7.11.1.116.g8228a23

^ permalink raw reply related

* Re: [PATCH 0/3] atmel_lcdfb: fix 16-bpp regression
From: Johan Hovold @ 2013-01-26 15:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1355142530-10366-1-git-send-email-jhovold@gmail.com>

On Mon, Dec 10, 2012 at 01:28:47PM +0100, Johan Hovold wrote:
> These patches fix a regression in 16-bpp support for older SOCs which use
> IBGR:555 rather than BGR:565 pixel layout. Use SOC-type to determine if the
> controller uses the intensity-bit and restore the old layout in that case.

I understand the maintainer's been busy lately, but could someone pick
these regression fixes up so we can get them into v3.8-rc and the stable
trees?

Thanks,
Johan

^ permalink raw reply

* Re: [PATCH] goldfish: framebuffer
From: Alan Cox @ 2013-01-26  0:27 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20130125153100.7317.23971.stgit@bob.linux.org.uk>

> hm, it's unclear whether the [] comments apply to the line above or to
> the line below.  I converted it to my usual form.  I hope I got it
> right!

Below so you got them all one out 8)

Alan

^ permalink raw reply

* Re: [PATCH] goldfish: framebuffer
From: Andrew Morton @ 2013-01-26  0:12 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20130125153100.7317.23971.stgit@bob.linux.org.uk>

On Sat, 26 Jan 2013 00:27:45 +0000
Alan Cox <alan@linux.intel.com> wrote:

> > hm, it's unclear whether the [] comments apply to the line above or to
> > the line below.  I converted it to my usual form.  I hope I got it
> > right!
> 
> Below so you got them all one out 8)
> 

o dwat.

[sheng@linux.intel.com: cleaned up to handle x86]
[thomas.keel@intel.com: ported to 3.4]
[alan@linux.intel.com: cleaned up for style and 3.7, moved blank methods]
Signed-off-by: Mike A. Chan <mikechan@google.com>
Signed-off-by: Arve Hj_nnev_g <arve@android.com>
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com>
Signed-off-by: Xiaohui Xin <xiaohui.xin@intel.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Bruce Beare <bruce.j.beare@intel.com>
Signed-off-by: Tom Keel <thomas.keel@intel.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>

^ permalink raw reply

* Re: RE: RE: [PATCH v4 12/12] video: da8xx-fb: CCF clock divider handling
From: Mike Turquette @ 2013-01-25 22:44 UTC (permalink / raw)
  To: Mohammed, Afzal, linux-fbdev@vger.kernel.org,
	linux-omap@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: Florian Tobias Schandinat, Tomi Valkeinen, Grant Likely,
	Rob Herring, Rob Landley, paul, rnayak, b-cousson
In-Reply-To: <C8443D0743D26F4388EA172BF4E2A7A93EA9383E@DBDE01.ent.ti.com>

Quoting Mohammed, Afzal (2013-01-25 04:05:44)
> Hi Mike,
> 
> On Thu, Jan 24, 2013 at 22:30:44, Mike Turquette wrote:
> > Quoting Mohammed, Afzal (2013-01-24 03:36:02)
> 
> > > So there are 3 - LIDD is actually not for present use case, CORE could
> > > be clubbed with the divider to have a composite clock. And CORE is
> > > in functional clock path and logically it's perfectly alright to have
> > > the composite clock.
> 
> > Some of the clock names are a bit generic, so a question that I'm going
> > to repeat throughout my response: "is this clock only inside of your
> > video IP ?"
> 
> Yes these three clocks are inside LCDC IP.
> 
> > Regarding the CORE clock, is this only inside of your IP or are you
> > referring to the SoC CORE clock which is driven by a DPLL and clocks
> > DDR and many other peripherals (often MMC, UART, etc)?
> 
> Sorry for the confusion, here CORE refers to clock inside LCDC IP. This
> CORE should not be confused with CORE PLL. Actually I used CORE so that
> it corresponds to the nomenclature in LCDC section of TRM.
> 
> > Note that this is from my past experience with OMAP, and I'm making an
> > assumption that the clock scheme between OMAP and Da Vinci/AM335x parts
> > isn't very different.
> 
> Additional detail: DaVinci doesn't have these 3 clocks controls available,
> so these three are required only on AM335x (which has IP version 2 )
> 
> > Is there a public TRM I can look at?  It would help me understand this
> > without having to ask you so many annoying questions ;)
> 
> No problem, http://www.ti.com/product/am3359
> 
> 
> > > And now we are left with DMA, this is actually in the interface clock
> > > path which driver in unaware. An option would be to have DMA clock
> > > as child of CORE plus divider composite clock, even though logically
> > > DMA can't be considered in the same path.
> 
> > Why is the driver unaware of the interface clk?  For instance OMAP3 had
> > separate fclk and iclk for IPs and drivers would call clk_enable on
> > both.  Or am I misunderstanding something?
> 
> HWMOD handles enabling those upon pm_runtime calls, HWMOD makes an alias
> for main clock with "fck", but not for "ick", so currently "ick" is
> unavailable for the driver, continued below ..
> 
> > In general I don't think the clock subtree should be modeled in a way
> > that is convenient for software, but instead model the actual hardware.
> > Trust me, if you don't model the actual hardware then you will be very
> > confused when you come back and revisit this code in 6 months and can't
> > remember why things are so weird looking.
> 
> Ok, then it seems an omap clock entry for con-id "ick" should be created
> as follows (dpll_core_m4_ck supplies interface clock),
> 
> CLK("4830e000.lcdc",    "ick",          &dpll_core_m4_ck,       CK_AM33XX)
> 
> And then in the driver, DMA gate clock should be made a child of this clock
> (obtained with con-id "ick").
> 
> Let me know your opinion on this.
> 

I think Paul W. or someone on the TI side should weigh in on your clkdev
entries.  My main point is that the actual tree should be modeled and
clocks shouldn't be globbed together unnecessarily.  As mentioned in the
other mail thread you might be better off making a divider for your LCDC
IP block and modeling each node individually.

Regards,
Mike

> Regards
> Afzal

^ permalink raw reply

* Re: [PATCH] goldfish: framebuffer
From: Andrew Morton @ 2013-01-25 22:44 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20130125153100.7317.23971.stgit@bob.linux.org.uk>

On Fri, 25 Jan 2013 15:31:17 +0000
Alan Cox <alan@linux.intel.com> wrote:

> Framebuffer support for the Goldfish emulator. This takes the Google emulator
> and applies the x86 cleanups as well as moving the blank methods to the usual
> Linux place and dropping the Android early suspend logic (for now at least, that
> can be looked at as Android and upstream converge). Dropped various oddities
> like setting MTRRs on a virtual frame buffer emulation...
> 
> With the drivers so far you can now boot a Linux initrd and have fun.

That looks nice and clean.  I'll grab it; hopefully for Florian to take
off my hands when he returns.


> Signed-off-by: Mike A. Chan <mikechan@google.com>
> Signed-off-by: Arve Hj__nnev__g <arve@android.com>
> [cleaned up to handle x86]
> Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com>
> Signed-off-by: Xiaohui Xin <xiaohui.xin@intel.com>
> Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
> Signed-off-by: Bruce Beare <bruce.j.beare@intel.com>
> [ported to 3.4]
> Signed-off-by: Tom Keel <thomas.keel@intel.com>
> [cleaned up for style and 3.7, moved blank methods]
> Signed-off-by: Alan Cox <alan@linux.intel.com>

hm, it's unclear whether the [] comments apply to the line above or to
the line below.  I converted it to my usual form.  I hope I got it right!

[arve@android.com: cleaned up to handle x86]
[bruce.j.beare@intel.com: ported to 3.4]
[thomas.keel@intel.com: cleaned up for style and 3.7, moved blank methods]
Signed-off-by: Mike A. Chan <mikechan@google.com>
Signed-off-by: Arve Hj_nnev_g <arve@android.com>
Signed-off-by: Sheng Yang <sheng@linux.intel.com>
Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com>
Signed-off-by: Xiaohui Xin <xiaohui.xin@intel.com>
Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
Signed-off-by: Bruce Beare <bruce.j.beare@intel.com>
Signed-off-by: Tom Keel <thomas.keel@intel.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>



^ permalink raw reply

* [PATCH 3/3] fb: smscufx: fix hang at disconnect
From: Alexander Holler @ 2013-01-25 18:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fbdev, Florian Tobias Schandinat, Bernie Thompson,
	Steve Glendinning, Alexander Holler, stable
In-Reply-To: <1359139768-32294-1-git-send-email-holler@ahsoftware.de>

When a device was disconnected the driver may hang at waiting for urbs it never
will get. Fix this by using a timeout while waiting for the used semaphore.

There is still a memory leak if a timeout happens, but at least the driver
now continues his disconnect routine.

Cc: <stable@vger.kernel.org>
Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 drivers/video/smscufx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/smscufx.c b/drivers/video/smscufx.c
index 97bd662..f26fa4f 100644
--- a/drivers/video/smscufx.c
+++ b/drivers/video/smscufx.c
@@ -1838,8 +1838,9 @@ static void ufx_free_urb_list(struct ufx_data *dev)
 
 	/* keep waiting and freeing, until we've got 'em all */
 	while (count--) {
-		/* Getting interrupted means a leak, but ok at shutdown*/
-		ret = down_interruptible(&dev->urbs.limit_sem);
+		/* Timeout likely occurs at disconnect (resulting in a leak) */
+		ret = down_timeout_killable(&dev->urbs.limit_sem,
+						FREE_URB_TIMEOUT);
 		if (ret)
 			break;
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH 2/3 v2] fb: udlfb: fix hang at disconnect
From: Alexander Holler @ 2013-01-25 18:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fbdev, Florian Tobias Schandinat, Bernie Thompson,
	Steve Glendinning, Alexander Holler, stable
In-Reply-To: <1359139768-32294-1-git-send-email-holler@ahsoftware.de>

When a device was disconnected the driver may hang at waiting for urbs it never
will get. Fix this by using a timeout while waiting for the used semaphore.

There is still a memory leak if a timeout happens, but at least the driver
now continues his disconnect routine.

Cc: <stable@vger.kernel.org>
Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 drivers/video/udlfb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/udlfb.c b/drivers/video/udlfb.c
index 86d449e..db6cc66 100644
--- a/drivers/video/udlfb.c
+++ b/drivers/video/udlfb.c
@@ -1832,8 +1832,9 @@ static void dlfb_free_urb_list(struct dlfb_data *dev)
 	/* keep waiting and freeing, until we've got 'em all */
 	while (count--) {
 
-		/* Getting interrupted means a leak, but ok at disconnect */
-		ret = down_interruptible(&dev->urbs.limit_sem);
+		/* Timeout likely occurs at disconnect (resulting in a leak) */
+		ret = down_timeout_killable(&dev->urbs.limit_sem,
+						FREE_URB_TIMEOUT);
 		if (ret)
 			break;
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH 1/3] semaphore: introduce down_timeout_killable()
From: Alexander Holler @ 2013-01-25 18:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fbdev, Florian Tobias Schandinat, Bernie Thompson,
	Steve Glendinning, Alexander Holler
In-Reply-To: <50F2A310.5010006@ahsoftware.de>

The name says it all, it's like down_timeout() but returns on fatal signals
too.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 include/linux/semaphore.h |  2 ++
 kernel/semaphore.c        | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
index dc368b8..b508d4d 100644
--- a/include/linux/semaphore.h
+++ b/include/linux/semaphore.h
@@ -41,6 +41,8 @@ extern int __must_check down_interruptible(struct semaphore *sem);
 extern int __must_check down_killable(struct semaphore *sem);
 extern int __must_check down_trylock(struct semaphore *sem);
 extern int __must_check down_timeout(struct semaphore *sem, long jiffies);
+extern int __must_check down_timeout_killable(struct semaphore *sem,
+						long jiffies);
 extern void up(struct semaphore *sem);
 
 #endif /* __LINUX_SEMAPHORE_H */
diff --git a/kernel/semaphore.c b/kernel/semaphore.c
index 4567fc0..4a5e823 100644
--- a/kernel/semaphore.c
+++ b/kernel/semaphore.c
@@ -37,6 +37,8 @@ static noinline void __down(struct semaphore *sem);
 static noinline int __down_interruptible(struct semaphore *sem);
 static noinline int __down_killable(struct semaphore *sem);
 static noinline int __down_timeout(struct semaphore *sem, long jiffies);
+static noinline int __down_timeout_killable(struct semaphore *sem,
+						long jiffies);
 static noinline void __up(struct semaphore *sem);
 
 /**
@@ -169,6 +171,35 @@ int down_timeout(struct semaphore *sem, long jiffies)
 EXPORT_SYMBOL(down_timeout);
 
 /**
+ * down_timeout_killable - acquire the semaphore within a specified time or
+ * until a fatal signal arrives.
+ * @sem: the semaphore to be acquired
+ * @jiffies: how long to wait before failing
+ *
+ * Attempts to acquire the semaphore.  If no more tasks are allowed to
+ * acquire the semaphore, calling this function will put the task to sleep.
+ * If the semaphore is not released within the specified number of jiffies,
+ * this function returns -ETIME. If the sleep is interrupted by a fatal signal,
+ * this function will return -EINTR. If the semaphore is successfully acquired,
+ * this function returns 0.
+ */
+int down_timeout_killable(struct semaphore *sem, long jiffies)
+{
+	unsigned long flags;
+	int result = 0;
+
+	raw_spin_lock_irqsave(&sem->lock, flags);
+	if (likely(sem->count > 0))
+		sem->count--;
+	else
+		result = __down_timeout_killable(sem, jiffies);
+	raw_spin_unlock_irqrestore(&sem->lock, flags);
+
+	return result;
+}
+EXPORT_SYMBOL(down_timeout_killable);
+
+/**
  * up - release the semaphore
  * @sem: the semaphore to release
  *
@@ -253,6 +284,12 @@ static noinline int __sched __down_timeout(struct semaphore *sem, long jiffies)
 	return __down_common(sem, TASK_UNINTERRUPTIBLE, jiffies);
 }
 
+static noinline int __sched __down_timeout_killable(struct semaphore *sem,
+							long jiffies)
+{
+	return __down_common(sem, TASK_KILLABLE, jiffies);
+}
+
 static noinline void __sched __up(struct semaphore *sem)
 {
 	struct semaphore_waiter *waiter = list_first_entry(&sem->wait_list,
-- 
1.7.11.7


^ permalink raw reply related

* Re: [PATCH] fbcon: fix locking harder
From: Daniel Vetter @ 2013-01-25 15:55 UTC (permalink / raw)
  To: Dave Airlie; +Cc: linux-fbdev, linux-kernel, dri-devel
In-Reply-To: <1359078224-3327-1-git-send-email-airlied@gmail.com>

On Fri, Jan 25, 2013 at 2:43 AM, Dave Airlie <airlied@gmail.com> wrote:
> Okay so Alan's patch handled the case where there was no registered fbcon,
> however the other path entered in set_con2fb_map pit.
>
> In there we called fbcon_takeover, but we also took the console lock in a couple
> of places. So push the console lock out to the callers of set_con2fb_map,
>
> this means fbmem and switcheroo needed to take the lock around the fb notifier
> entry points that lead to this.
>
> This should fix the efifb regression seen by Maarten.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/vga/vga_switcheroo.c |  3 +++
>  drivers/video/console/fbcon.c    | 11 ++++++++---
>  drivers/video/fbmem.c            |  2 ++
>  3 files changed, 13 insertions(+), 3 deletions(-)
>
[cut]

>         ret = vgasr_priv.handler->switchto(new_client->id);
> diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> index 2aef9ca..2e2d959 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -842,6 +842,8 @@ static void con2fb_init_display(struct vc_data *vc, struct fb_info *info,
>   *
>   *     Maps a virtual console @unit to a frame buffer device
>   *     @newidx.
> + *
> + *     This should be called with the console lock held.
>   */
>  static int set_con2fb_map(int unit, int newidx, int user)
>  {

What about throwing a WARN_CONSOLE_UNLOCKED(); in here to make sure
this new rule is obeyed?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ 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