Date: Sat, 13 Dec 2008 15:21:29 -0800 From: Harvey Harrison Subject: ieee1934: dv1394: interrupt enabling/disabling broken on big-endian After annotating the frame structs, this was left: drivers/ieee1394/dv1394.c:2113:23: warning: invalid assignment: |= drivers/ieee1394/dv1394.c:2113:23: left side has type restricted __le32 drivers/ieee1394/dv1394.c:2113:23: right side has type int drivers/ieee1394/dv1394.c:2121:24: warning: invalid assignment: &= drivers/ieee1394/dv1394.c:2121:24: left side has type restricted __le32 drivers/ieee1394/dv1394.c:2121:24: right side has type int drivers/ieee1394/dv1394.c:2123:24: warning: invalid assignment: |= drivers/ieee1394/dv1394.c:2123:24: left side has type restricted __le32 drivers/ieee1394/dv1394.c:2123:24: right side has type int Which looks like a real bug on a big-endian arch as it would set/clear the wrong bit. Signed-off-by: Harvey Harrison Bill Fink writes: I finally got a chance to test the patch on my kernel, and live DV viewing using xine still worked fine. Although I admit to being mystified how it works both before and after the patch, since the cpu_to_le32() calls that were added should result in byte swapping on PPC that wasn't being done before. I guess that either the code paths involved aren't actually being triggered by my xine DV viewing, or there's some fortuitous palindromic setting of bits. Tested-by: Bill Fink Signed-off-by: Stefan Richter --- drivers/ieee1394/dv1394.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) Index: linux/drivers/ieee1394/dv1394.c =================================================================== --- linux.orig/drivers/ieee1394/dv1394.c +++ linux/drivers/ieee1394/dv1394.c @@ -2110,17 +2110,17 @@ static void ir_tasklet_func(unsigned lon f = video->frames[next_i / MAX_PACKETS]; next = &(f->descriptor_pool[next_i % MAX_PACKETS]); next_dma = ((unsigned long) block - (unsigned long) f->descriptor_pool) + f->descriptor_pool_dma; - next->u.in.il.q[0] |= 3 << 20; /* enable interrupt */ - next->u.in.il.q[2] = 0; /* disable branch */ + next->u.in.il.q[0] |= cpu_to_le32(3 << 20); /* enable interrupt */ + next->u.in.il.q[2] = cpu_to_le32(0); /* disable branch */ /* link previous to next */ prev_i = (next_i == 0) ? (MAX_PACKETS * video->n_frames - 1) : (next_i - 1); f = video->frames[prev_i / MAX_PACKETS]; prev = &(f->descriptor_pool[prev_i % MAX_PACKETS]); if (prev_i % (MAX_PACKETS/2)) { - prev->u.in.il.q[0] &= ~(3 << 20); /* no interrupt */ + prev->u.in.il.q[0] &= ~cpu_to_le32(3 << 20); /* no interrupt */ } else { - prev->u.in.il.q[0] |= 3 << 20; /* enable interrupt */ + prev->u.in.il.q[0] |= cpu_to_le32(3 << 20); /* enable interrupt */ } prev->u.in.il.q[2] = cpu_to_le32(next_dma | 1); /* set Z=1 */ wmb();