Re: [PATCH v3 2/4] docs, xen/arm: Introduce static heap memory

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi,

> On 7 Sep 2022, at 12:48, Henry Wang <Henry.Wang@xxxxxxx> wrote:
> 
> Hi Julien,
> 
>> -----Original Message-----
>> From: Julien Grall <julien@xxxxxxx>
>> Subject: Re: [PATCH v3 2/4] docs, xen/arm: Introduce static heap memory
>> 
>> Hi Henry,
>> 
>> While reviewing the binding sent by Penny I noticed some inconsistency
>> with the one you introduced. See below.
>> 
>> On 07/09/2022 09:36, Henry Wang wrote:
>>> +- xen,static-heap
>>> +
>>> +    Property under the top-level "chosen" node. It specifies the address
>>> +    and size of Xen static heap memory. Note that at least a 64KB
>>> +    alignment is required.
>>> +
>>> +- #xen,static-heap-address-cells and #xen,static-heap-size-cells
>>> +
>>> +    Specify the number of cells used for the address and size of the
>>> +    "xen,static-heap" property under "chosen".
>>> +
>>> +Below is an example on how to specify the static heap in device tree:
>>> +
>>> +    / {
>>> +        chosen {
>>> +            #xen,static-heap-address-cells = <0x2>;
>>> +            #xen,static-heap-size-cells = <0x2>;
>> 
>> Your binding, is introduce #xen,static-heap-{address, size}-cells
>> whereas Penny's one is using #{address, size}-cells even if the property
>> is not "reg".
>> 
>> I would like some consistency in the way we define bindings. Looking at
>> the tree, we already seem to have introduced
>> #xen-static-mem-address-cells. So maybe we should follow your approach?
>> 
>> That said, I am wondering whether we should just use one set of property
>> name.

The more I dig, the less I find a use case where we could need different values 
here.
Maybe just:
#xen,address-cells = <2>
#xen,size-cells = <2>
Could be enough. If some parameter needs a different value it could introduce a 
specific name.

Or maybe just memory-address-cells and memory-size-cells if we see a 
possibility to require a different value for an other address or size.

This would also make life easier for users as we could just always put those 2 
in our examples.

Cheers
Bertrand

> 
> IMO now we have the pair
> #xen,static-heap-{address, size}-cells and xen,static-heap for static heap.
> and the pair
> #xen,static-mem-{address, size}-cells and xen,static-mem for static
> memory allocation for dom0less.
> 
> So at least these two are consistent.
> 
> I guess the concern you raised is related to the static shared memory for
> dom0less,
> ...
> 
>> 
>> I am open to suggestion here. My only request is we are consistent (i.e.
>> this doesn't depend on who wrote the bindings).
> 
> I am not sure if Penny and Stefano have some specific requirements
> regarding the static shared memory usage. So I will wait for Stefano's input.
> But yeah we need to keep the consistency so if we are agreed that I need to
> change the binding, I will do the corresponding change.
> 
> Kind regards,
> Henry
> 
>> 
>> Cheers,
>> 
>> --
>> Julien Grall