framebuffer work (especially i.MX)

PATCH RESEND In review Daniel Mierswa 4 participants 10 messages Jan 17, 2013 – Jan 18, 2013

Subsystems (4)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape Framebuffer layer

lore

Share & tools

lore

Daniel Mierswa

author

2013-01-17 06:33 UTC #0 lore raw

PATCH RESEND 1/1 framebuffer work (especially i.MX)

I adjusted the patch series and added the fixes as discussed on the mailinglist.

I still somehow feel that the offscreenbuf member should be set by the fd_open routine because I expect fd_close to always work with the struct set by fd_open no matter what offscreen parameter the function got.

Daniel Mierswa (4):
i.MX21: Add periph. clock register name macros i.MX21/27: don't enable lcd bus clocks too early i.MX27: fix shift amount for PCCR1_PERCLK3_EN video/imx: initialize offscreenbuf member

 arch/arm/mach-imx/clk-imx21.c               | 67 ++++++++++++++++++++++++++---
 arch/arm/mach-imx/clk-imx27.c               | 16 ++++---
 arch/arm/mach-imx/include/mach/imx21-regs.h |  6 ---
 commands/splash.c                           |  8 ++++
 drivers/video/imx.c                         | 40 ++++++++++++++---
 5 files changed, 111 insertions(+), 26 deletions(-)

Signature · 7 lines

1.8.1


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Daniel Mierswa

author

2013-01-17 06:33 UTC #1 lore raw

PATCH 1/4 i.MX21: Add periph. clock register name macros

Also put those names solely in the .c file as it's done with the i.MX27 code.

Signed-off-by Daniel Mierswa <[email protected]>

---
 arch/arm/mach-imx/clk-imx21.c               | 56 +++++++++++++++++++++++++----
 arch/arm/mach-imx/include/mach/imx21-regs.h |  6 ----
 2 files changed, 50 insertions(+), 12 deletions(-)

arch/arm/mach-imx/clk-imx21.c +50 -6

index 9e7af81..784951d 100644
@@ -45,6 +45,48 @@
 #define CCM_PMCOUNT		0x30
 #define CCM_WKGDCTL		0x34
 
