Hi Jonathan, Please, see below: > > +static int mcp3564_update_8bits(struct mcp3564_state *adc, u8 reg, > > u32 mask, u8 val) > > +{ > > + u8 tmp; > > + int ret; > > + > > + ret = mcp3564_read_8bits(adc, reg, &tmp); > > + if (ret < 0) > > + return ret; > > + > > + val &= mask; > This looks wrong - would expect this to be > val &= ~mask; // wipe out the bits in mask. > > Actually am doing the opperation twice (the val is already correct by using FIELD_PREP when calling mcp3564_update_8bits). I'm making sure that the value received has only the mask bits valid. > > + val |= tmp & ~mask; > and > val |= tmp & mask; //write the bitss in mask. > > Is the mask the inverse ? At first glance it doesn't seem to be. > > The tmp is the value that was read from the hardware (I need not to touch/overwrite (~mask) bits). "tmp & ~mask" will clear the mask bits from the hardware, in order to make the "|" with the val. Actually my logic is a little bit twisted. I can rewrite it to be more clear. Thanks, Marius