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

[Michal Orzel <michal.orzel@xxxxxxx>]

On 07/09/2022 15:28, Bertrand Marquis wrote:
> 
> 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%7C83da1eb9d32441cb9e8108da90d4f2d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637981541539851438%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=3M9aT3LjCEOhZUHWSbgSSmKppY1Wion4TT3BeKLnWSo%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.
I agree. The only drawback is the need to modify the already introduced 
properties
to be coherent.

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