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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Michal,

> On 7 Sep 2022, at 14:09, Michal Orzel <michal.orzel@xxxxxxx> wrote:
> 
> 
> 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.

I think we want in general to reduce complexity when possible.
Here we are adding a lot of entries in the device tree where we know that
in all cases having only 2 will work all the time.

I am not convinced by the arguments on not using #address-cells and will
leave that one to Stefano

But in any case we should only add one pair here for sure, as you say the
only implication is to add a couple of 0 in the worst case.

Cheers
Bertrand

> 
>> 
>> Cheers,
>> 
>> --
>> Julien Grall