* re: OMAPDSS: HDMI: Add OMAP5 HDMI support
@ 2014-12-03 12:22 Dan Carpenter
2014-12-03 12:42 ` Tomi Valkeinen
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2014-12-03 12:22 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: OMAPDSS: HDMI: Add OMAP5 HDMI support
2014-12-03 12:22 OMAPDSS: HDMI: Add OMAP5 HDMI support Dan Carpenter
@ 2014-12-03 12:42 ` Tomi Valkeinen
2014-12-03 15:05 ` Jyri Sarha
0 siblings, 1 reply; 3+ messages in thread
From: Tomi Valkeinen @ 2014-12-03 12:42 UTC (permalink / raw)
To: Dan Carpenter, Jyri Sarha; +Cc: linux-omap
[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]
Hi Dan, Jyri,
On 03/12/14 14:22, Dan Carpenter wrote:
> 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.
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...
> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: OMAPDSS: HDMI: Add OMAP5 HDMI support
2014-12-03 12:42 ` Tomi Valkeinen
@ 2014-12-03 15:05 ` Jyri Sarha
0 siblings, 0 replies; 3+ messages in thread
From: Jyri Sarha @ 2014-12-03 15:05 UTC (permalink / raw)
To: Tomi Valkeinen, Dan Carpenter; +Cc: linux-omap
[-- Attachment #1: Type: text/plain, Size: 1988 bytes --]
On 12/03/2014 02:42 PM, Tomi Valkeinen wrote:
> Hi Dan, Jyri,
>
> On 03/12/14 14:22, Dan Carpenter wrote:
>> 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.
>
> 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...
>
>> 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.
>
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...
[-- Attachment #2: 0001-OMAPDSS-hdmi5-Fix-bit-field-for-IEC958_AES2_CON_SOUR.patch --]
[-- Type: text/x-patch, Size: 1109 bytes --]
>From 08460e1db02f1babd75b72769e3078462f8e5d81 Mon Sep 17 00:00:00 2001
From: Jyri Sarha <jsarha@ti.com>
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).
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
drivers/video/fbdev/omap2/dss/hdmi5_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/dss/hdmi5_core.c b/drivers/video/fbdev/omap2/dss/hdmi5_core.c
index a711579..a3cfe3d 100644
--- a/drivers/video/fbdev/omap2/dss/hdmi5_core.c
+++ b/drivers/video/fbdev/omap2/dss/hdmi5_core.c
@@ -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);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-12-03 15:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-03 12:22 OMAPDSS: HDMI: Add OMAP5 HDMI support Dan Carpenter
2014-12-03 12:42 ` Tomi Valkeinen
2014-12-03 15:05 ` Jyri Sarha
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.