On Sun, Jul 5, 2020 at 2:59 AM David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> 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".
It's a narrow set of cases if you assume an equal distribution of
expressions, but I'd bet you'd be hard pressed to really find examples
beyond (-literal).
But I guess at least the above examples could be supported. We'd have
to assume that if there's any subtraction, then types are signed and
we carry that thru. And perhaps give up and revert to unsigned if
there's anything more complex (signed values plus logic ops). In that
case, rather than tracking 'is_negative', we'd track is_signed and
then the output side would have to check that plus the data value to
output a negative.
> 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.
We could do that to some extent. The type is abstracted from the
binding and it just specifies 'int8' for example. That could be
internally set to { maximum: 0xff, minimum: 0 }. This works until you
want to further constrain the values for the specific bindings (e.g. {
enum: [ -1, -2, 127 ]}). We could process the schemas to find every
negative number and transform them to an unsigned value, but this gets
messy and fragile and is something I want to minimize.
> > 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.
That probably means we should have a schema type for [...], but so far
I haven't seen a need.
> > 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.
Okay, fine by me.
Rob
