alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* old bug in au88x0_core.c
@ 2016-06-27 13:03 Dan Carpenter
  2016-06-29 13:40 ` Takashi Iwai
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2016-06-27 13:03 UTC (permalink / raw)
  To: alsa-devel

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: old bug in au88x0_core.c
  2016-06-27 13:03 old bug in au88x0_core.c Dan Carpenter
@ 2016-06-29 13:40 ` Takashi Iwai
  0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2016-06-29 13:40 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: alsa-devel

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

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-06-29 13:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-27 13:03 old bug in au88x0_core.c Dan Carpenter
2016-06-29 13:40 ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).