On 30/08/2022 08:29, Michal Orzel wrote:
> Hi Henry,
>
> On 30/08/2022 02:58, Henry Wang wrote:
>>
>> Hi Stefano and Michal,
>>
>>> -----Original Message-----
>>> From: Stefano Stabellini <sstabell...@kernel.org>
>>> Sent: Tuesday, August 30, 2022 8:47 AM
>>> To: Henry Wang <henry.w...@arm.com>
>>> Cc: Michal Orzel <michal.or...@amd.com>; xen-devel@lists.xenproject.org;
>>> Stefano Stabellini <sstabell...@kernel.org>; Julien Grall <jul...@xen.org>;
>>> Bertrand Marquis <bertrand.marq...@arm.com>; Wei Chen
>>> <wei.c...@arm.com>; Volodymyr Babchuk
>>> <volodymyr_babc...@epam.com>; Penny Zheng <penny.zh...@arm.com>
>>> Subject: RE: [PATCH 1/2] docs, xen/arm: Introduce reserved heap memory
>>>
>>> On Thu, 25 Aug 2022, Henry Wang wrote:
>>>>>> const char *name,
>>>>>> u32 address_cells, u32
>>>>>> size_cells)
>>>>>> {
>>>>>> @@ -301,16 +303,40 @@ static void __init process_chosen_node(const
>>>>> void *fdt, int node,
>>>>>> paddr_t start, end;
>>>>>> int len;
>>>>>>
>>>>>> + if ( fdt_get_property(fdt, node, "xen,static-mem", NULL) )
>>>>>> + {
>>>>>> + u32 address_cells = device_tree_get_u32(fdt, node,
>>>>>> +
>>>>>> "#xen,static-mem-address-cells",
>>>>>> + 0);
>>>>>> + u32 size_cells = device_tree_get_u32(fdt, node,
>>>>>> +
>>>>>> "#xen,static-mem-size-cells", 0);
>>>>>> + int rc;
>>>>>> +
>>>>>> + printk("Checking for reserved heap in /chosen\n");
>>>>>> + if ( address_cells < 1 || size_cells < 1 )
>>>>> address_cells and size_cells cannot be negative so you could just check if
>>>>> there are 0.
>>>>
>>>> In bootfdt.c function device_tree_get_meminfo(), the address and size cells
>>>> are checked using <1 instead of =0. I agree they cannot be negative, but I
>>> am
>>>> not very sure if there were other reasons to do the "<1" check in
>>>> device_tree_get_meminfo(). Are you fine with we don't keep the
>>> consistency
>>>> here?
>>>
>>> I would keep the < 1 check but it doesn't make much difference either
>>> way
>>
>> I also would prefer to keep these two places consistent and I agree Michal is
>> making a good point.
> I'm ok with that so let's keep the consistency.
Actually, why do we want to duplicate exactly the same check in
process_chosen_node that is already
present in device_tree_get_meminfo? There is no need for that so just remove it
from process_chosen_node.
>
>>
>> Kind regards,
>> Henry
>>
>
> ~Michal
