[coreboot-gerrit] Change in coreboot[master]: soc/samsung/exynos5420: Refactor fimd vidtcon access

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34633 )

Change subject: soc/samsung/exynos5420: Refactor fimd vidtcon access
......................................................................

soc/samsung/exynos5420: Refactor fimd vidtcon access

Accessing the higher vidtcon variables using pointer arithmetic from the
lower address FIMD_CTRL struct is undefined behaviour, since pointers
manipulations are not allowed outside the objects they point to. The
standard-blessed way is to perform the arithmetic using integer
addresses first, and then convert that to a pointer. The end result is
the same, but avoids the risk of unsafe optimizations from an
over-zealous compiler.

Signed-off-by: Jacob Garber 
Found-by: Coverity CID 1402096, 1402124, 1402131, 1402169
Change-Id: I13ed23836e8e9076ae0bfd88c05c4f2badac9c49
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34633
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
---
M src/soc/samsung/exynos5420/fimd.c
M src/soc/samsung/exynos5420/include/soc/dp.h
M src/soc/samsung/exynos5420/include/soc/fimd.h
3 files changed, 9 insertions(+), 12 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved

diff --git a/src/soc/samsung/exynos5420/fimd.c b/src/soc/samsung/exynos5420/fimd.c
index 756d2fb..2b3552a 100644
--- a/src/soc/samsung/exynos5420/fimd.c
+++ b/src/soc/samsung/exynos5420/fimd.c
@@ -317,9 +317,10 @@
 void exynos_fimd_lcd_init(vidinfo_t *vid)
 {
    unsigned int cfg = 0, rgb_mode;
-	unsigned int offset;
+	struct exynos_fb *fimd;

-	offset = exynos_fimd_get_base_offset();
+	fimd = (void *)(FIMD_CTRL_ADDR + EXYNOS5_LCD_IF_BASE_OFFSET);
+
    printk(BIOS_SPEW, "%s\n", __func__);
    exynos5_set_system_display();

@@ -349,19 +350,19 @@
    	if (!vid->vl_dp)
    		cfg |= EXYNOS_VIDCON1_IVDEN_INVERT;

-		lwritel(cfg, &FIMD_CTRL->vidcon1 + offset);
+		lwritel(cfg, &fimd->vidcon1);

/* set timing */
    	cfg = EXYNOS_VIDTCON0_VFPD(vid->vl_vfpd - 1);
    	cfg |= EXYNOS_VIDTCON0_VBPD(vid->vl_vbpd - 1);
    	cfg |= EXYNOS_VIDTCON0_VSPW(vid->vl_vspw - 1);
-		lwritel(cfg, &FIMD_CTRL->vidtcon0 + offset);
+		lwritel(cfg, &fimd->vidtcon0);

cfg = EXYNOS_VIDTCON1_HFPD(vid->vl_hfpd - 1);
    	cfg |= EXYNOS_VIDTCON1_HBPD(vid->vl_hbpd - 1);
    	cfg |= EXYNOS_VIDTCON1_HSPW(vid->vl_hspw - 1);

-		lwritel(cfg, &FIMD_CTRL->vidtcon1 + offset);
+		lwritel(cfg, &fimd->vidtcon1);

/* set lcd size */
    	cfg = EXYNOS_VIDTCON2_HOZVAL(vid->vl_col - 1) |
@@ -369,7 +370,7 @@
    		EXYNOS_VIDTCON2_HOZVAL_E(vid->vl_col - 1) |
    		EXYNOS_VIDTCON2_LINEVAL_E(vid->vl_row - 1);

-		lwritel(cfg, &FIMD_CTRL->vidtcon2 + offset);
+		lwritel(cfg, &fimd->vidtcon2);
    }

/* set display mode */
diff --git a/src/soc/samsung/exynos5420/include/soc/dp.h b/src/soc/samsung/exynos5420/include/soc/dp.h
index 28db73a..6b33a76 100644
--- a/src/soc/samsung/exynos5420/include/soc/dp.h
+++ b/src/soc/samsung/exynos5420/include/soc/dp.h
@@ -884,11 +884,6 @@
 /* LCD IF register offset */
 #define EXYNOS5_LCD_IF_BASE_OFFSET			0x20000

-static inline u32 exynos_fimd_get_base_offset(void)
-{
-	return EXYNOS5_LCD_IF_BASE_OFFSET/4;
-}
-
 /*
  *  Register offsets
 */
diff --git a/src/soc/samsung/exynos5420/include/soc/fimd.h b/src/soc/samsung/exynos5420/include/soc/fimd.h
index d9d86cb..3e9d6a4 100644
--- a/src/soc/samsung/exynos5420/include/soc/fimd.h
+++ b/src/soc/samsung/exynos5420/include/soc/fimd.h
@@ -136,7 +136,8 @@
 #define OSD_RIGHTBOTX_F_OFFSET		11
 #define OSD_RIGHTBOTY_F_OFFSET		0

-#define FIMD_CTRL ((struct exynos_fb *)0x14400000)
+#define FIMD_CTRL_ADDR			0x14400000
+#define FIMD_CTRL			((struct exynos_fb *)FIMD_CTRL_ADDR)

/* from u-boot fb.h. It needs to be merged with these dp structs maybe. */
 enum {