Re: [PATCH 0/3] dtc: Preserve negative integers in yaml and dts output

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2765 bytes --]

On Fri, Jun 26, 2020 at 05:25:25PM +0100, Andrei Ziureaev wrote:
> Currently, in yaml output, negative values are indistinguishable from
> positive ones. Some bindings (for example
> Documentation/devicetree/bindings/iio/accel/lis302.txt) mention negative
> values. If those binding are converted to yaml, dt-schema validation
> wouldn't work with them.

> The first patch is a mechanical change and shouldn't affect dtc's
> behaviour.
> 
> The second one is the functional change. When applied, dts to dts and
> dts to yaml conversions preserve the '-' sign in front of integers.
> 
> For now, in dts parsing, only the unary '-' operator creates negative
> values (the other operators just set the 'is_negative' flag to false),
> but it should be easy to add support for other operators if needed.

Hmmmmm.  I'm really not convinced by this.  It seems like a lot of
fiddly work to preserve the sign only in a pretty narrow set of cases.
Basically it will only be reliably correct in the case of just
(-literal).  Something like (5-3) will still show as 0xfe rather than
-0x2, and if you do -(3-5) or (-3+5) you'll get the very strange
looking "-0xfe".

I also don't see why this is vital to schema validation.  The
validator should know the binding, and will therefore know which
values are signed, and can appropriately treat 0xfe and -2 (or
equivalent at whatever width) as the same.

> One issue is that there are two ways to format an array of bytes in dts:
> '/bits/ 8 <...>' and '[...]'.

So, these are intended for different purposes.  If you want to think
of your bytes as a bunch of 8-bit numbers, the former is appropriate.
The [ ... ] notation is more intended to be a compact way of putting
in "untyped" binary data.  For example, a blob of information which
gets verbatim copied into a device without interpretation by the
driver would make sense to use this form.

> Only the former supports negative
> integers. But, in dts output, only the latter is used. Therefore, I
> didn't include support for negative 8-bit values in dts output (I did
> add support for them in yaml output). Some possible alternatives are:
> 
> - switch to the '/bits/ 8 <>' format in dts output
> 
> - only switch to the '/bits/ 8 <>' format if the array contains negative
>   values
> 
> - add another marker to differentiate between the two formats

I'd be ok with this, given the distinction above.  This is similar to
putting different for markers for say "abc" and <0x61626300> which
likewise represent the same bytes.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]