On Sun, Dec 13, 2020 at 8:59 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Wed, 9 Dec 2020 11:46:49 +0100
> Lars-Peter Clausen <lars@xxxxxxxxxx> wrote:
>
> > The power-down mask of the ad5504 is actually a power-up mask. Meaning if
> > a bit is set the corresponding channel is powered up and if it is not set
> > the channel is powered down.
> >
> > The driver currently has this the wrong way around, resulting in the
> > channel being powered up when requested to be powered down and vice versa.
> >
> > Fixes: 3bbbf150ffde ("staging:iio:dac:ad5504: Use strtobool for boolean values")
> > Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> > ---
> > Stumbled upon this while looking at something else. It is untested, but I
> > think it should be right. Alex can you double check?
>
> I read the datasheet as saying this fix is right but will wait for
> a confirmation from Alex.
Oh.
I see why the initial confusion existed.
Was confused a bit myself.
The thermal shutdown follows the initial/unchanged logic.
While the actual powerdown channel bits are inverted.
Acked-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/iio/dac/ad5504.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
> > index 28921b62e642..e9297c25d4ef 100644
> > --- a/drivers/iio/dac/ad5504.c
> > +++ b/drivers/iio/dac/ad5504.c
> > @@ -187,9 +187,9 @@ static ssize_t ad5504_write_dac_powerdown(struct iio_dev *indio_dev,
> > return ret;
> >
> > if (pwr_down)
> > - st->pwr_down_mask |= (1 << chan->channel);
> > - else
> > st->pwr_down_mask &= ~(1 << chan->channel);
> > + else
> > + st->pwr_down_mask |= (1 << chan->channel);
> >
> > ret = ad5504_spi_write(st, AD5504_ADDR_CTRL,
> > AD5504_DAC_PWRDWN_MODE(st->pwr_down_mode) |
>