+#define PCCR0_UART1_EN		(1 << 0)
+#define PCCR0_UART2_EN		(1 << 1)
+#define PCCR0_UART3_EN		(1 << 2)
+#define PCCR0_UART4_EN		(1 << 3)
+#define PCCR0_CSPI1_EN		(1 << 4)
+#define PCCR0_CSPI2_EN		(1 << 5)
+#define PCCR0_SSI1_EN		(1 << 6)
+#define PCCR0_SSI2_EN		(1 << 7)
+#define PCCR0_FIRI_EN		(1 << 8)
+#define PCCR0_SDHC1_EN		(1 << 9)
+#define PCCR0_SDHC2_EN		(1 << 10)
+#define PCCR0_GPIO_EN		(1 << 11)
+#define PCCR0_I2C_EN		(1 << 12)
+#define PCCR0_DMA_EN		(1 << 13)
+#define PCCR0_USBOTG_EN		(1 << 14)
+#define PCCR0_EMMA_EN		(1 << 15)
+#define PCCR0_SSI2_BAUD_EN	(1 << 16)
+#define PCCR0_SSI1_BAUD_EN	(1 << 17)
+#define PCCR0_PERCLK3_EN	(1 << 18)
+#define PCCR0_NFC_EN		(1 << 19)
+#define PCCR0_FRI_BAUD_EN	(1 << 20)
+#define PCCR0_SLDC_EN		(1 << 21)
+#define PCCR0_PERCLK4_EN	(1 << 22)
+#define PCCR0_HCLK_BMI_EN	(1 << 23)
+#define PCCR0_HCLK_USBOTG_EN	(1 << 24)
+#define PCCR0_HCLK_SLCDC_EN	(1 << 25)
+#define PCCR0_HCLK_LCDC_EN	(1 << 26)
+#define PCCR0_HCLK_EMMA_EN	(1 << 27)
+#define PCCR0_HCLK_BROM_EN	(1 << 28)
+#define PCCR0_HCLK_DMA_EN	(1 << 30)
+#define PCCR0_HCLK_CSI_EN	(1 << 31)
+
+#define PCCR1_CSPI3_EN	(1 << 23)
+#define PCCR1_WDT_EN	(1 << 24)
+#define PCCR1_GPT1_EN	(1 << 25)
+#define PCCR1_GPT2_EN	(1 << 26)
+#define PCCR1_GPT3_EN	(1 << 27)
+#define PCCR1_PWM_EN	(1 << 28)
+#define PCCR1_RTC_EN	(1 << 29)
+#define PCCR1_KPP_EN	(1 << 30)
+#define PCCR1_OWIRE_EN	(1 << 31)
+
 enum imx21_clks {
 	ckil, ckih, fpm, mpll_sel, spll_sel, mpll, spll, fclk, hclk, ipg, per1,
 	per2, per3, per4, usb_div, nfc_div, lcdc_per_gate, clk_max

@@ -70,14 +112,16 @@ static int imx21_ccm_probe(struct device_d *dev)
 
 	base = dev_request_mem_region(dev, 0);
 
-	writel((1 << 0) | (1 << 1) | (1 << 2) | (1 << 3) | (1 << 4) | (1 << 5) |
-			(1 << 9) | (1 << 10) | (1 << 11) | (1 << 12) |
-			(1 << 13) | (1 << 14) | (1 << 19) | (1 << 22) |
-			(1 << 24) | (1 << 26) |	(1 << 30),
+	writel(PCCR0_UART1_EN | PCCR0_UART2_EN | PCCR0_UART3_EN | PCCR0_UART4_EN |
+			PCCR0_CSPI1_EN | PCCR0_CSPI2_EN | PCCR0_SDHC1_EN |
+			PCCR0_SDHC2_EN | PCCR0_GPIO_EN | PCCR0_I2C_EN | PCCR0_DMA_EN |
+			PCCR0_USBOTG_EN | PCCR0_NFC_EN | PCCR0_PERCLK4_EN |
+			PCCR0_HCLK_USBOTG_EN | PCCR0_HCLK_LCDC_EN | PCCR0_HCLK_DMA_EN,
 			base + CCM_PCCR0);
 
-	writel((1 << 23) | (1 << 24) | (1 << 25) | (1 << 26) | (1 << 27) |
-			(1 << 28) | (1 << 29) | (1 << 30) | (1 << 31),
+	writel(PCCR1_CSPI3_EN | PCCR1_WDT_EN | PCCR1_GPT1_EN | PCCR1_GPT2_EN |
+			PCCR1_GPT3_EN | PCCR1_PWM_EN | PCCR1_RTC_EN | PCCR1_KPP_EN |
+			PCCR1_OWIRE_EN,
 			base + CCM_PCCR1);
 
 	clks[ckil] = clk_fixed("ckil", lref);

arch/arm/mach-imx/include/mach/imx21-regs.h +0 -6

index 1c4b550..87bd99c 100644
@@ -129,12 +129,6 @@
 #define MX21_MPCTL1_BRMO	(1 << 6)
 #define MX21_MPCTL1_LF		(1 << 15)
 
-#define MX21_PCCR0_PERCLK3_EN	(1 << 18)
-#define MX21_PCCR0_NFC_EN	(1 << 19)
-#define MX21_PCCR0_HCLK_LCDC_EN	(1 << 26)
-
-#define MX21_PCCR1_GPT1_EN	(1 << 25)
-
 #define MX21_CCSR_32K_SR	(1 << 15)
 
 #endif /* _IMX21_REGS_H */

Signature · 7 lines

1.8.1


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Daniel Mierswa

author

2013-01-17 06:33 UTC #2 lore raw

PATCH 2/4 i.MX21/27: don't enable lcd bus clocks too early

In reply to Daniel Mierswa

On the MX27 based board phycard-i.MX27 the display won't properly come up.
Before removing imx-regs.h and the code that sets the register in the i.MX video driver, the PCCR registers were set _after_ the screen start (LSSAR) was set.
This restores that old behaviour and makes the display come up properly again.
I did not have a chance to test this on any other i.MX27 or i.MX21 hardware though I assume that the "old" order is required there too.

Signed-off-by Daniel Mierswa <[email protected]>

---
 arch/arm/mach-imx/clk-imx21.c | 13 +++++++++++--
 arch/arm/mach-imx/clk-imx27.c | 14 +++++++++-----
 drivers/video/imx.c           | 40 +++++++++++++++++++++++++++++++++-------
 3 files changed, 53 insertions(+), 14 deletions(-)

arch/arm/mach-imx/clk-imx21.c +11 -2

index 784951d..6e91424 100644
@@ -89,7 +89,8 @@
 
 enum imx21_clks {
 	ckil, ckih, fpm, mpll_sel, spll_sel, mpll, spll, fclk, hclk, ipg, per1,
-	per2, per3, per4, usb_div, nfc_div, lcdc_per_gate, clk_max
+	per2, per3, per4, usb_div, nfc_div, lcdc_per_gate, lcdc_ahb_gate,
+	lcdc_ipg_gate, clk_max
 };
 
 static struct clk *clks[clk_max];

@@ -116,7 +117,7 @@ static int imx21_ccm_probe(struct device_d *dev)
 			PCCR0_CSPI1_EN | PCCR0_CSPI2_EN | PCCR0_SDHC1_EN |
 			PCCR0_SDHC2_EN | PCCR0_GPIO_EN | PCCR0_I2C_EN | PCCR0_DMA_EN |
 			PCCR0_USBOTG_EN | PCCR0_NFC_EN | PCCR0_PERCLK4_EN |
-			PCCR0_HCLK_USBOTG_EN | PCCR0_HCLK_LCDC_EN | PCCR0_HCLK_DMA_EN,
+			PCCR0_HCLK_USBOTG_EN | PCCR0_HCLK_DMA_EN,
 			base + CCM_PCCR0);
 
 	writel(PCCR1_CSPI3_EN | PCCR1_WDT_EN | PCCR1_GPT1_EN | PCCR1_GPT2_EN |

@@ -143,6 +144,12 @@ static int imx21_ccm_probe(struct device_d *dev)
 	clks[usb_div] = imx_clk_divider("usb_div", "spll", base + CCM_CSCR, 26, 3);
 	clks[nfc_div] = imx_clk_divider("nfc_div", "ipg", base + CCM_PCDR0, 12, 4);
 	clks[lcdc_per_gate] = imx_clk_gate("lcdc_per_gate", "per3", base + CCM_PCCR0, 18);
+	clks[lcdc_ahb_gate] = imx_clk_gate("lcdc_ahb_gate", "ahb", base + CCM_PCCR0, 26);
+	/*
+	 * i.MX21 doesn't have an IPG clock for the LCD. To avoid even more conditionals
+	 * in the framebuffer code, provide a dummy clock.
+	 */
+	clks[lcdc_ipg_gate] = clk_fixed("dummy", 0);
 
 	clkdev_add_physbase(clks[per1], MX21_GPT1_BASE_ADDR, NULL);
 	clkdev_add_physbase(clks[per1], MX21_GPT2_BASE_ADDR, NULL);

@@ -158,6 +165,8 @@ static int imx21_ccm_probe(struct device_d *dev)
 	clkdev_add_physbase(clks[ipg], MX21_SDHC1_BASE_ADDR, NULL);
 	clkdev_add_physbase(clks[ipg], MX21_SDHC2_BASE_ADDR, NULL);
 	clkdev_add_physbase(clks[lcdc_per_gate], MX21_LCDC_BASE_ADDR, NULL);
+	clkdev_add_physbase(clks[lcdc_ahb_gate], MX21_LCDC_BASE_ADDR, "ahb");
+	clkdev_add_physbase(clks[lcdc_ipg_gate], MX21_LCDC_BASE_ADDR, "ipg");
 
 	return 0;
 }

arch/arm/mach-imx/clk-imx27.c +9 -5

index 222d2a6..958495e 100644
@@ -93,7 +93,7 @@
 enum mx27_clks {
 	dummy, ckih, ckil, mpll, spll, mpll_main2, ahb, ipg, nfc_div, per1_div,
 	per2_div, per3_div, per4_div, usb_div, cpu_sel,	clko_sel, cpu_div, clko_div,
-	clko_en, lcdc_per_gate, clk_max
+	clko_en, lcdc_per_gate, lcdc_ahb_gate, lcdc_ipg_gate, clk_max
 };
 
 static struct clk *clks[clk_max];

@@ -136,7 +136,7 @@ static int imx27_ccm_probe(struct device_d *dev)
 	base = dev_request_mem_region(dev, 0);
 
 	writel(PCCR0_SDHC3_EN | PCCR0_SDHC2_EN | PCCR0_SDHC1_EN |
-			PCCR0_PWM_EN | PCCR0_KPP_EN | PCCR0_LCDC_EN | PCCR0_IIM_EN |
+			PCCR0_PWM_EN | PCCR0_KPP_EN | PCCR0_IIM_EN |
 			PCCR0_I2C2_EN | PCCR0_I2C1_EN | PCCR0_GPT6_EN | PCCR0_GPT5_EN |
 			PCCR0_GPT4_EN | PCCR0_GPT3_EN | PCCR0_GPT2_EN | PCCR0_GPT1_EN |
 			PCCR0_GPIO_EN | PCCR0_FEC_EN | PCCR0_CSPI3_EN | PCCR0_CSPI2_EN |

@@ -144,9 +144,9 @@ static int imx27_ccm_probe(struct device_d *dev)
 			base + CCM_PCCR0);
 
 	writel(PCCR1_NFC_BAUDEN | PCCR1_PERCLK4_EN | PCCR1_PERCLK2_EN | PCCR1_PERCLK1_EN |
-			PCCR1_HCLK_USB | PCCR1_HCLK_LCDC | PCCR1_HCLK_FEC | PCCR1_HCLK_EMI |
-			PCCR1_WDT_EN | PCCR1_USB_EN | PCCR1_UART6_EN | PCCR1_UART5_EN |
-			PCCR1_UART4_EN | PCCR1_UART3_EN | PCCR1_UART2_EN | PCCR1_UART1_EN,
+			PCCR1_HCLK_USB | PCCR1_HCLK_FEC | PCCR1_HCLK_EMI | PCCR1_WDT_EN |
+			PCCR1_USB_EN | PCCR1_UART6_EN | PCCR1_UART5_EN | PCCR1_UART4_EN |
+			PCCR1_UART3_EN | PCCR1_UART2_EN | PCCR1_UART1_EN,
 			base + CCM_PCCR1);
 
 	clks[dummy] = clk_fixed("dummy", 0);

@@ -180,6 +180,8 @@ static int imx27_ccm_probe(struct device_d *dev)
 		clks[cpu_div] = imx_clk_divider("cpu_div", "cpu_sel", base + CCM_CSCR, 13, 3);
 	clks[clko_div] = imx_clk_divider("clko_div", "clko_sel", base + CCM_PCDR0, 22, 3);
 	clks[lcdc_per_gate] = imx_clk_gate("lcdc_per_gate", "per3_div", base + CCM_PCCR1, 7);
+	clks[lcdc_ahb_gate] = imx_clk_gate("lcdc_ahb_gate", "ahb", base + CCM_PCCR1, 15);
+	clks[lcdc_ipg_gate] = imx_clk_gate("lcdc_ipg_gate", "ipg", base + CCM_PCCR0, 14);
 
 	clkdev_add_physbase(clks[per1_div], MX27_GPT1_BASE_ADDR, NULL);
 	clkdev_add_physbase(clks[per1_div], MX27_GPT2_BASE_ADDR, NULL);

@@ -202,6 +204,8 @@ static int imx27_ccm_probe(struct device_d *dev)
 	clkdev_add_physbase(clks[per2_div], MX27_SDHC2_BASE_ADDR, NULL);
 	clkdev_add_physbase(clks[per2_div], MX27_SDHC3_BASE_ADDR, NULL);
 	clkdev_add_physbase(clks[lcdc_per_gate], MX27_LCDC_BASE_ADDR, NULL);
+	clkdev_add_physbase(clks[lcdc_ahb_gate], MX27_LCDC_BASE_ADDR, "ahb");
+	clkdev_add_physbase(clks[lcdc_ipg_gate], MX27_LCDC_BASE_ADDR, "ipg");
 	clkdev_add_physbase(clks[ipg], MX27_FEC_BASE_ADDR, NULL);
 
 	return 0;

drivers/video/imx.c +33 -7

index 8381690..54fb700 100644
@@ -138,7 +138,13 @@ struct imxfb_rgb {
 
 struct imxfb_info {
 	void __iomem		*regs;
-	struct clk		*clk;
+
+#if defined(CONFIG_ARCH_IMX21) || defined(CONFIG_ARCH_IMX27)
+	struct clk		*ahb_clk;
+	struct clk		*ipg_clk;
+#endif
+
+	struct clk		*per_clk;
 
 	u_int			pcr;
 	u_int			pwmr;

@@ -252,7 +258,12 @@ static void imxfb_enable_controller(struct fb_info *info)
 
 	writel(RMCR_LCDC_EN, fbi->regs + LCDC_RMCR);
 
-	clk_enable(fbi->clk);
+#if defined(CONFIG_ARCH_IMX21) || defined(CONFIG_ARCH_IMX27)
+	clk_enable(fbi->ahb_clk);
+	clk_enable(fbi->ipg_clk);
+#endif
+
+	clk_enable(fbi->per_clk);
 
 	if (fbi->enable)
 		fbi->enable(1);

@@ -267,7 +278,12 @@ static void imxfb_disable_controller(struct fb_info *info)
 
 	writel(0, fbi->regs + LCDC_RMCR);
 
-	clk_disable(fbi->clk);
+	clk_disable(fbi->per_clk);
+
+#if defined(CONFIG_ARCH_IMX21) || defined(CONFIG_ARCH_IMX27)
+	clk_disable(fbi->ipg_clk);
+	clk_disable(fbi->ahb_clk);
+#endif
 }
 
 static void set_reg_overlay(struct fb_info *overlay)

@@ -335,7 +351,7 @@ static int imxfb_activate_var(struct fb_info *info)
 	writel(readl(fbi->regs + LCDC_CPOS) & ~(CPOS_CC0 | CPOS_CC1),
 		fbi->regs + LCDC_CPOS);
 
-	lcd_clk = clk_get_rate(fbi->clk);
+	lcd_clk = clk_get_rate(fbi->per_clk);
 
 	tmp = mode->pixclock * (unsigned long long)lcd_clk;

@@ -549,9 +565,19 @@ static int imxfb_probe(struct device_d *dev)
 	fbi = xzalloc(sizeof(*fbi));
 	info = &fbi->info;
 
-	fbi->clk = clk_get(dev, NULL);
-	if (IS_ERR(fbi->clk))
-		return PTR_ERR(fbi->clk);
+	fbi->per_clk = clk_get(dev, NULL);
+	if (IS_ERR(fbi->per_clk))
+		return PTR_ERR(fbi->per_clk);
+
+#if defined(CONFIG_ARCH_IMX21) || defined(CONFIG_ARCH_IMX27)
+	fbi->ahb_clk = clk_get(dev, "ahb");
+	if (IS_ERR(fbi->ahb_clk))
+		return PTR_ERR(fbi->ahb_clk);
+
+	fbi->ipg_clk = clk_get(dev, "ipg");
+	if (IS_ERR(fbi->ipg_clk))
+		return PTR_ERR(fbi->ipg_clk);
+#endif
 
 	fbi->mode = pdata->mode;
 	fbi->regs = dev_request_mem_region(dev, 0);

Signature · 7 lines

1.8.1


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Daniel Mierswa

author

2013-01-17 06:33 UTC #3 lore raw

PATCH 3/4 i.MX27: fix shift amount for PCCR1_PERCLK3_EN

In reply to Daniel Mierswa

Signed-off-by Daniel Mierswa <[email protected]>

---
 arch/arm/mach-imx/clk-imx27.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/mach-imx/clk-imx27.c +1 -1

index 958495e..e221928 100644
@@ -179,7 +179,7 @@ static int imx27_ccm_probe(struct device_d *dev)
 	else
 		clks[cpu_div] = imx_clk_divider("cpu_div", "cpu_sel", base + CCM_CSCR, 13, 3);
 	clks[clko_div] = imx_clk_divider("clko_div", "clko_sel", base + CCM_PCDR0, 22, 3);
-	clks[lcdc_per_gate] = imx_clk_gate("lcdc_per_gate", "per3_div", base + CCM_PCCR1, 7);
+	clks[lcdc_per_gate] = imx_clk_gate("lcdc_per_gate", "per3_div", base + CCM_PCCR1, 8);
 	clks[lcdc_ahb_gate] = imx_clk_gate("lcdc_ahb_gate", "ahb", base + CCM_PCCR1, 15);
 	clks[lcdc_ipg_gate] = imx_clk_gate("lcdc_ipg_gate", "ipg", base + CCM_PCCR0, 14);

Signature · 7 lines

1.8.1


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Daniel Mierswa

author

2013-01-17 06:33 UTC #4 lore raw

PATCH 4/4 video/imx: always initialize offscreenbuf member

In reply to Daniel Mierswa

If offscreen was not passed to fd_open the resulting offscreenbuf member was a dangling pointer and the free() call in fd_close would result in undefined behaviour.

Signed-off-by Daniel Mierswa <[email protected]>

---
 commands/splash.c | 8 ++++++++
 1 file changed, 8 insertions(+)

commands/splash.c +8 -0

index 4cc463e..75c7074 100644
@@ -59,6 +59,14 @@ static int do_splash(int argc, char *argv[])
 		return 1;
 	}
 
+	/*
+	 * sc.offscreenbuf is dangling here, if no offscreen buffer was
+	 * requested, so set it to NULL otherwise the free in fd_close
+	 * will cause undefined behaviour
+	 */
+	if (!offscreen)
+		sc.offscreenbuf = NULL;
+
 	if (sc.offscreenbuf) {
 		if (do_bg)
 			memset_pixel(&info, sc.offscreenbuf, bg_color,

Signature · 7 lines

1.8.1


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Jean-Christophe PLAGNIOL-VILLARD

NAK

2013-01-17 11:57 UTC #5 lore raw

Re: [PATCH 4/4] video/imx: always initialize offscreenbuf member

Quoted 15 lines · Daniel Mierswa

If offscreen was not passed to fd_open the resulting offscreenbuf member was a dangling pointer and the free() call in fd_close would result in undefined behaviour.
Signed-off-by Daniel Mierswa <[email protected]>
---
 commands/splash.c | 8 ++++++++
 1 file changed, 8 insertions(+)
commands/splash.c +0 -0
index 4cc463e..75c7074 100644
@@ -59,6 +59,14 @@ static int do_splash(int argc, char *argv[])
 		return 1;

commands/splash.c static int do_splash(int argc, char *argv[]) · line 69

Quoted 10 lines

commands/splash.c +8 -0

 	}
 
+	/*
+	 * sc.offscreenbuf is dangling here, if no offscreen buffer was
+	 * requested, so set it to NULL otherwise the free in fd_close
+	 * will cause undefined behaviour
+	 */
+	if (!offscreen)
+		sc.offscreenbuf = NULL;
+

NAck

this is already done by memset

Best Regards,
J.

Quoted 11 lines

commands/splash.c +0 -0

 	if (sc.offscreenbuf) {
 		if (do_bg)
 			memset_pixel(&info, sc.offscreenbuf, bg_color,

Signature · 7 lines

1.8.1


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Daniel Mierswa

2013-01-17 13:03 UTC #6 lore raw

Re: [PATCH 4/4] video/imx: always initialize offscreenbuf member

Quoted 11 lines · Jean-Christophe PLAGNIOL-VILLARD

commands/splash.c static int do_splash(int argc, char *argv[]) · line 69
Quoted 8 lines · Daniel Mierswa
commands/splash.c +8 -0
+	/*
+	 * sc.offscreenbuf is dangling here, if no offscreen buffer was
+	 * requested, so set it to NULL otherwise the free in fd_close
+	 * will cause undefined behaviour
+	 */
+	if (!offscreen)
+		sc.offscreenbuf = NULL;
+
NAck

Quoted 7 lines

this is already done by memset
Best Regards,
J.
Quoted 3 lines
commands/splash.c +0 -0
 	if (sc.offscreenbuf) {
 		if (do_bg)
 			memset_pixel(&info, sc.offscreenbuf, bg_color,

Huh? I don't get it.

Signature · 9 lines

Mierswa, Daniel

If you still don't like it, that's ok: that's why I'm boss. I simply know better than you do.
               --- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Sascha Hauer

maintainer

2013-01-17 17:55 UTC #7 lore raw

Re: [PATCH 4/4] video/imx: always initialize offscreenbuf member

Quoted 14 lines · Daniel Mierswa

Quoted 13 lines · Jean-Christophe PLAGNIOL-VILLARD
commands/splash.c static int do_splash(int argc, char *argv[]) · line 69
Quoted 8 lines · Daniel Mierswa
commands/splash.c +8 -0
+	/*
+	 * sc.offscreenbuf is dangling here, if no offscreen buffer was
+	 * requested, so set it to NULL otherwise the free in fd_close
+	 * will cause undefined behaviour
+	 */
+	if (!offscreen)
+		sc.offscreenbuf = NULL;
+
NAck
this is already done by memset

Quoted 7 lines

Quoted 5 lines
Best Regards,
J.
Quoted 3 lines
commands/splash.c +0 -0
 	if (sc.offscreenbuf) {
 		if (do_bg)
 			memset_pixel(&info, sc.offscreenbuf, bg_color,
Huh? I don't get it.

sc is initialized to 0 in do_splash:

	memset(&s, 0, sizeof(s));
	memset(&sc, 0, sizeof(sc));
	memset(&info, 0, sizeof(info));

so sc.offscreenbuf = NULL is a no-op.

Sascha

Signature · 9 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Daniel Mierswa

2013-01-18 04:22 UTC #8 lore raw

Re: [PATCH 4/4] video/imx: always initialize offscreenbuf member

Quoted 7 lines · Sascha Hauer

sc is initialized to 0 in do_splash:
	memset(&s, 0, sizeof(s));
	memset(&sc, 0, sizeof(sc));
	memset(&info, 0, sizeof(info));
so sc.offscreenbuf = NULL is a no-op.

Oh bugger. My working copy didn't have the allocate commit d179981b2ddace3dfbf79f828c592a970cd2f009.
Ignore this one then. :)

Signature · 9 lines

Mierswa, Daniel

If you still don't like it, that's ok: that's why I'm boss. I simply know better than you do.
               --- Linus Torvalds, comp.os.linux.advocacy, 1996/07/22

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Sascha Hauer

maintainer

2013-01-18 10:23 UTC #9 lore raw

In reply to Daniel Mierswa

Quoted 8 lines · Daniel Mierswa

I adjusted the patch series and added the fixes as discussed on the mailinglist.
I still somehow feel that the offscreenbuf member should be set by the fd_open routine because I expect fd_close to always work with the struct set by fd_open no matter what offscreen parameter the function got.

Quoted 4 lines

Daniel Mierswa (4):
i.MX21: Add periph. clock register name macros i.MX21/27: don't enable lcd bus clocks too early i.MX27: fix shift amount for PCCR1_PERCLK3_EN

Applied up to this patch.

Thanks
Sascha

Quoted 18 lines

video/imx: initialize offscreenbuf member

 arch/arm/mach-imx/clk-imx21.c               | 67 ++++++++++++++++++++++++++---
 arch/arm/mach-imx/clk-imx27.c               | 16 ++++---
 arch/arm/mach-imx/include/mach/imx21-regs.h |  6 ---
 commands/splash.c                           |  8 ++++
 drivers/video/imx.c                         | 40 ++++++++++++++---
 5 files changed, 111 insertions(+), 26 deletions(-)

Signature · 7 lines

1.8.1


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Signature · 9 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox