Hi,
> On 7 Sep 2022, at 12:48, Henry Wang <[email protected]> wrote:
>
> Hi Julien,
>
>> -----Original Message-----
>> From: Julien Grall <[email protected]>
>> 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