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

[Michal Orzel <michal.orzel@amd.com>]

On 07/09/2022 14:45, Julien Grall 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.devicetree.org%2Fspecifications%2F&amp;data=05%7C01%7Cmichal.orzel%40amd.com%7C4f35e9f93b7443ac73c808da90cecc22%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637981515122993111%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=TiESYS6RXdiPLX8WFUV0CsztAvK7mHSud%2B0xoJqwAw0%3D&amp;reserved=0:
>> "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 think this is a matter of how someone reads these sentences.
I do not think that such documents need to state:
"This property is for the reg. Do not use it for other purposes."
The first part of the sentence is enough to inform what is supported.

On the other hand, looking at [1] these properties got new purposes
so I think we could do the same. Now the question is whether we want that.
I think it is doable to just have a single pair of #address/#size properties.
For instance xen,shared-mem requiring just 0x1 for address/size
and reg requiring 0x2. This would just imply putting additional 0x00.

> 
> Cheers,
> 
> --
> Julien Grall