Hi, > On 7 Sep 2022, at 14:37, Michal Orzel <[email protected]> wrote: > > > > On 07/09/2022 15:33, Bertrand Marquis wrote: >> >>> On 7 Sep 2022, at 14:31, Michal Orzel <[email protected]> wrote: >>> >>> >>> >>> On 07/09/2022 15:28, Bertrand Marquis wrote: >>>> >>>> Hi Michal, >>>> >>>>> On 7 Sep 2022, at 14:09, Michal Orzel <[email protected]> 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&data=05%7C01%7Cmichal.orzel%40amd.com%7Cc677a7983cd94e48620708da90d5a15f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637981544487489692%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=1uwtf%2F6shf2PiKu0XZPFNQ%2B6iyhLrMsYb1XEru3IGlg%3D&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. >> >> Agree, someone will need to do a pass on the whole doc which might be easier >> with all things in. >> > Well, not only docs. If we decide to use a single pair of #address-cells and > #size-cells, then > we need to modify the code that expects different properties e.g. > xen,static-mem-{address/size}-cells.
Right I forgot that some parts are already in. So we will need an extra patch to handle those. Bertrand > >> Cheers >> Bertrand >> >>> >>>> >>>> Cheers >>>> Bertrand >>>> >>>>> >>>>>> >>>>>> Cheers, >>>>>> >>>>>> -- >>>>>> Julien Grall
