Hi Michal,
> -----Original Message-----
> From: Michal Orzel <michal.or...@amd.com>
> >>>>>> + 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.
Well, yes and no IMHO, because we are using "#xen,static-heap-address-cells"
and "#xen,static-heap-size-cells" instead of normal "#address-cells" and
"#size-cells". These properties are dependent on user's input so I would say
adding a check and proper printk to inform user with the related information
would be a good idea. Also I think catching the incorrect
"#xen,static-heap-address-cells" and "#xen,static-heap-size-cells" and return
early would also be a good idea.
Kind regards,
Henry
