Re: [PATCH] iio: potentiometer: Add driver for Microchip MCP41xxx/42xxx

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux