[PATCH] iio: dac: mcp4725: add missing powerdown bits in store eeprom

[PATCH] iio: dac: mcp4725: add missing powerdown bits in store eeprom

[Jean-Francois Dagenais]

When issuing the write DAC register and write eeprom command, the two
powerdown bits (PD0 and PD1) are assumed by the chip to be present in
the bytes sent. Leaving them at 0 implies "powerdown disabled" which is
a different state that the current one. By adding the current state of
the powerdown in the i2c write, the chip will correctly power-on exactly
like as it is at the moment of store_eeprom call.

This is documented in MCP4725's datasheet, FIGURE 6-2: "Write Commands
for DAC Input Register and EEPROM" and MCP4726's datasheet, FIGURE 6-3:
"Write All Memory Command".

Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
---
 drivers/iio/dac/mcp4725.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index 6ab1f23e5a79..97bb350d1e77 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -98,6 +98,7 @@ static ssize_t mcp4725_store_eeprom(struct device *dev,
 
 	inoutbuf[0] = 0x60; /* write EEPROM */
 	inoutbuf[0] |= data->ref_mode << 3;
+	inoutbuf[0] |= data->powerdown ? ((data->powerdown_mode+1) << 1) : 0;
 	inoutbuf[1] = data->dac_value >> 4;
 	inoutbuf[2] = (data->dac_value & 0xf) << 4;
 
-- 
2.11.0

Re: [PATCH] iio: dac: mcp4725: add missing powerdown bits in store eeprom

[Jonathan Cameron]

+Cc Peter.
On Wed,  6 Mar 2019 15:56:06 -0500
Jean-Francois Dagenais <jeff.dagenais@gmail.com> wrote:

> When issuing the write DAC register and write eeprom command, the two
> powerdown bits (PD0 and PD1) are assumed by the chip to be present in
> the bytes sent. Leaving them at 0 implies "powerdown disabled" which is
> a different state that the current one. By adding the current state of
> the powerdown in the i2c write, the chip will correctly power-on exactly
> like as it is at the moment of store_eeprom call.
> 
> This is documented in MCP4725's datasheet, FIGURE 6-2: "Write Commands
> for DAC Input Register and EEPROM" and MCP4726's datasheet, FIGURE 6-3:
> "Write All Memory Command".
> 
> Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>
sounds fine to me, but I'd like to give Peter a chance to comment.
Give me a poke if it turns out Peter is busy and doesn't get back
to us in a week or two.

Thanks,

Jonathan

> ---
>  drivers/iio/dac/mcp4725.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
> index 6ab1f23e5a79..97bb350d1e77 100644
> --- a/drivers/iio/dac/mcp4725.c
> +++ b/drivers/iio/dac/mcp4725.c
> @@ -98,6 +98,7 @@ static ssize_t mcp4725_store_eeprom(struct device *dev,
>  
>  	inoutbuf[0] = 0x60; /* write EEPROM */
>  	inoutbuf[0] |= data->ref_mode << 3;
> +	inoutbuf[0] |= data->powerdown ? ((data->powerdown_mode+1) << 1) : 0;
>  	inoutbuf[1] = data->dac_value >> 4;
>  	inoutbuf[2] = (data->dac_value & 0xf) << 4;
>  

Re: [PATCH] iio: dac: mcp4725: add missing powerdown bits in store eeprom

[Peter Meerwald-Stadler]

> +Cc Peter.
 
> > When issuing the write DAC register and write eeprom command, the two
> > powerdown bits (PD0 and PD1) are assumed by the chip to be present in
> > the bytes sent. Leaving them at 0 implies "powerdown disabled" which is
> > a different state that the current one. By adding the current state of
> > the powerdown in the i2c write, the chip will correctly power-on exactly
> > like as it is at the moment of store_eeprom call.
> > 
> > This is documented in MCP4725's datasheet, FIGURE 6-2: "Write Commands
> > for DAC Input Register and EEPROM" and MCP4726's datasheet, FIGURE 6-3:
> > "Write All Memory Command".
> > 
> > Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>

> sounds fine to me, but I'd like to give Peter a chance to comment.
> Give me a poke if it turns out Peter is busy and doesn't get back
> to us in a week or two.

I'd suggest whitespace around the + operator, otherwise

Acked-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>

> > ---
> >  drivers/iio/dac/mcp4725.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
> > index 6ab1f23e5a79..97bb350d1e77 100644
> > --- a/drivers/iio/dac/mcp4725.c
> > +++ b/drivers/iio/dac/mcp4725.c
> > @@ -98,6 +98,7 @@ static ssize_t mcp4725_store_eeprom(struct device *dev,
> >  
> >  	inoutbuf[0] = 0x60; /* write EEPROM */
> >  	inoutbuf[0] |= data->ref_mode << 3;
> > +	inoutbuf[0] |= data->powerdown ? ((data->powerdown_mode+1) << 1) : 0;
> >  	inoutbuf[1] = data->dac_value >> 4;
> >  	inoutbuf[2] = (data->dac_value & 0xf) << 4;
> >  
> 

-- 

Peter Meerwald-Stadler
Mobile: +43 664 24 44 418

Re: [PATCH] iio: dac: mcp4725: add missing powerdown bits in store eeprom

[Jonathan Cameron]

On Sun, 10 Mar 2019 12:31:16 +0100 (CET)
Peter Meerwald-Stadler <pmeerw@pmeerw.net> wrote:

> > +Cc Peter.  
>  
> > > When issuing the write DAC register and write eeprom command, the two
> > > powerdown bits (PD0 and PD1) are assumed by the chip to be present in
> > > the bytes sent. Leaving them at 0 implies "powerdown disabled" which is
> > > a different state that the current one. By adding the current state of
> > > the powerdown in the i2c write, the chip will correctly power-on exactly
> > > like as it is at the moment of store_eeprom call.
> > > 
> > > This is documented in MCP4725's datasheet, FIGURE 6-2: "Write Commands
> > > for DAC Input Register and EEPROM" and MCP4726's datasheet, FIGURE 6-3:
> > > "Write All Memory Command".
> > > 
> > > Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com>  
> 
> > sounds fine to me, but I'd like to give Peter a chance to comment.
> > Give me a poke if it turns out Peter is busy and doesn't get back
> > to us in a week or two.  
> 
> I'd suggest whitespace around the + operator, otherwise
> 
Good point, added...
> Acked-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>

Applied to the fixes-togreg branch of iio.git and marked
for stable. It's not super critical as fixes go but
things are currently not quite doing what someone might
expect.

Thanks,

Jonathan

> 
> > > ---
> > >  drivers/iio/dac/mcp4725.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
> > > index 6ab1f23e5a79..97bb350d1e77 100644
> > > --- a/drivers/iio/dac/mcp4725.c
> > > +++ b/drivers/iio/dac/mcp4725.c
> > > @@ -98,6 +98,7 @@ static ssize_t mcp4725_store_eeprom(struct device *dev,
> > >  
> > >  	inoutbuf[0] = 0x60; /* write EEPROM */
> > >  	inoutbuf[0] |= data->ref_mode << 3;
> > > +	inoutbuf[0] |= data->powerdown ? ((data->powerdown_mode+1) << 1) : 0;
> > >  	inoutbuf[1] = data->dac_value >> 4;
> > >  	inoutbuf[2] = (data->dac_value & 0xf) << 4;
> > >    
> >   
>