* [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
2021-07-29 1:23 [PATCH 0/3] dtoc: Improve support for 'ranges' property Simon Glass
@ 2021-07-29 1:23 ` Simon Glass
2021-08-02 2:45 ` Walter Lozano
2021-07-29 1:23 ` [PATCH 2/3] dtoc: Fix widening an int array to an int Simon Glass
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-07-29 1:23 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass
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 <sjg@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)
- 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 = []
--
2.32.0.432.gabb21c7263-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
2021-07-29 1:23 ` [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion Simon Glass
@ 2021-08-02 2:45 ` Walter Lozano
2021-08-02 2:50 ` Simon Glass
0 siblings, 1 reply; 14+ messages in thread
From: Walter Lozano @ 2021-08-02 2:45 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Tom Rini
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 <sjg@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)
>
> - 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,
Walter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
2021-08-02 2:45 ` Walter Lozano
@ 2021-08-02 2:50 ` Simon Glass
2021-08-02 19:28 ` Walter Lozano
0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-08-02 2:50 UTC (permalink / raw)
To: Walter Lozano; +Cc: U-Boot Mailing List, Tom Rini
Hi Walter,
On Sun, 1 Aug 2021 at 20:45, Walter Lozano <wlozano@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 <sjg@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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
2021-08-02 2:50 ` Simon Glass
@ 2021-08-02 19:28 ` Walter Lozano
2021-11-25 0:12 ` Simon Glass
0 siblings, 1 reply; 14+ messages in thread
From: Walter Lozano @ 2021-08-02 19:28 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini
Hi Simon,
On 8/1/21 11:50 PM, Simon Glass wrote:
> Hi Walter,
>
> On Sun, 1 Aug 2021 at 20:45, Walter Lozano <wlozano@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 <sjg@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.
Thanks for taking the time to explain. I understand the idea behind your
explanation but I still not sure if I follow you completely. In any
case, let me add a few words in order to be more clear.
It is my impression that when you say 'bytes' (and not BYTE like in the
declaration) you are referring to a list. Is that the case?
If not, BYTE (8 bit) seems to be narrower than INT (32 bits), isn't it?
Also, another example is INT, BOOL and INT64. It is clear that INT is
wider than BOOL, but why BOOL is wider than INT64?
As reference I have been checking
https://devicetree-specification.readthedocs.io/en/stable/devicetree-basics.html#property-values
>>> - 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
Regards,
Walter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
2021-08-02 19:28 ` Walter Lozano
@ 2021-11-25 0:12 ` Simon Glass
2021-12-07 19:13 ` Walter Lozano
0 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-11-25 0:12 UTC (permalink / raw)
To: Walter Lozano; +Cc: U-Boot Mailing List, Tom Rini
Hi Walter,
On Mon, 2 Aug 2021 at 13:29, Walter Lozano <wlozano@collabora.com> wrote:
>
> Hi Simon,
>
> On 8/1/21 11:50 PM, Simon Glass wrote:
> > Hi Walter,
> >
> > On Sun, 1 Aug 2021 at 20:45, Walter Lozano <wlozano@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 <sjg@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.
>
> Thanks for taking the time to explain. I understand the idea behind your
> explanation but I still not sure if I follow you completely. In any
> case, let me add a few words in order to be more clear.
>
> It is my impression that when you say 'bytes' (and not BYTE like in the
> declaration) you are referring to a list. Is that the case?
>
> If not, BYTE (8 bit) seems to be narrower than INT (32 bits), isn't it?
>
A bit long in finding this email again...
Actually bytes means the Python bytes type which holds a sequence of
bytes. So it can hold an int, whereas an int cannot hold a bytes
value, in general (although it could if it happened to be 4 bytes).
> Also, another example is INT, BOOL and INT64. It is clear that INT is
> wider than BOOL, but why BOOL is wider than INT64?
It isn't actually, but INT64 is a bit of a special case and I had to
put it somewhere.
>
> As reference I have been checking
>
> https://devicetree-specification.readthedocs.io/en/stable/devicetree-basics.html#property-values
[..]
Regards,
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
2021-11-25 0:12 ` Simon Glass
@ 2021-12-07 19:13 ` Walter Lozano
0 siblings, 0 replies; 14+ messages in thread
From: Walter Lozano @ 2021-12-07 19:13 UTC (permalink / raw)
To: Simon Glass, Walter Lozano; +Cc: U-Boot Mailing List, Tom Rini
Hi Simon,
On 11/24/21 21:12, Simon Glass wrote:
> Hi Walter,
>
> On Mon, 2 Aug 2021 at 13:29, Walter Lozano <wlozano@collabora.com> wrote:
>> Hi Simon,
>>
>> On 8/1/21 11:50 PM, Simon Glass wrote:
>>> Hi Walter,
>>>
>>> On Sun, 1 Aug 2021 at 20:45, Walter Lozano <wlozano@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 <sjg@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.
>> Thanks for taking the time to explain. I understand the idea behind your
>> explanation but I still not sure if I follow you completely. In any
>> case, let me add a few words in order to be more clear.
>>
>> It is my impression that when you say 'bytes' (and not BYTE like in the
>> declaration) you are referring to a list. Is that the case?
>>
>> If not, BYTE (8 bit) seems to be narrower than INT (32 bits), isn't it?
>>
> A bit long in finding this email again...
>
> Actually bytes means the Python bytes type which holds a sequence of
> bytes. So it can hold an int, whereas an int cannot hold a bytes
> value, in general (although it could if it happened to be 4 bytes).
It is more clear now, thank you.
>
>> Also, another example is INT, BOOL and INT64. It is clear that INT is
>> wider than BOOL, but why BOOL is wider than INT64?
> It isn't actually, but INT64 is a bit of a special case and I had to
> put it somewhere.
I see, thank you for the explanation!
>
> As reference I have been checking
>
> https://devicetree-specification.readthedocs.io/en/stable/devicetree-basics.html#property-values
> [..]
Regards,
Walter
--
Walter Lozano
Collabora Ltd.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] dtoc: Fix widening an int array to an int
2021-07-29 1:23 [PATCH 0/3] dtoc: Improve support for 'ranges' property Simon Glass
2021-07-29 1:23 ` [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion Simon Glass
@ 2021-07-29 1:23 ` Simon Glass
2021-08-02 2:45 ` Walter Lozano
2021-07-29 1:23 ` [PATCH 3/3] dtoc: Support widening a bool value Simon Glass
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Simon Glass @ 2021-07-29 1:23 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Tom Rini, Simon Glass, Walter Lozano
An int array can hold a single int so we should not need to do anything
in the widening operation. However due to a quirk in the code, an int[3]
widened with an int produced an int[4]. Fix this and add a test.
Fix a comment typo while we are here.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reported-by: Tom Rini <trini@konsulko.com>
---
test/dm/of_platdata.c | 4 +---
tools/dtoc/fdt.py | 15 ++++++++-------
tools/dtoc/test_dtoc.py | 6 +++---
tools/dtoc/test_fdt.py | 11 ++++++++++-
4 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
index 0f89c7a7da8..e3fa01afddf 100644
--- a/test/dm/of_platdata.c
+++ b/test/dm/of_platdata.c
@@ -35,11 +35,10 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
plat = dev_get_plat(dev);
ut_assert(plat->boolval);
ut_asserteq(1, plat->intval);
- ut_asserteq(4, ARRAY_SIZE(plat->intarray));
+ ut_asserteq(3, ARRAY_SIZE(plat->intarray));
ut_asserteq(2, plat->intarray[0]);
ut_asserteq(3, plat->intarray[1]);
ut_asserteq(4, plat->intarray[2]);
- ut_asserteq(0, plat->intarray[3]);
ut_asserteq(5, plat->byteval);
ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
ut_asserteq(6, plat->bytearray[0]);
@@ -61,7 +60,6 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
ut_asserteq(5, plat->intarray[0]);
ut_asserteq(0, plat->intarray[1]);
ut_asserteq(0, plat->intarray[2]);
- ut_asserteq(0, plat->intarray[3]);
ut_asserteq(8, plat->byteval);
ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
ut_asserteq(1, plat->bytearray[0]);
diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index 9749966d5fb..429e95f9a96 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -163,13 +163,14 @@ class Prop:
self.value = new_value
self.type = newprop.type
- if type(newprop.value) == list and type(self.value) != list:
- self.value = [self.value]
-
- if type(self.value) == list and len(newprop.value) > len(self.value):
- val = self.GetEmpty(self.type)
- while len(self.value) < len(newprop.value):
- self.value.append(val)
+ if type(newprop.value) == list:
+ if type(self.value) != list:
+ self.value = [self.value]
+
+ if len(newprop.value) > len(self.value):
+ val = self.GetEmpty(self.type)
+ while len(self.value) < len(newprop.value):
+ self.value.append(val)
@classmethod
def GetEmpty(self, type):
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index 863ede90b7a..44d5d0c354a 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -296,7 +296,7 @@ struct dtd_sandbox_spl_test {
\tbool\t\tboolval;
\tunsigned char\tbytearray[3];
\tunsigned char\tbyteval;
-\tfdt32_t\t\tintarray[4];
+\tfdt32_t\t\tintarray[3];
\tfdt32_t\t\tintval;
\tunsigned char\tlongbytearray[9];
\tunsigned char\tnotstring[5];
@@ -354,7 +354,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test = {
\t.boolval\t\t= true,
\t.bytearray\t\t= {0x6, 0x0, 0x0},
\t.byteval\t\t= 0x5,
-\t.intarray\t\t= {0x2, 0x3, 0x4, 0x0},
+\t.intarray\t\t= {0x2, 0x3, 0x4},
\t.intval\t\t\t= 0x1,
\t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10,
\t\t0x11},
@@ -377,7 +377,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test2 = {
\t.acpi_name\t\t= "\\\\_SB.GPO0",
\t.bytearray\t\t= {0x1, 0x23, 0x34},
\t.byteval\t\t= 0x8,
-\t.intarray\t\t= {0x5, 0x0, 0x0, 0x0},
+\t.intarray\t\t= {0x5, 0x0, 0x0},
\t.intval\t\t\t= 0x3,
\t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0x0, 0x0, 0x0, 0x0,
\t\t0x0},
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index 856392b1bd9..857861c14ed 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -379,7 +379,7 @@ class TestProp(unittest.TestCase):
self.assertEqual(Type.INT, prop.type)
self.assertEqual(1, fdt32_to_cpu(prop.value))
- # Convert singla value to array
+ # Convert single value to array
prop2 = self.node.props['intarray']
prop.Widen(prop2)
self.assertEqual(Type.INT, prop.type)
@@ -422,6 +422,15 @@ class TestProp(unittest.TestCase):
self.assertTrue(isinstance(prop.value, list))
self.assertEqual(3, len(prop.value))
+ # Widen an array of ints with an int (should do nothing)
+ prop = self.node.props['intarray']
+ prop2 = node2.props['intarray']
+ self.assertEqual(Type.INT, prop.type)
+ self.assertEqual(3, len(prop.value))
+ prop.Widen(prop2)
+ self.assertEqual(Type.INT, prop.type)
+ self.assertEqual(3, len(prop.value))
+
def testAdd(self):
"""Test adding properties"""
self.fdt.pack()
--
2.32.0.432.gabb21c7263-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] dtoc: Fix widening an int array to an int
2021-07-29 1:23 ` [PATCH 2/3] dtoc: Fix widening an int array to an int Simon Glass
@ 2021-08-02 2:45 ` Walter Lozano
2021-08-02 2:50 ` Simon Glass
0 siblings, 1 reply; 14+ messages in thread
From: Walter Lozano @ 2021-08-02 2:45 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Tom Rini, Walter Lozano
Hi Simon,
I know you already merged this one but I have some doubts I would like
to check with you.
On 7/28/21 10:23 PM, Simon Glass wrote:
> An int array can hold a single int so we should not need to do anything
> in the widening operation. However due to a quirk in the code, an int[3]
> widened with an int produced an int[4]. Fix this and add a test.
>
> Fix a comment typo while we are here.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reported-by: Tom Rini <trini@konsulko.com>
> ---
>
> test/dm/of_platdata.c | 4 +---
> tools/dtoc/fdt.py | 15 ++++++++-------
> tools/dtoc/test_dtoc.py | 6 +++---
> tools/dtoc/test_fdt.py | 11 ++++++++++-
> 4 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
> index 0f89c7a7da8..e3fa01afddf 100644
> --- a/test/dm/of_platdata.c
> +++ b/test/dm/of_platdata.c
> @@ -35,11 +35,10 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
> plat = dev_get_plat(dev);
> ut_assert(plat->boolval);
> ut_asserteq(1, plat->intval);
> - ut_asserteq(4, ARRAY_SIZE(plat->intarray));
> + ut_asserteq(3, ARRAY_SIZE(plat->intarray));
> ut_asserteq(2, plat->intarray[0]);
> ut_asserteq(3, plat->intarray[1]);
> ut_asserteq(4, plat->intarray[2]);
> - ut_asserteq(0, plat->intarray[3]);
> ut_asserteq(5, plat->byteval);
> ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
> ut_asserteq(6, plat->bytearray[0]);
> @@ -61,7 +60,6 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
> ut_asserteq(5, plat->intarray[0]);
> ut_asserteq(0, plat->intarray[1]);
> ut_asserteq(0, plat->intarray[2]);
> - ut_asserteq(0, plat->intarray[3]);
> ut_asserteq(8, plat->byteval);
> ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
> ut_asserteq(1, plat->bytearray[0]);
> diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
> index 9749966d5fb..429e95f9a96 100644
> --- a/tools/dtoc/fdt.py
> +++ b/tools/dtoc/fdt.py
> @@ -163,13 +163,14 @@ class Prop:
> self.value = new_value
> self.type = newprop.type
>
> - if type(newprop.value) == list and type(self.value) != list:
> - self.value = [self.value]
> -
> - if type(self.value) == list and len(newprop.value) > len(self.value):
> - val = self.GetEmpty(self.type)
> - while len(self.value) < len(newprop.value):
> - self.value.append(val)
> + if type(newprop.value) == list:
> + if type(self.value) != list:
> + self.value = [self.value]
> +
> + if len(newprop.value) > len(self.value):
> + val = self.GetEmpty(self.type)
> + while len(self.value) < len(newprop.value):
> + self.value.append(val)
>
> @classmethod
> def GetEmpty(self, type):
> diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
> index 863ede90b7a..44d5d0c354a 100755
> --- a/tools/dtoc/test_dtoc.py
> +++ b/tools/dtoc/test_dtoc.py
> @@ -296,7 +296,7 @@ struct dtd_sandbox_spl_test {
> \tbool\t\tboolval;
> \tunsigned char\tbytearray[3];
> \tunsigned char\tbyteval;
> -\tfdt32_t\t\tintarray[4];
> +\tfdt32_t\t\tintarray[3];
> \tfdt32_t\t\tintval;
> \tunsigned char\tlongbytearray[9];
> \tunsigned char\tnotstring[5];
> @@ -354,7 +354,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test = {
> \t.boolval\t\t= true,
> \t.bytearray\t\t= {0x6, 0x0, 0x0},
> \t.byteval\t\t= 0x5,
> -\t.intarray\t\t= {0x2, 0x3, 0x4, 0x0},
> +\t.intarray\t\t= {0x2, 0x3, 0x4},
> \t.intval\t\t\t= 0x1,
> \t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10,
> \t\t0x11},
> @@ -377,7 +377,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test2 = {
> \t.acpi_name\t\t= "\\\\_SB.GPO0",
> \t.bytearray\t\t= {0x1, 0x23, 0x34},
> \t.byteval\t\t= 0x8,
> -\t.intarray\t\t= {0x5, 0x0, 0x0, 0x0},
> +\t.intarray\t\t= {0x5, 0x0, 0x0},
> \t.intval\t\t\t= 0x3,
> \t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0x0, 0x0, 0x0, 0x0,
> \t\t0x0},
> diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
> index 856392b1bd9..857861c14ed 100755
> --- a/tools/dtoc/test_fdt.py
> +++ b/tools/dtoc/test_fdt.py
> @@ -379,7 +379,7 @@ class TestProp(unittest.TestCase):
> self.assertEqual(Type.INT, prop.type)
> self.assertEqual(1, fdt32_to_cpu(prop.value))
>
> - # Convert singla value to array
> + # Convert single value to array
> prop2 = self.node.props['intarray']
> prop.Widen(prop2)
> self.assertEqual(Type.INT, prop.type)
> @@ -422,6 +422,15 @@ class TestProp(unittest.TestCase):
> self.assertTrue(isinstance(prop.value, list))
> self.assertEqual(3, len(prop.value))
>
> + # Widen an array of ints with an int (should do nothing)
> + prop = self.node.props['intarray']
> + prop2 = node2.props['intarray']
I was expecting intval instead of intarray here.
> + self.assertEqual(Type.INT, prop.type)
> + self.assertEqual(3, len(prop.value))
> + prop.Widen(prop2)
> + self.assertEqual(Type.INT, prop.type)
> + self.assertEqual(3, len(prop.value))
> +
> def testAdd(self):
> """Test adding properties"""
> self.fdt.pack()
Regards,
Walter
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] dtoc: Fix widening an int array to an int
2021-08-02 2:45 ` Walter Lozano
@ 2021-08-02 2:50 ` Simon Glass
0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-08-02 2:50 UTC (permalink / raw)
To: Walter Lozano; +Cc: U-Boot Mailing List, Tom Rini, Walter Lozano
Hi Walter,
On Sun, 1 Aug 2021 at 20:45, Walter Lozano <wlozano@collabora.com> wrote:
>
> Hi Simon,
>
> I know you already merged this one but I have some doubts I would like
> to check with you.
>
> On 7/28/21 10:23 PM, Simon Glass wrote:
> > An int array can hold a single int so we should not need to do anything
> > in the widening operation. However due to a quirk in the code, an int[3]
> > widened with an int produced an int[4]. Fix this and add a test.
> >
> > Fix a comment typo while we are here.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Reported-by: Tom Rini <trini@konsulko.com>
> > ---
> >
> > test/dm/of_platdata.c | 4 +---
> > tools/dtoc/fdt.py | 15 ++++++++-------
> > tools/dtoc/test_dtoc.py | 6 +++---
> > tools/dtoc/test_fdt.py | 11 ++++++++++-
> > 4 files changed, 22 insertions(+), 14 deletions(-)
> >
> > diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
> > index 0f89c7a7da8..e3fa01afddf 100644
> > --- a/test/dm/of_platdata.c
> > +++ b/test/dm/of_platdata.c
> > @@ -35,11 +35,10 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
> > plat = dev_get_plat(dev);
> > ut_assert(plat->boolval);
> > ut_asserteq(1, plat->intval);
> > - ut_asserteq(4, ARRAY_SIZE(plat->intarray));
> > + ut_asserteq(3, ARRAY_SIZE(plat->intarray));
> > ut_asserteq(2, plat->intarray[0]);
> > ut_asserteq(3, plat->intarray[1]);
> > ut_asserteq(4, plat->intarray[2]);
> > - ut_asserteq(0, plat->intarray[3]);
> > ut_asserteq(5, plat->byteval);
> > ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
> > ut_asserteq(6, plat->bytearray[0]);
> > @@ -61,7 +60,6 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
> > ut_asserteq(5, plat->intarray[0]);
> > ut_asserteq(0, plat->intarray[1]);
> > ut_asserteq(0, plat->intarray[2]);
> > - ut_asserteq(0, plat->intarray[3]);
> > ut_asserteq(8, plat->byteval);
> > ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
> > ut_asserteq(1, plat->bytearray[0]);
> > diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
> > index 9749966d5fb..429e95f9a96 100644
> > --- a/tools/dtoc/fdt.py
> > +++ b/tools/dtoc/fdt.py
> > @@ -163,13 +163,14 @@ class Prop:
> > self.value = new_value
> > self.type = newprop.type
> >
> > - if type(newprop.value) == list and type(self.value) != list:
> > - self.value = [self.value]
> > -
> > - if type(self.value) == list and len(newprop.value) > len(self.value):
> > - val = self.GetEmpty(self.type)
> > - while len(self.value) < len(newprop.value):
> > - self.value.append(val)
> > + if type(newprop.value) == list:
> > + if type(self.value) != list:
> > + self.value = [self.value]
> > +
> > + if len(newprop.value) > len(self.value):
> > + val = self.GetEmpty(self.type)
> > + while len(self.value) < len(newprop.value):
> > + self.value.append(val)
> >
> > @classmethod
> > def GetEmpty(self, type):
> > diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
> > index 863ede90b7a..44d5d0c354a 100755
> > --- a/tools/dtoc/test_dtoc.py
> > +++ b/tools/dtoc/test_dtoc.py
> > @@ -296,7 +296,7 @@ struct dtd_sandbox_spl_test {
> > \tbool\t\tboolval;
> > \tunsigned char\tbytearray[3];
> > \tunsigned char\tbyteval;
> > -\tfdt32_t\t\tintarray[4];
> > +\tfdt32_t\t\tintarray[3];
> > \tfdt32_t\t\tintval;
> > \tunsigned char\tlongbytearray[9];
> > \tunsigned char\tnotstring[5];
> > @@ -354,7 +354,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test = {
> > \t.boolval\t\t= true,
> > \t.bytearray\t\t= {0x6, 0x0, 0x0},
> > \t.byteval\t\t= 0x5,
> > -\t.intarray\t\t= {0x2, 0x3, 0x4, 0x0},
> > +\t.intarray\t\t= {0x2, 0x3, 0x4},
> > \t.intval\t\t\t= 0x1,
> > \t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10,
> > \t\t0x11},
> > @@ -377,7 +377,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test2 = {
> > \t.acpi_name\t\t= "\\\\_SB.GPO0",
> > \t.bytearray\t\t= {0x1, 0x23, 0x34},
> > \t.byteval\t\t= 0x8,
> > -\t.intarray\t\t= {0x5, 0x0, 0x0, 0x0},
> > +\t.intarray\t\t= {0x5, 0x0, 0x0},
> > \t.intval\t\t\t= 0x3,
> > \t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0x0, 0x0, 0x0, 0x0,
> > \t\t0x0},
> > diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
> > index 856392b1bd9..857861c14ed 100755
> > --- a/tools/dtoc/test_fdt.py
> > +++ b/tools/dtoc/test_fdt.py
> > @@ -379,7 +379,7 @@ class TestProp(unittest.TestCase):
> > self.assertEqual(Type.INT, prop.type)
> > self.assertEqual(1, fdt32_to_cpu(prop.value))
> >
> > - # Convert singla value to array
> > + # Convert single value to array
> > prop2 = self.node.props['intarray']
> > prop.Widen(prop2)
> > self.assertEqual(Type.INT, prop.type)
> > @@ -422,6 +422,15 @@ class TestProp(unittest.TestCase):
> > self.assertTrue(isinstance(prop.value, list))
> > self.assertEqual(3, len(prop.value))
> >
> > + # Widen an array of ints with an int (should do nothing)
> > + prop = self.node.props['intarray']
> > + prop2 = node2.props['intarray']
>
> I was expecting intval instead of intarray here.
Yes that is a bug, thanks for spotting it. It isn't testing what it
should be. I'll come up with a fix.
>
> > + self.assertEqual(Type.INT, prop.type)
> > + self.assertEqual(3, len(prop.value))
> > + prop.Widen(prop2)
> > + self.assertEqual(Type.INT, prop.type)
> > + self.assertEqual(3, len(prop.value))
> > +
> > def testAdd(self):
> > """Test adding properties"""
> > self.fdt.pack()
>
>
Regards,
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] dtoc: Support widening a bool value
2021-07-29 1:23 [PATCH 0/3] dtoc: Improve support for 'ranges' property Simon Glass
2021-07-29 1:23 ` [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion Simon Glass
2021-07-29 1:23 ` [PATCH 2/3] dtoc: Fix widening an int array to an int Simon Glass
@ 2021-07-29 1:23 ` Simon Glass
2021-07-31 23:03 ` Simon Glass
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-07-29 1:23 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Simon Glass, Heiko Schocher, Heinrich Schuchardt,
Philippe Reynes, Vincent Stehlé,
Walter Lozano
At present if we see 'ranges' property (with no value) we assume it is a
boolean, as per the devicetree spec.
But another node may define 'ranges' with a value, forcing us to widen it
to an int array. At present this is not supported and causes an error.
Fix this and add some test cases.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reported-by: Tom Rini <trini@konsulko.com>
---
arch/sandbox/dts/sandbox.dtsi | 2 ++
test/dm/of_platdata.c | 3 +++
tools/dtoc/fdt.py | 12 ++++++++++++
tools/dtoc/test/dtoc_test_simple.dts | 2 ++
tools/dtoc/test_dtoc.py | 3 +++
tools/dtoc/test_fdt.py | 18 ++++++++++++++++--
6 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index 31db50db352..200fcab6a41 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -231,6 +231,7 @@
boolval;
intval = <1>;
intarray = <2 3 4>;
+ maybe-empty-int = <>;
byteval = [05];
bytearray = [06];
longbytearray = [09 0a 0b 0c 0d 0e 0f 10 11];
@@ -254,6 +255,7 @@
u-boot,dm-pre-reloc;
compatible = "sandbox,spl-test";
stringarray = "one";
+ maybe-empty-int = <1>;
};
spl-test5 {
diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
index e3fa01afddf..0463cf0b433 100644
--- a/test/dm/of_platdata.c
+++ b/test/dm/of_platdata.c
@@ -40,6 +40,8 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
ut_asserteq(3, plat->intarray[1]);
ut_asserteq(4, plat->intarray[2]);
ut_asserteq(5, plat->byteval);
+ ut_asserteq(1, ARRAY_SIZE(plat->maybe_empty_int));
+ ut_asserteq(0, plat->maybe_empty_int[0]);
ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
ut_asserteq(6, plat->bytearray[0]);
ut_asserteq(0, plat->bytearray[1]);
@@ -78,6 +80,7 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
ut_asserteq_str("one", plat->stringarray[0]);
ut_asserteq_str("", plat->stringarray[1]);
ut_asserteq_str("", plat->stringarray[2]);
+ ut_asserteq(1, plat->maybe_empty_int[0]);
ut_assertok(uclass_next_device_err(&dev));
plat = dev_get_plat(dev);
diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index 429e95f9a96..32a7aa98290 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -153,6 +153,18 @@ class Prop:
specific.
"""
if self.type.needs_widening(newprop.type):
+
+ # A boolean has an empty value: if it exists it is True and if not
+ # it is False. So when widening we always start with an empty list
+ # since the only valid integer property would be an empty list of
+ # integers.
+ # e.g. this is a boolean:
+ # some-prop;
+ # and it would be widened to int list by:
+ # some-prop = <1 2>;
+ if self.type == Type.BOOL:
+ self.type = Type.INT
+ self.value = [self.GetEmpty(self.type)]
if self.type == Type.INT and newprop.type == Type.BYTE:
if type(self.value) == list:
new_value = []
diff --git a/tools/dtoc/test/dtoc_test_simple.dts b/tools/dtoc/test/dtoc_test_simple.dts
index b5c1274bb7c..5a6fa88d5cc 100644
--- a/tools/dtoc/test/dtoc_test_simple.dts
+++ b/tools/dtoc/test/dtoc_test_simple.dts
@@ -14,6 +14,7 @@
u-boot,dm-pre-reloc;
compatible = "sandbox,spl-test";
boolval;
+ maybe-empty-int = <>;
intval = <1>;
intarray = <2 3 4>;
byteval = [05];
@@ -42,6 +43,7 @@
compatible = "sandbox,spl-test";
stringarray = "one";
longbytearray = [09 0a 0b 0c 0d 0e 0f 10];
+ maybe-empty-int = <1>;
};
i2c@0 {
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index 44d5d0c354a..752061f27a4 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -299,6 +299,7 @@ struct dtd_sandbox_spl_test {
\tfdt32_t\t\tintarray[3];
\tfdt32_t\t\tintval;
\tunsigned char\tlongbytearray[9];
+\tfdt32_t\t\tmaybe_empty_int[1];
\tunsigned char\tnotstring[5];
\tconst char *\tstringarray[3];
\tconst char *\tstringval;
@@ -358,6 +359,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test = {
\t.intval\t\t\t= 0x1,
\t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10,
\t\t0x11},
+\t.maybe_empty_int\t= {0x0},
\t.notstring\t\t= {0x20, 0x21, 0x22, 0x10, 0x0},
\t.stringarray\t\t= {"multi-word", "message", ""},
\t.stringval\t\t= "message",
@@ -398,6 +400,7 @@ U_BOOT_DRVINFO(spl_test2) = {
static struct dtd_sandbox_spl_test dtv_spl_test3 = {
\t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10,
\t\t0x0},
+\t.maybe_empty_int\t= {0x1},
\t.stringarray\t\t= {"one", "", ""},
};
U_BOOT_DRVINFO(spl_test3) = {
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index 857861c14ed..1119e6b7847 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -122,8 +122,9 @@ class TestFdt(unittest.TestCase):
node = self.dtb.GetNode('/spl-test')
props = self.dtb.GetProps(node)
self.assertEqual(['boolval', 'bytearray', 'byteval', 'compatible',
- 'intarray', 'intval', 'longbytearray', 'notstring',
- 'stringarray', 'stringval', 'u-boot,dm-pre-reloc'],
+ 'intarray', 'intval', 'longbytearray',
+ 'maybe-empty-int', 'notstring', 'stringarray',
+ 'stringval', 'u-boot,dm-pre-reloc'],
sorted(props.keys()))
def testCheckError(self):
@@ -431,6 +432,19 @@ class TestProp(unittest.TestCase):
self.assertEqual(Type.INT, prop.type)
self.assertEqual(3, len(prop.value))
+ # Widen an empty bool to an int
+ prop = self.node.props['maybe-empty-int']
+ prop3 = node3.props['maybe-empty-int']
+ self.assertEqual(Type.BOOL, prop.type)
+ self.assertEqual(True, prop.value)
+ self.assertEqual(Type.INT, prop3.type)
+ self.assertFalse(isinstance(prop.value, list))
+ self.assertEqual(4, len(prop3.value))
+ prop.Widen(prop3)
+ self.assertEqual(Type.INT, prop.type)
+ self.assertTrue(isinstance(prop.value, list))
+ self.assertEqual(1, len(prop.value))
+
def testAdd(self):
"""Test adding properties"""
self.fdt.pack()
--
2.32.0.432.gabb21c7263-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] dtoc: Support widening a bool value
2021-07-29 1:23 [PATCH 0/3] dtoc: Improve support for 'ranges' property Simon Glass
` (2 preceding siblings ...)
2021-07-29 1:23 ` [PATCH 3/3] dtoc: Support widening a bool value Simon Glass
@ 2021-07-31 23:03 ` Simon Glass
2021-07-31 23:03 ` [PATCH 2/3] dtoc: Fix widening an int array to an int Simon Glass
2021-07-31 23:03 ` [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion Simon Glass
5 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-07-31 23:03 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Heiko Schocher, Heinrich Schuchardt, Philippe Reynes,
Vincent Stehlé,
Walter Lozano, U-Boot Mailing List
At present if we see 'ranges' property (with no value) we assume it is a
boolean, as per the devicetree spec.
But another node may define 'ranges' with a value, forcing us to widen it
to an int array. At present this is not supported and causes an error.
Fix this and add some test cases.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reported-by: Tom Rini <trini@konsulko.com>
---
arch/sandbox/dts/sandbox.dtsi | 2 ++
test/dm/of_platdata.c | 3 +++
tools/dtoc/fdt.py | 12 ++++++++++++
tools/dtoc/test/dtoc_test_simple.dts | 2 ++
tools/dtoc/test_dtoc.py | 3 +++
tools/dtoc/test_fdt.py | 18 ++++++++++++++++--
6 files changed, 38 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] dtoc: Fix widening an int array to an int
2021-07-29 1:23 [PATCH 0/3] dtoc: Improve support for 'ranges' property Simon Glass
` (3 preceding siblings ...)
2021-07-31 23:03 ` Simon Glass
@ 2021-07-31 23:03 ` Simon Glass
2021-07-31 23:03 ` [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion Simon Glass
5 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-07-31 23:03 UTC (permalink / raw)
To: Simon Glass; +Cc: Tom Rini, Walter Lozano, U-Boot Mailing List
An int array can hold a single int so we should not need to do anything
in the widening operation. However due to a quirk in the code, an int[3]
widened with an int produced an int[4]. Fix this and add a test.
Fix a comment typo while we are here.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reported-by: Tom Rini <trini@konsulko.com>
---
test/dm/of_platdata.c | 4 +---
tools/dtoc/fdt.py | 15 ++++++++-------
tools/dtoc/test_dtoc.py | 6 +++---
tools/dtoc/test_fdt.py | 11 ++++++++++-
4 files changed, 22 insertions(+), 14 deletions(-)
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
2021-07-29 1:23 [PATCH 0/3] dtoc: Improve support for 'ranges' property Simon Glass
` (4 preceding siblings ...)
2021-07-31 23:03 ` [PATCH 2/3] dtoc: Fix widening an int array to an int Simon Glass
@ 2021-07-31 23:03 ` Simon Glass
5 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2021-07-31 23:03 UTC (permalink / raw)
To: Simon Glass; +Cc: Tom Rini, U-Boot Mailing List
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 <sjg@chromium.org>
---
tools/dtoc/fdt.py | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread