Re: [PATCH 2/6] dt-bindings: reserved-memory: Support framebuffer reserved memory

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3383 bytes --]

On Tue, Sep 06, 2022 at 09:27:21AM -0500, Rob Herring wrote:
> On Mon, Sep 05, 2022 at 06:32:56PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Document the "framebuffer" compatible string for reserved memory nodes
> > to annotate reserved memory regions used for framebuffer carveouts.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  .../bindings/reserved-memory/framebuffer.yaml | 46 +++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml b/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
> > new file mode 100644
> > index 000000000000..80574854025d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reserved-memory/framebuffer.yaml
> > @@ -0,0 +1,46 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reserved-memory/framebuffer.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: /reserved-memory framebuffer node bindings
> > +
> > +maintainers:
> > +  - devicetree-spec@vger.kernel.org
> > +
> > +allOf:
> > +  - $ref: "reserved-memory.yaml"
> 
> Don't need quotes.
> 
> > +
> > +properties:
> > +  compatible:
> > +    const: framebuffer
> > +    description: >
> > +      This indicates a region of memory meant to be used as a framebuffer for
> > +      a set of display devices. It can be used by an operating system to keep
> > +      the framebuffer from being overwritten and use it as the backing memory
> > +      for a display device (such as simple-framebuffer).
> 
> I'm on the fence whether we need this. It doesn't really add anything 
> because 'simple-framebuffer' will reference this node and you can find 
> it that way. I guess a bootloader may not setup 'simple-framebuffer', 
> but then it should probably not have this node either.
> 
> On the flip side, better to have compatibles than not to identify nodes.

Yeah, I think given some of the comments that Robin Murphy had on the
iommu-addresses thread, having some compatible string to derive more
information from will be useful. I'm not sure if "framebuffer" is the
greatest choice, but it says exactly what this is, so if there are no
better suggestions, I'll stick with that.

Adding Robin in case he has anything to contribute on this side of the
discussion. In retrospect it might have been good to have more overlap
between the recipients of both series.

Thierry

> 
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> 
> Use '/ {' to skip the boilerplate causing the error.
> 
> > +      chosen {
> > +        framebuffer {
> > +          compatible = "simple-framebuffer";
> > +          memory-region = <&fb>;
> > +        };
> > +      };
> > +
> > +      reserved-memory {
> > +          #address-cells = <1>;
> > +          #size-cells = <1>;
> > +          ranges;
> > +
> > +          fb: framebuffer@80000000 {
> > +              compatible = "framebuffer";
> > +              reg = <0x80000000 0x007e9000>;
> > +          };
> > +      };
> > +
> > +...
> > -- 
> > 2.37.2
> > 
> > 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]