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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi,

> On 7 Sep 2022, at 13:45, Julien Grall <julien@xxxxxxx> wrote:
> 
> 
> 
> On 07/09/2022 13:41, Michal Orzel wrote:
>> On 07/09/2022 14:32, Julien Grall wrote:
>>> [CAUTION: External Email]
>>> 
>>> On 07/09/2022 13:12, Michal Orzel wrote:
>>>> Hi Julien,
>>> 
>>> Hi Michal,
>>> 
>>>> On 07/09/2022 13:36, Julien Grall wrote:
>>>>> 
>>>>> 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.
>>>>> 
>>>>> I am open to suggestion here. My only request is we are consistent (i.e.
>>>>> this doesn't depend on who wrote the bindings).
>>>>> 
>>>> In my opinion we should follow the device tree specification which states
>>>> that the #address-cells and #size-cells correspond to the reg property.
>>> 
>>> Hmmm.... Looking at [1], the two properties are not exclusive to 'reg'
>>> Furthermore, I am not aware of any restriction for us to re-use them. Do
>>> you have a pointer?
>> As we are discussing re-usage of #address-cells and #size-cells for custom 
>> properties that are not "reg",
>> I took this info from the latest device tree specs found under 
>> https://www.devicetree.org/specifications/:
>> "The #address-cells property defines the number of <u32> cells used to 
>> encode the address field in a child node's reg property"
>> and
>> "The #size-cells property defines the number of <u32> cells used to encode 
>> the size field in a child node’s reg property"
> 
> Right. But there is nothing in the wording suggesting that #address-cells and 
> #size-cells can't be re-used. From [1], it is clear that the meaning has 
> changed.
> 
> So why can't we do the same?

I agree here, those are used for how reg is encoded but nothing says that we 
cannot reuse them for the encoding of something else.
Even if we do not use “reg” for those sets, they are still defining memory 
addresses and sizes which is coherent.

In some cases reg is used to encode something different so those could have 
different values that we could not use but for the chosen node, they should 
always set the encoding for something addressing a memory area.

So if we have a good reason then I would vote for xen,memory-* proposal but so 
far I could not find a reason not to use the standard ones.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall