[alsa-devel] old bug in au88x0_core.c
Hi ALSA devs,
The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads to the following static checker warning:
sound/pci/au88x0/au88x0_core.c:1449 vortex_wtdma_bufshift() warn: mask and shift to zero
sound/pci/au88x0/au88x0_core.c 1441 static int vortex_wtdma_bufshift(vortex_t * vortex, int wtdma) 1442 { 1443 stream_t *dma = &vortex->dma_wt[wtdma]; 1444 int page, p, pp, delta, i; 1445 1446 page = 1447 (hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)) & 1448 WT_SUBBUF_MASK) 1449 >> WT_SUBBUF_SHIFT;
This is always zero because:
#define WT_SUBBUF_MASK 0x3 #define WT_SUBBUF_SHIFT 0x15
1450 if (dma->nr_periods >= 4) 1451 delta = (page - dma->period_real) & 3; 1452 else { 1453 delta = (page - dma->period_real); 1454 if (delta < 0) 1455 delta += dma->nr_periods; 1456 } 1457 if (delta == 0) 1458 return 0;
1493 #if 0 1494 static void 1495 vortex_wtdma_getposition(vortex_t * vortex, int wtdma, int *subbuf, int *pos) 1496 { 1497 int temp; 1498 temp = hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)); 1499 *subbuf = (temp >> WT_SUBBUF_SHIFT) & WT_SUBBUF_MASK;
From this ifdef zero code, it looks like we should shift first then do
the mask.
1500 *pos = temp & POS_MASK; 1501 } 1502 1503 static int vortex_wtdma_getcursubuffer(vortex_t * vortex, int wtdma) 1504 { 1505 return ((hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)) >> 1506 POS_SHIFT) & POS_MASK); 1507 } 1508 #endif
regards, dan carpenter
On Mon, 27 Jun 2016 15:03:47 +0200, Dan Carpenter wrote:
Hi ALSA devs,
The patch 1da177e4c3f4: "Linux-2.6.12-rc2" from Apr 16, 2005, leads to the following static checker warning:
sound/pci/au88x0/au88x0_core.c:1449 vortex_wtdma_bufshift() warn: mask and shift to zero
sound/pci/au88x0/au88x0_core.c 1441 static int vortex_wtdma_bufshift(vortex_t * vortex, int wtdma) 1442 { 1443 stream_t *dma = &vortex->dma_wt[wtdma]; 1444 int page, p, pp, delta, i; 1445 1446 page = 1447 (hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)) & 1448 WT_SUBBUF_MASK) 1449 >> WT_SUBBUF_SHIFT;
This is always zero because:
#define WT_SUBBUF_MASK 0x3 #define WT_SUBBUF_SHIFT 0x15
1450 if (dma->nr_periods >= 4) 1451 delta = (page - dma->period_real) & 3; 1452 else { 1453 delta = (page - dma->period_real); 1454 if (delta < 0) 1455 delta += dma->nr_periods; 1456 } 1457 if (delta == 0) 1458 return 0;
1493 #if 0 1494 static void 1495 vortex_wtdma_getposition(vortex_t * vortex, int wtdma, int *subbuf, int *pos) 1496 { 1497 int temp; 1498 temp = hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)); 1499 *subbuf = (temp >> WT_SUBBUF_SHIFT) & WT_SUBBUF_MASK;
From this ifdef zero code, it looks like we should shift first then do
the mask.
1500 *pos = temp & POS_MASK; 1501 } 1502 1503 static int vortex_wtdma_getcursubuffer(vortex_t * vortex, int wtdma) 1504 { 1505 return ((hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)) >> 1506 POS_SHIFT) & POS_MASK); 1507 } 1508 #endif
Correct, this should be fixed in that way. Below is the fix patch I'm going to queue.
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: au88x0: Fix calculation in vortex_wtdma_bufshift()
vortex_wtdma_bufshift() function does calculate the page index wrongly, first masking then shift, which always results in zero. The proper computation is to first shift, then mask.
Reported-by: Dan Carpenter dan.carpenter@oracle.com Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/au88x0/au88x0_core.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/pci/au88x0/au88x0_core.c b/sound/pci/au88x0/au88x0_core.c index 4a054d720112..d3125c169684 100644 --- a/sound/pci/au88x0/au88x0_core.c +++ b/sound/pci/au88x0/au88x0_core.c @@ -1444,9 +1444,8 @@ static int vortex_wtdma_bufshift(vortex_t * vortex, int wtdma) int page, p, pp, delta, i;
page = - (hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)) & - WT_SUBBUF_MASK) - >> WT_SUBBUF_SHIFT; + (hwread(vortex->mmio, VORTEX_WTDMA_STAT + (wtdma << 2)) + >> WT_SUBBUF_SHIFT) & WT_SUBBUF_MASK; if (dma->nr_periods >= 4) delta = (page - dma->period_real) & 3; else {
participants (2)
-
Dan Carpenter
-
Takashi Iwai