* [RESEND PATCH] framebuffer work (especially i.MX)
@ 2013-01-17 6:32 Daniel Mierswa
2013-01-17 6:32 ` [PATCH 1/4] i.MX21: Add periph. clock register name macros Daniel Mierswa
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Daniel Mierswa @ 2013-01-17 6:32 UTC (permalink / raw)
To: barebox
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(-)
--
1.8.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/4] i.MX21: Add periph. clock register name macros 2013-01-17 6:32 [RESEND PATCH] framebuffer work (especially i.MX) Daniel Mierswa @ 2013-01-17 6:32 ` Daniel Mierswa 2013-01-17 6:32 ` [PATCH 2/4] i.MX21/27: don't enable lcd bus clocks too early Daniel Mierswa ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Daniel Mierswa @ 2013-01-17 6:32 UTC (permalink / raw) To: barebox Also put those names solely in the .c file as it's done with the i.MX27 code. Signed-off-by: Daniel Mierswa <d.mierswa@phytec.de> --- 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(-) diff --git a/arch/arm/mach-imx/clk-imx21.c b/arch/arm/mach-imx/clk-imx21.c index 9e7af81..784951d 100644 --- a/arch/arm/mach-imx/clk-imx21.c +++ b/arch/arm/mach-imx/clk-imx21.c @@ -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); diff --git a/arch/arm/mach-imx/include/mach/imx21-regs.h b/arch/arm/mach-imx/include/mach/imx21-regs.h index 1c4b550..87bd99c 100644 --- a/arch/arm/mach-imx/include/mach/imx21-regs.h +++ b/arch/arm/mach-imx/include/mach/imx21-regs.h @@ -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 */ -- 1.8.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] i.MX21/27: don't enable lcd bus clocks too early 2013-01-17 6:32 [RESEND PATCH] framebuffer work (especially i.MX) Daniel Mierswa 2013-01-17 6:32 ` [PATCH 1/4] i.MX21: Add periph. clock register name macros Daniel Mierswa @ 2013-01-17 6:32 ` Daniel Mierswa 2013-01-17 6:32 ` [PATCH 3/4] i.MX27: fix shift amount for PCCR1_PERCLK3_EN Daniel Mierswa ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Daniel Mierswa @ 2013-01-17 6:32 UTC (permalink / raw) To: barebox 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 <d.mierswa@phytec.de> --- 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(-) diff --git a/arch/arm/mach-imx/clk-imx21.c b/arch/arm/mach-imx/clk-imx21.c index 784951d..6e91424 100644 --- a/arch/arm/mach-imx/clk-imx21.c +++ b/arch/arm/mach-imx/clk-imx21.c @@ -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; } diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c index 222d2a6..958495e 100644 --- a/arch/arm/mach-imx/clk-imx27.c +++ b/arch/arm/mach-imx/clk-imx27.c @@ -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; diff --git a/drivers/video/imx.c b/drivers/video/imx.c index 8381690..54fb700 100644 --- a/drivers/video/imx.c +++ b/drivers/video/imx.c @@ -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); -- 1.8.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] i.MX27: fix shift amount for PCCR1_PERCLK3_EN 2013-01-17 6:32 [RESEND PATCH] framebuffer work (especially i.MX) Daniel Mierswa 2013-01-17 6:32 ` [PATCH 1/4] i.MX21: Add periph. clock register name macros Daniel Mierswa 2013-01-17 6:32 ` [PATCH 2/4] i.MX21/27: don't enable lcd bus clocks too early Daniel Mierswa @ 2013-01-17 6:32 ` Daniel Mierswa 2013-01-17 6:32 ` [PATCH 4/4] video/imx: always initialize offscreenbuf member Daniel Mierswa 2013-01-18 10:23 ` [RESEND PATCH] framebuffer work (especially i.MX) Sascha Hauer 4 siblings, 0 replies; 10+ messages in thread From: Daniel Mierswa @ 2013-01-17 6:32 UTC (permalink / raw) To: barebox Signed-off-by: Daniel Mierswa <d.mierswa@phytec.de> --- arch/arm/mach-imx/clk-imx27.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c index 958495e..e221928 100644 --- a/arch/arm/mach-imx/clk-imx27.c +++ b/arch/arm/mach-imx/clk-imx27.c @@ -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); -- 1.8.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] video/imx: always initialize offscreenbuf member 2013-01-17 6:32 [RESEND PATCH] framebuffer work (especially i.MX) Daniel Mierswa ` (2 preceding siblings ...) 2013-01-17 6:32 ` [PATCH 3/4] i.MX27: fix shift amount for PCCR1_PERCLK3_EN Daniel Mierswa @ 2013-01-17 6:32 ` Daniel Mierswa 2013-01-17 11:56 ` Jean-Christophe PLAGNIOL-VILLARD 2013-01-18 10:23 ` [RESEND PATCH] framebuffer work (especially i.MX) Sascha Hauer 4 siblings, 1 reply; 10+ messages in thread From: Daniel Mierswa @ 2013-01-17 6:32 UTC (permalink / raw) To: barebox 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 <d.mierswa@phytec.de> --- commands/splash.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/commands/splash.c b/commands/splash.c index 4cc463e..75c7074 100644 --- a/commands/splash.c +++ b/commands/splash.c @@ -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, -- 1.8.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] video/imx: always initialize offscreenbuf member 2013-01-17 6:32 ` [PATCH 4/4] video/imx: always initialize offscreenbuf member Daniel Mierswa @ 2013-01-17 11:56 ` Jean-Christophe PLAGNIOL-VILLARD 2013-01-17 13:04 ` Daniel Mierswa 0 siblings, 1 reply; 10+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-01-17 11:56 UTC (permalink / raw) To: Daniel Mierswa; +Cc: barebox On 07:32 Thu 17 Jan , Daniel Mierswa wrote: > 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 <d.mierswa@phytec.de> > --- > commands/splash.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/commands/splash.c b/commands/splash.c > index 4cc463e..75c7074 100644 > --- a/commands/splash.c > +++ b/commands/splash.c > @@ -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; > + NAck this is already done by memset Best Regards, J. > if (sc.offscreenbuf) { > if (do_bg) > memset_pixel(&info, sc.offscreenbuf, bg_color, > -- > 1.8.1 > > > _______________________________________________ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] video/imx: always initialize offscreenbuf member 2013-01-17 11:56 ` Jean-Christophe PLAGNIOL-VILLARD @ 2013-01-17 13:04 ` Daniel Mierswa 2013-01-17 17:55 ` Sascha Hauer 0 siblings, 1 reply; 10+ messages in thread From: Daniel Mierswa @ 2013-01-17 13:04 UTC (permalink / raw) To: barebox On 01/17/2013 12:56 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 07:32 Thu 17 Jan , Daniel Mierswa wrote: >> + /* >> + * 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. >> if (sc.offscreenbuf) { >> if (do_bg) >> memset_pixel(&info, sc.offscreenbuf, bg_color, Huh? I don't get it. -- 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 barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] video/imx: always initialize offscreenbuf member 2013-01-17 13:04 ` Daniel Mierswa @ 2013-01-17 17:55 ` Sascha Hauer 2013-01-18 4:24 ` Daniel Mierswa 0 siblings, 1 reply; 10+ messages in thread From: Sascha Hauer @ 2013-01-17 17:55 UTC (permalink / raw) To: Daniel Mierswa; +Cc: barebox On Thu, Jan 17, 2013 at 02:04:22PM +0100, Daniel Mierswa wrote: > On 01/17/2013 12:56 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > > On 07:32 Thu 17 Jan , Daniel Mierswa wrote: > >> + /* > >> + * 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. > >> 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 -- 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 barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] video/imx: always initialize offscreenbuf member 2013-01-17 17:55 ` Sascha Hauer @ 2013-01-18 4:24 ` Daniel Mierswa 0 siblings, 0 replies; 10+ messages in thread From: Daniel Mierswa @ 2013-01-18 4:24 UTC (permalink / raw) To: barebox On 01/17/2013 06:55 PM, Sascha Hauer wrote: > 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. :) -- 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 barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RESEND PATCH] framebuffer work (especially i.MX) 2013-01-17 6:32 [RESEND PATCH] framebuffer work (especially i.MX) Daniel Mierswa ` (3 preceding siblings ...) 2013-01-17 6:32 ` [PATCH 4/4] video/imx: always initialize offscreenbuf member Daniel Mierswa @ 2013-01-18 10:23 ` Sascha Hauer 4 siblings, 0 replies; 10+ messages in thread From: Sascha Hauer @ 2013-01-18 10:23 UTC (permalink / raw) To: Daniel Mierswa; +Cc: barebox On Thu, Jan 17, 2013 at 07:32:55AM +0100, Daniel Mierswa wrote: > 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 Applied up to this patch. Thanks Sascha > 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(-) > > -- > 1.8.1 > > > _______________________________________________ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox > -- 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 barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-01-18 10:23 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-17 6:32 [RESEND PATCH] framebuffer work (especially i.MX) Daniel Mierswa 2013-01-17 6:32 ` [PATCH 1/4] i.MX21: Add periph. clock register name macros Daniel Mierswa 2013-01-17 6:32 ` [PATCH 2/4] i.MX21/27: don't enable lcd bus clocks too early Daniel Mierswa 2013-01-17 6:32 ` [PATCH 3/4] i.MX27: fix shift amount for PCCR1_PERCLK3_EN Daniel Mierswa 2013-01-17 6:32 ` [PATCH 4/4] video/imx: always initialize offscreenbuf member Daniel Mierswa 2013-01-17 11:56 ` Jean-Christophe PLAGNIOL-VILLARD 2013-01-17 13:04 ` Daniel Mierswa 2013-01-17 17:55 ` Sascha Hauer 2013-01-18 4:24 ` Daniel Mierswa 2013-01-18 10:23 ` [RESEND PATCH] framebuffer work (especially i.MX) Sascha Hauer
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.