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

[Julien Grall <julien@xxxxxxx>]

On 07/09/2022 13:12, Bertrand Marquis wrote:
> Hi,
Hi Bertrand,

> > On 7 Sep 2022, at 12:48, Henry Wang <Henry.Wang@xxxxxxx> wrote:
> >
> > Hi Julien,
> >
> > > -----Original Message-----
> > > From: Julien Grall <julien@xxxxxxx>
> > > Subject: Re: [PATCH v3 2/4] docs, xen/arm: Introduce static heap memory
> > >
> > > 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.
> The more I dig, the less I find a use case where we could need different values 
> here.
This is what I thought as well..

> Maybe just:
> #xen,address-cells = <2>
> #xen,size-cells = <2>
> Could be enough. If some parameter needs a different value it could introduce a 
> specific name.
I think '#xen,...' is ambiguous because it doesn't tell you whether it 
applies to the memory range or interrupt range. So I would got with
> Or maybe just memory-address-cells and memory-size-cells if we see a 
> possibility to require a different value for an other address or size.
"#xen,memory-*".

That said, any reason to not reuse #address-cells and #size-cells here?

Cheers,

--
Julien Grall