[OpenBIOS] [PATCH v3 3/5] Pretty-print reg property

Segher Boessenkool segher at kernel.crashing.org
Fri Nov 12 01:35:25 CET 2010


>>>>> +\ HELPER: get #size-cells value (from parent)
>>>>> +\ Legal values are 1..4 (we may optionally support larger sizes)
>>>>> +: my-#scells ( -- #size-cells )
>>>>> +  my-self ?dup if >in.device-node @ else active-package then
>>>>> +  ?dup if >dn.parent @ then
>>>>> +  ?dup if
>>>>> +    " #size-cells" rot get-package-property if 1 exit then
>>>
>>>>> +    \ we don't have to support more than 4 (and 0 is illegal)
>>>>> +    decode-int nip nip 4 min 1 max
>>>
>>> This seems wrong: 0 appears be a valid #size-cells value for
>>> device_type cpu.
>>
>> It is valid.  Why are you bounding the value here at all?
>
> Segher, thinking more about your question, are you referring to what
> became in v4 "0 max" (arguments are signed so this should still rule
> out -1, no?) or the general concept of limiting the value range here?

[Sorry for not replying earlier -- my mail went dead, and
then I forgot.]

I mean the "4 min", it makes no sense.  If something similar is
in the original code, it is a bug there as well.

It makes no sense to limit the number of address/size cells
unnecessary.  It is absolutely wrong to silently use a different
number, instead of failing, when the number requested is bigger
than implementation limits.

> I am planning to apply the series tomorrow unless I hear a strong
> objection. Optimizations could still be applied later.

This is not an optimisation; but you can incrementally fix it,
as far as I am concerned.  Ask someone else for an ack though.


Segher




More information about the OpenBIOS mailing list