Hi Walter, On Sun, 1 Aug 2021 at 20:45, Walter Lozano <wloz...@collabora.com> wrote: > > Hi Simon, > > Thanks for checking this bug, I'm glad that you were able to come with > fix quickly. I have some questions, I hope that you find some time to > help me understand. > > On 7/28/21 10:23 PM, Simon Glass wrote: > > The current name is confusing because the logic is actually backwards from > > what you might expect. Rename it to needs_widening() and update the > > comments. > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > --- > > > > tools/dtoc/fdt.py | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py > > index 3996971e39c..9749966d5fb 100644 > > --- a/tools/dtoc/fdt.py > > +++ b/tools/dtoc/fdt.py > > @@ -24,16 +24,19 @@ from patman import tools > > > > # A list of types we support > > class Type(IntEnum): > > + # Types in order from widest to narrowest > > (BYTE, INT, STRING, BOOL, INT64) = range(5) > > Sorry but I don't understand why BYTE is wider than INT (or INT64)
I think perhaps we need a better name. A wider type is one that can hold the values of a narrower one, plus more. In this case a 'bytes' type can hold anything (bytes, int, int64, bool) so is the 'widest' there is. It is the lowest common denominator in the devicetree. > > > > > - def is_wider_than(self, other): > > - """Check if another type is 'wider' than this one > > + def needs_widening(self, other): > > + """Check if this type needs widening to hold a value from another > > type > > > > - A wider type is one that holds more information than an earlier > > one, > > - similar to the concept of type-widening in C. > > + A wider type is one that can hold a wider array of information than > > + another one, or is less restrictive, so it can hold the > > information of > > + another type as well as its own. This is similar to the concept of > > + type-widening in C. > > > > This uses a simple arithmetic comparison, since type values are > > in order > > - from narrowest (BYTE) to widest (INT64). > > + from widest (BYTE) to narrowest (INT64). > > > > Args: > > other: Other type to compare against > > @@ -149,7 +152,7 @@ class Prop: > > update the current property to be like the second, since it is > > less > > specific. > > """ > > - if self.type.is_wider_than(newprop.type): > > + if self.type.needs_widening(newprop.type): > > if self.type == Type.INT and newprop.type == Type.BYTE: > > if type(self.value) == list: > > new_value = [] > Regards, Simon