The DS442x DAC uses sign-magnitude encoding, so -128 cannot be represented
in hardware (7-bit magnitude).

Previously, passing -128 resulted in a truncated value that programmed
0mA (magnitude 0) instead of the expected maximum negative current,
effectively failing silently.

Reject -128 to avoid producing the wrong current.

Fixes: d632a2bd8ffc ("iio: dac: ds4422/ds4424 dac driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

---
changes v4:
- Restore v1 implementation: Keep this patch as a minimal fix suitable
  for stable backports
- Move the refactoring (bitwise operations and GENMASK usage) to a
  separate follow-up patch.
changes v3:
- (Merged into refactoring patch in v3, now split again)
---
 drivers/iio/dac/ds4424.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ds4424.c b/drivers/iio/dac/ds4424.c
index a8198ba4f98a..059acca45f64 100644
--- a/drivers/iio/dac/ds4424.c
+++ b/drivers/iio/dac/ds4424.c
@@ -141,7 +141,7 @@ static int ds4424_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (val < S8_MIN || val > S8_MAX)
+		if (val <= S8_MIN || val > S8_MAX)
 			return -EINVAL;
 
 		if (val > 0) {
-- 
2.47.3

On Tue, Feb 03, 2026 at 10:34:21AM +0100, Oleksij Rempel wrote:
> The DS442x DAC uses sign-magnitude encoding, so -128 cannot be represented
> in hardware (7-bit magnitude).
> 
> Previously, passing -128 resulted in a truncated value that programmed
> 0mA (magnitude 0) instead of the expected maximum negative current,
> effectively failing silently.
> 
> Reject -128 to avoid producing the wrong current.

...

>  	case IIO_CHAN_INFO_RAW:
> -		if (val < S8_MIN || val > S8_MAX)
> +		if (val <= S8_MIN || val > S8_MAX)
>  			return -EINVAL;

I still consider using -127, 127 is better than type _MIN/_MAX.
This is all due to '='.

-- 
With Best Regards,
Andy Shevchenko

On Tue, Feb 03, 2026 at 11:54:35AM +0200, Andy Shevchenko wrote:
> On Tue, Feb 03, 2026 at 10:34:21AM +0100, Oleksij Rempel wrote:
> > The DS442x DAC uses sign-magnitude encoding, so -128 cannot be represented
> > in hardware (7-bit magnitude).
> > 
> > Previously, passing -128 resulted in a truncated value that programmed
> > 0mA (magnitude 0) instead of the expected maximum negative current,
> > effectively failing silently.
> > 
> > Reject -128 to avoid producing the wrong current.
> 
> ...
> 
> >  	case IIO_CHAN_INFO_RAW:
> > -		if (val < S8_MIN || val > S8_MAX)
> > +		if (val <= S8_MIN || val > S8_MAX)
> >  			return -EINVAL;
> 
> I still consider using -127, 127 is better than type _MIN/_MAX.
> This is all due to '='.

The use of S8_MIN here is intentional to satisfy the requirement for a minimal
stable backport, as requested by Jonathan:
https://lore.kernel.org/all/20260201144226.218a43cb@jic23-huawei/

This patch: Strict "Fix only" for stable. Uses minimal logic changes (<=
S8_MIN) to avoid introducing new bugs during backporting.

N++ patch: Full refactoring.

Can we accept this temporary state to facilitate the stable process?

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

On Tue, Feb 03, 2026 at 11:28:45AM +0100, Oleksij Rempel wrote:
> On Tue, Feb 03, 2026 at 11:54:35AM +0200, Andy Shevchenko wrote:
> > On Tue, Feb 03, 2026 at 10:34:21AM +0100, Oleksij Rempel wrote:

...

> > >  	case IIO_CHAN_INFO_RAW:
> > > -		if (val < S8_MIN || val > S8_MAX)
> > > +		if (val <= S8_MIN || val > S8_MAX)
> > >  			return -EINVAL;
> > 
> > I still consider using -127, 127 is better than type _MIN/_MAX.
> > This is all due to '='.
> 
> The use of S8_MIN here is intentional to satisfy the requirement for a minimal
> stable backport, as requested by Jonathan:
> https://lore.kernel.org/all/20260201144226.218a43cb@jic23-huawei/
> 
> This patch: Strict "Fix only" for stable. Uses minimal logic changes (<=
> S8_MIN) to avoid introducing new bugs during backporting.
> 
> N++ patch: Full refactoring.
> 
> Can we accept this temporary state to facilitate the stable process?

Ah, if it's request by the maintainer, I can't and won't overrule it.

-- 
With Best Regards,
Andy Shevchenko

On Tue, 3 Feb 2026 13:52:59 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Tue, Feb 03, 2026 at 11:28:45AM +0100, Oleksij Rempel wrote:
> > On Tue, Feb 03, 2026 at 11:54:35AM +0200, Andy Shevchenko wrote:  
> > > On Tue, Feb 03, 2026 at 10:34:21AM +0100, Oleksij Rempel wrote:  
> 
> ...
> 
> > > >  	case IIO_CHAN_INFO_RAW:
> > > > -		if (val < S8_MIN || val > S8_MAX)
> > > > +		if (val <= S8_MIN || val > S8_MAX)
> > > >  			return -EINVAL;  
> > > 
> > > I still consider using -127, 127 is better than type _MIN/_MAX.
> > > This is all due to '='.  
> > 
> > The use of S8_MIN here is intentional to satisfy the requirement for a minimal
> > stable backport, as requested by Jonathan:
> > https://lore.kernel.org/all/20260201144226.218a43cb@jic23-huawei/
> > 
> > This patch: Strict "Fix only" for stable. Uses minimal logic changes (<=
> > S8_MIN) to avoid introducing new bugs during backporting.
> > 
> > N++ patch: Full refactoring.
> > 
> > Can we accept this temporary state to facilitate the stable process?  
> 
> Ah, if it's request by the maintainer, I can't and won't overrule it.

FWIW I didn't really feel strongly about the -127 vs <= S8_MIN
was more after a trivial backportable fix.  

Meh, it's temporary state for upstream (if not stable). Let's not worry about it.

Thanks

Jonathan

>