[Please include any former reviewers in new versions.] > The DS4520 is a 9-bit nonvolatile (NV) I/O expander. > It offers users a digitally programmable alternative > to hardware jumpers and mechanical switches that are > being used to control digital logic node. Ok, what I just noticed is that this is an open-drain output buffer with an optional pull-up, that should really go into the commit message. Also the commit message is misleading "it offers users a digitally programmable alternative to hardware jumpers". While the hardware is capable of that, this driver doesn't make use of it. > + config.reg_dat_base = base + IO_STATUS0; > + config.reg_set_base = base + PULLUP0; > + config.reg_dir_out_base = base + IO_CONTROL0; Given the above, I don't think this is correct. You pull the line low if the line is in input mode (?). The line will be pulled low if the corresponding bit in IO_CONTROL is zero. A one means, the pin is floating. With open-drain buffers there are usually an external pull-ups, so I'd treat the internal pull-up as optional and it is not necessary to switch the actual line state. In that case the following should be sufficient: config.reg_dat_base = base + IO_STATUS0; config.reg_set_base = base + IO_CONTROL0; I'm not sure about the direction though. Technically speaking there is no direction register. I'm not familiar with how open drain output are modeled in linux. I'd expect the above is enough. Bartosz/Linus/Andy? To enable the optional pull-up, you should refer to .set_config. (You don't need to disable the pull-up if you pull the line low). Regarding the SEE bit and wear out: The SEE bit seem to be shadowed by the EEPROM, so if someone is setting the SEE bit it will be persisent. Changing direction or output value will result in an EEPROM write and might wear out the EEPROM. I'd like to hear others opinion on that. The worst case write cycles are 50000. Fail the probe if the SEE bit is set seems not ideal. Just ignoring that problem for now (or at least warn the user)? -michael
