Skip to contentSkip to conversation

OMAPDSS: HDMI: Add OMAP5 HDMI support

DISCUSSION Dan Carpenter 3 participants 3 messages Dec 3, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hello Tomi Valkeinen,

The patch f5bab2229190: "OMAPDSS: HDMI: Add OMAP5 HDMI support" from Mar 13, 2014, leads to the following static checker warning:

	drivers/video/fbdev/omap2/dss/hdmi5_core.c:719 hdmi5_core_audio_config()
	warn: '(3) - (4)' negative one

Uh... This is really weird, I have no idea what this warning is or why it's triggered. I've even looked at the code which triggers it but that has no documentation. :P Still it seems like something worth looking into.

drivers/video/fbdev/omap2/dss/hdmi5_core.c

   713          /* PCM audio mode */
   714          val = (cfg->iec60958_cfg->status[0] & IEC958_AES0_CON_MODE) >> 6;
   715          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 6, 4);
   716  
   717          /* Source number */
   718          val = cfg->iec60958_cfg->status[2] & IEC958_AES2_CON_SOURCE;
   719          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 3, 4);
                                                                  ^^^^

Aren't these reversed? This seems like an invalid bitfield range.

   720  
   721          /* Channel number right 0  */
   722          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(3), 2, 3, 0);
   723          /* Channel number right 1*/
   724          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(3), 4, 7, 4);
   725          /* Channel number right 2  */
   726          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(4), 6, 3, 0);
   727          /* Channel number right 3*/
   728          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(4), 8, 7, 4);
   729          /* Channel number left 0  */
   730          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(5), 1, 3, 0);
   731          /* Channel number left 1*/
   732          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(5), 3, 7, 4);
   733          /* Channel number left 2  */
   734          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(6), 5, 3, 0);
   735          /* Channel number left 3*/

regards,
dan carpenter

#1 lore raw

Hi Dan, Jyri,

Quoted 5 lines · Dan Carpenter

Hello Tomi Valkeinen,

The patch f5bab2229190: "OMAPDSS: HDMI: Add OMAP5 HDMI support" from Mar 13, 2014, leads to the following static checker warning:

Quoted 7 lines
	drivers/video/fbdev/omap2/dss/hdmi5_core.c:719 hdmi5_core_audio_config()
	warn: '(3) - (4)' negative one

Uh... This is really weird, I have no idea what this warning is or why it's triggered. I've even looked at the code which triggers it but that has no documentation. :P Still it seems like something worth looking into.

I presume it somehow is caused by this line in the dss.h:

#define FLD_MASK(start, end) (((1 << ((start) - (end) + 1)) - 1) << (end))

But I don't quite see why the warnings happens. start=3 and end=4 should produce valid code there, even if the end result is not what's expected...

Quoted 10 lines

drivers/video/fbdev/omap2/dss/hdmi5_core.c

   713          /* PCM audio mode */
   714          val = (cfg->iec60958_cfg->status[0] & IEC958_AES0_CON_MODE) >> 6;
   715          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 6, 4);
   716  
   717          /* Source number */
   718          val = cfg->iec60958_cfg->status[2] & IEC958_AES2_CON_SOURCE;
   719          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 3, 4);
                                                                  ^^^^

Aren't these reversed? This seems like an invalid bitfield range.

Good catch! Indeed it's reversed.

Jyri, this is in the hdmi audio side. Can you check it out? Fix is trivial, but I'd like to know if it has caused wrong configuration, and if it has, why haven't we noticed anything.

Tomi

#2 lore raw
Quoted 34 lines · Tomi Valkeinen

Hi Dan, Jyri,

Quoted 5 lines · Dan Carpenter

Hello Tomi Valkeinen,

The patch f5bab2229190: "OMAPDSS: HDMI: Add OMAP5 HDMI support" from Mar 13, 2014, leads to the following static checker warning:

Quoted 7 lines
	drivers/video/fbdev/omap2/dss/hdmi5_core.c:719 hdmi5_core_audio_config()
	warn: '(3) - (4)' negative one

Uh... This is really weird, I have no idea what this warning is or why it's triggered. I've even looked at the code which triggers it but that has no documentation. :P Still it seems like something worth looking into.

I presume it somehow is caused by this line in the dss.h:

#define FLD_MASK(start, end) (((1 << ((start) - (end) + 1)) - 1) << (end))

But I don't quite see why the warnings happens. start=3 and end=4 should produce valid code there, even if the end result is not what's expected...

Quoted 10 lines

drivers/video/fbdev/omap2/dss/hdmi5_core.c

    713          /* PCM audio mode */
    714          val = (cfg->iec60958_cfg->status[0] & IEC958_AES0_CON_MODE) >> 6;
    715          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 6, 4);
    716
    717          /* Source number */
    718          val = cfg->iec60958_cfg->status[2] & IEC958_AES2_CON_SOURCE;
    719          REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 3, 4);
                                                                   ^^^^

Aren't these reversed? This seems like an invalid bitfield range.

Quoted 6 lines

Good catch! Indeed it's reversed.

Jyri, this is in the hdmi audio side. Can you check it out? Fix is trivial, but I'd like to know if it has caused wrong configuration, and if it has, why haven't we noticed anything.

The bits written there are always zero and hardly significant. The HDMI audio still works after the attached patch.

Best regards,
Jyri

ps. One of these days I am going to have to go through these settings register by register...

Quoted 1 line

From 08460e1db02f1babd75b72769e3078462f8e5d81 Mon Sep 17 00:00:00 2001

From: Jyri Sarha <[email protected]>
Date: Wed, 3 Dec 2014 16:24:06 +0200
Subject: [PATCH] OMAPDSS: hdmi5: Fix bit field for IEC958_AES2_CON_SOURCE

The bit field for IEC958_AES2_CON_SOURCE is bit 3-0 in HDMI_CORE_FC_AUDSCHNLS2, not imaginary bits 3-4 (reverse order).

---
 drivers/video/fbdev/omap2/dss/hdmi5_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/video/fbdev/omap2/dss/hdmi5_core.c +1 -1
index a711579..a3cfe3d 100644
@@ -716,7 +716,7 @@ static void hdmi5_core_audio_config(struct hdmi_core_data *core,
 
 	/* Source number */
 	val = cfg->iec60958_cfg->status[2] & IEC958_AES2_CON_SOURCE;
-	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 3, 4);
+	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(2), val, 3, 0);
 
 	/* Channel number right 0  */
 	REG_FLD_MOD(base, HDMI_CORE_FC_AUDSCHNLS(3), 2, 3, 0);
Signature · 1 line
1.7.9.5