* [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 [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 [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 [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 [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox