On Fri, Jan 30, 2009 at 3:50 PM, Mark Lord <liml@xxxxxx> wrote: > Fix mv_fill_sg() to zero out the reserved word (required for ATAPI), > and to include a memory barrier. This may also help with problems > reported by Jens on the PPC platform. > > Signed-off-by: Mark Lord <mlord@xxxxxxxxx> > > --- old/drivers/ata/sata_mv.c 2009-01-30 18:31:03.000000000 -0500 > +++ linux/drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 > @@ -1364,12 +1364,13 @@ > u32 offset = addr & 0xffff; > u32 len = sg_len; > > - if ((offset + sg_len > 0x10000)) > + if ((offset + len > 0x10000)) Why the double parens? > len = 0x10000 - offset; > > mv_sg->addr = cpu_to_le32(addr & 0xffffffff); > mv_sg->addr_hi = cpu_to_le32((addr >> 16) >> 16); > mv_sg->flags_size = cpu_to_le32(len & 0xffff); > + mv_sg->reserved = 0; > > sg_len -= len; > addr += len; > @@ -1381,6 +1382,7 @@ > > if (likely(last_sg)) > last_sg->flags_size |= cpu_to_le32(EPRD_FLAG_END_OF_TBL); > + mb(); /* ensure data structure is visible to the chipset */ It's not obvious to me what you are racing against here. Normally the mb() is to prevent the above store from getting executed *after* some MMIO read or write that would tell the chip to read the flags_size field (or anything recently stored in that data structure). I guess I'm asking for the comment to indicate which MMIO write it's racing with. thanks, grant > } > > static void mv_crqb_pack_cmd(__le16 *cmdw, u8 data, u8 addr, unsigned last) > -- > To unsubscribe from this list: send the line "unsubscribe linux-ide" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
