Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
MIME-Version: 1.0
References: <20230921124459.1.I91ddcfacf9b234af5cc3eabea4b62edb31153317@changeid>
 <CAL_Jsq+WuYDU+yY98opTHr1PT-J9mFYJQBjVMnk+FSWLDUO33w@mail.gmail.com>
 <CAPnjgZ1pfxaMG1n5yOBhiOhsNrRjck1K92U7Ga=+VTY_jjjrVg@mail.gmail.com> <20230922174649.GA3320366-robh@kernel.org>
In-Reply-To: <20230922174649.GA3320366-robh@kernel.org>
From:   Simon Glass <sjg@chromium.org>
Date:   Fri, 22 Sep 2023 12:12:20 -0600
Message-ID: <CAPnjgZ3ojfAv=BHqOhM=-NnYqCm81Ny=PsGKiNphKTmw++fk9w@mail.gmail.com>
Subject: Re: [PATCH] dt-bindings: mtd: Add a schema for binman
To:     Rob Herring <robh@kernel.org>
Cc:     devicetree@vger.kernel.org,
        U-Boot Mailing List <u-boot@lists.denx.de>,
        linux-mtd@lists.infradead.org, Tom Rini <trini@konsulko.com>,
        Conor Dooley <conor+dt@kernel.org>,
        Dhruva Gole <d-gole@ti.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>,
        Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Rob,

On Fri, 22 Sept 2023 at 11:46, Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Sep 22, 2023 at 11:01:18AM -0600, Simon Glass wrote:
> > Hi Rob,
> >
> > On Fri, 22 Sept 2023 at 10:00, Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Thu, Sep 21, 2023 at 1:45=E2=80=AFPM Simon Glass <sjg@chromium.org=
> wrote:
> > > >
> > > > Binman[1] is a tool for creating firmware images. It allows you to
> > > > combine various binaries and place them in an output file.
> > > >
> > > > Binman uses a DT schema to describe an image, in enough detail that
> > > > it can be automatically built from component parts, disassembled,
> > > > replaced, listed, etc.
> > > >
> > > > Images are typically stored in flash, which is why this binding is
> > > > targeted at mtd. Previous discussion is at [2] [3].
> > > >
> > > > [1] https://u-boot.readthedocs.io/en/stable/develop/package/binman.=
html
> > > > [2] https://lore.kernel.org/u-boot/20230821180220.2724080-3-sjg@chr=
omium.org/
> > > > [3] https://www.spinics.net/lists/devicetree/msg626149.html
> > >
> > > You missed:
> > >
> > > https://github.com/devicetree-org/dt-schema/pull/110
> > >
> > > where I said: We certainly shouldn't duplicate the existing partition=
s
> > > bindings. What's missing from them (I assume we're mostly talking
> > > about "fixed-partitions" which has been around forever I think (befor=
e
> > > me))?
> > >
> > > To repeat, unless there is some reason binman partitions conflict wit=
h
> > > fixed-partitions, you need to start there and extend it. From what's
> > > posted here, it neither conflicts nor needs extending.
> >
> > I think at this point I am just hopelessly confused. Have you taken a
> > look at the binman schema? [1]
>
> Why do I need to? That's used for some tool and has nothing to do with a
> device's DTB. However, I thought somewhere in this discussion you showed
> it under a flash device node.

Yes, that is the intent (under a flash node).

> Then I care because then it overlaps with
> what we already have for partitions. If I misunderstood that, then just
> put your schema with your tool. Only users of the tool should care about
> the tool's schema.

OK. I believe that binman will fit into both camps, since its input is
not necessarily fully formed. E.g. if you don't specify the offset of
an entry, then it will be packed automatically. But the output is
fully formed, in that Binman now knows the offset so can write it to
the DT.

>
> >
> > I saw this file, which seems to extend a partition.
> >
> > Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partition=
s.yaml
>
> IIRC, that's a different type where partition locations are stored in
> the flash, so we don't need location and size in DT.

OK.

>
> >
> > I was assuming that I should create a top-level compatible =3D "binman"
> > node, with subnodes like compatible =3D "binman,bl31-atf", for example.
> > I should use the compatible string to indicate the contents, right?
>
> Yes for subnodes, and we already have some somewhat standard ones for
> "u-boot" and "u-boot-env". Though historically, "label" was used.

Binman has common properties for all entries, including "compress"
which sets the compression algorithm.

So perhaps I should start by defining a new binman,bl31-atf which has
common properties from an "binman,entry" definition?

>
> Top-level, meaning the root of the DT? That sound like just something
> for the tool, so I don't care, but it doesn't belong in the DTB.

Sorry, I mean 'top-level' with respect to the partitions.

>
> >
> > Re extending, what is the minimum I can do? Are you looking for
> > something like a "compress" property that indicates that the entry is
> > compressed?
> >
> > I'm really just a bit lost.
>
> Me too.

Regards,
Simon