Jean, > > + /* Comparator mode (D3=0), standby mode (D6=0) */ > > + gl518_write_value(client, GL518_REG_CONF, 0xb7 & regvalue); > > I'd suggest 0x37 as the mask. D7 is something we don't want to set to 1. > I agree it doesn't change anything practically. Just looks safer. > ok > > + /* Clear status register (D5=1), start (D6=1) */ > > + gl518_write_value(client, GL518_REG_CONF, 0x20 | regvalue); > > + gl518_write_value(client, GL518_REG_CONF, 0x40 | regvalue); > > It works, but isn't correct. At this point, regvalue doesn't hold the > current configuration register. I suggest we use (regvalue &= 0x37) in > the first gl518_write_value() call above, so that it does. > There is already regvalue = gl518_read_value() & 0x14; above that, since regvalue is supposed to only hold values from D6,D3, so there is no need to &= 0x37 again. I don't think we should change this. Just the 0xb7 to 0x37 above. Hong-Gunn
