On Fri, Jan 23, 2026 at 09:33:57AM +0000, Jonathan Cameron wrote:

On Mon, 19 Jan 2026 21:03:46 +0200
Andy Shevchenko [off-list ref] wrote:

quoted

On Mon, Jan 19, 2026 at 07:24:20PM +0100, Oleksij Rempel wrote:

quoted

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

With the previous check, userspace could pass -128, which gets converted
to a magnitude of 128 and then truncated by the 7-bit DAC field. This
ends up programming a zero magnitude with the sign bit set, i.e. an
unintended output (effectively 0 mA instead of -128 steps).

Reject -128 to avoid silently producing the wrong current.  

...

quoted

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

Hmm... So the range is [ -127 .. 0 .. 127 ] ?

I think in such case the plain numbers would be more specific than
the type related limits.

Check the abs(val) <= 127 given that's what we care about I think?
Or make it explicit and do
FIELD_FIT() against a mask that you then use to fill the register
value (another mask for the sign bit).

Btw use abs(val) to set raw.dx and drop it out of the conditional.
Even better get rid of the bitfield stuff and just add
two defines + fill val directly in this function using FIELD_PREP().
Then both the checking and the field filling use the same defines
and it should be easy to see what is going on.

FIELD_* macros require compile-time constant masks. Since the next patch
adds support for variants with different data widths (making the mask a
runtime variable), I prefer using an implementation now that remains
consistent with the followup changes.

-- 
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 |