diff mbox

OMAPDSS: reading past end of array in dispc_dump_regs()

Message ID 20121214150133.GB15839@elgon.mountain (mailing list archive)
State New, archived
Headers show

Commit Message

Dan Carpenter Dec. 14, 2012, 3:01 p.m. UTC 
We added another kind of plane in 66a0f9e4ac "OMAPDSS: Use WB fifo for
GFX overlay" so this array needs a new entry as well.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Static checker work.  I don't have a way to test this.

--
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

Comments

Tomi Valkeinen Dec. 17, 2012, 12:09 p.m. UTC | #1 
On 2012-12-14 17:01, Dan Carpenter wrote:
> We added another kind of plane in 66a0f9e4ac "OMAPDSS: Use WB fifo for
> GFX overlay" so this array needs a new entry as well.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static checker work.  I don't have a way to test this.
> 
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index fedbd2c..bfe62cc 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -3163,6 +3163,7 @@ static void dispc_dump_regs(struct seq_file *s)
>  		[OMAP_DSS_VIDEO1]	= "VID1",
>  		[OMAP_DSS_VIDEO2]	= "VID2",
>  		[OMAP_DSS_VIDEO3]	= "VID3",
> +		[OMAP_DSS_WB]		= "WB",
>  	};
>  	const char **p_names;
>  
> 

We don't count WB as an overlay currently, as it's handled a bit
differently, so we never try to access that array with OMAP_DSS_WB. We
don't actually dump any WB related registers currently, it seems.

So I think I'll leave this out for now.

Why does the static checker think OMAP_DSS_WB is needed in the array? I
wonder if I'm reading the code wrong, and we indeed do access the array
with OMAP_DSS_WB...

 Tomi
Dan Carpenter Dec. 17, 2012, 12:39 p.m. UTC | #2 
On Mon, Dec 17, 2012 at 02:09:00PM +0200, Tomi Valkeinen wrote:
> Why does the static checker think OMAP_DSS_WB is needed in the array?

drivers/video/omap2/dss/dispc.c +3284

  3274  #define DISPC_REG(plane, name, i) name(plane, i)
  3275  #define DUMPREG(plane, name, i) \
  3276          seq_printf(s, "%s_%d(%s)%*s %08x\n", #name, i, p_names[plane], \
  3277          (int)(46 - strlen(#name) - strlen(p_names[plane])), " ", \
  3278          dispc_read_reg(DISPC_REG(plane, name, i)))
  3279  
  3280          /* Video pipeline coefficient registers */
  3281  
  3282          /* start from OMAP_DSS_VIDEO1 */
  3283          for (i = 1; i < dss_feat_get_num_ovls(); i++) {
  3284                  for (j = 0; j < 8; j++)
  3285                          DUMPREG(i, DISPC_OVL_FIR_COEF_H, j);

The logic here is that we pass i to DISPC_OVL_FIR_COEF_H() which
passes i to DISPC_FIR_COEF_H_OFFSET().  Anything higher than
OMAP_DSS_WB will trigger a BUG() in DISPC_FIR_COEF_H_OFFSET().

So it's not rock hard logic at all.

regards,
dan carpenter

--
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
diff mbox

Patch

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index fedbd2c..bfe62cc 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3163,6 +3163,7 @@  static void dispc_dump_regs(struct seq_file *s)
 		[OMAP_DSS_VIDEO1]	= "VID1",
 		[OMAP_DSS_VIDEO2]	= "VID2",
 		[OMAP_DSS_VIDEO3]	= "VID3",
+		[OMAP_DSS_WB]		= "WB",
 	};
 	const char **p_names;