On Tue, Nov 06, 2018 at 08:08:15PM +0000, Peter Rosin wrote: > On 2018-11-06 17:37, Peter Rosin wrote: > > Hi! > > > > Some comments inline... > > [snip] > >> + > >> + data->buf[0] = MCP41010_WIPER_ENABLE << channel; > >> + data->buf[0] |= MCP41010_WRITE; > > > > Will this not clobber the other channel for mcp42xxx chips??? > > > I had a peak in the datasheet, and no, it will not. It was just the naming > with ..._WIPER_ENABLE that threw me off. It assumed, from the naming, that > each channel has a separate enable bit in the command byte and that you > were required to keep the state of those intact for each and every command > you sent. After looking at the datasheet, that is obviously not the case, > and the code is fine. But may I ask for a change in naming here? > > MCP41010_WIPER_CHANNEL instead of MCP41010_WIPER_ENABLE perhaps? > > Cheers, > Peter > I agree -- my naming for this is indeed ambiguous/confusing. I'll change it for v2. Thanks again, Chris
